summaryrefslogtreecommitdiff
path: root/gdb
Commit message (Collapse)AuthorAgeFilesLines
* gdb: make target_ops::follow_fork return voidSimon Marchi2021-04-0712-40/+44
| | | | | | | | | | | | | | | | | | | | | | | | | I noticed that all implementations return false, so target_ops::follow_fork doesn't really need to return a value. Change it to return void. gdb/ChangeLog: * target.h (struct target_ops) <follow_fork>: Return void. (target_follow_fork): Likewise. * target.c (default_follow_fork): Likewise. (target_follow_fork): Likewise. * infrun.c (follow_fork_inferior): Adjust. * fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Return void. * fbsd-nat.c (fbsd_nat_target:::follow_fork): Likewise. * linux-nat.h (class linux_nat_target) <follow_fork>: Likewise. * linux-nat.c (linux_nat_target::follow_fork): Return void. * obsd-nat.h (class obsd_nat_target) <follow_fork>: Return void. * obsd-nat.c (obsd_nat_target::follow_fork): Likewise. * remote.c (class remote_target) <follow_fork>: Likewise. (remote_target::follow_fork): Likewise. * target-delegates.c: Re-generate. Change-Id: If908c2f68b29fa275be2b0b9deb41e4c6a1b7879
* CTF: handle forward reference typeWeimin Pan2021-04-075-13/+91
| | | | | | | | | | | | | | | | | | Added function fetch_tid_type which calls get_tid_type and will set up the type, associated with a tid, if it is not read in yet. Also implement function read_forward_type which handles the CTF_K_FORWARD kind. Expanded gdb.base/ctf-ptype.exp to add cases with forward references. gdb/ChangeLog: * ctfread.c (fetch_tid_type): New function, use throughout file. (read_forward_type): New function. (read_type_record): Call read_forward_type. gdb/testsuite/ChangeLog: * gdb.base/ctf-ptype.c: Add struct link containing a forward reference type. * gdb.base/ctf-ptype.exp: Add "ptype struct link".
* gdb/fortran: handle dynamic types within arrays and structuresAndrew Burgess2021-04-077-4/+333
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit replaces this patch: https://sourceware.org/pipermail/gdb-patches/2021-January/174933.html which was itself a replacement for this patch: https://sourceware.org/pipermail/gdb-patches/2020-July/170335.html The motivation behind the original patch can be seen in the new test, which currently gives a GDB session like this: (gdb) ptype var8 type = Type type6 PTR TO -> ( Type type2 :: ptr_1 ) PTR TO -> ( Type type2 :: ptr_2 ) End Type type6 (gdb) ptype var8%ptr_2 type = PTR TO -> ( Type type2 integer(kind=4) :: spacer Type type1, allocatable :: t2_array(:) <------ Issue #1 End Type type2 ) (gdb) ptype var8%ptr_2%t2_array Cannot access memory at address 0x38 <------ Issue #2 (gdb) Issue #1: Here we see the abstract dynamic type, rather than the resolved concrete type. Though in some cases the user might be interested in the abstract dynamic type, I think that in most cases showing the resolved concrete type will be of more use. Plus, the user can always figure out the dynamic type (by source code inspection if nothing else) given the concrete type, but it is much harder to figure out the concrete type given only the dynamic type. Issue #2: In this example, GDB evaluates the expression in EVAL_AVOID_SIDE_EFFECTS mode (due to ptype). The value returned for var8%ptr_2 will be a non-lazy, zero value of the correct dynamic type. However, when GDB asks about the type of t2_array this requires GDB to access the value of var8%ptr_2 in order to read the dynamic properties. As this value was forced to zero (thanks to the use of EVAL_AVOID_SIDE_EFFECTS) then GDB ends up accessing memory at a base of zero plus some offset. Both this patch, and my previous two attempts, have all tried to resolve this problem by stopping EVAL_AVOID_SIDE_EFFECTS replacing the result value with a zero value in some cases. This new patch is influenced by how Ada handles its tagged typed. There are plenty of examples in ada-lang.c, but one specific case is ada_structop_operation::evaluate. When GDB spots that we are dealing with a tagged (dynamic) type, and we're in EVAL_AVOID_SIDE_EFFECTS mode, then GDB re-evaluates the child operation in EVAL_NORMAL mode. This commit handles two cases like this specifically for Fortran, a new fortran_structop_operation, and the already existing fortran_undetermined, which is where we handle array accesses. In these two locations we spot when we are dealing with a dynamic type and re-evaluate the child operation in EVAL_NORMAL mode so that we are able to access the dynamic properties of the type. The rest of this commit message is my attempt to record why my previous patches failed. To understand my second patch, and why it failed lets consider two expressions, this Fortran expression: (gdb) ptype var8%ptr_2%t2_array --<A> Operation: STRUCTOP_STRUCT --(1) Operation: STRUCTOP_STRUCT --(2) Operation: OP_VAR_VALUE --(3) Symbol: var8 Block: 0x3980ac0 String: ptr_2 String: t2_array And this C expression: (gdb) ptype ptr && ptr->a == 3 --<B> Operation: BINOP_LOGICAL_AND --(4) Operation: OP_VAR_VALUE --(5) Symbol: ptr Block: 0x45a2a00 Operation: BINOP_EQUAL --(6) Operation: STRUCTOP_PTR --(7) Operation: OP_VAR_VALUE --(8) Symbol: ptr Block: 0x45a2a00 String: a Operation: OP_LONG --(9) Type: int Constant: 0x0000000000000003 In expression <A> we should assume that t2_array is of dynamic type. Nothing has dynamic type in expression <B>. This is how GDB currently handles expression <A>, in all cases, EVAL_AVOID_SIDE_EFFECTS or EVAL_NORMAL, an OP_VAR_VALUE operation always returns the real value of the symbol, this is not forced to a zero value even in EVAL_AVOID_SIDE_EFFECTS mode. This means that (3), (5), and (8) will always return a real lazy value for the symbol. However a STRUCTOP_STRUCT will always replace its result with a non-lazy, zero value with the same type as its result. So (2) will lookup the field ptr_2 and create a zero value with that type. In this case the type is a pointer to a dynamic type. Then, when we evaluate (1) to figure out the resolved type of t2_array, we need to read the types dynamic properties. These properties are stored in memory relative to the objects base address, and the base address is in var8%ptr_2, which we already figured out has the value zero. GDB then evaluates the DWARF expressions that take the base address, add an offset and dereference. GDB then ends up trying to access addresses like 0x16, 0x8, etc. To fix this, I proposed changing STRUCTOP_STRUCT so that instead of returning a zero value we instead returned the actual value representing the structure's field in the target. My thinking was that GDB would not try to access the value's contents unless it needed it to resolve a dynamic type. This belief was incorrect. Consider expression <B>. We already know that (5) and (8) will return real values for the symbols being referenced. The BINOP_LOGICAL_AND, operation (4) will evaluate both of its children in EVAL_AVOID_SIDE_EFFECTS in order to get the types, this is required for C++ operator lookup. This means that even if the value of (5) would result in the BINOP_LOGICAL_AND returning false (say, ptr is NULL), we still evaluate (6) in EVAL_AVOID_SIDE_EFFECTS mode. Operation (6) will evaluate both children in EVAL_AVOID_SIDE_EFFECTS mode, operation (9) is easy, it just returns a value with the constant packed into it, but (7) is where the problem lies. Currently in GDB this STRUCTOP_STRUCT will always return a non-lazy zero value of the correct type. When the results of (7) and (9) are back in the BINOP_LOGICAL_AND operation (6), the two values are passed to value_equal which performs the comparison and returns a result. Note, the two things compared here are the immediate value (9), and a non-lazy zero value from (7). However, with my proposed patch operation (7) no longer returns a zero value, instead it returns a lazy value representing the actual value in target memory. When we call value_equal in (6) this code causes GDB to try and fetch the actual value from target memory. If `ptr` is NULL then this will cause GDB to access some invalid address at an offset from zero, this will most likely fail, and cause GDB to throw an error instead of returning the expected type. And so, we can now describe the problem that we're facing. The way GDB's expression evaluator is currently written we assume, when in EVAL_AVOID_SIDE_EFFECTS mode, that any value returned from a child operation can safely have its content read without throwing an error. If child operations start returning real values (instead of the fake zero values), then this is simply not true. If we wanted to work around this then we would need to rewrite almost all operations (I would guess) so that EVAL_AVOID_SIDE_EFFECTS mode does not cause evaluation of an operation to try and read the value of a child operation. As an example, consider this current GDB code from eval.c: struct value * eval_op_equal (struct type *expect_type, struct expression *exp, enum noside noside, enum exp_opcode op, struct value *arg1, struct value *arg2) { if (binop_user_defined_p (op, arg1, arg2)) { return value_x_binop (arg1, arg2, op, OP_NULL, noside); } else { binop_promote (exp->language_defn, exp->gdbarch, &arg1, &arg2); int tem = value_equal (arg1, arg2); struct type *type = language_bool_type (exp->language_defn, exp->gdbarch); return value_from_longest (type, (LONGEST) tem); } } We could change this function to be this: struct value * eval_op_equal (struct type *expect_type, struct expression *exp, enum noside noside, enum exp_opcode op, struct value *arg1, struct value *arg2) { if (binop_user_defined_p (op, arg1, arg2)) { return value_x_binop (arg1, arg2, op, OP_NULL, noside); } else { struct type *type = language_bool_type (exp->language_defn, exp->gdbarch); if (noside == EVAL_AVOID_SIDE_EFFECTS) return value_zero (type, VALUE_LVAL (arg1)); else { binop_promote (exp->language_defn, exp->gdbarch, &arg1, &arg2); int tem = value_equal (arg1, arg2); return value_from_longest (type, (LONGEST) tem); } } } Now we don't call value_equal unless we really need to. However, we would need to make the same, or similar change to almost all operations, which would be a big task, and might not be a direction we wanted to take GDB in. So, for now, I'm proposing we go with the more targeted, Fortran specific solution, that does the minimal required in order to correctly resolve the dynamic types. gdb/ChangeLog: * f-exp.h (class fortran_structop_operation): New class. * f-exp.y (exp): Create fortran_structop_operation instead of the generic structop_operation. * f-lang.c (fortran_undetermined::evaluate): Re-evaluate expression as EVAL_NORMAL if the result type was dynamic so we can extract the actual array bounds. (fortran_structop_operation::evaluate): New function. gdb/testsuite/ChangeLog: * gdb.fortran/dynamic-ptype-whatis.exp: New file. * gdb.fortran/dynamic-ptype-whatis.f90: New file.
* gdb: allow casting to rvalue reference in more casesAndrew Burgess2021-04-075-2/+61
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It is not currently possible to cast some values to an rvaule reference. This happens when simple scalar values are cast to an rvalue reference of the same type, e.g.: int global_var; Then in GDB: (gdb) p static_cast<int&&> (global_var) Attempt to take address of value not located in memory. Which is clearly silly. The problem is that as part of the cast an intermediate value is created within GDB that becomes an lval_none rather than the original lval_memory. The casting logic basically goes like this: The call tree that leads to the error looks like this: value_cast value_cast value_ref value_addr error The first value_cast call is casting the value for 'global_var' to type 'int&&'. GDB spots that the target type is a reference, and so calls value_cast again, this time casting 'global_var' to type 'int'. We then call value_ref to convert the result of the second value_cast into a reference. Unfortunately, the second cast results in the value (for global_var) changing from an lval_memory to an lval_none. This is because int to int casting calls extract_unsigned_integer and then value_from_longest. In theory value_cast has a check at its head that should help in this case, the code is: if (value_type (arg2) == type) return arg2; However, this only works in some cases. In our case 'value_type (arg2)' will be an objfile owned type, while the type from the expression parser 'int&&' will be gdbarch owned. The pointers will not be equal, but the meaning of the type will be equal. I did consider making the int to int casting case smarter, but this obviously is only one example. We must also consider things like float to float, or pointer to pointer.... So, I instead decided to try and make the initial check smarter. Instead of a straight pointer comparison, I now propose that we use types_deeply_equal. If this is true then we are casting something back to its current type, in which case we can preserve the lval setting by using value_copy. gdb/ChangeLog: * valops.c (value_cast): Call value_deeply_equal before performing any cast. gdb/testsuite/ChangeLog: * gdb.cp/rvalue-ref-params.cc (f3): New function. (f4): New function. (global_int): New global variable. (global_float): Likeiwse. (main): Call both new functions. * gdb.cp/rvalue-ref-params.exp: Add new tests.
* gdb: move cheap pointer equality check earlier in types_equalAndrew Burgess2021-04-072-4/+9
| | | | | | | | | | | | | | | | | | | | | I noticed that in types equal we start with a cheap pointer equality check, then resolve typedefs, then do a series of (semi-)expensive checks, including checking type names, before, finally performing another pointer equality check. We should hoist the second pointer equality check to immediately after we have resolved typedefs. This would save performing the more expensive checks. This isn't going to give any noticable performance improvement, I just spotted this in passing and figured I might as well commit the fix. There should be no user visible changes after this commit. gdb/ChangeLog: * gdbtypes.c (types_equal): Move pointer equality check earlier in the function.
* gdb: handle relative paths to DWO filesCaroline Tice2021-04-075-0/+175
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | DWARF allows .dwo file paths to be relative rather than absolute. When they are relative, DWARF uses DW_AT_comp_dir to find the .dwo file. DW_AT_comp_dir can also be relative, making the entire search patch for the .dwo file relative. In this case, GDB currently searches relative to its current working directory, i.e. the directory from which the debugger was launched, but not relative to the directory containing the built binary. This cannot be right, as the compiler, when generating the relative paths, knows where it's building the binary but can have no idea where the debugger will be launched. The correct thing is to add the directory containing the binary to the search paths used for resolving relative locations of dwo files. That is what this patch does. gdb/ChangeLog: * dwarf2/read.c (try_open_dwop_file): Add path for the binary to the search paths used resolve relative location of .dwo file. gdb/testsuite/ChangeLog: * gdb.dwarf2/fission-relative-dwo.c: New file. * gdb.dwarf2/fission-relative-dwo.exp: New file.
* gdb/testsuite: fix fission support in the Dwarf assemblerAndrew Burgess2021-04-0711-446/+620
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit fixes fission support in the Dwarf assembler. I added the new test gdb.dwarf2/fission-absolute-dwo.exp which is a simple example of using the fission support. I also rewrote the existing test gdb.dwarf2/fission-multi-cu.exp to use the new functionality (instead of using an x86-64 only assembler file). To better support compiling the assembler files produced by the Dwarf assembler I have added the new proc build_executable_and_dwo_files in lib/dwarf.exp, this replaces build_executable_from_fission_assembler, all the tests that used the old proc have been updated. Where the old proc assumed a single .S source file which contained the entire test, the new proc allows for multiple source files. The Dwarf assembler already had some fission support, however, this was not actually used in any tests, and when I tried using it there were a few issues. The biggest change is that we now generate DW_FORM_GNU_addr_index instead of DW_FORM_addr for the low and high pc in _handle_macro_at_range, support for the DW_FORM_GNU_addr_index is new in this commit. gdb/testsuite/ChangeLog: * gdb.dwarf2/fission-absolute-dwo.c: New file. * gdb.dwarf2/fission-absolute-dwo.exp: New file. * gdb.dwarf2/fission-base.exp: Use build_executable_and_dwo_files instead of build_executable_from_fission_assembler. * gdb.dwarf2/fission-loclists-pie.exp: Likewise. * gdb.dwarf2/fission-loclists.exp: Likewise.
* gdb: Handle missing .debug_str sectionAndrew Burgess2021-04-074-2/+63
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While messing with the Dwarf assembler (gdb/testsuite/lib/dwarf.exp) I managed to create an ELF which made use of DW_FORM_strp, but didn't include a .debug_str section. When I started GDB on this ELF, GDB crashed. I would have expected to get an error instead. I tracked this down to an unfortunate design choice in dwarf2_section_info, a class which wraps around a bfd section, and is used for reading in debug information. GBB creates many dwarf2_section_info objects, one for each debug section that might need to be read, then as we find the input bfd sections we associate them with the corresponding dwarf2_section_info. If no matching input bfd section is found then the dwarf2_section_info is left in an unassociated state, its internal bfd section pointer is null. If later GDB tries to read content from the dwarf2_section_info, for example, which trying to read the string associated with DW_FORM_strp, we spot that there is no associated bfd section and issue an error message. To make the users life easier, the error message includes the section name being looked for, and the bfd from which the section was obtained. However, we get the section name by calling bfd_section_name on the associated section, and we get the bfd filename by calling bfd_get_filename on the owner of the associated section. Of course, if there is no associated section then both the calls bfd_section_name and dwarf2_section_info::get_bfd_owner will result in undefined behaviour (e.g. a crash). The solution I propose in this patch is, I know, not ideal. I simply spot the case where there is no associated section, and print a simpler error message, leaving out the section name and filename. A better solution would involve redesigning dwarf2_section_info, we could associate each dwarf2_section_info with the initial bfd being parsed. We would then display this filename if there's nothing better to display (e.g. if we find a section in a dwo/dwp split dwarf file then we would probably use that filename in preference). Each dwarf2_section_info could also have the concept of the default section name that would be read for that section, for example, string data might appear in ".debug_str" or ".zdebug_str", but if neither is found, then it would probably be OK to just say ".debug_str" is missing. Anyway, I didn't do any of that redesign, I just wanted to stop GDB crashing for now, so instead we get this: Dwarf Error: DW_FORM_strp used without required section Which isn't the best, but in context, isn't too bad: Reading symbols from /path/to/executable... Dwarf Error: DW_FORM_strp used without required section (No debugging symbols found in /path/to/executable) I also added some asserts into dwarf2_section_info which should trigger before GDB crashes in future, if we trigger any other bad paths through this code. And there's a test for the specific issue I hit. gdb/ChangeLog: * dwarf2/section.c (dwarf2_section_info::get_bfd_owner): Add an assert. (dwarf2_section_info::get_file_name): Add an assert. (dwarf2_section_info::read_string): Display a minimal, sane error when the dwarf2_section_info is not associated with a bfd section. gdb/testsuite/ChangeLog: * gdb.dwarf2/dw2-using-debug-str.exp: Add an additional test.
* gdb/py: fix gdb.parameter('data-directory')Andrew Burgess2021-04-074-2/+78
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It was reported on IRC that using gdb.parameter('data-directory') doesn't work correctly. The problem is that the data directory is stored in 'gdb_datadir', however the set/show command is associated with a temporary 'staged_gdb_datadir'. When the user does 'set data-directory VALUE', the VALUE is stored in 'staged_gdb_datadir' by GDB, then set_gdb_datadir is called. This in turn calls set_gdb_data_directory to copy the value from staged_gdb_datadir into gdb_datadir. However, set_gdb_data_directory will resolve relative paths, so the value stored in gdb_datadir might not match the value in staged_gdb_datadir. The Python gdb.parameter API fetches the parameter values by accessing the variable associated with the show command, so in this case staged_gdb_datadir. This causes two problems: 1. Initially staged_gdb_datadir is NULL, and remains as such until the user does 'set data-directory VALUE' (which might never happen), but gdb_datadir starts with GDB's default data-directory value. So initially from Python gdb.parameter('data-directory') will return the empty string, even though at GDB's CLI 'show data-directory' prints a real path. 2. If the user does 'set data-directory ./some/relative/path', GDB will resolve the relative path, thus, 'show data-directory' at the CLI will print an absolute path. However, the value is staged_gdb_datadir will still be the relative path, and gdb.parameter('data-directory') from Python will return the relative path. In this commit I fix both of these issues by: 1. Initialising the value in staged_gdb_datadir based on the initial value in gdb_datadir, and 2. In set_gdb_datadir, after calling set_gdb_data_directory, I copy the value in gdb_datadir back into staged_gdb_datadir. With these two changes in place the value in staged_gdb_datadir should always match the value in gdb_datadir, and accessing data-directory from Python should now work correctly. gdb/ChangeLog: * top.c (staged_gdb_datadir): Update comment. (set_gdb_datadir): Copy the value of gdb_datadir back into staged_datadir. (init_main): Initialise staged_gdb_datadir. gdb/testsuite/ChangeLog: * gdb.python/py-parameter.exp: Add test for reading data-directory using gdb.parameter API.
* [gdb/breakpoints] Workaround missing line-table entryTom de Vries2021-04-064-16/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When running test-case gdb.opt/inline-cmds.exp, we run into this KFAIL with gcc: ... Breakpoint 7, main () at gdb.opt/inline-cmds.c:71^M 71 result = 0; /* set breakpoint 3 here */^M (gdb) PASS: gdb.opt/inline-cmds.exp: continue to breakpoint: consecutive func1 next^M 73 func1 (); /* first call */^M (gdb) PASS: gdb.opt/inline-cmds.exp: next to first func1 next^M 75 marker ();^M (gdb) KFAIL: gdb.opt/inline-cmds.exp: next to second func1 (PRMS: gdb/25884) ... while with clang we have instead: ... next^M 74 func1 (); /* second call */^M (gdb) PASS: gdb.opt/inline-cmds.exp: next to second func1 ... The relevant bit of the test source is here in inline-cmds.c: ... 71 result = 0; /* set breakpoint 3 here */ 72 73 func1 (); /* first call */ 74 func1 (); /* second call */ 75 marker (); ... with func1 defined as: ... 33 inline __attribute__((always_inline)) int func1(void) 34 { 35 bar (); 36 return x * y; 37 } ... The corresponding insns are: ... 40050b: movl $0x0,0x200b1f(%rip) # 601034 <result> 400515: callq 40057b <bar> 40051a: callq 40057b <bar> 40051f: callq 400596 <marker> ... and the line number info is: ... Line number Starting address View Stmt 71 0x40050b x 35 0x400515 x 75 0x40051f x ... The line number info is missing an entry for the insn at 40051a, and that is causing the FAIL. This is a gcc issue, filed as PR gcc/98780 -" Missing line table entry for inlined stmt at -g -O0". [ For contrast, with clang we have an extra entry: ... Line number Starting address View Stmt 71 0x40050b x 35 0x400515 x 35 0x40051a 75 0x40051f x ... though it appears to be missing the start-of-statement marker. ] However, there is debug info that indicates that the insn at 40051a is not part of the line table entry for the insn at 400515: ... <2><1c4>: Abbrev Number: 8 (DW_TAG_inlined_subroutine) <1c5> DW_AT_abstract_origin: <0x2a2> <1c9> DW_AT_low_pc : 0x400515 <1d1> DW_AT_high_pc : 0x5 <1d9> DW_AT_call_file : 1 <1da> DW_AT_call_line : 73 <2><1db>: Abbrev Number: 8 (DW_TAG_inlined_subroutine) <1dc> DW_AT_abstract_origin: <0x2a2> <1e0> DW_AT_low_pc : 0x40051a <1e8> DW_AT_high_pc : 0x5 <1f0> DW_AT_call_file : 1 <1f1> DW_AT_call_line : 74 ... and indeed lldb manages to "next" from line 73 to line 74. Work around the missing line table entry, by using the inline frame info to narrow the stepping range in prepare_one_step. Tested on x86_64-linux. gdb/ChangeLog: 2021-04-06 Tom de Vries <tdevries@suse.de> PR breakpoints/25884 * infcmd.c (prepare_one_step): Using inline frame info to narrow stepping range. gdb/testsuite/ChangeLog: 2021-04-06 Tom de Vries <tdevries@suse.de> PR breakpoints/25884 * gdb.opt/inline-cmds.exp: Remove kfail.
* [gdb/tui] Fix len_without_escapes in tui-disasm.cTom de Vries2021-04-062-2/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On openSUSE Tumbleweed I run into: ... FAIL: gdb.tui/basic.exp: asm window shows main ERROR: invalid command name "_csi_L" ... Using a minimal example, we get: ... $ gdb -q outputs/gdb.tui/basic/basic -ex "tui enable" -ex "layout asm" <TUI output> src/gdb/ui-style.c:243: internal-error: bool \ ui_file_style::parse(const char*, size_t*): Assertion `match == 0' failed. ... The problem is in len_without_escapes, where we detect the start of an escape sequence, but then pass ptr to style.parse while ptr no longer points to the escape due to the ptr++ in the while condition: ... while ((c = *ptr++) != '\0') { if (c == '\033') { ui_file_style style; size_t n_read; if (style.parse (ptr, &n_read)) ... Fix this by removing the ++ in the while condition, and adding ptr++ in the loop body where appropriate. Tested on x86_64-linux. gdb/ChangeLog: 2021-04-06 Tom de Vries <tdevries@suse.de> PR tui/27680 * tui/tui-disasm.c (len_without_escapes): Pass ptr pointing at escape to style.parse.
* [gdb/testsuite] Fix xfail handling in gdb.threads/gcore-thread.expTom de Vries2021-04-062-4/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When running test-case gdb.threads/gcore-thread.exp on openSUSE Tumbleweed, I run into these XFAILs: ... XFAIL: gdb.threads/gcore-thread.exp: clear __stack_user.next XFAIL: gdb.threads/gcore-thread.exp: clear stack_used.next ... Apart from the xfail, the test-case also sets core0file to "": ... -re "No symbol \"${symbol}\" in current context\\.\r\n$gdb_prompt $" { xfail $test # Do not do the verification. set core0file "" } ... After which we run into this FAIL, because gdb_core_cmd fails to load a core file called "": ... (gdb) core ^M No core file now.^M (gdb) FAIL: gdb.threads/gcore-thread.exp: core0file: \ re-load generated corefile ... Fix this FAIL by skipping gdb_core_cmd if the core file is "". Tested on x86_64-linux. gdb/testsuite/ChangeLog: 2021-04-06 Tom de Vries <tdevries@suse.de> PR testsuite/27691 * gdb.threads/gcore-thread.exp: Don't call gdb_core_cmd with core file "".
* gdb: fix internal error in avr_frame_unwind_cacheSimon Marchi2021-04-042-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When trying to do pretty much anything that requires unwinding a frame on AVR, we get /home/simark/src/wt/avr/gdb/trad-frame.h:143: internal-error: LONGEST trad_frame_saved_reg::addr() const: Assertion `m_kind == trad_frame_saved_reg_kind::ADDR' failed. This is likely coming from the trad-frame refactor in 098caef485a4 ("Refactor struct trad_frame_saved_regs"). Here's an example of how to reproduce it: In one terminal: $ cat test.c int foo(int x) { return x * 7; } int main() { return foo(2); } $ avr-gcc -gdwarf-4 -mmcu=atmega2560 test.c $ /tmp/simavr/bin/simavr --mcu atmega2560 -g a.out Loaded 330 .text at address 0x0 Loaded 0 .data And in another one: $ ./gdb -q -nx --data-directory=data-directory a.out -ex "tar rem :1234" -ex "b foo" -ex c -ex bt Reading symbols from a.out... Remote debugging using :1234 0x00000000 in __vectors () Breakpoint 1 at 0x110: file test.c, line 3. Note: automatically using hardware breakpoints for read-only addresses. Continuing. Breakpoint 1, foo (x=2) at test.c:3 3 return x * 7; #0 foo (x=2) at test.c:3 /home/simark/src/wt/avr/gdb/trad-frame.h:143: internal-error: LONGEST trad_frame_saved_reg::addr() const: Assertion `m_kind == trad_frame_saved_reg_kind::ADDR' failed. What the AVR code does is: 1. In avr_scan_prologue, in the block that says "First stage of the prologue scanning.", look for "push rX" instructions and note that rX is saved on the stack. But instead of putting the actual stack address directly, it puts an offset (from the previous frame's sp). 2. Back in avr_frame_unwind_cache, in the block that says "Adjust all the saved registers", adjust all these values to be real stack addresses. To check whether a register was assigned an address (and therefore if it needs adjustment), the code does: if (info->saved_regs[i].addr () > 0) Since commit 098caef485a4, it's invalid to call the `addr` getter of trad_frame_saved_reg if the register hasn't been assigned an address. Instead, the code could use the `is_addr` getter to verify if the register has been assigned an address. This is what this patch does. gdb/ChangeLog: * avr-tdep.c (avr_frame_unwind_cache): Use trad_frame_saved_reg::is_addr. Change-Id: I5803089160b829400178746c5e3bca0c1cd11c00
* gdb: remove objfile parameter from get_objfile_bfd_dataSimon Marchi2021-04-022-6/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I noticed it was unused. I think that makes sense, as it shows that objfile_per_bfd_storage is not specific to one objfile (it can be shared by multiple objfiles that have the same bfd). There is one thing I wonder though, maybe I'm missing something. If the BFD doesn't require relocation, get_objfile_bfd_data stores the newly allocated object in objfiles_bfd_data, so we can assume that objfiles_bfd_data is the owner of the object. When the bfd's refcount drops to 0, the corresponding objfile_per_bfd_storage object in objfiles_bfd_data is deleted. But if the BFD requires relocation, get_objfile_bfd_data returns a newly allocated object that isn't kept anywhere else (and isn't shared). So the objfile becomes the owner of the objfile_per_bfd_storage object. In objfile::~objfile, we have this: if (obfd) gdb_bfd_unref (obfd); else delete per_bfd; I'm thinking that obfd could be non-nullptr, and it could require relocation. In that case, it would never be freed. Anyway, that's not really connected to this patch. gdb/ChangeLog: * objfiles.c (get_objfile_bfd_data): Remove objfile parameter, adjust callers. Change-Id: Ifa3158074ea6b42686780ba09d0c964b0cf14cf1
* gdb: pass objfile_per_bfd_storage instead of objfile to partial_symtabSimon Marchi2021-04-0211-46/+75
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since partial_symtab is supposed to be objfile-independent (since series [1]), I think it would make sense for partial_symtab to not take an objfile as a parameter in its constructor. This patch replaces that parameter with an objfile_per_bfd_storage parameter. The objfile is used for two things: - to get the objfile_name, for debug messages. We can get that name from the bfd instead. - to intern the partial symtab filename. Even though it goes through an objfile method, the request is actually forwarded to the underlying objfile_per_bfd_storage. So we can ask the new objfile_per_bfd_storage instead. In order to get a reference to the BFD from the objfile_per_bfd_storage, the BFD is saved in the objfile_per_bfd_storage object. [1] https://sourceware.org/pipermail/gdb-patches/2021-February/176625.html gdb/ChangeLog: * psympriv.h (struct partial_symtab) <partial_symtab>: Change objfile parameter for objfile_per_bfd_storage, adjust callers. (struct standard_psymtab) <standard_psymtab>: Likewise. (struct legacy_psymtab) <legacy_psymtab>: Likewise. * psymtab.c (partial_symtab::partial_symtab): Likewise. * ctfread.c (struct ctf_psymtab): Likewise. * dwarf2/read.h (struct dwarf2_psymtab): Likewise. * dwarf2/read.c (struct dwarf2_include_psymtab): Likewise. (dwarf2_create_include_psymtab): Likewise. * objfiles.h (struct objfile_per_bfd_storage) <objfile_per_bfd_storage>: Add bfd parameter, adjust callers. <get_bfd>: New method. <m_bfd>: New field. * objfiles.c (get_objfile_bfd_data): Adjust. Change-Id: I2ed3ab5d2e6f27d034bd4dc26ae2fae7b0b8a2b9
* gdb: use std::string in partial_symtab::partial_symtab / allocate_symtabSimon Marchi2021-04-023-12/+16
| | | | | | | | | | | | This simplifies the code a bit. gdb/ChangeLog: * psymtab.c (partial_symtab::partial_symtab): Change last_objfile_name to be an std::string. * symfile.c (allocate_symtab): Likewise. Change-Id: I3dfe217233ed9346c2abc04a9b1be0df69a90af8
* gdb: add intern methods to objfile_per_bfd_storageSimon Marchi2021-04-022-3/+26
| | | | | | | | | | | | | | | | This allows keeping the objfile_per_bfd_storage implementation details into objfile_per_bfd_storage, instead of into objfile. And this makes the intern methods usable for code that only has an objfile_per_bfd_storage to work with. gdb/ChangeLog: * objfiles.h (struct objfile_per_bfd_storage) <intern>: New methods. (struct objfile) <intern>: Use objfile::objfile_per_bfd_storage::intern. Change-Id: Ifd54026c5efaeffafac9b84ff84c199acc7ce78a
* gdb: remove TYPE_FLAG_ENUMSimon Marchi2021-04-013-3/+6
| | | | | | | | | gdb/ChangeLog: * gdbtypes.h (TYPE_FLAG_ENUM): Remove, replace all uses with type::is_flag_enum. Change-Id: I74e23893066eecd6df641045b859a6d6ebb13dd0
* gdb: add type::is_flag_enum / type::set_is_flag_enumSimon Marchi2021-04-013-7/+24
| | | | | | | | | | | | | | | | | Add the `is_flag_enum` and `set_is_flag_enum` methods on `struct type`, in order to remove the `TYPE_FLAG_ENUM` macro. In this patch, the macro is changed to use the getter, so all the call sites of the macro that are used as a setter are changed to use the setter method directly. The next patch will remove the macro completely. gdb/ChangeLog: * gdbtypes.h (struct type) <is_flag_enum, set_is_flag_enum>: New methods. (TYPE_FLAG_ENUM): Use type::is_flag_enum, change all write call sites to use type::set_is_flag_enum. Change-Id: I9c56c91626c8d784947ba94fcb97818526b81d1c
* gdb: remove TYPE_DECLARED_CLASSSimon Marchi2021-04-019-22/+19
| | | | | | | | | gdb/ChangeLog: * gdbtypes.h (TYPE_DECLARED_CLASS): Remove, replace all uses with type::is_declared_class. Change-Id: Ifecb2342417ecd7bf570c3205344b09d706daab2
* gdb: add type::is_declared_class / type::set_is_declared_classSimon Marchi2021-04-013-4/+28
| | | | | | | | | | | | | | | | | | Add the `is_declared_class` and `set_is_declared_class` methods on `struct type`, in order to remove the `TYPE_DECLARED_CLASS` macro. In this patch, the macro is changed to use the getter, so all the call sites of the macro that are used as a setter are changed to use the setter method directly. The next patch will remove the macro completely. gdb/ChangeLog: * gdbtypes.h (struct type) <is_declared_class, set_is_declared_class>: New methods. (TYPE_DECLARED_CLASS): Use type::is_declared_class, change all write call sites to use type::set_is_declared_class. Change-Id: Idf08d32e137c885a0aba0a18f556a899c1cbfd68
* Fix obvious typo in gdb/testsuite/lib/pdtrace.inEgeyar Bagcioglu2021-04-012-1/+5
|
* Use importlib instead of imp module on python 3.4+Boris Staletic2021-04-012-2/+9
| | | | | | | | | | | | | | | Python 3.4 has deprecated the imp module in favour of importlib. This patch avoids the DeprecationWarning. This warning is visible to users whose libpython.so has been compiled with --with-pydebug. Considering that even python 3.5 has reached end of life, would it be better to just use importlib and drop support for python 3.0 to 3.3? 2021-02-28 Boris Staletic <boris.staletic@gmail.com> * gdb/python/lib/gdb/__init__.py: Use importlib on python 3.4+ to avoid deprecation warnings.
* [gdb/testsuite] Fix unset of DEBUGINFOD_URLS in default_gdb_initTom de Vries2021-04-011-3/+1
| | | | | | | | | | | | | | In commit cfcbd506fb0 "[gdb/testsuite] Ignore DEBUGINFOD_URLS" I added unsetting of env(DEBUGINFOD_URLS), but it doesn't work because I forgot to add :: in front. Fix this, and rewrite using "unset -nocomplain" instead of unsetenv, which allows us to drop the "info exists" test. 2021-04-01 Tom de Vries <tdevries@suse.de> * lib/gdb.exp (default_gdb_init): Use ::env. Use unset -nocomplain ::env(V) instead of unsetenv V.
* Use startswith in gdb subfolder.Martin Liska2021-04-016-42/+50
| | | | | | | | | | gdb/ChangeLog: * cp-name-parser.y: Use startswith instead of strncmp. * m2-exp.y: Likewise. * macroexp.c (substitute_args): Likewise. * mi/mi-main.c (command_notifies_uscc_observer): Likewise. * rust-exp.y: Likewise.
* Remove two trivial functions from dwarf2/read.cTom Tromey2021-03-312-35/+17
| | | | | | | | | | | | | This removes dw2_map_matching_symbols and dw2_expand_symtabs_matching, merging them with their sole trivial callers. gdb/ChangeLog 2021-03-31 Tom Tromey <tom@tromey.com> * dwarf2/read.c (dwarf2_gdb_index::map_matching_symbols): Merge with dw2_map_matching_symbols. (dwarf2_gdb_index::expand_symtabs_matching): Merge with dw2_expand_symtabs_matching.
* Fix typo in dwarf2/stringify.hTom Tromey2021-03-312-1/+5
| | | | | | | | | | Pedro pointed out a typo in a comment in dwarf2/stringify.h. This fixes it. gdb/ChangeLog 2021-03-31 Tom Tromey <tromey@adacore.com> * dwarf2/stringify.h: Fix typo.
* Add some error checking to DWARF assemblerTom Tromey2021-03-312-33/+57
| | | | | | | | | | | | | | | | | | | | | | | I had written a DWARF location expression like DW_OP_const1u DW_OP_stack_value ... and was surprised to see that the DW_OP_stack_value didn't appear in the "readelf" output. The problem here is that DW_OP_const1u requires an operand, but neither the DWARF assembler nor gas diagnosed this problem. This patch adds some checking to Dwarf::_location to try to avoid this in the future. The checking is done via a helper proc that also dissects the argument list and sets an array in the caller's frame. gdb/testsuite/ChangeLog 2021-03-31 Tom Tromey <tromey@adacore.com> * lib/dwarf.exp (Dwarf::_get_args): New proc. (Dwarf::_location): Use it.
* [gdb/testsuite] Ignore DEBUGINFOD_URLSTom de Vries2021-03-312-0/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | On openSUSE Tumbleweed, DEBUGINFOD_URLS is now defined by default: ... $ echo $DEBUGINFOD_URLS https://debuginfod.opensuse.org/ ... With DEBUGINFOD_URLS defined we run into: ... FAIL: gdb.mi/mi-sym-info.exp: List all functions from debug information only \ (timeout) ... as reported in PR27667. There's a latency of ~0.5s per request, which is ok-ish for interactive usage. But the symbol-info-functions command ends up issuing 21 source requests, which means we easily run into the 10s timeout. Fix this by unsetting DEBUGINFOD_URLS in default_gdb_init. gdb/testsuite/ChangeLog: 2021-03-31 Tom de Vries <tdevries@suse.de> PR testsuite/27667 * lib/gdb.exp (default_gdb_init): Unset DEBUGINFOD_URLS.
* gdb/dwarf: disable per-BFD resource sharing for -readnow objfilesSimon Marchi2021-03-306-46/+122
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | New in v2: - Disable sharing only for -readnow objfiles, not all objfiles. As described in PR 27541, we hit an internal error when loading a binary the standard way and then loading it with the -readnow option: $ ./gdb -nx -q --data-directory=data-directory ~/a.out -ex "set confirm off" -ex "file -readnow ~/a.out" Reading symbols from /home/simark/a.out... Reading symbols from ~/a.out... /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8098: internal-error: void create_all_comp_units(dwarf2_per_objfile*): Assertion `per_objfile->per_bfd->all_comp_units.empty ()' failed. This is a recurring problem that exposes a design issue in the DWARF per-BFD sharing feature. Things work well when loading a binary with the same method (with/without index, with/without readnow) twice in a row. But they don't work so well when loading a binary with different methods. See this previous fix, for example: efb763a5ea35 ("gdb: check for partial symtab presence in dwarf2_initialize_objfile") That one handled the case where the first load is normal (uses partial symbols) and the second load uses an index. The problem is that when loading an objfile with a method A, we create a dwarf2_per_bfd and some dwarf2_per_cu_data and initialize them with the data belonging to that method. When loading another obfile sharing the same BFD but with a different method B, it's not clear how to re-use the dwarf2_per_bfd/dwarf2_per_cu_data previously created, because they contain the data specific to method A. I think the most sensible fix would be to not share a dwarf2_per_bfd between two objfiles loaded with different methods. That means that two objfiles sharing the same BFD and loaded the same way would share a dwarf2_per_bfd. Two objfiles sharing the same BFD but loaded with different methods would use two different dwarf2_per_bfd structures. However, this isn't a trivial change. So to fix the known issue quickly (including in the gdb 10 branch), this patch just disables all dwarf2_per_bfd sharing for objfiles using READNOW. Generalize the gdb.base/index-cache-load-twice.exp test to test all the possible combinations of loading a file with partial symtabs, index and readnow. Move it to gdb.dwarf2, since it really exercises features of the DWARF reader. gdb/ChangeLog: PR gdb/27541 * dwarf2/read.c (dwarf2_has_info): Don't share dwarf2_per_bfd with objfiles using READNOW. gdb/testsuite/ChangeLog: PR gdb/27541 * gdb.base/index-cache-load-twice.exp: Remove. * gdb.base/index-cache-load-twice.c: Remove. * gdb.dwarf2/per-bfd-sharing.exp: New. * gdb.dwarf2/per-bfd-sharing.c: New. Change-Id: I9ffcf1e136f3e75242f70e4e58e4ba1fd3083389
* [gdb/testsuite] Add missing .debug_abbrev terminator in dw2-cu-size.STom de Vries2021-03-302-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 27012aba8a6 "Remove Irix 6 workaround from DWARF abbrev reader" we have: ... (gdb) file dw2-cu-size^M Reading symbols from dw2-cu-size...^M DW_FORM_strp pointing outside of .debug_str section [in module dw2-cu-size]^M (No debugging symbols found in dw2-cu-size)^M (gdb) ptype noloc^M No symbol table is loaded. Use the "file" command.^M (gdb) FAIL: gdb.dwarf2/dw2-cu-size.exp: ptype noloc ... The problem is a missing .debug_abbrev terminator in dw2-cu-size.S, which causes the .debug_abbrev contribution to be merged with the next: ... Number TAG (0x9b) 1 DW_TAG_compile_unit [has children] DW_AT_name DW_FORM_string DW_AT_producer DW_FORM_string DW_AT_language DW_FORM_data1 DW_AT value: 0 DW_FORM value: 0 2 DW_TAG_variable [no children] DW_AT_name DW_FORM_string DW_AT_type DW_FORM_ref4 DW_AT_external DW_FORM_flag DW_AT value: 0 DW_FORM value: 0 3 DW_TAG_base_type [no children] DW_AT_name DW_FORM_string DW_AT_byte_size DW_FORM_data1 DW_AT_encoding DW_FORM_data1 DW_AT value: 0 DW_FORM value: 0 4 DW_TAG_const_type [no children] DW_AT_type DW_FORM_ref_udata DW_AT value: 0 DW_FORM value: 0 1 DW_TAG_compile_unit [has children] DW_AT_producer DW_FORM_strp DW_AT_language DW_FORM_data1 DW_AT_name DW_FORM_strp DW_AT_comp_dir DW_FORM_strp DW_AT_low_pc DW_FORM_addr DW_AT_high_pc DW_FORM_data8 DW_AT_stmt_list DW_FORM_sec_offset DW_AT value: 0 DW_FORM value: 0 ... and consequently, abbreviation code 1 no longer refers to a unique entry. The eventually causes us to access the first attribute of this DIE: ... <0><124>: Abbrev Number: 1 (DW_TAG_compile_unit) <125> DW_AT_name : file1.txt <12f> DW_AT_producer : GNU C 3.3.3 <13b> DW_AT_language : 1 (ANSI C) ... which has form DW_FORM_string, using DW_FORM_strp. Fix this by adding the missing .debug_abbrev terminator in dw2-cu-size.S. gdb/testsuite/ChangeLog: 2021-03-30 Tom de Vries <tdevries@suse.de> PR testsuite/27604 * gdb.dwarf2/dw2-cu-size.S: Add missing .debug_abbrev terminator.
* Remove parameter from language_infoTom Tromey2021-03-294-14/+15
| | | | | | | | | | | | I noticed that language_info is only ever called with a value of '1'. This patch removes the parameter. 2021-03-29 Tom Tromey <tromey@adacore.com> * top.c (check_frame_language_change): Update. * language.c (language_info): Remove parameter. * language.h (language_info): Remove parameter.
* Don't pass empty options to GCCLuis Machado2021-03-292-1/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | On aarch64-linux, I noticed the compile command didn't work at all. It always gave the following error: aarch64-linux-gnu-g++: error: : No such file or directory Turns out we're passing an empty argv entry to GCC (because aarch64 doesn't have a -m64 option), and GCC's behavior is to think that is a file it needs to open. One can reproduce it like so: gcc "" "" "" "" gcc: error: : No such file or directory gcc: error: : No such file or directory gcc: error: : No such file or directory gcc: error: : No such file or directory gcc: fatal error: no input files compilation terminated. The solution is to check for an empty string and skip adding that to argv. Regression tested on aarch64-linux/Ubuntu 18.04/20.04. gdb/ChangeLog: 2021-03-29 Luis Machado <luis.machado@linaro.org> * compile/compile.c (get_args): Don't add empty argv entries.
* Fix memory tagging section typeLuis Machado2021-03-292-1/+6
| | | | | | | | | | | | | | | | | | | | | It was reported to me that on Ubuntu 14.04 (fairly old) the documentation fails to build with the following: gdb/doc/gdb.texinfo:10888: warning: node `Memory' is up for `Memory Tagging' in sectioning but not in menu gdb/doc/gdb.texinfo:10693: node `Memory' lacks menu item for `Memory Tagging' despite being its Up target Makefile:491: recipe for target 'gdb.info' failed make[3]: *** [gdb.info] Error 1 This doesn't seem to happen on Ubuntu 18.04/20.04, but it does make sense. Fix this by turning @subsection into a @section and adding the "Memory Tagging" entry to the menu. gdb/doc/ChangeLog: 2021-03-29 Luis Machado <luis.machado@linaro.org> * gdb.textinfo (Memory Tagging): Make it a @section.
* testsuite, mi: avoid a clang bug in 'user-selected-context-sync.exp'Tankut Baris Aktemur2021-03-292-2/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This test causes several timeouts for Clang, taking too long time to finish. The reason is, for an infinite loop of the form while(1); /* suppose this is line 30. */ Clang generates code that looks like 0x00000000004004d4 <+4>: jmp 0x4004d9 <loop+9> 0x00000000004004d9 <+9>: jmp 0x4004d9 <loop+9> So, the real loop is the instruction at address 0x4004d9. But a breakpoint that's defined at the loop line (assume line 30 in this case) is inserted at address 0x4004d4. (gdb) break 30 Breakpoint 1 at 0x4004d4: file test.c, line 30. Therefore, continuing a thread that was spinning on the loop does not hit the breakpoint. The bug is reported at https://bugs.llvm.org/show_bug.cgi?id=49614 Tweak the infinite loop to spin on a variable to avoid this bug. The test is unrelated to the bug. gdb/testsuite/ChangeLog: 2021-03-29 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> * gdb.mi/user-selected-context-sync.exp: Spin on a variable in the infinite loop to avoid a Clang bug.
* Restore procfs.c compilationRainer Orth2021-03-292-2/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | Since c8fbd44a018a9923f906bfd2be5489caa87b602a (gdb: remove target_is_pushed free function), procfs.c compilation is broken, which went unnoticed for lack of functioning buildbots: /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In member function 'virtual void procfs_target::attach(const char*, int)': /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:1772:8: error: 'inf' was not declared in this scope; did you mean 'info'? 1772 | if (!inf->target_is_pushed (this)) | ^~~ | info /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In member function 'virtual void procfs_target::create_inferior(const char*, const string&, char**, int)': /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:2865:8: error: 'inf' was not declared in this scope; did you mean 'info'? 2865 | if (!inf->target_is_pushed (this)) | ^~~ | info Fixed by defining inf. Tested on amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11. 2021-03-29 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> gdb: * procfs.c (procfs_target::attach): Define inf. Use it. (procfs_target::create_inferior): Likewise.
* Simplify DWARF reader initializationTom Tromey2021-03-284-68/+66
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that the quick functions are separate from the object file format, there's no need to have elfread.c push a new entry on the objfile 'qf' list. Instead, this detail can be pushed into the DWARF reader. That is what this patch implements. I wasn't sure whether lazy reading still makes sense or not. It's still only used by ELF, and only in certain situations (like vfork, I think). It may not be carrying its weight, so we may want to consider removing this in the future. Also, I'm unclear on why the various indices are only used for ELF. This seems sub-optimal. However, I haven't tried to address that here. gdb/ChangeLog 2021-03-28 Tom Tromey <tom@tromey.com> * elfread.c (can_lazily_read_symbols): Move to dwarf2/read.c. (elf_symfile_read): Simplify. * dwarf2/read.c (struct lazy_dwarf_reader): Move from elfread.c. (make_lazy_dwarf_reader): New function. (make_dwarf_gdb_index, make_dwarf_debug_names): Now static. (dwarf2_initialize_objfile): Return void. Remove index_kind parameter. Push on 'qf' list. * dwarf2/public.h (dwarf2_initialize_objfile): Change return type. Remove 'index_kind' parameter. (make_dwarf_gdb_index, make_dwarf_debug_names): Don't declare.
* Don't declare elf_sym_fns_lazy_psymsTom Tromey2021-03-272-2/+4
| | | | | | | | | | | An earlier patch neglected to delete a forward declaration of elf_sym_fns_lazy_psyms. This is no longer defined. This patch removes it. gdb/ChangeLog 2021-03-27 Tom Tromey <tom@tromey.com> * elfread.c (elf_sym_fns_lazy_psyms): Don't declare.
* Don't clear 'qf' in elf_symfile_readTom Tromey2021-03-272-1/+4
| | | | | | | | | | | | | | I noticed that I forgot to make a change in my series to make it possible to attach multiple debug readers to an objfile. In one spot, elf_symfile_read still clears the 'qf' list. However, this should have been removed toward the end of that series. This patch fixes the offending spot. Tested on x86-64 Fedora 32. gdb/ChangeLog 2021-03-27 Tom Tromey <tom@tromey.com> * elfread.c (elf_symfile_read): Don't clear 'qf'.
* gdb/testsuite: make some test names unique in gdb.arch/powerpc-*.expWill Schmidt2021-03-273-4/+11
| | | | | | | | | | | | | Resolve some duplicate test name warnings in gdb.arch/powerpc-*.exp tests by either extending the existing test names, or providing a new test name. gdb/testsuite/ChangeLog: * gdb.arch/powerpc-disassembler-options.exp: Extend some test names for uniqueness. * gdb.arch/powerpc-fpscr-gcore.exp: Add more test names for uniqueness.
* gdb-add-index.sh: Remove use of non posix 'local'Lancelot SIX2021-03-262-13/+17
| | | | | | | | | | | | | | | | | While working on gdb-add-index.sh, it appeared that it uses the non POSIX 'local' keyword. Instead of using local to allow variable shadowing, I rename the local one to avoid name conflicts altogether. This commit gets rid of the following shellcheck warning: In gdb-add-index.sh line 63: local file="$1" ^--------^ SC2039: In POSIX sh, 'local' is undefined. gdb/ChangeLog: * contrib/gdb-add-index.sh: Avoid variable shadowing and get rid of 'local'.
* Use function view in quick_symbol_functions::map_symbol_filenamesTom Tromey2021-03-2611-81/+100
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This changes quick_symbol_functions::map_symbol_filenames to use a function_view, and updates all the uses. It also changes the final parameter to 'bool'. A couple of spots are further updated to use operator() rather than a lambda. gdb/ChangeLog 2021-03-26 Tom Tromey <tom@tromey.com> * symtab.c (struct output_source_filename_data): Add 'output' method and operator(). (output_source_filename_data::output): Rename from output_source_filename. (output_partial_symbol_filename): Remove. (info_sources_command): Update. (struct add_partial_filename_data): Add operator(). (add_partial_filename_data::operator()): Rename from maybe_add_partial_symtab_filename. (make_source_files_completion_list): Update. * symfile.c (quick_symbol_functions): Update. * symfile-debug.c (objfile::map_symbol_filenames): Update. * quick-symbol.h (symbol_filename_ftype): Change type of 'fun' and 'need_fullname'. Remove 'data' parameter. (struct quick_symbol_functions) <map_symbol_filenames>: Likewise. * psymtab.c (psymbol_functions::map_symbol_filenames): Update. * psympriv.h (struct psymbol_functions) <map_symbol_filenames>: Change type of 'fun' and 'need_fullname'. Remove 'data' parameter. * objfiles.h (struct objfile) <map_symbol_filenames>: Change type of 'fun' and 'need_fullname'. Remove 'data' parameter. * mi/mi-cmd-file.c (print_partial_file_name): Remove 'ignore' parameter. (mi_cmd_file_list_exec_source_files): Update. * dwarf2/read.c (dwarf2_base_index_functions::map_symbol_filenames): Update.
* Simplify use of map_matching_symbols in ada-lang.cTom Tromey2021-03-262-28/+24
| | | | | | | | | | | | | | | I noticed that ada-lang.c creates a lambda to call aux_add_nonlocal_symbols. However, this code can be simplified a bit by changing match_data to implement operator(), and then simply passing the object as the callback. That is what this patch implements. gdb/ChangeLog 2021-03-26 Tom Tromey <tom@tromey.com> * ada-lang.c (struct match_data): Add operator(). (match_data::operator()): Rename from aux_add_nonlocal_symbols. (callback): Remove 'callback'.
* Simplify psymbol_functions::expand_symtabs_matchingTom Tromey2021-03-262-1/+10
| | | | | | | | | | | | I noticed that psymbol_functions::expand_symtabs_matching calls make_ignore_params once per psymtab that is matched. This seems possibly expensive, so this patch hoists the call out of the loop. gdb/ChangeLog 2021-03-26 Tom Tromey <tom@tromey.com> * psymtab.c (psymbol_functions::expand_symtabs_matching): Only call make_ignore_params once.
* Allow expand_symtabs_matching to examine imported psymtabsTom Tromey2021-03-262-5/+5
| | | | | | | | | | | | | | | | | | | | | Currently the psymtab variant of expand_symtabs_matching has this check: /* We skip shared psymtabs because file-matching doesn't apply to them; but we search them later in the loop. */ if (ps->user != NULL) continue; In a larger series I'm working on, it's convenient to remove this check. And, I noticed that a similar check is not done for expand_symtabs_with_fullname. So, it made sense to me to remove the check here as well. gdb/ChangeLog 2021-03-26 Tom Tromey <tom@tromey.com> * psymtab.c (psymbol_functions::expand_symtabs_matching): Remove "user" check.
* gdb/testsuite: more testing of pretty printer 'array' display_hintAndrew Burgess2021-03-264-0/+45
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds a couple of tests to the python pretty printer testing. I've added a test for the 'array' display hint. This display hint is tested by gdb.python/py-mi.exp, however, the MI testing is done via the varobj interface, and this code makes its own direct calls to the Python pretty printers from gdb/varobj.c. What this means is that the interface to the pretty printers in gdb/python/py-prettyprint.c is not tested for the 'array' display hint path. I also added a test for what happens when the display_hint method raises an exception. There wasn't a bug that inspired this test, just while adding the previous test I thought, I wonder what happens if... The current behaviour of GDB seems reasonable, GDB displays the Python exception, and then continues printing the value as if display_hint had returned None. I added a test to lock in this behaviour. gdb/testsuite/ChangeLog: * gdb.python/py-prettyprint.c (struct container): Add 'is_array_p' member. (make_container): Initialise is_array_p. * gdb.python/py-prettyprint.exp: Add new tests. * gdb.python/py-prettyprint.py (ContainerPrinter.display_hint): Check is_array_p and possibly return 'array'.
* gdb: defer commit resume until all available events are consumedSimon Marchi2021-03-268-0/+108
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rationale --------- Let's say you have multiple threads hitting a conditional breakpoint at the same time, and all of these are going to evaluate to false. All these threads will need to be resumed. Currently, GDB fetches one target event (one SIGTRAP representing the breakpoint hit) and decides that the thread should be resumed. It calls resume and commit_resume immediately. It then fetches the second target event, and does the same, until it went through all threads. The result is therefore something like: - consume event for thread A - resume thread A - commit resume (affects thread A) - consume event for thread B - resume thread B - commit resume (affects thread B) - consume event for thread C - resume thread C - commit resume (affects thread C) For targets where it's beneficial to group resumptions requests (most likely those that implement target_ops::commit_resume), it would be much better to have: - consume event for thread A - resume thread A - consume event for thread B - resume thread B - consume event for thread C - resume thread C - commit resume (affects threads A, B and C) Implementation details ---------------------- To achieve this, this patch adds another check in maybe_set_commit_resumed_all_targets to avoid setting the commit-resumed flag of targets that readily have events to provide to infrun. To determine if a target has events readily available to report, this patch adds an `has_pending_events` target_ops method. The method returns a simple bool to say whether or not it has pending events to report. Testing ======= To test this, I start GDBserver with a program that spawns multiple threads: $ ../gdbserver/gdbserver --once :1234 ~/src/many-threads-stepping-over-breakpoints/many-threads-stepping-over-breakpoints I then connect with GDB and install a conditional breakpoint that always evaluates to false (and force the evaluation to be done by GDB): $ ./gdb -nx --data-directory=data-directory \ /home/simark/src/many-threads-stepping-over-breakpoints/many-threads-stepping-over-breakpoints \ -ex "set breakpoint condition-evaluation host" \ -ex "set pag off" \ -ex "set confirm off" \ -ex "maint set target-non-stop on" \ -ex "tar rem :1234" \ -ex "tb main" \ -ex "b 13 if 0" \ -ex c \ -ex "set debug infrun" \ -ex "set debug remote 1" \ -ex "set debug displaced" I then do "continue" and look at the log. The remote target receives a bunch of stop notifications for all threads that have hit the breakpoint. infrun consumes and processes one event, decides it should not cause a stop, prepares a displaced step, after which we should see: [infrun] maybe_set_commit_resumed_all_process_targets: not requesting commit-resumed for target remote, target has pending events Same for a second thread (since we have 2 displaced step buffers). For the following threads, their displaced step is deferred since there are no more buffers available. After consuming the last event the remote target has to offer, we get: [infrun] maybe_set_commit_resumed_all_process_targets: enabling commit-resumed for target remote [infrun] maybe_call_commit_resumed_all_process_targets: calling commit_resumed for target remote [remote] Sending packet: $vCont;s:p14d16b.14d1b1;s:p14d16b.14d1b2#55 [remote] Packet received: OK Without the patch, there would have been one vCont;s just after each prepared displaced step. gdb/ChangeLog: yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com> Pedro Alves <pedro@palves.net> * async-event.c (async_event_handler_marked): New. * async-event.h (async_event_handler_marked): Declare. * infrun.c (maybe_set_commit_resumed_all_targets): Switch to inferior before calling target method. Don't commit-resumed if target_has_pending_events is true. * remote.c (remote_target::has_pending_events): New. * target-delegates.c: Regenerate. * target.c (target_has_pending_events): New. * target.h (target_ops::has_pending_events): New target method. (target_has_pending_events): New. Change-Id: I18112ba19a1ff4986530c660f530d847bb4a1f1d
* gdb: generalize commit_resume, avoid commit-resuming when threads have ↵Simon Marchi2021-03-2612-98/+575
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pending statuses The rationale for this patch comes from the ROCm port [1], the goal being to reduce the number of back and forths between GDB and the target when doing successive operations. I'll start with explaining the rationale and then go over the implementation. In the ROCm / GPU world, the term "wave" is somewhat equivalent to a "thread" in GDB. So if you read if from a GPU stand point, just s/thread/wave/. ROCdbgapi, the library used by GDB [2] to communicate with the GPU target, gives the illusion that it's possible for the debugger to control (start and stop) individual threads. But in reality, this is not how it works. Under the hood, all threads of a queue are controlled as a group. To stop one thread in a group of running ones, the state of all threads is retrieved from the GPU, all threads are destroyed, and all threads but the one we want to stop are re-created from the saved state. The net result, from the point of view of GDB, is that the library stopped one thread. The same thing goes if we want to resume one thread while others are running: the state of all running threads is retrieved from the GPU, they are all destroyed, and they are all re-created, including the thread we want to resume. This leads to some inefficiencies when combined with how GDB works, here are two examples: - Stopping all threads: because the target operates in non-stop mode, when the user interface mode is all-stop, GDB must stop all threads individually when presenting a stop. Let's suppose we have 1000 threads and the user does ^C. GDB asks the target to stop one thread. Behind the scenes, the library retrieves 1000 thread states and restores the 999 others still running ones. GDB asks the target to stop another one. The target retrieves 999 thread states and restores the 998 remaining ones. That means that to stop 1000 threads, we did 1000 back and forths with the GPU. It would have been much better to just retrieve the states once and stop there. - Resuming with pending events: suppose the 1000 threads hit a breakpoint at the same time. The breakpoint is conditional and evaluates to true for the first thread, to false for all others. GDB pulls one event (for the first thread) from the target, decides that it should present a stop, so stops all threads using stop_all_threads. All these other threads have a breakpoint event to report, which is saved in `thread_info::suspend::waitstatus` for later. When the user does "continue", GDB resumes that one thread that did hit the breakpoint. It then processes the pending events one by one as if they just arrived. It picks one, evaluates the condition to false, and resumes the thread. It picks another one, evaluates the condition to false, and resumes the thread. And so on. In between each resumption, there is a full state retrieval and re-creation. It would be much nicer if we could wait a little bit before sending those threads on the GPU, until it processed all those pending events. To address this kind of performance issue, ROCdbgapi has a concept called "forward progress required", which is a boolean state that allows its user (i.e. GDB) to say "I'm doing a bunch of operations, you can hold off putting the threads on the GPU until I'm done" (the "forward progress not required" state). Turning forward progress back on indicates to the library that all threads that are supposed to be running should now be really running on the GPU. It turns out that GDB has a similar concept, though not as general, commit_resume. One difference is that commit_resume is not stateful: the target can't look up "does the core need me to schedule resumed threads for execution right now". It is also specifically linked to the resume method, it is not used in other contexts. The target accumulates resumption requests through target_ops::resume calls, and then commits those resumptions when target_ops::commit_resume is called. The target has no way to check if it's ok to leave resumed threads stopped in other target methods. To bridge the gap, this patch generalizes the commit_resume concept in GDB to match the forward progress concept of ROCdbgapi. The current name (commit_resume) can be interpreted as "commit the previous resume calls". I renamed the concept to "commit_resumed", as in "commit the threads that are resumed". In the new version, we have two things: - the commit_resumed_state field in process_stratum_target: indicates whether GDB requires target stacks using this target to have resumed threads committed to the execution target/device. If false, an execution target is allowed to leave resumed threads un-committed at the end of whatever method it is executing. - the commit_resumed target method: called when commit_resumed_state transitions from false to true. While commit_resumed_state was false, the target may have left some resumed threads un-committed. This method being called tells it that it should commit them back to the execution device. Let's take the "Stopping all threads" scenario from above and see how it would work with the ROCm target with this change. Before stopping all threads, GDB would set the target's commit_resumed_state field to false. It would then ask the target to stop the first thread. The target would retrieve all threads' state from the GPU and mark that one as stopped. Since commit_resumed_state is false, it leaves all the other threads (still resumed) stopped. GDB would then proceed to call target_stop for all the other threads. Since resumed threads are not committed, this doesn't do any back and forth with the GPU. To simplify the implementation of targets, this patch makes it so that when calling certain target methods, the contract between the core and the targets guarantees that commit_resumed_state is false. This way, the target doesn't need two paths, one for commit_resumed_state == true and one for commit_resumed_state == false. It can just assert that commit_resumed_state is false and work with that assumption. This also helps catch places where we forgot to disable commit_resumed_state before calling the method, which represents a probable optimization opportunity. The commit adds assertions in the target method wrappers (target_resume and friends) to have some confidence that this contract between the core and the targets is respected. The scoped_disable_commit_resumed type is used to disable the commit resumed state of all process targets on construction, and selectively re-enable it on destruction (see below for criteria). Note that it only sets the process_stratum_target::commit_resumed_state flag. A subsequent call to maybe_call_commit_resumed_all_targets is necessary to call the commit_resumed method on all target stacks with process targets that got their commit_resumed_state flag turned back on. This separation is because we don't want to call the commit_resumed methods in scoped_disable_commit_resumed's destructor, as they may throw. On destruction, commit-resumed is not re-enabled for a given target if: 1. this target has no threads resumed, or 2. this target has at least one resumed thread with a pending status known to the core (saved in thread_info::suspend::waitstatus). The first point is not technically necessary, because a proper commit_resumed implementation would be a no-op if the target has no resumed threads. But since we have a flag do to a quick check, it shouldn't hurt. The second point is more important: together with the scoped_disable_commit_resumed instance added in fetch_inferior_event, it makes it so the "Resuming with pending events" described above is handled efficiently. Here's what happens in that case: 1. The user types "continue". 2. Upon destruction, the scoped_disable_commit_resumed in the `proceed` function does not enable commit-resumed, as it sees some threads have pending statuses. 3. fetch_inferior_event is called to handle another event, the breakpoint hit evaluates to false, and that thread is resumed. Because there are still more threads with pending statuses, the destructor of scoped_disable_commit_resumed in fetch_inferior_event still doesn't enable commit-resumed. 4. Rinse and repeat step 3, until the last pending status is handled by fetch_inferior_event. In that case, scoped_disable_commit_resumed's destructor sees there are no more threads with pending statues, so it asks the target to commit resumed threads. This allows us to avoid all unnecessary back and forths, there is a single commit_resumed call once all pending statuses are processed. This change required remote_target::remote_stop_ns to learn how to handle stopping threads that were resumed but pending vCont. The simplest example where that happens is when using the remote target in all-stop, but with "maint set target-non-stop on", to force it to operate in non-stop mode under the hood. If two threads hit a breakpoint at the same time, GDB will receive two stop replies. It will present the stop for one thread and save the other one in thread_info::suspend::waitstatus. Before this patch, when doing "continue", GDB first resumes the thread without a pending status: Sending packet: $vCont;c:p172651.172676#f3 It then consumes the pending status in the next fetch_inferior_event call: [infrun] do_target_wait_1: Using pending wait status status->kind = stopped, signal = GDB_SIGNAL_TRAP for Thread 1517137.1517137. [infrun] target_wait (-1.0.0, status) = [infrun] 1517137.1517137.0 [Thread 1517137.1517137], [infrun] status->kind = stopped, signal = GDB_SIGNAL_TRAP It then realizes it needs to stop all threads to present the stop, so stops the thread it just resumed: [infrun] stop_all_threads: Thread 1517137.1517137 not executing [infrun] stop_all_threads: Thread 1517137.1517174 executing, need stop remote_stop called Sending packet: $vCont;t:p172651.172676#04 This is an unnecessary resume/stop. With this patch, we don't commit resumed threads after proceeding, because of the pending status: [infrun] maybe_commit_resumed_all_process_targets: not requesting commit-resumed for target extended-remote, a thread has a pending waitstatus When GDB handles the pending status and stop_all_threads runs, we stop a resumed but pending vCont thread: remote_stop_ns: Enqueueing phony stop reply for thread pending vCont-resume (1520940, 1520976, 0) That thread was never actually resumed on the remote stub / gdbserver, so we shouldn't send a packet to the remote side asking to stop the thread. Note that there are paths that resume the target and then do a synchronous blocking wait, in sort of nested event loop, via wait_sync_command_done. For example, inferior function calls, or any run control command issued from a breakpoint command list. We handle that making wait_sync_command_one a "sync" point -- force forward progress, or IOW, force-enable commit-resumed state. gdb/ChangeLog: yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com> Pedro Alves <pedro@palves.net> * infcmd.c (run_command_1, attach_command, detach_command) (interrupt_target_1): Use scoped_disable_commit_resumed. * infrun.c (do_target_resume): Remove target_commit_resume call. (commit_resume_all_targets): Remove. (maybe_set_commit_resumed_all_targets): New. (maybe_call_commit_resumed_all_targets): New. (enable_commit_resumed): New. (scoped_disable_commit_resumed::scoped_disable_commit_resumed) (scoped_disable_commit_resumed::~scoped_disable_commit_resumed) (scoped_disable_commit_resumed::reset) (scoped_disable_commit_resumed::reset_and_commit) (scoped_enable_commit_resumed::scoped_enable_commit_resumed) (scoped_enable_commit_resumed::~scoped_enable_commit_resumed): New. (proceed): Use scoped_disable_commit_resumed and maybe_call_commit_resumed_all_targets. (fetch_inferior_event): Use scoped_disable_commit_resumed. * infrun.h (struct scoped_disable_commit_resumed): New. (maybe_call_commit_resumed_all_process_targets): New. (struct scoped_enable_commit_resumed): New. * mi/mi-main.c (exec_continue): Use scoped_disable_commit_resumed. * process-stratum-target.h (class process_stratum_target): <commit_resumed_state>: New. * record-full.c (record_full_wait_1): Change commit_resumed_state around calling commit_resumed. * remote.c (class remote_target) <commit_resume>: Rename to... <commit_resumed>: ... this. (struct stop_reply): Move up. (remote_target::commit_resume): Rename to... (remote_target::commit_resumed): ... this. Check if there is any thread pending vCont resume. (remote_target::remote_stop_ns): Generate stop replies for resumed but pending vCont threads. (remote_target::wait_ns): Add gdb_assert. * target-delegates.c: Regenerate. * target.c (target_wait, target_resume): Assert that the current process_stratum target isn't in commit-resumed state. (defer_target_commit_resume): Remove. (target_commit_resume): Remove. (target_commit_resumed): New. (make_scoped_defer_target_commit_resume): Remove. (target_stop): Assert that the current process_stratum target isn't in commit-resumed state. * target.h (struct target_ops) <commit_resume>: Rename to ... <commit_resumed>: ... this. (target_commit_resume): Remove. (target_commit_resumed): New. (make_scoped_defer_target_commit_resume): Remove. * top.c (wait_sync_command_done): Use scoped_enable_commit_resumed. [1] https://github.com/ROCm-Developer-Tools/ROCgdb/ [2] https://github.com/ROCm-Developer-Tools/ROCdbgapi Change-Id: I836135531a29214b21695736deb0a81acf8cf566
* target_is_non_stop_p and sync targetsPedro Alves2021-03-262-4/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | gdb.base/maint-target-async-off.exp fails if you test against gdbserver with "maint set target-non-stop on" forced. (gdb) run Starting program: build/gdb/testsuite/outputs/gdb.base/maint-target-async-off/maint-target-async-off Breakpoint 1, main () at src/gdb/testsuite/gdb.base/maint-target-async-off.c:21 21 return 0; (gdb) FAIL: gdb.base/maint-target-async-off.exp: continue until exit (timeout) Above, GDB just stopped listening to stdin. Basically, GDB assumes that a target working in non-stop mode operation also supports async mode; it's a requirement. GDB misbehaves badly otherwise, and even hits failed assertions. Fix this by making target_is_non_stop_p return false if async is off. gdb/ChangeLog: * target.c (target_always_non_stop_p): Also check whether the target can async. Change-Id: I7e52e1061396a5b9b02ada462f68a14b76d68974
* Avoid some pointer chasing in DWARF readerTom Tromey2021-03-262-7/+13
| | | | | | | | | | | | | | | | I noticed a spot in the DWARF reader using "per_objfile->per_bfd", where a local per_bfd variable had already been created. Looking through the file, I found a number of such spots. This patch changes them to use the already-existing local, avoiding a bit of excess pointer chasing. gdb/ChangeLog 2021-03-26 Tom Tromey <tom@tromey.com> * dwarf2/read.c (dwarf2_read_debug_names) (dwarf2_build_psymtabs_hard, create_addrmap_from_aranges) (dw2_debug_names_iterator::next, create_type_unit_group): Simplify.
View Lorry Controller Status