aboutsummaryrefslogtreecommitdiff
path: root/gdb/remote.c
AgeCommit message (Collapse)AuthorFilesLines
12 days[gdb] Fix common misspellingsTom de Vries1-4/+4
Fix the following common misspellings: ... accidently -> accidentally additonal -> additional addresing -> addressing adress -> address agaisnt -> against albiet -> albeit arbitary -> arbitrary artifical -> artificial auxillary -> auxiliary auxilliary -> auxiliary bcak -> back begining -> beginning cannonical -> canonical compatiblity -> compatibility completetion -> completion diferent -> different emited -> emitted emiting -> emitting emmitted -> emitted everytime -> every time excercise -> exercise existance -> existence fucntion -> function funtion -> function guarentee -> guarantee htis -> this immediatly -> immediately layed -> laid noone -> no one occurances -> occurrences occured -> occurred originaly -> originally preceeded -> preceded preceeds -> precedes propogate -> propagate publically -> publicly refering -> referring substract -> subtract substracting -> subtracting substraction -> subtraction taht -> that targetting -> targeting teh -> the thier -> their thru -> through transfered -> transferred transfering -> transferring upto -> up to vincinity -> vicinity whcih -> which whereever -> wherever wierd -> weird withing -> within writen -> written wtih -> with doesnt -> doesn't ... Tested on x86_64-linux.
2024-09-24gdb: testsuite: Test whether PC register is expedited in ↵Thiago Jung Bauermann1-1/+26
gdb.server/server-run.exp One thing GDB always does when the inferior stops is finding out where it's stopped at, by way of querying the value of the program counter register. To save a packet round trip, the remote target can send the PC value (often alongside other frequently consulted registers such as the stack pointer) in the stop reply packet as an "expedited register". Test that this is actually done for the targets where gdbserver is supposed to. Extend the "maintenance print remote-registers" command output with an "Expedited" column which says "yes" if the register was seen by GDB in the last stop reply packet it received, and is left blank otherwise. Tested for regressions on aarch64-linux-gnu native-extended-remote. The testcase was tested on aarch64-linux-gnu, i686-linux-gnu and x86_64-linux-gnu native-remote and native-extended-remote targets. Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Tom Tromey <tom@tromey.com>
2024-09-24[gdb] Handle ^C in ~scoped_remote_fdTom de Vries1-0/+12
While reviewing "catch (...)" uses I came across: ... try { fileio_error remote_errno; m_remote->remote_hostio_close (m_fd, &remote_errno); } catch (...) { /* Swallow exception before it escapes the dtor. If something goes wrong, likely the connection is gone, and there's nothing else that can be done. */ } ... This also swallows gdb_exception_quit and gdb_exception_forced_quit. I don't know whether these can actually happen here, but if not it's better to accommodate for the possibility anyway. Fix this by handling gdb_exception_quit and gdb_exception_forced_quit explicitly. It could be that "catch (...)" should be replaced by "catch (const gdb_exception &)" but that depends on what kind of exception remote_hostio_close is expected to throw, and I don't know that, so I'm leaving it as is. Tested on aarch64-linux. Approved-By: Tom Tromey <tom@tromey.com>
2024-09-24btrace: Enable event tracing on Linux for Intel PT.Felix Willgerodt1-1/+49
Event tracing allows GDB to show information about interesting asynchronous events when tracing with Intel PT. Subsequent patches will add support for displaying each type of event. Enabling event-tracing unconditionally would result in rather noisy output, as breakpoints themselves result in interrupt events. Which is why this patch adds a set/show command to allow the user to enable/disable event-tracing before starting a recording. The event-tracing setting has no effect on an already active recording. The default setting is off. As event tracing will use the auxiliary infrastructure added by ptwrite, the user can still disable printing events, even when event-tracing was enabled, by using the /a switch for the record instruction-history/function-call-history commands. Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Markus Metzger <markus.t.metzger@intel.com>
2024-08-14btrace, gdbserver: Add ptwrite to btrace_config_pt.Felix Willgerodt1-1/+41
This enables gdb and gdbserver to communicate about ptwrite support. If ptwrite support would be enabled unconditionally, GDBs with older libipt versions would break. Approved-By: Markus Metzger <markus.t.metzger@intel.com> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
2024-08-12gdb: add program_space parameter to lookup_minimal_symbolSimon Marchi1-1/+2
>From what I can see, lookup_minimal_symbol doesn't have any dependencies on the global current state other than the single reference to current_program_space. Add a program_space parameter and make that current_program_space reference bubble up one level. Change-Id: I759415e2f9c74c9627a2fe05bd44eb4147eee6fe Reviewed-by: Keith Seitz <keiths@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-08-12gdb: make lookup_minimal_symbol objf and sfile parameters optionalSimon Marchi1-2/+1
Most calls to lookup_minimal_symbol don't pass a value for sfile and objf. Make these parameters optional (have a default value of nullptr). And since passing a value to `objf` is much more common than passing a value to `sfile`, swap the order so `objf` comes first, to avoid having to pass a nullptr value to `sfile` when wanting to pass a value to `objf`. Change-Id: I8e9cc6b942e593bec640f9dfd30f62786b0f5a27 Reviewed-by: Keith Seitz <keiths@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-08-12gdb: drop struct keyword when using bound_minimal_symbolSimon Marchi1-3/+2
This is a simple find / replace from "struct bound_minimal_symbol" to "bound_minimal_symbol", to make things shorter and more consisten througout. In some cases, move variable declarations where first used. Change-Id: Ica4af11c4ac528aa842bfa49a7afe8fe77a66849 Reviewed-by: Keith Seitz <keiths@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-07-24Remove redundant macro definitions from remote.cTom Tromey1-11/+0
I happened to notice that a few macros are defined twice in remote.c. This patch removes one copy. Tested by rebuilding. Reviewed-By: Tom de Vries <tdevries@suse.de>
2024-07-22gdb/remote: remove an out of date commentAndrew Burgess1-3/+0
A comment above an `if` check was accidentally left in place after this commit: commit ddb3f3d89cf62df6be3cb9e110504def19625160 Date: Tue Mar 19 12:34:34 2024 +0100 Add "error_message+" feature to qSupported The comment relates to how 'E.msg' style remote replies are not supported by every packet, but after the above commit they are supported in all cases (that call packet_check_result), and the comment should have been removed.
2024-07-18gdb: add GDB side target_ops::fileio_stat implementationAndrew Burgess1-11/+72
This commit adds the GDB side of target_ops::fileio_stat. There's an implementation for inf_child_target, which just calls 'lstat', and there's an implementation for remote_target, which sends a new vFile:stat packet. The new packet is documented. There's still no users of target_fileio_stat as I have not yet added support for vFile::stat to gdbserver. If these packets are currently sent to gdbserver then they will be reported as not supported and the ENOSYS error code will be returned. Reviewed-By: Eli Zaretskii <eliz@gnu.org>
2024-07-16gdb, gdbserver, gdbsupport: use [[noreturn]] instead of ATTRIBUTE_NORETURNSimon Marchi1-2/+2
C++ 11 has a built-in attribute for this, no need to use a compat macro. Change-Id: I90e4220d26e8f3949d91761f8a13cd9c37da3875 Reviewed-by: Lancelot Six <lancelot.six@amd.com>
2024-07-16gdb: fix indentation in remote.cSimon Marchi1-1/+1
Change-Id: If344acdf703fdd3892f73f75fc891d5473808b79
2024-07-16gdb: add ATTRIBUTE_NORETURN to remote_unpush_targetSimon Marchi1-1/+1
My IDE (well, clangd) suggested this. It doesn't hurt to have it. Change-Id: If6001983c17dbed3dceebac3078c8deb12c04d6b
2024-07-15gdb: pass program space to no_shared_librariesSimon Marchi1-1/+1
Make the current program space reference bubble up one level. Pass `current_program_space` everywhere, except in some cases where we can get the pspace another way, and it's relatively obvious that it's the same as the current program space. Change-Id: Id86b79f1e44f92a398f49d137d57457174dfa96d Approved-By: Tom Tromey <tom@tromey.com> Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
2024-07-15gdb: split no_shared_libraries, command vs implementationSimon Marchi1-1/+1
The `no_shared_libraries` function is currently used to implement the `nosharedlibrary` command, but it also used internally by other functions. This does not make a very good internal API. Add the `no_shared_libraries_command` function to implement the CLI command. Remove the unused parameters from `no_shared_libraries`. Remove the `from_tty` parameter of `target_pre_inferior`, since it's now unused. Change-Id: I4fcba5ee1e0f7d250aab1a7b62b9ea16265fe962 Approved-By: Tom Tromey <tom@tromey.com> Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
2024-07-15gdb: make objfile::pspace privateSimon Marchi1-1/+1
Rename to m_pspace, add getter. An objfile's pspace never changes, so no setter is necessary. Change-Id: If4dfb300cb90dc0fb9776ea704ff92baebb8f626
2024-06-12Add "error_message+" feature to qSupportedAlexandra Hájková1-32/+46
Add a new 'error_message' feature to the qSupported packet. When GDB supports this feature then gdbserver is able to send errors in the E.errtext format for the qRcmd and m packets. Update qRcmd packet and m packets documentation as qRcmd newly accepts errors in a E.errtext format. Previously these two packets didn't support E.errtext style errors. Approved-By: Tom Tromey <tom@tromey.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-06-11gdb: warn of slow remote file reading only after a successful openAndrew Burgess1-17/+19
While working on a later patch in this series, I noticed that GDB would print the message: Reading /path/to/file from remote target... Even when /path/to/file doesn't exist on the remote target. GDB does indeed try to open /path/to/file, but I'm not sure we really need to tell the user unless we actually manage to open the file, and plan to read content from it. If we consider how GDB probes for separate debug files, we can attempt to open multiple possible files, most of them will not exist. When we are native debugging we don't bother telling the user about each file we're checking for, we just announce any file we finally use. I think it makes sense to do a similar thing for remote files. So, in remote_target::remote_hostio_open(), I'd like to move the block of code that prints the above message to after the open call has been made, and we should only print the message if the open succeeds. Now GDB only tells the user about files that we actually open and read from the remote. Approved-By: Tom Tromey <tom@tromey.com>
2024-06-11gdb/fileio: fix errno for packets where an attachment is expectedAndrew Burgess1-0/+3
In remote.c lives remote_target::remote_hostio_send_command(), which is used to handle sending a fileio packet to the remote, and for parsing the reply packet. Some commands, e.g. open, pwrite, close, send some arguments to the remote, and then get back a single integer return value. Other commands though, e.g. pread, readlink, fstat, send some arguments and get back an integer return value and some additional data. This additional data is called the attachment. Except, we only get the attachment if the command completes successfully. For example, calling readlink with a non existent path will result in a return packet: 'F-1,2' with no attachment. This is as expected. Within remote_hostio_send_command we call remote_hostio_parse_result, this parses the status code (-1 in our example above) and then parses the errno value (2 in our example above). Back in remote_hostio_parse_result we then hit this block: /* Make sure we saw an attachment if and only if we expected one. */ if ((attachment_tmp == NULL && attachment != NULL) || (attachment_tmp != NULL && attachment == NULL)) { *remote_errno = FILEIO_EINVAL; return -1; } Which ensures that commands that expect an attachment, got an attachment. The problem is, we'll only get an attachment if the command succeeded. If it didn't, then there is no attachment, and that is as expected. As remote_hostio_parse_result always sets the returned error number to FILEIO_SUCCESS unless the packet contained an actual error number (e.g. 2 in our example above), I suggest we should return early if remote_hostio_parse_result indicates an error packet. I ran into this issue while working on another patch. In that patch I was checking the error code returned by a remote readlink call and spotted that when I passed an invalid path I got EINVAL instead of ENOENT. This patch fixes this issue. Unfortunately the patch I was working on evolved, and my need to check the error code went away, and so, right now, I have no way to reveal this bug. But I think this is an obviously correct fix, and worth merging even without a test. Approved-By: Tom Tromey <tom@tromey.com>
2024-06-07gdb: replace `get_exec_file (0)` calls with ↵Simon Marchi1-1/+1
`current_program_space->exec_filename ()` Calls of `get_exec_file (0)` are equivalent to just getting the exec filename from the current program space. I'm looking to remove `get_exec_file`, so replace these uses with `current_program_space->exec_filename ()`. Remove the `err` parameter of `get_exec_wrapper` since all the calls that remain pass 1, meaning to error out if no executable is specified. Change-Id: I7729ea4c7f03dbb046211cc5aa3858ab3a551965 Approved-By: Tom Tromey <tom@tromey.com>
2024-06-05gdb: more filename styling in remote.c and target.cAndrew Burgess1-5/+9
I spotted a few more places where we could apply filename styling in remote.c and target.c. Other than the styling, there should be no user visible changes after this commit. Approved-By: Tom Tromey <tom@tromey.com>
2024-05-30gdb: remove unused includes in utils.hSimon Marchi1-0/+1
Remove some includes reported as unused by clangd. Add some includes in other files that were previously relying on the transitive include. Change-Id: Ibdd0a998b04d21362a20d0ca8e5267e21e2e133e
2024-05-28Use bool in thread_eventsTom Tromey1-2/+2
This changes target_ops::thread_events and target_thread_events to use 'bool'. The callers were already doing this. Tested by rebuilding. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2024-05-16gdb: initialize packet_result::m_textual_err_msgSimon Marchi1-1/+1
When building GDB with -O2 and --enable-ubsan, I get some random errors in the packet_result self test: /home/smarchi/src/binutils-gdb/gdb/remote.c:161:7: runtime error: load of value 92, which is not a valid value for type 'bool' This happens because packet_result::m_textual_err_msg is uninitialized when using the second constructor. When such a packet_result object gets copied, an invalid value for m_textual_err_msg (a bool field) is loaded, which triggers ubsan. Avoid this by initializing m_textual_err_msg. Change-Id: I3ce44816bb0bfc6e442067292f993e5c17301b85 Approved-By: Tom Tromey <tom@tromey.com>
2024-05-10Pass stream to remote_console_outputTom Tromey1-8/+12
I noticed that remote_target::rcmd did not pass its ui_file argument down to remote_console_output. This patch fixes this oversight. Tested-By: Ciaran Woodward <ciaranwoodward@xmos.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-04-29gdb/remote: fix qRcmd error handlingAndrew Burgess1-10/+15
This commit: commit 3623271997a5c0d79609aa6a1f35ef61b4469054 Date: Tue Jan 30 15:55:47 2024 +0100 remote.c: Use packet_check_result Introduced a bug in the error handling of the qRcmd packet. Prior to this commit if a packet had status PACKET_OK then, if the packet contained the text "OK" we considered the packet handled. But, if the packet contained any other content (that was not an error message) then the content was printed to the user. After the above commit this was no longer the case, any non-error packet that didn't contain "OK" would be treated as an error. Currently, gdbserver doesn't exercise this path so it's not possible to write a simple test for this case. When gdbserver wishes to print output it sends back an 'O' string output packet, these packets are handled earlier in the process. Then once gdbserver has finished sending output an 'OK' packet is sent. Approved-By: Tom Tromey <tom@tromey.com>
2024-04-26Improve vRun error reportingPedro Alves1-12/+55
After the previous commit, if starting the inferior process with "run" (vRun packet) fails, GDBserver reports an error using the "E." textual error packet. On the GDB side, however, GDB doesn't yet do anything with the textual error string. This commit improves that. This makes remote debugging output the same as native output, when possible, another small step in the "local/remote parity" project. E.g., before, against GNU/Linux GDBserver: (gdb) run Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox Running ".../gdb.base/run-fail-twice/run-fail-twice.nox" on the remote target failed After, against GNU/Linux GDBserver (same as native): (gdb) run Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox During startup program exited with code 126. To know whether we have a textual error message, extend packet_result to carry that information. While at it, convert packet_result to use factory methods, and change its std::string parameter to a plain const char *, as that it always what we have handy to pass to it. Change-Id: Ib386f267522603f554b52a885b15229c9639e870 Approved-By: Tom Tromey <tom@tromey.com>
2024-04-25gdb: remove gdbcmd.hSimon Marchi1-1/+1
Most files including gdbcmd.h currently rely on it to access things actually declared in cli/cli-cmds.h (setlist, showlist, etc). To make things easy, replace all includes of gdbcmd.h with includes of cli/cli-cmds.h. This might lead to some unused includes of cli/cli-cmds.h, but it's harmless, and much faster than going through the 170 or so files by hand. Change-Id: I11f884d4d616c12c05f395c98bbc2892950fb00f Approved-By: Tom Tromey <tom@tromey.com>
2024-04-19gdb/testsuite: Add unit tests for qIsAddressTagged packetGustavo Romero1-0/+67
Add unit tests for testing qIsAddressTagged packet request creation and reply checks. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com>
2024-04-19gdb: Add qIsAddressTagged packetGustavo Romero1-0/+75
This commit adds a new packet, qIsAddressTagged, allowing GDB remote targets to use it to query the stub if a given address is tagged. Currently, the memory tagging address check is done via a read query, where the contents of /proc/<PID>/smaps is read and the flags are inspected for memory tagging-related flags that indicate the address is in a memory tagged region. This is not ideal, for example, for QEMU stub and other cases, such as on bare-metal, where there is no notion of an OS file like 'smaps.' Hence, the introduction of qIsAddressTagged packet allows checking if an address is tagged in an agnostic way. The is_address_tagged target hook in remote.c attempts to use the qIsAddressTagged packet first for checking if an address is tagged and if the stub does not support such a packet (reply is empty) it falls back to using the current mechanism that reads the contents of /proc/<PID>/smaps via vFile requests. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com>
2024-04-19gdb: Introduce is_address_tagged target hookGustavo Romero1-0/+10
This commit introduces a new target hook, target_is_address_tagged, which is used instead of the gdbarch_tagged_address_p gdbarch hook in the upper layer (printcmd.c). This change enables easy specialization of memory tagging address check per target in the future. As target_is_address_tagged continues to utilize the gdbarch_tagged_address_p hook, there is no change in behavior for all the targets that use the new target hook (i.e., the remote.c, aarch64-linux-nat.c, and corelow.c targets). Just the gdbarch_tagged_address_p signature is changed for convenience, since target_is_address_tagged takes the address to be checked as a CORE_ADDR type. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com>
2024-04-11gdb: fix format in remote.cTankut Baris Aktemur1-3/+3
Fix space-before-parenthesis format at three spots in remote.c.
2024-04-08remote.c: Make packet_ok return struct packet_resultAlexandra Hájková1-103/+96
This allows the error message stored in a packet_result to be easily printed in the calling function. Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-04-08remote.c: Use packet_check_resultAlexandra Hájková1-35/+46
when processing the GDBserver reply to qRcmd packet. Print error message or the error code. Currently, when qRcmd request returns an error, GDB just prints: Protocol error with Rcmd After this change, GDB will also print the error code: Protocol error with Rcmd: 01. Add an accept_msg argument to packet_check result. qRcmd request (such as many other packets) does not recognise "E.msg" form as an error right now. We want to recognise "E.msg" as an error response only for the packets where it's documented. Also use packet_check result in remote_read_bytes_1. Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-03-26gdb, gdbserver, gdbsupport: remove includes of early headersSimon Marchi1-1/+0
Now that defs.h, server.h and common-defs.h are included via the `-include` option, it is no longer necessary for source files to include them. Remove all the inclusions of these files I could find. Update the generation scripts where relevant. Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837 Approved-By: Pedro Alves <pedro@palves.net>
2024-02-12remote.c: Make packet_check_result return a structureAlexandra Hájková1-34/+104
packet_check_result currently returns an packet_result enum. If GDB will recieve an error in a format E.errtext, which is possible for some q packets, such errtext is lost if treated by packet_check_result. Introduce a new structure which bundles enum packet_result with possible error message or error code returned by the GDBserver. I plan to make more GDBserver response checking functions to use packet_check_result to make remote.c code more consistent. This will also allow to use E.errtext more in the future. Beside adding the unit test, I tested this by modifying store_registers_using_G function to get an error message: [remote] Sending packet: $GG00000000 ... gdbserver: Wrong sized register packet (expected 1792 bytes, got 1793) gdbserver: Invalid hex digit 71 Killing process(es): 1104002 [remote] Packet received: E01 Could not write registers; remote failure reply '01' Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
2024-02-04Use reference result of emplace_backTom Tromey1-2/+1
Starting with C++17, emplace_back returns a reference to the new object. This patch changes code that uses emplace_back followed by a call to back() to simply use this reference instead. Approved-By: Simon Marchi <simon.marchi@efficios.com>
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