Jeff Law [Wed, 6 Apr 2022 15:06:53 +0000 (11:06 -0400)]
Fix "bins" simulation for v850e3v5
I've been carrying this for a few years. One test in the GCC testsuite is
failing due to a bug in the handling of the v850e3v5 instruction "bins".
When the "bins" instruction specifies a 32bit bitfield size, the simulator
exhibits undefined behavior by trying to shift a 32 bit quantity by 32 bits.
In the case of a 32 bit shift, we know what the resultant mask should be. So
we can just set it.
That seemed better than using 1UL for the constant (on a 32bit host unsigned
long might still just be 32 bits) or needlessly forcing everything to
long long types.
Thankfully the case where this shows up is only bins <src>, 0, 32, <dest>
which would normally be encoded as a simple move.
* testsuite/v850/allinsns.exp: Add v850e3v5.
* testsuite/v850/bins.cgs: New test.
* v850/simops.c (v850_bins): Avoid undefined behavior on left shift.
Tiezhu Yang [Thu, 31 Mar 2022 11:56:51 +0000 (19:56 +0800)]
gdb: LoongArch: prepend tramp frame unwinder for signal
Implement the "init" method of struct tramp_frame to prepend tramp
frame unwinder for signal on LoongArch.
With this patch, the following failed testcases can be fixed:
FAIL: gdb.base/annota1.exp: backtrace @ signal handler (timeout)
FAIL: gdb.base/annota3.exp: backtrace @ signal handler (pattern 2)
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Andrew Burgess [Wed, 24 Nov 2021 17:18:34 +0000 (17:18 +0000)]
gdb: make interp_add static
Since this commit:
commit
8322445e0584be846f5873b9aab257dc9fbda05d
Date: Tue Jun 21 01:11:45 2016 +0100
Introduce interpreter factories
Interpreters should be registered with GDB, not by calling interp_add,
but with a call to interp_factory_register. I've checked the insight
source, and it too has moved over to using interp_factory_register.
In this commit I make interp_add static within interps.c.
There should be no user visible change after this commit.
Nick Clifton [Wed, 6 Apr 2022 13:43:37 +0000 (14:43 +0100)]
Add code to display the contents of .debug_loclists sections which contain offset entry tables.
PR 28981
* dwarf.c (fetch_indexed_value): Rename to fecth_indexed_addr and
return the address, rather than a string.
(fetch_indexed_value): New function - returns a value indexed by a
DW_FORM_loclistx or DW_FORM_rnglistx form.
(read_and_display_attr_value): Add support for DW_FORM_loclistx
and DW_FORM_rnglistx.
(process_debug_info): Load the loclists and rnglists sections.
(display_loclists_list): Add support for DW_LLE_base_addressx,
DW_LLE_startx_endx, DW_LLE_startx_length and
DW_LLE_default_location.
(display_offset_entry_loclists): New function. Displays a
.debug_loclists section that contains offset entry tables.
(display_debug_loc): Call the new function.
(display_debug_rnglists_list): Add support for
DW_RLE_base_addressx, DW_RLE_startx_endx and DW_RLE_startx_length.
(display_debug_ranges): Display the contents of the section's
header.
* dwarf.h (struct debug_info): Add loclists_base field.
* testsuite/binutils-all/dw5.W: Update expected output.
* testsuite/binutils-all/x86-64/pr26808.dump: Likewise.
Luis Machado [Mon, 1 Nov 2021 20:14:26 +0000 (17:14 -0300)]
Enable ARMv8.1-m PACBTI support
This set of changes enable support for the ARMv8.1-m PACBTI extensions [1].
The goal of the PACBTI extensions is similar in scope to that of a-profile
PAC/BTI (aarch64 only), but the underlying implementation is different.
One important difference is that the pointer authentication code is stored
in a separate register, thus we don't need to mask/unmask the return address
from a function in order to produce a correct backtrace.
The patch introduces the following modifications:
- Extend the prologue analyser for 32-bit ARM to handle some instructions
from ARMv8.1-m PACBTI: pac, aut, pacg, autg and bti. Also keep track of
return address signing/authentication instructions.
- Adds code to identify object file attributes that indicate the presence of
ARMv8.1-m PACBTI (Tag_PAC_extension, Tag_BTI_extension, Tag_PACRET_use and
Tag_BTI_use).
- Adds support for DWARF pseudo-register RA_AUTH_CODE, as described in the
aadwarf32 [2].
- Extends the dwarf unwinder to track the value of RA_AUTH_CODE.
- Decorates backtraces with the "[PAC]" identifier when a frame has signed
the return address.
- Makes GDB aware of a new XML feature "org.gnu.gdb.arm.m-profile-pacbti". This
feature is not included as an XML file on GDB's side because it is only
supported for bare metal targets.
- Additional documentation.
[1] https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension
[2] https://github.com/ARM-software/abi-aa/blob/main/aadwarf32/aadwarf32.rst
Andrew Burgess [Mon, 4 Apr 2022 20:07:54 +0000 (21:07 +0100)]
gdb: move gdb_disassembly_flag into a new disasm-flags.h file
While working on the disassembler I was getting frustrated. Every
time I touched disasm.h it seemed like every file in GDB would need to
be rebuilt. Surely the disassembler can't be required by that many
parts of GDB, right?
Turns out that disasm.h is included in target.h, so pretty much every
file was being rebuilt!
The only thing from disasm.h that target.h needed is the
gdb_disassembly_flag enum, as this is part of the target_ops api.
In this commit I move gdb_disassembly_flag into its own file. This is
then included in target.h and disasm.h, after which, the number of
files that depend on disasm.h is much reduced.
I also audited all the other includes of disasm.h and found that the
includes in mep-tdep.c and python/py-registers.c are no longer needed,
so I've removed these.
Now, after changing disasm.h, GDB rebuilds much quicker.
There should be no user visible changes after this commit.
GDB Administrator [Wed, 6 Apr 2022 00:00:09 +0000 (00:00 +0000)]
Automatic date update in version.in
Tom Tromey [Tue, 5 Apr 2022 13:44:59 +0000 (07:44 -0600)]
Introduce wrapped_file
Simon pointed out that timestamped_file probably needed to implement a
few more methods. This patch introduces a new file-wrapping file that
forwards most of its calls, making it simpler to implement new such
files. It also converts timestamped_file and pager_file to use it.
Regression tested on x86-64 Fedora 34.
Tom Tromey [Tue, 5 Apr 2022 13:25:10 +0000 (07:25 -0600)]
Don't call init_thread_list in windows-nat.c
I don't think there's any need to call init_thread_list in
windows-nat.c. This patch removes it. I tested this using the
internal AdaCore test suite on Windows, which FWIW does include some
multi-threaded inferiors.
Simon Marchi [Mon, 4 Apr 2022 18:45:36 +0000 (14:45 -0400)]
gdb/testsuite: fix intermittent failure in gdb.base/vfork-follow-parent.exp
Tom de Vries reported some failures in this test:
continue
Continuing.
[New inferior 2 (process 14967)]
Thread 1.1 "vfork-follow-pa" hit Breakpoint 2, break_parent () at /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/vfork-follow-parent.c:23
23 }
(gdb) FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: continue to end of inferior 2
inferior 1
[Switching to inferior 1 [process 14961] (/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.base/vfork-follow-parent/vfork-follow-parent)]
[Switching to thread 1.1 (process 14961)]
#0 break_parent () at /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/vfork-follow-parent.c:23
23 }
(gdb) PASS: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: inferior 1
continue
Continuing.
[Inferior 2 (process 14967) exited normally]
(gdb) FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: continue to break_parent (the program exited)
Here, we continue both the vfork parent and child, since
schedule-multiple is on. The child exits, which un-freezes the parent
and makes an exit event available to GDB. We expect GDB to consume this
exit event and present it to the user. Here, we see that GDB shows the
parent hitting a breakpoint before showing the child exit.
Because of the vfork, we know that chronologically, the child exiting
must have happend before the parent hitting a breakpoint. However,
scheduling being what it is, it is possible for the parent to un-freeze
and exit quickly, such that when GDB pulls events out of the kernel,
exit events for both processes are available. And then, GDB may chose
at random to return the one for the parent first. This is what I
imagine what causes the failure shown above.
We could change the test to expect both possible outcomes, but I wanted
to avoid complicating the .exp file that way. Instead, add a variable
that the parent loops on that we set only after we confirmed the exit of
the child. That should ensure that the order is always the same.
Note that I wasn't able to reproduce the failure, so I can't tell if
this fix really fixes the problem.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29021
Change-Id: Ibc8e527e0e00dac54b22021fe4d9d8ab0f3b28ad
Simon Marchi [Mon, 4 Apr 2022 16:08:22 +0000 (12:08 -0400)]
gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp
I got failures like this once on a CI:
frame^M
&"frame\n"^M
~"#0 child_sub_function () at /home/jenkins/workspace/binutils-gdb_master_build/arch/amd64/target_board/unix/src/binutils-gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:33\n"^M
~"33\t dummy = !dummy; /* thread loop line */\n"^M
^done^M
(gdb) ^M
FAIL: gdb.mi/mi-cmd-user-context.exp: frame 1 (unexpected output)
The problem is that the test expects the following regexp:
".*#0 0x.*"
And that typically works, when the output of the frame command looks
like:
#0 0x00005555555551bb in child_sub_function () at ...
Note the lack of hexadecimal address in the failing case. Whether or
not the hexadecimal address is printed (roughly) depends on whether the
current PC is at the beginning of a line. So depending on where thread
2 was when GDB stopped it (after thread 1 hit its breakpoint), we can
get either output. Adjust the regexps to not expect an hexadecimal
prefix (0x) but a function name instead (either child_sub_function or
child_function). That one is always printed, and is also a good check
that we are in the frame we expect.
Note that for test "frame 5", we are showing a pthread frame (on my
system), so the function name is internal to pthread, not something we
can rely on. In that case, it's almost certain that we are not at the
beginning of a line, or that we don't have debug info, so I think it's
fine to expect the hex prefix.
And for test "frame 6", it's ok to _not_ expect a hex prefix (what the
test currently does), since we are showing thread 1, which has hit a
breakpoint placed at the beginning of a line.
When testing this, Tom de Vries pointed out that the current test code
doesn't ensure that the child threads are in child_sub_function when
they are stopped. If the scheduler chooses so, it is possible for the
child threads to be still in the pthread_barrier_wait or child_function
functions when they get stopped. So that would be another racy failure
waiting to happen.
The only way I can think of to ensure the child threads are in the
child_sub_function function when they get stopped is to synchronize the
threads using some variables instead of pthread_barrier_wait. So,
replace the barrier with an array of flags (one per child thread). Each
child thread flips its flag in child_sub_function to allow the main
thread to make progress and eventually hit the breakpoint.
I copied user-selected-context-sync.c to a new mi-cmd-user-context.c and
made modifications to that, to avoid interfering with
user-selected-context-sync.exp.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29025
Change-Id: I919673bbf9927158beb0e8b7e9e980b8d65eca90
Luis Machado [Thu, 31 Mar 2022 15:45:53 +0000 (16:45 +0100)]
Fix qRcmd error code parsing
Someone at IRC spotted a bug in qRcmd handling. This looks like an oversight
or it is that way for historical reasons.
The code in gdb/remote.c:remote_target::rcmd uses isdigit instead of
isxdigit. One could argue that we are expecting decimal numbers, but further
below we use fromhex ().
Update the function to use isxdigit instead and also update the documentation.
I see there are lots of other cases of undocumented number format for error
messages, mostly described as NN instead of nn. For now I'll just update
this particular function.
Simon Marchi [Fri, 14 Jan 2022 21:18:03 +0000 (16:18 -0500)]
gdb: resume ongoing step after handling fork or vfork
The test introduced by this patch would fail in this configuration, with
the native-gdbserver or native-extended-gdbserver boards:
FAIL: gdb.threads/next-fork-other-thread.exp: fork_func=fork: target-non-stop=auto: non-stop=off: displaced-stepping=auto: i=2: next to for loop
The problem is that the step operation is forgotten when handling the
fork/vfork. With "debug infrun" and "debug remote", it looks like this
(some lines omitted for brevity). We do the next:
[infrun] proceed: enter
[infrun] proceed: addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT
[infrun] resume_1: step=1, signal=GDB_SIGNAL_0, trap_expected=0, current thread [
4154304.
4154304.0] at 0x5555555553bf
[infrun] do_target_resume: resume_ptid=
4154304.0.0, step=1, sig=GDB_SIGNAL_0
[remote] Sending packet: $vCont;r5555555553bf,
5555555553c4:p3f63c0.3f63c0;c:p3f63c0.-1#cd
[infrun] proceed: exit
We then handle a fork event:
[infrun] fetch_inferior_event: enter
[remote] wait: enter
[remote] Packet received: T05fork:p3f63ee.3f63ee;06:
0100000000000000;07:
b08e59f6ff7f0000;10:
bf60e8f7ff7f0000;thread:p3f63c0.3f63c6;core:17;
[remote] wait: exit
[infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
[infrun] print_target_wait_results:
4154304.
4154310.0 [Thread
4154304.
4154310],
[infrun] print_target_wait_results: status->kind = FORKED, child_ptid =
4154350.
4154350.0
[infrun] handle_inferior_event: status->kind = FORKED, child_ptid =
4154350.
4154350.0
[remote] Sending packet: $D;3f63ee#4b
[infrun] resume_1: step=0, signal=GDB_SIGNAL_0, trap_expected=0, current thread [
4154304.
4154310.0] at 0x7ffff7e860bf
[infrun] do_target_resume: resume_ptid=
4154304.0.0, step=0, sig=GDB_SIGNAL_0
[remote] Sending packet: $vCont;c:p3f63c0.-1#73
[infrun] fetch_inferior_event: exit
In the first snippet, we resume the stepping thread with the range-stepping (r)
vCont command. But after handling the fork (detaching the fork child), we
resumed the whole process freely. The stepping thread, which was paused by
GDBserver while reporting the fork event, was therefore resumed freely, instead
of confined to the addresses of the stepped line. Note that since this
is a "next", it could be that we have entered a function, installed a
step-resume breakpoint, and it's ok to continue freely the stepping
thread, but that's not the case here. The two snippets shown above were
next to each other in the logs.
For the fork case, we can resume stepping right after handling the
event.
However, for the vfork case, where we are waiting for the
external child process to exec or exit, we only resume the thread that
called vfork, and keep the others stopped (see patch "gdb: fix handling of
vfork by multi-threaded program" prior in this series). So we can't
resume the stepping thread right now. Instead, do it after handling the
vfork-done event.
Change-Id: I92539c970397ce880110e039fe92b87480f816bd
Simon Marchi [Tue, 18 Jan 2022 01:49:07 +0000 (20:49 -0500)]
gdb/remote: remove_new_fork_children don't access target_waitstatus::child_ptid if kind == TARGET_WAITKIND_THREAD_EXITED
Following the previous patch, running
gdb.threads/forking-threads-plus-breakpoints.exp continuously eventually
gives me an internal error.
gdb/target/waitstatus.h:372: internal-error: child_ptid: Assertion `m_kind == TARGET_WAITKIND_FORKED || m_kind == TARGET_WAITKIND_VFORKED' failed.^M
FAIL: gdb.threads/forking-threads-plus-breakpoint.exp: cond_bp_target=0: detach_on_fork=on: displaced=off: inferior 1 exited (GDB internal error)
The backtrace is:
0x55925b679c85 internal_error(char const*, int, char const*, ...)
/home/simark/src/binutils-gdb/gdbsupport/errors.cc:55
0x559258deadd2 target_waitstatus::child_ptid() const
/home/simark/src/binutils-gdb/gdb/target/waitstatus.h:372
0x55925a7cbac9 remote_target::remove_new_fork_children(threads_listing_context*)
/home/simark/src/binutils-gdb/gdb/remote.c:7311
0x55925a79dfdb remote_target::update_thread_list()
/home/simark/src/binutils-gdb/gdb/remote.c:3981
0x55925ad79b83 target_update_thread_list()
/home/simark/src/binutils-gdb/gdb/target.c:3793
0x55925addbb15 update_thread_list()
/home/simark/src/binutils-gdb/gdb/thread.c:2031
0x559259d64838 stop_all_threads(char const*, inferior*)
/home/simark/src/binutils-gdb/gdb/infrun.c:5104
0x559259d88b45 keep_going_pass_signal
/home/simark/src/binutils-gdb/gdb/infrun.c:8215
0x559259d8951b keep_going
/home/simark/src/binutils-gdb/gdb/infrun.c:8251
0x559259d78835 process_event_stop_test
/home/simark/src/binutils-gdb/gdb/infrun.c:6858
0x559259d750e9 handle_signal_stop
/home/simark/src/binutils-gdb/gdb/infrun.c:6580
0x559259d6c07b handle_inferior_event
/home/simark/src/binutils-gdb/gdb/infrun.c:5832
0x559259d57db8 fetch_inferior_event()
/home/simark/src/binutils-gdb/gdb/infrun.c:4222
Indeed, the code accesses target_waitstatus::child_ptid when the kind
is TARGET_WAITKIND_THREAD_EXITED, which is not right. A
TARGET_WAITKIND_THREAD_EXITED event does not have a child_ptid value
associated, it has an exit status, which we are not interested in. The
intent is to remove from the thread list the thread that has exited.
Its ptid is found in the stop reply event, get it from there.
Change-Id: Icb298cbb80b8779fdf0c660dde9a5314d5591535
Simon Marchi [Sat, 15 Jan 2022 15:55:31 +0000 (10:55 -0500)]
gdbserver: report correct status in thread stop race condition
The test introduced by the following patch would sometimes fail in this
configuration:
FAIL: gdb.threads/next-fork-other-thread.exp: fork_func=vfork: target-non-stop=on: non-stop=off: displaced-stepping=auto: i=14: next to for loop
The test has multiple threads constantly forking or vforking while the
main thread keep doing "next"s.
(After writing the commit message, I realized this also fixes a similar
failure in gdb.threads/forking-threads-plus-breakpoint.exp with the
native-gdbserver and native-extended-gdbserver boards.)
As stop_all_threads is called, because the main thread finished its
"next", it inevitably happens at some point that we ask the remote
target to stop a thread and wait() reports that this thread stopped with
a fork or vfork event, instead of the SIGSTOP we sent to try to stop it.
While running this test, I attached to GDBserver and stopped at
linux-low.cc:3626. We can see that the status pulled from the kernel
for
2742805 is indeed a vfork event:
(gdb) p/x w
$3 = 0x2057f
(gdb) p WIFSTOPPED(w)
$4 = true
(gdb) p WSTOPSIG(w)
$5 = 5
(gdb) p/x (w >> 8) & (PTRACE_EVENT_VFORK << 8)
$6 = 0x200
However, the statement at line 3626 overrides that:
ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w)));
OURSTATUS becomes "stopped by a SIGTRAP". The information about the
fork or vfork is lost.
It's then all downhill from there, stop_all_threads eventually asks for
a thread list update. That thread list includes the child of that
forgotten fork or vfork, the remote target goes "oh cool, a new process,
let's attach to it!", when in fact that vfork child's destiny was to be
detached.
My reverse-engineered understanding of the code around there is that the
if/else between lines 3562 and 3583 (in the original code) makes sure
OURSTATUS is always initialized (not "ignore"). Either the details are
already in event_child->waitstatus (in the case of fork/vfork, for
example), in which case we just copy event_child->waitstatus to
ourstatus. Or, if the event is a plain "stopped by a signal" or a
syscall event, OURSTATUS is set to "stopped", but without a signal
number. Lines 3601 to 3629 (in the original code) serve to fill in that
last bit of information.
The problem is that when `w` holds the vfork status, the code wrongfully
takes this branch, because WSTOPSIG(w) returns SIGTRAP:
else if (current_thread->last_resume_kind == resume_stop
&& WSTOPSIG (w) != SIGSTOP)
The intent of this branch is, for example, when we sent SIGSTOP to try
to stop a thread, but wait() reports that it stopped with another signal
(that it must have received from somewhere else simultaneously), say
SIGWINCH. In that case, we want to report the SIGWINCH. But in our
fork/vfork case, we don't want to take this branch, as the thread didn't
really stop because it received a signal. For the non "stopped by a
signal" and non "syscall signal" cases, we would ideally skip over all
that snippet that fills in the signal or syscall number.
The fix I propose is to move this snipppet of the else branch of the
if/else above. In addition to moving the code, the last two "else if"
branches:
else if (current_thread->last_resume_kind == resume_stop
&& WSTOPSIG (w) != SIGSTOP)
{
/* A thread that has been requested to stop by GDB with vCont;t,
but, it stopped for other reasons. */
ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w)));
}
else if (ourstatus->kind () == TARGET_WAITKIND_STOPPED)
ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w)));
are changed into a single else:
else
ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w)));
This is the default path we take if:
- W is not a syscall status
- W does not represent a SIGSTOP that have sent to stop the thread and
therefore want to suppress it
Change-Id: If2dc1f0537a549c293f7fa3c53efd00e3e194e79
Simon Marchi [Fri, 14 Jan 2022 20:40:59 +0000 (15:40 -0500)]
gdb: fix handling of vfork by multi-threaded program (follow-fork-mode=parent, detach-on-fork=on)
There is a problem with how GDB handles a vfork happening in a
multi-threaded program. This problem was reported to me by somebody not
using vfork directly, but using system(3) in a multi-threaded program,
which may be implemented using vfork.
This patch only deals about the follow-fork-mode=parent,
detach-on-fork=on case, because it would be too much to chew at once to
fix the bugs in the other cases as well (I tried).
The problem
-----------
When a program vforks, the parent thread is suspended by the kernel
until the child process exits or execs. Specifically, in a
multi-threaded program, only the thread that called vfork is suspended,
other threads keep running freely. This is documented in the vfork(2)
man page ("Caveats" section).
Let's suppose GDB is handling a vfork and the user's desire is to detach
from the child. Before detaching the child, GDB must remove the software
breakpoints inserted in the shared parent/child address space, in case
there's a breakpoint in the path the child is going to take before
exec'ing or exit'ing (unlikely, but possible). Otherwise the child could
hit a breakpoint instruction while running outside the control of GDB,
which would make it crash. GDB must also avoid re-inserting breakpoints
in the parent as long as it didn't receive the "vfork done" event (that
is, when the child has exited or execed): since the address space is
shared with the child, that would re-insert breakpoints in the child
process also. So what GDB does is:
1. Receive "vfork" event for the parent
2. Remove breakpoints from the (shared) address space and set
program_space::breakpoints_not_allowed to avoid re-inserting them
3. Detach from the child thread
4. Resume the parent
5. Wait for and receive "vfork done" event for the parent
6. Clean program_space::breakpoints_not_allowed and re-insert
breakpoints
7. Resume the parent
Resuming the parent at step 4 is necessary in order for the kernel to
report the "vfork done" event. The kernel won't report a ptrace event
for a thread that is ptrace-stopped. But the theory behind this is that
between steps 4 and 5, the parent won't actually do any progress even
though it is ptrace-resumed, because the kernel keeps it suspended,
waiting for the child to exec or exit. So it doesn't matter for that
thread if breakpoints are not inserted.
The problem is when the program is multi-threaded. In step 4, GDB
resumes all threads of the parent. The thread that did the vfork stays
suspended by the kernel, so that's fine. But other threads are running
freely while breakpoints are removed, which is a problem because they
could miss a breakpoint that they should have hit.
The problem is present with all-stop and non-stop targets. The only
difference is that with an all-stop targets, the other threads are
stopped by the target when it reports the vfork event and are resumed by
the target when GDB resumes the parent. With a non-stop target, the
other threads are simply never stopped.
The fix
-------
There many combinations of settings to consider (all-stop/non-stop,
target-non-stop on/off, follow-fork-mode parent/child, detach-on-fork
on/off, schedule-multiple on/off), but for this patch I restrict the
scope to follow-fork-mode=parent, detach-on-fork=on. That's the
"default" case, where we detach the child and keep debugging the
parent. I tried to fix them all, but it's just too much to do at once.
The code paths and behaviors for when we don't detach the child are
completely different.
The guiding principle for this patch is that all threads of the vforking
inferior should be stopped as long as breakpoints are removed. This is
similar to handling in-line step-overs, in a way.
For non-stop targets (the default on Linux native), this is what
happens:
- In follow_fork, we call stop_all_threads to stop all threads of the
inferior
- In follow_fork_inferior, we record the vfork parent thread in
inferior::thread_waiting_for_vfork_done
- Back in handle_inferior_event, we call keep_going, which resumes only
the event thread (this is already the case, with a non-stop target).
This is the thread that will be waiting for vfork-done.
- When we get the vfork-done event, we go in the (new) handle_vfork_done
function to restart the previously stopped threads.
In the same scenario, but with an all-stop target:
- In follow_fork, no need to stop all threads of the inferior, the
target has stopped all threads of all its inferiors before returning
the event.
- In follow_fork_inferior, we record the vfork parent thread in
inferior::thread_waiting_for_vfork_done.
- Back in handle_inferior_event, we also call keep_going. However, we
only want to resume the event thread here, not all inferior threads.
In internal_resume_ptid (called by resume_1), we therefore now check
whether one of the inferiors we are about to resume has
thread_waiting_for_vfork_done set. If so, we only resume that
thread.
Note that when resuming multiple inferiors, one vforking and one not
non-vforking, we could resume the vforking thread from the vforking
inferior plus all threads from the non-vforking inferior. However,
this is not implemented, it would require more work.
- When we get the vfork-done event, the existing call to keep_going
naturally resumes all threads.
Testing-wise, add a test that tries to make the main thread hit a
breakpoint while a secondary thread calls vfork. Without the fix, the
main thread keeps going while breakpoints are removed, resulting in a
missed breakpoint and the program exiting.
Change-Id: I20eb78e17ca91f93c19c2b89a7e12c382ee814a1
Simon Marchi [Thu, 13 Jan 2022 19:32:27 +0000 (14:32 -0500)]
gdb/infrun: add logging statement to do_target_resume
This helped me, it shows which ptid we actually call target_resume with.
Change-Id: I2dfd771e83df8c25f39371a13e3e91dc7882b73d
Simon Marchi [Thu, 13 Jan 2022 19:15:45 +0000 (14:15 -0500)]
gdb/infrun: add inferior parameters to stop_all_threads and restart_threads
A following patch will want to stop all threads of a given inferior (as
opposed to all threads of all inferiors) while handling a vfork, and
restart them after. To help with this, add inferior parameters to
stop_all_threads and restart_threads. This is done as a separate patch
to make sure this doesn't cause regressions on its own, and to keep the
following patches more concise.
No visible changes are expected here, since all calls sites pass
nullptr, which should keep the existing behavior.
Change-Id: I4d9ba886ce842042075b4e346cfa64bbe2580dbf
Simon Marchi [Wed, 12 Jan 2022 02:40:34 +0000 (21:40 -0500)]
gdb: replace inferior::waiting_for_vfork_done with inferior::thread_waiting_for_vfork_done
The inferior::waiting_for_vfork_done flag indicates that some thread in
that inferior is waiting for a vfork-done event. Subsequent patches
will need to know which thread precisely is waiting for that event.
Replace the boolean flag (waiting_for_vfork_done) with a thread_info
pointer (thread_waiting_for_vfork_done).
I think there is a latent buglet in that waiting_for_vfork_done is
currently not reset on inferior exec or exit. I could imagine that if a
thread in the parent process calls exec or exit while another thread of
the parent process is waiting for its vfork child to exec or exit, we
could end up with inferior::waiting_for_vfork_done without a thread
actually waiting for a vfork-done event anymore. And since that flag is
checked in resume_1, things could misbehave there.
Since the new field points to a thread_info object, and those are
destroyed on exec or exit, it could be worse now since we could try to
access freed memory, if thread_waiting_for_vfork_done were to point to a
stale thread_info. To avoid this, clear the field in
infrun_inferior_exit and infrun_inferior_execd.
Change-Id: I31b847278613a49ba03fc4915f74d9ceb228fdce
Simon Marchi [Tue, 5 Apr 2022 00:53:52 +0000 (20:53 -0400)]
gdb: make timestamped_file implement write_async_safe
Trying to use "set debug linux-nat 1", I get an internal error:
/home/smarchi/src/binutils-gdb/gdb/ui-file.h:70: internal-error: write_async_safe: write_async_safe
The problem is that timestamped_file doesn't implement write_async_safe,
which linux-nat's sigchld_handler uses. Implement it.
Change-Id: I830981010c6119f13ae673605ed015cced0f5ee8
GDB Administrator [Tue, 5 Apr 2022 00:00:06 +0000 (00:00 +0000)]
Automatic date update in version.in
Andrew Burgess [Thu, 31 Mar 2022 18:42:17 +0000 (19:42 +0100)]
gdb/testsuite: fix timeout in server-pipe.exp test
I noticed that the gdb.server/server-pipe.exp test would sometimes
timeout when my machine was more heavily loaded. Turns out the test
is reading all the shared libraries over GDB's remote protocol, which
can be slow.
We avoid this in other tests by setting the sysroot in GDBFLAGS,
something which is missing from the gdb.server/server-pipe.exp test.
Fix the timeouts by setting sysroot in GDBFLAGS, after this the shared
libraries are no longer copied over the remote protocol, and I no
longer see the test timeout.
John Baldwin [Mon, 4 Apr 2022 22:08:15 +0000 (15:08 -0700)]
Handle TLS variable lookups when using separate debug files.
Commit
df22c1e5d53c38f38bce6072bb46de240f9e0e2b handled the case that
a separate debug file was passed as the objfile for a shared library
to svr4_fetch_objfile_link_map. However, a separate debug file can
also be passed for TLS variables in the main executable. In addition,
frv_fetch_objfile_link_map also expects to be passed the original
objfile rather than a separate debug file, so pull the code to resolve
a separate debug file to the main objfile up into
target_translate_tls_address.
Lancelot SIX [Fri, 1 Apr 2022 10:59:29 +0000 (11:59 +0100)]
gdb: Add maint set ignore-prologue-end-flag
The previous patch added support for the DWARF prologue-end flag in line
table. This flag can be used by DWARF producers to indicate where to
place breakpoints past a function prologue. However, this takes
precedence over prologue analyzers. So if we have to debug a program
with erroneous debug information, the overall debugging experience will
be degraded.
This commit proposes to add a maintenance command to instruct GDB to
ignore the prologue_end flag.
Tested on x86_64-gnu-linux.
Change-Id: Idda6d1b96ba887f4af555b43d9923261b9cc6f82
Lancelot SIX [Fri, 25 Feb 2022 23:41:47 +0000 (23:41 +0000)]
gdb: Add support for DW_LNS_set_prologue_end in line-table
Add support for DW_LNS_set_prologue_end when building line-tables. This
attribute can be set by the compiler to indicate that an instruction is
an adequate place to set a breakpoint just after the prologue of a
function.
The compiler might set multiple prologue_end, but considering how
current skip_prologue_using_sal works, this commit modifies it to accept
the first instruction with this marker (if any) to be the place where a
breakpoint should be placed to be at the end of the prologue.
The need for this support came from a problematic usecase generated by
hipcc (i.e. clang). The problem is as follows: There's a function
(lets call it foo) which covers PC from 0xa800 to 0xa950. The body of
foo begins with a call to an inlined function, covering from 0xa800 to
0xa94c. The issue is that when placing a breakpoint at 'foo', GDB
inserts the breakpoint at 0xa818. The 0x18 offset is what GDB thinks is
foo's first address past the prologue.
Later, when hitting the breakpoint, GDB reports the stop within the
inlined function because the PC falls in its range while the user
expects to stop in FOO.
Looking at the line-table for this location, we have:
INDEX LINE ADDRESS IS-STMT
[...]
14 293 0x000000000000a66c Y
15 END 0x000000000000a6e0 Y
16 287 0x000000000000a800 Y
17 END 0x000000000000a818 Y
18 287 0x000000000000a824 Y
[...]
For comparison, let's look at llvm-dwarfdump's output for this CU:
Address Line Column File ISA Discriminator Flags
------------------ ------ ------ ------ --- ------------- -------------
[...]
0x000000000000a66c 293 12 2 0 0 is_stmt
0x000000000000a6e0 96 43 82 0 0 is_stmt
0x000000000000a6f8 102 18 82 0 0 is_stmt
0x000000000000a70c 102 24 82 0 0
0x000000000000a710 102 18 82 0 0
0x000000000000a72c 101 16 82 0 0 is_stmt
0x000000000000a73c 2915 50 83 0 0 is_stmt
0x000000000000a74c 110 1 1 0 0 is_stmt
0x000000000000a750 110 1 1 0 0 is_stmt end_sequence
0x000000000000a800 107 0 1 0 0 is_stmt
0x000000000000a800 287 12 2 0 0 is_stmt prologue_end
0x000000000000a818 114 59 81 0 0 is_stmt
0x000000000000a824 287 12 2 0 0 is_stmt
0x000000000000a828 100 58 82 0 0 is_stmt
[...]
The main difference we are interested in here is that llvm-dwarfdump's
output tells us that 0xa800 is an adequate place to place a breakpoint
past a function prologue. Since we know that foo covers from 0xa800 to
0xa94c, 0xa800 is the address at which the breakpoint should be placed
if the user wants to break in foo.
This commit proposes to add support for the prologue_end flag in the
line-program processing.
The processing of this prologue_end flag is made in skip_prologue_sal,
before it calls gdbarch_skip_prologue_noexcept. The intent is that if
the compiler gave information on where the prologue ends, we should use
this information and not try to rely on architecture dependent logic to
guess it.
The testsuite have been executed using this patch on GNU/Linux x86_64.
Testcases have been compiled with both gcc/g++ (verison 9.4.0) and
clang/clang++ (version 10.0.0) since at the time of writing GCC does not
set the prologue_end marker. Tests done with GCC 11.2.0 (not over the
entire testsuite) show that it does not emit this flag either.
No regression have been observed with GCC or Clang. Note that when
using Clang, this patch fixes a failure in
gdb.opt/inline-small-func.exp.
Change-Id: I720449a8a9b2e1fb45b54c6095d3b1e9da9152f8
Lancelot SIX [Mon, 7 Mar 2022 17:36:53 +0000 (17:36 +0000)]
gdb/buildsym: Line record use a record flag
Currently when recording a line entry (with
buildsym_compunit::record_line), a boolean argument argument is used to
indicate that the is_stmt flag should be set for this particular record.
As a later commit will add support for new flags, instead of adding a
parameter to record_line for each possible flag, transform the current
is_stmt parameter into a enum flag. This flags parameter will allow
greater flexibility in future commits.
This enum flags type is not propagated into the linetable_entry type as
this would require a lot of changes across the codebase for no practical
gain (it currently uses a bitfield where each interesting flag only
occupy 1 bit in the structure).
Tested on x86_64-linux, no regression observed.
Change-Id: I5d061fa67bdb34918742505ff983d37453839d6a
Simon Marchi [Thu, 31 Mar 2022 17:32:20 +0000 (13:32 -0400)]
gdb: make timestamped_file implement can_emit_style_escape
In our AMDGPU downstream port, we use styling in some logging output.
We noticed it stopped working after the gdb_printf changes. Making
timestamped_file implement can_emit_style_escape (returning the value of
the stream it wraps) fixes it. To show that it works, modify some
logging statements in auto-load.c to output style filenames. You can
see it in action by setting "set debug auto-load 1" and running a
program. We can incrementally add styling to other debug statements
throughout GDB, as needed.
Change-Id: I78a2fd1e078f80f2263251cf6bc53b3a9de9c17a
Simon Marchi [Thu, 31 Mar 2022 20:38:33 +0000 (16:38 -0400)]
gdb: remove assertion in psymbol_functions::expand_symtabs_matching
psymtab_to_symtab is documented as possibly returning nullptr, if the
primary symtab of the partial symtab has no symbols. However,
psymbol_functions::expand_symtabs_matching asserts that the result of
psymtab_to_symtab as non-nullptr.
I caught this assert by trying the CTF symbol reader on a library I
built with -gctf:
$ ./gdb --data-directory=data-directory /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
...
Reading symbols from /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0...
(gdb) maintenance expand-symtabs
/home/simark/src/binutils-gdb/gdb/psymtab.c:1142: internal-error: expand_symtabs_matching: Assertion `symtab != nullptr' failed.
The "symtab" in question is:
$ readelf --ctf=.ctf /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
...
CTF archive member: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c:
Header:
Magic number: 0xdff2
Version: 4 (CTF_VERSION_3)
Flags: 0xe (CTF_F_NEWFUNCINFO, CTF_F_IDXSORTED, CTF_F_DYNSTR)
Parent name: .ctf
Compilation unit name: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c
Type section: 0x0 -- 0x13 (0x14 bytes)
String section: 0x14 -- 0x5f (0x4c bytes)
Labels:
Data objects:
Function objects:
Variables:
Types:
0x80000001: (kind 5) bt_bool (*) (const bt_value *) (aligned at 0x8)
Strings:
0x0:
0x1: .ctf
0x6: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c
It contains a single type, and it is skipped by ctf_add_type_cb, because
an identical type was already seen earlier in this objfile. As a
result, no compunit_symtab is created.
Change psymbol_functions::expand_symtabs_matching to expect that
psymtab_to_symtab can return nullptr.
Another possibility would be to make the CTF symbol reader always create
a compunit_symtab, even if there are no symbols in it (like the DWARF
parser does), but so far I don't see any advantage in doing so.
Change-Id: Ic43c38202c838a5eb87630ed1fd61d33528164f4
Andrew Burgess [Mon, 4 Apr 2022 21:38:04 +0000 (22:38 +0100)]
sim: fixes for libopcodes styled disassembler
In commit:
commit
60a3da00bd5407f07d64dff82a4dae98230dfaac
Date: Sat Jan 22 11:38:18 2022 +0000
objdump/opcodes: add syntax highlighting to disassembler output
I broke several sim/ targets by forgetting to update their uses of the
libopcodes disassembler to take account of the new styled printing.
These should all be fixed by this commit.
I've not tried to add actual styled output to the simulator traces,
instead, the styled print routines just ignore the style and print the
output unstyled.
Tom Tromey [Thu, 31 Mar 2022 19:41:02 +0000 (13:41 -0600)]
Remove some globals from nat/windows-nat.c
nat/windows-nat.c has a number of globals that it uses to communicate
with its clients (gdb and gdbserver). However, if we ever want the
Windows ports to be multi-inferior, globals won't work.
This patch takes a step toward that by moving most nat/windows-nat.c
globals into a new struct windows_process_info. Many functions are
converted to be methods on this object.
A couple of globals remain, as they are needed to truly be global due
to the way that the Windows debugging APIs work.
The clients still have a global for the current process. That is,
this patch is a step toward the end goal, but doesn't implement the
goal itself.
Tom Tromey [Thu, 31 Mar 2022 16:58:55 +0000 (10:58 -0600)]
Remove windows_thread_info destructor
windows_thread_info declares and defines a destructor, but this
doesn't need to be explicit.
Tom Tromey [Thu, 31 Mar 2022 16:08:45 +0000 (10:08 -0600)]
Use unique_ptr in the Windows thread list
windows-nat.c uses some manual memory management when manipulating the
thread_list global. Changing this to use unique_ptr simplifies the
code, in particular windows_init_thread_list. (Note that, while I
think the the call to init_thread_list in there is wrong, I haven't
removed it in this patch.)
Tom Tromey [Thu, 31 Mar 2022 15:45:23 +0000 (09:45 -0600)]
Use auto_obstack in windows-nat.c
One spot in windows-nat.c can use auto_obstack, removing some manual
memory management.
Tom Tromey [Thu, 31 Mar 2022 15:43:49 +0000 (09:43 -0600)]
Simplify windows-nat.c solib handling
Currently windows-nat.c uses struct so_list to record its local idea
of which shared libraries have been loaded. However, many fields in
this are not needed, and furthermore I found this quite confusing at
first -- Windows actually uses solib-target and so the use of so_list
here is weird.
This patch simplifies this code by changing it to use a std::vector
and a new type that holds exactly what's needed for the Windows code.
Pedro Alves [Fri, 1 Apr 2022 13:31:18 +0000 (14:31 +0100)]
Avoid undefined behavior in gdbscm_make_breakpoint
Running gdb.guile/scm-breakpoint.exp against an --enable-ubsan build,
we see:
UNRESOLVED: gdb.guile/scm-breakpoint.exp: test_watchpoints: create a breakpoint with an invalid type number
...
guile (define wp2 (make-breakpoint "result" #:wp-class WP_WRITE #:type 999))
../../src/gdb/guile/scm-breakpoint.c:377:11: runtime error: load of value 999, which is not a valid value for type 'bptype'
ERROR: GDB process no longer exists
Fix this by parsing the user/guile input as plain int, and cast to
internal type only after we know we have a number that would be valid.
Change-Id: I03578d07db00be01b610a8f5ce72e5521aea6a4b
Tom Tromey [Tue, 22 Feb 2022 20:12:02 +0000 (13:12 -0700)]
Add context-sensitive field name completion to Ada parser
This updates the Ada expression parser to implement context-sensitive
field name completion. This is PR ada/28727.
This is somewhat complicated due to some choices in the Ada lexer --
it chooses to represent a sequence of "."-separated identifiers as a
single token, so the parser must partially recreate the completer's
logic to find the completion word boundaries.
Despite the minor warts in this patch, though, it is a decent
improvement. It's possible that the DWARF reader rewrite will help
fix the package completion problem pointed out in this patch as well.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28727
Tom Tromey [Wed, 23 Feb 2022 15:48:40 +0000 (08:48 -0700)]
Consolidate single-char tokens in ada-lex.l
There are two rules in ada-lex.l that match single-character tokens.
This merges them.
Also, this removes '.' from the list of such tokens. '.' is not used
in any production in ada-exp.y, and removing it here helps the
subsequent completion patches.
Tom Tromey [Tue, 22 Feb 2022 19:56:09 +0000 (12:56 -0700)]
Remove the Ada DOT_ALL token
The Ada parser has a DOT_ALL token to represent ".all", and another
token to represent other ".<identifier>" forms. However, for
completion it is a bit more convenient to unify these cases, so this
patch removes DOT_ALL.
Tom Tromey [Tue, 22 Feb 2022 19:53:14 +0000 (12:53 -0700)]
Refactor ada-lex.l:processId
processId in ada-lex.l is a bit funny -- it uses an "if" and a
"switch", and a nested loop. This patch cleans it up a bit, changing
it to use a boolean flag and a simpler "if".
Tom Tromey [Tue, 22 Feb 2022 18:18:01 +0000 (11:18 -0700)]
Implement completion for Ada attributes
This adds a completer for Ada attributes. Some work in the lexer is
required in order to match end-of-input correctly, as flex does not
have a general-purpose way of doing this. (The approach taken here is
recommended in the flex manual.)
Tom Tromey [Tue, 22 Feb 2022 16:48:25 +0000 (09:48 -0700)]
Refactor expression completion
This refactors the gdb expression completion code to make it easier to
add more types of completers.
In the old approach, just two kinds of completers were supported:
field names for some sub-expression, or tag names (like "enum
something"). The data for each kind was combined in single structure,
"expr_completion_state", and handled explicitly by
complete_expression.
In the new approach, the parser state just holds an object that is
responsible for implementing completion. This way, new completion
types can be added by subclassing this base object.
The structop completer is moved into structop_base_operation, and new
objects are defined for use by the completion code. This moves much
of the logic of expression completion out of completer.c as well.
Tom Tromey [Tue, 22 Feb 2022 18:41:24 +0000 (11:41 -0700)]
Enable "set debug parser" for Ada
I noticed that "set debug parser 1" did not affect Ada parsing. This
patch fixes the problem.
Because this is rarely useful, and pretty much only for maintainers, I
didn't write a test case.
Tom Tromey [Tue, 22 Feb 2022 19:02:10 +0000 (12:02 -0700)]
Fix bug in Ada attributes lexing
The Ada lexer allows whitespace between the apostrophe and the
attribute text, but processAttribute does not handle this. This patch
fixes the problem and introduces a regression test.
Tom Tromey [Tue, 22 Feb 2022 18:05:41 +0000 (11:05 -0700)]
Remove null sentinel from 'attributes'
In a subsequent patch, it's handy if the 'attributes' array in
ada-lex.l does not have a NULL sentinel at the end. In C++, this is
easy to avoid.
Tom Tromey [Tue, 15 Mar 2022 15:03:14 +0000 (09:03 -0600)]
Handle ghost entities in symbol lookup
Normally, SPARK ghost entities are removed from the executable.
However, with -gnata, they will be preserved. In this situation, it's
handy to be able to inspect them. This patch allows this by removing
the "___ghost_" prefix in the appropriate places.
Simon Marchi [Mon, 28 Mar 2022 22:17:17 +0000 (18:17 -0400)]
gdb: rename start_symtab/end_symtab to start_compunit_symtab/end_compunit_symtab
It's a bit confusing because we have both "compunit_symtab" and "symtab"
types, and many methods and functions containing "start_symtab" or
"end_symtab", which actually deal with compunit_symtabs. I believe this
comes from the time before compunit_symtab was introduced, where
symtab did the job of both.
Rename everything I found containing start_symtab or end_symtab to use
start_compunit_symtab or end_compunit_symtab.
Change-Id: If3849b156f6433640173085ad479b6a0b085ade2
Simon Marchi [Mon, 28 Mar 2022 22:17:16 +0000 (18:17 -0400)]
gdb: remove some unused buildsym-legacy functions
Pretty much self-explanatory.
Change-Id: I5b658d017cd891ecdd1df61075eacb0f44316935
Fangrui Song [Mon, 4 Apr 2022 15:43:50 +0000 (08:43 -0700)]
gas: copy st_size only if unset
For
```
.size foo1, 1
foo1:
.set bar1, foo1
.size bar1, 2
.size bar2, 2
.set bar2, foo1
.set bar3, foo2
.size bar3, 2
.size bar4, 2
.set bar4, foo2
.size foo2, 1
foo2:
```
bar1's size is 2 while bar2, bar3, bar4's is 1. The behavior of bar1 makes sense
(generally directives on the new symbol should win) and is relied upon by glibc
stdio-common/errlist.c:
```
.hidden _sys_errlist_internal
.globl _sys_errlist_internal
.type _sys_errlist_internal, @object
.size _sys_errlist_internal, 1072
_sys_errlist_internal:
.globl __GLIBC_2_1_sys_errlist
.set __GLIBC_2_1_sys_errlist, _sys_errlist_internal
.type __GLIBC_2_1_sys_errlist, %object
.size __GLIBC_2_1_sys_errlist, 125 * (64 / 8)
// glibc expects that .size __GLIBC_2_1_sys_errlist, 125 * (64 / 8) wins.
```
The behavior of bar2/bar3/bar4 seems brittle. To avoid the reordering of the two
code blocks which will result in the bar3 situation, glibc compiles errlist.c
with gcc -fno-toplevel-reorder (previously -fno-unit-at-a-time).
To fix the inconsistency and improve robustness, make bar2/bar3/bar4 match bar1,
removing the directive order sensitivity.
There is a pity that `.size dest, 0` is indistinguishable from the case where
dest is unset, but the compromise seems fine.
PR gas/29012
* config/obj-elf.c (elf_copy_symbol_attributes): don't copy if src's size
has been set.
* testsuite/gas/elf/elf.exp: New test.
* testsuite/gas/elf/size.d: New file.
* testsuite/gas/elf/size.s: Likewise.
Andrew Burgess [Mon, 4 Apr 2022 12:41:49 +0000 (13:41 +0100)]
gdb: remove use of vfprintf_filtered
Commit:
commit
60a3da00bd5407f07d64dff82a4dae98230dfaac
Date: Sat Jan 22 11:38:18 2022 +0000
objdump/opcodes: add syntax highlighting to disassembler output
Introduced a new use of vfprintf_filtered, which has been deprecated.
This commit replaces this with gdb_vprintf instead.
Andrew Burgess [Sat, 5 Feb 2022 11:25:14 +0000 (11:25 +0000)]
opcodes/i386: partially implement disassembler style support
This commit adds partial support for disassembler styling in the i386
disassembler.
The i386 disassembler collects the instruction arguments into an array
of strings, and then loops over the array printing the arguments out
later on. The problem is that by the time we print the arguments out
it's not obvious what the type of each argument is.
Obviously this can be fixed, but I'd like to not do that as part of
this commit, rather, I'd prefer to keep this commit as small as
possible to get the basic infrastructure in place, then we can improve
on this, to add additional styling, in later commits.
For now then, I think this commit should correctly style mnemonics,
some immediates, and comments. Everything else will be printed as
plain text, which will include most instruction arguments, unless the
argument is printed as a symbol, by calling the print_address_func
callback.
Ignoring colours, there should be no other user visible changes in the
output of the disassembler in either objdump or gdb.
opcodes/ChangeLog:
* disassembler.c (disassemble_init_for_target): Set
created_styled_output for i386 based targets.
* i386-dis.c: Changed throughout to use fprintf_styled_func
instead of fprintf_func.
Andrew Burgess [Sat, 5 Feb 2022 11:25:24 +0000 (11:25 +0000)]
opcodes/riscv: implement style support in the disassembler
Update the RISC-V disassembler to supply style information. This
allows objdump to apply syntax highlighting to the disassembler
output (when the appropriate command line flag is used).
Ignoring colours, there should be no other user visible changes in the
output of the disassembler in either objdump or gdb.
opcodes/ChangeLog:
* disassembler.c (disassemble_init_for_target): Set
created_styled_output for riscv.
* riscv-dis.c: Changed throughout to use fprintf_styled_func
instead of fprintf_func.
Andrew Burgess [Sat, 22 Jan 2022 11:38:18 +0000 (11:38 +0000)]
objdump/opcodes: add syntax highlighting to disassembler output
This commit adds the _option_ of having disassembler output syntax
highlighted in objdump. This option is _off_ by default. The new
command line options are:
--disassembler-color=off # The default.
--disassembler-color=color
--disassembler-color=extended-color
I have implemented two colour modes, using the same option names as we
use of --visualize-jumps, a basic 8-color mode ("color"), and an
extended 8bit color mode ("extended-color").
The syntax highlighting requires that each targets disassembler be
updated; each time the disassembler produces some output we now pass
through an additional parameter indicating what style should be
applied to the text.
As updating all target disassemblers is a large task, the old API is
maintained. And so, a user of the disassembler (i.e. objdump, gdb)
must provide two functions, the current non-styled print function, and
a new, styled print function.
I don't currently have a plan for converting every single target
disassembler, my hope is that interested folk will update the
disassemblers they are interested in. But it is possible some might
never get updated.
In this initial series I intend to convert the RISC-V disassembler
completely, and also do a partial conversion of the x86 disassembler.
Hopefully having the x86 disassembler at least partial converted will
allow more people to try this out easily and provide feedback.
In this commit I have focused on objdump. The changes to GDB at this
point are the bare minimum required to get things compiling, GDB makes
no use of the styling information to provide any colors, that will
come later, if this commit is accepted.
This first commit in the series doesn't convert any target
disassemblers at all (the next two commits will update some targets),
so after this commit, the only color you will see in the disassembler
output, is that produced from objdump itself, e.g. from
objdump_print_addr_with_sym, where we print an address and a symbol
name, these are now printed with styling information, and so will have
colors applied (if the option is on).
Finally, my ability to pick "good" colors is ... well, terrible. I'm
in no way committed to the colors I've picked here, so I encourage
people to suggest new colors, or wait for this commit to land, and
then patch the choice of colors.
I do have an idea about using possibly an environment variable to
allow the objdump colors to be customised, but I haven't done anything
like that in this commit, the color choices are just fixed in the code
for now.
binutils/ChangeLog:
* NEWS: Mention new feature.
* doc/binutils.texi (objdump): Describe --disassembler-color
option.
* objdump.c (disassembler_color): New global.
(disassembler_extended_color): Likewise.
(disassembler_in_comment): Likewise.
(usage): Mention --disassembler-color option.
(long_options): Add --disassembler-color option.
(objdump_print_value): Use fprintf_styled_func instead of
fprintf_func.
(objdump_print_symname): Likewise.
(objdump_print_addr_with_sym): Likewise.
(objdump_color_for_disassembler_style): New function.
(objdump_styled_sprintf): New function.
(fprintf_styled): New function.
(disassemble_jumps): Use disassemble_set_printf, and reset
disassembler_in_comment.
(null_styled_print): New function.
(disassemble_bytes): Use disassemble_set_printf, and reset
disassembler_in_comment.
(disassemble_data): Update init_disassemble_info call.
(main): Handle --disassembler-color option.
include/ChangeLog:
* dis-asm.h (enum disassembler_style): New enum.
(struct disassemble_info): Add fprintf_styled_func field, and
created_styled_output field.
(disassemble_set_printf): Declare.
(init_disassemble_info): Add additional parameter.
(INIT_DISASSEMBLE_INFO): Add additional parameter.
opcodes/ChangeLog:
* dis-init.c (init_disassemble_info): Take extra parameter,
initialize the new fprintf_styled_func and created_styled_output
fields.
* disassembler.c (disassemble_set_printf): New function definition.
Tom Tromey [Fri, 1 Apr 2022 20:29:35 +0000 (14:29 -0600)]
Remove more Python 2 code
I found another more place that still had a workaround for Python 2.
This patch removes it.
Tom de Vries [Mon, 4 Apr 2022 08:56:51 +0000 (10:56 +0200)]
[gdb/testsuite] Fix KPASS in gdb.ada/arrayptr.exp
On openSUSE Leap 15.3 I run into:
...
KPASS: gdb.ada/arrayptr.exp: scenario=minimal: print pa_ptr.all \
(PRMS minimal encodings)
KPASS: gdb.ada/arrayptr.exp: scenario=minimal: print pa_ptr(3) \
(PRMS minimal encodings)
KPASS: gdb.ada/arrayptr.exp: scenario=minimal: print pa_ptr.all(3) \
(PRMS minimal encodings)
...
The test-case KFAILs some tests. However, the analysis in the corresponding
PR talks of a compiler problem, so it should use XFAILs instead.
The KFAILs are setup for pre-gcc-12, but apparantly the fix has been
backported to system compiler 7.5.0, hence the KPASS.
Fix this by:
- using an XFAIL instead of a KFAIL
- matching the specific gdb output that corresponds to the XFAILs
(reproduced on Fedora 34).
Tested on x86_64-linux, specifically openSUSE Leap 15.3 and Fedora 34.
GDB Administrator [Mon, 4 Apr 2022 00:00:06 +0000 (00:00 +0000)]
Automatic date update in version.in
rupothar [Thu, 17 Mar 2022 18:56:23 +0000 (18:56 +0000)]
gdb: add support for Fortran's ASSUMED RANK arrays
This patch adds a new dynamic property DYN_PROP_RANK, this property is
read from the DW_AT_rank attribute and stored within the type just
like other dynamic properties.
As arrays with dynamic ranks make use of a single
DW_TAG_generic_subrange to represent all ranks of the array, support
for this tag has been added to dwarf2/read.c.
The final piece of this puzzle is to add support in gdbtypes.c so that
we can resolve an array type with dynamic rank. To do this the
existing resolve_dynamic_array_or_string function is split into two,
there's a new resolve_dynamic_array_or_string_1 core that is
responsible for resolving each rank of the array, while the now outer
resolve_dynamic_array_or_string is responsible for figuring out the
array rank (which might require resolving a dynamic property) and then
calling the inner core.
The resolve_dynamic_range function now takes a rank, which is passed
on to the dwarf expression evaluator. This rank will only be used in
the case where the array itself has dynamic rank, but we now pass the
rank in all cases, this should be harmless if the rank is not needed.
The only small nit is that resolve_dynamic_type_internal actually
handles resolving dynamic ranges itself, which now obviously requires
us to pass a rank value. But what rank value to use? In the end I
just passed '1' through here as a sane default, my thinking is that if
we are in resolve_dynamic_type_internal to resolve a range, then the
range isn't part of an array with dynamic rank, and so the range
should actually be using the rank value at all.
An alternative approach would be to make the rank value a
gdb::optional, however, this ends up adding a bunch of complexity to
the code (e.g. having to conditionally build the array to pass to
dwarf2_evaluate_property, and handling the 'rank - 1' in
resolve_dynamic_array_or_string_1) so I haven't done that, but could,
if people think that would be a better approach.
Finally, support for assumed rank arrays was only fixed very recently
in gcc, so you'll need the latest gcc in order to run the tests for
this.
Here's an example test program:
PROGRAM arank
REAL :: a1(10)
CALL sub1(a1)
CONTAINS
SUBROUTINE sub1(a)
REAL :: a(..)
PRINT *, RANK(a)
END SUBROUTINE sub1
END PROGRAM arank
Compiler Version:
gcc (GCC) 12.0.0
20211122 (experimental)
Compilation command:
gfortran assumedrank.f90 -gdwarf-5 -o assumedrank
Without Patch:
gdb -q assumedrank
Reading symbols from assumedrank...
(gdb) break sub1
Breakpoint 1 at 0x4006ff: file assumedrank.f90, line 10.
(gdb) run
Starting program: /home/rupesh/STAGING-BUILD-2787/bin/assumedrank
Breakpoint 1, arank::sub1 (a=<unknown type in /home/rupesh/STAGING-BUILD-2787
/bin/assumedrank, CU 0x0, DIE 0xd5>) at assumedrank.f90:10
10 PRINT *, RANK(a)
(gdb) print RANK(a)
'a' has unknown type; cast it to its declared type
With patch:
gdb -q assumedrank
Reading symbols from assumedrank...
(gdb) break sub1
Breakpoint 1 at 0x4006ff: file assumedrank.f90, line 10.
(gdb) run
Starting program: /home/rupesh/STAGING-BUILD-2787/bin/assumedrank
Breakpoint 1, arank::sub1 (a=...) at assumedrank.f90:10
10 PRINT *, RANK(a)
(gdb) print RANK(a)
$1 = 1
(gdb) ptype a
type = real(kind=4) (10)
(gdb)
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Andrew Burgess [Wed, 16 Mar 2022 17:32:25 +0000 (17:32 +0000)]
gdb/dwarf: pass an array of values to the dwarf evaluator
When we need to evaluate a DWARF expression in order to resolve some
dynamic property of a type we call the dwarf2_evaluate_property
function, which is declared in gdb/dwarf/loc.h and defined in
gdb/dwarf/loc.c.
Currently, this function takes (amongst other things) an argument of
type property_addr_info called addr_stack and a boolean called
push_initial_value. When push_initial_value then the top value of
addr_stack is pushed onto the dwarf expression evaluation stack before
the expression is evaluated.
So far this has worked fine, as the only two cases we needed to handle
are the case the DWARF expression doesn't require the object
address (what the top of addr_stack represents), and the case where
the DWARF expression does require the address.
In the next commit this is going to change. As we add support for
Fortran assumed rank arrays, we need to start resolving the dynamic
properties of arrays. To do this, we need to push the array rank onto
the dwarf expression evaluation stack before the expression is
evaluated.
This commit is a refactoring commit aimed at making it easier to
support Fortran assumed rank arrays. Instead of passing a boolean,
and using this to decide if we should push the object address or not,
we instead pass an array (view) of values that should be pushed to the
dwarf expression evaluation stack.
In the couple of places where we previously passed push_initial_value
as true (mostly this was defaulting to false), we now have to pass the
address from the addr_stack as an item in the array view.
In the next commit, when we want to handle passing the array rank,
this will easily be supported too.
There should be no user visible changes after this commit.
Andrew Burgess [Wed, 16 Mar 2022 15:32:50 +0000 (15:32 +0000)]
gdb: small simplification in dwarf2_locexpr_baton_eval
While examining the dwarf expression evaluator, I noticed that in
dwarf2_locexpr_baton_eval, whenever push_initial_value is true, the
addr_stack will never be nullptr.
This allows for a small cleanup, replacing an if/then/else with an
assertion.
There should be no user visible changes after this commit.
Andrew Burgess [Sun, 3 Apr 2022 15:48:09 +0000 (16:48 +0100)]
gdb/testsuite: resolve some duplicate test names in gdb.base
This commit resolves all the duplicate test names that I see in the
script:
gdb.base/attach-pie-misread.exp
The duplicate names all come from a second call to
build_executable_own_libs, so in this commit I've places the second
call inside a with_test_prefix block.
While I was making this change I've also modified the value being
passed as the testname for the second build_executable_own_libs call.
Previously we used ${test}, however, I think this was likely a
mistake, the 'test' variable is setup for the previous test. I
suspect that ${testfile} is a better choice - especially now we have a
testname prefix.
As the testname is only used (after various calls) from within
build_executable_from_specs should the build fail, I don't think this
change really makes much difference though.
There should be no change in what is tested after this commit.
Andrew Burgess [Sun, 3 Apr 2022 15:36:48 +0000 (16:36 +0100)]
gdb/testsuite: resolve a duplicate test name in a gdb.mi test
Solve two duplicate test names in the test script:
gdb.mi/mi-catch-cpp-exceptions.exp
by moving the call to restart_for_test inside the with_test_prefix
block. There should be no difference in what is tested after this
commit.
Andrew Burgess [Wed, 23 Mar 2022 10:29:49 +0000 (10:29 +0000)]
gdb/Makefile.in: move ALLDEPFILES earlier in Makefile.in
If I do 'make tags' in the gdb build directory, the tags target does
complete, but I see these warnings:
../../src/gdb/arm.c: No such file or directory
../../src/gdb/arm-get-next-pcs.c: No such file or directory
../../src/gdb/arm-linux.c: No such file or directory
The reason for this is the ordering of build rules and make variables
in gdb/Makefile.in, specifically, the placement of the tags related
rules, and the ALLDEPFILES variable. The ordering is like this:
TAGFILES_NO_SRCDIR = .... $(ALLDEPFILES) ....
TAGS: $(TAGFILES_NO_SRCDIR) ....
# Recipe uses $(TAGFILES_NO_SRCDIR)
tags: TAGS
ALLDEPFILES = .....
When the TAGS rule is parsed TAGFILES_NO_SRCDIR is expanded, which
then expands ALLDEPFILES, which, at that point in the Makefile is
undefined, and so expands to the empty string. As a result TAGS does
not depend on any file listed in ALLDEPFILES.
However, when the TAGS recipe is invoked ALLDEPFILES is now defined.
As a result, all the files in ALLDEPFILES are passed to the etags
program.
The ALLDEPFILES references three files, arm.c, arm-get-next-pcs.c, and
arm-linux.c, which are actually in the gdb/arch/ directory, but, in
ALLDEPFILES these files don't include the arch/ prefix. As a result,
the etags program ends up looking for these files in the wrong
location.
As ALLDEPFILES is only used by the TAGS rule, this mistake was not
previously noticed (the TAGS rule itself was broken until a recent
commit).
In this commit I make two changes, first, I move ALLDEPFILES to be
defined before TAGFILES_NO_SRCDIR, this means that the TAGS rule will
depend on all the files in ALLDEPFILES. With this change the TAGS
rule now breaks complaining that there's no rule to build the 3 files
mentioned above.
Next, I have added all *.c files in gdb/arch/ to ALLDEPFILES,
including their arch/ prefix, and removed the incorrect (missing arch/
prefix) references.
With these two changes the TAGS (or tags if you prefer) target now
builds without any errors or warnings.
Reuben Thomas [Tue, 22 Mar 2022 22:10:17 +0000 (22:10 +0000)]
gdb/Makefile.in: fix 'make tags' build target
The gdb_select.h file was moved to the gdbsupport directory long ago,
but a reference was accident left in gdb/Makefile.in (in the
HFILES_NO_SRCDIR variable), this commit removes that reference.
Before this commit, if I use 'make tags' here's what I see:
$ make tags
make: *** No rule to make target 'gdb_select.h', needed by 'TAGS'. Stop.
After this commit 'make tags' completes, but I still see these
warnings:
../../src/gdb/arm.c: No such file or directory
../../src/gdb/arm-get-next-pcs.c: No such file or directory
../../src/gdb/arm-linux.c: No such file or directory
These are caused by a separate issue, and will be addressed in the
next commit.
Andrew Burgess [Wed, 23 Mar 2022 10:02:49 +0000 (10:02 +0000)]
gdb/Makefile.in: remove SOURCES variable
The SOURCES variable was added to gdb/Makefile.in as part of commit:
commit
fb40c20903110ed8af9701ce7c2635abd3770d52
Date: Wed Feb 23 00:25:43 2000 +0000
Add mi/ and testsuite/gdb.mi/ subdirectories.
But as far as I can tell was not used at the time it was added, and is
not used today.
Lets remove it.
Andrew Burgess [Tue, 1 Feb 2022 14:59:11 +0000 (14:59 +0000)]
gdb/tui: fair split of delta after a resize
Currently, in master gdb, when a tui window is changed in size, the
screen delta is mostly just added to the next available window. We
do take care to respect the min/max size, but in most cases, these
limits are just "the terminal size", and so, we end up placing the
whole delta on the next window.
Consider these steps in an 80 column, 24 line terminal:
(gdb) tui enable
(gdb) layout src
(gdb) layout split
(gdb) info win
Name Lines Columns Focus
src 8 80 (has focus)
asm 8 80
status 1 80
cmd 8 80
(gdb) winheight cmd +2
(gdb) info win
Name Lines Columns Focus
src 6 80 (has focus)
asm 8 80
status 1 80
cmd 10 80
Notice that initially, the windows were balanced, 8 lines each for the
major windows. Then, when the cmd window was adjusted, the extra two
lines were given to the asm window.
I think it would be nicer if the delta was spread more evenly over the
available windows. In the example above, after the adjustment the
layout now looks like:
(gdb) info win
Name Lines Columns Focus
src 7 80 (has focus)
asm 7 80
status 1 80
cmd 10 80
This is achieved within tui_layout_split::set_size, by just handing
out the delta in increments of 1 to each window (except for the window
the user adjusted), until there's no more delta left. Of course, we
continue to respect the min/max window sizes.
Andrew Burgess [Tue, 1 Feb 2022 14:09:26 +0000 (14:09 +0000)]
gdb/tui: relax restrictions on window max height and width
This commit removes some arbitrary adjustments made in
tui_cmd_window::max_height, tui_win_info::max_height, and
tui_win_info::max_width.
These member functions all subtract some constant from the theoretical
maximum height or width. I've looked back through the history a
little and can see no real reason why these adjustments should be
needed, with these adjustments removed all the existing tui tests
still pass.
However, retaining these restrictions causes some bugs, consider:
(gdb) tui new-layout hsrc {-horizontal src 1 cmd 1} 1
When this layout is selected with current master, gdb will leave a 4
line gap at the bottom of the terminal.
The problem is that the maximum height is restricted, for the cmd
window, to 4 less than the terminal height.
By removing this restriction gdb is able to size the windows to the
complete terminal height, and the layout is done correctly.
This 4 line restriction is also what prevents this layout from working
correctly:
(gdb) tui new-layout conly cmd 1
Previously, this layout would present a cmd window only, but there
would be a 4 line gap at the bottom of the terminal. This issue was
mentioned in an earlier commit in this series (when a different bug
was fixed), but with this commit, the above layout now correctly fills
the terminal. The associated test is updated.
After removing the adjustment in tui_cmd_window::max_height, the
implementation is now the same as the implementation in the parent
class tui_win_info, so I've completely removed the max_height call
from tui_cmd_window.
Andrew Burgess [Tue, 1 Feb 2022 14:24:15 +0000 (14:24 +0000)]
gdb/testsuite: some additional tests in gdb.tui/scroll.exp
This commit just adds an extra check of the src window size prior to
sending all the commands to gdb. We also set the cmd window height to
its existing height, this (obviously) shouldn't change the window
layout, which we check.
My main motivation was adding the initial window layout check, the
winheight and recheck are just extras. All of these test pass both
before and after this commit.
Andrew Burgess [Tue, 1 Feb 2022 14:02:19 +0000 (14:02 +0000)]
gdb/tui: support placing the cmd window into a horizontal layout
This commit allows the user to place the cmd window within horizontal
tui layouts. Consider this set of steps, carried out in an 80 columns
by 24 lines terminal, using current master gdb:
(gdb) tui new-layout hsrc { -horizontal src 1 cmd 1 } 1 status 1
(gdb) tui layout hsrc
What you end up with is a full width cmd window with the status bar
beneath. Where's the src window gone? We then try:
(gdb) info win
Name Lines Columns Focus
src 23 3 (has focus)
cmd 23 80
status 1 80
(gdb)
Something weird has gone on, gdb has overlapped the cmd window with
the src window. If we trigger the src window to redraw is content,
for example, 'list main', then we see corruption in the cmd window as
the src window overwrites it.
So, what's going on?
The problem is some code in tui_layout_split::apply, in tui-layout.c.
Within 'Step 1', there is a loop that calculates the min/max window
sizes for all windows within a tui_layout_split. However, there's a
special case for the cmd window.
This special case is trying to have the cmd window retain its current
size when a layout is re-applied, or a new layout is applied. This
makes sense, consider moving from the 'src' layout to the 'asm'
layout, this looks something like this (status window removed):
.-------. .-------.
| src | | asm |
|-------| ====> |-------|
| cmd | | cmd |
'-------' '-------'
If the user has gone to the effort of adjusting the cmd window size,
then, the thinking goes, we shouldn't reset the cmd window size when
switching layouts like this.
The problem though, is that when we do a switch more like this:
.-----------. .-----------.
| src | | | |
|-----------| ====> | asm | cmd |
| cmd | | | |
'-----------' '-----------'
Now retaining the cmd window width makes no sense; the new layout has
a completely different placement for the cmd window, instead of sizing
by height, we're now sizing by width. The existing code doesn't
understand this though, and tried to retain the full width for the cmd
window.
To solve this problem, I propose we introduce the idea of a layout
"fingerprint". The fingerprint tries to capture, in an abstract way,
where the cmd window lives within the layout.
Only when two layouts have the same fingerprint will we attempt to
retain the cmd window size.
The fingerprint for a layout is represented as a string, the string is
a series of 'V' or 'H' characters, ending with a single 'C'
character. The series of 'V' and 'H' characters represent the
vertical or horizontal layouts that must be passed through to find the
cmd window.
Here are a few examples:
# This layout is equivalent to the builtin 'src' layout.
# Fingerprint: VC
tui new-layout example1 src 2 status 0 cmd 1
# This layout is equivalent to the builtin 'split' layout.
# Fingerprint: VC
tui new-layout example2 src 1 asm 1 status 0 cmd 1
# This is the same layout that was given at the top.
# Fingerprint: VHC
tui new-layout hsrc { -horizontal src 1 cmd 1 } 1 status 1
And so, when switching between example1 and example2, gdb knows that
the cmd window is, basically, in the same sort of position within the
layout, and will retain the cmd window size.
In contrast, when switching to the hsrc layout, gdb understands that
the position of the cmd window is different, and does not try to
retain the cmd window size.
Andrew Burgess [Tue, 1 Feb 2022 12:29:57 +0000 (12:29 +0000)]
gdb/tui: allow cmd window to change size in tui_layout_split::apply
When we switch layouts we call the tui_layout_split::apply member
function to reapply the layout, and recalculate all the window sizes.
One special case is the cmd window, which we try to keep at its
existing size.
However, in some cases it is not appropriate to keep the cmd window at
its existing size. I will describe two such cases here, in one, we
want the cmd window to reduce in size, and in the other, we want the
cmd window to grow in size.
Try these steps in a 80 columns, by 24 lines terminal:
(gdb) tui enable
(gdb) layout src
(gdb) winheight cmd 20
(gdb) layout split
You should see that the status window is missing from the new layout,
and that the cmd window has been placed over the border of the asm
window. The 'info win' output is:
(gdb) info win
Name Lines Columns Focus
src 3 80 (has focus)
asm 3 80
status 1 80
cmd 20 80
Notice that gdb has assigned 27 lines of screen space, even with the
border overlap between the src and asm windows, this is still 2 lines
too many.
The problem here is that after switching layouts, gdb has forced the
cmd window to retain its 20 line height. Really, we want the cmd
window to reduce in height so that the src and asm windows can occupy
their minimum required space.
This commit allows this (details on how are below). After this
commit, in the above situation, we now see the status window displayed
correctly, and the 'info win' output is:
(gdb) info win
Name Lines Columns Focus
src 3 80 (has focus)
asm 3 80
status 1 80
cmd 18 80
The cmd window has been reduced in size by 2 lines so that everything
can fit on the screen.
The second example is one which was discussed in a recent commit,
consider this case (still in the 80 column, 24 line terminal):
(gdb) tui enable
(gdb) tui new-layout conly cmd 1
(gdb) layout conly
(gdb) info win
Name Lines Columns Focus
cmd 8 80 (has focus)
(gdb)
This layout only contains a cmd window, which we would expect to
occupy the entire terminal. But instead, the cmd window only occupies
the first 8 lines, and the rest of the terminal is unused!
The reason is, again, that the cmd window is keeping its previous
size (8 lines).
After this commit things are slightly different, the 'info win' output
is now:
(gdb) info win
Name Lines Columns Focus
cmd 20 80 (has focus)
Which is a little better, but why only 20 lines? Turns out there's
yet another bug hitting this case. That bug will be addressed in a
later commit, so, for now, we're accepting the 20 lines.
What this commit does is modify the phase of tui_layout_split::apply
that handles any left over space. Usually, in "Step 2", each
sub-layout has a size calculated. As the size is an integer, then,
when all sizes are calculated we may have some space left over.
This extra space is then distributed between all the windows fairly
until all the space is used up.
When we consider windows minimum size, or fixed size windows, then it
is possible that we might try to use more space than is available,
this was our first example above. The same code that added extra
space to the windows, can also be used to reclaim space (in the over
allocation case) to allow all windows to fit.
The problem then is the cmd window, which we often force to a fixed
size. Inside the loop that handles the allocation of excess space, if
we find that we have tried every window, and still have space either
left to give, or we need to claim back more space, then, if the cmd
window was changed to a fixed size, we can change the cmd window back
to a non-fixed-size window, and proceed to either give, or take space
from the cmd window as needed.
Andrew Burgess [Tue, 1 Feb 2022 10:28:18 +0000 (10:28 +0000)]
gdb/tui: fairer distribution of excess space during apply
When applying layouts gdb computes the size of each window (or rather,
each sub-layout within a layout) using integer arithmetic. As this
rounds down the results, then, when all sub-layouts are sized, there
is the possibility that we have some space left over.
Currently, this space is just assigned to an arbitrary sub-layout.
This can result in some unbalanced results. Consider this set of
steps with current master:
(gdb) tui enable
(gdb) layout regs
(gdb) info win
Name Lines Columns Focus
regs 7 80
src 9 80 (has focus)
status 1 80
cmd 8 80
Notice the weird split between the src and regs windows, the original
layout specification has these windows given equal weight. The
problem is that, with rounding, both the regs and src windows are
initially sized to 7, the extra 2 lines are then arbitrarily added to
the src window.
In this commit, rather than add all the extra space to one single
window, I instead hand out the extra space 1 line at a time, looping
over all the sub-layouts. We take care to respect the min/max sizes,
and so, we now get this result:
(gdb) tui enable
(gdb) layout regs
(gdb) info win
Name Lines Columns Focus
regs 8 80
src 8 80 (has focus)
status 1 80
cmd 8 80
This looks more natural to me.
This is obviously a change in behaviour, and so, lots of the existing
tests need to be updated to take this into account. None of the
changes are huge, it's just a line or two (or column for width) moved
between windows.
Andrew Burgess [Tue, 1 Feb 2022 16:53:56 +0000 (16:53 +0000)]
gdb/tui: avoid fp exception when applying layouts
Consider:
(gdb) tui enable
(gdb) layout src
(gdb) tui new-layout conly cmd 1
(gdb) layout conly
After this, with current master, gdb crashes with a floating-point
exception.
The problem is that in tui_layout_split::apply, when we switch from
'src' to 'conly', we will try to retain the cmd window height. As
such, the cmd window will become a fixed size window, which decreases
the available_size, but doesn't count towards the total_weight.
As the cmd window is the only window, the total_weight stays at zero,
and, when we move into step 2, where we attempt to size the windows,
we perform a divide by zero, and crash.
After this commit we avoid the divide by zero, and just directly set
the window size based on the fixed size.
There is still a problem after this commit, when the conly layout is
selected the cmd window retains its original height, which will only
be part of the terminal. The rest of the terminal is left unused.
This issue will be addressed in a later commit, this commit is just
about the floating-point exception.
Andrew Burgess [Tue, 1 Feb 2022 13:36:58 +0000 (13:36 +0000)]
gdb/tui/testsuite: refactor new-layout.exp test
This commit changes the gdb.tui/new-layout.exp test to make use of a
list of test descriptions, and a loop to check each description in
turn. There's no change to what is actually tested after this commit.
In future commits I plan to add additional tests to this file, and
this will be easier now that all I have to do is add a new test
description to the list.
Andrew Burgess [Mon, 31 Jan 2022 12:13:38 +0000 (12:13 +0000)]
gdb/tui: add left_boxed_p and right_boxed_p member functions
When I initially saw this code in tui_layout_split::apply, I assumed
that this must be a bug:
/* Two adjacent boxed windows will share a border, making a bit
more size available. */
if (i > 0
&& m_splits[i - 1].layout->bottom_boxed_p ()
&& m_splits[i].layout->top_boxed_p ())
...
After all, the apply might be laying out a horizontal layout, right?
So checking bottom_boxed_p and top_boxed_p is clearly wrong.
Well, it turns on, that due to the implementations of these things,
bottom_boxed_p is equivalent to an imagined right_boxed_p, and
top_boxed_p is equivalent to an imagined left_boxed_p.
In this commit I've renamed both top_boxed_p and bottom_boxed_p to
first_edge_has_border_p and last_edge_has_border_p respectively, and
extended the comments in tui_layout_base to mention that these methods
handle both horizontal and vertical layouts.
Now, hopefully, the code shouldn't look like it only applies for
vertical layouts.
There should be no user visible changes after this commit.
Andrew Burgess [Wed, 26 Jan 2022 18:52:56 +0000 (18:52 +0000)]
gdb/tui: add a tui debugging flag
This commit adds 'set debug tui on|off' and 'show debug tui'. This
commit adds the control variable, and the printing macros in
tui/tui.h. I've then added some uses of these in tui.c and
tui-layout.c.
To help produce more useful debug output in tui-layout.c, I've added
some helper member functions in the class tui_layout_split, and also
moved the size_info struct out of tui_layout_split::apply into the
tui_layout_split class.
If tui debug is not turned on, then there should be no user visible
changes after this commit.
One thing to note is that, due to the way that the tui terminal is
often cleared, the only way I've found this useful is when I do:
(gdb) tui enable
(gdb) set logging file /path/to/file
(gdb) set logging debugredirect on
(gdb) set logging enable on
Additionally, gdb has some quirks when it comes to setting up logging
redirect and switching interpreters. Thus, the above only really
works if the logging is enabled after the tui is enabled, and disabled
again before the tui is disabled.
Enabling logging and switching interpreters can cause undefined
results, including crashes. This is an existing bug in gdb[1], and
has nothing directly to do with tui debug, but it is worth mentioning
here I think.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=28948
Andrew Burgess [Mon, 24 Jan 2022 22:02:59 +0000 (22:02 +0000)]
gdb/tui: add new 'tui window width' command and 'winwidth' alias
This commit adds a new command 'tui window width', and an alias
'winwidth'. This command is equivalent to the old 'winheight'
command (which was recently renamed 'tui window height').
Even though I recently moved the old tui commands under the tui
namespace, and I would strongly encourage all new tui commands to be
added as 'tui ....' only (users can create their own top-level aliases
if they want), I'm breaking that suggestion here, and adding a
'winwidth' alias.
Given that we already have 'winheight' and have done for years, it
just didn't seem right to no have the matching 'winwidth'.
You might notice in the test that the window resizing doesn't quite
work right. I setup a horizontal layout, then grow and shrink the
windows. At the end of the test the windows should be back to their
original size...
... they are not. This isn't my fault, honest! GDB's window resizing
is a little ... temperamental, and is prone to getting things slightly
wrong during resizes, off by 1 type things. This is true for height
resizing, as well as the new width resizing.
Later patches in this series will rework the resizing algorithm, which
should improve things in this area. For now, I'm happy that the width
resizing is as good as the height resizing, given the existing quirks.
For the docs side I include a paragraph that explains how multiple
windows are required before the width can be adjusted. For
completeness, I've added the same paragraph to the winheight
description. With the predefined layouts this extra paragraph is not
really needed for winheight, as there are always multiple windows on
the screen. However, with custom layouts, this might not be true, so
adding the paragraph seems like a good idea.
As for the changes in gdb itself, I've mostly just taken the existing
height adjustment code, changed the name to make it generic 'size'
adjustment, and added a boolean flag to indicate if we are adjusting
the width or the height.
Andrew Burgess [Fri, 28 Jan 2022 12:00:31 +0000 (12:00 +0000)]
gdb/tui: rename tui_layout_split:set_weights_from_heights
In a following commit I'm going to add the ability to change the width
of a tui window (when in a horizontal layout). As a result, some of
the places where we currently hard-code references to height need to
be changed to handle either height, or width, based on whether we are
in a vertical, or horizontal layout.
This commit renames set_weights_from_heights to
set_weights_from_sizes, and makes the function use either the height,
or width as appropriate.
Currently, the only place that we call this function is from the
tui_layout_split::set_height function, in a part of the code we will
only reach for vertical layouts, so the new code is not actually being
used, but, this small change will help make later patches smaller, so
I'm proposing this as a stand alone change.
There should be no user visible changes after this commit.
Andrew Burgess [Mon, 24 Jan 2022 17:49:12 +0000 (17:49 +0000)]
gdb/tui: rename tui_layout_base::adjust_size to ::set_height
Rename tui_layout_base::adjust_size to tui_layout_base::set_height,
the new name more accurately reflects what this member function does,
and makes it easier for a later commit to add a new
tui_layout_base::set_width member function.
There should be no user visible changes after this commit.
Andrew Burgess [Mon, 24 Jan 2022 17:23:20 +0000 (17:23 +0000)]
gdb: move some commands into the tui namespace
There are a lot of tui related commands that live in the top-level
command name space, e.g. layout, focus, refresh, winheight.
Having them at the top level means less typing for the user, which is
good, but, I think, makes command discovery harder.
In this commit, I propose moving all of the above mentioned commands
into the tui namespace, so 'layout' becomes 'tui layout', etc. But I
will then add aliases so that the old commands will still work,
e.g. I'll make 'layout' an alias for 'tui layout'.
The benefit I see in this work is that tui related commands can be
more easily discovered by typing 'tui ' and then tab-completing. Also
the "official" command is now a tui-sub-command, this is visible in,
for example, the help output, e.g.:
(gdb) help layout
tui layout, layout
Change the layout of windows.
Usage: tui layout prev | next | LAYOUT-NAME
List of tui layout subcommands:
tui layout asm -- Apply the "asm" layout.
tui layout next -- Apply the next TUI layout.
tui layout prev -- Apply the previous TUI layout.
tui layout regs -- Apply the TUI register layout.
tui layout split -- Apply the "split" layout.
tui layout src -- Apply the "src" layout.
Which I think is a good thing, it makes it clearer that this is a tui
command.
I've added a NEWS entry and updated the docs to mention the new and
old command names, with the new name being mentioned first.
Simon Marchi [Sun, 3 Apr 2022 13:54:41 +0000 (09:54 -0400)]
gdb: fix gdb_print -> gdb_printf typo
This caused a build failure with !CXX_STD_THREAD.
Change-Id: I30f0c89c43a76f85c0db34809192644fa64a9d18
Alan Modra [Sun, 3 Apr 2022 12:04:53 +0000 (21:34 +0930)]
Move microblaze relax info to target specific data
Target specific data shouldn't be put in struct bfd_section.
* section.c (struct bfd_section): Delete relax and relax_count.
(BFD_FAKE_SECTION): Adjust to suit.
(struct relax_table): Move to..
* elf32-microblaze.c (struct relax_table): ..here.
(struct _microblaze_elf_section_data): New.
(microblaze_elf_section_data): Define.
(microblaze_elf_new_section_hook): New function.
(bfd_elf32_new_section_hook): Define.
(calc_fixup): Return a size_t. Adjust to suit new location of
relax and relax_count.
(microblaze_elf_relax_section): Adjust to suit new location of
relax and relax_count. Make some variables size_t.
* bfd-in2.h: Regenerate.
Alan Modra [Thu, 24 Mar 2022 05:12:52 +0000 (15:42 +1030)]
Revert commit
240d6706c6a2
PR 28592
PR 15994
PR 15935
* dwarf2.c (lookup_address_in_line_info_table): Return bool rather
than a range.
(comp_unit_find_nearest_line): Likewise. Return true if function
info found without line info.
(_bfd_dwarf2_find_nearest_line): Revert range handling code.
Alan Modra [Sun, 3 Apr 2022 11:23:44 +0000 (20:53 +0930)]
Regen bfd po/SRC-POTFILES.in
GDB Administrator [Sun, 3 Apr 2022 00:00:07 +0000 (00:00 +0000)]
Automatic date update in version.in
Tiezhu Yang [Mon, 21 Mar 2022 06:43:41 +0000 (14:43 +0800)]
gdb: rename floatformats_ia64_quad to floatformats_ieee_quad
It is better to rename floatformats_ia64_quad to floatformats_ieee_quad
to reflect the reality, and then we can clean up the related code.
As Tom Tromey said [1]:
These files are maintained in gcc and then imported into the
binutils-gdb repository, so any changes to them will have to
be proposed there first.
the related changes have been merged into gcc master now [2], it is time
to do it for gdb.
[1] https://sourceware.org/pipermail/gdb-patches/2022-March/186569.html
[2] https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=
b2dff6b2d9d6
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
GDB Administrator [Sat, 2 Apr 2022 00:00:06 +0000 (00:00 +0000)]
Automatic date update in version.in
John Baldwin [Fri, 1 Apr 2022 22:21:09 +0000 (15:21 -0700)]
Remove unused variable.
Aaron Merey [Fri, 1 Apr 2022 20:39:18 +0000 (16:39 -0400)]
gdb/debuginfod-support.c: Always display debuginfod errors
Errors encountered when downloading files from debuginfod servers
are not displayed if debuginfod verbosity is set to 0 (via
'set debuginfod verbose 0').
Tom recommended that these errors always be displayed, regardless
of the verbosity setting [1]. Fix this.
[1] https://sourceware.org/pipermail/gdb-patches/2022-March/186350.html
John Baldwin [Fri, 1 Apr 2022 20:16:46 +0000 (13:16 -0700)]
Use I386_GSBASE_REGNUM in i386fbsd_get_thread_local_address.
32-bit x86 arches always the I386_*BASE_REGNUM values. Only code that
needs to support both 64-bit and 32-bit arches needs to use
tdep->fsbase_regnum to compute a segment base register number.
John Baldwin [Fri, 1 Apr 2022 20:16:46 +0000 (13:16 -0700)]
FreeBSD/x86: Read segment base registers from NT_X86_SEGBASES.
FreeBSD kernels recently grew a new register core dump note containing
the base addresses of the %fs and %gs segments (corresponding to the
%fsbase and %gsbase registers). Parse this note to permit inspecting
TLS variables in core dumps. Native processes already supported TLS
via older ptrace() operations.
John Baldwin [Fri, 1 Apr 2022 20:16:46 +0000 (13:16 -0700)]
Use pseudosections for NT_FREEBSD_X86_SEGBASES core dump notes.
This includes adding pseudosections when reading a core dump as well
as support for writing out a core dump note from a pseudosection.
bfd/ChangeLog:
* elf-bfd.h (elfcore_write_x86_segbases): New.
* elf.c (elfcore_grok_freebsd_note): Add pseudosections for
NT_FREEBSD_X86_SEGBASES register notes.
(elfcore_write_x86_segbases): New.
(elfcore_write_register_note): Write NT_FREEBSD_X86_SEGBASES
register notes.
John Baldwin [Fri, 1 Apr 2022 20:16:46 +0000 (13:16 -0700)]
Recognize FreeBSD core dump note for x86 segment base registers.
This core dump note contains the value of the base address of the %fs
and %gs segments for both i386 and amd64 core dumps. It is primarily
useful in resolving the address of TLS variables in core dumps.
binutils/ChangeLog:
* readelf.c (get_freebsd_elfcore_note_type): Handle
NT_FREEBSD_X86_SEGBASES.
include/ChangeLog:
* elf/common.h (NT_FREEBSD_X86_SEGBASES): Define.
John Baldwin [Fri, 1 Apr 2022 20:16:46 +0000 (13:16 -0700)]
elfcore_grok_freebsd_note: Remove checks of note->namesz.
This function is only called if the note name is "FreeBSD", so
checking the name size is unnecessary.
bfd/ChangeLog:
* elf.c (elfcore_grok_freebsd_note): Remove checks for namesz.
Andrew Burgess [Wed, 26 Jan 2022 18:41:59 +0000 (18:41 +0000)]
gdb/testing/tui: add new _csi_{L,S,T}
This patch was original part of this series:
https://sourceware.org/pipermail/gdb-patches/2022-March/186429.html
https://sourceware.org/pipermail/gdb-patches/2022-March/186433.html
I've pulled this out as it might be useful ahead of the bigger series
being merged.
This commit adds:
_csi_L - insert line
_csi_S - pan down
_csi_T - pan up
H.J. Lu [Thu, 24 Mar 2022 19:06:04 +0000 (12:06 -0700)]
x86: Remove bfd_arch_l1om and bfd_arch_k1om
Remove bfd_arch_l1om and bfd_arch_k1om since L1OM/K1OM support has been
removed from gas, ld and opcodes.
bfd/
* Makefile.am (ALL_MACHINES): Remove cpu-l1om.lo and cpu-k1om.lo.
(ALL_MACHINES_CFILES): Remove cpu-l1om.c and cpu-k1om.c.
* archures.c (bfd_mach_l1om): Removed.
(bfd_mach_l1om_intel_syntax): Likewise.
(bfd_mach_k1om): Likewise.
(bfd_mach_k1om_intel_syntax): Likewise.
(bfd_k1om_arch): Likewise.
(bfd_l1om_arch): Likewise.
(bfd_archures_list): Remove bfd_k1om_arch and bfd_l1om_arch
references.
* config.bfd (targ_selvecs): Remove l1om_elf64_vec.
l1om_elf64_fbsd_vec, k1om_elf64_vec and k1om_elf64_fbsd_vec.
(targ_archs): Remove bfd_l1om_arch and bfd_k1om_arch.
* configure.ac (k1om_elf64_vec): Removed.
(k1om_elf64_fbsd_vec): Likewise.
(l1om_elf64_vec): Likewise.
(l1om_elf64_fbsd_vec): Likewise.
* cpu-k1om.c: Removed.
* cpu-l1om.c: Likewise.
* elf64-x86-64.c (elf64_l1om_elf_object_p): Removed.
(elf64_k1om_elf_object_p): Likewise.
(l1om_elf64_vec): Removed.
(l1om_elf64_fbsd_vec): Likewise.
(k1om_elf64_vec): Likewise.
(k1om_elf64_fbsd_vec): Likewise.
(ELF_TARGET_OS): Undefine.
* targets.c (_bfd_target_vector): Remove k1om_elf64_vec,
k1om_elf64_fbsd_vec, l1om_elf64_vec and l1om_elf64_fbsd_vec.
* Makefile.in: Regenerate.
* bfd-in2.h: Likewise.
* configure: Likewise.
opcodes/
* configure.ac: Remove bfd_arch_l1om/bfd_arch_k1om references.
* disassemble.c (disassembler): Likewise.
* configure: Regenerate.
Simon Marchi [Thu, 31 Mar 2022 21:20:19 +0000 (17:20 -0400)]
gdb/ctf: pass partial symtab's filename to buildsym_compunit
I noticed that the CTF symbol reader passes the objfile's name to all
buildsym_compunit instances it creates. The result is that all
compunit_symtabs created have the same name, that of the objfile:
{ objfile /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct objfile *) 0x613000005d00)
{ ((struct compunit_symtab *) 0x621000286760)
debugformat ctf
producer (null)
name libbabeltrace2.so.0.0.0
dirname (null)
blockvector ((struct blockvector *) 0x6210003911d0)
user ((struct compunit_symtab *) (null))
{ symtab /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct symtab *) 0x6210003911f0)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
{ ((struct compunit_symtab *) 0x621000275c10)
debugformat ctf
producer (null)
name libbabeltrace2.so.0.0.0
dirname (null)
blockvector ((struct blockvector *) 0x621000286710)
user ((struct compunit_symtab *) (null))
{ symtab /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct symtab *) 0x621000286730)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
Notice the two "name libbabeltrace2.so.0.0.0".
Change it to pass the partial_symtab's filename instead. The output
becomes:
{ objfile /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct objfile *) 0x613000005d00)
{ ((struct compunit_symtab *) 0x621000295610)
debugformat ctf
producer (null)
name libbabeltrace2.so.0.0.0
dirname (null)
blockvector ((struct blockvector *) 0x6210003a15d0)
user ((struct compunit_symtab *) (null))
{ symtab /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct symtab *) 0x6210003a15f0)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
{ ((struct compunit_symtab *) 0x621000288700)
debugformat ctf
producer (null)
name current-thread.c
dirname (null)
blockvector ((struct blockvector *) 0x6210002955c0)
user ((struct compunit_symtab *) (null))
{ symtab /home/simark/src/babeltrace/src/lib/current-thread.c ((struct symtab *) 0x6210002955e0)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
Note that the first compunit_symtab still has libbabeltrace2.so.0.0.0 as
its name. This is because the CTF symbol reader really creates a
partial symtab named like this. It appears to be because the debug info
contains information that has been factored out of all CUs and is at the
"top-level" of the objfile, outside any real CU. So it creates a
partial symtab and an artificial CU that's named after the objfile.
Change-Id: I576316bab2a3668adf87b4e6cebda900a8159b1b
Simon Marchi [Thu, 31 Mar 2022 21:20:18 +0000 (17:20 -0400)]
gdb: print compunit_symtab name in "maint info symtabs"
I think it would make sense to print a compunit_symtab's name in "maint
info symtabs". If you are looking for a given CU in the list, that's
probably the field you will be looking at. As the doc of
compunit_symtab::name says, it is not meant to be a reliable file name,
it is for debugging purposes (and "maint info symtabs" exists for
debugging purposes).
Sample output with the new field:
(gdb) maintenance info symtabs
{ objfile /home/simark/build/binutils-gdb-one-target/gdb/a.out ((struct objfile *) 0x613000005d00)
{ ((struct compunit_symtab *) 0x621000131630)
debugformat DWARF 5
producer GNU C17 11.2.0 -mtune=generic -march=x86-64 -g3 -O0
name test.c
dirname /home/simark/build/binutils-gdb-one-target/gdb
blockvector ((struct blockvector *) 0x621000131d10)
user ((struct compunit_symtab *) (null))
{ symtab test.c ((struct symtab *) 0x6210001316b0)
fullname (null)
linetable ((struct linetable *) 0x621000131d40)
}
{ symtab /home/simark/build/binutils-gdb-one-target/gdb/test.h ((struct symtab *) 0x6210001316e0)
fullname (null)
linetable ((struct linetable *) 0x0)
}
{ symtab /usr/include/stdc-predef.h ((struct symtab *) 0x621000131710)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
{ ((struct compunit_symtab *) 0x6210001170a0)
debugformat DWARF 5
producer GNU C17 11.2.0 -mtune=generic -march=x86-64 -g3 -O0
name foo.c
dirname /home/simark/build/binutils-gdb-one-target/gdb
blockvector ((struct blockvector *) 0x621000131580)
user ((struct compunit_symtab *) (null))
{ symtab foo.c ((struct symtab *) 0x621000117120)
fullname (null)
linetable ((struct linetable *) 0x6210001315b0)
}
{ symtab /usr/include/stdc-predef.h ((struct symtab *) 0x621000117150)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
}
Change-Id: I17b87adfac2f6551cb5bda30d59f6c6882789211
Simon Marchi [Thu, 31 Mar 2022 21:20:17 +0000 (17:20 -0400)]
gdb/ctf: don't create a buildsym_compunit when building partial symbols
I am trying to do some changes to buildsym_compunit, so I am auditing
the current uses. Something seems odd with this use of
buildsym_compunit (that this patch removes).
A buildsym_compunit is normally used when building a compunit_symtab.
That is, when expanding a partial symtab into a full compunit symtab.
In ctfread.c, a buildsym_compunit is created in ctf_start_archive, which
is only used when creating partial symtabs. At this moment, I don't
see how that's useful. ctf_start_archive creates a new
buildsym_compunit and starts a subfile. But that buildsym_compunit is
never used again. It's just overriden in ctf_start_symtab, which means
we leak the old buildsym_compunit, I suppose.
Remove ctf_start_archive completely. Add an assert in
ctf_start_symtab to verify that we are not overwriting an existing
buildsym_compunit (meaning we'd leak the existing one). This assert
triggers without the other part of the fix. When doing:
$ ./gdb --data-directory=data-directory /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
...
(gdb) maintenance expand-symtabs
/home/simark/src/binutils-gdb/gdb/ctfread.c:1255: internal-error: ctf_start_symtab: Assertion `!ccp->builder' failed.
Change-Id: I666d146454a019f08e7305f3a1c4a974d27b4592
Tom Tromey [Thu, 10 Mar 2022 00:26:37 +0000 (17:26 -0700)]
Style URLs in GDB output
I noticed that GDB will display URLs in a few spots. This changes
them to be styled. Originally I thought I'd introduce a new "url"
style, but there aren't many places to use this, so I just reused
filename styling instead. This patch also changes the debuginfod URL
list to be printed one URL per line. I think this is probably a bit
easier to read.
GDB Administrator [Fri, 1 Apr 2022 00:00:12 +0000 (00:00 +0000)]
Automatic date update in version.in
Simon Marchi [Thu, 31 Mar 2022 18:20:02 +0000 (14:20 -0400)]
gdb: initialize ctf_context::builder in create_partial_symtab
I built a random project with -gctf, in order to test the CTF support in
GDB. With my ASan/UBSan/etc-enabled build of GDB, I get:
$ ./gdb --data-directory=data-directory /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
...
Reading symbols from /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0...
/home/simark/src/binutils-gdb/gdb/ctfread.c:1545:31: runtime error: member call on misaligned address 0xbebebebebebebebe for type 'struct buildsym_compunit', which requires 8 byte alignment
0xbebebebebebebebe: note: pointer points here
The 0xbebebebebebebebe value is a sign that the ctf_context::builder
field is uninitialized. The problem probably goes under the radar if
the field happens to be zero-initialized, because ctf_start_archive
contains this code:
if (ccx->builder == nullptr)
{
ccx->builder = new buildsym_compunit (of,
of->original_name, nullptr, language_c, 0);
If the field was zero-initialized (by chance), this will create a new
buildsym_compunit. But if the field was purposely filled with random
bytes by one of the sanitizers, we won't create a buildsym_compunit here
and we'll continue with ccx->builder equal to 0xbebebebebebebebe.
Fix this the easy way by initializing ccx->builder where the other
ctf_context fields are initialized (yeah, this code could be made nicer
C++, but I am going for the obvious fix here).
With this patch, this passes cleanly on my system:
$ make check TESTS="gdb.ctf/*.exp" RUNTESTFLAGS="CC_FOR_TARGET=/opt/gcc/git/bin/gcc"
# of expected passes 40
... where /opt/gcc/git/bin/gcc is a gcc with CTF support, given my
system gcc does not have it.
Change-Id: Idea1b0cf3e3708b72ecb16b1b60222439160f9b9
Tom Tromey [Fri, 4 Mar 2022 18:29:27 +0000 (11:29 -0700)]
Remove dbx mode
This patch removes gdb's dbx mode. Regression tested on x86-64 Fedora
34.