In remote_target::thread_info_to_thread_handle we return a copy:
...
gdb::byte_vector
remote_target::thread_info_to_thread_handle (struct thread_info *tp)
{
remote_thread_info *priv = get_remote_thread_info (tp);
return priv->thread_handle;
}
...
Fix this by returning a gdb::array_view instead:
...
gdb::array_view<const gdb_byte>
remote_target::thread_info_to_thread_handle (struct thread_info *tp)
...
Tested on x86_64-linux.
This fixes the build when building with -std=c++20.
Approved-By: Pedro Alves <pedro@palves.net>
opcodes/
* kvx-dis.c (print_insn_kvx): Change the loop condition so that
wordcount is always less than KVXMAXBUNDLEWORDS.
(decode_prologue_epilogue_bundle): Likewise.
This commits tackles 2 problems in the test
gdb.reverse/insn-reverse.exp. They are, broadly: flawed logic when an
unexpected error occurs, and badly formed asm expressions.
For the first, what happens is that if the inferior stops progressing
for some reason, the test will emit an UNSUPPORTED and continue testing
by reversing from the current location and checking all registers for
every instruction. However, due to how the outputs are indexed in the
test, this early exit will cause most of the subsequent tests to be
de-synced and will emit many unrelated failures.
This commit changes the UNSUPPORTED for a FAIL, since the test has in
fact failed to record the execution of the whole function, and
decrements the recorded instruction count by one so that the indexes are
in sync once more.
At the time of committing, this reduces the amount of failures when
testing with clang-15 from around 150 to 2, and correctly identifies
where the issue lies.
The second problem is in how the asm statements in the *-x86.c file
are written. As an example, let's examine the following line:
__asm__ volatile ("rdrand %%ebp;" : "=r" (number));
This statement says that number is being used as the output variable,
but is not indicating which registers were clobbered so that the
compiler is able to properly output. GCC decides to just not save
anything, whereas clang assumes that the output is in %rax, and writes
it to the variable. This hid the problem that any compiler is not good
at dealing with asm statements that change the rbp register. It can be
seen more explicitly by informing gcc that rbp has been clobbered like
so:
__asm__ volatile ("rdrand %%ebp;" : "=r" (number) : : "%ebp");
This statement gets compiled into the following assembly:
rdrandl %ebp
movl %eax, -4(%rbp)
Which is clearly using the incorrect rbp to find the memory location of
the variable. Since the test only exercises GDB's ability to record the
register changes, this commit removes the output to memory.
Finally, correctly informing the compiler of clobbered registers
makes gcc throw an error that the rsp is no longer usable at the end of
the function. To avoid that, this commit compresses the 3 asm statements
that would save, change and reset registers into a single asm statement.
Approved-By: Tom Tromey <tom@tromey.com>
When testing using reverse-stepi to fully step through a function, the
code checks for an infinite loop by seeing if we land on the line that
contains the return statement multiple times. This assumption only works
if there is only one instruction associated with that line, which is how
GCC handles line information, but other compilers may handle it differently.
Clang-15, for instance, associates 6. Because of this, the inferior used
to get seriously out of sync with the test expectations, and result in 13
spurious failures. The same issue occurs with gdb.reverse/step-precsave.exp.
This commit changes the test so that we check for PC instead of line
number. The test still only happens when the same line is detected, to
simplify the resulting log. With this change, no new failures are
emitted when using clang.
Approved-By: Tom Tromey <tom@tromey.com>
The tests gdb.reverse/solib-precsave.exp and solib-reverse.exp have the
assumption that line tables will have an entry for the closing } in a
function. Not all compiles do this, one example being clang. To fix
this, this commit changes the function in shr2.c to have multiple lines,
and the test to accept either line as a correct step location.
To properly re-sync the inferiors, the function repeat_cmd_until had to
be slightly changed to work with empty "current locations", so that we
are able to step through multiple lines.
This also changes the annotations used to determine the breakpoint
locations in solib-reverse.c, adding a simple variable assignment right
before the return statement. This way GDB will not set a breakpoint in
the closing } line.
Approved-By: Tom Tromey <tom@tromey.com>
Clang does not add line information for lines that only contain a
closing } in functions. Many tests in the gdb.reverse folder set a
breakpoint in that line, but don't seem to use information available
after the return statement is executed, so this commit moves the
breakpoint to the previous line, where the return statement is.
Approved-By: Tom Tromey <tom@tromey.com>
As commit fa4f2d46f9 did for x86, there a few other targets that
wrongly use the output section rather than the dynamic section for
DT_JMPREL and others.
* elfnn-kvx.c (elfNN_kvx_finish_dynamic_sections): Use input
section for DT_JMPREL.
* elf32-sh.c (sh_elf_finish_dynamic_sections): Use input
section for DT_JMPREL and DT_PLTRELSZ.
* elf32-nds32.c (nds32_elf_finish_dynamic_sections): Likewise,
and for DT_PLTGOT and when adjusting DT_RELA.
This fixes test failures caused by the new linker warning which report:
./ld/ld-new: warning: load.S.x has a LOAD segment with RWX permissions
Fix this by splitting the linker MEMORY into ram and rom to avoid
generating RWX sections. This required tests to be adjusted to fix
issues with the move. Namely:
- fpu tests: were incorrectly using l.ori with ha(anchor) which now
that we pushed the anchor up in memory it exposes the bug. Update
to used the correct l.movhi instruction instead.
- adrp test: the test reports ram offset addresses, now that we have
moved memory layout around a bit I adjusted the test output. Some
padding is added before pi to show that the actual address of pi and
the adrp page offset are not the same.
Bug: https://sourceware.org/PR29957
When processing a STB_GNU_UNIQUE symbol we did not update has_gnu_osabi
correctly.
* config/tc-kvx.c (kvx_end): Do not write to e_ident.
(kvx_type): Properly handle STB_GNU_UNIQUE symbols.
When doing this in target.h:
...
- virtual gdb::byte_vector thread_info_to_thread_handle (struct thread_info *)
+ virtual gdb::byte_vector &thread_info_to_thread_handle (struct thread_info *)
...
make-target-delegates.py drops the function.
By handling '&' in POINTER_PART we can prevent that the function is dropped,
but when recompiling target.o we get:
...
gdb/target-delegates.c: In member function ‘virtual gdb::byte_vector& \
debug_target::thread_info_to_thread_handle(thread_info*)’:
gdb/target-delegates.c:1889:22: error: ‘result’ declared as reference but not \
initialized
gdb::byte_vector & result;
^~~~~~
make: *** [Makefile:1923: target.o] Error 1
...
Fix this by making sure result is initialized.
Regenerate target-delegates.c using this new style.
Tested on x86_64-linux.
Approved-By: Pedro Alves <pedro@palves.net>
If a linker script does not place the PLT relocations and "normal"
relocations in separate ELF sections, `ld` will currently output incorrect
values for DT_JMPREL and DT_PLTRELSZ - they cover the entire ELF section,
rather than just the PLT relocations
Don't ignore the extent of the BFD section - use the size of the srelplt
BFD section and its offset from the output_secttion
bfd/
PR ld/30787
* elfxx-x86.c (_bfd_x86_elf_finish_dynamic_sections): Use input
section for DT_JMPREL and DT_PLTRELSZ.
ld/
PR ld/30787
* testsuite/ld-i386/i386.exp: Run pr30787.
* testsuite/ld-x86-64/x86-64.exp: Likewise.
* testsuite/ld-i386/pr30787.d: New file.
* testsuite/ld-i386/pr30787.s: Likewise.
* testsuite/ld-i386/pr30787.t: Likewise.
* testsuite/ld-x86-64/pr30787.d: Likewise.
* testsuite/ld-x86-64/pr30787.s: Likewise.
* testsuite/ld-x86-64/pr30787.t: Likewise.
Since b080fe54fb "gdb: add inferior-specific breakpoints", the
breakpoint class has an "inferior" member used to handle
inferior-specific breakpoints. This creates a compilation error
in amd_dbgapi_target_breakpoint::check_status which declares a local
variable "inferior *inf".
Fix this by using "struct inferior *inf" instead.
Change-Id: Icc4dc1ba96c7d3ff9d33f9cb384ffcf64eba26fb
Approved-By: Pedro Alves <pedro@palves.net>
After running a number of programs under Windows gdb and detaching
them, I typed run in gdb, and got a hang, here:
(top-gdb) bt
#0 sharing_input_terminal (pid=4672) at /home/pedro/gdb/src/gdb/mingw-hdep.c:388
#1 0x00007ff71a2d8678 in sharing_input_terminal (inf=0x23bf23dafb0) at /home/pedro/gdb/src/gdb/inflow.c:269
#2 0x00007ff71a2d887b in child_terminal_save_inferior (self=0x23bf23de060) at /home/pedro/gdb/src/gdb/inflow.c:423
#3 0x00007ff71a2c80c0 in inf_child_target::terminal_save_inferior (this=0x23bf23de060) at /home/pedro/gdb/src/gdb/inf-child.c:111
#4 0x00007ff71a429c0f in target_terminal_is_ours_kind (desired_state=target_terminal_state::is_ours_for_output) at /home/pedro/gdb/src/gdb/target.c:1037
#5 0x00007ff71a429e02 in target_terminal::ours_for_output () at /home/pedro/gdb/src/gdb/target.c:1094
#6 0x00007ff71a2ccc8e in post_create_inferior (from_tty=0) at /home/pedro/gdb/src/gdb/infcmd.c:245
#7 0x00007ff71a2cd431 in run_command_1 (args=0x0, from_tty=0, run_how=RUN_NORMAL) at /home/pedro/gdb/src/gdb/infcmd.c:502
#8 0x00007ff71a2cd58b in run_command (args=0x0, from_tty=0) at /home/pedro/gdb/src/gdb/infcmd.c:527
The problem is that the loop around GetConsoleProcessList looped
forever, because there were exactly 10 processes to return.
GetConsoleProcessList's documentation says:
If the buffer is too small to hold all the valid process identifiers,
the return value is the required number of array elements. The
function will have stored no identifiers in the buffer. In this
situation, use the return value to allocate a buffer that is large
enough to store the entire list and call the function again.
In this case, the buffer wasn't too small, it was exactly the right
size, so we should have broken out of the loop. We didn't due to a
"<" check that should have been "<=". That is fixed by this patch.
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Change-Id: I14e4909f2ac2fa83d0d9b6e64418b5831ac4e4e3
Spotted a few places where a char is being treated as a bool. The GDB
style is to use explicit comparisons, so fix things up.
There should be no user visible changes after this commit.
PR 30791
* dwarf.h (debug_info): Add range_versions field.
* dwarf.c (read_and_display_attr_value): When recording a range arribute also ecord the dwarf version number.
(is_range_list_for_this_section): New function.
(display_debug_ranges): Only show debug ranges whose version is suitable for the secction being displayed.
This recent commit:
commit b1c0ab2080
Date: Wed Jul 12 21:56:50 2023 +0100
gdb: avoid double stop after failed breakpoint condition check
Addressed a problem where two MI stopped events would be reported if a
breakpoint condition failed due to a signal, this commit was a
replacement for this commit:
commit 2e411b8c68
Date: Fri Oct 14 14:53:15 2022 +0100
gdb: don't always print breakpoint location after failed condition check
which solved the two stop problem, but only for the CLI. Before both
of these commits, if a b/p condition failed due to a signal then the
user would see two stops reported, the first at the location where the
signal occurred, and the second at the location of the breakpoint.
By default GDB remains at the location where the signal occurred, so
the second reported stop can be confusing, this is the problem that
commit 2e411b8c68 tried to solve (for the CLI) and b1c0ab2080
extended also address the issue for MI too.
However, while working on another patch I realised that there was a
problem with GDB after the above commits. Neither of the above
commits considered 'set unwindonsignal on'. With this setting on,
when an inferior function call fails with a signal GDB will unwind the
stack back to the location where the inferior function call started.
In the b/p case we're looking at, the stop should be reported at the
location of the breakpoint, not at the location where the signal
occurred, and this isn't what happens.
This commit fixes this by ensuring that when unwindonsignal is 'on',
GDB reports a single stop event at the location of the breakpoint,
this fixes things for both CLI and MI.
The function call_thread_fsm::should_notify_stop is called when the
inferior function call completes and GDB is figuring out if the user
should be notified about this stop event by calling normal_stop from
fetch_inferior_event in infrun.c. If normal_stop is called, then this
notification will be for the location where the inferior call stopped,
which will be the location at which the signal occurred.
Prior to this commit, the only time that normal_stop was not called,
was if the inferior function call completed successfully, this was
controlled by ::should_notify_stop, which only turns false when the
inferior function call has completed successfully.
In this commit I have extended the logic in ::should_notify_stop. Now
there are three cases in which ::should_notify_stop will return false,
and we will not announce the first stop (by calling normal_stop).
These three reasons are:
1. If the inferior function call completes successfully, this is
unchanged behaviour,
2. If the inferior function call stopped due to a signal and 'set
unwindonsignal on' is in effect, and
3. If the inferior function call stopped due to an uncaught C++
exception, and 'set unwind-on-terminating-exception on' is in
effect.
However, if we don't call normal_stop then we need to call
async_enable_stdin in call_thread_fsm::should_stop. Prior to this
commit this was only done for the case where the inferior function
call completed successfully.
In this commit I now call ::should_notify_stop and use this to
determine if we need to call async_enable_stdin. With this done we
now call async_enable_stdin for each of the three cases listed above,
which means that GDB will exit wait_sync_command_done correctly (see
run_inferior_call in infcall.c).
With these two changes the problem is mostly resolved. However, the
solution isn't ideal, we've still lost some information.
Here is how GDB 13.1 behaves, this is before commits b1c0ab2080 and
2e411b8c68:
$ gdb -q /tmp/mi-condbreak-fail \
-ex 'set unwindonsignal on' \
-ex 'break 30 if (cond_fail())' \
-ex 'run'
Reading symbols from /tmp/mi-condbreak-fail...
Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
Starting program: /tmp/mi-condbreak-fail
Program received signal SIGSEGV, Segmentation fault.
0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
24 return *p; /* Crash here. */
Error in testing breakpoint condition:
The program being debugged was signaled while in a function called from GDB.
GDB has restored the context to what it was before the call.
To change this behavior use "set unwindonsignal off".
Evaluation of the expression containing the function
(cond_fail) will be abandoned.
Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
30 global_counter += 1; /* Set breakpoint here. */
(gdb)
In this state we see two stop notifications, the first is where the
signal occurred, while the second is where the breakpoint is located.
As GDB has unwound the stack (thanks to unwindonsignal) the second
stop notification reflects where the inferior is actually located.
Then after commits b1c0ab2080 and 2e411b8c68 the behaviour changed
to this:
$ gdb -q /tmp/mi-condbreak-fail \
-ex 'set unwindonsignal on' \
-ex 'break 30 if (cond_fail())' \
-ex 'run'
Reading symbols from /tmp/mi-condbreak-fail...
Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
Starting program: /tmp/mi-condbreak-fail
Program received signal SIGSEGV, Segmentation fault.
0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
24 return *p; /* Crash here. */
Error in testing condition for breakpoint 1:
The program being debugged was signaled while in a function called from GDB.
GDB has restored the context to what it was before the call.
To change this behavior use "set unwindonsignal off".
Evaluation of the expression containing the function
(cond_fail) will be abandoned.
(gdb) bt 1
#0 foo () at /tmp/mi-condbreak-fail.c:30
(More stack frames follow...)
(gdb)
This is the broken state. GDB is reports the SIGSEGV location, but
not the unwound breakpoint location. The final 'bt 1' shows that the
inferior is not located in cond_fail, which is the only location GDB
reported, so this is clearly wrong.
After implementing the fixes described above we now get this
behaviour:
$ gdb -q /tmp/mi-condbreak-fail \
-ex 'set unwindonsignal on' \
-ex 'break 30 if (cond_fail())' \
-ex 'run'
Reading symbols from /tmp/mi-condbreak-fail...
Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
Starting program: /tmp/mi-condbreak-fail
Error in testing breakpoint condition for breakpoint 1:
The program being debugged was signaled while in a function called from GDB.
GDB has restored the context to what it was before the call.
To change this behavior use "set unwindonsignal off".
Evaluation of the expression containing the function
(cond_fail) will be abandoned.
Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
30 global_counter += 1; /* Set breakpoint here. */
(gdb)
This is better. GDB now reports a single stop at the location of the
breakpoint, which is where the inferior is actually located. However,
by removing the first stop notification we have lost some potentially
useful information about which signal caused the inferior to stop.
To address this I've reworked the message that is printed to include
the signal information. GDB now reports this:
$ gdb -q /tmp/mi-condbreak-fail \
-ex 'set unwindonsignal on' \
-ex 'break 30 if (cond_fail())' \
-ex 'run'
Reading symbols from /tmp/mi-condbreak-fail...
Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
Starting program: /tmp/mi-condbreak-fail
Error in testing condition for breakpoint 1:
The program being debugged received signal SIGSEGV, Segmentation fault
while in a function called from GDB. GDB has restored the context
to what it was before the call. To change this behavior use
"set unwindonsignal off". Evaluation of the expression containing
the function (cond_fail) will be abandoned.
Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
30 global_counter += 1; /* Set breakpoint here. */
(gdb)
This is better, the user now sees a single stop notification at the
correct location, and the error message describes which signal caused
the inferior function call to stop.
However, we have lost the information about where the signal
occurred. I did consider trying to include this information in the
error message, but, in the end, I opted not too. I wasn't sure it was
worth the effort. If the user has selected to unwind on signal, then
surely this implies they maybe aren't interested in debugging failed
inferior calls, so, hopefully, just knowing the signal name will be
enough. I figure we can always add this information in later if
there's a demand for it.
New helper proc mi_info_frame which takes care of running the MI
-stack-info-frame command and matching its output.
Like the breakpoint helper procs, this new proc takes a name/value
argument list and uses this to build the expected result regexp. This
means that we can now write something like:
mi_info_frame "test name here" \
-level 0 -func name -line 123
Instead of the current equivalent:
mi_gdb_test "235-stack-info-frame" \
"235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"name\",file=\".*\",fullname=\".*\",line=\"123\",arch=\".*\"\}" \
"test name here"
There's also a helper proc mi_make_info_frame_regexp which is
responsible for building the 'frame={...}' part of the pattern.
I've update the two existing tests that use -stack-info-frame and
expect the command to succeed. There is another test that runs
-stack-info-frame and expects the command to fail -- the helper proc
doesn't help with this case, so that test is not changed.
Currently, each target backend is responsible for printing "[Thread
...exited]" before deleting a thread. This leads to unnecessary
differences between targets, like e.g. with the remote target, we
never print such messages, even though we do print "[New Thread ...]".
E.g., debugging the gdb.threads/attach-many-short-lived-threads.exp
with gdbserver, letting it run for a bit, and then pressing Ctrl-C, we
currently see:
(gdb) c
Continuing.
^C[New Thread 3850398.3887449]
[New Thread 3850398.3887500]
[New Thread 3850398.3887551]
[New Thread 3850398.3887602]
[New Thread 3850398.3887653]
...
Thread 1 "attach-many-sho" received signal SIGINT, Interrupt.
0x00007ffff7e6a23f in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7fffffffda80, rem=rem@entry=0x7fffffffda80)
at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
78 in ../sysdeps/unix/sysv/linux/clock_nanosleep.c
(gdb)
Above, we only see "New Thread" notifications, even though threads
were deleted.
After this patch, we'll see:
(gdb) c
Continuing.
^C[Thread 3558643.3577053 exited]
[Thread 3558643.3577104 exited]
[Thread 3558643.3577155 exited]
[Thread 3558643.3579603 exited]
...
[New Thread 3558643.3597415]
[New Thread 3558643.3600015]
[New Thread 3558643.3599965]
...
Thread 1 "attach-many-sho" received signal SIGINT, Interrupt.
0x00007ffff7e6a23f in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7fffffffda80, rem=rem@entry=0x7fffffffda80)
at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
78 in ../sysdeps/unix/sysv/linux/clock_nanosleep.c
(gdb) q
This commit fixes this by moving the thread exit printing to common
code instead, triggered from within delete_thread (or rather,
set_thread_exited).
There's one wrinkle, though. While most targest want to print:
[Thread ... exited]
the Windows target wants to print:
[Thread ... exited with code <exit_code>]
... and sometimes wants to suppress the notification for the main
thread. To address that, this commits adds a delete_thread_with_code
function, only used by that target (so far).
This fix was originally posted as part of a larger series:
https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-1-pedro@palves.net/
But didn't really need to be part of that series. In order to get
this fix merged sooner, I (Andrew Burgess) have rebased this commit
outside of the original series. Any bugs introduced while splitting
this patch out and rebasing, are entirely my own.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30129
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
After the previous commit, exit_inferior_1 no longer makes use of the
silent parameter. This commit removes this parameter and cleans up
the callers.
After doing this exit_inferior_1, exit_inferior, and
exit_inferior_silent are all equivalent, so rename exit_inferior_1 to
exit_inferior and delete exit_inferior_silent, update all the callers.
Also I spotted the declaration exit_inferior_num_silent in inferior.h,
but this function is not defined anywhere, so I deleted the
declaration.
There should be no user visible changes after this commit.
After this commit:
commit a78ef87574
Date: Wed Jun 22 18:10:00 2022 +0100
Always emit =thread-exited notifications, even if silent
The function mi_interp::on_thread_exited (or mi_thread_exit as the
function was called back then) no longer makes use of the "silent"
parameter.
As a result there is no difference between inferior::clear_thread_list
with silent true or false, because:
- None of the interpreter ::on_thread_exited functions rely on the
silent parameter, and
- None of GDB's thread_exit observers rely on the silent parameter
either.
This commit removes the silent parameter from
inferior::clear_thread_list, and makes the function always silent.
This commit was originally part of a larger series:
https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-1-pedro@palves.net/
But didn't really need to be part of that series. I had an interest
in seeing this patch merged:
https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-31-pedro@palves.net/
Which also didn't really need to be part of the larger series, but
does depend, at least a little, on this commit. In order to get the
fix I'm interested in merged quicker, I (Andrew Burgess) have rebased
this commit outside of the original series. Any bugs introduced while
splitting this patch out and rebasing, are entirely my own.
There should be no user visible changes after this commit.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Remove the static mi_parse::make functions, and instead use the
mi_parse constructor.
This is a partial revert of the commit:
commit fde3f93adb
Date: Mon Mar 20 10:56:55 2023 -0600
Introduce "static constructor" for mi_parse
which introduced the mi_parse::make functions, though after discussion
on the list the reasons for seem to have been lost[1]. Given there
are no test regressions when moving back to using the constructors, I
propose we should do that for now.
There should be no user visible changes after this commit.
[1] https://inbox.sourceware.org/gdb-patches/20230404-dap-loaded-sources-v2-5-93f229095e03@adacore.com/
Approved-By: Tom Tromey <tom@tromey.com>
Have the mi_out_new function return a std::unique_ptr instead of a raw
pointer. Update the two uses of mi_out_new.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
While GDB is still C++11, lets add a gdb::make_unique template
function that can be used to create std::unique_ptr objects, just like
the C++14 std::make_unique.
If GDB is being compiled with a C++14 compiler then the new
gdb::make_unique function will delegate to the std::make_unique. I
checked with gcc, and at -O1 and above gdb::make_unique will be
optimised away completely in this case.
If C++14 (or later) becomes our minimum, then it will be easy enough
to go through the code and replace gdb::make_unique with
std::make_unique later on.
I've make use of this function in all the places I think this can
easily be used, though I'm sure I've probably missed some.
Should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
In this commit:
commit b080fe54fb
Date: Tue Nov 8 12:32:51 2022 +0000
gdb: add inferior-specific breakpoints
limited support was added in lib/mi-support.exp to help with testing
of inferior specific breakpoints.
Though the changes that were added were not wrong, while working on a
later patch, I realised that I had added the support in the wrong
place -- I only added support to mi_make_breakpoint_multi, when really
I should have added the support to mi_make_breakpoint_1, which is used
by all of the MI procs that create breakpoints.
This commit moves the support to mi_make_breakpoint_1, and updates all
the procs that use mi_make_breakpoint_1 to accept, and then pass
through, and (optional) inferior argument. This will make it much
easier to write MI tests for inferior specific breakpoints.
There's no change in what is tested after this commit.
The commit:
commit b080fe54fb
Date: Tue Nov 8 12:32:51 2022 +0000
gdb: add inferior-specific breakpoints
introduced a bug in the function breakpoint_set_inferior. The above
commit includes this line:
gdb::observers::breakpoint_modified.notify (b);
when it should have instead used this line:
notify_breakpoint_modified (b);
The change to use notify_breakpoint_modified was introduced to GDB
after commit b080fe54fb was written, but before it was merged, and I
failed to update this part of the code during the rebase.
The consequence of this error is that the MI interpreter will not emit
breakpoint-modified notifications when breakpoint_set_inferior is
called.
In this commit I update the code to call notify_breakpoint_modified,
and add a test that checks the MI events are being emitted correctly
in this case.
bfd/
* Makefile.am: Move elf32-kvx.lo from BFD32_BACKENDS to
BFD64_BACKENDS. Remove elfxx-kvx.lo from BFD32_BACKENDS.
Remove elfxx-kvx.c from BFD32_BACKENDS_CFILES.
* Makefile.in: Regenerate.
* config.bfd: Adjust targ_defvec and targ_selvecs and gate them
behind BFD64.
* configure.ac: Add target_size=64 to kvx_elf64_*vec.
* configure: Regenerate.
* elfnn-kvx.c (elfNN_kvx_stub_name): Cast rel->r_addend to
uint64_t to match format string.
(elfNN_kvx_relocate_section): Similarly for r_offset, and
use PRIx64 in format string.
* targets.c (_bfd_target_vector <kvx_elf32_vec>): Move inside
#ifdef BFD64.
ld/
* Makefile.am: Move eelf32kvx.c from ALL_EMULATION_SOURCES to
ALL_64_EMULATION_SOURCES.
* Makefile.in: Regenerate.
O_pseudo_fixup was defined as O_max+1, missing the fact that O_md1
through O_md32 enums are for use by target code. Worse, kvx-parse.c
used 64 rather than O_pseudo_fixup. Fix this, and wrap some overlong
lines.
* config/tc-kvx.h (O_pseudo_fixup): Define.
* config/tc-kvx.c (O_pseudo_fixup): Don't define here.
(insert_operand): Delete bogus comment and cast.
* config/kvx-parse.c (promote_token): Use O_pseudo_fixup
rather than hardcoded value. Wrap overlong lines.
(get_token_class): Likewise.
(parse_with_restarts): Wrap overlong line.
This fixes a few places where ubsan complains about signed integer
overflow when running the testsuite, and that clz(0) is undefined.
When fixing the clz problem, I also noticed that we'd get complaints
if pval is ever LLONG_MIN. Fix that by using unsigned arithmetic.
* config/kvx-parse.c (get_token_class): Avoid signed overflow.
Don't clz(0).
* config/tc-kvx.c (PARALLEL_BIT): Avoid signed overflow.
kvx-parse.c:parse_with_restarts does
if (!tok.insn[tok.begin])
tok.class_id = -3;
then a little later
printf_debug (1, "\nEntering rule: %d (Trying to match: (%s)[%d])\n", jump_target,
TOKEN_NAME (CLASS_ID (tok)), CLASS_ID (tok));
This results in a buffer overrun in TOKEN_NAME. Fix that.
* config/tc-kvx.h (TOKEN_NAME): Check for tok <= 0, not just -1.
It is generally a good idea to avoid signed arithmetic on values
extracted from object files, to avoid ubsan warnings on overflow.
This patch replaces some uses of bfd_signed_vma in the kvx backend
with bfd_vma, and removes _bfd_kvx_elf_resolve_relocation, a
do-nothing function. In the process of making this patch I noticed
some dead code in the GOT entry handling, setting value to
got_entry_addr but using "off" in the _bfd_final_link_relocate call.
Since kvx_calculate_got_entry_vma also returns the GOT offset, I
presume the code is correct, but I've left the dead code and comment
there.
* elfxx-kvx.h (_bfd_kvx_elf_resolve_relocation): Delete.
* elfxx-kvx.c (kvx_signed_overflow): Rewrite using unsigned
arithmetic.
(_bfd_kvx_elf_resolve_relocation): Delete.
* elfnn-kvx.c (kvx_relocate): Update for
_bfd_kvx_elf_resolve_relocation removal.
(elfNN_kvx_final_link_relocate): Likewise. Don't use a signed
addend.
Some places matching the first char of a string against
bfd_get_symbol_leading_char, which may be zero, didn't check for "".
This could lead to accesses past the end of the string and potential
buffer overruns. Fix that, and also get rid of a stupid optimisation
in dbxread when looking for "__DYNAMIC" that also might access past
the end of a string.
Some places matching the first char of a string against
bfd_get_symbol_leading_char, which may be zero, didn't check for the
string being "". This patch adds the check to stop accesses past the
end of the string and potential buffer overruns.
The dlltool one was found by oss-fuzz quite a while ago.
bfd/
* cofflink.c (_bfd_coff_link_input_bfd): Ensure a zero
bfd_get_symbol_leading_char doesn't lead to accessing past the
zero string terminator.
* linker.c (bfd_wrapped_link_hash_lookup): Likewise.
(unwrap_hash_lookup): Likewise.
binutils/
* dlltool.c (scan_filtered_symbols): Ensure a zero
bfd_get_symbol_leading_char doesn't lead to accessing past the
zero string terminator.
When building gdb with -flto -O2, I run into:
...
opcodes/mep-desc.h:250:14: warning: type 'cgen_operand_type' violates the \
C++ One Definition Rule [-Wodr]
typedef enum cgen_operand_type {
^
opcodes/or1k-desc.h:624:14: note: an enum with different value name is \
defined in another translation unit
typedef enum cgen_operand_type {
^
opcodes/mep-desc.h:212:14: warning: type 'cgen_hw_type' violates the C++ One \
Definition Rule [-Wodr]
typedef enum cgen_hw_type {
^
opcodes/or1k-desc.h:433:14: note: an enum with different value name is \
defined in another translation unit
typedef enum cgen_hw_type {
^
...
Fix this by making the conflicting type names unique, adding a target-specific
prefix using a define before the include:
...
#define cgen_operand_type <target-name>_cgen_operand_type
#define cgen_hw_type <target-name>_cgen_hw_type
#include "opcodes/<target-name>-desc.h"
...
and move those defines into a new file cgen-remap.h, similar to how that's
done for yacc in yy-remap.h.
Likewise for targets frv and lm32, the two other targets that include
opcodes/<target-name>-desc.h.
Likewise for more cgen symbols that I got the same warning for when using
-flto-partition=one.
A PR has been filed to take care of this in the opcodes dir instead (PR30758).
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR build/30757
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30757
I noticed a call to value::copy in gdbpy_get_varobj_pretty_printer,
and I couldn't figure out why it was there. I think maybe it came
from the time when value_to_value_object would release values from the
value chain -- but that was removed in commit f3d3bbbc.
This patch removes this call. Regression tested on x86-64 Fedora 36.
Historically, flags and variables relating to architectural revisions
for the A-profile architecture omitted the trailing `A' such that, for
example, assembling for `-march=armv8.4-a' set the `AARCH64_ARCH_V8_4'
flag in the assembler.
This leads to some ambiguity, since Binutils also targets the
R-profile Arm architecture. Therefore, it seems prudent to have
everything associated with the A-profile cores end in `A' and likewise
`R' for the R-profile. Referring back to the example above, the flag
set for `-march=armv8.4-a' is better characterized if labeled
`AARCH64_ARCH_V8_4A'.
The only exception to the rule of appending `A' to variables is found
in the handling of the `AARCH64_FEATURE_V8' macro, as it is the
baseline from which ALL processors derive and should therefore be left
unchanged.
In reflecting the `ARM' architectural nomenclature choices, where we
have `ARM_ARCH_V8A' and `ARM_ARCH_V8R', the choice is made to not have
an underscore separating the numerical revision number and the
A/R-profile indicator suffix. This has meant that renaming of
R-profile related flags and variables was warranted, thus going from
`.*_[vV]8_[rR]' to `.*_[vV]8[rR]'.
Finally, this is more in line with conventions within GCC and adds consistency
across the toolchain.
gas/ChangeLog:
* gas/config/tc-aarch64.c:
(aarch64_cpus): Reference to arch feature macros updated.
(aarch64_archs): Likewise.
include/ChangeLog:
* include/opcode/aarch64.h:
(AARCH64_FEATURE_V8A): Updated name: V8_A -> V8A.
(AARCH64_FEATURE_V8_1A): A-suffix added.
(AARCH64_FEATURE_V8_2A): Likewise.
(AARCH64_FEATURE_V8_3A): Likewise.
(AARCH64_FEATURE_V8_4A): Likewise.
(AARCH64_FEATURE_V8_5A): Likewise.
(AARCH64_FEATURE_V8_6A): Likewise.
(AARCH64_FEATURE_V8_7A): Likewise.
(AARCH64_FEATURE_V8_8A):Likewise.
(AARCH64_FEATURE_V9A): Likewise.
(AARCH64_FEATURE_V8R): Updated name: V8_R -> V8R.
(AARCH64_ARCH_V8A_FEATURES): Updated name: V8_A -> V8A.
(AARCH64_ARCH_V8_1A_FEATURES): A-suffix added.
(AARCH64_ARCH_V8_2A_FEATURES): Likewise.
(AARCH64_ARCH_V8_3A_FEATURES): Likewise.
(AARCH64_ARCH_V8_4A_FEATURES): Likewise.
(AARCH64_ARCH_V8_5A_FEATURES): Likewise.
(AARCH64_ARCH_V8_6A_FEATURES): Likewise.
(AARCH64_ARCH_V8_7A_FEATURES): Likewise.
(AARCH64_ARCH_V8_8A_FEATURES): Likewise.
(AARCH64_ARCH_V9A_FEATURES): Likewise.
(AARCH64_ARCH_V9_1A_FEATURES): Likewise.
(AARCH64_ARCH_V9_2A_FEATURES): Likewise.
(AARCH64_ARCH_V9_3A_FEATURES): Likewise.
(AARCH64_ARCH_V8A): Updated name: V8_A -> V8A.
(AARCH64_ARCH_V8_1A): A-suffix added.
(AARCH64_ARCH_V8_2A): Likewise.
(AARCH64_ARCH_V8_3A): Likewise.
(AARCH64_ARCH_V8_4A): Likewise.
(AARCH64_ARCH_V8_5A): Likewise.
(AARCH64_ARCH_V8_6A): Likewise.
(AARCH64_ARCH_V8_7A): Likewise.
(AARCH64_ARCH_V8_8A): Likewise.
(AARCH64_ARCH_V9A): Likewise.
(AARCH64_ARCH_V9_1A): Likewise.
(AARCH64_ARCH_V9_2A): Likewise.
(AARCH64_ARCH_V9_3A): Likewise.
(AARCH64_ARCH_V8_R): Updated name: V8_R -> V8R.
opcodes/ChangeLog:
* opcodes/aarch64-opc.c (SR_V8A): Updated name: V8_A -> V8A.
(SR_V8_1A): A-suffix added.
(SR_V8_2A): Likewise.
(SR_V8_3A): Likewise.
(SR_V8_4A): Likewise.
(SR_V8_6A): Likewise.
(SR_V8_7A): Likewise.
(SR_V8_8A): Likewise.
(aarch64_sys_regs): Reference to arch feature macros updated.
(aarch64_pstatefields): Reference to arch feature macros updated.
(aarch64_sys_ins_reg_supported_p): Reference to arch feature macros
updated.
* opcodes/aarch64-tbl.h:
(aarch64_feature_v8_2a): a-suffix added.
(aarch64_feature_v8_3a): Likewise.
(aarch64_feature_fp_v8_3a): Likewise.
(aarch64_feature_v8_4a): Likewise.
(aarch64_feature_fp_16_v8_2a): Likewise.
(aarch64_feature_v8_5a): Likewise.
(aarch64_feature_v8_6a): Likewise.
(aarch64_feature_v8_7a): Likewise.
(aarch64_feature_v8r): Updated name: v8_r-> v8r.
(ARMV8R): Updated name: V8_R-> V8R.
(ARMV8_2A): A-suffix added.
(ARMV8_3A): Likewise.
(FP_V8_3A): Likewise.
(ARMV8_4A): Likewise.
(FP_F16_V8_2A): Likewise.
(ARMV8_5): Likewise.
(ARMV8_6A): Likewise.
(ARMV8_6A_SVE): Likewise.
(ARMV8_7A): Likewise.
(V8_2A_INSN): `A' added to macro symbol.
(V8_3A_INSN): Likewise.
(V8_4A_INSN): Likewise.
(FP16_V8_2A_INSN): Likewise.
(V8_5A_INSN): Likewise.
(V8_6A_INSN): Likewise.
(V8_7A_INSN): Likewise.
(V8R_INSN): Updated name: V8_R-> V8R.
Fuzzers have found that objdump -W takes a really long time if
the entry count uleb is ridiculously large, and format attributes
don't consume data (which doesn't make sense for a table of names).
* dwarf.c (display_formatted_table): Sanity check count of
table entries.
kvx_dis_init currently always returns true, but error conditions do so
by "return -1" which converts to true. The return status is ignored
anyway, and it doesn't make much sense to error on unexpected arch or
mach: If print_insn_kvx is called then the atch is known to be kvx,
and it's better to choose some default for a user passing an unknown
mach value rather than segfaulting in decode_insn when env.opc_table
is NULL.
I've chosen the default mach to be bfd_mach_kv3_1, the default in
bfd/cpu-kvx.c, not that it matters very much. In normal objdump/gdb
usage, info->mach won't be an unexpected value.
* kvx-dis.c (kvx_dis_init): Return void. Don't error on
unexpected arch or mach. Default to bfd_mach_kv3_1 for
unknown mach. Don't clear info->disassembler_options.
A misplaced line, resulting in testsuite errors when attempting to use
as -m32.
* config.bfd (kvx-*-linux*): Add targ_selvecs.
(kvx-*-*): Remove targ_selvecs.
