aboutsummaryrefslogtreecommitdiff
path: root/gdb/remote.c
AgeCommit message (Collapse)AuthorFilesLines
2024-01-12Update copyright year range in header of all files managed by GDBAndrew Burgess1-1/+1
This commit is the result of the following actions: - Running gdb/copyright.py to update all of the copyright headers to include 2024, - Manually updating a few files the copyright.py script told me to update, these files had copyright headers embedded within the file, - Regenerating gdbsupport/Makefile.in to refresh it's copyright date, - Using grep to find other files that still mentioned 2023. If these files were updated last year from 2022 to 2023 then I've updated them this year to 2024. I'm sure I've probably missed some dates. Feel free to fix them up as you spot them.
2023-12-20Fix bug in previous remote unique_ptr changePedro Alves1-2/+3
By inspection, I noticed that the previous patch went too far, here: @@ -7705,7 +7713,8 @@ remote_target::discard_pending_stop_replies (struct inferior *inf) if (rs->remote_desc == NULL) return; - reply = (struct stop_reply *) rns->pending_event[notif_client_stop.id]; + stop_reply_up reply + = as_stop_reply_up (std::move (rns->pending_event[notif_client_stop.id])); /* Discard the in-flight notification. */ if (reply != NULL && reply->ptid.pid () == inf->pid) That is always moving the stop reply from pending_event, when we only really want to peek into it. The code further below that even says: /* Discard the in-flight notification. */ if (reply != NULL && reply->ptid.pid () == inf->pid) { /* Leave the notification pending, since the server expects that we acknowledge it with vStopped. But clear its contents, so that later on when we acknowledge it, we also discard it. */ This commit reverts that hunk back, adjusted to use unique_ptr::get(). Change-Id: Ifc809d1a8225150a4656889f056d51267100ee24
2023-12-20Complete use of unique_ptr with notif_event and stop_replyPedro Alves1-40/+44
We already use unique_ptr with notif_event and stop_reply in some places around the remote target, but not fully. There are several code paths that still use raw pointers. This commit makes all of the ownership of these objects tracked by unique pointers, making the lifetime flow much more obvious, IMHO. I notice that it fixes a leak -- in remote_notif_stop_ack, We weren't destroying the stop_reply object if it was of TARGET_WAITKIND_IGNORE kind. Approved-By: Tom Tromey <tom@tromey.com> Change-Id: Id81daf39653d8792c8795b2a145772176bfae77c
2023-12-20Make cached_reg_t own its dataPedro Alves1-18/+6
struct cached_reg_t owns its data buffer, but currently that is managed manually. Convert it to use a unique_xmalloc_ptr. Approved-By: Tom Tromey <tom@tromey.com> Change-Id: I05a107098b717299e76de76aaba00d7fbaeac77b
2023-12-04Remove incorrect "fall-through" commentTom Tromey1-1/+0
I found a "fall-through" comment in gdb/remote.c that was incorrect -- the code here cannot in fact fall through.
2023-11-29Use C++17 [[fallthrough]] attributeTom Tromey1-3/+3
This changes gdb to use the C++17 [[fallthrough]] attribute rather than special comments. This was mostly done by script, but I neglected a few spellings and so also fixed it up by hand. I suspect this fixes the bug mentioned below, by switching to a standard approach that, presumably, clang supports. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23159 Approved-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Luis Machado <luis.machado@arm.com> Approved-By: Pedro Alves <pedro@palves.net>
2023-11-27Change serial_readchar to throwTom Tromey1-35/+21
This changes serial_readchar to throw an exception rather than trying to set and preserve errno. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30770
2023-11-27Change serial_send_break and serial_write to throwTom Tromey1-8/+32
This changes serial_send_break and serial_write to throw exceptions rather than attempt to set errno and return an error indicator. This lets us correctly report failures on Windows. Both functions had to be converted in a single patch because one implementation of send_break works via write. This also introduces remote_serial_send_break to handle error checking when attempting to send a break. This was previously ignored. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30770
2023-11-27Change serial "open" functions to throw exceptionTom Tromey1-2/+0
remote.c assumes that a failure to open the serial connection will set errno. This is somewhat true, because the Windows code tries to set errno appropriately -- but only somewhat, because it isn't clear that the "pex" code sets it, and the tcp code seems to do the wrong thing. It seems better to simply have the serial open functions throw on error. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30770
2023-11-27Change serial_setbaudrate to throw exceptionTom Tromey1-2/+6
remote.c has this code: if (serial_setbaudrate (rs->remote_desc, baud_rate)) { /* The requested speed could not be set. Error out to top level after closing remote_desc. Take care to set remote_desc to NULL to avoid closing remote_desc more than once. */ serial_close (rs->remote_desc); rs->remote_desc = NULL; perror_with_name (name); The perror here cannot be correct, because if serial_setbaudrate did set errno, it may be obscured by serial_close. This patch changes serial_setbaudrate to throw an exception instead. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30770
2023-11-21gdb: Replace gdb::optional with std::optionalLancelot Six1-8/+8
Since GDB now requires C++17, we don't need the internally maintained gdb::optional implementation. This patch does the following replacing: - gdb::optional -> std::optional - gdb::in_place -> std::in_place - #include "gdbsupport/gdb_optional.h" -> #include <optional> This change has mostly been done automatically. One exception is gdbsupport/thread-pool.* which did not use the gdb:: prefix as it already lives in the gdb namespace. Change-Id: I19a92fa03e89637bab136c72e34fd351524f65e9 Approved-By: Tom Tromey <tom@tromey.com> Approved-By: Pedro Alves <pedro@palves.net>
2023-11-17gdb: remove regcache's address spaceSimon Marchi1-3/+5
While looking at the regcache code, I noticed that the address space (passed to regcache when constructing it, and available through regcache::aspace) wasn't relevant for the regcache itself. Callers of regcache::aspace use that method because it appears to be a convenient way of getting the address space for a thread, if you already have the regcache. But there is always another way to get the address space, as the callers pretty much always know which thread they are dealing with. The regcache code itself doesn't use the address space. This patch removes anything related to address_space from the regcache code, and updates callers to get it from the thread in context. This removes a bit of unnecessary complexity from the regcache code. The current get_thread_arch_regcache function gets an address_space for the given thread using the target_thread_address_space function (which calls the target_ops::thread_address_space method). This suggest that there might have been the intention of supporting per-thread address spaces. But digging through the history, I did not find any such case. Maybe this method was just added because we needed a way to get an address space from a ptid (because constructing a regcache required an address space), and this seemed like the right way to do it, I don't know. The only implementations of thread_address_space and process_stratum_target::thread_address_space and linux_nat_target::thread_address_space, which essentially just return the inferior's address space. And thread_address_space is only used in the current get_thread_arch_regcache, which gets removed. So, I think that the thread_address_space target method can be removed, and we can assume that it's fine to use the inferior's address space everywhere. Callers of regcache::aspace are updated to get the address space from the relevant inferior, either using some context they already know about, or in last resort using the current global context. So, to summarize: - remove everything in regcache related to address spaces - in particular, remove get_thread_arch_regcache, and rename get_thread_arch_aspace_regcache to get_thread_arch_regcache - remove target_ops::thread_address_space, and target_thread_address_space - adjust all users of regcache::aspace to get the address space another way Change-Id: I04fd41b22c83fe486522af7851c75bcfb31c88c7
2023-11-13Introduce GDB_THREAD_OPTION_EXIT thread option, fix step-over-thread-exitPedro Alves1-0/+9
When stepping over a breakpoint with displaced stepping, GDB needs to be informed if the stepped thread exits, otherwise the displaced stepping buffer that was allocated to that thread leaks, and this can result in deadlock, with other threads waiting for their turn to displaced step, but their turn never comes. Similarly, when stepping over a breakpoint in line, GDB also needs to be informed if the stepped thread exits, so that is can clear the step over state and re-resume threads. This commit makes it possible for GDB to ask the target to report thread exit events for a given thread, using the new "thread options" mechanism introduced by a previous patch. This only adds the core bits. Following patches in the series will teach the Linux backends (native & gdbserver) to handle the GDB_THREAD_OPTION_EXIT option, and then a later patch will make use of these thread exit events to clean up displaced stepping and inline stepping state properly. Reviewed-By: Andrew Burgess <aburgess@redhat.com> Change-Id: I96b719fdf7fee94709e98bb3a90751d8134f3a38 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338
2023-11-13all-stop/synchronous RSP support thread-exit eventsPedro Alves1-1/+6
Currently, GDB does not understand the THREAD_EXITED stop reply in remote all-stop mode. There's no good reason for this, it just happened that THREAD_EXITED was only ever reported in non-stop mode so far. This patch teaches GDB to parse that event in all-stop RSP too. There is no need to add a qSupported feature for this, because the server won't send a THREAD_EXITED event unless GDB explicitly asks for it, with QThreadEvents, or with the GDB_THREAD_OPTION_EXIT QThreadOptions option added in the next patch. Change-Id: Ide5d12391adf432779fe4c79526801c4a5630966
2023-11-13Thread options & clone events (core + remote)Pedro Alves1-1/+181
A previous patch taught GDB about a new TARGET_WAITKIND_THREAD_CLONED event kind, and made the Linux target report clone events. A following patch will teach Linux GDBserver to do the same thing. However, for remote debugging, it wouldn't be ideal for GDBserver to report every clone event to GDB, when GDB only cares about such events in some specific situations. Reporting clone events all the time would be potentially chatty. We don't enable thread create/exit events all the time for the same reason. Instead we have the QThreadEvents packet. QThreadEvents is target-wide, though. This patch makes GDB instead explicitly request that the target reports clone events or not, on a per-thread basis. In order to be able to do that with GDBserver, we need a new remote protocol feature. Since a following patch will want to enable thread exit events on per-thread basis too, the packet introduced here is more generic than just for clone events. It lets you enable/disable a set of options at once, modelled on Linux ptrace's PTRACE_SETOPTIONS. IOW, this commit introduces a new QThreadOptions packet, that lets you specify a set of per-thread event options you want to enable. The packet accepts a list of options/thread-id pairs, similarly to vCont, processed left to right, with the options field being a number interpreted as a bit mask of options. The only option defined in this commit is GDB_THREAD_OPTION_CLONE (0x1), which ask the remote target to report clone events. Another patch later in the series will introduce another option. For example, this packet sets option "1" (clone events) on thread p1000.2345: QThreadOptions;1:p1000.2345 and this clears options for all threads of process 1000, and then sets option "1" (clone events) on thread p1000.2345: QThreadOptions;0:p1000.-1;1:p1000.2345 This clears options of all threads of all processes: QThreadOptions;0 The target reports the set of supported options by including "QThreadOptions=<supported options>" in its qSupported response. infrun is then tweaked to enable GDB_THREAD_OPTION_CLONE when stepping over a breakpoint. Unlike PTRACE_SETOPTIONS, fork/vfork/clone children do NOT inherit their parent's thread options. This is so that GDB can send e.g., "QThreadOptions;0;1:TID" without worrying about threads it doesn't know about yet. Documentation for this new remote protocol feature is included in a documentation patch later in the series. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19675 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830 Reviewed-By: Andrew Burgess <aburgess@redhat.com> Change-Id: Ie41e5093b2573f14cf6ac41b0b5804eba75be37e
2023-11-13Avoid duplicate QThreadEvents packetsPedro Alves1-0/+8
Similarly to QProgramSignals and QPassSignals, avoid sending duplicate QThreadEvents packets. Approved-By: Andrew Burgess <aburgess@redhat.com> Change-Id: Iaf5babb0b64e1527ba4db31aac8674d82b17e8b4
2023-11-13Support clone events in the remote protocolPedro Alves1-33/+65
The previous patch taught GDB about a new TARGET_WAITKIND_THREAD_CLONED event kind, and made the Linux target report clone events. A following patch will teach Linux GDBserver to do the same thing. But before we get there, we need to teach the remote protocol about TARGET_WAITKIND_THREAD_CLONED. That's what this patch does. Clone is very similar to vfork and fork, and the new stop reply is likewise handled similarly. The stub reports "T05clone:...". GDBserver core is taught to handle TARGET_WAITKIND_THREAD_CLONED and forward it to GDB in this patch, but no backend actually emits it yet. That will be done in a following patch. Documentation for this new remote protocol feature is included in a documentation patch later in the series. Reviewed-By: Andrew Burgess <aburgess@redhat.com> Change-Id: If271f20320d864f074d8ac0d531cc1a323da847f Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19675 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830
2023-10-26gdb: handle main thread exiting during detachAndrew Burgess1-1/+26
Overview ======== Consider the following situation, GDB is in non-stop mode, the main thread is running while a second thread is stopped. The user has the second thread selected as the current thread and asks GDB to detach. At the exact moment of detach the main thread exits. This situation currently causes crashes, assertion failures, and unexpected errors to be reported from GDB for both native and remote targets. This commit addresses this situation for native and remote targets. There are a number of different fixes, but all are required in order to get this functionality working correct for native and remote targets. Native Linux Target =================== For the native Linux target, detaching is handled in the function linux_nat_target::detach. In here we call stop_wait_callback for each thread, and it is this callback that will spot that the main thread has exited. GDB then detaches from everything except the main thread by calling detach_callback. After this the first problem is this assert: /* Only the initial process should be left right now. */ gdb_assert (num_lwps (pid) == 1); The num_lwps call will return 0 as the main thread has exited and all of the other threads have now been detached. I fix this by changing the assert to allow for 0 or 1 lwps at this point. As the 0 case can only happen in non-stop mode, the assert becomes: gdb_assert (num_lwps (pid) == 1 || (target_is_non_stop_p () && num_lwps (pid) == 0)); The next problem is that we do: main_lwp = find_lwp_pid (ptid_t (pid)); and then proceed assuming that main_lwp is not nullptr. In the case that the main thread has exited though, main_lwp will be nullptr. However, we only need main_lwp so that GDB can detach from the thread. If the main thread has exited, and GDB has already detached from every other thread, then GDB has finished detaching, GDB can skip the calls that try to detach from the main thread, and then tell the user that the detach was a success. For Remote Targets ================== On remote targets there are two problems. First is that when the exit occurs during the early phase of the detach, we see the stop notification arrive while GDB is removing the breakpoints ahead of the detach. The 'set debug remote on' trace looks like this: [remote] Sending packet: $z0,7f1648fe0241,1#35 [remote] Notification received: Stop:W0;process:2a0ac8 # At this point an unpatched gdbserver segfaults, and the connection # is broken. A patched gdbserver continues as below... [remote] Packet received: E01 [remote] Sending packet: $z0,7f1648ff00a8,1#68 [remote] Packet received: E01 [remote] Sending packet: $z0,7f1648ff132f,1#6b [remote] Packet received: E01 [remote] Sending packet: $D;2a0ac8#3e [remote] Packet received: E01 I was originally running into Segmentation Faults, from within gdbserver/mem-break.cc, in the function find_gdb_breakpoint. This function calls current_process() and then dereferences the result to find the breakpoint list. However, in our case, the current process has already exited, and so the current_process() call returns nullptr. At the point of failure, the gdbserver backtrace looks like this: #0 0x00000000004190e4 in find_gdb_breakpoint (z_type=48 '0', addr=4198762, kind=1) at ../../src/gdbserver/mem-break.cc:982 #1 0x000000000041930d in delete_gdb_breakpoint (z_type=48 '0', addr=4198762, kind=1) at ../../src/gdbserver/mem-break.cc:1093 #2 0x000000000042d8db in process_serial_event () at ../../src/gdbserver/server.cc:4372 #3 0x000000000042dcab in handle_serial_event (err=0, client_data=0x0) at ../../src/gdbserver/server.cc:4498 ... The problem is that, as a result non-stop being on, the process exiting is only reported back to GDB after the request to remove a breakpoint has been sent. Clearly gdbserver can't actually remove this breakpoint -- the process has already exited -- so I think the best solution is for gdbserver just to report an error, which is what I've done. The second problem I ran into was on the gdb side, as the process has already exited, but GDB has not yet acknowledged the exit event, the detach -- the 'D' packet in the above trace -- fails. This was being reported to the user with a 'Can't detach process' error. As the test actually calls detach from Python code, this error was then becoming a Python exception. Though clearly the detach has returned an error, and so, maybe, having GDB throw an error would be fine, I think in this case, there's a good argument that the remote error can be ignored -- if GDB tries to detach and gets back an error, and if there's a pending exit event for the pid we tried to detach, then just ignore the error and pretend the detach worked fine. We could possibly check for a pending exit event before sending the detach packet, however, I believe that it might be possible (in non-stop mode) for the stop notification to arrive after the detach is sent, but before gdbserver has started processing the detach. In this case we would still need to check for pending stop events after seeing the detach fail, so I figure there's no point having two checks -- we just send the detach request, and if it fails, check to see if the process has already exited. Testing ======= In order to test this issue I needed to ensure that the exit event arrives at the same time as the detach call. The window of opportunity for getting the exit to arrive is so small I've never managed to trigger this in real use -- I originally spotted this issue while working on another patch, which did manage to trigger this issue. However, if we trigger both the exit and the detach from a single Python function then we never return to GDB's event loop, as such GDB never processes the exit event, and so the first time GDB gets a chance to see the exit is during the detach call. And so that is the approach I've taken for testing this patch. Tested-By: Kevin Buettner <kevinb@redhat.com> Approved-By: Kevin Buettner <kevinb@redhat.com>
2023-10-19gdb: remove target_section_table typedefSimon Marchi1-1/+2
Remove this typedef. I think that hiding the real type (std::vector) behind a typedef just hinders readability. Change-Id: I80949da3392f60a2826c56c268e0ec6f503ad79f Approved-By: Pedro Alves <pedro@palves.net> Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
2023-10-10gdb: add assertion when marking the remote async flagSimon Marchi1-1/+4
As reported in bug 30630 [1], we hit a case where the remote target's async flag is marked while the target is not configured (yet) to work async. This should not happen. It is caught thanks to this assert in remote_target::wait: /* Start by clearing the flag that asks for our wait method to be called, we'll mark it again at the end if needed. If the target is not in async mode then the async token should not be marked. */ if (target_is_async_p ()) rs->clear_async_event_handler (); else gdb_assert (!rs->async_event_handler_marked ()); This is helpful, but I think that we could have caught the problem earlier than that, at the moment we marked the handler. Catching problems earlier makes them easier to debug. [1] https://sourceware.org/bugzilla/show_bug.cgi?id=30630 Change-Id: I7e229c74b04da82bef6a817d5a676be5cf52e833 Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-10-10gdb: add remote_state::{is_async_p,can_async_p}Simon Marchi1-4/+16
A subsequent patch will want to know if the remote is async within a remote_state method. Add a helper method for that, and for "can async" as well, for symmetry. Change-Id: Id0f648ee4896736479fa942f5453eeeb0e5d4352 Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-10-10gdb: make remote_state's async token privateSimon Marchi1-27/+44
Make remote_async_inferior_event_token private (rename to m_async_event_handler_token) and add methods for the various operations we do on it. This will help by: - allowing to break on those methods when debugging - allowing to add assertions in the methods Change-Id: Ia3b8a2bc48ad4849dbbe83442c3f83920f03334d Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-10-10gdb: remove trailing whitespaces in remote.cSimon Marchi1-10/+9
Change-Id: I88d136b6b5a0a54d1c8a9f8a0068762a5456a29a
2023-10-10gdb: remove target_gdbarchSimon Marchi1-32/+37
This function is just a wrapper around the current inferior's gdbarch. I find that having that wrapper just obscures where the arch is coming from, and that it's often used as "I don't know which arch to use so I'll use this magical target_gdbarch function that gets me an arch" when the arch should in fact come from something in the context (a thread, objfile, symbol, etc). I think that removing it and inlining `current_inferior ()->arch ()` everywhere will make it a bit clearer where that arch comes from and will trigger people into reflecting whether this is the right place to get the arch or not. Change-Id: I79f14b4e4934c88f91ca3a3155f5fc3ea2fadf6b Reviewed-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-10-10gdb: add inferior::{arch, set_arch}Simon Marchi1-2/+2
Make the inferior's gdbarch field private, and add getters and setters. This helped me by allowing putting breakpoints on set_arch to know when the inferior's arch was set. A subsequent patch in this series also adds more things in set_arch. Change-Id: I0005bd1ef4cd6b612af501201cec44e457998eec Reviewed-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-10-05gdb: add all_objfiles_removed observerSimon Marchi1-5/+13
The new_objfile observer is currently used to indicate both when a new objfile is added to program space (when passed non-nullptr) and when all objfiles of a program space were just removed (when passed nullptr). I think this is confusing (and Andrew apparently thinks so too [1]). Add a new "all_objfiles_removed" observer to remove the second role from "new_objfile". Some existing users of new_objfile do nothing if the passed objfile is nullptr. For them, we can simply drop the nullptr check. For others, add a new all_objfiles_removed callback, and refactor things a bit to keep the existing behavior as much as possible. Some callbacks relied on current_program_space, and following the refactoring now use either objfile->pspace or the pspace passed to all_objfiles_removed. I think this should be relatively safe, and in general a step in the right direction. On the notify side, I found only one call site to change from new_objfile to all_objfiles_removed, in clear_symtab_users. It is not entirely clear to me that this is entirely correct. clear_symtab_users appears to be called in spots that don't remove all objfiles (functions finish_new_objfile, remove_symbol_file_command, reread_symbols, do_module_cleanups). But I think that this patch at least makes the current code clearer. [1] https://gitlab.com/gnutools/binutils-gdb/-/commit/a0a031bce0527b1521788b5dad640e7883b3a252 Change-Id: Icb648f72862e056267f30f44dd439bd4ec766f13 Approved-By: Tom Tromey <tom@tromey.com>
2023-09-19Use gdb::checked_static_cast for tracepointsTom Tromey1-6/+5
This replaces some casts to 'tracepoint *' with checked_static_cast. Some functions are changed to accept a 'tracepoint *' now, for better type safety. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-09-11gdb: c++ify btrace_target_infoMarkus Metzger1-18/+6
Following the example of private_thread_info and private_inferior, turn struct btrace_target_info into a small class hierarchy. Also merge btrace_tinfo_bts with btrace_tinfo_pt and inline into linux_btrace_target_info. Fixes PR gdb/30751.
2023-09-11gdb, btrace: move xml parsing into remote.cMarkus Metzger1-0/+317
The code is only used in remote.c and all functions can be declared static.
2023-08-29Default getpkt 'forever' parameter to 'false'Tom Tromey1-80/+81
This patch changes remote.c so that the getpkt 'forever' parameter now defaults to 'false' and fixes up all the callers. Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-08-29Unify getpkt and getpkt_or_notif_saneTom Tromey1-30/+5
getpkt and getpkt_or_notif_sane are just wrappers for getpkt_or_notif_sane_1. This patch adds the is_notif parameter to getpkt, with a suitable default, and removes the wrappers. Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-08-29Use bool in getpktTom Tromey1-102/+102
This changes getpkt and related functions to use bool rather than int. Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-08-29Remove expecting_notif parameter from getpkt_or_notif_sane_1Tom Tromey1-11/+12
For getpkt_or_notif_sane_1, expecting_notif is redundant, because it always reflects whether the is_notif parameter is non-NULL. This patch removes the redundant parameter. Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-08-29Remove getpkt_saneTom Tromey1-26/+14
I noticed that getpkt is just a wrapper around getpk_sane, so this patch unifies the two of them. Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-08-24[gdb/build] Return gdb::array_view in thread_info_to_thread_handleTom de Vries1-3/+3
In remote_target::thread_info_to_thread_handle we return a copy: ... gdb::byte_vector remote_target::thread_info_to_thread_handle (struct thread_info *tp) { remote_thread_info *priv = get_remote_thread_info (tp); return priv->thread_handle; } ... Fix this by returning a gdb::array_view instead: ... gdb::array_view<const gdb_byte> remote_target::thread_info_to_thread_handle (struct thread_info *tp) ... Tested on x86_64-linux. This fixes the build when building with -std=c++20. Approved-By: Pedro Alves <pedro@palves.net>
2023-08-17[gdb/build, c++20] Fix deprecated implicit capture of thisTom de Vries1-1/+1
When building gdb with -std=c++20 I run into: ... gdb/ada-lang.c:10713:16: error: implicit capture of ‘this’ via ‘[=]’ is \ deprecated in C++20 [-Werror=deprecated] 10713 | auto do_op = [=] (LONGEST x, LONGEST y) | ^ gdb/ada-lang.c:10713:16: note: add explicit ‘this’ or ‘*this’ capture ... Fix this by using "[this]". Likewise in two more spots. Tested on x86_64-linux.
2023-08-17[gdb/build, c++20] Fix Wdeprecated-enum-enum-conversionTom de Vries1-4/+8
When building gdb with clang 15 and -std=c++20, I run into: ... gdbsupport/common-exceptions.h:203:32: error: arithmetic between different \ enumeration types ('const enum return_reason' and 'const enum errors') is \ deprecated [-Werror,-Wdeprecated-enum-enum-conversion] size_t result = exc.reason + exc.error; ~~~~~~~~~~ ^ ~~~~~~~~~ ... Fix this by using to_underlying. Likewise in a few other places. Tested on x86_64-linux.
2023-08-04Remove extra '.' from error messageTom Tromey1-2/+2
A local gdb test failed with this error message: Remote communication error. Target disconnected.: Arg list too long. The ".:" seemed weird to me. This patch removes the ".". Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-08-03gdb: fix possible nullptr dereference in a remote_debug_printf callAndrew Burgess1-2/+2
While working on another patch I triggered a segfault from within the function remote_target::discard_pending_stop_replies. Turns out this was caused by a cut&paste error introduced in this commit: commit df5ad102009c41ab4dfadbb8cfb8c8b2a02a4f78 Date: Wed Dec 1 09:40:03 2021 -0500 gdb, gdbserver: detach fork child when detaching from fork parent This commit adds a remote_debug_printf call that was copied from earlier in the function, however, the new call wasn't updated to use the appropriate local variable. The local variable that it is using might be nullptr, in which case we trigger undefined behaviour, and could crash, which is what I was seeing. Fixed by updating to use the correct local variable.
2023-06-20Use gdb::byte_vector in agent_exprTom Tromey1-7/+7
This changes agent_expr to use gdb::byte_vector rather than manually reimplementing a vector. Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-06-20Use byte_vector in remote.c:readahead_cacheTom Tromey1-11/+8
This patch changes the remote.c readahead_cache to use gdb::byte_vector. This simplifies the code by removing manual memory management. Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-06-03[gdb] Fix typosTom de Vries1-2/+2
Fix a few typos: - implemention -> implementation - convertion(s) -> conversion(s) - backlashes -> backslashes - signoring -> ignoring - (un)ambigious -> (un)ambiguous - occured -> occurred - hidding -> hiding - temporarilly -> temporarily - immediatelly -> immediately - sillyness -> silliness - similiar -> similar - porkuser -> pokeuser - thats -> that - alway -> always - supercede -> supersede - accomodate -> accommodate - aquire -> acquire - priveleged -> privileged - priviliged -> privileged - priviledges -> privileges - privilige -> privilege - recieve -> receive - (p)refered -> (p)referred - succesfully -> successfully - successfuly -> successfully - responsability -> responsibility - wether -> whether - wich -> which - disasbleable -> disableable - descriminant -> discriminant - construcstor -> constructor - underlaying -> underlying - underyling -> underlying - structureal -> structural - appearences -> appearances - terciarily -> tertiarily - resgisters -> registers - reacheable -> reachable - likelyhood -> likelihood - intepreter -> interpreter - disassemly -> disassembly - covnersion -> conversion - conviently -> conveniently - atttribute -> attribute - struction -> struct - resonable -> reasonable - popupated -> populated - namespaxe -> namespace - intialize -> initialize - identifer(s) -> identifier(s) - expection -> exception - exectuted -> executed - dungerous -> dangerous - dissapear -> disappear - completly -> completely - (inter)changable -> (inter)changeable - beakpoint -> breakpoint - automativ -> automatic - alocating -> allocating - agressive -> aggressive - writting -> writing - reguires -> requires - registed -> registered - recuding -> reducing - opeartor -> operator - ommitted -> omitted - modifing -> modifying - intances -> instances - imbedded -> embedded - gdbaarch -> gdbarch - exection -> execution - direcive -> directive - demanged -> demangled - decidely -> decidedly - argments -> arguments - agrument -> argument - amespace -> namespace - targtet -> target - supress(ed) -> suppress(ed) - startum -> stratum - squence -> sequence - prompty -> prompt - overlow -> overflow - memember -> member - languge -> language - geneate -> generate - funcion -> function - exising -> existing - dinking -> syncing - destroh -> destroy - clenaed -> cleaned - changep -> changedp (name of variable) - arround -> around - aproach -> approach - whould -> would - symobl -> symbol - recuse -> recurse - outter -> outer - freeds -> frees - contex -> context Tested on x86_64-linux. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-05-30gdb: add interp::on_normal_stop methodSimon Marchi1-1/+2
Same idea as the previous patch, but for the normal_stop event. Change-Id: I4fc8ca8a51c63829dea390a2b6ce30b77f9fb863
2023-05-30gdb: add interp::on_signal_received methodSimon Marchi1-1/+1
Instead of having the interpreter code registering observers for the signal_received observable, add a "signal_received" virtual method to struct interp. Add a interps_notify_signal_received function that loops over all UIs and calls the signal_received method on the interpreter. Finally, add a notify_signal_received function that calls interps_notify_signal_received and then notifies the observers. Replace all existing notifications to the signal_received observers with calls to notify_signal_received. Before this patch, the CLI and MI code both register a signal_received observer. These observer go over all UIs, and, for those that have a interpreter of the right kind, print the stop notifiation. After this patch, we have just one "loop over all UIs", inside interps_notify_signal_received. Since the interp::on_signal_received method gets called once for each interpreter, the implementations only need to deal with the current interpreter (the "this" pointer). The motivation for this patch comes from a future patch, that makes the amdgpu code register an observer to print a warning after the CLI's signal stop message. Since the amdgpu and the CLI code both use observers, the order of the two messages is not stable, unless we define the priority using the observer dependency system. However, the approach of using virtual methods on the interpreters seems like a good change anyway, I think it's more straightforward and simple to understand than the current solution that uses observers. We are sure that the amdgpu message gets printed after the CLI message, since observers are notified after interpreters. Keep the signal_received, even if nothing uses if, because we will be using it in the upcoming amdgpu patch implementing the warning described above. Change-Id: I4d8614bb8f6e0717f4bfc2a59abded3702f23ac4
2023-05-25gdb: remove bp_location_pointer_iteratorSimon Marchi1-2/+2
Remove the bp_location_pointer_iterator layer. Adjust all users of breakpoint::locations to use references instead of pointers. Change-Id: Iceed34f5e0f5790a9cf44736aa658be6d1ba1afa Reviewed-By: Andrew Burgess <aburgess@redhat.com>
2023-04-27gdb: remove some trailing newlines from warning messagesAndrew Burgess1-2/+2
While working on a later patch in this series, which tightens up some of our pattern matching when using gdb_test, I ran into some failures caused by some warnings having a trailing newline character. The warning function already adds a trailing newline, and it is my understanding that we should not be adding a second by including a newline at the end of any warning message. The problem cases I found were in language.c and remote.c, in this patch I fix the cases I hit, but I also checked all the other warning calls in these two files and removed any additional trailing newlines I found. In remote.c the warning actually had a newline character in the middle of the warning message (in addition to the trailing newline), which I've removed. I don't think it's helpful to forcibly split a warning as was done here -- in the middle of a sentence. Additionally, the message isn't even that long (71 characters), so I think removing this newline is an improvement. None of the expected test result need updating with this commit, currently the patterns in gdb_test will match one or more newline sequences, so the tests are as happy with one newline (after this commit) as they are with two newlines (before this commit). A later commit will change gdb_test so that it is not so forgiving, and these warnings would have caused some failures. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-04-04gdb: make find_thread_ptid a process_stratum_target methodSimon Marchi1-9/+9
Make find_thread_ptid (the overload that takes a process_stratum_target) a method of process_stratum_target. Change-Id: Ib190a925a83c6b93e9c585dc7c6ab65efbdd8629 Reviewed-By: Tom Tromey <tom@tromey.com>
2023-03-09gdb, gdbserver, gdbsupport: fix whitespace issuesSimon Marchi1-5/+5
Replace spaces with tabs in a bunch of places. Change-Id: If0f87180f1d13028dc178e5a8af7882a067868b0
2023-01-30gdb: Remove workaround for the vCont packetChristina Schimpe1-19/+4
The workaround for the vCont packet is no longer required due to the former commit "gdb: Make global feature array a per-remote target array". The vCont packet is now checked once when the connection is started and the supported vCont actions are set to the target's remote state attribute.
2023-01-30gdb: Add per-remote target variables for memory read and write configChristina Schimpe1-46/+117
This patch adds per-remote target variables for the configuration of memory read- and write packet size. It is a further change to commit "gdb: Make global feature array a per-remote target array" to apply the fixme notes described in commit 5b6d1e4 "Multi-target support". The former global variables for that configuration are still available to allow the command line configuration for all future remote connections. Similar to the command line configuration of the per- remote target feature array, the commands - set remotewritesize (deprecated) - set remote memory-read-packet-size - set remote memory-write-packet-size will configure the current target (if available). If no target is available, the default configuration for future remote connections is adapted. The show command will display the current remote target's packet size configuration. If no remote target is selected, the default configuration for future connections will be shown. It is required to adapt the test gdb.base/remote.exp which is failing for --target_board=native-extended-gdbserver. With that board GDB connects to gdbserver at gdb start time. Due to this patch two loggings "The target may not be able to.." are shown if the command 'set remote memory-write-packet-size fixed' is executed while a target is connected for the current inferior. To fix this, the clean_restart command is moved to a later time point of the test. It is sufficient to be connected to the server when "runto_main" is executed. Now the connection time is similar to a testrun with --target_board=native-gdbserver. To allow the user to distinguish between the packet-size configuration for future remote connections and for the currently selected target, the commands' loggings are adapted.