aboutsummaryrefslogtreecommitdiff
path: root/gdb/mi
AgeCommit message (Collapse)AuthorFilesLines
2023-02-27QUIT processing w/ explicit throw for gdb_exception_forced_quitKevin Buettner1-0/+4
This commit contains changes which have an explicit throw for gdb_exception_forced_quit, or, in a couple of cases for gdb_exception, but with a throw following a check to see if 'reason' is RETURN_FORCED_QUIT. Most of these are straightforward - it made sense to continue to allow an existing catch of gdb_exception to also catch gdb_exception_quit; in these cases, a catch/throw for gdb_exception_forced_quit was added. There are two cases, however, which deserve a more detailed explanation. 1) remote_fileio_request in gdb/remote-fileio.c: The try block calls do_remote_fileio_request which can (in turn) call one of the functions in remote_fio_func_map[]. Taking the first one, remote_fileio_func_open(), we have the following call path to maybe_quit(): remote_fileio_func_open(remote_target*, char*) -> target_read_memory(unsigned long, unsigned char*, long) -> target_read(target_ops*, target_object, char const*, unsigned char*, unsigned long, long) -> maybe_quit() Since there is a path to maybe_quit(), we must ensure that the catch block is not permitted to swallow a QUIT representing a SIGTERM. However, for this case, we must take care not to change the way that Ctrl-C / SIGINT is handled; we want to send a suitable EINTR reply to the remote target should that happen. That being the case, I added a catch/throw for gdb_exception_forced_quit. I also did a bit of rewriting here, adding a catch for gdb_exception_quit in favor of checking the 'reason' code in the catch block for gdb_exception. 2) mi_execute_command in gdb/mi/mi-main.c: The try block calls captured_mi_execute_command(); there exists a call path to maybe_quit(): captured_mi_execute_command(ui_out*, mi_parse*) -> mi_cmd_execute(mi_parse*) -> get_current_frame() -> get_prev_frame_always_1(frame_info*) -> frame_register_unwind_location(frame_info*, int, int*, lval_type*, unsigned long*, int*) -> frame_register_unwind(frame_info*, int, int*, int*, lval_type*, unsigned long*, int*, unsigned char*) -> value_entirely_available(value*) -> value_fetch_lazy(value*) -> value_fetch_lazy_memory(value*) -> read_value_memory(value*, long, int, unsigned long, unsigned char*, unsigned long) -> maybe_quit() That being the case, we can't allow the exception handler (catch block) to swallow a gdb_exception_quit for SIGTERM. However, it does seem reasonable to output the exception via the mi interface so that some suitable message regarding SIGTERM might be printed; therefore, I check the exception's 'reason' field for RETURN_FORCED_QUIT and do a throw for this case. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761 Tested-by: Tom de Vries <tdevries@suse.de> Approved-By: Pedro Alves <pedro@palves.net>
2023-02-27Catch gdb_exception_error instead of gdb_exception (in many places)Kevin Buettner2-2/+2
As described in the previous commit for this series, I became concerned that there might be instances in which a QUIT (due to either a SIGINT or SIGTERM) might not cause execution to return to the top level. In some (though very few) instances, it is okay to not propagate the exception for a Ctrl-C / SIGINT, but I don't think that it is ever okay to swallow the exception caused by a SIGTERM. Allowing that to happen would definitely be a deviation from the current behavior in which GDB exits upon receipt of a SIGTERM. I looked at all cases where an exception handler catches a gdb_exception. Handlers which did NOT need modification were those which satisifed one or more of the following conditions: 1) There is no call path to maybe_quit() in the try block. I used a static analysis tool to help make this determination. In instances where the tool didn't provide an answer of "yes, this call path can result in maybe_quit() being called", I reviewed it by hand. 2) The catch block contains a throw for conditions that it doesn't want to handle; these "not handled" conditions must include the quit exception and the new "forced quit" exception. 3) There was (also) a catch for gdb_exception_quit. Any try/catch blocks not meeting the above conditions could potentially swallow a QUIT exception. My first thought was to add catch blocks for gdb_exception_quit and then rethrow the exception. But Pedro pointed out that this can be handled without adding additional code by simply catching gdb_exception_error instead. That's what this patch series does. There are some oddball cases which needed to be handled differently, plus the extension languages, but those are handled in later patches. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761 Tested-by: Tom de Vries <tdevries@suse.de> Approved-by: Pedro Alves <pedro@palves.net>
2023-02-19Remove ALL_BLOCK_SYMBOLSTom Tromey1-3/+1
This removes ALL_BLOCK_SYMBOLS in favor of foreach.
2023-02-13Turn many optimized-out value functions into methodsTom Tromey2-5/+4
This turns many functions that are related to optimized-out or availability-checking to be methods of value. The static function value_entirely_covered_by_range_vector is also converted to be a private method. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn value_contents_eq into a methodTom Tromey1-2/+2
This changes value_contents_eq to be a method of value. It also converts the static function value_contents_bits_eq into a private method. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn some value offset functions into methodTom Tromey1-1/+1
This changes various offset-related functions to be methods of value. Much of this patch was written by script. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn value_type into methodTom Tromey2-4/+4
This changes value_type to be a method of value. Much of this patch was written by script. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-08Simplify interp::exec / interp_exec - let exceptions propagatePedro Alves2-9/+3
This patch implements a simplication that I suggested here: https://sourceware.org/pipermail/gdb-patches/2022-March/186320.html Currently, the interp::exec virtual method interface is such that subclass implementations must catch exceptions and then return them via normal function return. However, higher up the in chain, for the CLI we get to interpreter_exec_cmd, which does: for (i = 1; i < nrules; i++) { struct gdb_exception e = interp_exec (interp_to_use, prules[i]); if (e.reason < 0) { interp_set (old_interp, 0); error (_("error in command: \"%s\"."), prules[i]); } } and for MI we get to mi_cmd_interpreter_exec, which has: void mi_cmd_interpreter_exec (const char *command, char **argv, int argc) { ... for (i = 1; i < argc; i++) { struct gdb_exception e = interp_exec (interp_to_use, argv[i]); if (e.reason < 0) error ("%s", e.what ()); } } Note that if those errors are reached, we lose the original exception's error code. I can't see why we'd want that. And, I can't see why we need to have interp_exec catch the exception and return it via the normal return path. That's normally needed when we need to handle propagating exceptions across C code, like across readline or ncurses, but that's not the case here. It seems to me that we can simplify things by removing some try/catch-ing and just letting exceptions propagate normally. Note, the "error in command" error shown above, which only exists in the CLI interpreter-exec command, is only ever printed AFAICS if you run "interpreter-exec console" when the top level interpreter is already the console/tui. Like: (gdb) interpreter-exec console "foobar" Undefined command: "foobar". Try "help". error in command: "foobar". You won't see it with MI's "-interpreter-exec console" from a top level MI interpreter: (gdb) -interpreter-exec console "foobar" &"Undefined command: \"foobar\". Try \"help\".\n" ^error,msg="Undefined command: \"foobar\". Try \"help\"." (gdb) nor with MI's "-interpreter-exec mi" from a top level MI interpreter: (gdb) -interpreter-exec mi "-foobar" ^error,msg="Undefined MI command: foobar",code="undefined-command" ^done (gdb) in both these cases because MI's -interpreter-exec just does: error ("%s", e.what ()); You won't see it either when running an MI command with the CLI's "interpreter-exec mi": (gdb) interpreter-exec mi "-foobar" ^error,msg="Undefined MI command: foobar",code="undefined-command" (gdb) This last case is because MI's interp::exec implementation never returns an error: gdb_exception mi_interp::exec (const char *command) { mi_execute_command_wrapper (command); return gdb_exception (); } Thus I think that "error in command" error is pretty pointless, and since it simplifies things to not have it, the patch just removes it. The patch also ends up addressing an old FIXME. Change-Id: I5a6432a80496934ac7127594c53bf5221622e393 Approved-By: Tom Tromey <tromey@adacore.com> Approved-By: Kevin Buettner <kevinb@redhat.com>
2023-01-25Clean up unusual code in mi-cmd-stack.cTom Tromey1-7/+9
I noticed some unusual code in mi-cmd-stack.c. This code is a switch, where one of the cases appears in the middle of another block. It seemed cleaner to me to have the earlier case just conditionally fall through.
2023-01-20gdb: make frame_info_ptr auto-reinflatableSimon Marchi1-1/+0
This is the second step of making frame_info_ptr automatic, reinflate on demand whenever trying to obtain the wrapper frame_info pointer, either through the get method or operator->. Make the reinflate method private, it is used as a convenience method in those two. Add an "is_null" method, because it is often needed to know whether the frame_info_ptr wraps an frame_info or is empty. Make m_ptr mutable, so that it's possible to reinflate const frame_info_ptr objects. Whether m_ptr is nullptr or not does not change the logical state of the object, because we re-create it on demand. I believe this is the right use case for mutable. Change-Id: Icb0552d0035e227f81eb3c121d8a9bb2f9d25794 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20gdb: make frame_info_ptr grab frame level and id on constructionSimon Marchi1-1/+0
This is the first step of making frame_info_ptr automatic. Remove the frame_info_ptr::prepare_reinflate method, move that code to the constructor. Change-Id: I85cdae3ab1c043c70e2702e7fb38e9a4a8a675d8 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-06gdb/mi: add no-history stop reasonBruno Larsen2-0/+2
When executing in reverse and runs out of recorded history, GDB prints a warning to the user, but does not add a reason in the stopped record, for example: *stopped,frame={addr="0x000000000040113e",func="main",args=[],file="/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c",fullname="/home/blarsen/Documents/binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c",line="27",arch="i386:x86-64"},thread-id="1",stopped-threads="all",core="1" This problem was reported as record/29260. This commit adds the reason no-history to the record, making it easier for interfaces using the mi interpreter to report the result. It also changes the test gdb.mi/mi-reverse.exp to test that the reason shows up correctly. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29260
2023-01-01Update copyright year range in header of all files managed by GDBJoel Brobecker27-27/+27
This commit is the result of running the gdb/copyright.py script, which automated the update of the copyright year range for all source files managed by the GDB project to be updated to include year 2023.
2022-12-19Use bool constants for value_print_optionsTom Tromey2-5/+5
This changes the uses of value_print_options to use 'true' and 'false' rather than integers.
2022-12-19Remove MI version 1Tom Tromey5-50/+3
MI version 1 is long since obsolete. Several years ago, I filed PR mi/23170 for this. I think it's finally time to remove this. Any users of MI 1 can and should upgrade to a newer version. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23170
2022-12-15gdb: remove static buffer in command_line_inputSimon Marchi1-1/+1
[I sent this earlier today, but I don't see it in the archives. Resending it through a different computer / SMTP.] The use of the static buffer in command_line_input is becoming problematic, as explained here [1]. In short, with this patch [2] that attempt to fix a post-hook bug, when running gdb.base/commands.exp, we hit a case where we read a "define" command line from a script file using command_command_line_input. The command line is stored in command_line_input's static buffer. Inside the define command's execution, we read the lines inside the define using command_line_input, which overwrites the define command, in command_line_input's static buffer. After the execution of the define command, execute_command does a command look up to see if a post-hook is registered. For that, it uses a now stale pointer that used to point to the define command, in the static buffer, causing a use-after-free. Note that the pointer in execute_command points to the dynamically-allocated buffer help by the static buffer in command_line_input, not to the static object itself, hence why we see a use-after-free. Fix that by removing the static buffer. I initially changed command_line_input and other related functions to return an std::string, which is the obvious but naive solution. The thing is that some callees don't need to return an allocated string, so this this an unnecessary pessimization. I changed it to passing in a reference to an std::string buffer, which the callee can use if it needs to return dynamically-allocated content. It fills the buffer and returns a pointers to the C string inside. The callees that don't need to return dynamically-allocated content simply don't use it. So, it started with modifying command_line_input as described above, all the other changes derive directly from that. One slightly shady thing is in handle_line_of_input, where we now pass a pointer to an std::string's internal buffer to readline's history_value function, which takes a `char *`. I'm pretty sure that this function does not modify the input string, because I was able to change it (with enough massaging) to take a `const char *`. A subtle change is that we now clear a UI's line buffer using a SCOPE_EXIT in command_line_handler, after executing the command. This was previously done by this line in handle_line_of_input: /* We have a complete command line now. Prepare for the next command, but leave ownership of memory to the buffer . */ cmd_line_buffer->used_size = 0; I think the new way is clearer. [1] https://inbox.sourceware.org/gdb-patches/becb8438-81ef-8ad8-cc42-fcbfaea8cddd@simark.ca/ [2] https://inbox.sourceware.org/gdb-patches/20221213112241.621889-1-jan.vrany@labware.com/ Change-Id: I8fc89b1c69870c7fc7ad9c1705724bd493596300 Reviewed-By: Tom Tromey <tom@tromey.com>
2022-11-30Use ui_file_up in mi_interpTom Tromey2-8/+6
This changes mi_interp to use ui_file_up rather than explicit management. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2022-11-28Don't let tee_file own a streamTom Tromey2-10/+7
Right now, tee_file owns the second stream it writes to. This is done for the convenience of the users. In a subsequent patch, this will no longer be convenient, so this patch moves the responsibility for ownership to the users of tee_file.
2022-11-10gdb/debuginfod: Improve progress updatesAaron Merey2-10/+50
If the download size is known, a progress bar is displayed along with the percentage of completion and the total download size. Downloading separate debug info for /lib/libxyz.so [############ ] 25% (10.01 M) If the download size is not known, a progress indicator is displayed with a ticker ("###") that moves across the screen at a rate of 1 tick every 0.5 seconds. Downloading separate debug info for /lib/libxyz.so [ ### ] If the output stream is not a tty, batch mode is enabled, the screen is too narrow or width has been set to 'unlimited', then only a static description of the download is printed. No bar or ticker is displayed. Downloading separate debug info for /lib/libxyz.so... In any case, if the size of the download is known at the time the description is printed then it will be included in the description. Downloading 10.01 MB separate debug info for /lib/libxyz.so...
2022-10-25gdb: remove spurious spaces after frame_info_ptrSimon Marchi1-1/+1
Fix some whitespace issues introduced with the frame_info_ptr patch. Change-Id: I158d30d8108c97564276c647fc98283ff7b12163
2022-10-19internal_error: remove need to pass __FILE__/__LINE__Pedro Alves3-6/+4
Currently, every internal_error call must be passed __FILE__/__LINE__ explicitly, like: internal_error (__FILE__, __LINE__, "foo %d", var); The need to pass in explicit __FILE__/__LINE__ is there probably because the function predates widespread and portable variadic macros availability. We can use variadic macros nowadays, and in fact, we already use them in several places, including the related gdb_assert_not_reached. So this patch renames the internal_error function to something else, and then reimplements internal_error as a variadic macro that expands __FILE__/__LINE__ itself. The result is that we now should call internal_error like so: internal_error ("foo %d", var); Likewise for internal_warning. The patch adjusts all calls sites. 99% of the adjustments were done with a perl/sed script. The non-mechanical changes are in gdbsupport/errors.h, gdbsupport/gdb_assert.h, and gdb/gdbarch.py. Approved-By: Simon Marchi <simon.marchi@efficios.com> Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
2022-10-10gdb/frame: Add reinflation method for frame_info_ptrBruno Larsen1-0/+2
Currently, despite having a smart pointer for frame_infos, GDB may attempt to use an invalidated frame_info_ptr, which would cause internal errors to happen. One such example has been documented as PR python/28856, that happened when printing frame arguments calls an inferior function. To avoid failures, the smart wrapper was changed to also cache the frame id, so the pointer can be reinflated later. For this to work, the frame-id stuff had to be moved to their own .h file, which is included by frame-info.h. Frame_id caching is done explicitly using the prepare_reinflate method. Caching is done manually so that only the pointers that need to be saved will be, and reinflating has to be done manually using the reinflate method because the get method and the -> operator must not change the internals of the class. Finally, attempting to reinflate when the pointer is being invalidated causes the following assertion errors: check_ptrace_stopped_lwp_gone: assertion `lp->stopped` failed. get_frame_pc: Assertion `frame->next != NULL` failed. As for performance concerns, my personal testing with `time make chec-perf GDB_PERFTEST_MODE=run` showed an actual reduction of around 10% of time running. This commit also adds a testcase that exercises the python/28856 bug with 7 different triggers, run, continue, step, backtrace, finish, up and down. Some of them can seem to be testing the same thing twice, but since this test relies on stale pointers, there is always a chance that GDB got lucky when testing, so better to test extra. Regression tested on x86_64, using both gcc and clang. Approved-by: Tom Tomey <tom@tromey.com>
2022-10-10Change GDB to use frame_info_ptrTom Tromey2-16/+16
This changes GDB to use frame_info_ptr instead of frame_info * The substitution was done with multiple sequential `sed` commands: sed 's/^struct frame_info;/class frame_info_ptr;/' sed 's/struct frame_info \*/frame_info_ptr /g' - which left some issues in a few files, that were manually fixed. sed 's/\<frame_info \*/frame_info_ptr /g' sed 's/frame_info_ptr $/frame_info_ptr/g' - used to remove whitespace problems. The changed files were then manually checked and some 'sed' changes undone, some constructors and some gets were added, according to what made sense, and what Tromey originally did Co-Authored-By: Bruno Larsen <blarsen@redhat.com> Approved-by: Tom Tomey <tom@tromey.com>
2022-10-10Remove frame_id_eqTom Tromey2-3/+3
This replaces frame_id_eq with operator== and operator!=. I wrote this for a version of this series that I later abandoned; but since it simplifies the code, I left this patch in. Approved-by: Tom Tomey <tom@tromey.com>
2022-10-02gdb: update now gdbarch_register_name doesn't return nullptrAndrew Burgess1-14/+6
After the previous few commit, gdbarch_register_name no longer returns nullptr. This commit audits all the calls to gdbarch_register_name and removes any code that checks the result against nullptr. There should be no visible change after this commit.
2022-10-02gdb/mi: new options for -data-disassemble commandAndrew Burgess1-7/+68
Now that the disassembler has two different strategies for laying out the opcode bytes of an instruction (see /r vs /b for the disassemble command), I wanted to add support for this to the MI disassemble command. Currently the -data-disassemble command takes a single 'mode' value, which currently has 6 different values (0 -> 5), 3 of these modes relate to opcode display. So, clearly I should just add an additional 3 modes to handle the new opcode format, right? No, I didn't think that was a great idea either. So, I wonder, could I adjust the -data-disassemble command in a backward compatible way, that would allow GDB to move away from using the mode value altogether? I think we can. In this commit, I propose adding two new options to -data-disassemble, these are: --opcodes none|bytes|display --source Additionally, I will make the mode optional, and default to mode 0 if no mode value is given. Mode 0 is the simplest, no source code, no opcodes disassembly mode. The two new options are only valid for mode 0, if they are used with any other mode then an error is thrown. The --opcodes option can add opcodes to the result, with 'bytes' being equivalent to 'disassemble /b' and 'display' being 'disassemble /r'. The --source option will enable the /s style source code display, this is equivalent to modes 4 and 5. There is no way, using the new command options to get the now deprecated /m style source code display, that is mode 1 and 3. My hope is that new users of the MI will not use the mode at all, and instead will just use the new options to achieve the output they need. Existing MI users can continue to use the mode, and will not need to be updated to use the new options.
2022-10-02gdb/mi: some int to bool conversionAndrew Burgess1-12/+12
Just some simple int to bool conversion in mi_cmd_disassemble. There should be no user visible changes after this commit.
2022-10-02gdb: disassembler opcode display formattingAndrew Burgess1-3/+3
This commit changes the format of 'disassemble /r' to match GNU objdump. Specifically, GDB will now display the instruction bytes in as 'objdump --wide --disassemble' does. Here is an example for RISC-V before this patch: (gdb) disassemble /r 0x0001018e,0x0001019e Dump of assembler code from 0x1018e to 0x1019e: 0x0001018e <call_me+66>: 03 26 84 fe lw a2,-24(s0) 0x00010192 <call_me+70>: 83 25 c4 fe lw a1,-20(s0) 0x00010196 <call_me+74>: 61 65 lui a0,0x18 0x00010198 <call_me+76>: 13 05 85 6a addi a0,a0,1704 0x0001019c <call_me+80>: f1 22 jal 0x10368 <printf> End of assembler dump. And here's an example after this patch: (gdb) disassemble /r 0x0001018e,0x0001019e Dump of assembler code from 0x1018e to 0x1019e: 0x0001018e <call_me+66>: fe842603 lw a2,-24(s0) 0x00010192 <call_me+70>: fec42583 lw a1,-20(s0) 0x00010196 <call_me+74>: 6561 lui a0,0x18 0x00010198 <call_me+76>: 6a850513 addi a0,a0,1704 0x0001019c <call_me+80>: 22f1 jal 0x10368 <printf> End of assembler dump. There are two differences here. First, the instruction bytes after the patch are grouped based on the size of the instruction, and are byte-swapped to little-endian order. Second, after the patch, GDB now uses the bytes-per-line hint from libopcodes to add whitespace padding after the opcode bytes, this means that in most cases the instructions are nicely aligned. It is still possible for a very long instruction to intrude into the disassembled text space. The next example is x86-64, before the patch: (gdb) disassemble /r main Dump of assembler code for function main: 0x0000000000401106 <+0>: 55 push %rbp 0x0000000000401107 <+1>: 48 89 e5 mov %rsp,%rbp 0x000000000040110a <+4>: c7 87 d8 00 00 00 01 00 00 00 movl $0x1,0xd8(%rdi) 0x0000000000401114 <+14>: b8 00 00 00 00 mov $0x0,%eax 0x0000000000401119 <+19>: 5d pop %rbp 0x000000000040111a <+20>: c3 ret End of assembler dump. And after the patch: (gdb) disassemble /r main Dump of assembler code for function main: 0x0000000000401106 <+0>: 55 push %rbp 0x0000000000401107 <+1>: 48 89 e5 mov %rsp,%rbp 0x000000000040110a <+4>: c7 87 d8 00 00 00 01 00 00 00 movl $0x1,0xd8(%rdi) 0x0000000000401114 <+14>: b8 00 00 00 00 mov $0x0,%eax 0x0000000000401119 <+19>: 5d pop %rbp 0x000000000040111a <+20>: c3 ret End of assembler dump. Most instructions are aligned, except for the very long instruction. Notice too that for x86-64 libopcodes doesn't request that GDB group the instruction bytes. This matches the behaviour of objdump. In case the user really wants the old behaviour, I have added a new modifier 'disassemble /b', this displays the instruction byte at a time. For x86-64, which never groups instruction bytes, /b and /r are equivalent, but for RISC-V, using /b gets the old layout back (except that the whitespace for alignment is still present). Consider our original RISC-V example, this time using /b: (gdb) disassemble /b 0x0001018e,0x0001019e Dump of assembler code from 0x1018e to 0x1019e: 0x0001018e <call_me+66>: 03 26 84 fe lw a2,-24(s0) 0x00010192 <call_me+70>: 83 25 c4 fe lw a1,-20(s0) 0x00010196 <call_me+74>: 61 65 lui a0,0x18 0x00010198 <call_me+76>: 13 05 85 6a addi a0,a0,1704 0x0001019c <call_me+80>: f1 22 jal 0x10368 <printf> End of assembler dump. Obviously, this patch is a potentially significant change to the behaviour or /r. I could have added /b with the new behaviour and left /r alone. However, personally, I feel the new behaviour is significantly better than the old, hence, I made /r be what I consider the "better" behaviour. The reason I prefer the new behaviour is that, when I use /r, I almost always want to manually decode the instruction for some reason, and having the bytes displayed in "instruction order" rather than memory order, just makes this easier. The 'record instruction-history' command also takes a /r modifier, and has been modified in the same way as disassemble; /r gets the new behaviour, and /b has been added to retain the old behaviour. Finally, the MI command -data-disassemble, is unchanged in behaviour, this command now requests the raw bytes of the instruction, which is equivalent to the /b modifier. This means that the MI output will remain backward compatible.
2022-09-21gdb: remove TYPE_LENGTHSimon Marchi1-1/+1
Remove the macro, replace all uses with calls to type::length. Change-Id: Ib9bdc954576860b21190886534c99103d6a47afb
2022-09-12Use checked_static_cast in more placesTom Tromey1-5/+2
I went through all the uses of dynamic_cast<> in gdb, looking for ones that could be replaced with checked_static_cast. This patch is the result. Regression tested on x86-64 Fedora 34.
2022-08-31Remove a call to clear_interpreter_hooksTom Tromey1-4/+0
mi_interp::resume does not need to call clear_interpreter_hooks, because this is already done by interp_set.
2022-08-31Remove two unused members from mi_interpTom Tromey2-7/+2
These members of mi_interp aren't used and can be removed.
2022-08-31Use ui_out_redirect_pop in more placesTom Tromey1-21/+8
This changes ui_out_redirect_pop to also perform the redirection, and then updates several sites to use this, rather than explicit redirects.
2022-08-10gdb/mi: fix breakpoint script field outputSimon Marchi6-4/+42
The "script" field, output whenever information about a breakpoint with commands is output, uses wrong MI syntax. $ ./gdb -nx -q --data-directory=data-directory -x script -i mi =thread-group-added,id="i1" =breakpoint-created,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",original-location="main"} =breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",script={"aaa","bbb","ccc"},original-location="main"} (gdb) -break-info ^done,BreakpointTable={nr_rows="1",nr_cols="6",hdr=[{width="7",alignment="-1",col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4",alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},{width="18",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_name="what",colhdr="What"}],body=[bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",script={"aaa","bbb","ccc"},original-location="main"}]} (gdb) In both the =breakpoint-modified and -break-info output, we have: script={"aaa","bbb","ccc"} According to the output syntax [1], curly braces means tuple, and a tuple contains key=value pairs. This looks like it should be a list, but uses curly braces by mistake. This would make more sense: script=["aaa","bbb","ccc"] Fix it, keeping the backwards compatibility by introducing a new MI version (MI4), in exactly the same way as was done when fixing multi-locations breakpoint output in [2]. - Add a fix_breakpoint_script_output uiout flag. MI uiouts will use this flag if the version is >= 4. - Add a fix_breakpoint_script_output_globally variable and the -fix-breakpoint-script-output MI command to set it, if frontends want to use the fixed output for this without using the newer MI version. - When emitting the script field, use list instead of tuple, if we want the fixed output (depending on the two criteria above) - [1] https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Output-Syntax.html#GDB_002fMI-Output-Syntax [2] https://gitlab.com/gnutools/binutils-gdb/-/commit/b4be1b0648608a2578bbed39841c8ee411773edd Change-Id: I7113c6892832c8d6805badb06ce42496677e2242 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24285
2022-07-18Remove cli_out_newTom Tromey1-1/+1
cli_out_new is just a small wrapper around 'new'. This patch removes it, replacing it with uses of 'new' instead.
2022-07-11Always emit =thread-exited notifications, even if silentPedro Alves1-3/+0
[Note: the testcased added by this commit depends on https://sourceware.org/pipermail/gdb-patches/2022-June/190259.html, otherwise GDB just crashes when detaching the core] Currently, in MI, =thread-created are always emitted, like: =thread-group-started,id="i1",pid="195680" =thread-created,id="1",group-id="i1" ... but on teardown, if the target uses exit_inferior_silent, then you'll only see the inferior exit notification (thread-group-exited), no notification for threads. The core target is one of the few targets that use exit_inferior_silent. Here's an example session: -target-select core $corefile =thread-group-started,id="i1",pid="195680" =thread-created,id="1",group-id="i1" ... ^connected,frame=.... (gdb) -target-detach =thread-group-exited,id="i1" ^done (gdb) This imbalance of emitting =thread-created but then not =thread-exited seems off to me. (And, it complicates changes I want to do to centralize emitting thread exit notifications for the CLI, which is why I'm looking at this.) And then, since most other targets use exit_inferior instead of exit_inferior_silent, MI is already emitting =thread-exited notifications when tearing down an inferior, for most targets. This commit makes MI always emit the =thread-exited notifications, even for exit_inferior_silent. Afterwards, when debugging a core, MI outputs: (gdb) -target-detach =thread-exited,id="1",group-id="i1" << new line =thread-group-exited,id="i1" ^done (gdb) Surprisingly, there's no MI testcase debugging a core file. This commit adds the first. Change-Id: I5100501a46f07b6bbad3e04d120c2562a51c93a4
2022-06-22Move mi_interpreter to mi-interp.hTom Tromey3-51/+51
I noticed that touching interps.h caused a lot of recompilation. I tracked this down to mi-common.h including this file. This patch moves the MI interpreter to mi-interp.h, which cuts down on recompilation when modifying interps.h.
2022-06-17Eliminate the two-level data structures behind location_specsPedro Alves1-13/+12
Currently, there's the location_spec hierarchy, and then some location_spec subclasses have their own struct type holding all their data fields. I.e., there is this: location_spec explicit_location_spec linespec_location_spec address_location_spec probe_location_spec and then these separate types: explicit_location linespec_location where: explicit_location_spec has-a explicit_location linespec_location_spec has-a linespec_location This patch eliminates explicit_location and linespec_location, inlining their members in the corresponding location_spec type. The location_spec subclasses were the ones currently defined in location.c, so they are moved to the header. Since the definitions of the classes are now visible, we no longer need location_spec_deleter. Some constructors that are used for cloning location_specs, like: explicit explicit_location_spec (const struct explicit_location *loc) ... were converted to proper copy ctors. In the process, initialize_explicit_location is eliminated, and some functions that returned the "data type behind a locspec", like get_linespec_location are converted to downcast functions, like as_linespec_location_spec. Change-Id: Ia31ccef9382b25a52b00fa878c8df9b8cf2a6c5a
2022-06-17event_location -> location_specPedro Alves1-6/+6
Currently, GDB internally uses the term "location" for both the location specification the user input (linespec, explicit location, or an address location), and for actual resolved locations, like the breakpoint locations, or the result of decoding a location spec to SaLs. This is expecially confusing in the breakpoints module, as struct breakpoint has these two fields: breakpoint::location; breakpoint::loc; "location" is the location spec, and "loc" is the resolved locations. And then, we have a method called "locations()", which returns the resolved locations as range... The location spec type is presently called event_location: /* Location we used to set the breakpoint. */ event_location_up location; and it is described like this: /* The base class for all an event locations used to set a stop event in the inferior. */ struct event_location { and even that is incorrect... Location specs are used for finding actual locations in the program in scenarios that have nothing to do with stop events. E.g., "list" works with location specs. To clean all this confusion up, this patch renames "event_location" to "location_spec" throughout, and then all the variables that hold a location spec, they are renamed to include "spec" in their name, like e.g., "location" -> "locspec". Similarly, functions that work with location specs, and currently have just "location" in their name are renamed to include "spec" in their name too. Change-Id: I5814124798aa2b2003e79496e78f95c74e5eddca
2022-05-20Rename base_breakpoint -> code_breakpointPedro Alves1-2/+2
Even after the previous patches reworking the inheritance of several breakpoint types, the present breakpoint hierarchy looks a bit surprising, as we have "breakpoint" as the superclass, and then "base_breakpoint" inherits from "breakpoint". Like so, simplified: breakpoint base_breakpoint ordinary_breakpoint internal_breakpoint momentary_breakpoint ada_catchpoint exception_catchpoint tracepoint watchpoint catchpoint exec_catchpoint ... The surprising part to me is having "base_breakpoint" being a subclass of "breakpoint". I'm just refering to naming here -- I mean, you'd expect that it would be the top level baseclass that would be called "base". Just flipping the names of breakpoint and base_breakpoint around wouldn't be super great for us, IMO, given we think of every type of *point as a breakpoint at the user visible level. E.g., "info breakpoints" shows watchpoints, tracepoints, etc. So it makes to call the top level class breakpoint. Instead, I propose renaming base_breakpoint to code_breakpoint. The previous patches made sure that all code breakpoints inherit from base_breakpoint, so it's fitting. Also, "code breakpoint" contrasts nicely with a watchpoint also being typically known as a "data breakpoint". After this commit, the resulting hierarchy looks like: breakpoint code_breakpoint ordinary_breakpoint internal_breakpoint momentary_breakpoint ada_catchpoint exception_catchpoint tracepoint watchpoint catchpoint exec_catchpoint ... ... which makes a lot more sense to me. I've left this patch as last in the series in case people want to bikeshed on the naming. "code" has a nice property that it's exactly as many letters as "base", so this patch didn't require any reindentation. :-) Change-Id: Id8dc06683a69fad80d88e674f65e826d6a4e3f66
2022-04-29Remove vtable_breakpoint_opsTom Tromey1-2/+2
There's no need to have vtable_breakpoint_ops any more, so remove it in favor of base_breakpoint_ops.
2022-04-29Convert dprintf to vtable opsTom Tromey1-1/+1
This converts dprintf to use vtable_breakpoint_ops.
2022-04-29Convert ordinary breakpoints to vtable opsTom Tromey1-1/+1
This converts "ordinary" breakpoint to use vtable_breakpoint_ops. Recall that an ordinary breakpoint is both the kind normally created by users, and also a base class used by other classes.
2022-04-29Convert tracepoints to vtable opsTom Tromey1-2/+2
This converts tracepoints to use vtable_breakpoint_ops.
2022-04-27gdb: remove BLOCK_SUPERBLOCK macroSimon Marchi1-1/+1
Replace with equivalent methods. Change-Id: I334a319909a50b5cc5570a45c38c70e10dc00630
2022-04-27gdb: remove BLOCK_FUNCTION macroSimon Marchi1-1/+1
Replace with equivalent methods. Change-Id: I31ec00f5bf85335c8b23d306ca0fe0b84d489101
2022-04-20Replace symbol_symtab with symbol::symtabTom Tromey1-4/+4
This turns symbol_symtab into a method on symbol. It also replaces symbol_set_symtab with a method.
2022-04-11gdb: remove symbol value macrosSimon Marchi1-1/+1
Remove all macros related to getting and setting some symbol value: #define SYMBOL_VALUE(symbol) (symbol)->value.ivalue #define SYMBOL_VALUE_ADDRESS(symbol) \ #define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value) \ #define SYMBOL_VALUE_BYTES(symbol) (symbol)->value.bytes #define SYMBOL_VALUE_COMMON_BLOCK(symbol) (symbol)->value.common_block #define SYMBOL_BLOCK_VALUE(symbol) (symbol)->value.block #define SYMBOL_VALUE_CHAIN(symbol) (symbol)->value.chain #define MSYMBOL_VALUE(symbol) (symbol)->value.ivalue #define MSYMBOL_VALUE_RAW_ADDRESS(symbol) ((symbol)->value.address + 0) #define MSYMBOL_VALUE_ADDRESS(objfile, symbol) \ #define BMSYMBOL_VALUE_ADDRESS(symbol) \ #define SET_MSYMBOL_VALUE_ADDRESS(symbol, new_value) \ #define MSYMBOL_VALUE_BYTES(symbol) (symbol)->value.bytes #define MSYMBOL_BLOCK_VALUE(symbol) (symbol)->value.block Replace them with equivalent methods on the appropriate objects. Change-Id: Iafdab3b8eefc6dc2fd895aa955bf64fafc59ed50
2022-04-07gdb: remove symtab::objfileSimon Marchi1-1/+1
Same idea as previous patch, but for symtab::objfile. I find it clearer without this wrapper, as it shows that the objfile is common to all symtabs of a given compunit. Otherwise, you could think that each symtab (of a given compunit) can have a specific objfile. Change-Id: Ifc0dbc7ec31a06eefa2787c921196949d5a6fcc6
2022-03-29Unify gdb printf functionsTom Tromey4-83/+83
Now that filtered and unfiltered output can be treated identically, we can unify the printf family of functions. This is done under the name "gdb_printf". Most of this patch was written by script.