aboutsummaryrefslogtreecommitdiff
path: root/gdb/mi/mi-main.c
AgeCommit message (Collapse)AuthorFilesLines
2024-08-15Add another constructor to scoped_restore_current_languageTom Tromey1-4/+1
While working on something else, I noticed that this is relatively common: scoped_restore_current_language save; set_language (something); This patch adds a second constructor to scoped_restore_current_language to simplify this idiom. Reviewed-By: Tom de Vries <tdevries@suse.de>
2024-06-07gdb: make progspace::exec_filename private, add getter / setterSimon Marchi1-5/+2
Just like the title says... I think this makes things a bit clearer, for instance where the exec filename is set. It also makes the read call sites a bit nicer, avoiding the `.get ()`. Change-Id: If8b58ae8f6270c8a34b868f6ca06128c6671ea3c 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-22gdb: move store/extract integer functions to extract-store-integer.{c,h}Simon Marchi1-0/+1
Move the declarations out of defs.h, and the implementations out of findvar.c. I opted for a new file, because this functionality of converting integers to bytes and vice-versa seems a bit to generic to live in findvar.c. Change-Id: I524858fca33901ee2150c582bac16042148d2251 Approved-By: John Baldwin <jhb@FreeBSD.org>
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-20gdb: pass frames as `const frame_info_ptr &`Simon Marchi1-2/+2
We currently pass frames to function by value, as `frame_info_ptr`. This is somewhat expensive: - the size of `frame_info_ptr` is 64 bytes, which is a bit big to pass by value - the constructors and destructor link/unlink the object in the global `frame_info_ptr::frame_list` list. This is an `intrusive_list`, so it's not so bad: it's just assigning a few points, there's no memory allocation as if it was `std::list`, but still it's useless to do that over and over. As suggested by Tom Tromey, change many function signatures to accept `const frame_info_ptr &` instead of `frame_info_ptr`. Some functions reassign their `frame_info_ptr` parameter, like: void the_func (frame_info_ptr frame) { for (; frame != nullptr; frame = get_prev_frame (frame)) { ... } } I wondered what to do about them, do I leave them as-is or change them (and need to introduce a separate local variable that can be re-assigned). I opted for the later for consistency. It might not be clear why some functions take `const frame_info_ptr &` while others take `frame_info_ptr`. Also, if a function took a `frame_info_ptr` because it did re-assign its parameter, I doubt that we would think to change it to `const frame_info_ptr &` should the implementation change such that it doesn't need to take `frame_info_ptr` anymore. It seems better to have a simple rule and apply it everywhere. Change-Id: I59d10addef687d157f82ccf4d54f5dde9a963fd0 Approved-By: Andrew Burgess <aburgess@redhat.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-14gdb: change value_of_register and value_of_register_lazy to take the next frameSimon Marchi1-1/+2
Some functions related to the handling of registers in frames accept "this frame", for which we want to read or write the register values, while other functions accept "the next frame", which is the frame next to that. The later is needed because we sometimes need to read register values for a frame that does not exist yet (usually when trying to unwind that frame-to-be). value_of_register and value_of_register_lazy both take "this frame", even if what they ultimately want internally is "the next frame". This is annoying if you are in a spot that currently has "the next frame" and need to call one of these functions (which happens later in this series). You need to get the previous frame only for those functions to get the next frame again. This is more manipulations, more chances of mistake. I propose to change these functions (and a few more functions in the subsequent patches) to operate on "the next frame". Things become a bit less awkward when all these functions agree on which frame they take. So, in this patch, change value_of_register_lazy and value_of_register to take "the next frame" instead of "this frame". This adds a lot of get_next_frame_sentinel_okay, but if we convert the user registers API to also use "the next frame" instead of "this frame", it will get simple again. Change-Id: Iaa24815e648fbe5ae3c214c738758890a91819cd Reviewed-By: John Baldwin <jhb@FreeBSD.org>
2023-11-21gdb: Replace gdb::optional with std::optionalLancelot Six1-6/+6
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-21gdb: Use C++17's std::make_unique instead of gdb::make_uniqueLancelot Six1-1/+1
gdb::make_unique is a wrapper around std::make_unique when compiled with C++17. Now that C++17 is required, use std::make_unique directly in the codebase, and remove gdb::make_unique. Change-Id: I80b615e46e4b7c097f09d78e579a9bdce00254ab Approved-By: Tom Tromey <tom@tromey.com> Approved-By: Pedro Alves <pedro@palves.net
2023-11-17gdb: remove get_current_regcacheSimon Marchi1-2/+1
Remove get_current_regcache, inlining the call to get_thread_regcache in callers. When possible, pass the right thread_info object known from the local context. Otherwise, fall back to passing `inferior_thread ()`. This makes the reference to global context bubble up one level, a small step towards the long term goal of reducing the number of references to global context (or rather, moving those references as close as possible to the top of the call tree). No behavior change expected. Change-Id: Ifa6980c88825d803ea586546b6b4c633c33be8d6
2023-10-10gdb: remove target_gdbarchSimon Marchi1-1/+1
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-09-20Remove explanatory comments from includesTom Tromey1-1/+1
I noticed a comment by an include and remembered that I think these don't really provide much value -- sometimes they are just editorial, and sometimes they are obsolete. I think it's better to just remove them. Tested by rebuilding. Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-09-07gdb/mi: make current_token a field of mi_interpSimon Marchi1-11/+16
Following the commit f818c32ba459 ("gdb/mi: fix ^running record with multiple MI interpreters"), I thought it would make sense to make current_token a field of mi_interp. This variable contains the token of the currently handled MI command, like the 222 in: 222-exec-continue I didn't find any bug related to that, it's just a "that seems nicer" cleanup, since the current token is a fundamentally per-interp thing. mi_execute_command needs a check similar to what we already have in mi_cmd_gdb_exit: when invoked from Python's gdb.execute_mi, the current interpreter is not an mi_interp. When using the Python gdb.execute_mi function, there is no such concept of token, so we can just skip that. There should be no user-visible change. Change-Id: Ib52b3c0cba4b7c9d805b432c809692a86e4945ad Approved-By: Tom Tromey <tom@tromey.com>
2023-08-23gdb: remove mi_parse::make functionsAndrew Burgess1-1/+1
Remove the static mi_parse::make functions, and instead use the mi_parse constructor. This is a partial revert of the commit: commit fde3f93adb50c9937cd2e1c93561aea2fd167156 Date: Mon Mar 20 10:56:55 2023 -0600 Introduce "static constructor" for mi_parse which introduced the mi_parse::make functions, though after discussion on the list the reasons for seem to have been lost[1]. Given there are no test regressions when moving back to using the constructors, I propose we should do that for now. There should be no user visible changes after this commit. [1] https://inbox.sourceware.org/gdb-patches/20230404-dap-loaded-sources-v2-5-93f229095e03@adacore.com/ Approved-By: Tom Tromey <tom@tromey.com>
2023-08-23gdb: have mi_out_new return std::unique_ptrAndrew Burgess1-1/+1
Have the mi_out_new function return a std::unique_ptr instead of a raw pointer. Update the two uses of mi_out_new. There should be no user visible changes after this commit. Approved-By: Tom Tromey <tom@tromey.com>
2023-08-17gdb: add inferior-specific breakpointsAndrew Burgess1-2/+16
This commit extends the breakpoint mechanism to allow for inferior specific breakpoints (but not watchpoints in this commit). As GDB gains better support for multiple connections, and so for running multiple (possibly unrelated) inferiors, then it is not hard to imagine that a user might wish to create breakpoints that apply to any thread in a single inferior. To achieve this currently, the user would need to create a condition possibly making use of the $_inferior convenience variable, which, though functional, isn't the most user friendly. This commit adds a new 'inferior' keyword that allows for the creation of inferior specific breakpoints. Inferior specific breakpoints are automatically deleted when the associated inferior is removed from GDB, this is similar to how thread-specific breakpoints are deleted when the associated thread is deleted. Watchpoints are already per-program-space, which in most cases mean watchpoints are already inferior specific. There is a small window where inferior-specific watchpoints might make sense, which is after a vfork, when two processes are sharing the same address space. However, I'm leaving that as an exercise for another day. For now, attempting to use the inferior keyword with a watchpoint will give an error, like this: (gdb) watch a8 inferior 1 Cannot use 'inferior' keyword with watchpoints A final note on the implementation: currently, inferior specific breakpoints, like thread-specific breakpoints, are inserted into every inferior, GDB then checks once the inferior stops if we are in the correct thread or inferior, and resumes automatically if we stopped in the wrong thread/inferior. An obvious optimisation here is to only insert breakpoint locations into the specific program space (which mostly means inferior) that contains either the inferior or thread we are interested in. This would reduce the number times GDB has to stop and then resume again in a multi-inferior setup. I have a series on the mailing list[1] that implements this optimisation for thread-specific breakpoints. Once this series has landed I'll update that series to also handle inferior specific breakpoints in the same way. For now, inferior specific breakpoints are just slightly less optimal, but this is no different to thread-specific breakpoints in a multi-inferior debug session, so I don't see this as a huge problem. [1] https://inbox.sourceware.org/gdb-patches/cover.1685479504.git.aburgess@redhat.com/
2023-07-07Remove unchecked casts to mi_interpTom Tromey1-18/+30
Simon noticed a crash that could be caused via new Python gdb.execute_mi function. Looking into this, I found a few unchecked casts to mi_interp, like: - struct mi_interp *mi = (struct mi_interp *) command_interp (); This patch replaces all such casts with safer variants. For -gdb-exit and mi_load_progress, I chose to have the functions simply not generate any output. It didn't seem useful to do so. Some casts I eliminated by adding a parameter to a function. Then, in mi_execute_command, I changed the code to use gdb::checked_static_cast. This is appropriate because this particular overload can only be called by the MI interpreter. There does not seem to be a very good way to test -gdb-exit. Regression tested on x86-64 Fedora 36.
2023-06-20Use unique_xmalloc_ptr for mi_parse::commandTom Tromey1-4/+5
This changes mi_parse::command to be a unique_xmalloc_ptr and fixes up all the uses. This avoids some manual memory management. std::string is not used here due to how the Python API works -- this approach avoids an extra copy there. Reviewed-by: Keith Seitz <keiths@redhat.com>
2023-06-20Use std::string for MI tokenTom Tromey1-11/+10
This changes the MI "token" to be a std::string, removing some manual memory management. It also makes current_token 'const' in order to support this change. Reviewed-by: Keith Seitz <keiths@redhat.com>
2023-05-30gdb: add interp::on_user_selected_context_changed methodSimon Marchi1-1/+1
Same as previous patches, but for user_selected_context_changed. Change-Id: I40de15be897671227d4bcf3e747f0fd595f0d5be
2023-05-29gdb/mi: fix ^running record with multiple MI interpretersSimon Marchi1-10/+4
I stumbled on the mi_proceeded and running_result_record_printed globals, which are shared by all MI interpreter instances (it's unlikely that people use multiple MI interpreter instances, but it's possible). After poking at it, I found this bug: 1. Start GDB in MI mode 2. Add a second MI interpreter with the new-ui command 3. Use -exec-run on the second interpreter This is the output I get on the first interpreter: =thread-group-added,id="i1" ~"Reading symbols from a.out...\n" ~"New UI allocated\n" (gdb) =thread-group-started,id="i1",pid="94718" =thread-created,id="1",group-id="i1" ^running *running,thread-id="all" And this is the output I get on the second intepreter: =thread-group-added,id="i1" (gdb) -exec-run =thread-group-started,id="i1",pid="94718" =thread-created,id="1",group-id="i1" *running,thread-id="all" The problem here is that the `^running` reply to the -exec-run command is printed on the wrong UI. It is printed on the first one, it should be printed on the second (the one on which we sent the -exec-run). What happens under the hood is that captured_mi_execute_command, while executing a command for the second intepreter, clears the running_result_record_printed and mi_proceeded globals. mi_about_to_proceed then sets mi_proceeded. Then, mi_on_resume_1 gets called for the first intepreter first. Since the !running_result_record_printed && mi_proceeded condition is true, it prints a ^running, and sets running_result_record_printed. When mi_on_resume_1 gets called for the second interpreter, running_result_record_printed is already set, so ^running is not printed there. It took me a while to understand the relationship between these two variables. I think that in the end, this is what we want to track: 1. When executing an MI command, take note if that command causes a "proceed". This is done in mi_about_to_proceed. 2. In mi_on_resume_1, if the command indeed caused a "proceed", we want to output a ^running record. And we want to remember that we did, because... 3. Back in captured_mi_execute_command, if we did not output a ^running, we want to output a ^done. Moving those two variables to the mi_interp struture appears to fix it. Only for the interpreter doing the -exec-run command does the running_result_record_printed flag get cleared, and therefore only or that one does the ^running record get printed. Add a new test for this, that does pretty much what the reproducer above shows. Without the fix, the test fails because mi_send_resuming_command_raw never sees the ^running record. Change-Id: I63ea30e6cb61a8e1dd5ef03377e6003381a9209b Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
2023-05-25Make MI commands const-correctTom Tromey1-63/+83
I've had this patch for a while now and figured I'd update it and send it. It changes MI commands to use a "const char * const" for their argv parameter. Regression tested on x86-64 Fedora 36. Acked-By: Simon Marchi <simon.marchi@efficios.com>
2023-05-23Implement gdb.execute_miTom Tromey1-0/+15
This adds a new Python function, gdb.execute_mi, that can be used to invoke an MI command but get the output as a Python object, rather than a string. This is done by implementing a new ui_out subclass that builds a Python object. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=11688 Reviewed-By: Eli Zaretskii <eliz@gnu.org>
2023-05-23Introduce "static constructor" for mi_parseTom Tromey1-1/+1
Change the mi_parse function to be a static method of mi_parse. This lets us remove the 'set_args' setter function.
2023-05-23Use accessor for mi_parse::argsTom Tromey1-1/+1
This changes mi_parse::args to be a private member, retrieved via accessor. It also changes this member to be a std::string. This makes it simpler for a subsequent patch to implement different behavior for argument parsing.
2023-05-04Don't treat references to compound values as "simple".Gareth Rees1-5/+2
SUMMARY The '--simple-values' argument to '-stack-list-arguments' and similar GDB/MI commands does not take reference types into account, so that references to arbitrarily large structures are considered "simple" and printed. This means that the '--simple-values' argument cannot be used by IDEs when tracing the stack due to the time taken to print large structures passed by reference. DETAILS Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals', '-stack-list-variables' and so on) take a PRINT-VALUES argument which may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2). In the '--simple-values' case, the command is supposed to print the name, type, and value of variables with simple types, and print only the name and type of variables with compound types. The '--simple-values' argument ought to be suitable for IDEs that need to update their user interface with the program's call stack every time the program stops. However, it does not take C++ reference types into account, and this makes the argument unsuitable for this purpose. For example, consider the following C++ program: struct s { int v[10]; }; int sum(const struct s &s) { int total = 0; for (int i = 0; i < 10; ++i) total += s.v[i]; return total; } int main(void) { struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } }; return sum(s); } If we start GDB in MI mode and continue to 'sum', the behaviour of '-stack-list-arguments' is as follows: (gdb) -stack-list-arguments --simple-values ^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}] Note that the value of the argument 's' was printed, even though 's' is a reference to a structure, which is not a simple value. See https://github.com/microsoft/MIEngine/pull/673 for a case where this behaviour caused Microsoft to avoid the use of '--simple-values' in their MIEngine debug adapter, because it caused Visual Studio Code to take too long to refresh the call stack in the user interface. SOLUTIONS There are two ways we could fix this problem, depending on whether we consider the current behaviour to be a bug. 1. If the current behaviour is a bug, then we can update the behaviour of '--simple-values' so that it takes reference types into account: that is, a value is simple if it is neither an array, struct, or union, nor a reference to an array, struct or union. In this case we must add a feature to the '-list-features' command so that IDEs can detect that it is safe to use the '--simple-values' argument when refreshing the call stack. 2. If the current behaviour is not a bug, then we can add a new option for the PRINT-VALUES argument, for example, '--scalar-values' (3), that would be suitable for use by IDEs. In this case we must add a feature to the '-list-features' command so that IDEs can detect that the '--scalar-values' argument is available for use when refreshing the call stack. PATCH This patch implements solution (1) as I think the current behaviour of not printing structures, but printing references to structures, is contrary to reasonable expectation. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554
2023-05-01gdb: move struct ui and related things to ui.{c,h}Simon Marchi1-0/+1
I'd like to move some things so they become methods on struct ui. But first, I think that struct ui and the related things are big enough to deserve their own file, instead of being scattered through top.{c,h} and event-top.c. Change-Id: I15594269ace61fd76ef80a7b58f51ff3ab6979bc
2023-05-01Remove evaluate_typeTom Tromey1-1/+1
Like evaluate_expression, evaluate_type is also just a simple wrapper. Removing it makes the code a little nicer.
2023-05-01Remove evaluate_expressionTom Tromey1-2/+2
evaluate_expression is just a little wrapper for a method on expression. Removing it also removes a lot of ugly (IMO) calls to get().
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-13Turn many optimized-out value functions into methodsTom Tromey1-1/+1
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 value_type into methodTom Tromey1-2/+2
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-01-01Update copyright year range in header of all files managed by GDBJoel Brobecker1-1/+1
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 Tromey1-4/+4
This changes the uses of value_print_options to use 'true' and 'false' rather than integers.
2022-12-19Remove MI version 1Tom Tromey1-1/+0
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-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-10Change GDB to use frame_info_ptrTom Tromey1-5/+5
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 Tromey1-1/+1
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-08-10gdb/mi: fix breakpoint script field outputSimon Marchi1-1/+10
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-06-22Move mi_interpreter to mi-interp.hTom Tromey1-1/+1
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-03-29Unify gdb printf functionsTom Tromey1-10/+10
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.
2022-03-29Unify gdb puts functionsTom Tromey1-21/+21
Now that filtered and unfiltered output can be treated identically, we can unify the puts family of functions. This is done under the name "gdb_puts". Most of this patch was written by script.
2022-03-29gdb/mi: fix use after free of frame_info causing spurious notificationsAndrew Burgess1-10/+34
In commit: commit a2757c4ed693cef4ecc4dcdcb2518353eb6b3c3f Date: Wed Mar 16 15:08:22 2022 +0000 gdb/mi: consistently notify user when GDB/MI client uses -thread-select Changes were made to GDB to address some inconsistencies in when notifications are sent from a MI terminal to a CLI terminal (when multiple terminals are in use, see new-ui command). Unfortunately, in order to track when the currently selected frame has changed, that commit grabs a frame_info pointer before and after an MI command has executed, and compares the pointers to see if the frame has changed. This is not safe. If the frame cache is deleted for any reason then the frame_info pointer captured before the command started, is no longer valid, and any comparisons based on that pointer are undefined. This was leading to random test failures for some folk, see: https://sourceware.org/pipermail/gdb-patches/2022-March/186867.html This commit changes GDB so we no longer hold frame_info pointers, but instead store the frame_id and frame_level, this is safe even when the frame cache is flushed.
2022-03-16gdb/mi: consistently notify user when GDB/MI client uses -thread-selectJan Vrany1-9/+42
GDB notifies users about user selected thread changes somewhat inconsistently as mentioned on gdb-patches mailing list here: https://sourceware.org/pipermail/gdb-patches/2022-February/185989.html Consider GDB debugging a multi-threaded inferior with both CLI and GDB/MI interfaces connected to separate terminals. Assuming inferior is stopped and thread 1 is selected, when a thread 2 is selected using '-thread-select 2' command on GDB/MI terminal: -thread-select 2 ^done,new-thread-id="2",frame={level="0",addr="0x00005555555551cd",func="child_sub_function",args=[],file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",fullname="/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c",line="30",arch="i386:x86-64"} (gdb) and on CLI terminal we get the notification (as expected): [Switching to thread 2 (Thread 0x7ffff7daa640 (LWP 389659))] #0 child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30 30 volatile int dummy = 0; However, now that thread 2 is selected, if thread 1 is selected using 'thread-select --thread 1 1' command on GDB/MI terminal terminal: -thread-select --thread 1 1 ^done,new-thread-id="1",frame={level="0",addr="0x0000555555555294",func="main",args=[],file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",fullname="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",line="66",arch="i386:x86-64"} (gdb) but no notification is printed on CLI terminal, despite the fact that user selected thread has changed. The problem is that when `-thread-select --thread 1 1` is executed then thread is switched to thread 1 before mi_cmd_thread_select () is called, therefore the condition "inferior_ptid != previous_ptid" there does not hold. To address this problem, we have to move notification logic up to mi_cmd_execute () where --thread option is processed and notify user selected contents observers there if context changes. However, this in itself breaks GDB/MI because it would cause context notification to be sent on MI channel. This is because by the time we notify, MI notification suppression is already restored (done in mi_command::invoke(). Therefore we had to lift notification suppression logic also up to mi_cmd_execute (). This change in made distinction between mi_command::invoke() and mi_command::do_invoke() unnecessary as all mi_command::invoke() did (after the change) was to call do_invoke(). So this patches removes do_invoke() and moves the command execution logic directly to invoke(). With this change, all gdb.mi tests pass, tested on x86_64-linux. Co-authored-by: Andrew Burgess <aburgess@redhat.com> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20631
2022-03-08gdb/mi: preserve user selected thread and frame when invoking MI commandsJan Vrany1-63/+11
Fix for PR gdb/20684. When invoking MI commands with --thread and/or --frame, the user selected thread and frame was not preserved: (gdb) info thread &"info thread\n" ~" Id Target Id Frame \n" ~"* 1 Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\" main () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60\n" ~" 2 Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n" ~" 3 Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n" ^done (gdb) info frame &"info frame\n" ~"Stack level 0, frame at 0x7fffffffdf90:\n" ~" rip = 0x555555555207 in main (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60); saved rip = 0x7ffff7c5709b\n" ~" source language c.\n" ~" Arglist at 0x7fffffffdf80, args: \n" ~" Locals at 0x7fffffffdf80, Previous frame's sp is 0x7fffffffdf90\n" ~" Saved registers:\n " ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n" ^done (gdb) -stack-info-depth --thread 3 ^done,depth="4" (gdb) info thread &"info thread\n" ~" Id Target Id Frame \n" ~" 1 Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\" main () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60\n" ~" 2 Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n" ~"* 3 Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n" ^done (gdb) info frame &"info frame\n" ~"Stack level 0, frame at 0x7ffff742dee0:\n" ~" rip = 0x555555555169 in child_sub_function (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30); saved rip = 0x555555555188\n" ~" called by frame at 0x7ffff742df00\n" ~" source language c.\n" ~" Arglist at 0x7ffff742ded0, args: \n" ~" Locals at 0x7ffff742ded0, Previous frame's sp is 0x7ffff742dee0\n" ~" Saved registers:\n " ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n" ^done (gdb) This caused problems for frontends that provide access to CLI because UI may silently change the context for CLI commands (as demonstrated above). This commit fixes the problem by restoring thread and frame in mi_cmd_execute (). With this change, there are only two GDB/MI commands that can change user selected context: -thread-select and -stack-select-frame. This allows us to remove all and rather complicated logic of notifying about user selected context change from mi_execute_command (), leaving it to these two commands themselves to notify. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20684
2022-03-07gdb/mi: add --no-connection to MI -add-inferior commandAndrew Burgess1-5/+40
Following on from the previous commit, where the -add-inferior command now uses the same connection as the current inferior, this commit adds a --no-connection option to -add-inferior. This new option matches the existing option of the same name for the CLI version of add-inferior; the new inferior is created with no connection. I've added a new 'connection' field to the MI output of -add-inferior, which includes the connection number and short name. I haven't included the longer description field, this is the MI after all. My expectation would be that if the frontend wanted to display all the connection details then this would be looked up from 'info connection' (or the MI equivalent if/when such a command is added). The existing -add-inferior tests are updated, as are the docs.
2022-03-07gdb/mi: fix regression in mi -add-inferior commandUmair Sair1-0/+4
Prior to the multi-target support commit: commit 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 Date: Fri Jan 10 20:06:08 2020 +0000 Multi-target support When a new inferior was added using the MI -add-inferior command, the new inferior would be using the same target as all the other inferiors. This makes sense, GDB only supported a single target stack at a time. After the above commit, each inferior has its own target stack. To maintain backward compatibility, for the CLI add-inferior command, when a new inferior is added the above commit has the new inferior inherit a copy of the target stack from the current inferior. Unfortunately, this same backward compatibility is missing for the MI. This commit fixes this oversight. Now, when the -add-inferior MI command is used, the new inferior will inherit a copy of the target stack from the current inferior.