Another patch I am working on induced some failures in CTF tests.
Looking into this, I found that ctfread.c seems to largely work by
accident. In particular, it often chooses the wrong domain for a
symbol.
In CTF, I believe there are 4 kinds of symbols: types, variables,
functions, and "data objects" (which IIUC may be either a variable or
a function).
ctfread.c was examining the type-kind of a variable and sometimes
treating one as a type. add_stt_entries and
ctf_psymtab_add_stt_entries only ever used VAR_DOMAIN (but are called
for functions, which should be in FUNCTION_DOMAIN). And
ctf_psymtab_type_cb sometimes used VAR_DOMAIN, but is only called for
types, and so should only ever use TYPE_DOMAIN or STRUCT_DOMAIN.
This patch cleans all this up, based on my understanding of the
situation. This passes the existing tests, and also works with my
aforementioned yet-to-be-submitted patch as well.
Finally, I renamed new_symbol because it is only used for type
symbols.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
I noticed that ctfread.c could create a symbol with the name "". This
happens because a couple of spots check that a name is not NULL -- but
libctf never returns such names. Instead check the string contents.
I left the NULL checks in for robustness.
Note that other spots in ctfread.c already do check the contents of
the name. I changed these to avoid strlen and instead check the first
character.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
ctfread.c creates two per-objfile registry keys. However, one is
sufficient. This patch combines the two.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
ctfread.c uses raw "new" and "delete", but for an object that could
just as easily be stack allocated.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
The new approach to searching (solely via the quick API) is more
sensitive to discrepancies between the partial and full readers. In
CTF, there is some disagreement about which scope to use. CTF doesn't
seem to really distinguish between the file and global scope, so this
patch takes the simple approach of putting all CTF symbols into the
global scope.
This changes one test as well. It seems to me that the behavior here
is arbitrary and the test is making unwarranted assumptions.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
The enum address_class and related fields and methods seem misnamed to
me. Generalize it to "location_class". The enumerators in
address_class are already prefixed with LOC, so the new name seems
logical to me. Rename related fields and methods as well.
Plus, address_class could easily be mistaken for other unrelated things
named "address class" in GDB or DWARF.
Tested by rebuilding.
Change-Id: I0dca3738df412b350715286c608041b08e9b4d82
Approved-by: Kevin Buettner <kevinb@redhat.com>
This updates the copyright headers to include 2025. I did this by
running gdb/copyright.py and then manually modifying a few files as
noted by the script.
Approved-By: Eli Zaretskii <eliz@gnu.org>
Commit f89276a2f3 ("change type of `general_symbol_info::m_section`
to int") did what it says in the title -- changed the type of the
section index from short to int. However, it seems incomplete, in
that there are uses of the section index that use the type 'short'.
This patch fixes the ones I found, first by searching for
"short.*sect" and then by looking at all the callers of section_index
(and then functions called with the resulting value) just to try to be
more sure.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
>From what I can see, lookup_minimal_symbol doesn't have any dependencies
on the global current state other than the single reference to
current_program_space. Add a program_space parameter and make that
current_program_space reference bubble up one level.
Change-Id: I759415e2f9c74c9627a2fe05bd44eb4147eee6fe
Reviewed-by: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
Most calls to lookup_minimal_symbol don't pass a value for sfile and
objf. Make these parameters optional (have a default value of
nullptr). And since passing a value to `objf` is much more common than
passing a value to `sfile`, swap the order so `objf` comes first, to
avoid having to pass a nullptr value to `sfile` when wanting to pass a
value to `objf`.
Change-Id: I8e9cc6b942e593bec640f9dfd30f62786b0f5a27
Reviewed-by: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
This is a simple find / replace from "struct bound_minimal_symbol" to
"bound_minimal_symbol", to make things shorter and more consisten
througout. In some cases, move variable declarations where first used.
Change-Id: Ica4af11c4ac528aa842bfa49a7afe8fe77a66849
Reviewed-by: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
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>
This patch changes the DWARF reader to use the new symbol domains. It
also adjusts many bits of associated code to adapt to this change.
The non-DWARF readers are updated on a best-effort basis. This is
somewhat simpler since most of them only support C and C++. I have no
way to test a few of these.
I went back and forth a few times on how to handle the "tag"
situation. The basic problem is that C has a special namespace for
tags, which is separate from the type namespace. Other languages
don't do this. So, the question is, should a DW_TAG_structure_type
end up in the tag domain, or the type domain, or should it be
language-dependent?
I settled on making it language-dependent using a thought experiment.
Suppose there was a Rust compiler that only emitted nameless
DW_TAG_structure_type objects, and specified all structure type names
using DW_TAG_typedef. This DWARF would be correct, in that it
faithfully represents the source language -- but would not work with a
purely struct-domain implementation in gdb. Therefore gdb would be
wrong.
Now, this approach is a little tricky for C++, which uses tags but
also enters a typedef for them. I notice that some other readers --
like stabsread -- actually emit a typedef symbol as well. And, I
think this is a reasonable approach. It uses more memory, but it
makes the internals simpler. However, DWARF never did this for
whatever reason, and so in the interest of keeping the series slightly
shorter, I've left some C++-specific hacks in place here.
Note that this patch includes language_minimal as a language that uses
tags. I did this to avoid regressing gdb.dwarf2/debug-names-tu.exp,
which doesn't specify the language for a type unit. Arguably this
test case is wrong.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30164
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.
This changes gdb to use the C++17 [[fallthrough]] attribute rather
than special comments.
This was mostly done by script, but I neglected a few spellings and so
also fixed it up by hand.
I suspect this fixes the bug mentioned below, by switching to a
standard approach that, presumably, clang supports.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23159
Approved-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
This changes main_type to hold a language, and updates the debug
readers to set this field. This is done by adding the language to the
type-allocator object.
Note that the non-DWARF readers are changed on a "best effort" basis.
This patch also reimplements type::is_array_like to use the type's
language, and it adds a new type::is_string_like as well. This in
turn lets us change the Python implementation of these methods to
simply defer to the type.
Add these two methods, rename the field to m_bitsize to make it pseudo
private.
Change-Id: Ief95e5cf106e72f2c22ae47b033d0fa47202b413
Approved-By: Tom Tromey <tom@tromey.com>
After finding this code in buildsym_compunit::finish_block_internal:
...
ftype->set_fields
((struct field *)
TYPE_ALLOC (ftype, nparams * sizeof (struct field)));
...
and fixing PR30810 by using TYPE_ZALLOC, I wondered if there were more
locations that needed fixing.
I decided to make things easier to spot by factoring out a new function
alloc_fields:
...
/* Allocate the fields array of this type, with NFIELDS elements. If INIT,
zero-initialize the allocated memory. */
void
type::alloc_fields (unsigned int nfields, bool init = true);
...
where:
- a regular use would be "alloc_fields (nfields)", and
- an exceptional use that needed no initialization would be
"alloc_fields (nfields, false)".
Pretty soon I discovered that most of the latter cases are due to
initialization by memcpy, so I added two variants of copy_fields as well.
After this rewrite there are 8 uses of set_fields left:
...
gdb/coffread.c: type->set_fields (nullptr);
gdb/coffread.c: type->set_fields (nullptr);
gdb/coffread.c: type->set_fields (nullptr);
gdb/eval.c: type->set_fields
gdb/gdbtypes.c: type->set_fields (args);
gdb/gdbtypes.c: t->set_fields (XRESIZEVEC (struct field, t->fields (),
gdb/dwarf2/read.c: type->set_fields (new_fields);
gdb/dwarf2/read.c: sub_type->set_fields (sub_type->fields () + 1);
...
These fall into the following categories:
- set to nullptr (coffread.c),
- type not owned by objfile or gdbarch (eval.c), and
- modifying an existing fields array, like adding an element at the end or
dropping an element at the start (the rest).
Tested on x86_64-linux.
psympriv.h was intended for use by code that created partial symbols.
Now that no generic code needs psymtab.h any more, psympriv.h can be
merged into psymtab.h.
This changes partial symbol tables to use unrelocated_addr for the
text_high and text_low members. This revealed some latent bugs in
ctfread.c, which are fixed here.
This renames objfile_type to be an overload of builtin_type, in
preparation for their unification.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
This changes the array type creation functions to accept a type
allocator, and updates all the callers. Note that symbol readers
should generally allocate on the relevant objfile, regardless of the
placement of the index type of the array, which is what this patch
implements.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
This changes the range type creation functions to accept a type
allocator, and updates all the callers. Note that symbol readers
should generally allocate on the relevant objfile, regardless of the
underlying type of the range, which is what this patch implements.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
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.
Add the `length` and `set_length` methods on `struct type`, in order to remove
the `TYPE_LENGTH` macro. In this patch, the macro is changed to use the
getter, so all the call sites of the macro that are used as a setter are
changed to use the setter method directly. The next patch will remove the
macro completely.
Change-Id: Id1090244f15c9856969b9be5006aefe8d8897ca4
Add the `target_type` and `set_target_type` methods on `struct type`, in order
to remove the `TYPE_TARGET_TYPE` macro. In this patch, the macro is changed to
use the getter, so all the call sites of the macro that are used as a setter
are changed to use the setter method directly. The next patch will remove the
macro completely.
Change-Id: I85ce24d847763badd34fdee3e14b8c8c14cb3161
This changes struct objfile to use a gdb_bfd_ref_ptr. In addition to
removing some manual memory management, this fixes a use-after-free
that was introduced by the registry rewrite series. The issue there
was that, in some cases, registry shutdown could refer to memory that
had already been freed. This help fix the bug by delaying the
destruction of the BFD reference (and thus the per-bfd object) until
after the registry has been shut down.
This rewrites registry.h, removing all the macros and replacing it
with relatively ordinary template classes. The result is less code
than the previous setup. It replaces large macros with a relatively
straightforward C++ class, and now manages its own cleanup.
The existing type-safe "key" class is replaced with the equivalent
template class. This approach ended up requiring relatively few
changes to the users of the registry code in gdb -- code using the key
system just required a small change to the key's declaration.
All existing users of the old C-like API are now converted to use the
type-safe API. This mostly involved changing explicit deletion
functions to be an operator() in a deleter class.
The old "save/free" two-phase process is removed, and replaced with a
single "free" phase. No existing code used both phases.
The old "free" callbacks took a parameter for the enclosing container
object. However, this wasn't truly needed and is removed here as
well.
It's a bit confusing because we have both "compunit_symtab" and "symtab"
types, and many methods and functions containing "start_symtab" or
"end_symtab", which actually deal with compunit_symtabs. I believe this
comes from the time before compunit_symtab was introduced, where
symtab did the job of both.
Rename everything I found containing start_symtab or end_symtab to use
start_compunit_symtab or end_compunit_symtab.
Change-Id: If3849b156f6433640173085ad479b6a0b085ade2
I noticed that the CTF symbol reader passes the objfile's name to all
buildsym_compunit instances it creates. The result is that all
compunit_symtabs created have the same name, that of the objfile:
{ objfile /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct objfile *) 0x613000005d00)
{ ((struct compunit_symtab *) 0x621000286760)
debugformat ctf
producer (null)
name libbabeltrace2.so.0.0.0
dirname (null)
blockvector ((struct blockvector *) 0x6210003911d0)
user ((struct compunit_symtab *) (null))
{ symtab /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct symtab *) 0x6210003911f0)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
{ ((struct compunit_symtab *) 0x621000275c10)
debugformat ctf
producer (null)
name libbabeltrace2.so.0.0.0
dirname (null)
blockvector ((struct blockvector *) 0x621000286710)
user ((struct compunit_symtab *) (null))
{ symtab /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct symtab *) 0x621000286730)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
Notice the two "name libbabeltrace2.so.0.0.0".
Change it to pass the partial_symtab's filename instead. The output
becomes:
{ objfile /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct objfile *) 0x613000005d00)
{ ((struct compunit_symtab *) 0x621000295610)
debugformat ctf
producer (null)
name libbabeltrace2.so.0.0.0
dirname (null)
blockvector ((struct blockvector *) 0x6210003a15d0)
user ((struct compunit_symtab *) (null))
{ symtab /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0 ((struct symtab *) 0x6210003a15f0)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
{ ((struct compunit_symtab *) 0x621000288700)
debugformat ctf
producer (null)
name current-thread.c
dirname (null)
blockvector ((struct blockvector *) 0x6210002955c0)
user ((struct compunit_symtab *) (null))
{ symtab /home/simark/src/babeltrace/src/lib/current-thread.c ((struct symtab *) 0x6210002955e0)
fullname (null)
linetable ((struct linetable *) 0x0)
}
}
Note that the first compunit_symtab still has libbabeltrace2.so.0.0.0 as
its name. This is because the CTF symbol reader really creates a
partial symtab named like this. It appears to be because the debug info
contains information that has been factored out of all CUs and is at the
"top-level" of the objfile, outside any real CU. So it creates a
partial symtab and an artificial CU that's named after the objfile.
Change-Id: I576316bab2a3668adf87b4e6cebda900a8159b1b
I am trying to do some changes to buildsym_compunit, so I am auditing
the current uses. Something seems odd with this use of
buildsym_compunit (that this patch removes).
A buildsym_compunit is normally used when building a compunit_symtab.
That is, when expanding a partial symtab into a full compunit symtab.
In ctfread.c, a buildsym_compunit is created in ctf_start_archive, which
is only used when creating partial symtabs. At this moment, I don't
see how that's useful. ctf_start_archive creates a new
buildsym_compunit and starts a subfile. But that buildsym_compunit is
never used again. It's just overriden in ctf_start_symtab, which means
we leak the old buildsym_compunit, I suppose.
Remove ctf_start_archive completely. Add an assert in
ctf_start_symtab to verify that we are not overwriting an existing
buildsym_compunit (meaning we'd leak the existing one). This assert
triggers without the other part of the fix. When doing:
$ ./gdb --data-directory=data-directory /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
...
(gdb) maintenance expand-symtabs
/home/simark/src/binutils-gdb/gdb/ctfread.c:1255: internal-error: ctf_start_symtab: Assertion `!ccp->builder' failed.
Change-Id: I666d146454a019f08e7305f3a1c4a974d27b4592
I built a random project with -gctf, in order to test the CTF support in
GDB. With my ASan/UBSan/etc-enabled build of GDB, I get:
$ ./gdb --data-directory=data-directory /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
...
Reading symbols from /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0...
/home/simark/src/binutils-gdb/gdb/ctfread.c:1545:31: runtime error: member call on misaligned address 0xbebebebebebebebe for type 'struct buildsym_compunit', which requires 8 byte alignment
0xbebebebebebebebe: note: pointer points here
The 0xbebebebebebebebe value is a sign that the ctf_context::builder
field is uninitialized. The problem probably goes under the radar if
the field happens to be zero-initialized, because ctf_start_archive
contains this code:
if (ccx->builder == nullptr)
{
ccx->builder = new buildsym_compunit (of,
of->original_name, nullptr, language_c, 0);
If the field was zero-initialized (by chance), this will create a new
buildsym_compunit. But if the field was purposely filled with random
bytes by one of the sanitizers, we won't create a buildsym_compunit here
and we'll continue with ccx->builder equal to 0xbebebebebebebebe.
Fix this the easy way by initializing ccx->builder where the other
ctf_context fields are initialized (yeah, this code could be made nicer
C++, but I am going for the obvious fix here).
With this patch, this passes cleanly on my system:
$ make check TESTS="gdb.ctf/*.exp" RUNTESTFLAGS="CC_FOR_TARGET=/opt/gcc/git/bin/gcc"
# of expected passes 40
... where /opt/gcc/git/bin/gcc is a gcc with CTF support, given my
system gcc does not have it.
Change-Id: Idea1b0cf3e3708b72ecb16b1b60222439160f9b9
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
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 a symbol's domain. Remove the
corresponding macro and adjust all callers.
Change-Id: I54465b50ac89739c663859a726aef8cdc6e4b8f3
