| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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.
|
|
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.
|
|
Now that the nto port is removed, this is unused.
Change-Id: I86565310cdbcde17a837eb10585cdd153f4f03d8
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
Make the current program space reference bubble up one level.
Change-Id: I6113c9ef57cb31ca8ea129ab58e7c318c09b5123
|
|
Make the current program space reference bubble up one level. Pass
`current_program_space` everywhere, except in some cases where we can
get the pspace another way, and it's relatively obvious that it's the
same as the current program space.
Change-Id: Id86b79f1e44f92a398f49d137d57457174dfa96d
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
The `no_shared_libraries` function is currently used to implement the
`nosharedlibrary` command, but it also used internally by other
functions. This does not make a very good internal API.
Add the `no_shared_libraries_command` function to implement the CLI
command. Remove the unused parameters from `no_shared_libraries`.
Remove the `from_tty` parameter of `target_pre_inferior`, since it's now
unused.
Change-Id: I4fcba5ee1e0f7d250aab1a7b62b9ea16265fe962
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
Make the current program space reference bubble up one level.
Change-Id: I08cfa77a0351c9602131ed2a294eabb1f1f59a6e
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
Rename to m_pspace, add getter. An objfile's pspace never changes, so
no setter is necessary.
Change-Id: If4dfb300cb90dc0fb9776ea704ff92baebb8f626
|
|
Remove some includes reported as unused by clangd. Add some includes in
other files that were previously relying on the transitive include.
Change-Id: Ibdd0a998b04d21362a20d0ca8e5267e21e2e133e
|
|
I spotted a few places in solib.c and build-id.c where we could apply
file name styling.
Other than the extra styling, there should be no user visible changes
after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Move the declarations out of defs.h, and the implementations out of
findvar.c.
I opted for a new file, because this functionality of converting
integers to bytes and vice-versa seems a bit to generic to live in
findvar.c.
Change-Id: I524858fca33901ee2150c582bac16042148d2251
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
Now that defs.h, server.h and common-defs.h are included via the
`-include` option, it is no longer necessary for source files to include
them. Remove all the inclusions of these files I could find. Update
the generation scripts where relevant.
Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837
Approved-By: Pedro Alves <pedro@palves.net>
|
|
solib_map_sections errors out if the library file name is longer than
SO_NAME_MAX_PATH_SIZE.
solib::so_name and solib::so_original_name used to be arrays of
SO_NAME_MAX_PATH_SIZE size, so that check made sense then.
However, since commit 98107b0b17ac ("gdb: make
so_list::{so_original_name,so_name} std::strings") those fields are of
std::string type, so there's really no need for the limit.
This commit simply removes the length limit check.
Approved-By: John Baldwin <jhb@FreeBSD.org>
Change-Id: I2ec676b231cd18ae900c61c5caea461f47e989e6
|
|
Since commit 74daa597e74 ("gdb: add all_objfiles_removed observer"), the
objfile passed to the free_objfile observable can't be nullptr.
Change-Id: If215aa051ab43c068b11746938022c7efca09caa
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Make the current_program_space reference bubble up one level.
Remove one unnecessary declaration of clear_solib.
Change-Id: I234e2c8c0b71713364fc7b76cee2bee2b026bd6d
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Make the current_program_space reference bubble up one level.
Change-Id: Ide917aa306bff1872d961244901d79f65d2da62e
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
I don't like the name `target_so_ops`, because:
- The name `target` is so overloaded, and in this case it's not even
related to target_ops or anything else called "target".
- We do have an implementation that actually fetches solibs from the
target (solib_target_so_op in solib-target.c), so it's confusing for
the "base class" to be called target_something as well.
Rename to solib_ops.
Change-Id: I46a983d44e81400470e22deb09aaf26ad8a3587f
Approved-By: Tom Tromey <tom@tromey.com>
|
|
`struct so_list` was recently renamed to `struct shobj` (in 3fe0dfd1604f
("gdb: rename struct so_list to shobj")). In hindsight, `solib` would
have been a better name. We have solib.c, the implementations in
solib-*.c, many functions with solib in their name, the solib_loaded /
solib_unloaded observables, etc.
Rename shobj to solib.
Change-Id: I0af1c7a9b29bdda027e9af633f6d37e1cfcacd5d
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This commit is the result of the following actions:
- Running gdb/copyright.py to update all of the copyright headers to
include 2024,
- Manually updating a few files the copyright.py script told me to
update, these files had copyright headers embedded within the
file,
- Regenerating gdbsupport/Makefile.in to refresh it's copyright
date,
- Using grep to find other files that still mentioned 2023. If
these files were updated last year from 2022 to 2023 then I've
updated them this year to 2024.
I'm sure I've probably missed some dates. Feel free to fix them up as
you spot them.
|
|
Consider a binary with an erroneous size of the .dynamic section:
$ readelf -S a.out
...
[24] .dynamic DYNAMIC 0000000000004c20 00003c20
000000fffffffa40 0000000000000010 WA 7 0 8
...
This binary causes a segfault in GDB. GDB is trying to write the .dynamic
section into memory allocated on the stack with alloca(). However, the
allocation silently fails and the subsequent access to the memory is
causing the segfault. (On my node at least.)
Stack allocation is a bad idea for something of variable size that GDB has
no control over. So I changed the code to heap allocation.
In addition, I changed the type of sect_size to the type that bfd actually
returns.
There should be no user visible change after this.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Commit 8971d2788e7 ("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
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
This function is just a wrapper around the current inferior's gdbarch.
I find that having that wrapper just obscures where the arch is coming
from, and that it's often used as "I don't know which arch to use so
I'll use this magical target_gdbarch function that gets me an arch" when
the arch should in fact come from something in the context (a thread,
objfile, symbol, etc). I think that removing it and inlining
`current_inferior ()->arch ()` everywhere will make it a bit clearer
where that arch comes from and will trigger people into reflecting
whether this is the right place to get the arch or not.
Change-Id: I79f14b4e4934c88f91ca3a3155f5fc3ea2fadf6b
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Use objfile->pspace instead of current_program_space.
Change-Id: I127a1788e155b321563114452ed5b530f1d1f618
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This backlink is not necessary, we always know the program space from
the context. Pass it down the solib_unloaded observer.
Change-Id: I45a503472dc791f517558b8141901472634e0556
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I'm starting to work on these files, I thought it would be a good time
to remove unused imports. These were identified by
include-what-you-use. Tested by rebuilding.
Change-Id: I3eaf3fa0ea3506c7ecfbc8ecff5031433b1dadb8
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
|
|
I noticed a comment by an include and remembered that I think these
don't really provide much value -- sometimes they are just editorial,
and sometimes they are obsolete. I think it's better to just remove
them. Tested by rebuilding.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
I noticed that some spots in gdb call bfd_set_cacheable after opening
a BFD.
The BFD file cache is a bit odd. BFDs that are opened locally are
unconditionally registered with the cache, and their underlying file
descriptor will always be closed when bfd_cache_close_all is called.
However, only "cacheable" BFDs will be eligible for reopening when
needed -- and by default BFD decides that if a file descriptor is
passed in, then it should not be cacheable. If a non-cacheable BFD's
file descriptor is closed, there is no offical way to reopen it.
gdb needs to call bfd_cache_close_all, because some systems cannot
start an executable when it has an open file descriptor referencing
it.
However, gdb also will sometimes passes an open file descriptor to the
various BFD open functions. And, due to lazy DWARF reading, gdb may
also need to reopen these BFDs.
Rather than having all the callers figure out when exactly to set the
cacheable flag, I think it makes sense to consolidate this logic into
the gdb_bfd.c wrapper functions. It is ok to do this because gdb
always passes a filename to these open functions, so reopening should
work ok.
Regression tested on x86-64 Fedora 38.
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
|
|
Same idea as previous patches, but for solib_unloaded.
Change-Id: Iad847de93f0b38b5c90679a173d3beeaed7af6c5
|
|
Same idea as previous patches, but for solib_loaded
Change-Id: I85edb0a4b377f4b2c39ffccf31cb75f38bae0f55
|
|
The upcoming patch to support exec in the amd-dbgapi target needs to
detach amd-dbgapi from the inferior doing the exec and attach amd-dbgapi
to the inferior continuing the execution. They may or may not be the
same, depending on the `set follow-exec-mode` setting. But even if they
are the same, we need to do the detach / attach dance.
With the current observable signature, the observers only receive the
inferior in which execution continues (the "following" inferior).
Change the signature to pass both inferiors, and update all existing
observers.
Change-Id: I259d1ea09f70f43be739378d6023796f2fce2659
Reviewed-By: Pedro Alves <pedro@palves.net>
|
|
Per an earlier discussion, this patch renames the existing "raw" APIs
to use the word "unrelocated" instead.
|
|
This changes minimal symbols to use unrelocated_addr. I believe this
detected a latent bug in add_pe_forwarded_sym.
|
|
This changes gdb_bfd_lookup_symbol to use a function_view. This
simplifies the code a little bit.
|
|
Replace spaces with tabs in a bunch of places.
Change-Id: If0f87180f1d13028dc178e5a8af7882a067868b0
|
