John Ericson [Wed, 11 Aug 2021 12:17:54 +0000 (13:17 +0100)]
Deprecate a.out support for NetBSD targets.
As discussed previously, a.out support is now quite deprecated, and in
some cases removed, in both Binutils itself and NetBSD, so this legacy
default makes little sense. `netbsdelf*` and `netbsdaout*` still work
allowing the user to be explicit about there choice. Additionally, the
configure script warns about the change as Nick Clifton requested.
One possible concern was the status of NetBSD on NS32K, where only a.out
was supported. But per [1] NetBSD has removed support, and if it were to
come back, it would be with ELF. The binutils implementation is
therefore marked obsolete, per the instructions in the last message.
With that patch and this one applied, I have confirmed the following:
Andrew Burgess [Wed, 21 Jul 2021 17:23:19 +0000 (18:23 +0100)]
gdb: don't print backtrace when dumping core after an internal error
Currently, when GDB hits an internal error, and the user selects to
dump core, the recently added feature to write a backtrace to the
console will kick in, and print a backtrace as well as dumping the
core.
This was certainly not my intention when adding the backtrace on fatal
signal functionality, this feature was intended to produce a backtrace
when GDB crashes due to some fatal signal, internal errors should have
continued to behave as they did before, unchanged.
In this commit I set the signal disposition of SIGABRT back to SIG_DFL
just prior to the call to abort() that GDB uses to trigger the core
dump, this prevents GDB reaching the code that writes the backtrace to
the console.
I've also added a test that checks we don't see a backtrace on the
console after an internal error.
Andrew Burgess [Fri, 18 Jun 2021 13:26:30 +0000 (14:26 +0100)]
gdb: register SIGBUS, SIGFPE, and SIGABRT handlers
Register handlers for SIGBUS, SIGFPE, and SIGABRT. All of these
signals are setup as fatal signals that will cause GDB to terminate.
However, by passing these signals through the handle_fatal_signal
function, a user can arrange to see a backtrace when GDB
terminates (see maint set backtrace-on-fatal-signal).
In normal use of GDB there should be no user visible changes after
this commit. Only if GDB terminates with one of the above signals
will GDB change slightly, potentially printing a backtrace before
aborting.
I've added new tests for SIGFPE, SIGBUS, and SIGABRT.
Andrew Burgess [Thu, 10 Jun 2021 15:57:24 +0000 (16:57 +0100)]
gdb: print backtrace on fatal SIGSEGV
This commit adds a new maintenance feature, the ability to print
a (limited) backtrace if GDB dies due to a fatal signal.
The backtrace is produced using the backtrace and backtrace_symbols_fd
functions which are declared in the execinfo.h header, and both of
which are async signal safe. A configure check has been added to
check for these features, if they are not available then the new code
is not compiled into GDB and the backtrace will not be printed.
The motivation for this new feature is to aid in debugging GDB in
situations where GDB has crashed at a users site, but the user is
reluctant to share core files, possibly due to concerns about what
might be in the memory image within the core file. Such a user might
be happy to share a simple backtrace that was written to stderr.
The production of the backtrace is on by default, but can switched off
using the new commands:
maintenance set backtrace-on-fatal-signal on|off
maintenance show backtrace-on-fatal-signal
Right now, I have hooked this feature in to GDB's existing handling of
SIGSEGV only, but this will be extended to more signals in a later
commit.
One additional change I have made in this commit is that, when we
decide GDB should terminate due to the fatal signal, we now
raise the same fatal signal rather than raising SIGABRT.
Currently, this is only effecting our handling of SIGSEGV. So,
previously, if GDB hit a SEGV then we would terminate GDB with a
SIGABRT. After this commit we will terminate GDB with a SIGSEGV.
This feels like an improvement to me, we should still get a core dump,
but in many shells, the user will see a more specific message once GDB
exits, in bash for example "Segmentation fault" rather than "Aborted".
Finally then, here is an example of the output a user would see if GDB
should hit an internal SIGSEGV:
Fatal signal: Segmentation fault
----- Backtrace -----
./gdb/gdb[0x8078e6]
./gdb/gdb[0x807b20]
/lib64/libpthread.so.0(+0x14b20)[0x7f6648c92b20]
/lib64/libc.so.6(__poll+0x4f)[0x7f66484d3a5f]
./gdb/gdb[0x1540f4c]
./gdb/gdb[0x154034a]
./gdb/gdb[0x9b002d]
./gdb/gdb[0x9b014d]
./gdb/gdb[0x9b1aa6]
./gdb/gdb[0x9b1b0c]
./gdb/gdb[0x41756d]
/lib64/libc.so.6(__libc_start_main+0xf3)[0x7f66484041a3]
./gdb/gdb[0x41746e]
---------------------
A fatal error internal to GDB has been detected, further
debugging is not possible. GDB will now terminate.
This is a bug, please report it. For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.
Segmentation fault (core dumped)
It is disappointing that backtrace_symbols_fd does not actually map
the addresses back to symbols, this appears, in part, to be due to GDB
not being built with -rdynamic as the manual page for
backtrace_symbols_fd suggests, however, even when I do add -rdynamic
to the build of GDB I only see symbols for some addresses.
We could potentially look at alternative libraries to provide the
backtrace (e.g. libunwind) however, the solution presented here, which
is available as part of glibc is probably a good baseline from which
we might improve things in future.
Andrew Burgess [Fri, 18 Jun 2021 11:56:03 +0000 (12:56 +0100)]
gdb: rename async_init_signals to gdb_init_signals
The async_init_signals has, for some time, dealt with async and sync
signals, so removing the async prefix makes sense I think.
Additionally, as pointed out by Pedro:
.....
The comments relating to SIGTRAP and SIGQUIT within this function are
out of date.
The comments for SIGTRAP talk about the signal disposition (SIG_IGN)
being passed to the inferior, meaning the signal disposition being
inherited by GDB's fork children. However, we now call
restore_original_signals_state prior to forking, so the comment on
SIGTRAP is redundant.
The comments for SIGQUIT are similarly out of date, further, the
comment on SIGQUIT talks about problems with BSD4.3 and vfork,
however, we have not supported BSD4.3 for several years now.
Given the above, it seems that changing the disposition of SIGTRAP is
no longer needed, so I've deleted the signal() call for SIGTRAP.
Finally, the header comment on the function now called
gdb_init_signals was getting quite out of date, so I've updated it
to (hopefully) better reflect reality.
There should be no user visible change after this commit.
Andrew Burgess [Tue, 15 Jun 2021 11:59:34 +0000 (12:59 +0100)]
gdb: register signal handler after setting up event token
This commit fixes the smallest of small possible bug related to signal
handling. If we look in async_init_signals we see code like this:
signal (SIGQUIT, handle_sigquit);
sigquit_token =
create_async_signal_handler (async_do_nothing, NULL, "sigquit");
Then if we look in handle_sigquit we see code like this:
mark_async_signal_handler (sigquit_token);
signal (sig, handle_sigquit);
Finally, in mark_async_signal_handler we have:
async_handler_ptr->ready = 1;
Where async_handler_ptr will be sigquit_token.
What this means is that if a SIGQUIT arrive in async_init_signals
after handle_sigquit has been registered, but before sigquit_token has
been initialised, then GDB will most likely crash.
The chance of this happening is tiny, but fixing this is trivial, just
ensure we call create_async_signal_handler before calling signal, so
lets do that.
There are no tests for this. Trying to land a signal in the right
spot is pretty hit and miss. I did try changing the current HEAD GDB
like this:
Andrew Burgess [Thu, 10 Jun 2021 09:44:43 +0000 (10:44 +0100)]
gdb: terminate upon receipt of SIGFPE
GDB's SIGFPE handling is broken, this is PR gdb/16505 and
PR gdb/17891.
We currently try to use an async event token to process SIGFPE. So,
when a SIGFPE arrives the signal handler calls
mark_async_signal_handler then returns, effectively ignoring the
signal (for now).
The intention is that later the event loop will see that the async
token associated with SIGFPE has been marked and will call the async
handler, which just throws an error.
The problem is that SIGFPE is not safe to ignore. Ignoring a
SIGFPE (unless it is generated artificially, e.g. by raise()) is
undefined behaviour, after ignoring the signal on many targets we
return to the instruction that caused the SIGFPE to be raised, which
immediately causes another SIGFPE to be raised, we get stuck in an
infinite loop. The behaviour is certainly true on x86-64.
To view this behaviour I simply added some dummy code to GDB that
performed an integer divide by zero, compiled this on x86-64
GNU/Linux, ran GDB and saw GDB hang.
In this commit, I propose to remove all special handling of SIGFPE and
instead just let GDB make use of the default SIGFPE action, that is,
to terminate the process.
The only user visible change here should be:
- If a user sends a SIGFPE to GDB using something like kill,
previously GDB would just print an error and remain alive, now GDB
will terminate. This is inline with what happens if the user
sends GDB a SIGSEGV from kill though, so I don't see this as an
issue.
- If a bug in GDB causes a real SIGFPE, previously the users GDB
session would hang. Now the GDB session will terminate. Again,
this is inline with what happens if GDB receives a SIGSEGV due to
an internal bug.
In bug gdb/16505 there is mention that it would be nice if GDB did
more than just terminate when receiving a fatal signal. I haven't
done that in this commit, but later commits will move in that
direction.
Alan Modra [Wed, 11 Aug 2021 06:04:12 +0000 (15:34 +0930)]
ldgram.y tidies
I've been tripped up before thinking the "end" rule was the "END"
token. Let's use a better name. The formatting changes are for
consistency within rules, and making it a little easier to visually
separate tokens from mid-rule actions.
* ldgram.y (separator): Rename from "end". Update uses.
(statement): Formatting. Move ';' match to beginning.
(paren_script_name): Formatting. Simplify.
(must_be_exp, section): Formatting.
Matt Jacobson [Wed, 11 Aug 2021 09:03:19 +0000 (10:03 +0100)]
Add a -mno-dollar-line-separator command line option to the AVR assembler.
Some frontends, like the gcc Objective-C frontend, emit symbols with $
characters in them. The AVR target code in gas treats $ as a line separator,
so the code doesn?t assemble correctly.
Provide a machine-specific option to disable treating $ as a line separator.
* config/tc-avr.c (enum options): Add option flag.
(struct option): Add option -mno-dollar-line-separator.
(md_parse_option): Adjust treatment of $ when option is present.
* config/tc-avr.h: Use avr_line_separator_chars.
Jan Beulich [Wed, 11 Aug 2021 06:36:53 +0000 (08:36 +0200)]
gas: fold IEEE encoding of -Inf with that of +Inf
The respective results differ only by the sign bits - there's no need to
have basically identical (partially even arch-specific) logic twice.
Simply set the sign bit at the end of encoding the various formats.
Jan Beulich [Wed, 11 Aug 2021 06:36:28 +0000 (08:36 +0200)]
gas: support NaN flavors
Like for infinity, there isn't just a single NaN. The sign bit may be
of interest and, going beyond infinity, whether the value is quiet or
signalling may be even more relevant to be able to encode.
Note that an anomaly with x86'es double extended precision NaN values
gets taken care of at the same time: For all other formats a positive
value with all mantissa bits set was used, while here a negative value
with all non-significant mantissa bits clear was chose for an unknown
reason.
For m68k, since I don't know their X_PRECISION floating point value
layout, a warning gets issued if any of the new flavors was attempted
to be encoded that way. However likely it may be that, given that the
code lives in a source file supposedly implementing IEEE-compliant
formats, the bit patterns of the individual words match x86'es, I didn't
want to guess so. And my very, very old paper doc doesn't even mention
floating point formats other than single and double.
Jan Beulich [Wed, 11 Aug 2021 06:35:42 +0000 (08:35 +0200)]
Arm64: leave .bfloat16 processing to common code
With x86 support having been implemented by extending atof-ieee.c, avoid
unnecessary code duplication in md_atof(). This will then also allow to
take advantage of adjustments made there without needing to mirror them
here.
Jan Beulich [Wed, 11 Aug 2021 06:35:18 +0000 (08:35 +0200)]
Arm32: leave more .bfloat16 processing to common code
With x86 support having been implemented by extending atof-ieee.c, avoid
unnecessary code duplication in md_atof(). This will then also allow to
take advantage of adjustments made there without needing to mirror them
here.
Jan Beulich [Wed, 11 Aug 2021 06:34:18 +0000 (08:34 +0200)]
gas: make 2nd argument of .dcb.* consistently optional
Unlike the forms consuming/producing integer data, the floating point
ones so far required the 2nd argument to be present, contrary to
documentation. To avoid code duplication, split float_length() out of
hex_float() (taking the opportunity to adjust error message wording).
Jan Beulich [Wed, 11 Aug 2021 06:33:49 +0000 (08:33 +0200)]
x86: introduce .bfloat16 directive
This is to be able to generate data acted upon by AVX512-BF16 and
AMX-BF16 insns. While not part of the IEEE standard, the format is
sufficiently standardized to warrant handling in config/atof-ieee.c.
Arm, where custom handling was implemented, may want to leverage this as
well. To be able to also use the hex forms supported for other floating
point formats, a small addition to the generic hex_float() is needed.
Jan Beulich [Wed, 11 Aug 2021 06:32:54 +0000 (08:32 +0200)]
x86: introduce .hfloat directive
This is to be able to generate data passed to {,V}CVTPH2PS and acted
upon by AVX512-FP16 insns. To be able to also use the hex forms
supported for other floating point formats, a small addition to the
generic hex_float() is needed.
Jan Beulich [Wed, 11 Aug 2021 06:31:41 +0000 (08:31 +0200)]
x86/ELF: fix .tfloat output with hex input
The ELF psABI-s are quite clear here: On 32-bit the data type is 12
bytes long (with 2 bytes of trailing padding), while on 64-bit it is 16
bytes long (with 6 bytes of padding). Make hex_float() capable of
handling such padding.
Note that this brings the emitted data size of .dc.x / .dcb.x in line
also for non-ELF targets; so far they were different depending on input
format (dec vs hex).
Jan Beulich [Wed, 11 Aug 2021 06:31:03 +0000 (08:31 +0200)]
x86/ELF: fix .ds.x output
The ELF psABI-s are quite clear here: On 32-bit the underlying data type
is 12 bytes long (with 2 bytes of trailing padding), while on 64-bit it
is 16 bytes long (with 6 bytes of padding). Make s_space() capable of
handling 'x' (and 'p') type floating point being other than 12 bytes
wide (also adjusting documentation). This requires duplicating the
definition of X_PRECISION in the target speciifc header; the compiler
would complain if this was out of sync with config/atof-ieee.c.
Note that for now padding space doesn't get separated from actual
storage, which means that things will work correctly only for little-
endian cases, and which also means that by specifying large enough
numbers padding space can be set to non-zero. Since the logic is needed
for a single little-endian architecture only for now, I'm hoping that
this might be acceptable for the time being; otherwise the change will
become more intrusive.
Note also that this brings the emitted data size of .ds.x vs .tfloat in
line for non-ELF targets as well; the issue will be even more obvious
when further taking into account a subsequent patch fixing .dc.x/.dcb.x
(where output sizes currently differ depending on input format).
Jan Beulich [Wed, 11 Aug 2021 06:30:26 +0000 (08:30 +0200)]
x86/ELF: fix .tfloat output
The ELF psABI-s are quite clear here: On 32-bit the data type is 12
bytes long (with 2 bytes of trailing padding), while on 64-bit it is 16
bytes long (with 6 bytes of padding). Make ieee_md_atof() capable of
handling such padding, and specify the needed padding for x86 (leaving
non-ELF targets alone for now). Split the existing x86 testcase.
Alan Modra [Wed, 11 Aug 2021 05:36:20 +0000 (15:06 +0930)]
PR28163, Segment fault in function rl78_special_reloc
Relocation offset checks were completely missing in the rl78 backend,
allowing a relocation to write over memory anywhere. This was true
for rl78_special_reloc, a function primarily used when applying debug
relocations, and in rl78_elf_relocate_section used by the linker.
This patch fixes those problems by correcting inaccuracies in the
relocation howtos, then uses those howtos to sanity check relocation
offsets before applying relocations. In addition, the patch
implements overflow checking using the howto information rather than
the ad-hoc scheme implemented in relocate_section. I implemented the
overflow checking in rl78_special_reloc too.
* elf32-rl78.c (RL78REL, RL78_OP_REL): Add mask parameter.
(rl78_elf_howto_table): Set destination masks. Correct size and
bitsize of DIR32_REV. Correct complain_on_overflow for many relocs
as per tests in relocate_section. Add RH_SFR. Correct bitsize
for RH_SADDR. Set size to 3 and bitsize to 0 for all OP relocs.
(check_overflow): New function.
(rl78_special_reloc): Check that reloc address is within section.
Apply relocations using reloc howto. Check for overflow.
(RANGE): Delete.
(rl78_elf_relocate_section): Sanity check r_offset. Perform
overflow checking using reloc howto.
Tom Tromey [Sun, 8 Aug 2021 15:36:12 +0000 (09:36 -0600)]
Ignore .debug_types when reading .debug_aranges
I noticed that the fission-reread.exp test case can cause a complaint
when run with --target_board=cc-with-debug-names:
warning: Section .debug_aranges in [...]/fission-reread has duplicate debug_info_offset 0x0, ignoring .debug_aranges.
The bug here is that this executable has both .debug_info and
.debug_types, and both have a CU at offset 0x0. This triggers the
duplicate warning.
Because .debug_types doesn't provide any address ranges, these CUs can
be ignored. That is, this bug turns out to be another regression from
the info/types merger patch.
This patch fixes the problem by having this loop igore type units.
fission-reread.exp is updated to test for the bug.
Tom Tromey [Fri, 6 Aug 2021 19:52:23 +0000 (13:52 -0600)]
Generalize addrmap dumping
While debugging another patch series, I wanted to dump an addrmap. I
came up with this patch, which generalizes the addrmap-dumping code
from psymtab.c and moves it to addrmap.c. psymtab.c is changed to use
the new code.
Simon Marchi [Thu, 5 Aug 2021 16:21:35 +0000 (12:21 -0400)]
gdb: iterate only on vfork parent threads in handle_vfork_child_exec_or_exit
I spotted what I think is a buglet in proceed_after_vfork_done. After a
vfork child exits or execs, we resume all the threads of the parent. To
do so, we iterate on all threads using iterate_over_threads with the
proceed_after_vfork_done callback. Each thread is resumed if the
following condition is true:
where `pid` is the pid of the vfork parent. This is not multi-target
aware: since it only filters on pid, if there is an inferior with the
same pid in another target, we could end up resuming a thread of that
other inferior. The chances of the stars aligning for this to happen
are tiny, but still.
Fix that by iterating only on the vfork parent's threads, instead of on
all threads. This is more efficient, as we iterate on just the required
threads (inferiors have their own thread list), and we can drop the pid
check. The resulting code is also more straightforward in my opinion,
so it's a win-win.
George Barrett [Sun, 6 Jun 2021 18:49:58 +0000 (04:49 +1000)]
guile: fix smob exports
Before Guile v2.1 [1], calls to `scm_make_smob_type' implicitly added
the created class to the exports list of (oop goops); v2.1+ does not
implicitly create bindings in any modules. This means that the GDB
manual subsection documenting exported types is not quite right when GDB
is linked against Guile <v2.1 (types are exported from (oop goops))
instead of (gdb)) and incorrect when linked against Guile v2.1+ (types
are not bound to any variables at all!).
There is a range of cases in which it's necessary or convenient to be
able to refer to a GDB smob type, for instance:
- Pattern matching based on the type of a value.
- Defining GOOPS methods handling values from GDB (GOOPS methods
typically use dynamic dispatch based on the types of the arguments).
- Type-checking assertions when applying some defensive programming on
an interface.
- Generally any other situation one might encounter in a dynamically
typed language that might need some introspection.
If you're more familiar with Python, it would be quite similar to being
unable to refer to the classes exported from the GDB module (which is to
say: not crippling for the most part, but makes certain tasks more
difficult than necessary).
This commit makes a small change to GDB's smob registration machinery
to make sure registered smobs get exported from the current
module. This will likely cause warnings to the user about conflicting
exports if they load both (gdb) and (oop goops) from a GDB linked
against Guile v2.0, but it shouldn't impact functionality (and seemed
preferable to trying to un-export bindings from (oop goops) if v2.0
was detected).
Nick Clifton [Mon, 9 Aug 2021 16:23:22 +0000 (17:23 +0100)]
GAS: DWARF-5: Ensure that the 0'th entry in the directory table contains the current working directory.
* dwarf2dbg.c (get_directory_table_entry): Ensure that dir[0]
contains current working directory.
(out_dir_and_file_list): Likewise.
* testsuite/gas/elf/dwarf-5-dir0.s: New test source file.
* testsuite/gas/elf/dwarf-5-dir0.d: New test driver.
* testsuite/gas/elf/elf.exp: Run the new test.
* testsuite/gas/elf/dwarf-5-file0.d: Adjust expected output.
* testsuite/gas/i386/dwarf5-line-1.d: Likewise.
* testsuite/gas/i386/dwarf5-line-2.d: Likewise.
Tom Tromey [Sun, 8 Aug 2021 14:53:17 +0000 (08:53 -0600)]
Include objfiles.h in a few .c files
I found a few .c files that rely on objfiles.h, but that only include
it indirectly, via dwarf2/read.h -> psympriv.h. If that include is
removed (something my new DWARF indexer series does), then the build
will break.
It seemed harmless and correct to add these includes now, making the
eventual series a little smaller.
Alan Modra [Sat, 7 Aug 2021 04:43:11 +0000 (14:13 +0930)]
PR28176, rl78 complex reloc divide by zero
This is a bit more than just preventing the divide by zero. Most of
the patch is tidying up error reporting, so that for example, linking
an object file with a reloc stack underflow produces a linker error
rather than just displaying a message that might be ignored.
PR 28176
* elf32-rl78.c (RL78_STACK_PUSH, RL78_STACK_POP): Delete.
(rl78_stack_push, rl78_stack_pop): New inline functions.
(rl78_compute_complex_reloc): Add status and error message params.
Use new inline stack handling functions. Report stack overflow
or underflow, and divide by zero.
(rl78_special_reloc): Return status and error message from
rl78_compute_complex_reloc.
(rl78_elf_relocate_section): Similarly. Modernise reloc error
reporting. Delete unused bfd_reloc_other case. Don't assume
DIR24S_PCREL overflow is due to undefined function.
(rl78_offset_for_reloc): Adjust to suit rl78_compute_complex_reloc.
Tom de Vries [Fri, 6 Aug 2021 14:44:17 +0000 (16:44 +0200)]
[gdb/symtab] Fix zero address complaint for shlib
In PR28004 the following warning / Internal error is reported:
...
$ gdb -q -batch \
-iex "set sysroot $(pwd -P)/repro" \
./repro/gdb \
./repro/core \
-ex bt
...
Program terminated with signal SIGABRT, Aborted.
#0 0x00007ff8fe8e5d22 in raise () from repro/usr/lib/libc.so.6
[Current thread is 1 (LWP 1762498)]
#1 0x00007ff8fe8cf862 in abort () from repro/usr/lib/libc.so.6
warning: (Internal error: pc 0x7ff8feb2c21d in read in psymtab, \
but not in symtab.)
warning: (Internal error: pc 0x7ff8feb2c218 in read in psymtab, \
but not in symtab.)
...
#2 0x00007ff8feb2c21e in __gnu_debug::_Error_formatter::_M_error() const \
[clone .cold] (warning: (Internal error: pc 0x7ff8feb2c21d in read in \
psymtab, but not in symtab.)
) from repro/usr/lib/libstdc++.so.6
...
The warning is about the following:
- in find_pc_sect_compunit_symtab we try to find the address
(0x7ff8feb2c218 / 0x7ff8feb2c21d) in the symtabs.
- that fails, so we try again in the partial symtabs.
- we find a matching partial symtab
- however, the partial symtab has a full symtab, so
we should have found a matching symtab in the first step.
The addresses are:
...
(gdb) info sym 0x7ff8feb2c218
__gnu_debug::_Error_formatter::_M_error() const [clone .cold] in \
section .text of repro/usr/lib/libstdc++.so.6
(gdb) info sym 0x7ff8feb2c21d
__gnu_debug::_Error_formatter::_M_error() const [clone .cold] + 5 in \
section .text of repro/usr/lib/libstdc++.so.6
...
which correspond to unrelocated addresses 0x9c218 and 0x9c21d:
...
$ nm -C repro/usr/lib/libstdc++.so.6.0.29 | grep 000000000009c218 000000000009c218 t __gnu_debug::_Error_formatter::_M_error() const \
[clone .cold]
...
which belong to function __gnu_debug::_Error_formatter::_M_error() in
/build/gcc/src/gcc/libstdc++-v3/src/c++11/debug.cc.
The partial symtab that is found for the addresses is instead the one for
/build/gcc/src/gcc/libstdc++-v3/src/c++98/bitmap_allocator.cc, which is
incorrect.
When reading the first range 0x0..0x56, it doesn't trigger the "start address
of zero" complaint here:
...
/* A not-uncommon case of bad debug info.
Don't pollute the addrmap with bad data. */
if (range_beginning + baseaddr == 0
&& !per_objfile->per_bfd->has_section_at_zero)
{
complaint (_(".debug_rnglists entry has start address of zero"
" [in module %s]"), objfile_name (objfile));
continue;
}
...
because baseaddr != 0, which seems incorrect given that when loading the
shared library individually in gdb (and consequently baseaddr == 0), we do see
the complaint.
Consequently, we run into this case in dwarf2_get_pc_bounds:
...
if (low == 0 && !per_objfile->per_bfd->has_section_at_zero)
return PC_BOUNDS_INVALID;
...
which then results in this code in process_psymtab_comp_unit_reader being
called with cu_bounds_kind == PC_BOUNDS_INVALID, which sets the set_addrmap
argument to 1:
...
scan_partial_symbols (first_die, &lowpc, &highpc,
cu_bounds_kind <= PC_BOUNDS_INVALID, cu);
...
and consequently, the CU addrmap gets build using address info from the
functions.
During that process, addrmap_set_empty is called with a range that includes
0x9c218 and 0x9c21d:
...
(gdb) p /x start
$7 = 0x9989c
(gdb) p /x end_inclusive
$8 = 0xb200d
...
but it's called for a function at DIE 0x54153 with DW_AT_ranges at 0x40ae:
... 000040ae00000000000b1ee000000000000b200e 000040b9000000000009989c00000000000998c4 000040c3 <End of list>
...
and neither range includes 0x9c218 and 0x9c21d.
This is caused by this code in partial_die_info::read:
...
if (dwarf2_ranges_read (ranges_offset, &lowpc, &highpc, cu,
nullptr, tag))
has_pc_info = 1;
...
which pretends that the function is located at addresses 0x9989c..0xb200d,
which is indeed not the case.
This patch fixes the first problem encountered: fix the "start address of
zero" complaint warning by removing the baseaddr part from the condition.
Same for dwarf2_ranges_process.
The effect is that:
- the complaint is triggered, and
- the warning / Internal error is no longer triggered.
This does not fix the observed problem in partial_die_info::read, which is
filed as PR28200.
Tested on x86_64-linux.
Co-Authored-By: Simon Marchi <simon.marchi@polymtl.ca>
gdb/ChangeLog:
2021-07-29 Simon Marchi <simon.marchi@polymtl.ca>
Tom de Vries <tdevries@suse.de>
PR symtab/28004
* gdb/dwarf2/read.c (dwarf2_rnglists_process, dwarf2_ranges_process):
Fix zero address complaint.
* gdb/testsuite/gdb.dwarf2/dw2-zero-range-shlib.c: New test.
* gdb/testsuite/gdb.dwarf2/dw2-zero-range.c: New test.
* gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp: New file.
Alan Modra [Fri, 6 Aug 2021 09:38:30 +0000 (19:08 +0930)]
bfd_reloc_offset_in_range overflow
This patch is more about the style of bounds checking we ought to use,
rather than a real problem. An overflow of "octet + reloc_size" can
only happen with huge sections which would certainly cause out of
memory errors.
* reloc.c (bfd_reloc_offset_in_range): Avoid possible overflow.
Alan Modra [Fri, 6 Aug 2021 09:05:23 +0000 (18:35 +0930)]
PR28173, nds32_elf_howto_table index out of bounds
Indexing the howto table was seriously broken by a missing entry, and
use of assertions about user input rather than testing the input.
PR 28173
* elf32-nds32.c (nds32_elf_howto_table): Add missing empty howto.
(bfd_elf32_bfd_reloc_type_table_lookup): Replace assertions with
range checks. Return NULL if unsupported reloc type. Remove
dead code. Take an unsigned int param.
(nds32_info_to_howto_rel): Test for NULL howto or howto name
return from lookup. Remove assertion.
(nds32_info_to_howto): Remove unnecessary ATTRIBUTE_UNUSED.
Test for NULL howto or howto name return from lookup.
Alan Modra [Fri, 6 Aug 2021 07:56:14 +0000 (17:26 +0930)]
PR28172, bfin_pcrel24_reloc heap-buffer-overflow
bfin pcrel24 relocs are weird, they apply to the reloc address minus
two. That means reloc addresses of 0 and 1 are invalid. Check that,
and fix other reloc range checking.
Will Schmidt [Thu, 5 Aug 2021 18:04:35 +0000 (13:04 -0500)]
[PATCH] GDB Testsuite, update compile-cplus.exp
[PATCH] GDB Testsuite, update compile-cplus.exp
Update the gdb.compile/compile-cplus.exp test to
handle errors generated when passing bad arguments
into the gdb-compile command.
This matches changes made to gdb.compile/compile.exp
in the past as part of
"Migrate rest of compile commands to new options framework" e6ed716cd5514c08b9d7c469d185b1aa177dbc22
Will Schmidt [Thu, 5 Aug 2021 17:46:32 +0000 (12:46 -0500)]
[gdb] Handle .TOC. sections during gdb-compile for rs6000 target.
[gdb] Handle .TOC. sections during gdb-compile for rs6000 target.
When we encounter a .TOC. symbol in the object we are loading,
we need to associate this with the .toc section in order to
properly resolve other symbols in the object. IF a .toc section
is not found, iterate the sections until we find one with the
SEC_ALLOC flag. If that also fails, fall back to using
the *ABS* section, pointed to by bfd_abs_section_ptr.
Simon Marchi [Thu, 10 Jun 2021 14:47:40 +0000 (10:47 -0400)]
gdb/testsuite: gdb.base/attach.exp: expose bug when testing with native-extended-gdbserver
In gdb.base/attach.exp, proc do_attach_failure_tests, we attach to a
process. When then try to attach to the same process in another
inferior, expecting it to fail. We then come back to the first inferior
and try to kill it, to clean up the test. When using the
native-extended-gdbserver board, this "kill" test passes, even though it
didn't actually work:
add-inferior
[New inferior 2]
Added inferior 2 on connection 1 (extended-remote localhost:2347)
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: add empty inferior 2
inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: switch to inferior 2
attach 817032
Attaching to process 817032
Attaching to process 817032 failed
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: fail to attach again
inferior 1
[Switching to inferior 1 [process 817032] (/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/attach/attach)]
[Switching to thread 1.1 (Thread 817032.817032)]
#0 main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/attach.c:19
19 while (! should_exit)
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: switch to inferior 1
kill
Kill the program being debugged? (y or n) y
Remote connection closed <==== That's unexpected
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: exit after attach failures
When the second attach fails, gdbserver seems to break the connection
(it hangs up on the existing remote target) and start listening again
for incoming connections. This is documented in PR 19558 [1].
Make the expected output regexp for the kill command tighter (it
currently accepts anything). Use "set confirm off" so we don't have to
deal with the confirmation. And to be really sure the extended-remote
target still works, try to run the inferior again after killing. The
now tests are kfail'ed when the target is gdbserver.
Simon Marchi [Thu, 10 Jun 2021 14:47:39 +0000 (10:47 -0400)]
gdb/testsuite: gdb.base/attach.exp: fix support check in test_command_line_attach_run
When running this test with the native-extended-gdbserver, we get:
main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/attach.c:19
19 while (! should_exit)
The program being debugged has been started already.
Start it from the beginning? (y or n) PASS: gdb.base/attach.exp: cmdline attach run: run to prompt
y
Don't know how to run. Try "help target".
(gdb) FAIL: gdb.base/attach.exp: cmdline attach run: run to main
This test tests using both "-p <pid>" and "-ex start" on the command line,
making sure that we first attach and then run.
Normally, after that "y", we should see the program running again.
However, a particuliarity of the native-extended-gdbserver is that it
uses "set auto-connect-native-target off" on the command line. The full
GDB command line is:
./gdb -nw -nx -data-directory /home/simark/build/binutils-gdb/gdb/testsuite/../data-directory \
-iex set height 0 -iex set width 0 -ex set auto-connect-native-target off \
-ex set sysroot -quiet -iex set height 0 -iex set width 0 --pid=536609 -ex start
The attach succeeds. I guess it is done before "set
auto-connect-native-target off", or it somehow bypasses it. When the
"start" is executed, the native target is unpushed, while killing the
existing process, but not re-pushed, due to "set
auto-connect-native-target off". So we get that "Don't know how to run"
message.
Really, I think it's a case of the test doing things incompatible with
the board, I think it should just be skipped. And as we can see with
the current code, there were some attempts at doing this, just using the
wrong checks:
- isnative: this is a dejagnu proc which checks if the target board has
the same triplet as the build machine. In the case of
native-extended-gdbserver, it does.
- is_remote target: this checks whether the target board is remote, as
in executing on a different machin. native-extended-gdbserver is not
remote.
Since the --pid option specifically attaches to a process using the
native target, change the test to use gdb_is_target_native instead.
gdb/testsuite/ChangeLog:
* gdb.base/attach.exp (test_command_line_attach_run): Use
gdb_is_target_native to check if test is supported.
Simon Marchi [Thu, 5 Aug 2021 15:56:43 +0000 (11:56 -0400)]
gdb: target_waitstatus_to_string: print extra info for FORKED, VFORKED, EXECD
Print the extra information contained in target_waitstatus for these
events. For TARGET_WAITKIND_{FORKED,VFORKED}, the extra information is
contained in related_pid, and is the ptid of the new process. For
TARGET_WAITKIND_EXECD, it,s the exec'd path name in execd_pathname.
Print it using the same format used for TARGET_WAITKIND_STOPPED and
others.
Simon Marchi [Thu, 5 Aug 2021 03:18:56 +0000 (23:18 -0400)]
gdb: use ptid_t::to_string in print_target_wait_results
The ptid_t::to_string method was introduced recently, to format a ptid_t
for debug purposes. It formats the ptid exactly as is done in
print_target_wait_results, so make print_target_wait_results use it.
There are cases where the result of the expression evaluation is
expected to be in a form of a value and not location description.
One place that has this requirement is dwarf_entry_parameter_to_value
function, but more are expected in the future. Until now, this
requirement was fulfilled by extending the evaluated expression with
a DW_OP_stack_value operation at the end.
New implementation, introduces a new evaluation argument instead.
Where constructor should only ever require a target architecture
information. This information is held in per object file
(dwarf2_per_objfile) structure, so it makes sense to keep that
structure as a constructor argument. It also makes sense to get the
address size from that structure, but unfortunately that interface
doesn't exist at the moment, so the dwarf_expr_context class user
needs to provide that information.
The push_address method is used to push a CORE_ADDR as a value on
top of the DWARF stack before the evaluation. This method can be
later changed to push any struct value object on the stack.
The evaluate method is the method that evaluates a DWARF expression
and provides the evaluation result, in a form of a single struct
value object that describes a location. To do this, the method requires
a context of the evaluation, as well as expected result type
information. If the type information is not provided, the DWARF generic
type will be used instead.
To avoid storing the gdbarch information in the evaluator object, that
information is now always acquired from the per_objfile object.
All data members are now private and only visible to the evaluator
class, so a m_ prefix was added to all of their names to reflect that.
To make this distinction clear, they are also accessed through objects
this pointer, wherever that was not the case before.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::dwarf_expr_context): Add
address size argument.
(dwarf_expr_context::read_mem): Change to use property_addr_info
structure.
(dwarf_expr_context::evaluate): New function.
(dwarf_expr_context::execute_stack_op): Change to use
property_addr_info structure.
* dwarf2/expr.h (struct dwarf_expr_context): New evaluate
declaration. Change eval and fetch_result method to private.
(dwarf_expr_context::gdbarch): Remove member.
(dwarf_expr_context::stack): Make private and add m_ prefix.
(dwarf_expr_context::addr_size): Make private and add
m_ prefix.
(dwarf_expr_context::recursion_depth): Make private and add
m_ prefix.
(dwarf_expr_context::max_recursion_depth): Make private and
add m_ prefix.
(dwarf_expr_context::len): Make private and add m_ prefix.
(dwarf_expr_context::data): Make private and add m_ prefix.
(dwarf_expr_context::initialized): Make private and add
m_ prefix.
(dwarf_expr_context::pieces): Make private and add m_ prefix.
(dwarf_expr_context::per_objfile): Make private and add
m_ prefix.
(dwarf_expr_context::frame): Make private and add m_ prefix.
(dwarf_expr_context::per_cu): Make private and add m_ prefix.
(dwarf_expr_context::addr_info): Make private and add
m_ prefix.
* dwarf2/frame.c (execute_stack_op): Change to call evaluate
method.
* dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Change to call
evaluate method.
(dwarf2_locexpr_baton_eval): Change to call evaluate method.
The patch is addressing the issue of class users writing and reading
the internal data of the dwarf_expr_context class.
At this point, all conditions are met for the DWARF evaluator to return
an evaluation result in a form of a single struct value object.
gdb/ChangeLog:
* dwarf2/expr.c (pieced_value_funcs): Chenge to static
function.
(allocate_piece_closure): Change to static function.
(dwarf_expr_context::fetch_result): New function.
* dwarf2/expr.h (struct piece_closure): Remove declaration.
(struct dwarf_expr_context): fetch_result new declaration.
fetch, fetch_address and fetch_in_stack_memory members move
to private.
(allocate_piece_closure): Remove.
* dwarf2/frame.c (execute_stack_op): Change to use
fetch_result.
* dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Change to use
fetch_result.
(dwarf2_locexpr_baton_eval): Change to use fetch_result.
* dwarf2/loc.h (invalid_synthetic_pointer): Expose function.
Zoran Zaric [Fri, 26 Feb 2021 13:59:28 +0000 (13:59 +0000)]
Make value_copy also copy the stack data member
Fixing a bug where the value_copy function did not copy the stack data
and initialized members of the struct value. This is needed for the
next patch where the DWARF expression evaluator is changed to return a
single struct value object.
* value.c (value_copy): Change to also copy the stack data
and initialized members.
Following 5 patches series is trying to clean up the interface of the
DWARF expression evaluator class (dwarf_expr_context).
After merging all expression evaluators into one class, the next
logical step is to make a clean user interface for that class. To do
that, we first need to address the issue of class users writing and
reading the internal data of the class directly.
Fixing the case of writing is simple, it makes sense for an evaluator
instance to be per architecture basis. Currently, the best separation
seems to be per object file, so having that data (dwarf2_per_objfile)
as a constructor argument makes sense. It also makes sense to get the
address size from that object file, but unfortunately that interface
does not exist at the moment.
Luckily, address size information is already available to the users
through other means. As a result, the address size also needs to be a
class constructor argument, at least until a better interface for
acquiring that information from an object file is implemented.
The rest of the user written data comes down to a context of an
evaluated expression (compilation unit context, frame context and
passed in buffer context) and a source type information that a result
of evaluating expression is representing. So, it makes sense for all of
these to be arguments of an evaluation method.
To address the problem of reading the dwarf_expr_context class
internal data, we first need to understand why it is implemented that
way?
This is actualy a question of which existing class can be used to
represent both values and a location descriptions and why it is not
used currently?
The answer is in a struct value class/structure, but the problem is
that before the evaluators were merged, only one evaluator had an
infrastructure to resolve composite and implicit pointer location
descriptions.
After the merge, we are now able to use the struct value to represent
any result of the expression evaluation. It also makes sense to move
all infrastructure for those location descriptions to the expr.c file
considering that that is the only place using that infrastructure.
What we are left with in the end is a clean public interface of the
dwarf_expr_context class containing:
The idea with this particular patch is to move piece_closure structure
and the interface that handles it (lval_funcs) to expr.c file.
While implicit pointer location descriptions are still not useful in
the CFI context (of the AMD's DWARF standard extensions), the composite
location descriptions are certainly necessary to describe a results of
specific compiler optimizations.
Considering that a piece_closure structure is used to represent both,
there was no benefit in splitting them.
gdb/ChangeLog:
* dwarf2/expr.c (struct piece_closure): Add from loc.c.
(allocate_piece_closure): Add from loc.c.
(bits_to_bytes): Add from loc.c.
(rw_pieced_value): Add from loc.c.
(read_pieced_value): Add from loc.c.
(write_pieced_value): Add from loc.c.
(check_pieced_synthetic_pointer): Add from loc.c.
(indirect_pieced_value): Add from loc.c.
(coerce_pieced_ref): Add from loc.c.
(copy_pieced_value_closure): Add from loc.c.
(free_pieced_value_closure): Add from loc.c.
(sect_variable_value): Add from loc.c.
* dwarf2/loc.c (sect_variable_value): Move to expr.c.
(struct piece_closure): Move to expr.c.
(allocate_piece_closure): Move to expr.c.
(bits_to_bytes): Move to expr.c.
(rw_pieced_value): Move to expr.c.
(read_pieced_value): Move to expr.c.
(write_pieced_value): Move to expr.c.
(check_pieced_synthetic_pointer): Move to expr.c.
(indirect_pieced_value): Move to expr.c.
(coerce_pieced_ref): Move to expr.c.
(copy_pieced_value_closure): Move to expr.c.
(free_pieced_value_closure): Move to expr.c.
The evaluate_for_locexpr_baton is the last derived class from the
dwarf_expr_context class. It's purpose is to support the passed in
buffer functionality.
Although, it is not really necessary to merge this class with it's
base class, doing that simplifies new expression evaluator design.
Considering that this functionality is going around the DWARF standard,
it is also reasonable to expect that with a new evaluator design and
extending the push object address functionality to accept any location
description, there will be no need to support passed in buffers.
Alternatively, it would also makes sense to abstract the interaction
between the evaluator and a given resource in the near future. The
passed in buffer would then be a specialization of that abstraction.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::read_mem): Merge with
evaluate_for_locexpr_baton implementation.
* dwarf2/loc.c (class evaluate_for_locexpr_baton): Remove
class.
(evaluate_for_locexpr_baton::read_mem): Move to
dwarf_expr_context.
(dwarf2_locexpr_baton_eval): Instantiate dwarf_expr_context
instead of evaluate_for_locexpr_baton class.
There are no virtual methods that require different specialization in
dwarf_expr_context class. This means that derived classes
dwarf_expr_executor and dwarf_evaluate_loc_desc are not needed any
more.
As a result of this, the evaluate_for_locexpr_baton class base class
is now the dwarf_expr_context class.
There might be a need for a better class hierarchy when we know more
about the direction of the future DWARF versions and gdb extensions,
but that is out of the scope of this patch series.
Following the idea of merging the evaluators, the
push_dwarf_reg_entry_value method can be moved from
dwarf_expr_executor and dwarf_evaluate_loc_desc classes
to their base class dwarf_expr_context.
Following the idea of merging the evaluators, the read_mem method can
be moved from dwarf_expr_executor and dwarf_evaluate_loc_desc classes
to their base class dwarf_expr_context.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::read_mem): Move from
dwarf_evaluate_loc_desc.
* dwarf2/frame.c (dwarf_expr_executor::read_mem): Remove
method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::read_mem): Move to
dwarf_expr_context.
Zoran Zaric [Fri, 9 Oct 2020 12:25:10 +0000 (13:25 +0100)]
Move get_object_address to dwarf_expr_context
Following the idea of merging the evaluators, the get_object_address
and can be moved from dwarf_expr_executor and dwarf_evaluate_loc_desc
classes to their base class dwarf_expr_context.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::get_object_address): Move
from dwarf_evaluate_loc_desc.
(class dwarf_expr_context): Add object address member to
dwarf_expr_context.
* dwarf2/expr.h (dwarf_expr_context::get_frame_pc): Remove
method.
* dwarf2/frame.c (dwarf_expr_executor::get_object_address):
Remove method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::get_object_address):
move to dwarf_expr_context.
(class dwarf_evaluate_loc_desc): Move object address member to
dwarf_expr_context.
Following the idea of merging the evaluators, the dwarf_call and
get_frame_pc method can be moved from dwarf_expr_executor and
dwarf_evaluate_loc_desc classes to their base class dwarf_expr_context.
Once this is done, the get_frame_pc can be replace with lambda
function.
This patch moves the compilation unit context information and support
from dwarf_expr_executor and dwarf_evaluate_loc_desc to
dwarf_expr_context evaluator. The idea is to report an error when a
given operation requires a compilation unit information to be resolved,
which is not available.
With this change, it also makes sense to always acquire ref_addr_size
information from the compilation unit context, considering that all
DWARF operations that refer to that information require a compilation
unit context to be present during their evaluation.
gdb/ChangeLog:
* dwarf2/expr.c (ensure_have_per_cu): New function.
(dwarf_expr_context::dwarf_expr_context): Add compilation unit
context information.
(dwarf_expr_context::get_base_type): Move from
dwarf_evaluate_loc_desc.
(dwarf_expr_context::get_addr_index): Remove method.
(dwarf_expr_context::dwarf_variable_value): Remove method.
(dwarf_expr_context::execute_stack_op): Call compilation unit
context info check. Inline get_addr_index and
dwarf_variable_value methods.
* dwarf2/expr.h (struct dwarf_expr_context): Add compilation
context info.
(dwarf_expr_context::get_addr_index): Remove method.
(dwarf_expr_context::dwarf_variable_value): Remove method.
(dwarf_expr_context::ref_addr_size): Remove member.
* dwarf2/frame.c (dwarf_expr_executor::get_addr_index): Remove
method.
(dwarf_expr_executor::dwarf_variable_value): Remove method.
* dwarf2/loc.c (sect_variable_value): Expose function.
(dwarf_evaluate_loc_desc::get_addr_index): Remove method.
(dwarf_evaluate_loc_desc::dwarf_variable_value): Remove method.
(class dwarf_evaluate_loc_desc): Move compilation unit context
information to dwarf_expr_context class.
* dwarf2/loc.h (sect_variable_value): Expose function.
Zoran Zaric [Fri, 9 Oct 2020 10:22:23 +0000 (11:22 +0100)]
Remove get_frame_cfa from dwarf_expr_context
Following the idea of merging the evaluators, the get_frame_cfa method
can be moved from dwarf_expr_executor and dwarf_evaluate_loc_desc
classes to their base class dwarf_expr_context. Once this is done,
it becomes apparent that the method is only called once and it can be
inlined.
It is also necessary to check if the frame context information was
provided before the DW_OP_call_frame_cfa operation is executed.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::get_frame_cfa): Remove
method.
(dwarf_expr_context::execute_stack_op): Call frame context info
check for DW_OP_call_frame_cfa. Remove use of get_frame_cfa.
* dwarf2/expr.h (dwarf_expr_context::get_frame_cfa): Remove
method.
* dwarf2/frame.c (dwarf_expr_context::get_frame_cfa): Remove
method.
* dwarf2/loc.c (dwarf_expr_context::get_frame_cfa): Remove
method.
Following 15 patches in this patch series is cleaning up the design of
the DWARF expression evaluator (dwarf_expr_context) to make future
extensions of that evaluator easier and cleaner to implement.
There are three subclasses of the dwarf_expr_context class
(dwarf_expr_executor, dwarf_evaluate_loc_desc and
evaluate_for_locexpr_baton). Here is a short description of each class:
- dwarf_expr_executor is evaluating a DWARF expression in a context
of a Call Frame Information. The overridden methods of this subclass
report an error if a specific DWARF operation, represented by that
method, is not allowed in a CFI context. The source code of this
subclass lacks the support for composite as well as implicit pointer
location description.
- dwarf_evaluate_loc_desc can evaluate any expression with no
restrictions. All of the methods that this subclass overrides are
actually doing what they are intended to do. This subclass contains
a full support for all location description types.
- evaluate_for_locexpr_baton subclass is a specialization of the
dwarf_evaluate_loc_desc subclass and it's function is to add
support for passed in buffers. This seems to be a way to go around
the fact that DWARF standard lacks a bit offset support for memory
location descriptions as well as using any location description for
the push object address functionality.
It all comes down to this question: what is a function of a DWARF
expression evaluator?
Is it to evaluate the expression in a given context or to check the
correctness of that expression in that context?
Currently, the only reason why there is a dwarf_expr_executor subclass
is to report an invalid DWARF expression in a context of a CFI, but is
that what the evaluator is supposed to do considering that the evaluator
is not tied to a given DWARF version?
There are more and more vendor and GNU extensions that are not part of
the DWARF standard, so is it that impossible to expect that some of the
extensions could actually lift the previously imposed restrictions of
the CFI context? Not to mention that every new DWARF version is lifting
some restrictions anyway.
The thing that makes more sense for an evaluator to do, is to take the
context of an evaluation and checks the requirements of every operation
evaluated against that context. With this approach, the evaluator would
report an error only if parts of the context, necessary for the
evaluation, are missing.
If this approach is taken, then the unification of the
dwarf_evaluate_loc_desc, dwarf_expr_executor and dwarf_expr_context
is the next logical step. This makes a design of the DWARF expression
evaluator cleaner and allows more flexibility when supporting future
vendor and GNU extensions.
Additional benefit here is that now all evaluators have access to all
location description types, which means that a vendor extended CFI
rules could support composite location description as well. This also
means that a new evaluator interface can be changed to return a single
struct value (that describes the result of the evaluation) instead of
a caller poking around the dwarf_expr_context internal data for answers
(like it is done currently).
This patch starts the merging process by moving the frame context
information and support from dwarf_expr_executor and
dwarf_evaluate_loc_desc to dwarf_expr_context evaluator. The idea
is to report an error when a given operation requires a frame
information to be resolved, if that information is not present.
gdb/ChangeLog:
* dwarf2/expr.c (ensure_have_frame): New function.
(read_addr_from_reg): Add from frame.c.
(dwarf_expr_context::dwarf_expr_context): Add frame info to
dwarf_expr_context.
(dwarf_expr_context::read_addr_from_reg): Remove.
(dwarf_expr_context::get_reg_value): Move from
dwarf_evaluate_loc_desc.
(dwarf_expr_context::get_frame_base): Move from
dwarf_evaluate_loc_desc.
(dwarf_expr_context::execute_stack_op): Call frame context info
check. Remove use of read_addr_from_reg method.
* dwarf2/expr.h (struct dwarf_expr_context): Add frame info
member, read_addr_from_reg, get_reg_value and get_frame_base
declaration.
(read_addr_from_reg): Move to expr.c.
* dwarf2/frame.c (read_addr_from_reg): Move to
dwarf_expr_context.
(dwarf_expr_executor::read_addr_from_reg): Remove.
(dwarf_expr_executor::get_frame_base): Remove.
(dwarf_expr_executor::get_reg_value): Remove.
(execute_stack_op): Use read_addr_from_reg function instead of
read_addr_from_reg method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::get_frame_base): Move
to dwarf_expr_context.
(dwarf_evaluate_loc_desc::get_reg_value): Move to
dwarf_expr_context.
(dwarf_evaluate_loc_desc::read_addr_from_reg): Remove.
(dwarf2_locexpr_baton_eval):Use read_addr_from_reg function
instead of read_addr_from_reg method.
Zoran Zaric [Fri, 26 Feb 2021 10:14:53 +0000 (10:14 +0000)]
Cleanup of the dwarf_expr_context constructor
Move the initial values for dwarf_expr_context class data members
to the class declaration in expr.h.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::dwarf_expr_context):
Remove initial data members values.
* dwarf2/expr.h (dwarf_expr_context): Add initial values
to the class data members.
Zoran Zaric [Fri, 14 Aug 2020 10:28:13 +0000 (11:28 +0100)]
Replace the symbol needs evaluator with a parser
This patch addresses a design problem with the symbol_needs_eval_context
class. It exposes the problem by introducing two new testsuite test
cases.
To explain the issue, I first need to explain the dwarf_expr_context
class that the symbol_needs_eval_context class derives from.
The intention behind the dwarf_expr_context class is to commonize the
DWARF expression evaluation mechanism for different evaluation
contexts. Currently in gdb, the evaluation context can contain some or
all of the following information: architecture, object file, frame and
compilation unit.
Depending on the information needed to evaluate a given expression,
there are currently three distinct DWARF expression evaluators:
- Frame: designed to evaluate an expression in the context of a call
frame information (dwarf_expr_executor class). This evaluator doesn't
need a compilation unit information.
- Location description: designed to evaluate an expression in the
context of a source level information (dwarf_evaluate_loc_desc
class). This evaluator expects all information needed for the
evaluation of the given expression to be present.
- Symbol needs: designed to answer a question about the parts of the
context information required to evaluate a DWARF expression behind a
given symbol (symbol_needs_eval_context class). This evaluator
doesn't need a frame information.
The functional difference between the symbol needs evaluator and the
others is that this evaluator is not meant to interact with the actual
target. Instead, it is supposed to check which parts of the context
information are needed for the given DWARF expression to be evaluated by
the location description evaluator.
The idea is to take advantage of the existing dwarf_expr_context
evaluation mechanism and to fake all required interactions with the
actual target, by returning back dummy values. The evaluation result is
returned as one of three possible values, based on operations found in a
given expression:
- SYMBOL_NEEDS_NONE,
- SYMBOL_NEEDS_REGISTERS and
- SYMBOL_NEEDS_FRAME.
The problem here is that faking results of target interactions can yield
an incorrect evaluation result.
For example, if we have a conditional DWARF expression, where the
condition depends on a value read from an actual target, and the true
branch of the condition requires a frame information to be evaluated,
while the false branch doesn't, fake target reads could conclude that a
frame information is not needed, where in fact it is. This wrong
information would then cause the expression to be actually evaluated (by
the location description evaluator) with a missing frame information.
This would then crash the debugger.
The gdb.dwarf2/symbol_needs_eval_fail.exp test introduces this
scenario, with the following DWARF expression:
DW_OP_addr $some_variable
DW_OP_deref
# conditional jump to DW_OP_bregx
DW_OP_bra 4
DW_OP_lit0
# jump to DW_OP_stack_value
DW_OP_skip 3
DW_OP_bregx $dwarf_regnum 0
DW_OP_stack_value
This expression describes a case where some variable dictates the
location of another variable. Depending on a value of some_variable, the
variable whose location is described by this expression is either read
from a register or it is defined as a constant value 0. In both cases,
the value will be returned as an implicit location description on the
DWARF stack.
Currently, when the symbol needs evaluator fakes a memory read from the
address behind the some_variable variable, the constant value 0 is used
as the value of the variable A, and the check returns the
SYMBOL_NEEDS_NONE result.
This is clearly a wrong result and it causes the debugger to crash.
The scenario might sound strange to some people, but it comes from a
SIMD/SIMT architecture where $some_variable is an execution mask. In
any case, it is a valid DWARF expression, and GDB shouldn't crash while
evaluating it. Also, a similar example could be made based on a
condition of the frame base value, where if that value is concluded to
be 0, the variable location could be defaulted to a TLS based memory
address.
The gdb.dwarf2/symbol_needs_eval_timeout.exp test introduces a second
scenario. This scenario is a bit more abstract due to the DWARF
assembler lacking the CFI support, but it exposes a different
manifestation of the same problem. Like in the previous scenario, the
DWARF expression used in the test is valid:
# conditional jump to DW_OP_drop
DW_OP_bra -9
DW_OP_stack_value
Similarly to the previous scenario, the location of a variable A is an
implicit location description with a constant value that depends on a
value held by a global variable. The difference from the previous case
is that DWARF expression contains a loop instead of just one branch. The
end condition of that loop depends on the expectation that a frame base
value is never zero. Currently, the act of faking the target reads will
cause the symbol needs evaluator to get stuck in an infinite loop.
Somebody could argue that we could change the fake reads to return
something else, but that would only hide the real problem.
The general impression seems to be that the desired design is to have
one class that deals with parsing of the DWARF expression, while there
are virtual methods that deal with specifics of some operations.
Using an evaluator mechanism here doesn't seem to be correct, because
the act of evaluation relies on accessing the data from the actual
target with the possibility of skipping the evaluation of some parts of
the expression.
To better explain the proposed solution for the issue, I first need to
explain a couple more details behind the current design:
There are multiple places in gdb that handle DWARF expression parsing
for different purposes. Some are in charge of converting the expression
to some other internal representation (decode_location_expression,
disassemble_dwarf_expression and dwarf2_compile_expr_to_ax), some are
analysing the expression for specific information
(compute_stack_depth_worker) and some are in charge of evaluating the
expression in a given context (dwarf_expr_context::execute_stack_op
and decode_locdesc).
The problem is that all those functions have a similar (large) switch
statement that handles each DWARF expression operation. The result of
this is a code duplication and harder maintenance.
As a step into the right direction to solve this problem (at least for
the purpose of a DWARF expression evaluation) the expression parsing was
commonized inside of an evaluator base class (dwarf_expr_context). This
makes sense for all derived classes, except for the symbol needs
evaluator (symbol_needs_eval_context) class.
As described previously the problem with this evaluator is that if the
evaluator is not allowed to access the actual target, it is not really
evaluating.
Instead, the desired function of a symbol needs evaluator seems to fall
more into expression analysis category. This means that a more natural
fit for this evaluator is to be a symbol needs analysis, similar to the
existing compute_stack_depth_worker analysis.
Another problem is that using a heavyweight mechanism of an evaluator
to do an expression analysis seems to be an unneeded overhead. It also
requires a more complicated design of the parent class to support fake
target reads.
The reality is that the whole symbol_needs_eval_context class can be
replaced with a lightweight recursive analysis function, that will give
more correct result without compromising the design of the
dwarf_expr_context class. The analysis treats the expression byte
stream as a DWARF operation graph, where each graph node can be
visited only once and each operation can decide if the frame context
is needed for their evaluation.
The downside of this approach is adding of one more similar switch
statement, but at least this way the new symbol needs analysis will be
a lightweight mechnism and it will provide a correct result for any
given DWARF expression.
A more desired long term design would be to have one class that deals
with parsing of the DWARF expression, while there would be a virtual
methods that deal with specifics of some DWARF operations. Then that
class would be used as a base for all DWARF expression parsing mentioned
at the beginning.
This however, requires a far bigger changes that are out of the scope
of this patch series.
The new analysis requires the DWARF location description for the
argc argument of the main function to change in the assembly file
gdb.python/amd64-py-framefilter-invalidarg.S. Originally, expression
ended with a 0 value byte, which was never reached by the symbol needs
evaluator, because it was detecting a stack underflow when evaluating
the operation before. The new approach does not simulate a DWARF
stack anymore, so the 0 value byte needs to be removed because it
makes the DWARF expression invalid.
gdb/ChangeLog:
* dwarf2/loc.c (class symbol_needs_eval_context): Remove.
(dwarf2_get_symbol_read_needs): New function.
(dwarf2_loc_desc_get_symbol_read_needs): Remove.
(locexpr_get_symbol_read_needs): Use
dwarf2_get_symbol_read_needs.
gdb/testsuite/ChangeLog:
* gdb.python/amd64-py-framefilter-invalidarg.S : Update argc
DWARF location expression.
* lib/dwarf.exp (_location): Handle DW_OP_fbreg.
* gdb.dwarf2/symbol_needs_eval.c: New file.
* gdb.dwarf2/symbol_needs_eval_fail.exp: New file.
* gdb.dwarf2/symbol_needs_eval_timeout.exp: New file.
Cui,Lili [Mon, 14 Jun 2021 03:15:51 +0000 (11:15 +0800)]
[PATCH 2/2] Add tests for Intel AVX512_FP16 instructions
Intel AVX512 FP16 instructions use maps 3, 5 and 6. Maps 5 and 6 use 3 bits
in the EVEX.mmm field (0b101, 0b110). Map 5 is for instructions that were FP32
in map 1 (0Fxx). Map 6 is for instructions that were FP32 in map 2 (0F38xx).
There are some exceptions to this rule. Some things in map 1 (0Fxx) with imm8
operands predated our current conventions; those instructions moved to map 3.
FP32 things in map 3 (0F3Axx) found new opcodes in map3 for FP16 because map3
is very sparsely populated. Most of the FP16 instructions share opcodes and
prefix (EVEX.pp) bits with the related FP32 operations.
Intel AVX512 FP16 instructions has new displacements scaling rules, please refer
to the public software developer manual for detail information.
gas/
2021-08-05 Igor Tsimbalist <igor.v.tsimbalist@intel.com>
H.J. Lu <hongjiu.lu@intel.com>
Wei Xiao <wei3.xiao@intel.com>
Lili Cui <lili.cui@intel.com>
Cui,Lili [Mon, 14 Jun 2021 03:05:05 +0000 (11:05 +0800)]
[PATCH 1/2] Enable Intel AVX512_FP16 instructions
Intel AVX512 FP16 instructions use maps 3, 5 and 6. Maps 5 and 6 use 3 bits
in the EVEX.mmm field (0b101, 0b110). Map 5 is for instructions that were FP32
in map 1 (0Fxx). Map 6 is for instructions that were FP32 in map 2 (0F38xx).
There are some exceptions to this rule. Some things in map 1 (0Fxx) with imm8
operands predated our current conventions; those instructions moved to map 3.
FP32 things in map 3 (0F3Axx) found new opcodes in map3 for FP16 because map3
is very sparsely populated. Most of the FP16 instructions share opcodes and
prefix (EVEX.pp) bits with the related FP32 operations.
Intel AVX512 FP16 instructions has new displacements scaling rules, please refer
to the public software developer manual for detail information.
gas/
2021-08-05 Igor Tsimbalist <igor.v.tsimbalist@intel.com>
H.J. Lu <hongjiu.lu@intel.com>
Wei Xiao <wei3.xiao@intel.com>
Lili Cui <lili.cui@intel.com>
Andrew Burgess [Tue, 8 Jun 2021 11:49:04 +0000 (12:49 +0100)]
gdb/testsuite: update test gdb.base/step-over-syscall.exp
I was looking at PR gdb/19675 and the related test
gdb.base/step-over-syscall.exp. This test includes a call to kfail
when we are testing a displaced step over a clone syscall.
While looking at the test I removed the call to kfail and ran the
test, and was surprised that the test passed.
I ran the test a few times and it does sometimes fail, but mostly it
passed fine.
PR gdb/19675 describes how, when we displaced step over a clone, the
new thread is created with a $pc in the displaced step buffer. GDB
then fails to "fix" this $pc (for the new thread), and the thread will
be set running with its current $pc value. This means that the new
thread will just start executing from whatever happens to be after the
displaced stepping buffer.
In the original PR gdb/19675 bug report Yao Qi was seeing the new
thread cause a segfault, the problem is, what actually happens is
totally undefined.
On my machine, I'm seeing the new thread reenter main, it then starts
trying to run the test again (in the new thread). This just happens
to be safe enough (in this simple test) that most of the time the
inferior doesn't crash.
In this commit I try to make the test slightly more likely to fail by
doing a couple of things.
First, I added a static variable to main, this is set true when the
first thread enters main, if a second thread ever enters main then I
force an abort.
Second, when the test is finishing I want to ensure that the new
threads have had a chance to do "something bad" if they are going to.
So I added a global counter, as each thread starts successfully it
decrements the counter. The main thread does not proceed to the final
marker function (where GDB has placed a breakpoint) until all threads
have started successfully. This means that if the newly created
thread doesn't successfully enter clone_fn then the counter will never
reach zero and the test will timeout.
With these two changes my hope is that the test should fail more
reliably, and so, I have also changed the test to call setup_kfail
before the specific steps that we expect to misbehave instead of just
calling kfail and skipping parts of the test completely. The benefit
of this is that if/when we fix GDB this test will start to KPASS and
we'll know to update this test to remove the setup_kfail call.
Lancelot SIX [Sun, 1 Aug 2021 15:47:29 +0000 (15:47 +0000)]
gdb: Use unwinder name in frame_info::to_string
While working on a stack unwinding issue using 'set debug frame on', I
noticed the frame_info::to_string method could be slightly improved.
Unwinders have been given a name in a154d838a70e96d888620c072e2d6ea8bdf044ca. Before this patch, frame_info
debug output prints the host address of the used unwinder, which is not
easy to interpret. This patch proposes to use the unwinder name
instead since we now have it.
Simon Marchi [Tue, 3 Aug 2021 15:31:14 +0000 (11:31 -0400)]
gdb/testsuite: fix gdb.base/info-macros.exp with clang
The test gdb.base/info-macros.exp says that it doesn't pass the "debug"
option to prepare_for_testing because that would cause -g to appear
after -g3 on the command line, and that would cause some gcc versions to
not include macro info. I don't know what gcc versions this refers to.
I tested with gcc 4.8, and that works fine with -g after -g3.
The current state is problematic when testing with CC_FOR_TARGET=clang,
because then only -fdebug-macro is included. No -g switch if included,
meaning we get a binary without any debug info, and the test fails.
One way to fix it would be to add "debug" to the options when the
compiler is clang.
However, the solution I chose was to specify "debug" in any case, even
for gcc. Other macro tests such as gdb.base/macscp.exp do perfectly
fine with it. Also, this lets the test use the debug flag specified by
the board file. For example, we can test with GCC and DWARF 5, with:
$ make check RUNTESTFLAGS="--target_board unix/gdb:debug_flags=-gdwarf-5" TESTS="gdb.base/info-macros.exp"
With the hard-coded -g3, this wouldn't actually test with DWARF 5.
Simon Marchi [Tue, 3 Aug 2021 15:31:13 +0000 (11:31 -0400)]
gdb: avoid dereferencing empty str_offsets_base optional in dwarf_decode_macros
Since 4d7188abfdf2 ("gdbsupport: add debug assertions in
gdb::optional::get"), some macro-related tests fail on Ubuntu 20.04 with
the system gcc 9.3.0 compiler when building with _GLIBCXX_DEBUG. For
example, gdb.base/info-macros.exp results in:
(gdb) break -qualified main
/home/smarchi/src/binutils-gdb/gdb/../gdbsupport/gdb_optional.h:206: internal-error: T& gdb::optional<T>::get() [with T = long unsigned int]: Assertion `this->has_value ()' failed.
The binary contains DWARF 4 debug info and includes a pre-standard
(pre-DWARF 5) .debug_macro section. The CU doesn't have a
DW_AT_str_offsets_base attribute (which doesn't exist in DWARF 4). The
field dwarf2_cu::str_offsets_base is therefore empty. At
dwarf2/read.c:24138, we unconditionally read the value in the optional,
which triggers the assertion shown above.
The same thing happens when building the test program with DWARF 5 with
the same gcc compiler, as that version of gcc doesn't use indirect
string forms, even with DWARF 5. So it still doesn't add a
DW_AT_str_offsets_base attribute on the CU.
Fix that by propagating down a gdb::optional<ULONGEST> for the str
offsets base instead of ULONGEST. That value is only used in
dwarf_decode_macro_bytes, when encountering an "strx" macro operation
(DW_MACRO_define_strx or DW_MACRO_undef_strx). Add a check there that
we indeed have a value in the optional before reading it. This is
unlikely to happen, but could happen in theory with an erroneous file
that uses DW_MACRO_define_strx but does not provide a
DW_AT_str_offsets_base (in practice, some things would probably have
failed before and stopped processing of debug info). I tested the
complaint by inverting the condition and using a clang-compiled binary,
which uses the strx operators. This is the result:
During symbol reading: use of DW_MACRO_define_strx with unknown string offsets base [in module /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/info-macros/info-macros]
The test now passes cleanly with the setup mentioned above, and the
testsuite looks on par with how it was before 4d7188abfdf2.
Simon Marchi [Tue, 3 Aug 2021 15:31:12 +0000 (11:31 -0400)]
gdb: fix typo in complaint in dwarf2/macro.c
I saw this complaint when my code had some bug, and spotted the typo.
Fix it, and while at it mention DW_MACRO as well (it would be confusing
to only see DW_MACINFO with a file that uses a DWARF 5 .debug_macro
section). I contemplated the idea of passing the knowledge of whether
we are dealing with a .debug_macro section or .debug_macinfo section, to
print only the right one. But in the end, I don't think that trouble is
necessary for a complaint nobody is going to see.
Simon Marchi [Tue, 3 Aug 2021 22:13:41 +0000 (18:13 -0400)]
gdb: fix warnings in bsd-kvm.c
Building on OpenBSD, I get warnings like:
CXX bsd-kvm.o
/home/simark/src/binutils-gdb/gdb/bsd-kvm.c:241:18: error: ISO C++11 does not allow conversion from string literal to 'char *' [-Werror,-Wwritable-strings]
nl[0].n_name = "_dumppcb";
^
Andreas Krebbel [Wed, 4 Aug 2021 14:51:36 +0000 (16:51 +0200)]
IBM Z: Remove lpswey parameter
opcodes/
* s390-opc.c (INSTR_SIY_RD): New instruction format.
(MASK_SIY_RD): New instruction mask.
* s390-opc.txt: Change instruction format of lpswey to SIY_RD.
gas/
* testsuite/gas/s390/zarch-arch14.d: Remove last operand of
lpswey.
* testsuite/gas/s390/zarch-arch14.s: Likewise.
Alan Modra [Wed, 4 Aug 2021 09:02:28 +0000 (18:32 +0930)]
PR28162, segment fault in mips_elf_assign_gp
For the testcase in the PR, _bfd_mips_elf32_gprel16_reloc is passed a
NULL output_bfd. As expected for reloc special functions if called by
objdump or when final linking. The function attempts to find the
output by
output_bfd = symbol->section->output_section->owner;
That makes some sense, since when handling a gp-relative reloc we need
the relevant gp to which the symbol is relative. Possibly the gp
value can be one for a shared library? But that doesn't seem useful
or supported by the various abi docs and won't work as written.
Symbols defined in shared libraries have section->output_section
NULL, and what's more the code in mips_elf_assign_gp isn't set up to
look at shared library symbols.
Also, if the symbol is a SHN_ABS one the owner of *ABS* section is
NULL, which will result in the testcase segfault. The only gp to
which an absolute symbol can be relative is the linker output bfd when
linking, or the input bfd when not. This patch arranges to do that
for all gp-relative reloc symbols.
* elf32-mips.c (_bfd_mips_elf32_gprel16_reloc): Don't use the
section symbol to find the output bfd, use input_section.
(mips_elf_gprel32_reloc, mips16_gprel_reloc): Likewise.
* elf64-mips.c (mips_elf64_gprel16_reloc): Likewise.
(mips_elf64_literal_reloc, mips_elf64_gprel32_reloc): Likewise.
(mips16_gprel_reloc): Likewise.
Tom de Vries [Wed, 4 Aug 2021 10:53:47 +0000 (12:53 +0200)]
[gdb/symtab] Implement addrmap_mutable_find
Currently addrmap_mutable_find is not implemented:
...
static void *
addrmap_mutable_find (struct addrmap *self, CORE_ADDR addr)
{
/* Not needed yet. */
internal_error (__FILE__, __LINE__,
_("addrmap_find is not implemented yet "
"for mutable addrmaps"));
}
...
I implemented this because I needed it during debugging, to be able to do:
...
(gdb) p ((dwarf2_psymtab *)addrmap_find (map, addr))->filename
...
before and after a call to addrmap_set_empty.
Since this is not used otherwise, added addrmap unit test.
Build on x86_64-linux, tested by doing:
...
$ gdb -q -batch -ex "maint selftest addrmap"
Running selftest addrmap.
Ran 1 unit tests, 0 failed
...
gdb/ChangeLog:
2021-08-03 Tom de Vries <tdevries@suse.de>
* gdb/addrmap.c (addrmap_mutable_find): Implement
[GDB_SELF_TESTS] (CHECK_ADDRMAP_FIND): New macro.
[GDB_SELF_TESTS] (core_addr, addrmap_foreach_check, test_addrmap)
(_initialize_addrmap): New function.
gas: always add dummy symbols when creating XCOFF sections.
Most of the algorithms for XCOFF in tc-ppc.c assume that
the csects field of a ppc_xcoff_section isn't NULL.
This was already made for most of the sections with the creation
of a dummy symbol.
This patch simply mades it default when creating a xcoff_section.
Simon Marchi [Fri, 28 May 2021 21:28:35 +0000 (17:28 -0400)]
gdb: follow-fork: push target and add thread in target_follow_fork
In the context of ROCm-gdb [1], the ROCm target sits on top of the
linux-nat target. when a process forks, it needs to carry over some
data from the forking inferior to the fork child inferior. Ideally, the
ROCm target would implement the follow_fork target_ops method, but there
are some small problems. This patch fixes these, which helps the ROCm
target, but also makes things more consistent and a bit nicer in
general, I believe.
The main problem is: when follow-fork-mode is "parent",
target_follow_fork is called with the parent as the current inferior.
When it's "child", target_follow_fork is called with the child as the
current inferior. This means that target_follow_fork is sometimes
called on the parent's target stack and sometimes on the child's target
stack.
The parent's target stack may contain targets above the process target,
such as the ROCm target. So if follow-fork-child is "parent", the ROCm
target would get notified of the fork and do whatever is needed. But
the child's target stack, at that moment, only contains the exec and
process target copied over from the parent. The child's target stack is
set up by follow_fork_inferior, before calling target_follow_fork. In
that case, the ROCm target wouldn't get notified of the fork.
For consistency, I think it would be good to always call
target_follow_fork on the parent inferior's target stack. I think it
makes sense as a way to indicate "this inferior has called fork, do
whatever is needed". The desired outcome of the fork (whether an
inferior is created for the child, do we need to detach from the child)
can be indicated by passed parameter.
I therefore propose these changes:
- make follow_fork_inferior always call target_follow_fork with the
parent as the current inferior. That lets all targets present on the
parent's target stack do some fork-related handling and push
themselves on the fork child's target stack if needed.
For this purpose, pass the child inferior down to target_follow_fork
and follow_fork implementations. This is nullptr if no inferior is
created for the child, because we want to detach from it.
- as a result, in follow_fork_inferior, detach from the parent inferior
(if needed) only after the target_follow_fork call. This is needed
because we want to call target_follow_fork before the parent's
target stack is torn down.
- hand over to the targets in the parent's target stack (including the
process target) the responsibility to push themselves, if needed, to
the child's target stack. Also hand over the responsibility to the
process target, at the same time, to create the child's initial
thread (just like we do for follow_exec).
- pass the child inferior to exec_on_vfork, so we don't need to swap
the current inferior between parent and child. Nothing in
exec_on_vfork depends on the current inferior, after this change.
Although this could perhaps be replaced with just having the exec
target implement follow_fork and push itself in the child's target
stack, like the process target does... We would just need to make
sure the process target calls beneath()->follow_fork(...). I'm not
sure about this one.
Carl Love [Tue, 20 Jul 2021 22:40:47 +0000 (17:40 -0500)]
Fixes for mi-fortran-modules.exp fixes
Output has additional information for a given filename.
gdb/testsuite/ChangeLog
* gdb.mi/mi-fortran-modules.exp (system_modules_pattern,
system_module_symbols_pattern): Add check for additional symbols
on the line
Simon Marchi [Thu, 29 Jul 2021 18:42:04 +0000 (14:42 -0400)]
gdbsupport: add debug assertions in gdb::optional::get
The libstdc++ version of optional contains some runtime checks enabled
when _GLIBCXX_DEBUG is defined. I think it would be useful if our
version contained similar checks.
Add checks in the two `get` methods, also conditional on _GLIBCXX_DEBUG.
I think it's simpler to use that macro rather than introducing a new
GDB-specific one, as I think that if somebody is interested in enabling
these runtime checks, they'll also be interested in enabling the
libstdc++ runtime checks (and vice-versa).
I implemented these checks using gdb_assert. Note that gdb_assert
throws (after querying the user), and we are in noexcept methods. That
means that std::terminate / abort will immediately be called. I think
this is ok, since if those were "real" _GLIBCXX_DEBUG checks, abort
would be called straight away.
If I add a dummy failure, it looks like so:
$ ./gdb -q -nx --data-directory=data-directory
/home/simark/src/binutils-gdb/gdb/../gdbsupport/gdb_optional.h:206: internal-error: T& gdb::optional<T>::get() [with T = int]: Assertion `this->has_value ()' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) n
[1] 658767 abort (core dumped) ./gdb -q -nx --data-directory=data-directory
[gdb/testsuite] templates.exp to accept clang++ output
Please consider below testcase with intended error.
``````````
constexpr const char cstring[] = "Eta";
template <const char*, typename T> class Column {};
using quick = Column<cstring,double>; // cstring without '&'
void lookup() {
quick c1;
c1.ls();
}
``````````
It produces below error.
``````````
no member named 'ls' in 'Column<&cstring, double>'.
``````````
Please note that error message contains '&' for cstring, which is absent
in actual program.
Clang++ does not generate & in such cases and this should also be
accepted as correct output.
gdb/testsuite/ChangeLog:
* gdb.cp/templates.exp: Accept different but correct output
from the Clang++ compiled binary also.
projects / thirdparty / binutils-gdb.git / log
