aboutsummaryrefslogtreecommitdiff
path: root/gdb
AgeCommit message (Collapse)AuthorFilesLines
2024-09-07gdb: allow quoted filenames for commands that have custom completionAndrew Burgess7-27/+43
This commit changes how GDB processes command arguments for the following commands: compile file maint print c-tdesc save gdb-index After this commit these commands will now expect their single filename argument to be (optionally) quoted if it contains any special characters (e.g. whit space or quotes). If the filename does not contain any special characters then nothing changes. As an example: (gdb) save gdb-index /path/to/some/directory/ will work before and after this patch. However, if the directory name contains a white space then before this patch a user would write: (gdb) save gdb-index /path/to some/directory/ But this will now fail as GDB will consider this as two arguments, '/path/to' and 'some/directory/'. To pass this single directory name a user must now do one of these: (gdb) save gdb-index "/path/to some/directory/" (gdb) save gdb-index '/path/to some/directory/' (gdb) save gdb-index /path/to\ some/directory/ This brings these commands into line with commands like 'file' and 'symbol-file', which have supported quoted filenames for a while. The motivation for this change is to make handling of filename arguments consistent throughout GDB. We can't move to all commands taking non-quoted filenames as the non-quoted style only allows for a single argument. Additionally, the non-quoted style doesn't allow for filenames that end in white space (though this is probably pretty rare). So, if we want to have consistency the only choice is to move towards supporting quote filenames. Reviewed-By: Eli Zaretskii <eliz@gnu.org>
2024-09-07gdb: add remove-symbol-file command completionAndrew Burgess5-148/+238
The 'remove-symbol-file' command doesn't currently offer command completion. This commit addresses this. The 'remove-symbol-file' uses gdb_argv to split its command arguments, this means that the filename the command expects can be quoted. However, the 'remove-symbol-file' command is a little weird in that it also has a '-a' option, if this option is passed then the command expects not a filename, but an address. Currently the remove_symbol_file_command function splits the command args using gdb_argv, checks for a '-a' flag by looking at the first argument value, and then expects the filename or address to occupy a single entry in the gdb_argv array. The first thing I do is handle the '-a' flag using GDB's option system. I model this option as a flag_option_def (a boolean option). I've dropped the use of gdb_argv and instead use the new(ish) function extract_single_filename_arg, which was added a couple of commits back, to parse the filename argument (when '-a' is not given). If '-a' is given the the remove-symbol-file command expects an address rather than a filename. As we previously split the arguments using gdb_argv this meant the address needed to appear as a single argument. So a user could write: (gdb) remove-symbol-file 0x1234 Or they could write: (gdb) remove-symbol-file some_function Both of these would work fine. But a user could not write: (gdb) remove-symbol-file some_function + 0x1000 As only the 'some_function' part would be processed. Now the user could do this: (gdb) remove-symbol-file "some_function + 0x1000" By enclosing the address expression in quotes this would be handled as a single argument. However, this is a little weird, that's not how commands like 'print' or 'x' work. Also this functionality was neither documented, or tested. And so, in this commit, by removing the use of gdb_argv I bring the 'remove-symbol-file' command inline with GDB's other commands that take an expression, the quotes are no longer needed. Usually in a completer we call 'complete_options', but don't actually capture the option values. But for remove-symbol-file I do. This allows me to spot when the '-a' option has been given, I can then complete the rest of the command line as either a filename or an expression. Reviewed-By: Eli Zaretskii <eliz@gnu.org>
2024-09-07gdb: extend completion of quoted filenames to work in brkchars phaseAndrew Burgess2-15/+91
Up to this point filename completion for possibly quoted filenames has always been handled during the second (non-brkchars) phase of completion. This works fine for commands that only want to complete on a single filename argument. In a later commit though I need to perform completion of a quoted filename argument during the first (brkchars) phase of completion. This will allow me to add a custom completer that completes both command options and arguments for a command (remove-symbol-file) that takes a possibly quoted filename argument. This commit doesn't add the remove-symbol-file completer, this commit is just about putting support for that in place. To achieve this I've added the new function advance_to_filename_maybe_quoted_complete_word_point, which is unused in this commit. I've then had to extend some other functions in order to extract the quoting state during the brkchars phase. As this commit doesn't use the new functionality, the important thing at this point is that I've not regressed the existing filename completion (or any of the other completion). The next commit in this series will make use of the new functionality, and will include tests. There should be no user visible changes after this commit.
2024-09-07gdb: new extract_single_filename_arg helper functionAndrew Burgess2-0/+32
This commit is in preparation for the next few commits, this commit adds a new function extract_single_filename_arg. This new function will be used to convert GDB commands that expect a single filename argument to have these commands take a possibly quoted or escaped string. There's no use of the new function in this commit, for that see the following commits.
2024-09-07gdb: implement readline rl_directory_rewrite_hook callbackAndrew Burgess2-0/+33
Implement the readline rl_directory_rewrite_hook callback function, this is used when readline needs to offer completions from within a directory. The important thing is that this function should remove any escaping, this allows GDB to correctly offer completions in situations like this: (gdb) file /tmp/directory\ with\ spaces/<TAB><TAB> Note the escaping in 'directory\ with\ spaces'. Without the rl_directory_rewrite_hook callback readline will try to open a directory literally called '/tmp/directory\ with\ spaces' which obviously doesn't exist. There are tests added to cover this new functionality.
2024-09-07gdb: improve gdb_rl_find_completion_word for quoted wordsAndrew Burgess2-18/+56
The function gdb_rl_find_completion_word is very similar to the readline function _rl_find_completion_word, but was either an older version of that function, or was trimmed when copying to remove code which was considered unnecessary. We maintain this copy because the _rl_find_completion_word function is not part of the public readline API, and we need to replicate the functionality of that function as part of the 'complete' command. Within gdb_rl_find_completion_word when looking for the completion word, if we don't find a unclosed quoted string (which would become the completion word) then we scan backwards looking for a word break character. For example, given: (gdb) complete file /tmp/foo There is no unclosed quoted string so we end up scanning backwards from the end looking for a word break character. In this case the space after 'file' and before '/tmp/foo' is found, so '/tmp/foo' becomes the completion word. However, given this: (gdb) complete file /tmp/foo\" There is still no unclosed quoted string, however, when we can backwards the '"' (double quotes) are treated as a word break character, and so we end up using the empty string as the completion word. The readline function _rl_find_completion_word avoids this mistake by using the rl_char_is_quoted_p hook. This function will return true for the double quote character as it is preceded by a backslash. An earlier commit in this series supplied a rl_char_is_quoted_p function for the filename completion case, however, gdb_rl_find_completion_word doesn't call rl_char_is_quoted_p so this doesn't help for the 'complete' case. In this commit I've copied the code to call rl_char_is_quoted_p from _rl_find_completion_word into gdb_rl_find_completion_word. This half solves the problem. In the case: (gdb) complete file /tmp/foo\" We do now try to complete on the string '/tmp/foo\"', however, when we reach filename_completer we call back into readline to actually perform filename completion. However, at this point the WORD variable points to a string that still contains the backslash. The backslash isn't part of the actual filename, that's just an escape character. Our expectation is that readline will remove the backslash when looking for matching filenames. However, readline contains an optimisation to avoid unnecessary work trying to remove escape characters. The readline variable rl_completion_found_quote is set in the readline function gen_completion_matches before the generation of completion matches. This variable is set to true (non-zero) if there is (or might be) escape characters within the completion word. The function rl_filename_completion_function, which generates the filename matches, only removes escape characters when rl_completion_found_quote is true. When GDB generates completions through readline (e.g. tab completion) then rl_completion_found_quote is set correctly. But when we use the 'complete' command we don't pass through readline, and so gen_completion_matches is never called and rl_completion_found_quote is not set. In this case when we call rl_filename_completion_function readline doesn't remove the escapes from the completion word, and so in our case above, readline looks for completions of the exact filename '/tmp/foo\"', that is, the filename including the backslash. To work around this problem I've added a new flag to our function gdb_rl_find_completion_word which is set true when we find any quoting or escaping. This matches what readline does. Then in the 'complete' function we can set rl_completion_found_quote prior to generating completion matches. With this done the 'complete' command now works correctly when trying to complete filenames that contain escaped word break characters. The tests have been updated accordingly.
2024-09-07gdb: apply escaping to filenames in 'complete' resultsAndrew Burgess2-54/+144
Building on the mechanism added in the previous commit(s), this commit applies escaping to filenames in the 'complete' command output. Consider a file: /tmp/xxx/aa"bb -- that is a filename that contains a double quote, currently the 'complete' command output looks like this: (gdb) complete file /tmp/xxx/a file /tmp/xxx/aa"bb Notice that the double quote in the output is not escaped. If we passed this same output back to GDB then the double quote will be treated as the start of a string. After this commit then the output looks like this: (gdb) complete file /tmp/xxx/a file /tmp/xxx/aa\"bb The double quote is now escaped. If we feed this output back to GDB then GDB will treat this as a single filename that contains a double quote, exactly what we want. To achieve this I've done a little refactoring, splitting out the core of gdb_completer_file_name_quote, and then added a new call from the filename_match_formatter function. There are updates to the tests to cover this new functionality.
2024-09-07gdb: add match formatter mechanism for 'complete' command outputAndrew Burgess3-51/+158
This commit solves a problem that existed prior to the previous commit, but the previous commit made more common. When completing a filename with the 'complete' command GDB will always add a trailing quote character, even if the completion is a directory name, in which case it would be better if the trailing quote was not added. Consider: (gdb) complete file '/tmp/xx file '/tmp/xxx/' The completion offered here is really only a partial completion, we've completed up to the end of the next directory name, but, until we have a filename then the completion is not finished and the trailing quote should not be added. This would match the readline behaviour, e.g.: (gdb) file '/tmp/xx<TAB> (gdb) file '/tmp/xxx/ In this case readline completes the directory name, but doesn't add the trailing quote character. Remember that the 'complete' command is intended for tools like e.g. emacs in order that they can emulate GDB's standard readline completion when implementing a CLI of their own. As such, not adding the trailing quote in this case matches the readline behaviour, and seems like the right way to go. To achieve this, I've added a new function pointer member variable completion_result::m_match_formatter. This contains a pointer to a callback function which is used by the 'complete' command to format each result. The default behaviour of this callback function is to just append the quote character (the character from before the completion string) to the end of the completion result. This matches the current behaviour. However, for filename completion we override the default value of m_match_formatter, this new function checks if the completion result is a directory or not. If the completion result is a directory then the closing quote is not added, instead we add a trailing '/' character. The code to add a trailing '/' character already exists within the filename_completer function. This is no longer needed in this location, instead this code is moved into the formatter callback. Tests are updated to handle the changes in functionality, this removes an xfail added in the previous commit.
2024-09-07gdb: simplify completion_result::print_matchesAndrew Burgess2-33/+50
Simplify completion_result::print_matches by removing one of the code paths. Now, every time we call ::print_matches we always add the trailing quote. Previously, when using the 'complete' command, if there was only one result then trailing quote was added in ::build_completion_result, but when we had multiple results the trailing quote was added in ::print_matches. As a consequence, ::print_matches had to understand not to add the trailing quote for the single result case. After this commit we don't add the trailing quote in ::build_completion_result, instead ::print_matches always adds the trailing quote, which makes ::print_matches simpler. However, there is a slight problem. When completion is being driven by readline, and not by the 'complete' command, we still need to manually add the trailing quote in the single result case, and as the printing is done by readline we can't add the quote at the time of printing, and so, in ::build_completion_result, we still add the trailing quote, but only when completion is being done for readline. And this does cause a small problem. When completing a filename, if the completion results in a directory name then, when using the 'complete' command, GDB should not be adding a trailing quote. For example, if we have the file /tmp/xxx/foo.c, then what we should see is this: (gdb) complete file '/tmp/xx file 'tmp/xxx/ But what we actually see after this commit is this: (gdb) complete file '/tmp/xx file 'tmp/xxx/' Previously we didn't get the trailing quote in this case, as when there is only a single result, the quote was added in ::build_completion_result, and for filename completion, GDB didn't know what the quote character was in ::build_completion_result, so no quote was added. Now that the trailing quote is always added in ::print_matches, and GDB does know the quote character at this point, so we are now getting the trailing quote, which is not correct. This is a regression, but really, GDB is now broken in a consistent way, if we create the file /tmp/xxa/bar.c, then previously if we did this: (gdb) complete file '/tmp/xx file '/tmp/xxa/' file '/tmp/xxx/' Notice how we get the trailing quote in this case, this is the before patch behaviour, and is also wrong. A later commit will fix things so that the trailing quote is not added in this filename completion case, but for now I'm going to accept this small regression. This change in behaviour caused some failures in one of the completion tests, I've tweaked the test case to expect the trailing quote as part of this commit, but will revert this in a later commit in this series. I've also added an extra test for when the 'complete' command does complete to a single complete filename, in which case the trailing quote is expected.
2024-09-07gdb: move display of completion results into completion_result classAndrew Burgess3-25/+47
This commit moves the printing of the 'complete' command results out of the 'complete_command' function. The printing is now done in a new member function 'completion_result::print_matches'. At this point, this is entirely a refactor. The motivation for this refactor is how 'complete' should print the completion of filename arguments. In some cases the filename results need to have escaping added to the output. This escaping needs to be done immediately prior to printing the result as adding too early will result in multiple 'complete' results potentially being sorted incorrectly. See the subsequent commits for more details. There should be no user visible changes after this commit. Approved-By: Tom Tromey <tom@tromey.com>
2024-09-07gdb: improve escaping when completing filenamesAndrew Burgess2-3/+199
This improves quoting and escaping when completing filenames for commands that allow filenames to be quoted and escaped. I've struggled a bit trying to split this series into chunks. There's a lot of dependencies between different parts of the completion system, and trying to get this working correctly is pretty messy. This first step is really about implementing 3 readline hooks: rl_char_is_quoted_p - Is a particular character quoted within readline's input buffer? rl_filename_dequoting_function - Remove quoting characters from a filename. rl_filename_quoting_function - Add quoting characters to a filename. See 'info readline' for full details, but with these hooks connected up, readline (on behalf of GDB) should do a better job inserting backslash escapes when completing filenames. There's still a bunch of stuff that doesn't work after this commit, mostly around the 'complete' command which of course doesn't go through readline, so doesn't benefit from all of these new functions yet, I'll add some of this in a later commit. Tab completion is now slightly improved though, it is possible to tab-complete a filename that includes a double or single quote, either in an unquoted string or within a string surrounded by single or double quotes, backslash escaping is used when necessary. There are some additional tests to cover the new functionality.
2024-09-07gdb: deprecated filename_completer and associated functionsAndrew Burgess23-59/+62
Following on from the previous commit, this commit marks the old unquoted filename completion related functions as deprecated. The aim of doing this is to make it more obvious to someone adding a new command that they should not be using the older unquoted style filename argument handling. I split this change from the previous to make for an easier review. This commit touches more files, but is _just_ function renaming. Check out gdb/completer.{c,h} for what has been renamed. All the other files have just been updated to use the new names. There should be no user visible changes after this commit.
2024-09-07gdb: split apart two different types of filename completionAndrew Burgess8-72/+269
Unfortunately we have two different types of filename completion in GDB. The majority of commands have what I call unquoted filename completion, this is for commands like 'set logging file ...', 'target core ...', and 'add-auto-load-safe-path ...'. For these commands everything after the command name (that is not a command option) is treated as a single filename. If the filename contains white space then this does not need to be escaped, nor does the filename need to be quoted. In fact, the filename argument is not de-quoted, and does not have any escaping removed, so if a user does try to add such things, they will be treated as part of the filename. As an example: (gdb) target core "/path/that contains/some white space" Will look for a directory calls '"' (double quotes) in the local directory. A small number of commands do de-quote and remove escapes from filename arguments. These command accept what I call quoted and escaped filenames. Right now these are the commands that specify the file for GDB to debug, so: file exec-file symbol-file add-symbol-file remove-symbol-file As an example of this in action: (gdb) file "/path/that contains/some white space" In this case GDB would load the file: /path/that contains/some white space Current filename completion always assumes that filenames can be quoted, though escaping doesn't work in completion right now. But the assumption that quoting is allowed is clearly wrong. This commit splits filename completion into two. The existing filename_completer is retained, and is used for unquoted filenames. A second filename_maybe_quoted_completer is added which can be used for completing quoted filenames. The filename completion test has been extended to cover more cases. As part of the extended testing I need to know the character that should be used to separate filenames within a path. For this TCL 8.6+ has $::tcl_platform(pathSeparator). To support older versions of TCL I've added some code to testsuite/lib/gdb.exp. You might notice that after this commit the completion for unquoted files is all done in the brkchars phase, that is the function filename_completer_handle_brkchars calculates the completions and marks the completion_tracker as using a custom word point. The reason for this is that we don't want to break on white space for this completion, but if we rely on readline to find the completion word, readline will consider the entire command line, and with no white space in the word break character set, readline will end up using the entire command line as the word to complete. For now at least, the completer for quoted filenames does generate its completions during the completion phase, though this is going to change in a later commit.
2024-09-07gdb: unify build-id to objfile lookup codeAndrew Burgess7-55/+136
There are 3 places where we currently call debuginfod_exec_query to lookup an objfile for a given build-id. In one of these places we first call build_id_to_exec_bfd which also looks up an objfile given a build-id, but this function looks on disk for a symlink in the .build-id/ sub-directory (within the debug-file-directory). I can't think of any reason why we shouldn't call build_id_to_exec_bfd before every call to debuginfod_exec_query. So, in this commit I have added a new function in build-id.c, find_objfile_by_build_id, this function calls build_id_to_exec_bfd, and if that fails, then calls debuginfod_exec_query. Everywhere we call debuginfod_exec_query is updated to call the new function, and in locate_exec_from_corefile_build_id, the existing call to build_id_to_exec_bfd is removed as calling find_objfile_by_build_id does this for us. One slight weird thing is in core_target::build_file_mappings, here we call find_objfile_by_build_id which returns a gdb_bfd_ref_ptr for the opened file, however we immediately reopen the file as "binary". The reason for this is that all the bfds opened in ::build_file_mappings need to be opened as "binary" (see the function comments for why). I did consider passing a target type into find_objfile_by_build_id, which could then be forwarded to build_id_to_exec_bfd and used to open the BFD as "binary", however, if you follow the call chain you'll end up in build_id_to_debug_bfd_1, where we actually open the bfd. Notice in here that we call build_id_verify to double check the build-id of the file we found, this requires that the bfd not be opened as "binary". What this means is that we always have to first open the bfd using the gnutarget target type (for the build-id check), and then we would have to reopen it as "binary". There seems little point pushing the reopen logic into find_objfile_by_build_id, so we just do this in the ::build_file_mappings function. I've extended the tests to cover the two cases which actually changed in this commit.
2024-09-07gdb: improve shared library build-id check for core-filesAndrew Burgess15-91/+847
When GDB opens a core file, in 'core_target::build_file_mappings ()', we collection information about the files that are mapped into the core file, specifically, the build-id and the DT_SONAME attribute for the file, which will be set for some shared libraries. We then cache the DT_SONAME to build-id information on the core file bfd object in the function set_cbfd_soname_build_id. Later, when we are loading the shared libraries for the core file, we can use the library's file name to look in the DT_SONAME to build-id map, and, if we find a matching entry, we can use the build-id to validate that we are loading the correct shared library. This works OK, but has some limitations: not every shared library will have a DT_SONAME attribute. Though it is good practice to add such an attribute, it's not required. A library without this attribute will not have its build-id checked, which can lead to GDB loading the wrong shared library. What I want to do in this commit is to improve GDB's ability to use the build-ids extracted in core_target::build_file_mappings to both validate the shared libraries being loaded, and then to use these build-ids to potentially find (via debuginfod) the shared library. To do this I propose making the following changes to GDB: (1) Rather than just recording the DT_SONAME to build-id mapping in set_cbfd_soname_build_id, we should also record, the full filename to build-id mapping, and also the memory ranges to build-id mapping for every memory range covered by every mapped file. (2) Add a new callback solib_ops::find_solib_addr. This callback takes a solib object and returns an (optional) address within the inferior that is part of this library. We can use this address to find a mapped file using the stored memory ranges which will increase the cases in which a match can be found. (3) Move the mapped file record keeping out of solib.c and into corelow.c. Future commits will make use of this information from other parts of GDB. This information was never solib specific, it lived in the solib.c file because that was the only user of the data, but really, the data is all about the core file, and should be stored in core_target, other parts of GDB can then query this data as needed. Now, when we load a shared library for a core file, we do the following lookups: 1. Is the exact filename of the shared library found in the filename to build-id map? If so then use this build-id for validation. 2. Find an address within the shared library using ::find_solib_addr and then look for an entry in the mapped address to build-id map. If an entry is found then use this build-id. 3. Finally, look in the soname to build-id map. If an entry is found then use this build-id. The addition of step #2 here means that GDB is now far more likely to find a suitable build-id for a shared library. Having acquired a build-id the existing code for using debuginfod to lookup a shared library object can trigger more often. On top of this, we also create a build-id to filename map. This is useful as often a shared library is implemented as a symbolic link to the actual shared library file. The mapped file information is stored based on the actual, real file name, while the shared library information holds the original symbolic link file name. If when loading the shared library, we find the symbolic link has disappeared, we can use the build-id to file name map to check if the actual file is still around, if it is (and if the build-id matches) then we can fall back to use that file. This is another way in which we can slightly increase the chances that GDB will find the required files when loading a core file. Adding all of the above required pretty much a full rewrite of the existing set_cbfd_soname_build_id function and the corresponding get_cbfd_soname_build_id function, so I have taken the opportunity to move the information caching out of solib.c and into corelow.c where it is now accessed through the function core_target_find_mapped_file. At this point the benefit of this move is not entirely obvious, though I don't think the new location is significantly worse than where it was originally. The benefit though is that the cached information is no longer tied to the shared library loading code. I already have a second set of patches (not in this series) that make use of this caching from elsewhere in GDB. I've not included those patches in this series as this series is already pretty big, but even if those follow up patches don't arrive, I think the new location is just as good as the original location. Rather that caching the information within the core file BFD via the registry mechanism, the information used for the mapped file lookup is now stored within the core_file target directly.
2024-09-07gdb/corefile: improve file backed mapping handlingAndrew Burgess5-73/+648
This commit improves how GDB handles file backed mappings within a core file, specifically, this is a restructuring of the function core_target::build_file_mapping. The primary motivation for this commit was to put in place the infrastructure to support the next commit in this series, but this commit does itself make some improvements. Currently in core_target::build_file_mapping we use gdbarch_read_core_file_mappings to iterate over the mapped regions within a core file. For each region a callback is invoked which is passed details of the mapping; the file the mapping is from, the offset into the file, and the address range at which the mapping exists. We are also passed the build-id for the mapped file in some cases. We are only told the build-id for the mapped region which actually contains the ELF header of the mapped file. Other regions of the same mapped ELF will not have the build-id passed to the callback. Within core_target::build_file_mapping, in the per-region callback, we try to find the mapped file based on its filename. If the file can't be found, and if we have a build-id then we'll ask debuginfod to download the file. However we find the file, we cache the opened bfd object, which is good. Subsequent mappings from the same file will not have a build-id set, but by that point we already have a cached open bfd object, so the lack of build-id is irrelevant. The problem with the above is that if we find a matching file based on the filename, then we accept that file, even if we have a build-id, and the build-id doesn't match. Currently, the mapped region processing is done in a single pass, we call gdbarch_read_core_file_mappings, and for each mapping, as we see it, we create the data structures needed to represent that mapping. In this commit, I will change this to a two phase process. In the first phase the mappings are grouped together based on the name of the mapped file. At the end of phase one we have a 'struct mapped_file', a new struct, for each mapped file. This struct associates an optional build-id with a list of mapped regions. In the second phase we try to find the file using its filename. If the file is found, and the 'struct mapped_file' has a build-id, then we'll compare the build-id with the file we found. This allows us to reject on-disk files which have changed since the core file was created. If no suitable file was found (either no file found, or a build-id mismatch) then we can use debuginfod to potentially download a suitable file. NOTE: In the future we could potentially add additional sanity checks here, for example, if a data-file is mapped, and has no build-id, we can estimate a minimum file size based on the expected mappings. If the file we find is not big enough then we can reject the on-disk file. But I don't know how useful this would actually be, so I've not done that for now. Having found (or not) a suitable file then we can create the data structures for each mapped region just as we did before. The new functionality here is the extra build-id check, and the possibility of rejecting an on-disk file if the build-id doesn't match. This change could have been done within the existing single phase approach I think, however, in the next approach I need to have all the mapped regions associated with the expected build-id, and the new two phase structure allows me to do that, this is the reason for such an extensive rewrite in this commit. There's a new test that exercises GDB's ability to find mapped files via the build-id, and this downloading from debuginfod.
2024-09-07gdb/corefile: don't pretend unavailable sections are readableAndrew Burgess2-72/+101
When GDB opens a core file the bfd library processes the core file and creates sections within the bfd object to represent each of the segments within the core file. GDB then creates two target_section lists, m_core_section_table and m_core_file_mappings, these, along with m_core_unavailable_mappings, are used by GDB to implement core_target::xfer_partial; this is the function used when GDB tries to read memory from a core file inferior. The m_core_section_table list represents sections within the core file itself. The sections in this list can be split into two groups based on whether the section has the SEC_HAS_CONTENTS flag set or not. Sections (from the core file) that have the SEC_HAS_CONTENTS flag had their contents copied into the core file when the core file was created. These correspond to writable sections within the original inferior (the inferior for which the core file was created). Sections (from the core file) that do not have the SEC_HAS_CONTENTS flag will not have had their contents copied into the core file when it was created. These sections correspond to read-only sections mapped from a file (possibly the initial executable, or possibly some other file) in the original inferior. The expectation is that the contents of these sections can still be found by looking in the file that was originally mapped. The m_core_file_mappings list is created when GDB parses the mapped file list in the core file. Every mapped region will be covered by entries in the m_core_section_table list (see above), but for read-only mappings the entry in m_core_section_table will not have the SEC_HAS_CONTENTS flag set. As GDB parses the mapped file list, if the file that was originally mapped can be found, then GDB creates an entry in the m_core_file_mappings list which represents the region of the file that was mapped into the original inferior. However, GDB only creates entries in m_core_file_mappings if it is able to find the correct on-disk file to open. If the file can't be found then an entry is added to m_core_unavailable_mappings instead. If is the handling m_core_unavailable_mappings which I think is currently not completely correct. When a read lands within an m_core_unavailable_mappings region we currently forward the read to the exec file stratum. The reason for this is this: when GDB read the mapped file list, if the executable file could not be found at the expected path then mappings within the executable will end up in the m_core_unavailable_mappings list. However, the user might provide the executable to GDB from a different location. If this happens then forwarding the read to the exec file stratum might give a result. But, if the exec file stratum does not resolve the access then currently we continue through ::xfer_partial, the next step of which is to handle m_core_section_table entries that don't have the SEC_HAS_CONTENTS flag set. Every m_core_unavailable_mappings entry will naturally have an m_core_section_table without the SEC_HAS_CONTENTS flag set, and so we treat the unavailable mapping as zero initialised memory and return all zeros. It is this fall through behaviour that I think is wrong. If a read falls in an unavailable region, and the exec file stratum cannot help, then I think the access should fail. To achieve this goal I have removed the xfer_memory_via_mappings helper function and moved its content inline into ::xfer_partial. Now, if an access is within an m_core_unavailable_mappings region, and the exec file stratum doesn't help, we immediately return with an error. The reset of ::xfer_partial is unchanged, I've extended some comments in the area that I have changed to (I hope) explain better what's going on. There's a new test that covers the new functionality, an inferior maps a file and generates a core file. We then remove the mapped file, load the core file and try to read from the mapped region. The expectation is that GDB should give an error rather than claiming that the region is full of zeros.
2024-09-06Fix 'catch exception' with -fltoTom Tromey5-11/+102
A user noticed that when an Ada program (including the runtime) is compiled with -flto, then "catch exception" does not work -- even though setting the equivalent breakpoint by hand does work. Looking into this, it turns out that GCC puts the exception functions from the Ada runtime into a CU that uses the C language, not Ada. Then, when trying to look up the relevant symbol, lookup_name_info::search_name_hash uses the "verbatim" form of the symbol name (like "<__gnat_debug_raise_exception>") rather than the "<>"-less form, causing the symbol not to be found. This patch fixes the problem in two steps. First, lookup_name_info::search_name_hash is changed to use the same hack that language_defn::get_symbol_name_matcher uses. That is, when the current language is Ada, verbatim-mode lookups are special-cased. (This is a bit unfortunate; perhaps a better long term approach would be to promote verbatim mode to a fundamental mode of lookup_name_info.) Second, although the above fixes the problem in the Ada language mode, the code still fails in other languages. However, due to the way these lookups are coded in ada-lang.c, I think it makes sense to temporarily set the current language to Ada in create_ada_exception_catchpoint. Tested on x86-64 Fedora 38. A new test case that mimics the -flto scenario is included. Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
2024-09-06Clear Ada symbol cacheTom Tromey3-2/+7
This patch changes "maint flush symbol-cache" to also flush the Ada-specific symbol cache. This can be helpful when working on the Ada code. Approved-By: Tom de Vries <tdevries@suse.de>
2024-09-06Test -fgnat-encodings=all in tagged_access.expTom Tromey1-10/+15
While working on a longer series, I needed to make sure this particular test kept working with -fgnat-encodings=all, so this patch adds it to the test.
2024-09-06Introduce and use foreach_gnat_encodingTom Tromey37-97/+93
gnat-llvm does not support the -fgnat-encodings flag. This patch prepares gdb's Ada tests to handle this situation by introducing a new foreach_gnat_encoding. A subsequent patch may change this to support gnat-llvm; meanwhile this is a little cleaner anyway.
2024-09-06Fix the build-id option for GCC default configurationBernd Edlinger1-4/+5
It is possible that the compiler is configured to do so automatically, but at least for GCC the configure option --enable-linker-build-id is not enabled by default. So the option -Wl,--build-id should be used regardless of which compiler is used. Approved-By: Tom de Vries <tdevries@suse.de>
2024-09-04Improve comments in dwarf2/parent-map.hTom Tromey1-4/+27
I noticed that the comments for class parent_map aren't very clear. This patch attempts to fix this, and also clarifies a point on parent_map_map::add_map. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2024-09-04gdb: reformat Python file with blackAndrew Burgess1-1/+3
Fix formatting of a Python file added in commit: commit a92e943014f5e8d6a2eaccaf8a725941ac47a121 Date: Wed Aug 14 15:16:46 2024 +0100 gdb: implement ::re_set method for catchpoint class No functional change after this commit.
2024-09-04gdb: implement ::re_set method for catchpoint classAndrew Burgess6-5/+378
It is possible to attach a condition to a catchpoint. This can't be done when the catchpoint is created, but can be done with the 'condition' command, this is documented in the GDB manual: You can also use the 'if' keyword with the 'watch' command. The 'catch' command does not recognize the 'if' keyword; 'condition' is the only way to impose a further condition on a catchpoint. A GDB crash was reported against Fedora GDB where a user had attached a condition to a catchpoint and then restarted the inferior. When the catchpoint was hit GDB would immediately segfault. I was able to reproduce the failure on upstream GDB: (gdb) file ./some/binary (gdb) catch syscall write (gdb) run ... Catchpoint 1 (returned from syscall write), 0x00007ffff7b594a7 in write () from /lib64/libc.so.6 (gdb) condition 1 $_streq((char *) $rsi, "foobar") == 0 (gdb) run ... Fatal signal: Segmentation fault ... What happened here is that on the system in question we had debug information available for both the main application and also for libc. When the condition was attached GDB was stopped inside libc and as the debug information was available GDB found a reference to the 'char' type (for the cast) inside libc's debug information. When the inferior is restarted GDB discards all of the objfiles associated with shared libraries, and this includes libc. As such the 'char' type, which is objfile owned, is discarded and the reference to it from the catchpoint's condition expression becomes invalid. Now, if it were a breakpoint instead of a catchpoint, what would happen is that after the shared library objfiles had been discarded we'd call the virtual breakpoint::re_set method on the breakpoint, and this would update the breakpoint's condition expression. This is because user breakpoints are actually instances of the code_breakpoint class and the code_breakpoint::re_set method contains the code to recompute the breakpoint's condition expression. However, catchpoints are instances of the catchpoint class which inherits from the base breakpoint class. The catchpoint class does not override breakpoint::re_set, and breakpoint::re_set is empty! The consequence of this is that catchpoint condition expressions are never recomputed, and the dangling pointer to the now deleted, objfile owned type 'char' is left around, and, when the catchpoint is hit, the invalid pointer is used when GDB tries to evaluate the condition expression. In this commit I have implemented catchpoint::re_set. This is pretty simple and just recomputes the condition expression as you'd expect. If the condition doesn't evaluate then the catchpoint is marked as disabled_by_cond. I have also made breakpoint::re_set pure virtual. With the addition of catchpoint::re_set every sub-class of breakpoint now implements the ::re_set method, and if new sub-classes are added in the future I think that they _must_ implement ::re_set in order to avoid this problem. As such falling back to an empty breakpoint::re_set doesn't seem helpful. For testing I have not relied on stopping in libc and having libc debug information available, this doesn't seem like a good idea for the GDB testsuite. Instead I create a (rather pointless) condition check that uses a type defined only within a shared library. When the inferior is restarted the catchpoint will temporarily be marked as disabled_by_cond (due to the type not being available), but once the shared library is loaded again the catchpoint will be re-enabled. Without the fixes above then the same crashing behaviour can be observed. One point of note: the dangling pointer of course exposes undefined behaviour, with no guarantee of a crash. Though a crash is what I usually see I have see GDB throw random errors from the expression evaluation code, and once, I saw no problem at all! If you recompile GDB with the address sanitizer, or run under valgrind, then the bug will be exposed every time. After fixing this bug I checked bugzilla and found PR gdb/29960 which is the same bug. I was able to reproduce the bug before this commit, and after this commit GDB is no longer crashing. Before: (gdb) file /tmp/hello.x Reading symbols from /tmp/hello.x... (gdb) run Starting program: /tmp/hello.x Hello World [Inferior 1 (process 1101855) exited normally] (gdb) catch syscall 1 Catchpoint 1 (syscall 'write' [1]) (gdb) condition 1 write.fd == 1 (gdb) run Starting program: /tmp/hello.x Fatal signal: Segmentation fault ... And after: (gdb) file /tmp/hello.x Reading symbols from /tmp/hello.x... (gdb) run Starting program: /tmp/hello.x Hello World Args: ( 0 , 1 , 2 , 3 , 4 , 5 , 6 , 7 ) [Inferior 1 (process 1102373) exited normally] (gdb) catch syscall 1 Catchpoint 1 (syscall 'write' [1]) (gdb) condition 1 write.fd == 1 (gdb) r Starting program: /tmp/hello.x Error in testing condition for breakpoint 1: Attempt to extract a component of a value that is not a structure. Catchpoint 1 (call to syscall write), 0x00007ffff7eb94a7 in write () from /lib64/libc.so.6 (gdb) ptype write type = <unknown return type> () (gdb) Notice we get the error now when the condition fails to evaluate. This seems reasonable given that 'write' will be a function, and indeed the final 'ptype' shows that it's a function, not a struct. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29960 Reviewed-By: Tom de Vries <tdevries@suse.de>
2024-09-04[gdb/testsuite] Fix gdb.arch/riscv-tdesc-regs.expTom de Vries1-13/+1
On riscv64-linux, with test-case gdb.arch/riscv-tdesc-regs.exp I get: ... (gdb) info registers fflags^M fflags 0x0 NV:0 DZ:0 OF:0 UF:0 NX:0^M (gdb) FAIL: gdb.arch/riscv-tdesc-regs.exp: info registers fflags info registers frm^M frm 0x0 FRM:0 [RNE (round to nearest; ties to even)]^M (gdb) FAIL: gdb.arch/riscv-tdesc-regs.exp: info registers frm ... The FAILs are produced by: ... foreach reg {fflags frm} { gdb_test_multiple "info registers $reg" "" { -re "^info registers $reg\r\n" { exp_continue } -wrap -re "^Invalid register `$reg`" { fail $gdb_test_name } -wrap -re "^$reg\\s+\[^\r\n\]+" { pass $gdb_test_name } } } ... The first clause is meant to consume the command. The '^' char was updated to mean "consume command", so that clause no longer works since it now attempts to consume the command twice. Also, it's unnecessary because the following clauses start with ^. Then, the second clause is unnecessary because there's a default clause producing the FAIL. Fix this by simplifying to: ... foreach reg {fflags frm} { gdb_test "info registers $reg" "^$reg\\s+\[^\r\n\]+" } ... Tested on riscv64-linux. Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-09-04MAINTAINERS: Update my email addressShahab Vahedi1-1/+1
2024-09-04[gdb/testsuite] Fix gdb.dwarf2/dw2-lines.exp on arm-linuxTom de Vries5-1/+143
With test-case gdb.dwarf2/dw2-lines.exp on arm-linux, I run into: ... (gdb) break bar_label^M Breakpoint 2 at 0x4004f6: file dw2-lines.c, line 29.^M (gdb) continue^M Continuing.^M ^M Breakpoint 2, bar () at dw2-lines.c:29^M 29 foo (2);^M (gdb) PASS: $exp: cv=2: cdw=32: lv=2: ldw=32: continue to breakpoint: foo \(1\) ... The pass is incorrect because the continue lands at line 29 with "foo (2)" instead of line line 27 with "foo (1)". A minimal version is: ... $ gdb -q -batch dw2-lines.cv-2-cdw-32-lv-2-ldw-32 -ex "b bar_label" Breakpoint 1 at 0x4f6: file dw2-lines.c, line 29. ... where: ... 000004ec <bar>: 4ec: b580 push {r7, lr} 4ee: af00 add r7, sp, #0 000004f0 <bar_label>: 4f0: 2001 movs r0, #1 4f2: f7ff fff1 bl 4d8 <foo> 000004f6 <bar_label_2>: 4f6: 2002 movs r0, #2 4f8: f7ff ffee bl 4d8 <foo> ... So, how does this happen? In short: - skip_prologue_sal calls arm_skip_prologue with pc == 0x4ec, - thumb_analyze_prologue returns 0x4f2 (overshooting by 1 insn, PR tdep/31981), and - skip_prologue_sal decides that we're mid-line, and updates to 0x4f6. However, this is a test-case about .debug_line info, so why didn't arm_skip_prologue use the line info to skip the prologue? The answer is that the line info starts at bar_label, not at bar. Fixing that allows us to work around PR tdep/31981. Likewise in gdb.dwarf2/dw2-line-number-zero.exp. Instead, add a new test-case gdb.arch/skip-prologue.exp that is dedicated to checking quality of architecture-specific prologue analysis, without being written in an architecture-specific way. If fails on arm-linux for both marm and mthumb: ... FAIL: gdb.arch/skip-prologue.exp: f2: $bp_addr == $prologue_end_addr (skipped too much) FAIL: gdb.arch/skip-prologue.exp: f4: $bp_addr == $prologue_end_addr (skipped too much) ... and passes for: - x86_64-linux for {m64,m32}x{-fno-PIE/-no-pie,-fPIE/-pie} - aarch64-linux. Tested on arm-linux.
2024-09-03[gdb] Fix typosTom de Vries18-18/+18
Fix a few typos. unconditionaly -> unconditionally gratuitiously -> gratuitously configureable -> configurable represention -> representation distiguished -> distinguished breakpointer -> breakpoint asssignments -> assignments architectual -> architectural compatibity -> compatibility adjustement -> adjustment unexcepted -> unexpected propogated -> propagated consistant -> consistent succeding -> succeeding higlight -> highlight detachs -> detach Tested by rebuilding on x86_64-linux. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2024-08-31[gdb/testsuite] Handle unsupported catch syscallTom de Vries3-28/+75
On riscv64-linux, I run into: ... Expecting: ^(catch syscall[^M ]+)?((&.*)*.*~"Catchpoint 5 .*\\n".*=breakpoint-created,bkpt=\{number="5",type="catchpoint".*\}.*\n\^done[^M ]+[(]gdb[)] ^M [ ]*) catch syscall^M &"catch syscall\n"^M &"The feature 'catch syscall' is not supported on this architecture yet.\n"^M ^error,msg="The feature 'catch syscall' is not supported on this architecture yet."^M (gdb) ^M FAIL: gdb.mi/mi-breakpoint-changed.exp: test_insert_delete_modify: catch syscall (unexpected output) ... Fix this by: - factoring out proc supports_catch_syscall out of gdb.base/catch-syscall.exp, and - using it in gdb.mi/mi-breakpoint-changed.exp. Tested on x86_64-linux and riscv64-linux. Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-08-30gdb: remove duplicate check in disable_breakpoints_in_freed_objfileAndrew Burgess1-4/+0
I spotted that we have a duplicate condition check in the function disable_breakpoints_in_freed_objfile. Lets remove it. There should be no user visible changes after this commit. Approved-By: Tom Tromey <tom@tromey.com>
2024-08-30gdb/dwarf2: cleanup includesSimon Marchi19-25/+21
Cleanup includes in dwarf2/*. 1. Add the necessary includes so that clangd reports no errors when opening header files. This ensures that header files include what they use. 2. Remove all includes reported as unused by clangd (except gdb-safe-ctype.h, which I think does some magic that affects what follows). Built-tested --enable-threading at "yes" and "no", since there are some portions of code gated by `#ifdef CXX_STD_THREAD`. Change-Id: I21debffcd7c2caf90f08e1e0fbba3ce30422d042 Approved-By: Tom Tromey <tom@tromey.com>
2024-08-30Minor formatting fix in breakpoint.cTom Tromey1-1/+2
I noticed a spot in breakpoint.c that doesn't follow gdb's formatting rules: the return type is on the same line as the method name.
2024-08-30Fix regexp quoting in gdb.ada test casesTom Tromey14-43/+43
I noticed that some gdb.ada tests used regular expressions like: "Continuing\..*$inferior_exited_re.*" \ Here, the "\." should either be "." or "\\." -- "\." is not really meaningful. This patch fixes all the cases of this I could find in gdb.ada. In one test (fun_renaming.exp), using "\\." would result in failures, and here I rewrote the tests to use -wrap. Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-08-30gdb/doc: fix typo in 'watch' commandNicolás Ortega Froysa1-1/+1
* gdb/breakpoint.c (watch_option_defs): Fix typo. Copyright-paperwork-exempt: yes.
2024-08-29gdb: reject inserting breakpoints between functionsAndrew Burgess16-56/+210
When debugging ROCm code, you might have something like this: __global__ void kernel () { ... // break here ... } int main () { // Code to call `kernel` } ... where kernel is a function compiled to execute on the GPU. It does not exist in the host x86-64 program that runs the main function, and GDB doesn't know about that function until it is called, at which point the runtime loads the corresponding code object and GDB learns about the code of the "kernel" function. Before the GPU code object is loaded, from the point of view of GDB, you might as well have blank lines instead of the "kernel" function. The DWARF in the host program doesn't describe anything at these lines. So, a common problem that users face is: - Start GDB with the host binary - Place a breakpoint by line number at the "break here" line - At this point, GDB only knows about the host code, the lines of the `kernel` function are a big void. - GDB finds no code mapped to the "break here" line and searches for the first following line that has code mapped to it. - GDB finds that the line with the opening bracket of the `main` function (or around there) has code mapped to it, places breakpoint there. - User runs the program. - The programs hits the breakpoint at the start of main. - User is confused, because they didn't ask for a breakpoint in main. If they continue, the code object eventually gets loaded, GDB reads the debug info from it, re-evaluates the breakpoint locations, and at this point the breakpoint is placed at the expected location. The goal of this patch is to get rid of this annoyance. A case similar to the one shown above can actually be simulated without GPU-specific code: using a single source file to generate a library and an executable loading that library (see the new test gdb.linespec/line-breakpoint-outside-function.c for an example). Before the library is loaded, trying to place a breakpoint in the library code results in the breakpoint "drifting" down to the main function. To address this problem, make it so that when a user requests a breakpoint outside a function, GDB makes a pending breakpoint, rather than placing a breakpoint at the next line with code, which happens to be in the next function. When the GPU kernel or shared library gets loaded, the breakpoint resolves to a location in the kernel or library. Note that we still want breakpoints placed inside a function to "drift" down to the next line with code. For example, here: 9 10 void foo() 11 { 12 int x; 13 14 x++; There is probably no code associated to lines 10, 12 and 13, but the user can still reasonably expect to be able to put a breakpoint there. In my experience, GCC maps the function prologue to the line with the opening curly bracket, so the user will be able to place a breakpoint there anyway (line 11 in the example). But I don't really see a use case to put a breakpoint above line 10 and expect to get a breakpoint in foo. So I think that is a reasonable behavior change for GDB. This is implemented using the following heuristic: - If a breakpoint is requested at line L but there is no code mapped to L, search for a following line with associated code (this already exists today). - However, if: 1. the found location falls in a function symbol's block 2. the found location's address is equal the entry PC of that function 3. the found location's line is greater that the requested line ... then we don't place a breakpoint at the found location, we will end up with a pending breakpoint. Change the message "No line X in file..." to "No compiled code for line X in file...". There is clearly a line 9 in the example above, so it would be weird to say "No line 9 in file...". What we mean is that there is no code associated to line 9. All the regressions that I found this patch to cause were: 1. tests specifically this behavior where placing a breakpoint before a function results in a breakpoint on that function, in which case I removed the tests or changed them to expect a pending breakpoint 2. linespec tests expecting things like "break -line N garbage" to error out because of the following garbage, but we now got a different error because line N now doesn't resolve to something anymore. For example, before: (gdb) break -line 3 if foofoofoo == 1 No symbol "foofoofoo" in current context. became (gdb) break -line 3 if foofoofoo == 1 No line 3 in the current file. These tests were modified to refer to a valid line with code, so that we can still test what we intended to test. Notes: - The CUDA compiler "solves" this problem by adding dummy function symbols between functions, that are never called. So when you try to insert a breakpoint in the not-yet-loaded kernel, the breakpoint still drifts, but is placed on some dummy symbol. For reasons that would be too long to explain here, the ROCm compiler does not do that, and it is not a desirable option. - You can have constructs like this: void host_function() { struct foo { static void __global__ kernel () { // Place breakpoint here } }; // Host code that calls `kernel` } The heuristic won't work then, as the breakpoint will drift somewhere inside the enclosing function, but won't be at the start of that function. So a bogus breakpoint location will be created on the host side. I don't think that people are going to use this kind of construct often though, so we can probably ignore it (or at least it shouldn't prevent making the more common case better). ROCm doesn't support passing a lambda kernel function to hipLaunchKernelGGL (the function used to launch kernels on the device), but if it eventually does, there will be the same problem. I think that to properly support this, we will need some DWARF improvements to be able to say "there is really nothing at these lines" in the line table. Co-Authored-By: Simon Marchi <simon.marchi@efficios.com> Change-Id: I3cc12cfa823dc7d8e24dd4d35bced8e8baf7f9b6
2024-08-29gdb/doc: move NEWS entry to the correct placeAndrew Burgess1-5/+7
In commit: commit 3055e3d2f13bb84db90b9c19d427c362053775d2 Date: Tue May 21 15:58:02 2024 +0100 gdb: add GDB side target_ops::fileio_stat implementation I managed to place a NEWS entry in the wrong place. I put the entry in 'Changes in GDB 15' rather than 'Changes since GDB 15'. This commit moves the entry to the correct place.
2024-08-29gdb: include gdbsupport/gdb_obstack.h in addrmap.hSimon Marchi1-1/+2
This header file uses auto_obstack, found in gdbsupport/gdb_obstack.h. This fixes an error shown when editing addrmap.h with clangd, and makes it so addrmap.h includes what it uses. Change-Id: I0b0c8d26638e2150fcb65c601098ed9df5a8945a
2024-08-29gdb: remove unused includes in linespec.cSimon Marchi1-2/+0
Remove some includes reported as unused by clangd. Change-Id: Id1d5130430cdd2a37da1325a5eb67677f37905df
2024-08-29[gdb/testsuite] Fix another regexp in gdb.threads/stepi-over-clone.expTom de Vries1-1/+1
On openSUSE Tumbleweed, I run into: ... (gdb) PASS: gdb.threads/stepi-over-clone.exp: catch process syscalls continue^M Continuing.^M ^M Catchpoint 2 (call to syscall clone3), __clone3 () at clone3.S:62^M (gdb) FAIL: gdb.threads/stepi-over-clone.exp: continue ... Fix this by updating another (see commit 8fbf220321d) regexp to also recognize __clone3. Tested on x86_64-linux.
2024-08-29[gdb/testsuite] Fix regexp in gdb.arch/i386-disp-step-self-call.expTom de Vries2-2/+2
Usually, with test-case gdb.arch/i386-disp-step-self-call.exp I get: ... (gdb) x/1wx 0xffffc4f8^M 0xffffc4f8: 0x08048472^M (gdb) PASS: $exp: check return address was updated correctly ... but sometimes I run into: ... (gdb) x/1wx 0xffffc5c8^M 0xffffc5c8: 0x0804917e^M (gdb) FAIL: $exp: check return address was updated correctly ... The problem is that here: ... set next_insn_addr 0x[format %08X $next_insn_addr] gdb_test "x/1wx 0x[format %x $sp]" "$hex:\\s+$next_insn_addr" \ "check return address was updated correctly" ... we're trying to match string 0x0804917e against regexp 0x0804917E due to using "%08X" as format string. We only run into this problem if the address contains letters, which apparently usually isn't the case. Fix this by using "%08x" instead as format string. Likewise in test-case gdb.arch/amd64-disp-step-self-call.exp. Tested on x86_64-linux. PR testsuite/32121 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32121
2024-08-28Don't check dwarf2_name in process_enumeration_scopeTom Tromey1-6/+1
I noticed that process_enumeration_scope checks the result of dwarf2_name. However, this isn't needed, because new_symbol does the same check. This patch removes the unnecessary code. Reviewed-by: Keith Seitz <keiths@redhat.com>
2024-08-28gdb/testsuite: fix gdb.btrace/non-stop.exp end of history checkGuinevere Larsen1-7/+9
The recent commit 089197010993b3a5dc50bf882470bab2de696d92 changed the warnings when GDB reaches the end of the recorded history, and updated tests to expect the new messages. The pattern used for gdb.btrace/non-stop.exp, however, was too broad and could cause the following test result: ... (gdb) PASS: gdb.btrace/non-stop.exp: no progress: all: thread apply all continue: prompt ^M Reached end of recorded history; stopping.^M Following forward execution will be added to history.^M test (arg=0x0) at /data/vries/gdb/src/gdb/testsuite/gdb.btrace/non-stop.c:30^M 30 return arg; /* bp.2 */^M ^M Reached end of recorded history; stopping.^M Following forward execution will be added to history.^M test (arg=0x0) at /data/vries/gdb/src/gdb/testsuite/gdb.btrace/non-stop.c:30^M 30 return arg; /* bp.2 */^M PASS: gdb.btrace/non-stop.exp: no progress: all: thread apply all continue: thread 0 FAIL: gdb.btrace/non-stop.exp: no progress: all: thread apply all continue: thread 1 (timeout) ... This happens because the pattern looks like one of these 2: "Reached end of recorded.*Backwards execution.*" "Reached end of recorded.*Following forward.*" What seems to have happened is that all the output came at once, and most of it was consumed by the first '.*' pattern when checking for thread 0, so there was no output left for checking thread 1. This commit fixes that by making the expected outputs more exact. I also fixed the whitespace errors in gdb_cont_to_no_history_backwards that pre-dated the commit above, since I was already touching that proc. Approved-By: Tom de Vries <tdevries@suse.de>
2024-08-28gdb/testsuite: add no-delete-breakpoints option to 'runto' procAndrew Burgess3-29/+20
New 'no-delete-breakpoints' option for the 'runto' proc. This option disables the delete_breakpoints call early on in this proc. There are a couple of places in the testsuite where I have used: proc no_delete_breakpoints {} {} with_override delete_breakpoints no_delete_breakpoints { if {![runto_main]} { return } } In order to avoid the deleting all breakpoints when I call runto_main. I was about to add yet another instance of this pattern and I figured that it's time to do this properly. This commit adds the new option to 'runto' which causes the delete_breakpoints call to be skipped. And, we now forward any arguments from 'runto_main' through to 'runto', this means I can now just do: if {![runto_main no-delete-breakpoints]} { return } which I think is cleaner and easier to understand. I've updated the two tests I found that use the old with_override approach. There should be no change in what is tested after this commit. Approved-By: Tom Tromey <tom@tromey.com>
2024-08-28gdb: add 'maint info blocks' commandAndrew Burgess4-1/+253
While reviewing a patch I wanted to understand which blocks existed at a given address. The 'maint print symbols' command does provide some of this information, but that command displays all blocks within a given symtab. If I want to know which blocks are at a given address I have to figure that out for myself based on the output of 'maint print symbols' ... and I'm too lazy for that! So this command lists just those blocks at a given address, along with information about the blocks type. This new command doesn't list the symbols within each block, for that my expectation is that you'd cross reference the output with that of 'maint print symbols'. The new command format is: maintenance info blocks maintenance info blocks ADDRESS This lists the blocks at ADDRESS, or at the current $pc if ADDRESS is not given. Blocks are listed starting at the global block, then the static block, and then the progressively narrower scoped blocks. For each block we list the internal block pointer (which allows easy cross referencing with 'maint print symbols'), the inferior address range, along with other useful information. Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Simon Marchi <simon.marchi@efficios.com>
2024-08-28gdb: Add 'maint info inline-frames' commandAndrew Burgess5-58/+534
While reviewing a patch I wanted to view GDB's inline frame state. I don't believe there's currently a maintenance command to view this information, so in this commit I've added one. The new command is: maintenance info inline-frames maintenance info inline-frames ADDRESS The command lists the inline frames that start at ADDRESS, or at the current $pc if no ADDRESS is given. The command also displays the "outer" function in which the inline functions are present. An example of the command output: (gdb) maintenance info inline-frames Cached inline state information for thread 1. program counter = 0x401137 skipped frames = 1 bar > foo main (gdb) This tells us that function 'main' called 'foo' which called 'bar'. The functions 'foo' and 'bar' are both inline and both start at the address 0x401137. Currently GDB considers the inferior to be stopped in frame 'foo' (note the '>' marker), this means that there is 1 skipped frame (function 'bar'). The function 'main' is the outer function. The outer function might not start at 0x401137, it is simply the function that contains the inline functions. If the user does a 'step' then GDB will not actually move the inferior forward, but will instead simply tell the user that the inferior entered 'bar'. The output of 'maint info inline-frames' will change like this: (gdb) step bar () at inline.c:6 6 ++global_counter; (gdb) maintenance info inline-frames Cached inline state information for thread 1. program counter = 0x401137 skipped frames = 0 > bar foo main (gdb) Now GDB is in function 'bar' and there are no skipped frames. I have renamed skipped_symbols to function symbols within the inline_state class. We are now going to carry the "outer" function (the function that contains all the inlined functions) within this list (as the last entry), so the old name didn't really make sense. As a consequence of this rename I've updated some comments. I've changed stopped_by_user_bp_inline_frame to take a symbol rather than a block. Previously we just used the block to access the associated function symbol. After this commit we can just pass in the function symbol directly, so lets do that. New function gather_inline_frames contains some of the logic pulled from skip_inline_frames. This new function builds the list of all symbols of inlined functions that start at a given $pc value and also the "outer" function that contains all of the inlined functions. In skip_inline_frames I've split the loop logic into two. The loop to build the function symbol list has moved to gather_inline_frames. The loop to figure out how many of the inlined functions we are skipping remains in skip_inline_frames and uses the result of calling gather_inline_frames. In inline_skipped_symbol there are some minor updates to the comment, and I've tweaked one of the asserts now that the function symbols list also contains the "outer" function (a <= becomes <). The maintenance_info_inline_frames function is now and implements the new maintenance command. And _initialize_inline_frame is updated to register the new command. I've added a basic test for the new command. Please excuse the file name for the new test, in the next commit I'll be adding additional tests and at that point the file name will make sense. Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Simon Marchi <simon.marchi@efficios.com>
2024-08-28gdb: make symbols const in struct inline_stateAndrew Burgess4-7/+7
Make the inline_state::skipped_symbols a vector of 'const symbol *', adding the const qualifier. There's only a couple of places this leaks into the rest of GDB and in both places its fine for the symbol to become const. There should be no functional change after this commit. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2024-08-28Revert "gdb: remove inline_frame::skipped_frames"Andrew Burgess1-22/+29
This reverts commit 713e89012e43c83a6c1bb957c43ff58e5433336c. Having inline_state::skipped_frames back will make a later patch in this series easier.
2024-08-27[gdb/testsuite] Fix regexp in gdb.dwarf2/dw2-inter-cu-error.expTom de Vries1-1/+1
In commit b5070480d74 ("[gdb/symtab] Change DWARF_ERROR from Dwarf Error to DWARF Error") I changed the dwarf error prefix, but failed to update test-case gdb.dwarf2/dw2-inter-cu-error.exp. Fix this by updating the corresponding regexp in the test-case. Tested on x86_64-linux.
2024-08-27[gdb/python] Use GDB_PY_SET_HANDLE_EXCEPTION more oftenTom de Vries4-8/+4
I found a few more places where we can use GDB_PY_SET_HANDLE_EXCEPTION. Tested on x86_64-linux. Approved-By: Tom Tromey <tom@tromey.com>