aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2022-01-01Update Copyright Year in gdb, gdbserver and gdbreplay version outputJoel Brobecker3-3/+3
This commit changes the copyright year printed by gdb, gdbserver and gdbreplay when printing the tool's version.
2022-01-01ubsan: next_char_of_string signed integer overflowAlan Modra1-2/+2
Squash another totally useless fuzz report that I should have ignored. * read.c (next_char_of_string): Avoid integer overflow.
2022-01-01ubsan: bfd_mach_o_build_commands shift exponent 64 is too largeAlan Modra1-8/+8
* mach-o.c (bfd_mach_o_read_section_32): Limit alignment further. (bfd_mach_o_read_section_64): Likewise.
2022-01-01ubsan: signed integer multiply overflowAlan Modra1-1/+6
9223371018427387904 * 2 cannot be represented in type 'long', yes, but we don't care. * expr.c (expr): Avoid signed overflow.
2022-01-01asan: Null-dereference in _bfd_xcoff_copy_private_bfd_dataAlan Modra1-2/+2
sec->output_section will be NULL when objcopy removes sections. * coff-rs6000.c (_bfd_xcoff_copy_private_bfd_data): Protect against objcopy removing sections.
2022-01-01ubsan: integer overflow in section filepos subtractionAlan Modra1-2/+1
* elf.c (assign_file_positions_for_non_load_sections): Avoid signed integer overflow.
2022-01-01Remove unnecessary ELF_MINPAGESIZE definesAlan Modra4-18/+1
The idea of this patch is to make it easy to see which targets (just sparc) have ELF_MINPAGESIZE != ELF_COMMONPAGESIZE. * elf32-arm.c (ELF_MINPAGESIZE): Don't define. * elf32-metag.c: Likewise. * elfnn-aarch64.c: Likewise. * elf64-x86-64.c: Likewise. Also don't redefine a bunch of other macros for l1om elf64-target.h use that are unchanged from default.
2021-12-31ld-x86-64: Pass options to linker with "-Wl,"H.J. Lu1-20/+24
* testsuite/ld-x86-64/x86-64.exp: Pass options to linker with "-Wl,".
2022-01-01Automatic date update in version.inGDB Administrator1-1/+1
2021-12-31Do not call reinitialize_more_filter from avr_io_reg_read_commandTom Tromey1-2/+0
avr_io_reg_read_command is an ordinary gdb command, and so should not be calling reinitialize_more_filter. This patch removes it. I'm checking this in as obvious. Tested by rebuilding.
2021-12-31x86: Define check_relocs_failed in elfxx-x86.hH.J. Lu3-8/+4
* elf32-i386.c (check_relocs_failed): Moved to ... * elfxx-x86.h (check_relocs_failed): Here. New. * elf64-x86-64.c (check_relocs_failed): Removed.
2021-12-31Define X86_PCREL_TYPE_P/X86_SIZE_TYPE_P in elfxx-x86.hH.J. Lu4-38/+41
* elf32-i386.c: Don't include "elf/i386.h". (X86_PCREL_TYPE_P): Removed. (X86_SIZE_TYPE_P): Likewise. (elf_i386_check_relocs): Pass false to NEED_DYNAMIC_RELOCATION_P. (elf_i386_relocate_section): Pass false to GENERATE_DYNAMIC_RELOCATION_P and COPY_INPUT_RELOC_P. * elf64-x86-64.c: Don't include "elf/x86-64.h". (X86_PCREL_TYPE_P): Removed. (X86_SIZE_TYPE_P): Likewise. (elf_x86_64_check_relocs): Pass true to NEED_DYNAMIC_RELOCATION_P and X86_PCREL_TYPE_P. (elf_x86_64_relocate_section): Pass true to X86_PCREL_TYPE_P, X86_SIZE_TYPE_P, GENERATE_DYNAMIC_RELOCATION_P and COPY_INPUT_RELOC_P. * elfxx-x86.c: Don't include "elf/i386.h" nor "elf/x86-64.h". * elfxx-x86.h (X86_64_PCREL_TYPE_P): New. (I386_PCREL_TYPE_P): Likewise. (X86_PCREL_TYPE_P): Likewise. (X86_64_SIZE_TYPE_P): Likewise. (I386_SIZE_TYPE_P): Likewise. (X86_SIZE_TYPE_P): Likewise. (NEED_DYNAMIC_RELOCATION_P): Add IS_X86_64 and pass it to X86_PCREL_TYPE_P. (COPY_INPUT_RELOC_P): Likewise. (GENERATE_DYNAMIC_RELOCATION_P): Add IS_X86_64, pass it to X86_PCREL_TYPE_P and X86_SIZE_TYPE_P.
2021-12-31ld: fix coff PE SEHTamar Christina1-2/+2
COFF_WITH_pex64 and COFF_WITH_peAArch64 can't be true at the same time. That means that two conditionals that control the sorting of the .pdata section became a falsum. The testsuite doesn't catch this because the linker does the sorting and to link you require library support from the unwinder so we can't test from binutils in isolation. bfd/ChangeLog: 2021-12-31 Tamar Christina <tamar.christina@arm.com> PR ld/28682 * peXXigen.c: Fix conditional.
2021-12-31Automatic date update in version.inGDB Administrator1-1/+1
2021-12-30Automatic date update in version.inGDB Administrator1-1/+1
2021-12-29Use filtered output in show callbacksTom Tromey4-15/+15
"show" command callbacks, like most ordinary gdb commands, should use filtered output. I found a few that did not, so this patch changes them to use the filtered form.
2021-12-29Consistently Use ui_file parameter to show callbacksTom Tromey11-42/+46
I happened to notice that one "show" callback was printing to gdb_stdout rather than to the passed-in ui_file parameter. I went through all such callbacks and fixed them to consistently use the ui_file. Regression tested on x86-64 Fedora 34.
2021-12-29Use gdb_stdlog for MI debuggingTom Tromey2-7/+4
When MI debugging is enabled, the logging output should be sent to gdb_stdlog. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Use debug_prefixed_printf_cond_nofunc in index-cacheTom Tromey1-24/+19
This changes index-cache.c to use debug_prefixed_printf_cond_nofunc. As a side effect, logs are now written to gdb_stdlog. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Send minsym logging to gdb_stdlogTom Tromey1-3/+4
This changes minsyms.c to send logging output to gdb_stdlog. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Use gdb_stdlog for separate debug file loggingTom Tromey2-18/+23
This changes the separate debug file logging code (spread across two files) to use gdb_stdlog for its output. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Use debug_prefixed_printf_cond_nofunc in machoreadTom Tromey1-23/+14
This changes machoread.c to use debug_prefixed_printf_cond_nofunc. As a side effect, the logs are now written to gdb_stdlog. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Use debug_prefixed_printf_cond_nofunc in microblaze.cTom Tromey1-12/+3
This changes microblaze.c to use the standard logging macro. As a side effect, logs will now go to gdb_stdlog. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Send debugging data to gdb_stdlog in mips-linux-nat.cTom Tromey1-16/+17
This changes mips-linux-nat.c to send some logging output to gdb_stdlog, rather than stdout. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Send arch-utils error messages to gdb_stderrTom Tromey1-4/+7
This changes arch-utils.c to send some error messages to gdb_stderr. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Use correct stream for process record outputTom Tromey13-84/+113
The process record code often emits unfiltered output. In some cases, this output ought to go to gdb_stderr (but see below). In other cases, the output is guarded by a logging variable and so ought to go to gdb_stdlog. This patch makes these changes. Note that in many cases, the output to stderr is followed by a "return -1", which is how process record indicates an error. It seems to me that calling error here would be preferable, because, in many cases, that's all the caller does when it sees a -1. However, I haven't made this change. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Send jit.c errors to gdb_stderrTom Tromey1-10/+13
jit.c writes some error messages to gdb_stdout, but using gdb_stderr is better. This is part of PR gdb/7233. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
2021-12-29Fix logging redirection bug with pagerTom Tromey3-1/+72
I noticed yesterday that if gdb output is redirected to a file, the pager will still be active. This is irritating, because the output isn't actually visible -- just the pager prompt. Looking in bugzilla, I found that this had been filed 17 years ago, as PR cli/8798. This patch fixes the bug. It changes the pagination code to query the particular ui-file to see if paging is allowable. The ui-file implementations are changed so that only the stdout implementation and a tee (where one sub-file is stdout) can page. Regression tested on x86-64 Fedora 34. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=8798
2021-12-29Remove unusual use of core_addr_eq and core_addr_hashTom Tromey3-27/+2
gdbtypes.h uses core_addr_eq and core_addr_hash in a weird way: taking the address of a member and then passing this (as a void*) to these functions. It seems better to simply inline the ordinary code here. CORE_ADDR is a scalar so it can be directly compared, and the identity hash function seems safe to assume as well. After this, core_addr_eq and core_addr_hash are unused, so this patch removes them.
2021-12-29gdb: Copy inferior properties in clone-inferiorLancelot SIX4-2/+123
This commit ensures that the following settings are cloned from one inferior to the new one when processing the clone-inferior command: - inferior-tty - environment variables - cwd - args Some of those parameters can be passed as command line arguments to GDB (-args and -tty), so one could expect the clone-inferior to respect those flags. The following debugging session illustrates that: gdb -nx -quiet -batch \ -ex "show args" \ -ex "show inferior-tty" \ -ex "clone-inferior" \ -ex "inferior 2" \ -ex "show args" \ -ex "show inferior-tty" \ -tty=/some/tty \ -args echo foo bar Argument list to give program being debugged when it is started is "foo bar". Terminal for future runs of program being debugged is "/some/tty". [New inferior 2] Added inferior 2. [Switching to inferior 2 [<null>] (/bin/echo)] Argument list to give program being debugged when it is started is "". Terminal for future runs of program being debugged is "". The other properties this commit copies on clone (i.e. CWD and the environment variables) are included since they are related (in the sense that they influence the runtime behavior of the program) even if they cannot be directly set using command line switches. There is a chance that this patch changes existing user workflow. I think that this change is mostly harmless. If users want to start a new inferior based on an existing one, they probably already propagate those settings to the new inferior in some way. Tested on x86_64-linux. Change-Id: I3b1f28b662f246228b37bb24c2ea1481567b363d
2021-12-29Automatic date update in version.inGDB Administrator1-1/+1
2021-12-28elf32-i386: Fix a typo in GOT commentsH.J. Lu1-1/+1
Entry offsets in the global offset table are multiples of 4, not 8. * elf32-i386.c (elf_i386_relocate_section): Fix a typo in GOT comments.
2021-12-28bfd: Don't check non-thin archive member file sizeH.J. Lu1-2/+4
There is no need to check member file size for thin archive member. * bfdio.c (bfd_bread): Don't check non-thin archive member file size.
2021-12-28gas reloc sortingAlan Modra6-45/+31
In some cases, eg. riscv_pre_output_hook, gas generates out-of-order relocations. Various places in the linker assume relocs are sorted by increasing r_offset, which is normally the case. Provide GAS_SORT_RELOCS to handle unsorted relocs. bfd/ PR 28709 * elf32-nds32.c (nds32_insertion_sort): Make static. * elf32-nds32.h (nds32_insertion_sort): Delete declaration. gas/ PR 28709 * write.c (write_relocs): Implement reloc sorting by r_offset when GAS_SORT_RELOCS. * config/tc-nds32.c (compar_relent, nds32_set_section_relocs): Delete. * config/tc-nds32.h (nds32_set_section_relocs): Don't declare. (SET_SECTION_RELOCS): Don't define. (GAS_SORT_RELOCS): Define. * config/tc-riscv.h (GAS_SORT_RELOCS): Define.
2021-12-28ld: Fix testcase errors due to -shared not support.jiawei2-1/+6
Reviewed-by: Jim Wilson <jim.wilson.gcc@gmail.com> ld/ChangeLog: * testsuite/ld-ctf/ctf.exp: Add shared lib check. * testsuite/ld-plugin/lto.exp: Add lto shared check.
2021-12-28Automatic date update in version.inGDB Administrator1-1/+1
2021-12-27elf: Update comments for check_relocs in elf_backend_dataH.J. Lu1-7/+5
Since commit 5c3261b0e834647cf9eb555320e20871b7854f98 Author: H.J. Lu <hjl.tools@gmail.com> Date: Mon Oct 16 03:49:54 2017 -0700 ELF: Call check_relocs after opening all inputs check_relocs is called after opening all inputs. * elf-bfd.h (elf_backend_data::check_relocs): Update comments.
2021-12-27ld: Remove emultempl/linux.emH.J. Lu1-226/+0
Remove emultempl/linux.em whose last usage was removed by commit c65c21e1ffd1e02d9970a4bca0b7e384788a50f0 Author: Alan Modra <amodra@gmail.com> Date: Mon Apr 16 22:14:01 2018 +0930 various i386-aout and i386-coff target removal Also tidies some other aout leftovers in binutils-common.exp.
2021-12-27Automatic date update in version.inGDB Administrator1-1/+1
2021-12-26Automatic date update in version.inGDB Administrator1-1/+1
2021-12-25Automatic date update in version.inGDB Administrator1-1/+1
2021-12-24Remove gdb_print_host_addressTom Tromey6-175/+107
gdb_print_host_address is just a simple wrapper around fprintf_filtered. However, it is readily replaced in all callers by a combination of %s and call to host_address_to_string. This also simplifies the code, so I think it's worthwhile to remove this function. Regression tested on x86-64 Fedora 64.
2021-12-24Move gdb_bfd_errmsg to gdb_bfd.cTom Tromey4-35/+37
gdb_bfd.c contains most of gdb's BFD-related utility functions. However, gdb_bfd_errmsg is in utils.c. It seemed better to me to move this out of util.[ch] and into the BFD-related file instead. Tested by rebuilding.
2021-12-24RISC-V: Rewrite the csr testcases.Nelson Chu42-1521/+3567
Maskray (Fangrui Song) had suggested me before that we should combine multiple testcases into one file as possible as we can. So that we can more easily understand what these test cases are testing, and easier to maintain. Therefore, this patch rewrites all csr testcases, to make them more clean. gas/ * testsuite/gas/riscv/csr-fail-nonexistent.d: Renamed from priv-reg-fail-nonexistent testcase. * testsuite/gas/riscv/csr-fail-nonexistent.: Likewise. * testsuite/gas/riscv/csr-fail-nonexistent.s: Likewise. * testsuite/gas/riscv/csr-insns-pseudo-noalias.d: Renamed from priv-reg-pseudo testcase. * testsuite/gas/riscv/csr-insns-pseudo.d: Likewise. * testsuite/gas/riscv/csr-insns-pseudo.s: Likewise. * testsuite/gas/riscv/csr-insns-read-only.d: Renamed from priv-reg-fail-read-only-02 testcase. * testsuite/gas/riscv/csr-insns-read-only.l: Likewise. * testsuite/gas/riscv/csr-insns-read-only.s: Likewise. * testsuite/gas/riscv/h-ext-32.d: Moved hypervisor csrs to csr.s. * testsuite/gas/riscv/h-ext-32.s: Likewise. * testsuite/gas/riscv/h-ext-64.d: Likewise. * testsuite/gas/riscv/h-ext-64.s: Likewise. * testsuite/gas/riscv/csr.s: Renamed from priv-reg.s, and then added the hypervisor csrs. * testsuite/gas/riscv/csr-version-1p9p1.d: The csr testcase when the privileged spec is 1.9.1. Also tested all invalid csr warnings when -mcsr-check is enabled. * testsuite/gas/riscv/csr-version-1p9p1.l: Likewise. * testsuite/gas/riscv/csr-version-1p10.d: Likewise, but the privileged spec is 1.10.. * testsuite/gas/riscv/csr-version-1p10.l: Likewise. * testsuite/gas/riscv/csr-version-1p11.d: Likewise, but the privileged spec is 1.11. * testsuite/gas/riscv/csr-version-1p11.l: Likewise. * testsuite/gas/riscv/csr-version-1p12.d: Likewise, but the privileged spec is 1.12. * testsuite/gas/riscv/csr-version-1p12.l: Likewise. * testsuite/gas/riscv/priv-reg*: Removed or Renamed.
2021-12-24RISC-V: Hypervisor ext: support Privileged Spec 1.12Vineet Gupta9-5/+436
This is the Hypervisor Extension 1.0 - Hypervisor Memory-Management Instructions HFENCE.VVMA, HFENCE.GVMA, - Hypervisor Virtual Machine Load and Store Instructions HLV.B, HLV.BU, HSV.B, HLV.H, HLV.HU, HLVX.HU, HSB.H, HLV.W, HLV.WU, HLVX.WU, HSV.W, HLV.D, HSV.D - Hypervisor CSRs (some new, some address changed) hstatus, hedeleg, hideleg, hie, hcounteren, hgeie, htval, hip, hvip, htinst, hgeip, henvcfg, henvcfgh, hgatp, hcontext, htimedelta, htimedeltah, vsstatus, vsie, vstvec, vsscratch, vsepc, vscause, vstval, vsip, vsatp, Note that following were added already as part of svinval extension support: HINVAL.GVMA, HINVAL.VVMA Signed-off-by: Vineet Gupta <vineetg@rivosinc.com> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> Reviewed-by: Nelson Chu <nelson.chu@sifive.com> bfd/ * cpu-riscv.c (riscv_priv_specs): Added entry for 1.12. * cpu-riscv.h (enum riscv_spec_class): Added PRIV_SPEC_CLASS_1P12. gas/ * config/tc-riscv.c (abort_version): Updated comment. (validate_riscv_insn): Annotate switch-break. * testsuite/gas/riscv/h-ext-32.d: New testcase for hypervisor. * testsuite/gas/riscv/h-ext-32.s: Likewise. * testsuite/gas/riscv/h-ext-64.d: Likewise. * testsuite/gas/riscv/h-ext-64.s: Likewise. include/ * opcode/riscv-opc.h: Added encodings for hypervisor csrs and instrcutions. opcodes/ * riscv-opc.c (riscv_opcodes): Added hypervisor instrcutions.
2021-12-24RISC-V: Hypervisor ext: drop Privileged Spec 1.9.1 implementation/testsVineet Gupta10-110/+0
This makes way for a clean 1.12 based Hypervisor Ext support. There are no known implementors of 1.9.1 H-ext. (Per Jim, kendryte k210 is based on priv spec 1.9.1, but it seems unlikely that they implemented H-ext). Signed-off-by: Vineet Gupta <vineetg@rivosinc.com> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> Reviewed-by: Nelson Chu <nelson.chu@sifive.com> gas/ * testsuite/gas/riscv/csr-dw-regnums.d: Drop the hypervisor csrs defined in the privileged spec 1.9.1. * testsuite/gas/riscv/csr-dw-regnums.s: Likewise. * testsuite/gas/riscv/priv-reg-fail-read-only-01.s: Likewise. * testsuite/gas/riscv/priv-reg-fail-version-1p10.l: Likewise. * testsuite/gas/riscv/priv-reg-fail-version-1p11.l: Likewise. * testsuite/gas/riscv/priv-reg-version-1p10.d: Likewise. * testsuite/gas/riscv/priv-reg-version-1p11.d: Likewise. * testsuite/gas/riscv/priv-reg-version-1p9p1.d: Likewise. * testsuite/gas/riscv/priv-reg.s: Likewise. include/ * opcode/riscv-opc.h: Drop the hypervisor csrs defined in the privileged spec 1.9.1.
2021-12-24Automatic date update in version.inGDB Administrator1-1/+1
2021-12-23gdb/testsuite: resolve some duplicate testnames in gdb.miAndrew Burgess6-148/+170
Set of fixes to resolve some duplicate test names in the gdb.mi/ directory. There should be no real test changes after this set of fixes, they are all either: - Adding with_test_prefix type constructs to make test names unique, or - Changing the test name to be more descriptive, or better reflect the test being run.
2021-12-23gdb/remote: handle attach when stop packet lacks thread-idAndrew Burgess2-52/+77
Bug PR gdb/28405 reports a regression when using attach with an extended-remote target. In this case the target is not including a thread-id in the stop packet it sends back after the attach. The regression was introduced with this commit: commit 8f66807b98f7634c43149ea62e454ea8f877691d Date: Wed Jan 13 20:26:58 2021 -0500 gdb: better handling of 'S' packets The problem is that when GDB processes the stop packet, it sees that there is no thread-id and so has to "guess" which thread the stop should apply to. In this case the target only has one thread, so really, there's no guessing needed, but GDB still runs through the same process, this shouldn't cause us any problems. However, after the above commit, GDB now expects itself to be more internally consistent, specifically, only a thread that GDB thinks is resumed, can be a candidate for having stopped. It turns out that, when GDB attaches to a process through an extended-remote target, the threads of the process being attached too, are not, initially, marked as resumed. And so, when GDB tries to figure out which thread the stop might apply too, it finds no threads in the processes marked resumed, and so an assert triggers. In extended_remote_target::attach we create a new thread with a call to add_thread_silent, rather than remote_target::remote_add_thread, the reason is that calling the latter will result in a call to 'add_thread' rather than 'add_thread_silent'. However, remote_target::remote_add_thread includes additional actions (i.e. calling remote_thread_info::set_resumed and set_running) which are missing from extended_remote_target::attach. These missing calls are what would serve to mark the new thread as resumed. In this commit I propose that we add an extra parameter to remote_target::remote_add_thread. This new parameter will force the new thread to be added with a call to add_thread_silent. We can now call remote_add_thread from the ::attach method, the extra actions (listed above) will now be performed, and the thread will be left in the correct state. Additionally, in PR gdb/28405, a segfault is reported. This segfault triggers when 'set debug remote 1' is used before trying to reproduce the original assertion failure. The cause of this is in remote_target::select_thread_for_ambiguous_stop_reply, where we do this: remote_debug_printf ("first resumed thread is %s", pid_to_str (first_resumed_thread->ptid).c_str ()); remote_debug_printf ("is this guess ambiguous? = %d", ambiguous); gdb_assert (first_resumed_thread != nullptr); Notice that when debug printing is on we dereference first_resumed_thread before we assert that the pointer is not nullptr. This is the cause of the segfault, and is resolved by moving the assert before the debug printing code. I've extended an existing test, ext-attach.exp, so that the original test is run multiple times; we run in the original mode, as normal, but also, we now run with different packets disabled in gdbserver. In particular, disabling Tthread would trigger the assertion as it was reported in the original bug. I also run the test in all-stop and non-stop modes now for extra coverage, we also run the tests with target-async enabled, and disabled. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28405
2021-12-23gdb: on x86-64 non-trivial C++ objects are returned in memoryAndrew Burgess3-4/+57
Fixes PR gdb/28681. It was observed that after using the `finish` command an incorrect value was displayed in some cases. Specifically, this behaviour was observed on an x86-64 target. Consider this test program: struct A { int i; A () { this->i = 0; } A (const A& a) { this->i = a.i; } }; A func (int i) { A a; a.i = i; return a; } int main () { A a = func (3); return a.i; } And this GDB session: $ gdb -q ex.x Reading symbols from ex.x... (gdb) b func Breakpoint 1 at 0x401115: file ex.cc, line 14. (gdb) r Starting program: /home/andrew/tmp/ex.x Breakpoint 1, func (i=3) at ex.cc:14 14 A a; (gdb) finish Run till exit from #0 func (i=3) at ex.cc:14 main () at ex.cc:23 23 return a.i; Value returned is $1 = { i = -19044 } (gdb) p a $2 = { i = 3 } (gdb) Notice how after the `finish` the contents of $1 are junk, but, when I immediately ask for the value of `a`, I get back the correct value. The problem here is that after the finish command GDB calls the function amd64_return_value to figure out where the return value can be found (on x86-64 targets anyway). This function makes the wrong choice for the struct A in our case, as sizeof(A) <= 8, then amd64_return_value decides that A will be returned in a register. GDB then reads the return value register an interprets the contents as an instance of A. Unfortunately, A is not trivially copyable (due to its copy constructor), and the sys-v specification for argument and return value passing, says that any non-trivial C++ object should have space allocated for it by the caller, and the address of this space is passed to the callee as a hidden first argument. The callee should then return the address of this space as the return value. And so, the register that GDB is treating as containing an instance of A, actually contains the address of an instance of A (in this case on the stack), this is why GDB shows the incorrect result. The call stack within GDB for where we actually go wrong is this: amd64_return_value amd64_classify amd64_classify_aggregate And it is in amd64_classify_aggregate that we should be classifying the type as AMD64_MEMORY, instead of as AMD64_INTEGER as we currently do (via a call to amd64_classify_aggregate_field). At the top of amd64_classify_aggregate we already have this logic: if (TYPE_LENGTH (type) > 16 || amd64_has_unaligned_fields (type)) { theclass[0] = theclass[1] = AMD64_MEMORY; return; } Which handles some easy cases where we know a struct will be placed into memory, that is (a) the struct is more than 16-bytes in size, or (b) the struct has any unaligned fields. All we need then, is to add a check here to see if the struct is trivially copyable. If it is not then we know the struct will be passed in memory. I originally structured the code like this: if (TYPE_LENGTH (type) > 16 || amd64_has_unaligned_fields (type) || !language_pass_by_reference (type).trivially_copyable) { theclass[0] = theclass[1] = AMD64_MEMORY; return; } This solved the example from the bug, and my small example above. So then I started adding some more extensive tests to the GDB testsuite, and I ran into a problem. I hit this error: gdbtypes.h:676: internal-error: loc_bitpos: Assertion `m_loc_kind == FIELD_LOC_KIND_BITPOS' failed. This problem is triggered from: amd64_classify_aggregate amd64_has_unaligned_fields field::loc_bitpos Inside the unaligned field check we try to get the bit position of each field. Unfortunately, in some cases the field location is not FIELD_LOC_KIND_BITPOS, but is FIELD_LOC_KIND_DWARF_BLOCK. An example that shows this bug is: struct B { short j; }; struct A : virtual public B { short i; A () { this->i = 0; } A (const A& a) { this->i = a.i; } }; A func (int i) { A a; a.i = i; return a; } int main () { A a = func (3); return a.i; } It is the virtual base class, B, that causes the problem. The base class is represented, within GDB, as a field within A. However, the location type for this field is a DWARF_BLOCK. I spent a little time trying to figure out how to convert the DWARF_BLOCK to a BITPOS, however, I realised that, in this case at least, conversion is not needed. The C++ standard says that a class is not trivially copyable if it has any virtual base classes. And so, in this case, even if I could figure out the BITPOS for the virtual base class fields, I know for sure that I would immediately fail the trivially_copyable check. So, lets just reorder the checks in amd64_classify_aggregate to: if (TYPE_LENGTH (type) > 16 || !language_pass_by_reference (type).trivially_copyable || amd64_has_unaligned_fields (type)) { theclass[0] = theclass[1] = AMD64_MEMORY; return; } Now, if we have a class with virtual bases we will fail quicker, and avoid the unaligned fields check completely. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28681
generated by cgit v1.1 at 2025-01-16 13:44:51 +0000