- Feb 25, 2021
-
-
Andrew Burgess authored
When attempting to call a Fortran function for which there is no debug information we currently trigger undefined behaviour in GDB by accessing non-existent type fields. The reason is that in order to prepare the arguments, for a call to a Fortran function, we need to know the type of each argument. If the function being called has no debug information then obviously GDB doesn't know about the argument types and we should either give the user an error or pick a suitable default. What we currently do is just assume the field exist and access undefined memory, which is clearly wrong. The reason GDB needs to know the argument type is to tell if the argument is artificial or not, artificial arguments will be passed by value while non-artificial arguments will be passed by reference. An ideal solution for this problem would be to allow the user to cast the function to the correct type, we already do this to some degree with the return value, for example: (gdb) print some_func_ () 'some_func_' has unknown return type; cast the call to its declared return type (gdb) print (integer) some_func_ () $1 = 1 But if we could extend this to allow casting to the full function type, GDB could figure out from the signature what are real parameters, and what are artificial parameters. Maybe something like this: (gdb) print ((integer () (integer, double)) some_other_func_ (1, 2.3) Alas, right now the Fortran expression parser doesn't seem to support parsing function signatures, and we certainly don't have support for figuring out real vs artificial arguments from a signature. Still, I think we can prevent GDB from accessing undefined memory and provide a reasonable default behaviour. In this commit I: - Only ask if the argument is artificial if the type of the argument is actually known. - Unknown arguments are assumed to be artificial and passed by value (non-artificial arguments are pass by reference). - If an artificial argument is prefixed with '&' by the user then we treat the argument as pass-by-reference. With these three changes we avoid undefined behaviour in GDB, and allow the user, in most cases, to get a reasonably natural default behaviour. gdb/ChangeLog: PR fortran/26155 * f-lang.c (fortran_argument_convert): Delete declaration. (fortran_prepare_argument): New function. (evaluate_subexp_f): Move logic to new function fortran_prepare_argument. gdb/testsuite/ChangeLog: PR fortran/26155 * gdb.fortran/call-no-debug-func.f90: New file. * gdb.fortran/call-no-debug-prog.f90: New file. * gdb.fortran/call-no-debug.exp: New file.
-
Andrew Burgess authored
This commit adds support for the ASSOCIATED builtin to the Fortran expression evaluator. The ASSOCIATED builtin takes one or two arguments. When passed a single pointer argument GDB returns a boolean indicating if the pointer is associated with anything. When passed two arguments the second argument should either be some a pointer could point at or a second pointer. If the second argument is a pointer target, then the result from associated indicates if the pointer is pointing at this target. If the second argument is another pointer, then the result from associated indicates if the two pointers are pointing at the same thing. gdb/ChangeLog: * f-exp.y (f77_keywords): Add 'associated'. * f-lang.c (fortran_associated): New function. (evaluate_subexp_f): Handle FORTRAN_ASSOCIATED. (operator_length_f): Likewise. (print_unop_or_binop_subexp_f): New function. (print_subexp_f): Make use of print_unop_or_binop_subexp_f for FORTRAN_ASSOCIATED, FORTRAN_LBOUND, and FORTRAN_UBOUND. (dump_subexp_body_f): Handle FORTRAN_ASSOCIATED. (operator_check_f): Likewise. * std-operator.def: Add FORTRAN_ASSOCIATED. gdb/testsuite/ChangeLog: * gdb.fortran/associated.exp: New file. * gdb.fortran/associated.f90: New file.
-
Andrew Burgess authored
gfortran supports .xor. as an alias for .neqv., see: https://gcc.gnu.org/onlinedocs/gfortran/_002eXOR_002e-operator.html this commit adds support for this operator to GDB. gdb/ChangeLog: * f-exp.y (fortran_operators): Add ".xor.". gdb/testsuite/ChangeLog: * gdb.fortran/dot-ops.exp (dot_operations): Test ".xor.".
-
Alan Modra authored
This makes IR objects use the same logic as normal objects with respect to what sort of ref/def makes an as-needed library needed. Testing the binding of the definition is just plain wrong. What matters is the binding of the reference. PR 27441 * elf-bfd.h (struct elf_link_hash_entry): Add ref_ir_nonweak. * elflink.c (elf_link_add_object_symbols): Set ref_ir_nonweak and use when deciding an as-needed library should be loaded instead of using the binding of the library definition.
-
GDB Administrator authored
-
Alan Modra authored
PR 27456 * rename.c (simple_copy): Mark target_stat ATTRIBUTE_UNUSED.
-
Tom de Vries authored
With test-case gdb.cp/temargs.exp on target board \ unix/gdb:debug_flags=-gdwarf-5 I run into: ... (gdb) info addr I^M ERROR: GDB process no longer exists GDB process exited with wait status 32286 exp19 0 0 CHILDKILLED SIGABRT SIGABRT UNRESOLVED: gdb.cp/temargs.exp: test address of I in templ_m ... This is a regression since commit 529908cb "Remove DW_UNSND". The problem is that this DW_AT_decl_file: ... <1><221>: Abbrev Number: 4 (DW_TAG_structure_type) <222> DW_AT_name : Base<double, 23, (& a_global), &S::f> <226> DW_AT_byte_size : 1 <226> DW_AT_decl_file : 1 <226> DW_AT_decl_line : 30 <227> DW_AT_sibling : <0x299> ... is not read by this code in new_symbol: .... attr = dwarf2_attr (die, inlined_func ? DW_AT_call_file : DW_AT_decl_file, cu); if (attr != nullptr && attr->form_is_unsigned ()) ... because DW_AT_decl_file has form DW_FORM_implicit_const: ... 4 DW_TAG_structure_type [has children] DW_AT_name DW_FORM_strp DW_AT_byte_size DW_FORM_implicit_const: 1 DW_AT_decl_file DW_FORM_implicit_const: 1 DW_AT_decl_line DW_FORM_data1 DW_AT_sibling DW_FORM_ref4 DW_AT value: 0 DW_FORM value: 0 ... which is a signed LEB128, so attr->form_is_unsigned () returns false. Fix this by introducing new functions is_nonnegative and as_nonnegative, and use these instead of form_is_unsigned and as_unsigned. Tested on x86_64-linux. gdb/ChangeLog: 2021-02-24 Tom de Vries <tdevries@suse.de> PR symtab/27336 * dwarf2/attribute.c (attribute::form_is_signed): New function factored out of ... * dwarf2/attribute.h (attribute::as_signed): ... here. (attribute::is_nonnegative, attribute::as_nonnegative): New function. (attribute::form_is_signed): Declare. * dwarf2/read.c (new_symbol): Use is_nonnegative and as_nonnegative for DW_AT_decl_file.
-
Kevin Buettner authored
gdb/ChangeLog: * nat/aarch64-linux-hw-point.c: Add comment regarding include order for <sys/ptrace.h> and <asm/ptrace.h>.
-
Kevin Buettner authored
Due to a recent glibc header file change, the file nat/aarch64-linux-hw-point.c no longer builds on Fedora rawhide. An enum for PTRACE_SYSEMU is now provided by <sys/ptrace.h>. In the past, PTRACE_SYSEMU was defined only in <asm/ptrace.h>. This is what it looks like... In <asm/ptrace.h>: #define PTRACE_SYSEMU 31 In <sys/ptrace.h>: enum __ptrace_request { ... PTRACE_SYSEMU = 31, #define PT_SYSEMU PTRACE_SYSEMU ... } When <asm/ptrace.h> and <sys/ptrace.h> are both included in a source file, we run into the following build problem when the former is included before the latter: In file included from nat/aarch64-linux-hw-point.c:26: /usr/include/sys/ptrace.h:86:3: error: expected identifier before numeric constant 86 | PTRACE_SYSEMU = 31, | ^~~~~~~~~~~~~ (There are more errors after this one too.) The file builds without error when <asm/ptrace.h> is included after <sys/ptrace.h>. I found that this is already done in nat/aarch64-sve-linux-ptrace.h (which is included by nat/aarch64-linux-ptrace.c). I've tested this change on Fedora rawhide and Fedora 33, both running on an aarch64 machine. gdb/ChangeLog: * nat/aarch64-linux-hw-point.c: Include <asm/ptrace.h> after <sys/ptrace.h>. -
Andrew Burgess authored
The 'section' command uses a fixed size buffer into which a section name is copied. This commit replaces this with a use of std::string so we can now display very long section names. The expected results of one test need to be updated. gdb/ChangeLog: * exec.c (set_section_command): Move variable declarations into the function body, and use std::string instead of a fixed size buffer. gdb/testsuite/ChangeLog: * gdb.base/sect-cmd.exp: Update expected results.
-
Andrew Burgess authored
The only target that implements target_ops::get_section_table in a meaningful way is exec_target. This target calls back into the program space to return the current global section_table. The global section table is populated whenever the user provides GDB with an executable, or when a symbol file is loaded, e.g. when a dynamic library is loaded, or when the user does add-symbol-file. I recently ran into a situation where a user, debugging a remote target, was not supplying GDB with a main executable at all. Instead the user attached to the target then did add-symbol-file, and then proceeded to debug the target. This works fine, but it was noticed that even when trust-readonly-sections was on GDB was still accessing the target to get the contents of readonly sections. The problem is that by not providing an executable there was no exec_target in the target stack, and so when GDB calls the target_ops::get_section_table function GDB ends up in dummy_target::get_section_table, which just returns NULL. What I want is that even when GDB doesn't have an exec_target in the target stack, a call to target_ops::get_section_table will still return the section_table from the current program space. When considering how to achieve this my first though was, why is the request for the section table going via the target stack at all? The set of sections loaded is a property of the program space, not the target. This is, after all, why the data is being stored in the program space. So I initially tried changing target_get_section_table so that, instead of calling into the target it just returns current_program_space->target_sections (). This would be fine except for one issue, target_bfd (from bfd-target.c). This code is used from solib-svr4.c to create a temporary target_ops structure that implements two functions target_bfd::xfer_partial and target_bfd::get_section_table. The purpose behind the code is to enable two targets, ppc64 and frv to decode function descriptors from the dynamic linker, based on the non-relocated addresses from within the dynamic linker bfd object. Both of the implemented functions in target_bfd rely on the target_bfd object holding a section table, and the ppc64 target requires that the target_bfd implement ::get_section_table. The frv target doesn't require ::get_section_table, instead it requires the ::xfer_partial. We could in theory change the ppc64 target to use the same approach as frv, however, this would be a bad idea. I believe that the frv target approach is broken. I'll explain: The frv target calls get_target_memory_unsigned to read the function descriptor. The address being read is the non-relocated address read from the dynamic linker in solib-srv4.c:enable_break. Calling get_target_memory_unsigned eventually ends up in target_xfer_partial with an object type of TARGET_OBJECT_RAW_MEMORY. This will then call memory_xfer_check_region. I believe that it is quite possible that a the non-relocated addresses pulled from the dynamic linker could be in a memory region that is not readable, while the relocated addresses are in a readable memory region. If this was ever the case for the frv target then GDB would reject the attempt to read the non-relocated function pointer. In contrast the ppc64 target calls target_section_by_addr, which calls target_get_section_table, which then calls the ::get_section_table function on the target. Thus, when reflecting on target_bfd we see two functions, ::xfer_partial and ::get_section_table. The former is required by the frv target, but that target is (I think) potentially broken. While the latter is required by the ppc64 target, but this forces ::get_section_table to exist as a target_ops member function. So my original plan, have target_get_section_table NOT call a target_ops member function appears to be flawed. My next idea was to remove exec_target::get_section_table, and instead move the implementation into dummy_target::get_section_table. Currently the dummy_target implementation always returns NULL indicating no section table, but plenty of other dummy_target member functions do more than just return null values. So now, dummy_target::get_section_table returns the section table from the current program space. This allows target_bfd to remain unchanged, so ppc64 and frv should not be affected. Making this change removes the requirement for the user to provide an executable, GDB can now always access the section_table, as the dummy_target always exists in the target stack. Finally, there's a test that the target_section table is not empty in the case where the user does add-symbol-file without providing an executable. gdb/ChangeLog: * exec.c (exec_target::get_section_table): Delete member function. (section_table_read_available_memory): Use current_top_target, not just the exec_ops target. * target-delegates.c: Regenerate. * target.c (default_get_section_table): New function. * target.h (target_ops::get_section_table): Change default behaviour to call default_get_section_table. (default_get_section_table): Declare.
-
Andrew Burgess authored
Following on from earlier commits which made access to the target_sections table more 'const', this commit makes the table private within the program_space class and provides member functions to access the table. Ideally I would have liked for the new target_sections member function (on program_space) to return a 'const' reference to the table within the program_space. Unfortunately, there are two places in solib-*.c, where code outside of the program_space class modifies the target_sections table, and so to support this we need to return a non-const reference. There should be no user visible changes after this commit. gdb/ChangeLog: * exec.c (exec_target::close): Call new clear_target_sections function. (program_space::add_target_sections): Update name of member variable. (program_space::foreach_target_section): New function. (program_space::add_target_sections): Update name of member variable. (program_space::remove_target_sections): Likewise. (exec_one_fork): Use new target_sections member function. (exec_target::get_section_table): Likewise. (exec_target::files_info): Likewise. (set_section_command): Use new foreach_target_section member function. (exec_set_section_address): Likewise. (exec_target::has_memory): Use new target_sections member function. * progspace.h (program_space::clear_target_sections): New member function. (program_space::target_sections): Rename member variable to m_target_sections, replace with a new member function. (program_space::foreach_target_section): Declare new member function. (program_space::m_target_sections): New member variable. * solib-dsbt.c (scan_dyntag): Use new member function. * solib-svr4.c (scan_dyntag): Likewise.
-
Andrew Burgess authored
During review of the next patch (which changes the 'section' command), a bug was pointed out. I wondered why no tests spotted this bug and I found that the 'section' command test (sect-cmd.exp) is only run on hppa targets! In this commit I have given this test script a bit of a spring clean, bringing it up to date with current testsuite style. I have made some of the patterns a little more robust, but in general my intention was not to change the underlying meaning of any of these tests. gdb/testsuite/ChangeLog: * gdb.base/sect-cmd.exp: Rewrite using modern testsuite techniques. Enable the test for all targets.
-
Andrew Burgess authored
The code to access the target section table can be made more const, so lets do that. There should be no user visible changes after this commit. gdb/ChangeLog: * gdb/bfd-target.c (class target_bfd) <get_section_table>: Make return type const. * gdb/exec.c (struct exec_target) <get_section_table>: Likewise. (section_table_read_available_memory): Make local const. (exec_target::xfer_partial): Make local const. (print_section_info): Make parameter const. * gdb/exec.h (print_section_info): Likewise. * gdb/ppc64-tdep.c (ppc64_convert_from_func_ptr_addr): Make local const. * gdb/record-btrace.c (record_btrace_target::xfer_partial): Likewise. * gdb/remote.c (remote_target::remote_xfer_live_readonly_partial): Likewise. * gdb/s390-tdep.c (s390_load): Likewise. * gdb/solib-dsbt.c (scan_dyntag): Likewise. * gdb/solib-svr4.c (scan_dyntag): Likewise. * gdb/target-debug.h (target_debug_print_target_section_table_p): Rename to... (target_debug_print_const_target_section_table_p): ...this. * gdb/target-delegates.c: Regenerate. * gdb/target.c (target_get_section_table): Make return type const. (target_section_by_addr): Likewise. Also make some locals const. (memory_xfer_partial_1): Make some locals const. * gdb/target.h (struct target_ops) <get_section_table>: Make return type const. (target_section_by_addr): Likewise. (target_get_section_table): Likewise.
-
Andrew Burgess authored
We already have a command 'maint info sections', this command prints all sections from all known object files. However, GDB maintains a second section table internally. This section table is used when GDB wants to read directly from an object file rather than actually reading memory on the target. As such only some sections (the allocatable ones) are added to this secondary section table. I recently ran into a situation where some of GDB's optimisations for reading directly from the files were not working. In 'maint info sections' I could see that GDB knew about the object file, and did know about the sections that it _should_ have been reading from. But I couldn't ask GDB which sections it had copied into its secondary section table. This commit adds a new command 'maint info target-sections' that fills this gap. This command lists only those sections that GDB has copied into its secondary table. You'll notice that the testsuite includes a comment indicating that there's a bug in GDB. Normally this is not something I would add to the testsuite, instead we should raise an actual bugzilla bug and then mark an xfail, however, a later patch in this series will remove this comment once the actual bug in GDB is fixed. gdb/ChangeLog: * NEWS: Mention new 'maint info target-sections' command. * maint.c (maintenance_info_target_sections): New function. (_initialize_maint_cmds): Register new command. gdb/doc/ChangeLog: * gdb.texinfo (Files): Document new 'maint info target-sections' command. gdb/testsuite/ChangeLog: * gdb.base/maint-info-sections.exp: Add new tests. (check_maint_info_target_sections_output): New proc.
-
Andrew Burgess authored
GDB for RISC-V always uses target descriptions. When the target doesn't provide a target description then a default is selected. Usually this default is selected based on the properties of the executable being debugged. However, when there is no executable being debugged we currently fallback to the riscv:rv64 target description as the default. This leads to strange behaviour like this: $ gdb (gdb) set architecture riscv:rv32 (gdb) p sizeof ($pc) $1 = 8 Despite the users specifically setting the architecture to riscv:rv32 GDB still thinks that the target has riscv:rv64 register sizes. The above is a bit of a contrived situation. I actually ran into this situation while trying to connect to a running riscv:rv32 target without supplying an executable (the target didn't provide a target description). When I tried to set a register on the target I ran into errors because GDB was passing 8 bytes to the target rather than the expected 4. Even when I manually specified the architecture (as above) I couldn't convince GDB to only send 4 bytes. This patch fixes this issue. Now, when we selected a default target description we will make use of the user selected architecture to guide our choice. In the above example we now get: $ gdb (gdb) set architecture riscv:rv32 (gdb) p sizeof ($pc) $1 = 4 And my real world example of connecting to a remote without an executable works fine. I've used the fact that we can ask GDB about $pc even when no executable is loaded as the basis for a test to cover this situation. gdb/ChangeLog: * riscv-tdep.c (riscv_features_from_gdbarch_info): Rename to... (riscv_features_from_bfd): ...this. Change parameter type to 'bfd*', and update as required. (riscv_find_default_target_description): Update call to riscv_features_from_bfd. Select a default xlen based on info.bfd_arch_info. (riscv_gdbarch_init): Update call to riscv_features_from_bfd. gdb/testsuite/ChangeLog: * gdb.arch/riscv-default-tdesc.exp: New file.
-
- Feb 24, 2021
-
-
Andrew Burgess authored
When evaluating and expression containing UNOP_IND in mode EVAL_AVOID_SIDE_EFFECTS, GDB currently (mostly) returns the result of a call to value_zero meaning we get back an object with the correct type, but its contents are all zero. If the target type contains fields with dynamic type then in order to resolve these dynamic fields GDB will need to read the value of the field from within the parent object. In this case the field value will be zero as a result of the call to value_zero mentioned above. The idea behind EVAL_AVOID_SIDE_EFFECTS is to avoid the chance that doing something like `ptype` will modify state within the target, for example consider: ptype i++. However, there is already precedence within GDB that sometimes, in order to get accurate type results, we can't avoid reading from the target, even when EVAL_AVOID_SIDE_EFFECTS is in effect. For example I would point to eval.c:evaluate_var_value, the handling of OP_REGISTER, the handling of value_x_unop in many places. I believe the Ada expression evaluator also ignore EVAL_AVOID_SIDE_EFFECTS in some cases. I am therefor proposing that, in the case where a pointer points at a dynamic type, we allow UNOP_IND to perform the actual indirection. This allows accurate types to be displayed in more cases. gdb/ChangeLog: * eval.c (evaluate_subexp_standard): Call value_ind for points to dynamic types in UNOP_IND. gdb/testsuite/ChangeLog: * gdb.fortran/pointer-to-pointer.exp: Additional tests.
-
Nick Clifton authored
PR 27285 * od-elf32_avr.c (elf32_avr_get_memory_usage): Check for overflows when adding together the section sizes.
-
Nick Clifton authored
* objcopy.c (merge_gnu_build_notes): Remove support for v1/v2 GNU build notes. * readelf.c (print_gnu_build_attribute_description): Likewise.
-
Alan Modra authored
PR 27459 * coff-stgo32.c (go32exe_check_format): Sanity check size of header to avoid a buffer overflow.
-
GDB Administrator authored
-
Alan Modra authored
A number of targets, bfin, rl78, rx, can show odd failures when bfd/reloc.c changes BFD_RELOC_* enum values, if recompiling over a build dir with existing objects. The problem is caused by bfin-parse.o and similar not being recompiled and so using stale BFD_RELOC_* values. This isn't fixed by making bfin-parse.c depend on bfd/reloc.c, which isn't necessary anyway. bfin-parse.o should have been recompiled due to bfd/bfd.h changing when extracted bfd/reloc.c BFD_RELOC_* values change, but that wasn't happening. The problem is that automake generates a makefile that loads gas/config/.deps/ dependency file for objects with corresponding sources mentioned in EXTRA_as_new_SOURCES. Unless we want to mess around generating explicit dependencies, I think that mean moving some object files to the build gas/config/. This patch does that, removing some hacks for m68k-parse.c that should no longer be necessary, and removes some rules that catered to old bison producing code that triggers compiler warnings. PR 23691 * Makefile.am (TARGET_CPU_CFILES): Split off config/xtensa-relax.c.. (TARGET_CPU_HFILES): ..and config/xtensa-relax.h.. (TARGET_EXTRA_FILES): ..to here. Add config/bfin-lex-wrapper.c, and use alongside TARGET_CPU_CFILES. (EXTRA_DIST): Update location of generated .c files. (config/m68k-parse.c): New rule replacing m68k-parse.c rule. (config/bfin-parse.c, config/rl78-parse.cm config/rx-parse.c), (config/bfin-lex.c, config/bfin-lex-wrapper.@OBJEXT@): Similarly. (itbl-lex-wrapper.@OBJEXT@): Simplify to just the needed dependencies. (itbl-parse.@OBJEXT@): Delete rule using NO_WERROR. (itbl-parse.c, itbl-parse.h): Tidy. * config/bfin-lex-wrapper.c: Include config/bfin-lex.c. * config/bfin-lex.l: Include config/bfin-parse.h. * configure.ac (extra_objects): Move object files corresponding to .y and .l files now in config/ to config/. * Makefile.in: Regenerate. * configure: Regenerate. * po/POTFILES.in: Regenerate.
-
Alan Modra authored
This patch makes use of the temp file descriptor in smart_rename rather than reopening the file. I don't believe there is a security issue in reopening the file, but this way is one less directory operation. The patch also attempts to preserve S_ISUID and S_ISGID. PR 27456 * bucomm.h (smart_rename): Update prototype. * rename.c (smart_rename): Add fromfd and preserve_dates params. Pass fromfd and target_stat to simple_copy. Call set_times when preserve_dates. (simple_copy): Accept fromfd rather than from filename. Add target_stat param. Rewind fromfd rather than opening. Open "to" file without O_CREAT. Try to preserve S_ISUID and S_ISGID. * ar.c (write_archive): Rename ofd to tmpfd. Dup tmpfd before closing output temp file, and pass tmpfd to smart_rename. * arsup.c (temp_fd): Rename from real_fd. (ar_save): Dup temp_fd and pass to smart_rename. * objcopy.c (strip_main, copy_main): Likewise, and pass preserve_dates.
-
Alan Modra authored
PR 27456 * rename.c: Tidy throughout. (smart_rename): Always copy. Remove windows specific code.
-
Simon Marchi authored
As described in the log of patch "gdb/dwarf: add assertion in maybe_queue_comp_unit", it would happen that a call to maybe_queue_comp_unit would enqueue a CU in the to-expand queue while nothing up the stack was processing the queue. This is not desirable, as items are then left lingering in the queue when we exit the dwarf2/read code. This is an inconsistent state. The normal case of using the queue is when we go through dw2_do_instantiate_symtab and process_queue. As depended-on CUs are found, they get added to the queue. process_queue expands CUs until the queue is empty. To catch these cases where things are enqueued while nothing up the stack is processing the queue, change dwarf2_per_bfd::queue to be an optional. The optional is instantiated in dwarf2_queue_guard, just before where we call process_queue. In the dwarf2_queue_guard destructor, the optional gets reset. Therefore, the queue object is instantiated only when something up the stack is handling it. If another entry point tries to enqueue a CU for expansion, an assertion will fail and we know we have something to fix. dwarf2_queue_guard sounds like the good place for this, as it's currently responsible for making sure the queue gets cleared if we exit due to an error. This also allows asserting that when age_comp_units or remove_all_cus run, the queue is not instantiated, and gives us one more level of assurance that we won't free the DIEs of a CU that is in the CUs-to-expand queue. gdb/ChangeLog: PR gdb/26828 * dwarf2/read.c (dwarf2_queue_guard) <dwarf2_queue_guard>: Instantiate queue. (~dwarf2_queue_guard): Clear queue. (queue_comp_unit): Assert that queue is instantiated. (process_queue): Adjust. * dwarf2/read.h (struct dwarf2_per_bfd) <queue>: Make optional. Change-Id: I8fe3d77845bb4ad3d309eac906acebe79d9f0a9d
-
Simon Marchi authored
The previous commit log described how items could be left lingering in the dwarf2_per_bfd::queue and how that could cause trouble. This patch fixes the issue by changing maybe_queue_comp_unit so that it doesn't put a CU in the to-expand queue if that CU is already expanded. This will make it so that when dwarf2_fetch_die_type_sect_off calls follow_die_offset and maybe_queue_comp_unit, it won't enqueue the target CU, because it will see the CU is already expanded. This assumes that if a CU is dwarf2_fetch_die_type_sect_off's target CU, it will have previously been expanded. I think it is the case, but I can't be 100% sure. If that's not true, the assertions added in the following patch will catch it, and it means we'll have to re-think a bit more how things work (it wouldn't be well handled at all today anyway). This fixes something else in maybe_queue_comp_unit that looks wrong. Imagine the DIEs of a CU are loaded in memory, but that CU is not expanded. In that case, maybe_queue_comp_unit will use this early return: /* If the compilation unit is already loaded, just mark it as used. */ dwarf2_cu *cu = per_objfile->get_cu (per_cu); if (cu != nullptr) { cu->last_used = 0; return 0; } ... so the CU won't be queued for expansion. Whether the DIEs of a CU are loaded in memory and whether that CU is expanded are two orthogonal things, but that function appears to mix them. So, move the queuing above that check / early return, so that if the CU's DIEs are loaded in memory but the CU is not expanded yet, it gets enqueued. I tried to improve maybe_queue_comp_unit's documentation to clarify what the return value means. By clarifying this, I noticed that two callers (follow_die_offset and follow_die_sig_1) access the CU's DIEs after calling maybe_queue_comp_unit, only relying on maybe_queue_comp_unit's return value to tell whether DIEs need to be loaded first or not. As explained in the new comment, this is problematic: maybe_queue_comp_unit's return value doesn't tell whether DIEs are currently loaded, it means whether maybe_queue_comp_unit requires the caller to load them. If the CU is already expanded but the DIEs to have been freed, maybe_queue_comp_unit returns 0, meaning "I don't need you to load the DIEs". So if these two functions (follow_die_offset and follow_die_sig_1) need to access the DIEs in any case, for their own usage, they should make sure to load them if they are not loaded already. I therefore added an extra check to the condition they use, making it so they will always load the DIEs if they aren't already. From what I found, other callers don't care for the CU's DIEs, they call maybe_queue_comp_unit to ensure the CU gets expanded eventually, but don't care for it after that. gdb/ChangeLog: PR gdb/26828 * dwarf2/read.c (maybe_queue_comp_unit): Check if CU is expanded to decide whether or not to enqueue it for expansion. (follow_die_offset, follow_die_sig_1): Ensure we load the DIEs after calling maybe_queue_comp_unit. Change-Id: Id98c6b60669f4b4b21b9be16d0518fc62bdf686a
-
- Feb 23, 2021
-
-
Simon Marchi authored
Same as the previous patch, but for GDBserver. The return value of this method is never used, change it to return void. gdbserver/ChangeLog: * linux-low.cc (linux_process_target::filter_event): Return void. * linux-low.h (class linux_process_target) <filter_event>: Return void. Change-Id: I79e5dc04d9b21b9f01c6d675fa463d1b1a703b3a
-
Simon Marchi authored
I noticed that linux_nat_filter_event returns a value, but its caller doesn't use it. This has been since 9c02b525 ("linux-nat.c: better starvation avoidance, handle non-stop mode too"). Before that commit, the return value was used to tell the caller whether to continue processing that event or not. But since then, the model is that we pull all events from the kernel and linux_nat_filter_event just saves the status to the lwp_info structure if it thinks it's relevant. And the caller, linux_nat_wait_1, selects a status at random amongst the threads with a pending status. So essentially, the return value of linux_nat_filter_event does not have a reason to be anymore. Change it so it returns void. gdb/ChangeLog: * linux-nat.c (linux_nat_filter_event): Return void. Change-Id: I35662868910f5122772ed92a512adfbf4da12d87
-
GDB Administrator authored
-
Tom Tromey authored
While looking at Andrew's recent target sections series, I saw that target_bfd_reopen took a "bfd *", leading to a call to new_reference. However, because the only caller of target_bfd_reopen is already using gdb_bfd_ref_ptr, this code can be simplified and the explicit call to new_reference can be removed. gdb/ChangeLog 2021-02-22 Tom Tromey <tromey@adacore.com> * solib-svr4.c (enable_break): Update. * bfd-target.c (class target_bfd) <target_bfd>: Change parameter type. (target_bfd_reopen): Change parameter type. * bfd-target.h (target_bfd_reopen): Change parameter type.
-
Simon Marchi authored
Unlike the previous patch, I don't propose that we take this patch into gdb-10-branch. This patch adds two asserts, prompted by investigating and fixing the bug fixed by the previous patch. The assert in find_thread_ptid would have caught the original issue before the segfault (I think it's slightly more use friendly). The assert in add_thread_silent would have made it clear that the solution proposed in [1] isn't the right one. The solution ended up passing nullptr as a target to add_thread. We don't want that, because add_thread_silent uses it to look up the inferior to which to add the thread. If the target is nullptr, we could find an inferior with the same pid, but belonging to an unrelated target. So we always want a non-nullptr target in add_thread_silent. gdb/ChangeLog: * thread.c (add_thread_silent): Add assert. (find_thread_ptid): Add assert. [1] https://sourceware.org/pipermail/gdb-patches/2021-February/176202.html Change-Id: Ie593ee45c5eb02235e8e9fbcda612d48ce883852
-
Simon Marchi authored
Since this is a GDB 9 -> 10 regression, I would like to push it to gdb-10-branch. This is a follow-up to: https://sourceware.org/pipermail/gdb-patches/2021-February/176202.html This patch fixes a segfault seen when attaching to a process on Solaris. The steps leading to the segfault are: - procfs_target::attach calls do_attach, at this point the inferior's process slot in the target stack is empty. - do_attach adds a thread with `add_thread (&the_procfs_target, ptid)` - in add_thread_silent, the passed target (&the_procfs_target) is passed to find_inferior_ptid - find_inferior_ptid returns nullptr, as there is no inferior with this ptid that has &the_procfs_target as its process target - the nullptr `inf` is passed to find_thread_ptid, which dereferences it, causing a segfault - back in procfs_target::attach, after do_attach, we push the the_procfs_target on the inferior's target stack, although we never reach this because the segfault happens before. To fix this, I think we need to do the same as is done in inf_ptrace_target::attach: push the target early and unpush it in case the attach fails (and keep it if the attach succeeds). Implement it by moving target_unpush_up to target.h, so it can be re-used here. Make procfs_target::attach use it. Note that just like is mentioned in inf_ptrace_target::attach, we should push the target before calling target_pid_to_str, so that calling target_pid_to_str ends up in procfs_target::pid_to_str. Tested by trying to attach on a process on gcc211 on the gcc compile farm. gdb/ChangeLog: PR gdb/27435 * inf-ptrace.c (struct target_unpusher): Move to target.h. (target_unpush_up): Likewise. * procfs.c (procfs_target::attach): Push target early. Use target_unpush_up to unpush target in case of error. * target.h (struct target_unpusher): Move here. (target_unpush_up): Likewise. Change-Id: I88aff8b20204e1ca1d792e27ac6bc34fc1aa0d52
-
- Feb 22, 2021
-
-
Alan Modra authored
It's not needed for sizing fixups since 0e2779e9, and wrong to emit this reloc to the object file. * coff-rs6000.c (_bfd_xcoff_reloc_type_lookup): Remove BFD_RELOC_16. * coff64-rs6000.c (xcoff64_reloc_type_lookup): Likewise.
-
GDB Administrator authored
-
- Feb 21, 2021
-
-
Mike Frysinger authored
This file is quite large and is getting unmanageable. Split it apart to follow aclocal best practices by putting one-macro-per-file. There shouldn't be any real functional changes here as can be seen in the configure script regens.
-
Alan Modra authored
Note that we don't even warn if scripts adjust a symbol as in ld-elf/var1 and ld-scripts/pr14962. include/ * bfdlink.h (struct bfd_link_info): Add warn_multiple_definition. ld/ * ldexp.c (exp_fold_tree_1): Warn on script defining a symbol defined in an object file. * ldmain.c (multiple_definition): Heed info->warn_multiple_definition. * testsuite/ld-scripts/defined5.d: Expect a warning.
-
Alan Modra authored
AC_CANONICAL_TARGET is needed for @target@ substitution in the makefile. AC_CANONICAL_HOST and AC_CANONICAL_BUILD are alread invoked indirectly, make them explicit. * configure.ac: Invoke AC_CANONICAL_TARGET, AC_CANONICAL_HOST and AC_CANONICAL_BUILD. * configure: Regenerate. * Makefile.in: Regenerate.
-
GDB Administrator authored
-
Nick Alcock authored
I'm dividing this into three groups for now: new features, bugfixes, and bugfixes also present on a stable branch. Only user-visible bugfixes, not build-system fixes, are listed.
-
Nick Alcock authored
The existing ctf_lookup_by_symbol and ctf_arc_lookup_symbol functions suffice to look up the types of symbols if the caller already has a symbol number. But the caller often doesn't have one of those and only knows the name of the symbol: also, in object files, the caller might not have a useful symbol number in any sense (and neither does libctf: the 'symbol number' we use in that case literally starts at 0 for the lexicographically first-sorted symbol in the symtypetab and counts those symbols, so it corresponds to nothing useful). This means that even though object files have a symtypetab (generated by the compiler or by ld -r), the only way we can look up anything in it is to iterate over all symbols in turn with ctf_symbol_next until we find the one we want. This is unhelpful and pointlessly inefficient. So add a pair of functions to look up symbols by name in a dict and in a whole archive: ctf_lookup_by_symbol_name and ctf_arc_lookup_symbol_name. These are identical to the existing functions except that they take symbol names rather than symbol numbers. To avoid insane repetition, we do some refactoring in the process, so that both ctf_lookup_by_symbol and ctf_arc_lookup_symbol turn into thin wrappers around internal functions that do both lookup by symbol index and lookup by name. This massively reduces code duplication because even the existing lookup-by-index stuff wants to use a name sometimes (when looking up in indexed sections), and the new lookup-by-name stuff has to turn it into an index sometimes (when looking up in non-indexed sections): doing it this way lets us share most of that. The actual name->index lookup is done by ctf_lookup_symbol_idx. We do not anticipate this lookup to be as heavily used as ld.so symbol lookup by many orders of magnitude, so using the ELF symbol hashes would probably take more time to read them than is saved by using the hashes, and it adds a lot of complexity. Instead, do a linear search for the symbol name, caching all the name -> index mappings as we go, so that future searches are likely to hit in the cache. To avoid having to repeat this search over and over in a CTF archive when ctf_arc_lookup_symbol_name is used, have cached archive lookups (the sort done by ctf_arc_lookup_symbol* and the ctf_archive_next iterator) pick out the first dict they cache in a given archive and store it in a new ctf_archive field, ctfi_crossdict_cache. This can be used to store cross-dictionary cached state that depends on things like the ELF symbol table rather than the contents of any one dict. ctf_lookup_symbol_idx then caches its name->index mappings in the dictionary named in the crossdict cache, if any, so that ctf_lookup_symbol_idx in other dicts in the same archive benefit from the previous linear search, and the symtab only needs to be scanned at most once. (Note that if you call ctf_lookup_by_symbol_name in one specific dict, and then follow it with a ctf_arc_lookup_symbol_name, the former will not use the crossdict cache because it's only populated by the dict opens in ctf_arc_lookup_symbol_name. This is harmless except for a small one-off waste of memory and time: it's only a cache, after all. We can fix this later by using the archive caching machinery more aggressively.) In ctf-archive, we do similar things, turning ctf_arc_lookup_symbol into a wrapper around a new function that does both index -> ID and name -> ID lookups across all dicts in an archive. We add a new ctfi_symnamedicts cache that maps symbol names to the ctf_dict_t * that it was found in (so that linear searches for symbols don't need to be repeated): but we also *remove* a cache, the ctfi_syms cache that was memoizing the actual ctf_id_t returned from every call to ctf_arc_lookup_symbol. This is pointless: all it saves is one call to ctf_lookup_by_symbol, and that's basically an array lookup and nothing more so isn't worth caching. (Equally, given that symbol -> index mappings are cached by ctf_lookup_by_symbol_name, those calls are nearly free after the first call, so there's no point caching the ctf_id_t in that case either.) We fix up one test that was doing manual symbol lookup to use ctf_arc_lookup_symbol instead, and enhance it to check that the caching layer is not totally broken: we also add a new test to do lookups in a .o file, and another to do lookups in an archive with conflicted types and make sure that sort of multi-dict lookup is actually working. include/ChangeLog 2021-02-17 Nick Alcock <nick.alcock@oracle.com> * ctf-api.h (ctf_arc_lookup_symbol_name): New. (ctf_lookup_by_symbol_name): Likewise. libctf/ChangeLog 2021-02-17 Nick Alcock <nick.alcock@oracle.com> * ctf-impl.h (ctf_dict_t) <ctf_symhash>: New. <ctf_symhash_latest>: Likewise. (struct ctf_archive_internal) <ctfi_crossdict_cache>: New. <ctfi_symnamedicts>: New. <ctfi_syms>: Remove. (ctf_lookup_symbol_name): Remove. * ctf-lookup.c (ctf_lookup_symbol_name): Propagate errors from parent properly. Make static. (ctf_lookup_symbol_idx): New, linear search for the symbol name, cached in the crossdict cache's ctf_symhash (if available), or this dict's (otherwise). (ctf_try_lookup_indexed): Allow the symname to be passed in. (ctf_lookup_by_symbol): Turn into a wrapper around... (ctf_lookup_by_sym_or_name): ... this, supporting name lookup too, using ctf_lookup_symbol_idx in non-writable dicts. Special-case name lookup in dynamic dicts without reported symbols, which have no symtab or dynsymidx but where name lookup should still work. (ctf_lookup_by_symbol_name): New, another wrapper. * ctf-archive.c (enosym): Note that this is present in ctfi_symnamedicts too. (ctf_arc_close): Adjust for removal of ctfi_syms. Free the ctfi_symnamedicts. (ctf_arc_flush_caches): Likewise. (ctf_dict_open_cached): Memoize the first cached dict in the crossdict cache. (ctf_arc_lookup_symbol): Turn into a wrapper around... (ctf_arc_lookup_sym_or_name): ... this. No longer cache ctf_id_t lookups: just call ctf_lookup_by_symbol as needed (but still cache the dicts those lookups succeed in). Add lookup-by-name support, with dicts of successful lookups cached in ctfi_symnamedicts. Refactor the caching code a bit. (ctf_arc_lookup_symbol_name): New, another wrapper. * ctf-open.c (ctf_dict_close): Free the ctf_symhash. * libctf.ver (LIBCTF_1.2): New version. Add ctf_lookup_by_symbol_name, ctf_arc_lookup_symbol_name. * testsuite/libctf-lookup/enum-symbol.c (main): Use ctf_arc_lookup_symbol rather than looking up the name ourselves. Fish it out repeatedly, to make sure that symbol caching isn't broken. (symidx_64): Remove. (symidx_32): Remove. * testsuite/libctf-lookup/enum-symbol-obj.lk: Test symbol lookup in an unlinked object file (indexed symtypetab sections only). * testsuite/libctf-writable/symtypetab-nonlinker-writeout.c (try_maybe_reporting): Check symbol types via ctf_lookup_by_symbol_name as well as ctf_symbol_next. * testsuite/libctf-lookup/conflicting-type-syms.*: New test of lookups in a multi-dict archive.
-