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.
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.
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.
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
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
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.
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
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
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
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
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
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
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
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.
Commit df22c1e5d5 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.
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
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
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
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
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
In commit:
commit 60a3da00bd
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.
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.
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.)
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.
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
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
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.
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.
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".
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.)
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.
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.
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.
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.
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
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.
Commit:
commit 60a3da00bd
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.
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.
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.
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.
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.
