During breakpoint re-setting, the source_filename of an
explicit_location_spec is used to lookup the symtabs associated with
the breakpoint being re-set. This source_filename is compared with each
known symtab filename in order to retrieve the breakpoint's symtabs.
However the source_filename may have been originally copied from a
symtab's fullname (the path where GDB found the source file) when the
breakpoint was first created. If a breakpoint symtab's filename and
fullname differ and there is no substitute-path rule that converts the
fullname to the filename, this will cause a NOT_FOUND_ERROR to be thrown
during re-setting.
Fix this by using a symtab's filename to set the explicit_location_spec
source_filename instead of the symtab's fullname.
Although the bool want_start_sal isn't actually used without being assigned
a value, initialize it to be false in order to prevent the following
-Wmaybe-uninitialized warning:
linespec.c: In function ‘void minsym_found(linespec_state*, objfile*, minimal_symbol*, std::vector<symtab_and_line>*)’:
linespec.c:4150:19: warning: ‘want_start_sal’ may be used uninitialized [-Wmaybe-uninitialized]
4150 | if (is_function && want_start_sal)
This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
[I sent this earlier today, but I don't see it in the archives.
Resending it through a different computer / SMTP.]
The use of the static buffer in command_line_input is becoming
problematic, as explained here [1]. In short, with this patch [2] that
attempt to fix a post-hook bug, when running gdb.base/commands.exp, we
hit a case where we read a "define" command line from a script file
using command_command_line_input. The command line is stored in
command_line_input's static buffer. Inside the define command's
execution, we read the lines inside the define using command_line_input,
which overwrites the define command, in command_line_input's static
buffer. After the execution of the define command, execute_command does
a command look up to see if a post-hook is registered. For that, it
uses a now stale pointer that used to point to the define command, in
the static buffer, causing a use-after-free. Note that the pointer in
execute_command points to the dynamically-allocated buffer help by the
static buffer in command_line_input, not to the static object itself,
hence why we see a use-after-free.
Fix that by removing the static buffer. I initially changed
command_line_input and other related functions to return an std::string,
which is the obvious but naive solution. The thing is that some callees
don't need to return an allocated string, so this this an unnecessary
pessimization. I changed it to passing in a reference to an std::string
buffer, which the callee can use if it needs to return
dynamically-allocated content. It fills the buffer and returns a
pointers to the C string inside. The callees that don't need to return
dynamically-allocated content simply don't use it.
So, it started with modifying command_line_input as described above, all
the other changes derive directly from that.
One slightly shady thing is in handle_line_of_input, where we now pass a
pointer to an std::string's internal buffer to readline's history_value
function, which takes a `char *`. I'm pretty sure that this function
does not modify the input string, because I was able to change it (with
enough massaging) to take a `const char *`.
A subtle change is that we now clear a UI's line buffer using a
SCOPE_EXIT in command_line_handler, after executing the command.
This was previously done by this line in handle_line_of_input:
/* We have a complete command line now. Prepare for the next
command, but leave ownership of memory to the buffer . */
cmd_line_buffer->used_size = 0;
I think the new way is clearer.
[1] https://inbox.sourceware.org/gdb-patches/becb8438-81ef-8ad8-cc42-fcbfaea8cddd@simark.ca/
[2] https://inbox.sourceware.org/gdb-patches/20221213112241.621889-1-jan.vrany@labware.com/
Change-Id: I8fc89b1c69870c7fc7ad9c1705724bd493596300
Reviewed-By: Tom Tromey <tom@tromey.com>
With gcc 4.8/4.9, we run into this build failure (and other similar
ones):
/home/palves/gdb/binutils-gdb/src/gdb/location.h:224:59: error: could not convert ‘{0, LINE_OFFSET_UNKNOWN}’ from ‘<brace-enclosed initializer list>’ to ‘line_offset’
struct line_offset line_offset = {0, LINE_OFFSET_UNKNOWN};
^
The issue is that at around the GCC 4.8/4.9 era, a default member
initializer prevented the struct from being an aggregate, so you
cannot use aggregate initialization on them. That rule changed after
GCC 4.9 and GCC 5 & later uses new rules.
Fix this by not using aggregate initialization for struct line_offset.
The default member initization already leaves line_offset as {0,
LINE_OFFSET_UNKNOWN}, so initialization to those values can just go
away. The remaining cases are of the form {0, LINE_OFFSET_NONE}, and
those cases can be better rewritten to delay setting the sign field
until we know we have a valid offset.
Change-Id: I0506ea4a83c5fa2f15e159569db68b3b0a7509b4
This converts set_location_spec_string to a method of location_spec,
and makes the location_spec::as_string field protected, renaming it to
m_as_string along the way.
Change-Id: Iccfb1654e9fa7808d0512df89e775f9eacaeb9e0
This converts location_spec_to_string to a method of location_spec,
simplifying the code using it, as it no longer has to use
std::unique_ptr::get().
Change-Id: I621bdad8ea084470a2724163f614578caf8f2dd5
copy_location_spec is just a wrapper around location_spec::clone(), so
remove it and call clone() directly. This simplifies users, as they
no longer have to use std::unique_ptr::get().
Change-Id: I8ce8658589460b98888283b306b315a5b8f73976
Currently, there's the location_spec hierarchy, and then some
location_spec subclasses have their own struct type holding all their
data fields.
I.e., there is this:
location_spec
explicit_location_spec
linespec_location_spec
address_location_spec
probe_location_spec
and then these separate types:
explicit_location
linespec_location
where:
explicit_location_spec
has-a explicit_location
linespec_location_spec
has-a linespec_location
This patch eliminates explicit_location and linespec_location,
inlining their members in the corresponding location_spec type.
The location_spec subclasses were the ones currently defined in
location.c, so they are moved to the header. Since the definitions of
the classes are now visible, we no longer need location_spec_deleter.
Some constructors that are used for cloning location_specs, like:
explicit explicit_location_spec (const struct explicit_location *loc)
... were converted to proper copy ctors.
In the process, initialize_explicit_location is eliminated, and some
functions that returned the "data type behind a locspec", like
get_linespec_location are converted to downcast functions, like
as_linespec_location_spec.
Change-Id: Ia31ccef9382b25a52b00fa878c8df9b8cf2a6c5a
Currently, GDB internally uses the term "location" for both the
location specification the user input (linespec, explicit location, or
an address location), and for actual resolved locations, like the
breakpoint locations, or the result of decoding a location spec to
SaLs. This is expecially confusing in the breakpoints module, as
struct breakpoint has these two fields:
breakpoint::location;
breakpoint::loc;
"location" is the location spec, and "loc" is the resolved locations.
And then, we have a method called "locations()", which returns the
resolved locations as range...
The location spec type is presently called event_location:
/* Location we used to set the breakpoint. */
event_location_up location;
and it is described like this:
/* The base class for all an event locations used to set a stop event
in the inferior. */
struct event_location
{
and even that is incorrect... Location specs are used for finding
actual locations in the program in scenarios that have nothing to do
with stop events. E.g., "list" works with location specs.
To clean all this confusion up, this patch renames "event_location" to
"location_spec" throughout, and then all the variables that hold a
location spec, they are renamed to include "spec" in their name, like
e.g., "location" -> "locspec". Similarly, functions that work with
location specs, and currently have just "location" in their name are
renamed to include "spec" in their name too.
Change-Id: I5814124798aa2b2003e79496e78f95c74e5eddca
I noticed a few spots in GDB that use "typedef enum". However, in C++
this isn't as useful, as the tag is automatically entered as a
typedef. This patch removes most uses of "typedef enum" -- the
exceptions being in some nat-* code I can't compile, and
glibc_thread_db.h, which I think is more or less a copy of some C code
from elsewhere.
Tested by rebuilding.
Replace with calls to blockvector::blocks, and the appropriate method
call on the returned array_view.
Change-Id: I04d1f39603e4d4c21c96822421431d9a029d8ddd
Add a getter and a setter for a minimal symbol's type. Remove the
corresponding macro and adjust all callers.
Change-Id: I89900df5ffa5687133fe1a16b2e0d4684e67a77d
Same idea as previous patch, but for symtab::objfile. I find
it clearer without this wrapper, as it shows that the objfile is
common to all symtabs of a given compunit. Otherwise, you could think
that each symtab (of a given compunit) can have a specific objfile.
Change-Id: Ifc0dbc7ec31a06eefa2787c921196949d5a6fcc6
symtab::blockvector is a wrapper around compunit_symtab::blockvector.
It is a bit misleadnig, as it gives the impression that a symtab has a
blockvector. Remove it, change all users to fetch the blockvector
through the compunit instead.
Change-Id: Ibd062cd7926112a60d52899dff9224591cbdeebf
This adds a constructor to bound_minimal_symbol, to avoid a build
failure with clang that Simon pointed out.
I also took the opportunity to remove some redundant initializations,
and to change one use of push_back to emplace_back, as suggested by
Simon.
Add a getter and a setter for a symbol's line. Remove the corresponding macro
and adjust all callers.
Change-Id: I229f2b8fcf938c07975f641361313a8761fad9a5
Add a getter and a setter for a symbol's type. Remove the corresponding
macro and adjust all callers.
Change-Id: Ie1a137744c5bfe1df4d4f9ae5541c5299577c8de
Add a getter and a setter for whether a symbol is inlined. Remove the
corresponding macro and adjust all callers.
Change-Id: I934468da3b5a32dd6b161a6f252a6b1b94737279
Add a getter and a setter for a symbol's domain. Remove the
corresponding macro and adjust all callers.
Change-Id: I54465b50ac89739c663859a726aef8cdc6e4b8f3
Add a getter and a setter for a symtab's language. Remove the
corresponding macro and adjust all callers.
Change-Id: I9f4d840b11c19f80f39bac1bce020fdd1739e11f
I think that most remaining uses of COMPUNIT_FILETABS intend to get the
primary filetab of the compunit_symtab specifically (and not to iterate
over all filetabs, for example, those cases would use compunit_filetabs,
which has been converted to compunit_symtab::filetabs), so replace mosts
uses with compunit_symtab::primary_filetab.
In jit.c, function finalize_symtab, we can save the symtab object
returned by allocate_symtab and use it, it makes things simpler.
Change-Id: I4e51d6d4b40759de8768b61292e5e13c8eae2e38
If parse_linespec (linespec.c) is passed ARG as an empty string then
we end up calling `strchr (linespec_quote_characters, '\0')`, which
will return a pointer to the '\0' at the end of
linespec_quote_characters. This then results in GDB calling
skip_quote_char with `ARG + 1`, which is undefined behaviour (as ARG
only contained a single character, the '\0').
Fix this by checking for the first character of ARG being '\0' before
the call to strchr.
I have additionally added an assertion that ARG can't itself be
nullptr, as calling is_ada_operator with nullptr can end up calling
'startswith' on the nullptr, which is undefined behaviour.
Finally, I moved the declaration of TOKEN into the body of
parse_linespec, to where TOKEN is defined.
This patch came about while I was working on fixes for PR cli/28665
and PR gdb/28797. The actual fixes for these two issues will be in a
later commit in this series, but, with this patch in place, both of
the above bugs would hit the new assertion rather than accessing
invalid memory and crashing. The '\0' check is not currently ever
hit, but just makes the code a little safer.
Because this patch only changes the nature of the failure for the
above two bugs, there's no tests here. A later commit will fix the
above two issues, at which point I'll add some tests.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28665
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28797
This small cleanup removes a use of xfree from location.c, by
switching to unique_xmalloc_ptr. One function is only used in
location.c, so it is made static. And, another function is changed to
avoid a copy.
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.
struct linespec contains pointers to vectors, instead of containing
vectors directly. This is probably historical, when linespec_parser
(which contains a struct linespec field) was not C++-ified yet. But it
seems easy to change the pointers to vectors to just vectors today.
This simplifies the code, we don't need to manually allocate and delete
the vectors and there's no pointer that can be NULL.
As far as I understand, there was not meaningful distinction between a
NULL pointer to vector and an empty vector. So all NULL checks are
changed for !empty checks.
Change-Id: Ie759707da14d9d984169b93233343a86e2de9ee6
We can remove the empty check: if the vector has size 1, it is obviously
not empty. This code ended up like this because the empty check used to
be a NULL check.
Change-Id: I1571bd0228818ca93f6a6b444e9b010dc2da4c08
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
I guess this was used with the old VEC implementation, but there is no
reason to have this typedef anymore.
gdb/ChangeLog:
* linespec.c (linespec_p): Remove. Replace all uses with
"linespec *".
Change-Id: I4cea59ae1cd46985da9c08d3a69686846b1ad028
The current_top_target function is a hidden dependency on the current
inferior. Since I'd like to slowly move towards reducing our dependency
on the global current state, remove this function and make callers use
current_inferior ()->top_target ()
There is no expected change in behavior, but this one step towards
making those callers use the inferior from their context, rather than
refer to the global current inferior.
gdb/ChangeLog:
* target.h (current_top_target): Remove, make callers use the
current inferior instead.
* target.c (current_top_target): Remove.
Change-Id: Iccd457036f84466cdaa3865aa3f9339a24ea001d
This introduces wrappers for each function in quick_symbol_functions.
The wrappers are methods on objfile, and are defined in
symfile-debug.c, so that they can use the symfile_debug variable.
Places that call the quick functions are all updated to call these new
wrapper methods.
gdb/ChangeLog
2021-03-20 Tom Tromey <tom@tromey.com>
* symtab.c (iterate_over_symtabs, expand_symtab_containing_pc)
(lookup_symbol_via_quick_fns, find_quick_global_symbol_language)
(basic_lookup_transparent_type_quick)
(find_pc_sect_compunit_symtab, find_symbol_at_address)
(find_line_symtab, global_symbol_searcher::expand_symtabs):
Update.
* symmisc.c (print_objfile_statistics, dump_objfile)
(maintenance_expand_symtabs): Update.
* symfile.c (symbol_file_add_with_addrs)
(expand_symtabs_matching, map_symbol_filenames): Update.
* symfile-debug.c (objfile::has_partial_symbols)
(objfile::find_last_source_symtab)
(objfile::forget_cached_source_info)
(objfile::map_symtabs_matching_filename, objfile::lookup_symbol)
(objfile::print_stats, objfile::dump)
(objfile::expand_symtabs_for_function)
(objfile::expand_all_symtabs)
(objfile::expand_symtabs_with_fullname)
(objfile::map_matching_symbols)
(objfile::expand_symtabs_matching)
(objfile::find_pc_sect_compunit_symtab)
(objfile::map_symbol_filenames)
(objfile::find_compunit_symtab_by_address)
(objfile::lookup_global_symbol_language): New methods.
(debug_sym_quick_functions): Remove.
(debug_sym_fns, install_symfile_debug_logging): Update.
* source.c (forget_cached_source_info_for_objfile)
(select_source_symtab): Update.
* objfiles.h (struct objfile): Add methods corresponding to
quick_symbol_functions.
* objfiles.c (objfile::has_partial_symbols): Move to
symfile-debug.c.
* linespec.c (iterate_over_all_matching_symtabs): Update.
* cp-support.c (add_symbol_overload_list_qualified): Update.
* ada-lang.c (add_nonlocal_symbols): Update.
This commits the result of running gdb/copyright.py as per our Start
of New Year procedure...
gdb/ChangeLog
Update copyright year range in copyright header of all GDB files.
The break command's "-force-condition" flag is currently required to
be followed by the "if" keyword. This prevents flexibility when using
other keywords, e.g. "thread":
(gdb) break main -force-condition thread 1 if foo
Function "main -force-condition" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
Remove the requirement that "-force-condition" is always followed by
an "if", so that more flexibility is obtained when positioning
keywords.
gdb/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* linespec.c (linespec_lexer_lex_keyword): The "-force-condition"
keyword may be followed by any keyword.
* breakpoint.c (find_condition_and_thread): Advance 'tok' by
'toklen' in the case for "-force-condition".
gdb/testsuite/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.linespec/keywords.exp: Add tests to check positional
flexibility of "-force-condition".
