Clang doesn't use CFA information for variable locations. This makes it
so software breakpoints get a false hit when rbp gets popped, causing
a FAIL in gdb.python/py-watchpoint.exp. Since this is nothing wrong with
GDB itself, add an xfail to reduce noise.
Approved-By: Tom Tromey <tom@tromey.com>
The test gdb.python/py-explore-cc.exp was showing one unexpected
failure. This was due to how clang mapped instructions to lines,
resulting in the inferior seemingly stopping at a different location.
This patch adds a nop line in the relevant location so we don't need to
add XFAILs for existing clang releases, if this gets solved in future
versions.
Approved-By: Tom Tromey <tom@tromey.com>
get_symbol_address is only used symbol::value_address, make it a private
helper method.
Change-Id: I318ddcfcf1269d95045b8efe9137812df9c5113c
Approved-By: Tom Tromey <tom@tromey.com>
get_msymbol_address is only used in minimal_symbol::value_address. Make
it a private helper method.
Change-Id: I3f30e1b9d89ace6682fb08a7ebb91746db0ccf0f
Approved-By: Tom Tromey <tom@tromey.com>
When printing a value, I think the history reference -- the "$1" in
the output -- should be styled using the "variable" style. This patch
implements this.
Commit 8971d2788e ("gdb: link so_list using intrusive_list") introduced
a bug in clear_solib. Instead of passing an `so_list *` to
remove_target_sections, it passed an `so_list **`. This was not caught
by the compiler, because remove_target_sections takes a `void *` as the
"owner", so you can pass it any pointer and it won't complain.
This happened because I previously had a patch to change the type of the
disposer parameter to be a reference rather than a pointer, so had to
change `so` to `&so`. When dropping that patch, I forgot to revert this
bit and / or it got re-introduced when handling subsequent merge
conflicts. And I didn't properly retest.
Fix that, but try to make things less error prone. Add a union to
represent the possible owner kinds for a target_section. Trying to pass
a pointer to another type than those will not compile.
Change-Id: I600cab5ea0408ccc5638467b760768161ca3036c
Currently when gdb asks the source-highlight library to highlight a file, it
tells it what language file to use.
For instance, if gdb learns from the debug info that the file is language_c,
the language file "c.lang" is used. This mapping is hardcoded in
get_language_name.
However, if gdb doesn't know what language file to use, it falls back to using
python pygments, and in absence of that, unhighlighted source text.
In the case of python pygments, it autodetects which language to use based on
the file name.
Add the same capability when using the source-highlight library.
Tested on x86_64-linux.
Verified that it works by:
- making get_language_name return nullptr for language_c, and
- checking that source-highlight still manages to highlight a hello world.
Reviewed-By: Guinevere Larsen <blarsen@redhat.com>
Approved-By: Tom Tromey <tom@tromey.com>
PR cli/30966
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30966
dwarf2/read.h includes cooked-index.h, but it doesn't need to. This
patch removes the inclusion from this header, and adds one to
index-write.c to make up for the absence.
I noticed a few more style issues in commit 8b9c08edda ("[gdb/symtab] Add
name_of_main and language_of_main to the DWARF index"), after checking it
with gcc's check_GNU_style.{sh,py}.
Fix these.
Build on x86_64-linux.
The recent change to record the DWARF language in the per-CU data
yielded a race warning in my testing:
ThreadSanitizer: data race ../../binutils-gdb/gdb/dwarf2/read.c:21779 in prepare_one_comp_unit
This patch fixes the bug by applying the same style of fix that was
done for the ordinary (gdb) language.
I wonder if this code could be improved. Requiring an atomic for the
language in particular seems unfortunate, as it is often consulted
during index finalization. However, I haven't investigated this.
Regression tested on x86-64 Fedora 38.
Reviewed-by: Tom de Vries <tdevries@suse.de>
Fixes:
CXX solib-target.o
/home/smarchi/src/binutils-gdb/gdb/solib-target.c:57:8: error: ‘lm_info_vector’ does not name a type
57 | static lm_info_vector
| ^~~~~~~~~~~~~~
/home/smarchi/src/binutils-gdb/gdb/solib-target.c: In function ‘intrusive_list<shobj> solib_target_current_sos()’:
/home/smarchi/src/binutils-gdb/gdb/solib-target.c:244:7: error: ‘solib_target_parse_libraries’ was not declared in this scope
244 | = solib_target_parse_libraries (library_document->data ());
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Change-Id: Ib477d3343b401017d79729118242143bc95f24b2
Now that so_list lists are implemented using intrusive_list, it doesn't
really make sense for the element type to be named "_list". Rename to
just `struct shobj` (`struct so` was deemed to be not greppable enough).
Change-Id: I1063061901298bb40fee73bf0cce44cd12154c0e
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Remove this function, replace it with deleting the so_list in callers.
Change-Id: Idbd0cb84674ade1d8e17af471550dbd388264f60
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
I think this `so.clear ()` call is not useful.
- so_list::clear deletes some things that now get automatically deleted
when the so_list gets deleted right after in free_so.
- so_list::clear resets some scalar fields of so_list, which we don't
really care about since the so_list gets deleted right after.
- so_list::clear calls target_so_ops::clear_so, of which there is a
single implementation, svr4_clear_so. That implementation just
resets a field in lm_info_svr4, which we don't care about, as it will
get deleted when the so_list gets deleted right after.
Change-Id: Ie4d72f2a04a4129e55c460bb5c69bc0af0d12b32
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Replace the hand-made linked list implementation with intrusive_list,
simplying management of list items.
Change-Id: I7f55fd88325bb197cc655c9be5a2ec966d8cc48d
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Change these two fields, simplifying memory management and copying.
Change-Id: If2559284c515721e71e1ef56ada8b64667eebe55
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Change the field from a `bfd *` to a gdb_bfd_ref_ptr to automatically
manage the reference.
Change-Id: I3ace18bea985bc194c5e67bb559eec567e258950
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Make the field a vector directly, instead of a pointer to a vector.
This was needed when so_list had to be a trivial type, which is not the
case anymore.
Change-Id: I79a8378ce0d0d1e2206ca08a273ebf332cb3ba14
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Remove this typedef. I think that hiding the real type (std::vector)
behind a typedef just hinders readability.
Change-Id: I80949da3392f60a2826c56c268e0ec6f503ad79f
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
... just because it seems to make sense to do so.
Change-Id: Ie283c92d9b90c54e3deee96a43c6a942d8b5910b
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Make it a unique_ptr, so it gets automatically deleted when the so_list
is deleted.
Change-Id: Ib62d60ae2a80656239860b80e4359121c93da13d
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
I think this typedef hinders readability. First, it's not well named
(it's not clear it contains lm_info_target objects). And hiding the
fact that it contains unique pointers is not very useful either. I was
looking at the code in solib_target_current_sos where the unique
pointers get moved from the vector, and it wasn't obvious at all what
the source of the move was.
Change-Id: I4a5cda7c90554f018b7c466b1535b41d69cbcbe7
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Same rationale as the previous patch, but for solib-rocm.
- Introduce rocm_so, which is a name a unique_name (see comment in
rocm_update_solib_list for that) and a unique_ptr to the
lm_info_svr4.
- Change the internal lists from so_list lists to vectors of rocm_so.
- Remove rocm_free_solib_list, as everything is automatic now.
- Replace rocm_solib_copy_list with so_list_from_rocm_sos.
Change-Id: I71e06e3ea22d6420c9e4e500501c06e9a13398a8
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
A subsequent patch makes use of non-trivial types in struct so_list.
This trips on the fact that svr4_copy_library_list uses memcpy to copy
so_list objects:
so_list *newobj = new so_list;
memcpy (newobj, src, sizeof (struct so_list));
solib-svr4 maintains lists of so_list objects in its own internal data
structures. When requested to return a list of so_list objects (through
target_so_ops::current_sos), it duplicates the internal so_list lists,
using memcpy. When changing so_list to make it non-trivial, we would
need to replace this use of memcpy somehow. That would mean making
so_list copyable, with all the complexity that entails, just to satisfy
this internal usage of solib-svr4 (and solib-rocm, which does the same).
Change solib-svr4 to use its own data type for its internal lists. The
use of so_list is a bit overkill anyway, as most fields of so_list are
irrelevant for this internal use.
- Introduce svr4_so, which contains just an std::string for the name
and a unique_ptr for the lm_info.
- Change the internal so_list lists to be std::vector<svr4_so>. Vector
seems like a good choice for this, we don't need to insert/remove
elements in the middle of these internal lists.
- Remove svr4_free_library_list, free_solib_lists and ~svr4_info, as
everything is managed automatically now.
- Replace svr4_copy_library_list (which duplicated internal lists in
order to return them to the core) with so_list_from_svr4_sos, which
creates an so_list list from a vector of svr4_so.
- Generalize svr4_same a bit, because find_debug_base_for_solib now
needs to compare an so_list and an svr4_so to see if they are the
same.
Change-Id: I6012e48e07aace2a8172b74b389f9547ce777877
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Now that the lm_info class hierarchy has a virtual destructor and
therefore a vtable, use checked_static_cast instead of C-style cases to
ensure (when building in dev mode) that we're casting to the right kind
of lm_info.
Change-Id: I9a99b7d6aa9a44edbe76377d57a7008cfb75a744
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
target_so_ops::free_so is responsible for freeing the specific lm_info
object. All implementations basically just call delete. Remove that
method, make the destructor of lm_info virtual, and call delete directly
from the free_so function. Make the sub-classes final, just because
it's good practice.
Change-Id: Iee1fd4861c75034a9e41a656add8ed8dfd8964ee
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
The base class doesn't need to have "_base" in its name, all the
sub-classes have a specific suffix.
Change-Id: I87652105cfedd87898770a81f0eda343ff7f2bdb
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Initialize all fields in the class declaration, change allocations to
use "new", change deallocations to use "delete". This is needed by a
subsequent patches that use C++ stuff in so_list.
Change-Id: I4b140d9f1ec9ff809554a056f76e3eb2b9e23222
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
It is only used in solib.c.
Change-Id: I43461d13d84d65c4f6913d4033678d8983b9910b
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
A subsequent patch changes so_list to be linked using
intrusive_list. Iterating an intrusive_list yields some references to
the list elements. Convert some functions accepting so_list objects to
take references, to make things easier and more natural. Add const
where possible and convenient.
Change-Id: Id5ab5339c3eb6432e809ad14782952d6a45806f3
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
A subsequent patch changes the interp::on_solib_loaded and
interp::on_solib_unloaded methods to take references. This highlighted
that interps_notify did not work with reference parameters.
Fix that by changing interps_notify's `args` arg to be a universal
reference (&&). Change the type of the method to be auto-deduced as an
additional template parameter, otherwise the signature of the callback
function would never match:
CXX interps.o
/home/simark/src/binutils-gdb/gdb/interps.c: In function ‘void interps_notify_signal_received(gdb_signal)’:
/home/simark/src/binutils-gdb/gdb/interps.c:378:18: error: no matching function for call to ‘interps_notify(void (interp::*)(gdb_signal), gdb_signal&)’
378 | interps_notify (&interp::on_signal_received, sig);
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/simark/src/binutils-gdb/gdb/interps.c:363:1: note: candidate: ‘template<class ... Args> void interps_notify(void (interp::*)(Args ...), Args&& ...)’
363 | interps_notify (void (interp::*method) (Args...), Args&&... args)
| ^~~~~~~~~~~~~~
/home/simark/src/binutils-gdb/gdb/interps.c:363:1: note: template argument deduction/substitution failed:
/home/simark/src/binutils-gdb/gdb/interps.c:378:18: note: inconsistent parameter pack deduction with ‘gdb_signal’ and ‘gdb_signal&’
378 | interps_notify (&interp::on_signal_received, sig);
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Change-Id: I0cd9378e24ef039f30f8e14f054f8d7fb539c838
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
The clear_solib is implicitly meant to clear the resources associated to
the current program space (that's what the solib implementations that
actually support multi-program-space / multi-inferior do). Make that
explicit by adding a program_space parameter and pass down
current_program_space in call sites. The implementation of the
clear_solib callbacks is fairly simple, I don't think any of them rely
on global state other than accessing current_program_space.
Change-Id: I8d0cc4db7b4f8db8d7452879c0c62db03269bf46
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Make the target_so_ops::clear_solib method optional, remove two empty
implementations.
Change-Id: Ifda297d50c74327d337091c58cdb5b3b60382591
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
The gdb.rocm/multi-inferior-gpu.exp testcase uses a "continue $thread"
command, but this is incorrect. If "continue" is given an argument, it
sets the ignore count of the breakpoint the thread stopped at.
For this testcase it does not really matter since the breakpoint is not
meant to be hit anymore, so whatever the ignore count is won't influence
the outcome of the test. It is worth fixing nevertheless.
Change-Id: I0eb674d5529cdeb9e808b74870a29b6077265737
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Functions of the hip runtime returning a hipError_t can be marked
nodiscard depending on the configuration[1] (when compiled with C++17).
This patch makes sure that we always check the value returned by
hipDeviceSynchronize and friends, and print an error message when
appropriate. This avoid a wall of warnings when running the testsuite
if the compiler defaults to using C++17.
It is always a good practice to check the return values anyway.
[1] https://github.com/ROCm-Developer-Tools/HIP/blob/docs/5.7.1/include/hip/hip_runtime_api.h#L203-L218
Change-Id: I2a819a8ac45f4bcf814efe9a2ff12c6a7ad22f97
Approved-By: Simon Marchi <simon.marchi@efficios.com>
When running test-case gdb.base/jit-bfd-name.exp, I run into:
...
ERROR: tcl error sourcing gdb/testsuite/gdb.base/jit-bfd-name.exp.
ERROR: can't read "start": no such variable
...
The problem is that commit c96ceed9dc ("gdb: include the end address in
in-memory bfd filenames") introduced a use of variable start, but not a
definition.
Fix this by adding the missing definition.
Tested on x86_64-linux.
Post-commit review pointed out a few style issues in commit 8b9c08edda
("[gdb/symtab] Add name_of_main and language_of_main to the DWARF index").
Fix these.
Tested on x86_64-linux.
Reported-By: Tom Tromey <tom@tromey.com>
Approved-By: Tom Tromey <tom@tromey.com>
Commit
66984afd29 gdb: include the base address in in-memory bfd filenames
added the base address to in-memory bfd filenames. Also add the end
address to allow dumping the in-memory bfd using the 'dump memory'
command.
In source_cache::ensure, keep track of which files failed to be styled, and
don't attempt to style them again in case the file dropped out of the cache.
Tested on x86_64-linux.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Function source_cache::ensure contains some code using the GNU
source-highlight library.
The code is a sizable part of the function, and contains conditional
compilation in a slightly convoluted way:
...
if (!already_styled)
#endif /* HAVE_SOURCE_HIGHLIGHT */
{
...
Fix this by factoring out the code into new function try_source_highlight,
such that:
- source_cache::ensure is easier to read, and
- the conditional compilation is at the level of the function body.
Tested on x86_64-linux.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
In function source_cache::ensure we have:
...
std::ostringstream output;
...
contents = output.str ();
...
The last line causes an unnecessary string copy.
C++20 allows us to skip it, like this:
...
contents = std::move (output).str ();
...
Use the more efficient solution.
Tested on x86_64-linux.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
This header is only suitable for use on x86 hosts and is only included
there, so these fallbacks should not be needed.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
A pretty-printer's 'children' method may return values other than a
gdb.Value -- it may return any value that can be converted to a
gdb.Value.
I noticed that this case did not work for DAP. This patch fixes the
problem.
Andry pointed out that the DAP code did not properly handle
gdb.LazyString results from a pretty-printer, yielding:
TypeError: Object of type LazyString is not JSON serializable
This patch fixes the problem, partly with a small patch in varref.py,
but mainly by implementing tp_str for LazyString.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Andry noticed that given a DAP setExpression request, where the
expression to set is a register, DAP will return the wrong value -- it
will return the old value, not the updated one.
This happens because gdb.Value.assign (which was recently added for
DAP) does not update the value.
In this patch, I chose to have the assign method update the Value
in-place. It's also possible to have it return a new value, but this
didn't seem very useful to me.
Andry Ogorodnik, a co-worker, noticed that multiple "scopes" requests
with the same frame would yield different variableReference values in
the response.
This patch adds a regression test for this, and adds a scope cache in
scopes.py, ensuring that multiple identical requests will get the same
response.
Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
