aboutsummaryrefslogtreecommitdiff
path: root/gdb
AgeCommit message (Collapse)AuthorFilesLines
2022-04-05Introduce wrapped_fileTom Tromey2-35/+59
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.
2022-04-05Don't call init_thread_list in windows-nat.cTom Tromey1-1/+0
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.
2022-04-05gdb/testsuite: fix intermittent failure in gdb.base/vfork-follow-parent.expSimon Marchi2-1/+15
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
2022-04-05gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.expSimon Marchi2-5/+78
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
2022-04-05Fix qRcmd error code parsingLuis Machado2-2/+3
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.
2022-04-04gdb: resume ongoing step after handling fork or vforkSimon Marchi3-4/+227
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
2022-04-04gdb/remote: remove_new_fork_children don't access ↵Simon Marchi1-2/+3
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
2022-04-04gdb: fix handling of vfork by multi-threaded program ↵Simon Marchi3-8/+311
(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
2022-04-04gdb/infrun: add logging statement to do_target_resumeSimon Marchi1-0/+4
This helped me, it shows which ptid we actually call target_resume with. Change-Id: I2dfd771e83df8c25f39371a13e3e91dc7882b73d
2022-04-04gdb/infrun: add inferior parameters to stop_all_threads and restart_threadsSimon Marchi2-10/+36
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
2022-04-04gdb: replace inferior::waiting_for_vfork_done with ↵Simon Marchi2-9/+13
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
2022-04-04gdb: make timestamped_file implement write_async_safeSimon Marchi1-0/+3
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
2022-04-04gdb/testsuite: fix timeout in server-pipe.exp testAndrew Burgess1-3/+7
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.
2022-04-04Handle TLS variable lookups when using separate debug files.John Baldwin2-5/+5
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.
2022-04-04gdb: Add maint set ignore-prologue-end-flagLancelot SIX4-1/+54
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
2022-04-04gdb: Add support for DW_LNS_set_prologue_end in line-tableLancelot SIX13-12/+249
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
2022-04-04gdb/buildsym: Line record use a record flagLancelot SIX4-19/+36
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
2022-04-04gdb: make timestamped_file implement can_emit_style_escapeSimon Marchi2-4/+8
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
2022-04-04gdb: remove assertion in psymbol_functions::expand_symtabs_matchingSimon Marchi1-6/+3
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
2022-04-04Remove some globals from nat/windows-nat.cTom Tromey3-299/+330
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.
2022-04-04Remove windows_thread_info destructorTom Tromey2-6/+0
windows_thread_info declares and defines a destructor, but this doesn't need to be explicit.
2022-04-04Use unique_ptr in the Windows thread listTom Tromey1-16/+9
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.)
2022-04-04Use auto_obstack in windows-nat.cTom Tromey1-3/+1
One spot in windows-nat.c can use auto_obstack, removing some manual memory management.
2022-04-04Simplify windows-nat.c solib handlingTom Tromey1-66/+49
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.
2022-04-04Avoid undefined behavior in gdbscm_make_breakpointPedro Alves1-5/+5
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
2022-04-04Add context-sensitive field name completion to Ada parserTom Tromey6-21/+202
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
2022-04-04Consolidate single-char tokens in ada-lex.lTom Tromey1-3/+1
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.
2022-04-04Remove the Ada DOT_ALL tokenTom Tromey2-11/+9
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.
2022-04-04Refactor ada-lex.l:processIdTom Tromey1-18/+13
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".
2022-04-04Implement completion for Ada attributesTom Tromey4-12/+103
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.)
2022-04-04Refactor expression completionTom Tromey6-148/+196
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.
2022-04-04Enable "set debug parser" for AdaTom Tromey1-0/+3
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.
2022-04-04Fix bug in Ada attributes lexingTom Tromey2-8/+15
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.
2022-04-04Remove null sentinel from 'attributes'Tom Tromey1-12/+10
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.
2022-04-04Handle ghost entities in symbol lookupTom Tromey5-0/+108
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.
2022-04-04gdb: rename start_symtab/end_symtab to start_compunit_symtab/end_compunit_symtabSimon Marchi13-121/+131
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
2022-04-04gdb: remove some unused buildsym-legacy functionsSimon Marchi2-125/+0
Pretty much self-explanatory. Change-Id: I5b658d017cd891ecdd1df61075eacb0f44316935
2022-04-04gdb: remove use of vfprintf_filteredAndrew Burgess1-1/+1
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.
2022-04-04objdump/opcodes: add syntax highlighting to disassembler outputAndrew Burgess2-2/+39
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.
2022-04-04Remove more Python 2 codeTom Tromey1-10/+1
I found another more place that still had a workaround for Python 2. This patch removes it.
2022-04-04[gdb/testsuite] Fix KPASS in gdb.ada/arrayptr.expTom de Vries1-16/+45
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.
2022-04-03gdb: add support for Fortran's ASSUMED RANK arraysrupothar5-15/+263
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>
2022-04-03gdb/dwarf: pass an array of values to the dwarf evaluatorAndrew Burgess4-17/+16
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.
2022-04-03gdb: small simplification in dwarf2_locexpr_baton_evalAndrew Burgess1-9/+9
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.
2022-04-03gdb/testsuite: resolve some duplicate test names in gdb.baseAndrew Burgess1-4/+6
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.
2022-04-03gdb/testsuite: resolve a duplicate test name in a gdb.mi testAndrew Burgess1-1/+1
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.
2022-04-03gdb/Makefile.in: move ALLDEPFILES earlier in Makefile.inAndrew Burgess1-218/+229
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.
2022-04-03gdb/Makefile.in: fix 'make tags' build targetReuben Thomas1-1/+0
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.
2022-04-03gdb/Makefile.in: remove SOURCES variableAndrew Burgess1-1/+0
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.
2022-04-03gdb/tui: fair split of delta after a resizeAndrew Burgess2-12/+56
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.