summaryrefslogtreecommitdiff
path: root/gdb/infcmd.c
Commit message (Collapse)AuthorAgeFilesLines
* gdb: add some additional thread status debug outputAndrew Burgess2022-05-031-11/+8
| | | | | | | | | | | | | | | | | While working on this patch: https://sourceware.org/pipermail/gdb-patches/2022-January/185109.html I found it really useful to print the executing/resumed status of all threads (or all threads in a particular inferior) at various places (e.g. when a new inferior is started, when GDB attaches, etc). This debug was originally part of the above patch, but I wanted to rewrite this as a separate patch and move the code into a new function in infrun.h, which is what this patch does. Unless 'set debug infrun on' is in effect, then there should be no user visible changes after this commit.
* gdb: remove BLOCK_ENTRY_PC macroSimon Marchi2022-04-271-1/+1
| | | | | | Replace with equivalent method. Change-Id: I0e033095e7358799930775e61028b48246971a7d
* gdb: remove BLOCK_{START,END} macrosSimon Marchi2022-04-271-2/+2
| | | | | | Replace with equivalent methods. Change-Id: I10a6c8a2a86462d9d4a6a6409a3f07a6bea66310
* Replace symbol_objfile with symbol::objfileTom Tromey2022-04-201-1/+1
| | | | | | | This turns symbol_objfile into a method on symbol.
* gdb: remove symbol value macrosSimon Marchi2022-04-111-3/+3
| | | | | | | | | | | | | | | | | | | | | | | 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
* gdb: move struct reggroup into reggroups.h headerAndrew Burgess2022-04-071-1/+1
| | | | | | | | | Move 'struct reggroup' into the reggroups.h header. Remove the reggroup_name and reggroup_type accessor functions, and just use the name/type member functions within 'struct reggroup', update all uses of these removed functions. There should be no user visible changes after this commit.
* gdb: remove reggroup_next and reggroup_prevAndrew Burgess2022-04-071-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a new function gdbarch_reggroups that returns a reference to a vector containing all the reggroups for an architecture. Make use of this function throughout GDB instead of the existing reggroup_next and reggroup_prev functions. Finally, delete the reggroup_next and reggroup_prev functions. Most of these changes are pretty straight forward, using range based for loops instead of the old style look using reggroup_next. There are two places where the changes are less straight forward. In gdb/python/py-registers.c, the register group iterator needed to change slightly. As the iterator is tightly coupled to the gdbarch, I just fetch the register group vector from the gdbarch when needed, and use an index counter to find the next item from the vector when needed. In gdb/tui/tui-regs.c the tui_reg_next and tui_reg_prev functions are just wrappers around reggroup_next and reggroup_prev respectively. I've just inlined the logic of the old functions into the tui functions. As the tui function had its own special twist (wrap around behaviour) I think this is OK. There should be no user visible changes after this commit.
* gdb/infrun: add reason parameter to stop_all_threadsSimon Marchi2022-03-311-1/+1
| | | | | | | | | Add a "reason" parameter, only used to show in debug messages what is the reason for stopping all threads. This helped me understand the debug logs while adding some new uses of stop_all_threads, so I am proposing to merge it. Change-Id: I66c8c335ebf41836a7bc3d5fe1db92c195f65e55
* Unify gdb printf functionsTom Tromey2022-03-291-51/+51
| | | | | | | | | 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.
* Unify gdb puts functionsTom Tromey2022-03-291-14/+14
| | | | | | | | | 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.
* Remove some uses of printf_unfilteredTom Tromey2022-03-291-5/+5
| | | | | | | | | | | | | | | A number of spots call printf_unfiltered only because they are in code that should not be interrupted by the pager. However, I believe these cases are all handled by infrun's blanket ban on paging, and so can be converted to the default (_filtered) API. After this patch, I think all the remaining _unfiltered calls are ones that really ought to be. A few -- namely in complete_command -- could be replaced by a scoped assignment to pagination_enabled, but for the remainder, the code seems simple enough like this.
* Add more filename stylingTom Tromey2022-02-281-1/+2
| | | | | | | I found a few spots where filename styling ought to be applied, but is not.
* Enable async mode in the target in attach_cmd.John Baldwin2022-02-221-0/+4
| | | | | If the attach target supports async mode, enable it after the attach target's ::attach method returns.
* gdb: Respect the DW_CC_nocall attributeLancelot SIX2022-02-151-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It is possible for a compiler to optimize a function in a such ways that the function does not follow the calling convention of the target. In such situation, the compiler can use the DW_AT_calling_convention attribute with the value DW_CC_nocall to tell the debugger that it is unsafe to call the function. The DWARF5 standard states, in 3.3.1.1: > If the value of the calling convention attribute is the constant > DW_CC_nocall, the subroutine does not obey standard calling > conventions, and it may not be safe for the debugger to call this > subroutine. Non standard calling convention can affect GDB's assumptions in multiple ways, including how arguments are passed to the function, how values are returned, and so on. For this reason, it is unsafe for GDB to try to do the following operations on a function with marked with DW_CC_nocall: - call / print an expression requiring the function to be evaluated, - inspect the value a function returns using the 'finish' command, - force the value returned by a function using the 'return' command. This patch ensures that if a command which relies on GDB's knowledge of the target's calling convention is used on a function marked nocall, GDB prints an appropriate message to the user and does not proceed with the operation which is unreliable. Note that it is still possible for someone to use a vendor specific value for the DW_AT_calling_convention attribute for example to indicate the use of an alternative calling convention. This commit does not prevent this, and target dependent code can be adjusted if one wanted to support multiple calling conventions. Tested on x86_64-Linux, with no regression observed. Change-Id: I72970dae68234cb83edbc0cf71aa3d6002a4a540
* gdb: add a symbol* argument to get_return_valueLancelot SIX2022-02-151-6/+5
| | | | | | | | | | | | | | | | Add an argument to the get_return_value function to indicate the symbol of the function the debuggee is returning from. This will be used by the following patch. Since the function return type can be deduced from the symbol remove the value_type argument which becomes redundant. No user visible change after this patch. Tested on x86_64-linux. Change-Id: Idf1279f1f7199f5022738a6679e0fa63fbd22edc Co-authored-by: Simon Marchi <simon.marchi@polymtl.ca>
* gdb: fix until behavior with trailing !is_stmt linesBruno Larsen2022-02-111-0/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using the command "until", it is expected that GDB will exit a loop if the current instruction is the last one related to that loop. However, if there were trailing non-statement instructions, "until" would just behave as "next". This was noticeable in clang-compiled code, but might happen with gcc-compiled as well. PR gdb/17315 relates to this problem, as running gdb.base/watchpoint.exp with clang would fail for this reason. To better understand this issue, consider the following source code, with line numbers marked on the left: 10: for (i = 0; i < 10; ++i) 11: loop_body (); 12: other_stuff (); If we transform this to pseudo-assembler, and generate a line table, we could end up with something like this: Address | Pseudo-Assembler | Line | Is-Statement? 0x100 | i = 0 | 10 | Yes 0x104 | loop_body () | 11 | Yes 0x108 | i = i + 1 | 10 | Yes 0x10c | if (i < 10): | 10 | No 0x110 | goto 0x104 | 10 | No 0x114 | other_stuff () | 12 | Yes Notice the two non-statement instructions at the end of the loop. The problem is that when we reach address 0x108 and use 'until', hoping to leave the loop, GDB sets up a stepping range that runs from the start of the function (0x100 in our example) to the end of the current line table entry, that is 0x10c in our example. GDB then starts stepping forward. When 0x10c is reached GDB spots that we have left the stepping range, that the new location is not a statement, and that the new location is associated with the same source line number as the previous stepping range. GDB then sets up a new stepping range that runs from 0x10c to 0x114, and continues stepping forward. Within that stepping range the inferior hits the goto (at 0x110) and loops back to address 0x104. At 0x104 GDB spots that we have left the previous stepping range, that the new address is marked as a statement, and that the new address is for a different source line. As a result, GDB stops and returns control to the user. This is not what the user was expecting, they expected GDB to exit the loop. The fix proposed in this patch, is that, when the user issues the 'until' command, and GDB sets up the initial stepping range, GDB will check subsequent SALs (symtab_and_lines) to see if they are non-statements associated with the same line number. If they are then the end of the initial stepping range is extended to the end of the non-statement SALs. In our example above, the user is at 0x108 and uses 'until', GDB now sets up a stepping range from the start of the function 0x100 to 0x114, the first address associated with a different line. Now as GDB steps around the loop it never leaves the initial stepping range. It is only when GDB exits the loop that we leave the stepping range, and the stepping finishes at address 0x114. This patch also adds a test case that can be run with gcc to test that this functionality is not broken in the future. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17315
* gdb: make thread_info::m_thread_fsm a std::unique_ptrLancelot SIX2022-02-071-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | While working on function calls, I realized that the thread_fsm member of struct thread_info is a raw pointer to a resource it owns. This commit changes the type of the thread_fsm member to a std::unique_ptr in order to signify this ownership relationship and slightly ease resource management (no need to manually call delete). To ensure consistent use, the field is made a private member (m_thread_fsm). The setter method (set_thread_fsm) can then check that it is incorrect to associate a FSM to a thread_info object if another one is already in place. This is ensured by an assertion. The function run_inferior_call takes an argument as a pointer to a call_thread_fsm and installs it in it in a thread_info instance. Also change this function's signature to accept a unique_ptr in order to signify that the ownership of the call_thread_fsm is transferred during the call. No user visible change expected after this commit. Tested on x86_64-linux with no regression observed. Change-Id: Ia1224f72a4afa247801ce6650ce82f90224a9ae8
* gdb: remove SYMBOL_TYPE macroSimon Marchi2022-02-061-2/+2
| | | | | | | Add a getter and a setter for a symbol's type. Remove the corresponding macro and adjust all callers. Change-Id: Ie1a137744c5bfe1df4d4f9ae5541c5299577c8de
* gdb: remove SYMBOL_CLASS macro, add getterSimon Marchi2022-02-061-1/+1
| | | | Change-Id: I83211d5a47efc0564386e5b5ea4a29c00b1fd46a
* Reduce explicit use of gdb_stdoutTom Tromey2022-01-251-1/+1
| | | | | | | | | | | | | | | | In an earlier version of the pager rewrite series, it was important to audit unfiltered output calls to see which were truly necessary. This is no longer necessary, but it still seems like a decent cleanup to change calls to avoid explicitly passing gdb_stdout. That is, rather than using something like fprintf_unfiltered with gdb_stdout, the code ought to use plain printf_unfiltered instead. This patch makes this change. I went ahead and converted all the _filtered calls I could find, as well, for the same clarity.
* gdb: add some extra debug information to attach_commandAndrew Burgess2022-01-131-0/+12
| | | | | | | | | | While working on another patch I wanted to add some extra debug information to the attach_command function. This required me to add a new function to convert the thread_info::state variable to a string. The new debug might be useful to others, and the state to string function might be useful in other locations, so I thought I'd merge it.
* Use filtered output in kill commandTom Tromey2022-01-051-4/+3
| | | | | | | | | | This changes the kill command to use filtered output. I split this one into its own patch because, out of an abundance of caution, I changed the function to call bfd_cache_close_all a bit earlier, in case pagination caused an exception.
* Automatic Copyright Year update after running gdb/copyright.pyJoel Brobecker2022-01-011-1/+1
| | | | | | | | This commit brings all the changes made by running gdb/copyright.py as per GDB's Start of New Year Procedure. For the avoidance of doubt, all changes in this commits were performed by the script.
* Consistently Use ui_file parameter to show callbacksTom Tromey2021-12-291-3/+3
| | | | | | | | | I happened to notice that one "show" callback was printing to gdb_stdout rather than to the passed-in ui_file parameter. I went through all such callbacks and fixed them to consistently use the ui_file. Regression tested on x86-64 Fedora 34.
* [gdb/symtab] Support -readnow during rereadTom de Vries2021-12-071-2/+2
| | | | | | | | | | | | | | | | | When running test-case gdb.base/cached-source-file.exp with target board readnow, we run into: ... FAIL: gdb.base/cached-source-file.exp: rerun program (the program exited) ... The problem is that when rereading, the readnow is ignored. Fix this by copying the readnow handling code from symbol_file_add_with_addrs to reread_symbols. Tested on x86_64-linux. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26800
* gdb: introduce a new overload of target_can_async_pAndrew Burgess2021-11-251-1/+1
| | | | | | | | | | | | | | | | | | | There are a few places where we call the target_ops::can_async_p member function directly, instead of using the target_can_async_p wrapper. In some of these places this is because we need to ask before the target has been pushed, and in another location (in target.c) it seems unnecessary to go through the wrapper when we are already in target.c code. However, in the next commit I'd like to hoist some common checks out of target specific code into target.c. To achieve this, in this commit, I introduce a new overload of target_can_async_p which takes a target_ops pointer, and calls the ::can_async_p method directly. I then make use of the new overload where appropriate. There should be no user visible changes after this commit.
* gdb: tweak scoped_disable_commit_resumed uses when resuming all threads in ↵Simon Marchi2021-11-081-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | non-stop When doing "continue -a" in non-stop mode, each thread is individually resumed while the commit resumed state is enabled. This forces the target to commit each resumption immediately, instead of being able to batch things. The reason is that there is no scoped_disable_commit_resumed around the loop over threads in continue_1, when "non_stop && all_threads" is true. Since the proceed function is called once for each thread, the scoped_disable_commit_resumed in proceed therefore forces commit-resumed between each thread resumption. Add the necessary scoped_disable_commit_resumed in continue_1 to avoid that. I looked at the MI side of things, the function exec_continue, and found that it was correct. There is a similar iteration over threads, and there is a scoped_disable_commit_resumed at the function scope. This is not wrong, but a bit more than we need. The branches that just call continue_1 do not need it, as continue_1 takes care of disabling commit resumed. So, move the scoped_disable_commit_resumed to the inner scope where we iterate on threads and proceed them individually. Here's an example debugging a multi-threaded program attached by gdbserver (debug output trimmed for brevity): $ ./gdb -nx -q --data-directory=data-directory -ex "set non-stop" -ex "tar rem :1234" (gdb) set debug remote (gdb) set debug infrun (gdb) c -a Continuing. [infrun] proceed: enter [infrun] scoped_disable_commit_resumed: reason=proceeding [remote] Sending packet: $vCont;c:p14388.14388#90 [infrun] reset: reason=proceeding [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target remote [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target remote [infrun] proceed: exit [infrun] proceed: enter [infrun] scoped_disable_commit_resumed: reason=proceeding [remote] Sending packet: $vCont;c:p14388.1438a#b9 [infrun] reset: reason=proceeding [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target remote [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target remote [infrun] proceed: exit ... and so on for each thread ... Notice how we send one vCont;c for each thread. With the patch applied, we send a single vCont;c at the end: [infrun] scoped_disable_commit_resumed: reason=continue all threads in non-stop [infrun] proceed: enter [infrun] scoped_disable_commit_resumed: reason=proceeding [infrun] reset: reason=proceeding [infrun] proceed: exit [infrun] clear_proceed_status_thread: Thread 85790.85792 [infrun] proceed: enter [infrun] scoped_disable_commit_resumed: reason=proceeding [infrun] reset: reason=proceeding [infrun] proceed: exit ... proceeding threads individually ... [infrun] reset: reason=continue all threads in non-stop [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target remote [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target remote [remote] Sending packet: $vCont;c#a8 Change-Id: I331dd2473c5aa5114f89854196fed2a8fdd122bb
* gdb: remove bpstat typedef, rename bpstats to bpstatSimon Marchi2021-11-081-2/+2
| | | | | | | | | | | | | I don't find that the bpstat typedef, which hides a pointer, is particularly useful. In fact, it confused me many times, and I just see it as something to remember that adds cognitive load. Also, with C++, we might want to be able to pass bpstats objects by const-reference, not necessarily by pointer. So, remove the bpstat typedef and rename struct bpstats to bpstat (since it represents one bpstat, it makes sense that it is singular). Change-Id: I52e763b6e54ee666a9e045785f686d37b4f5f849
* gdb: change functions returning value contents to use gdb::array_viewSimon Marchi2021-10-251-2/+2
| | | | | | | | | | | | | | | | | | | | | The bug fixed by this [1] patch was caused by an out-of-bounds access to a value's content. The code gets the value's content (just a pointer) and then indexes it with a non-sensical index. This made me think of changing functions that return value contents to return array_views instead of a plain pointer. This has the advantage that when GDB is built with _GLIBCXX_DEBUG, accesses to the array_view are checked, making bugs more apparent / easier to find. This patch changes the return types of these functions, and updates callers to call .data() on the result, meaning it's not changing anything in practice. Additional work will be needed (which can be done little by little) to make callers propagate the use of array_view and reap the benefits. [1] https://sourceware.org/pipermail/gdb-patches/2021-September/182306.html Change-Id: I5151f888f169e1c36abe2cbc57620110673816f3
* gdb, gdbserver: make target_waitstatus safeSimon Marchi2021-10-211-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I stumbled on a bug caused by the fact that a code path read target_waitstatus::value::sig (expecting it to contain a gdb_signal value) while target_waitstatus::kind was TARGET_WAITKIND_FORKED. This meant that the active union field was in fact target_waitstatus::value::related_pid, and contained a ptid. The read signal value was therefore garbage, and that caused GDB to crash soon after. Or, since that GDB was built with ubsan, this nice error message: /home/simark/src/binutils-gdb/gdb/linux-nat.c:1271:12: runtime error: load of value 2686365, which is not a valid value for type 'gdb_signal' Despite being a large-ish change, I think it would be nice to make target_waitstatus safe against that kind of bug. As already done elsewhere (e.g. dynamic_prop), validate that the type of value read from the union matches what is supposed to be the active field. - Make the kind and value of target_waitstatus private. - Make the kind initialized to TARGET_WAITKIND_IGNORE on target_waitstatus construction. This is what most users appear to do explicitly. - Add setters, one for each kind. Each setter takes as a parameter the data associated to that kind, if any. This makes it impossible to forget to attach the associated data. - Add getters, one for each associated data type. Each getter validates that the data type fetched by the user matches the wait status kind. - Change "integer" to "exit_status", "related_pid" to "child_ptid", just because that's more precise terminology. - Fix all users. That last point is semi-mechanical. There are a lot of obvious changes, but some less obvious ones. For example, it's not possible to set the kind at some point and the associated data later, as some users did. But in any case, the intent of the code should not change in this patch. This was tested on x86-64 Linux (unix, native-gdbserver and native-extended-gdbserver boards). It was built-tested on x86-64 FreeBSD, NetBSD, MinGW and macOS. The rest of the changes to native files was done as a best effort. If I forgot any place to update in these files, it should be easy to fix (unless the change happens to reveal an actual bug). Change-Id: I0ae967df1ff6e28de78abbe3ac9b4b2ff4ad03b7
* gdb: make string-like set show commands use std::string variableSimon Marchi2021-10-031-8/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | String-like settings (var_string, var_filename, var_optional_filename, var_string_noescape) currently take a pointer to a `char *` storage variable (typically global) that holds the setting's value. I'd like to "mordernize" this by changing them to use an std::string for storage. An obvious reason is that string operations on std::string are often easier to write than with C strings. And they avoid having to do any manual memory management. Another interesting reason is that, with `char *`, nullptr and an empty string often both have the same meaning of "no value". String settings are initially nullptr (unless initialized otherwise). But when doing "set foo" (where `foo` is a string setting), the setting now points to an empty string. For example, solib_search_path is nullptr at startup, but points to an empty string after doing "set solib-search-path". This leads to some code that needs to check for both to check for "no value". Or some code that converts back and forth between NULL and "" when getting or setting the value. I find this very error-prone, because it is very easy to forget one or the other. With std::string, we at least know that the variable is not "NULL". There is only one way of representing an empty string setting, that is with an empty string. I was wondering whether the distinction between NULL and "" would be important for some setting, but it doesn't seem so. If that ever happens, it would be more C++-y and self-descriptive to use optional<string> anyway. Actually, there's one spot where this distinction mattered, it's in init_history, for the test gdb.base/gdbinit-history.exp. init_history sets the history filename to the default ".gdb_history" if it sees that the setting was never set - if history_filename is nullptr. If history_filename is an empty string, it means the setting was explicitly cleared, so it leaves it as-is. With the change to std::string, this distinction doesn't exist anymore. This can be fixed by moving the code that chooses a good default value for history_filename to _initialize_top. This is ran before -ex commands are processed, so an -ex command can then clear that value if needed (what gdb.base/gdbinit-history.exp tests). Another small improvement, in my opinion is that we can now easily give string parameters initial values, by simply initializing the global variables, instead of xstrdup-ing it in the _initialize function. In Python and Guile, when registering a string-like parameter, we allocate (with new) an std::string that is owned by the param_smob (in Guile) and the parmpy_object (in Python) objects. This patch started by changing all relevant add_setshow_* commands to take an `std::string *` instead of a `char **` and fixing everything that failed to build. That includes of course all string setting variable and their uses. string_option_def now uses an std::string also, because there's a connection between options and settings (see add_setshow_cmds_for_options). The add_path function in source.c is really complex and twisted, I'd rather not try to change it to work on an std::string right now. Instead, I added an overload that copies the std:string to a `char *` and back. This means more copying, but this is not used in a hot path at all, so I think it is acceptable. Change-Id: I92c50a1bdd8307141cdbacb388248e4e4fc08c93 Co-authored-by: Lancelot SIX <lsix@lancelotsix.com>
* gdb: make thread_suspend_state::stop_pc optionalAndrew Burgess2021-09-081-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently the stop_pc field of thread_suspect_state is a CORE_ADDR and when we want to indicate that there is no stop_pc available we set this field back to a special value. There are actually two special values used, in post_create_inferior the stop_pc is set to 0. This is a little unfortunate, there are plenty of embedded targets where 0 is a valid pc value. The more common special value for stop_pc though, is set in thread_info::set_executing, where the value (~(CORE_ADDR) 0) is used. This commit changes things so that the stop_pc is instead a gdb::optional. We can now explicitly reset the field to an uninitialised state, we also have asserts that we don't read the stop_pc when its in an uninitialised state (both in gdbsupport/gdb_optional.h, when compiling with _GLIBCXX_DEBUG defined, and in thread_info::stop_pc). One situation where a thread will not have a stop_pc value is when the thread is stopped as a consequence of GDB being in all stop mode, and some other thread stopped at an interesting event. When GDB brings all the other threads to a stop those other threads will not have a stop_pc set (thus avoiding an unnecessary read of the pc register). Previously, when GDB passed through handle_one (in infrun.c) the threads executing flag was set to false and the stop_pc field was left unchanged, i.e. it would (previous) have been left as ~0. Now, handle_one leaves the stop_pc with no value. This caused a problem when we later try to set these threads running again, in proceed() we compare the current pc with the cached stop_pc. If the thread was stopped via handle_one then the stop_pc would have been left as ~0, and the compare (in proceed) would (likely) fail. Now however, this compare tries to read the stop_pc when it has no value and this would trigger an assert. To resolve this I've added thread_info::stop_pc_p() which returns true if the thread has a cached stop_pc. We should only ever call thread_info::stop_pc() if we know that there is a cached stop_pc, however, this doesn't mean that every call to thread_info::stop_pc() needs to be guarded with a call to thread_info::stop_pc_p(), in most cases we know that the thread we are looking at stopped due to some interesting event in that thread, and so, we know that the stop_pc is valid. After running the testsuite I've seen no other situations where stop_pc is read uninitialised. There should be no user visible changes after this commit.
* gdb: make thread_info::executing privateAndrew Burgess2021-09-071-2/+2
| | | | | | | | | | | | | | | | Rename thread_info::executing to thread_info::m_executing, and make it private. Add a new get/set member functions, and convert GDB to make use of these. The only real change of interest in this patch is in thread.c where I have deleted the helper function set_executing_thread, and now just use the new set function thread_info::set_executing. However, the old helper function set_executing_thread included some code to reset the thread's stop_pc, so I moved this code into the new function thread_info::set_executing. However, I don't believe there is anywhere that this results in a change of behaviour, previously the executing flag was always set true through a call to set_executing_thread anyway.
* gdb: make inferior::m_terminal an std::stringSimon Marchi2021-07-231-4/+2
| | | | | | Same idea as the previous patch, but for m_terminal. Change-Id: If9367d5db8c976a4336680adca4ea5bc31ab64d2
* gdb: make inferior::m_cwd an std::stringSimon Marchi2021-07-231-8/+6
| | | | | | | | | | Same idea as the previous patch, but for m_cwd. To keep things consistent across the board, change get_inferior_cwd as well, which is shared with GDBserver. So update the related GDBserver code too. Change-Id: Ia2c047fda738d45f3d18bc999eb67ceb8400ce4e
* gdb: make inferior::m_args an std::stringSimon Marchi2021-07-231-2/+2
| | | | | | | | | With the current code, both a NULL pointer and an empty string can mean "no arguments". We don't need this distinction. Changing to a string has the advantage that there is now a single state for that (an empty string), which makes the code a bit simpler in my opinion. Change-Id: Icdc622820f7869478791dbaa84b4a1c7fec21ced
* gdb: add setter/getter for inferior cwdSimon Marchi2021-07-231-20/+4
| | | | | | | | | Add cwd/set_cwd to the inferior class, remove set_inferior_args. Keep get_inferior_args, because it is used from fork_inferior, in shared code. The cwd could eventually be passed as a parameter eventually, though, I think that would be cleaner. Change-Id: Ifb72ea865d7e6f9a491308f0d5c1595579d8427e
* gdb: add setter/getter for inferior argumentsSimon Marchi2021-07-231-27/+7
| | | | | | | | Add args/set_args to the inferior class, remove the set_inferior_args and get_inferior_args functions, that would just be wrappers around them. Change-Id: If87d52f3402ce08be26c32897ae8915d9f6d1ea3
* gdb: remove inferior::{argc,argv}Simon Marchi2021-07-231-19/+5
| | | | | | | | | | | | | | | | | | | | | | There are currently two states that the inferior args can be stored. The main one is the `args` field, where they are stored as a single string. The other one is the `argc`/`argv` fields. This last one is only used for arguments passed in GDB's command line. And the only outcome is that when get_inferior_args is called, `argc`/`argv` are serialized into `args`. So really, `argc`/`argv` is just a staging area before moving the arguments in `args`. Simplify this by only keeping the `args` field. Change set_inferior_args_vector to immediately serialize the arguments into `args`, work that would be done in get_inferior_args later anyway. The only time where this work would be "wasted" is when the user passes some arguments on the command line, but does not end up running the program. But that just seems unlikely. And it's not that much work. Change-Id: Ica0b9859397c095f6530350c8fb3c36905f2044a
* gdb: un-share set_inferior_cwd declarationSimon Marchi2021-07-231-2/+3
| | | | | | | | | | | | | | The declaration of set_inferior_cwd is currently shared between gdb and gdbserver, in gdbsupport/common-inferior.h. It doesn't need to be, as set_inferior_cwd is not called from common code. Only get_inferior_cwd needs to. The motivation for this is that a future patch will change the prototype of set_inferior_cwd in gdb, and I don't want to change it for gdbserver unnecessarily. I see this as a good cleanup in any case, to reduce to just the essential what is shared between GDB and GDBserver. Change-Id: I3127d27d078f0503ebf5ccc6fddf14f212426a73
* gdb: make thread_info::suspend private, add getters / settersSimon Marchi2021-07-121-16/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A following patch will want to take some action when a pending wait status is set on or removed from a thread. Add a getter and a setter on thread_info for the pending waitstatus, so that we can add some code in the setter later. The thing is, the pending wait status field is in the thread_suspend_state, along with other fields that we need to backup before and restore after the thread does an inferior function call. Therefore, make the thread_suspend_state member private (thread_info::suspend becomes thread_info::m_suspend), and add getters / setters for all of its fields: - pending wait status - stop signal - stop reason - stop pc For the pending wait status, add the additional has_pending_waitstatus and clear_pending_waitstatus methods. I think this makes the thread_info interface a bit nicer, because we now access the fields as: thread->stop_pc () rather than thread->suspend.stop_pc The stop_pc field being in the `suspend` structure is an implementation detail of thread_info that callers don't need to be aware of. For the backup / restore of the thread_suspend_state structure, add save_suspend_to and restore_suspend_from methods. You might wonder why `save_suspend_to`, as opposed to a simple getter like thread_suspend_state &suspend (); I want to make it clear that this is to be used only for backing up and restoring the suspend state, _not_ to access fields like: thread->suspend ()->stop_pc Adding some getters / setters allows adding some assertions. I find that this helps understand how things are supposed to work. Add: - When getting the pending status (pending_waitstatus method), ensure that there is a pending status. - When setting a pending status (set_pending_waitstatus method), ensure there is no pending status. There is one case I found where this wasn't true - in remote_target::process_initial_stop_replies - which needed adjustments to respect that contract. I think it's because process_initial_stop_replies is kind of (ab)using the thread_info::suspend::waitstatus to store some statuses temporarily, for its internal use (statuses it doesn't intent on leaving pending). process_initial_stop_replies pulls out stop replies received during the initial connection using target_wait. It always stores the received event in `evthread->suspend.waitstatus`. But it only sets waitstatus_pending_p, if it deems the event interesting enough to leave pending, to be reported to the core: if (ws.kind != TARGET_WAITKIND_STOPPED || ws.value.sig != GDB_SIGNAL_0) evthread->suspend.waitstatus_pending_p = 1; It later uses this flag a bit below, to choose which thread to make the "selected" one: if (selected == NULL && thread->suspend.waitstatus_pending_p) selected = thread; And ultimately that's used if the user-visible mode is all-stop, so that we print the stop for that interesting thread: /* In all-stop, we only print the status of one thread, and leave others with their status pending. */ if (!non_stop) { thread_info *thread = selected; if (thread == NULL) thread = lowest_stopped; if (thread == NULL) thread = first; print_one_stopped_thread (thread); } But in any case (all-stop or non-stop), print_one_stopped_thread needs to access the waitstatus value of these threads that don't have a pending waitstatus (those that had TARGET_WAITKIND_STOPPED + GDB_SIGNAL_0). This doesn't work with the assertions I've put. So, change the code to only set the thread's wait status if it is an interesting one that we are going to leave pending. If the thread stopped due to a non-interesting event (TARGET_WAITKIND_STOPPED + GDB_SIGNAL_0), don't store it. Adjust print_one_stopped_thread to understand that if a thread has no pending waitstatus, it's because it stopped with TARGET_WAITKIND_STOPPED + GDB_SIGNAL_0. The call to set_last_target_status also uses the pending waitstatus. However, given that the pending waitstatus for the thread may have been cleared in print_one_stopped_thread (and that there might not even be a pending waitstatus in the first place, as explained above), it is no longer possible to do it at this point. To fix that, move the call to set_last_target_status in print_one_stopped_thread. I think this will preserve the existing behavior, because set_last_target_status is currently using the current thread's wait status. And the current thread is the last one for which print_one_stopped_thread is called. So by calling set_last_target_status in print_one_stopped_thread, we'll get the same result. set_last_target_status will possibly be called multiple times, but only the last call will matter. It just means possibly more calls to set_last_target_status, but those are cheap. Change-Id: Iedab9653238eaf8231abcf0baa20145acc8b77a7
* gdb: try to load libthread_db only after reading all shared libraries when ↵Simon Marchi2021-06-081-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | attaching / handling a fork child When trying to attach to a pthread process on a Linux system with glibc 2.33, we get: $ ./gdb -q -nx --data-directory=data-directory -p 1472010 Attaching to process 1472010 [New LWP 1472013] [New LWP 1472014] [New LWP 1472015] Error while reading shared library symbols for /usr/lib/libpthread.so.0: Cannot find user-level thread for LWP 1472015: generic error 0x00007ffff6d3637f in poll () from /usr/lib/libc.so.6 (gdb) When attaching to a process (or handling a fork child, an operation very similar to attaching), GDB reads the shared library list from the process. For each shared library (if "set auto-solib-add" is on), it reads its symbols and calls the "new_objfile" observable. The libthread-db code monitors this observable, and if it sees an objfile named somewhat like "libpthread.so" go by, it tries to load libthread_db.so in the GDB process itself. libthread_db knows how to navigate libpthread's data structures to get information about the existing threads. To locate these data structures, libthread_db calls ps_pglobal_lookup (implemented in proc-service.c), passing in a symbol name and expecting an address in return. Before glibc 2.33, libthread_db always asked for symbols found in libpthread. There was no ordering problem: since we were always trying to load libthread_db in reaction to processing libpthread (and reading in its symbols) and libthread_db only asked symbols from libpthread, the requested symbols could always be found. Starting with glibc 2.33, libthread_db now asks for a symbol name that can be found in /lib/ld-linux-x86-64.so.2 (_rtld_global). And the ordering in which GDB reads the shared libraries from the inferior when attaching is unfortunate, in that libpthread is processed before ld-linux. So when loading libthread_db in reaction to processing libpthread, and libthread_db requests the symbol that is from ld-linux, GDB is not yet able to supply it. That problematic symbol lookup happens in the thread_from_lwp function, when we call td_ta_map_lwp2thr_p, and an exception is thrown at this point: #0 0x00007ffff6681012 in __cxxabiv1::__cxa_throw (obj=0x60e000006100, tinfo=0x555560033b50 <typeinfo for gdb_exception_error>, dest=0x55555d9404bc <gdb_exception_error::~gdb_exception_error()>) at /build/gcc/src/gcc/libstdc++-v3/libsupc++/eh_throw.cc:78 #1 0x000055555e5d3734 in throw_it(return_reason, errors, const char *, typedef __va_list_tag __va_list_tag *) (reason=RETURN_ERROR, error=GENERIC_ERROR, fmt=0x55555f0c5360 "Cannot find user-level thread for LWP %ld: %s", ap=0x7fffffffaae0) at /home/simark/src/binutils-gdb/gdbsupport/common-exceptions.cc:200 #2 0x000055555e5d37d4 in throw_verror (error=GENERIC_ERROR, fmt=0x55555f0c5360 "Cannot find user-level thread for LWP %ld: %s", ap=0x7fffffffaae0) at /home/simark/src/binutils-gdb/gdbsupport/common-exceptions.cc:208 #3 0x000055555e0b0ed2 in verror (string=0x55555f0c5360 "Cannot find user-level thread for LWP %ld: %s", args=0x7fffffffaae0) at /home/simark/src/binutils-gdb/gdb/utils.c:171 #4 0x000055555e5e898a in error (fmt=0x55555f0c5360 "Cannot find user-level thread for LWP %ld: %s") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:43 #5 0x000055555d06b4bc in thread_from_lwp (stopped=0x617000035d80, ptid=...) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:418 #6 0x000055555d07040d in try_thread_db_load_1 (info=0x60c000011140) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:912 #7 0x000055555d071103 in try_thread_db_load (library=0x55555f0c62a0 "libthread_db.so.1", check_auto_load_safe=false) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1014 #8 0x000055555d072168 in try_thread_db_load_from_sdir () at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1091 #9 0x000055555d072d1c in thread_db_load_search () at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1146 #10 0x000055555d07365c in thread_db_load () at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1203 #11 0x000055555d07373e in check_for_thread_db () at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1246 #12 0x000055555d0738ab in thread_db_new_objfile (objfile=0x61300000c0c0) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1275 #13 0x000055555bd10740 in std::__invoke_impl<void, void (*&)(objfile*), objfile*> (__f=@0x616000068d88: 0x55555d073745 <thread_db_new_objfile(objfile*)>) at /usr/include/c++/10.2.0/bits/invoke.h:60 #14 0x000055555bd02096 in std::__invoke_r<void, void (*&)(objfile*), objfile*> (__fn=@0x616000068d88: 0x55555d073745 <thread_db_new_objfile(objfile*)>) at /usr/include/c++/10.2.0/bits/invoke.h:153 #15 0x000055555bce0392 in std::_Function_handler<void (objfile*), void (*)(objfile*)>::_M_invoke(std::_Any_data const&, objfile*&&) (__functor=..., __args#0=@0x7fffffffb4a0: 0x61300000c0c0) at /usr/include/c++/10.2.0/bits/std_function.h:291 #16 0x000055555d3595c0 in std::function<void (objfile*)>::operator()(objfile*) const (this=0x616000068d88, __args#0=0x61300000c0c0) at /usr/include/c++/10.2.0/bits/std_function.h:622 #17 0x000055555d356b7f in gdb::observers::observable<objfile*>::notify (this=0x555566727020 <gdb::observers::new_objfile>, args#0=0x61300000c0c0) at /home/simark/src/binutils-gdb/gdb/../gdbsupport/observable.h:106 #18 0x000055555da3f228 in symbol_file_add_with_addrs (abfd=0x61200001ccc0, name=0x6190000d9090 "/usr/lib/libpthread.so.0", add_flags=..., addrs=0x7fffffffbc10, flags=..., parent=0x0) at /home/simark/src/binutils-gdb/gdb/symfile.c:1131 #19 0x000055555da3f763 in symbol_file_add_from_bfd (abfd=0x61200001ccc0, name=0x6190000d9090 "/usr/lib/libpthread.so.0", add_flags=<error reading variable: Cannot access memory at address 0xffffffffffffffb0>, addrs=0x7fffffffbc10, flags=<error reading variable: Cannot access memory at address 0xffffffffffffffc0>, parent=0x0) at /home/simark/src/binutils-gdb/gdb/symfile.c:1167 #20 0x000055555d95f9fa in solib_read_symbols (so=0x6190000d8e80, flags=...) at /home/simark/src/binutils-gdb/gdb/solib.c:681 #21 0x000055555d96233d in solib_add (pattern=0x0, from_tty=0, readsyms=1) at /home/simark/src/binutils-gdb/gdb/solib.c:987 #22 0x000055555d93646e in enable_break (info=0x608000008f20, from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2238 #23 0x000055555d93cfc0 in svr4_solib_create_inferior_hook (from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:3049 #24 0x000055555d96610d in solib_create_inferior_hook (from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib.c:1195 #25 0x000055555cdee318 in post_create_inferior (from_tty=0) at /home/simark/src/binutils-gdb/gdb/infcmd.c:318 #26 0x000055555ce00e6e in setup_inferior (from_tty=0) at /home/simark/src/binutils-gdb/gdb/infcmd.c:2439 #27 0x000055555ce59c34 in handle_one (event=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:4887 #28 0x000055555ce5cd00 in stop_all_threads () at /home/simark/src/binutils-gdb/gdb/infrun.c:5064 #29 0x000055555ce7f0da in stop_waiting (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:8006 #30 0x000055555ce67f5c in handle_signal_stop (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:6062 #31 0x000055555ce63653 in handle_inferior_event (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:5727 #32 0x000055555ce4f297 in fetch_inferior_event () at /home/simark/src/binutils-gdb/gdb/infrun.c:4105 #33 0x000055555cdbe3bf in inferior_event_handler (event_type=INF_REG_EVENT) at /home/simark/src/binutils-gdb/gdb/inf-loop.c:42 #34 0x000055555d018047 in handle_target_event (error=0, client_data=0x0) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:4060 #35 0x000055555e5ea77e in handle_file_event (file_ptr=0x60600008b1c0, ready_mask=1) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:575 #36 0x000055555e5eb09c in gdb_wait_for_event (block=0) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:701 #37 0x000055555e5e8d19 in gdb_do_one_event () at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:212 #38 0x000055555dd6e0d4 in wait_sync_command_done () at /home/simark/src/binutils-gdb/gdb/top.c:528 #39 0x000055555dd6e372 in maybe_wait_sync_command_done (was_sync=0) at /home/simark/src/binutils-gdb/gdb/top.c:545 #40 0x000055555d0ec7c8 in catch_command_errors (command=0x55555ce01bb8 <attach_command(char const*, int)>, arg=0x7fffffffe28d "1472010", from_tty=1, do_bp_actions=false) at /home/simark/src/binutils-gdb/gdb/main.c:452 #41 0x000055555d0f03ad in captured_main_1 (context=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1149 #42 0x000055555d0f1239 in captured_main (data=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1232 #43 0x000055555d0f1315 in gdb_main (args=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1257 #44 0x000055555bb70cf9 in main (argc=7, argv=0x7fffffffde88) at /home/simark/src/binutils-gdb/gdb/gdb.c:32 The exception is caught here: #0 __cxxabiv1::__cxa_begin_catch (exc_obj_in=0x60e0000060e0) at /build/gcc/src/gcc/libstdc++-v3/libsupc++/eh_catch.cc:84 #1 0x000055555d95fded in solib_read_symbols (so=0x6190000d8e80, flags=...) at /home/simark/src/binutils-gdb/gdb/solib.c:689 #2 0x000055555d96233d in solib_add (pattern=0x0, from_tty=0, readsyms=1) at /home/simark/src/binutils-gdb/gdb/solib.c:987 #3 0x000055555d93646e in enable_break (info=0x608000008f20, from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2238 #4 0x000055555d93cfc0 in svr4_solib_create_inferior_hook (from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:3049 #5 0x000055555d96610d in solib_create_inferior_hook (from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib.c:1195 #6 0x000055555cdee318 in post_create_inferior (from_tty=0) at /home/simark/src/binutils-gdb/gdb/infcmd.c:318 #7 0x000055555ce00e6e in setup_inferior (from_tty=0) at /home/simark/src/binutils-gdb/gdb/infcmd.c:2439 #8 0x000055555ce59c34 in handle_one (event=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:4887 #9 0x000055555ce5cd00 in stop_all_threads () at /home/simark/src/binutils-gdb/gdb/infrun.c:5064 #10 0x000055555ce7f0da in stop_waiting (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:8006 #11 0x000055555ce67f5c in handle_signal_stop (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:6062 #12 0x000055555ce63653 in handle_inferior_event (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:5727 #13 0x000055555ce4f297 in fetch_inferior_event () at /home/simark/src/binutils-gdb/gdb/infrun.c:4105 #14 0x000055555cdbe3bf in inferior_event_handler (event_type=INF_REG_EVENT) at /home/simark/src/binutils-gdb/gdb/inf-loop.c:42 #15 0x000055555d018047 in handle_target_event (error=0, client_data=0x0) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:4060 #16 0x000055555e5ea77e in handle_file_event (file_ptr=0x60600008b1c0, ready_mask=1) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:575 #17 0x000055555e5eb09c in gdb_wait_for_event (block=0) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:701 #18 0x000055555e5e8d19 in gdb_do_one_event () at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:212 #19 0x000055555dd6e0d4 in wait_sync_command_done () at /home/simark/src/binutils-gdb/gdb/top.c:528 #20 0x000055555dd6e372 in maybe_wait_sync_command_done (was_sync=0) at /home/simark/src/binutils-gdb/gdb/top.c:545 #21 0x000055555d0ec7c8 in catch_command_errors (command=0x55555ce01bb8 <attach_command(char const*, int)>, arg=0x7fffffffe28d "1472010", from_tty=1, do_bp_actions=false) at /home/simark/src/binutils-gdb/gdb/main.c:452 #22 0x000055555d0f03ad in captured_main_1 (context=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1149 #23 0x000055555d0f1239 in captured_main (data=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1232 #24 0x000055555d0f1315 in gdb_main (args=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1257 #25 0x000055555bb70cf9 in main (argc=7, argv=0x7fffffffde88) at /home/simark/src/binutils-gdb/gdb/gdb.c:32 Catching the exception at this point means that the thread_db_info object for this inferior will be left in place, despite the failure to load libthread_db. This means that there won't be further attempts at loading libthread_db, because thread_db_load will think that libthread_db is already loaded for this inferior and will always exit early. To fix this, add a try/catch around calling try_thread_db_load_1 in try_thread_db_load, such that if some exception is thrown while trying to load libthread_db, we reset / delete the thread_db_info for that inferior. That alone makes attach work fine again, because check_for_thread_db is called again in the thread_db_inferior_created observer (that happens after we learned about all shared libraries and their symbols), and libthread_db is successfully loaded then. When attaching, I think that the inferior_created observer is a good place to try to load libthread_db: it is called once everything has stabilized, when we learned about all shared libraries. The only problem then is that when we first try (and fail) to load libthread_db, in reaction to learning about libpthread, we show this warning: warning: Unable to find libthread_db matching inferior's thread library, thread debugging will not be available. This is misleading, because we do succeed in loading it later. So when attaching, I think we shouldn't try to load libthread_db in reaction to the new_objfile events, we should wait until we have learned about all shared libraries (using the inferior_created observable). To do so, add an `in_initial_library_scan` flag to struct inferior. This flag is used to postpone loading libthread_db if we are attaching or handling a fork child. When debugging remotely with GDBserver, the same problem happens, except that the qSymbol mechanism (allowing the remote side to ask GDB for symbols values) is involved. The fix there is the same idea, we make GDB wait until all shared libraries and their symbols are known before sending out a qSymbol packet. This way, we never present the remote side a state where libpthread.so's symbols are known but ld-linux's symbols aren't. gdb/ChangeLog: * inferior.h (class inferior) <in_initial_library_scan>: New. * infcmd.c (post_create_inferior): Set in_initial_library_scan. * infrun.c (follow_fork_inferior): Likewise. * linux-thread-db.c (try_thread_db_load): Catch exception thrown by try_thread_db_load_1 (thread_db_load): Return early if in_initial_library_scan is set. * remote.c (remote_new_objfile): Return early if in_initial_library_scan is set. Change-Id: I7a279836cfbb2b362b4fde11b196b4aab82f5efb
* gdb: fix tab after space indentation issuesSimon Marchi2021-05-271-1/+1
| | | | | | | | | | | | I spotted some indentation issues where we had some spaces followed by tabs at beginning of line, that I wanted to fix. So while at it, I did a quick grep to find and fix all I could find. gdb/ChangeLog: * Fix tab after space indentation issues throughout. Change-Id: I1acb414dd9c593b474ae2b8667496584df4316fd
* gdb: make add_info_alias accept target as a cmd_list_elementSimon Marchi2021-05-271-3/+4
| | | | | | | | | | | Same idea as previous patch, but for add_info_alias. gdb/ChangeLog: * command.h (add_info_alias): Accept target as cmd_list_element. Update callers. Change-Id: If830d423364bf42d7bea5ac4dd3a81adcfce6f7a
* gdb: make add_com_alias accept target as a cmd_list_elementSimon Marchi2021-05-271-22/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | The alias creation functions currently accept a name to specify the target command. They pass this to add_alias_cmd, which needs to lookup the target command by name. Given that: - We don't support creating an alias for a command before that command exists. - We always use add_info_alias just after creating that target command, and therefore have access to the target command's cmd_list_element. ... change add_com_alias to accept the target command as a cmd_list_element (other functions are done in subsequent patches). This ensures we don't create the alias before the target command, because you need to get the cmd_list_element from somewhere when you call the alias creation function. And it avoids an unecessary command lookup. So it seems better to me in every aspect. gdb/ChangeLog: * command.h (add_com_alias): Accept target as cmd_list_element. Update callers. Change-Id: I24bed7da57221cc77606034de3023fedac015150
* gdb: Pass std::strings to ui_out::field_string () where convenientMarco Barisione2021-05-191-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While adding a ui_out::text () overload accepting a std::string, I noticed that several callers of ui_out::field_string () were converting std::string instances to char pointers even if not necessary. gdb/ChangeLog: * ui-out.c (ui_out::field_string): Add missing style_argument to the overload accepting a std::string, to make it equivalent to the char pointer version. * ui-out.h (class ui_out): Ditto. * break-catch-sig.c (signal_catchpoint_print_one): Do not convert std::strings to char pointers before passing them to ui_out::field_string (). * break-catch-throw.c (print_one_detail_exception_catchpoint): Ditto. * cli/cli-setshow.c (do_show_command): Ditto. * disasm.c (gdb_pretty_print_disassembler::pretty_print_insn): Ditto. * infcmd.c (print_return_value_1): Ditto. * inferior.c (print_inferior): Ditto. * linux-thread-db.c (info_auto_load_libthread_db): Ditto. * mi/mi-cmd-var.c (print_varobj): Ditto. (mi_cmd_var_set_format): Ditto. (mi_cmd_var_info_type): Ditto. (mi_cmd_var_info_expression): Ditto. (mi_cmd_var_evaluate_expression): Ditto. (mi_cmd_var_assign): Ditto. (varobj_update_one): Ditto. * mi/mi-main.c (list_available_thread_groups): Ditto. (mi_cmd_data_read_memory_bytes): Ditto. (mi_cmd_trace_frame_collected): Ditto. * osdata.c (info_osdata): Ditto. * probe.c (info_probes_for_spops): Ditto. * target-connection.c (print_connection): Ditto. * thread.c (print_thread_info_1): Ditto. * tracepoint.c (print_one_static_tracepoint_marker): Ditto.
* gdb: generate the prefix name for prefix commands on demandMarco Barisione2021-05-121-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the prefixname field of struct cmd_list_element was manually set for prefix commands. This seems verbose and error prone as it required every single call to functions adding prefix commands to specify the prefix name while the same information can be easily generated. Historically, this was not possible as the prefix field was null for many commands, but this was fixed in commit 3f4d92ebdf7f848b5ccc9e8d8e8514c64fde1183 by Philippe Waroquiers, so we can rely on the prefix field being set when generating the prefix name. This commit also fixes a use after free in this scenario: * A command gets created via Python (using the gdb.Command class). The prefix name member is dynamically allocated. * An alias to the new command is created. The alias's prefixname is set to point to the prefixname for the original command with a direct assignment. * A new command with the same name as the Python command is created. * The object for the original Python command gets freed and its prefixname gets freed as well. * The alias is updated to point to the new command, but its prefixname is not updated so it keeps pointing to the freed one. gdb/ChangeLog: * command.h (add_prefix_cmd): Remove the prefixname argument as it can now be generated automatically. Update all callers. (add_basic_prefix_cmd): Ditto. (add_show_prefix_cmd): Ditto. (add_prefix_cmd_suppress_notification): Ditto. (add_abbrev_prefix_cmd): Ditto. * cli/cli-decode.c (add_prefix_cmd): Ditto. (add_basic_prefix_cmd): Ditto. (add_show_prefix_cmd): Ditto. (add_prefix_cmd_suppress_notification): Ditto. (add_prefix_cmd_suppress_notification): Ditto. (add_abbrev_prefix_cmd): Ditto. * cli/cli-decode.h (struct cmd_list_element): Replace the prefixname member variable with a method which generates the prefix name at runtime. Update all code reading the prefix name to use the method, and remove all code setting it. * python/py-cmd.c (cmdpy_destroyer): Remove code to free the prefixname member as it's now a method. (cmdpy_function): Determine if the command is a prefix by looking at prefixlist, not prefixname.
* gdb: some int to bool conversionAndrew Burgess2021-05-071-1/+1
| | | | | | | | | | | | | | | | | | Change int parameter to bool in remote_notice_new_inferior (remote.c) and notice_new_inferior (infcmd.c), and update the callers. There should be no user visible changes after this commit. gdb/ChangeLog: * infcmd.c (notice_new_inferior): Change parameter type. * inferior.h (notice_new_inferior): Change parameter type. * remote.c (remote_notice_new_inferior): Change parameter type to bool. Also update type of local variable to bool. (remote_target::update_thread_list): Change type of local variable to bool. (remote_target::process_stop_reply): Pass bool instead of int to remote_notice_new_inferior.
* gdb: make inferior::args a unique_xmalloc_ptrSimon Marchi2021-05-061-4/+7
| | | | | | | | | | | | | | Use unique_xmalloc_ptr to avoid manual memory management. gdb/ChangeLog: * inferior.h (class inferior) <args>: Change type to unique_xmalloc_ptr. * inferior.c (inferior::~inferior): Don't free args. * infcmd.c (get_inferior_args): Adjust. (set_inferior_args): Adjust. Change-Id: I96300e59eb2faf2d80660416a8f5694d243a944e
* gdb/continuations: turn continuation functions into inferior methodsTankut Baris Aktemur2021-04-221-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | Turn continuations-related functions into methods of the inferior class. This is a refactoring. gdb/ChangeLog: 2021-04-22 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> * Makefile.in (COMMON_SFILES): Remove continuations.c. * inferior.c (inferior::add_continuation): New method, adapted from 'add_inferior_continuation'. (inferior::do_all_continuations): New method, adapted from 'do_all_inferior_continuations'. (inferior::~inferior): Clear the list of continuations directly. * inferior.h (class inferior) <continuations>: Rename into... <m_continuations>: ...this and make private. * continuations.c: Remove. * continuations.h: Remove. * event-top.c: Don't include "continuations.h". Update the users below. * inf-loop.c (inferior_event_handler) * infcmd.c (attach_command) (notice_new_inferior): Update.
View Lorry Controller Status