| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
"cache" is just a bit too generic to be clear.
Change-Id: I8bf01c5fe84e076af1afd2453b1a115777630271
|
|
I ran codespell on gdb/*.[chyl] and fixed a bunch of simple typos.
Most of what remains is trickier, i.e., spots where a somewhat natural
name of something in the code is flagged as a typo.
Reviewed-By: Tom de Vries <tdevries@suse.de>
|
|
In commit af2b87e649b ("[gdb/testsuite] Add xfail for PR gcc/101633"), I added
an xfail that was controlled by variable old_gcc, triggering the xfail for
gcc 7 and before, but not for gcc 8 onwards:
...
set old_gcc [expr [test_compiler_info {gcc-[0-7]-*}]]
...
In commit 1411185a57e ("Introduce and use gnat_version_compare"), this changed
to:
...
set old_gcc [gnat_version_compare <= 7]
...
which still triggered the xfail for gcc 7, because of a bug in
gnat_version_compare.
After that bug got fixed, the xfail was no longer triggered because the gnatmake
version is 7.5.0, and [version_compare {7 5 0} <= {7}] == 0.
We could have the semantics for version_compare where we clip the input
arguments to the length of the shortest, and so we'd have
[version_compare {7 5 0} <= {7}] == [version_compare {7} <= {7}] == 1.
But let's stick with the current version-sort semantics, and fix this by
using [gnat_version_compare < 8] instead.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Add a test-case gdb.testsuite/version-compare.exp that excercises proc
version_compare, and a note to proc version_compare that it considers
v1 < v1.0 instead of v1 == v1.0.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Some architectures, such as MIPS, have signed addresses and this changes
read_addrmap_from_aranges to record them as signed when required.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32658
Approved-By: Tom Tromey <tom@tromey.com>
|
|
gccrs still can't process all of gdb's Rust tests, but I did manage to
manually test it on a few. In addition to filing some bug reports, I
came up with this patch.
There are two fixes here. First, gccrs emits tuple field names as
integers ("0", "1", etc) whereas rustc uses a leading double
underscore ("__0", "__1", etc). This patch changes gdb to accept the
gccrs output, which IMO makes sense (and for which there's already a
rustc feature request).
Second, it changes rust_struct_anon::evaluate to use check_typedef.
This is a gdb necessity in general, so could be described as an
oversight; but in this case it works around the gccrs oddity that most
named types are emitted as DW_TAG_typedef. I've filed a gccrs bug
report for that.
|
|
The cooked index worker maintains the state for the various state
transition in the scanner. It is held by the cooked_index while
scanning is in progress, then deleted once this has completed.
I noticed that none of the arguments to cooked_index::done_reading
were really needed -- the cooked_index already has access to the
worker should it need it. Removing these parameters makes the code a
bit simpler and also cleans up some confusing code around the use of
the deferred warnings object.
Regression tested on x86-64 Fedora 40.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
copyright.py needed an addition for unordered_dense.h.
Then, when running it, I saw it complain about some .pyc files I had
in the source tree. I don't know why I had these, but the script
should ignore them.
For this, Kévin suggested using "git ls-files" to determine which
files to update -- that should automatically exclude any random files
in the tree. This version of the patch makes this change.
There were complaints about some sim/ppc files that were renamed.
Ignoring the entire directory seems simpler given the comment.
I also made a few more minor changes:
* Removed the 'CVS' exclusion, as this hasn't been relevant in years.
* Moved the 'copying.c' exclusion to EXCLUDE_LIST
* Changed the script to run from the top level (we could have it
automatically find this if we really wanted).
After this lands, I plan to run it and check in the result. The patch
may be too large (and certainly too uninteresting) to post, so if/when
this happens I will send a brief note to the list about it.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Remove some includes reported as unused by clangd.
Change-Id: I841938c3c6254e4f0d154a1e172c4968ff326333
|
|
Remove includes reported as unused by clangd.
Change-Id: I12e5cf254d211f42f3cfdab90d1f42a5986e53a3
|
|
Compile a 32-bit x86 executable and then stop within a system call.
Change the sysroot to a non-existent directory, GDB should try (and
fail) to reload the currently loaded shared libraries. However, GDB
should retain the symbols for the vDSO library as that is not loaded
from the file system.
Check the backtrace to ensure that the __kernel_vsyscall symbol is
still in the backtrace, this indicates GDB still has the vDSO
symbols available.
This test was present in Fedora for a long time and was
originally written by Jan Kratochvil for this fix
829a902da291e72ad17e8c44fa8d9ead3db41b1f.
Co-Authored-By: Jan Kratochvil <jan.kratochvil@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
The relocate method of addrmap is unnecessarily virtual. Only
addrmap_fixed provides a meaningful implementation. Move the method to
addrmap_fixed only and make it non-virtual.
Change-Id: If61d5e70abc12c17d1e600adf0dd0707e77a6ba2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Add support for the gdb dir in the codespell section of gdb/contrib/setup.cfg,
specifically adding files in the skip line.
This allows us to run codespell from the command line on the gdb dir:
...
$ codespell --config gdb/contrib/setup.cfg gdb 2>/dev/null | wc -l
1665
...
without running into warnings in generated files.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This updates the cooked_index comment with some notes about object
lifetimes, in an attempt to make navigating this code a bit simpler.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The two readers currently using cooked_index_worker shared some code.
This patch factors this out into a new "done_reading" method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
cooked_index_worker::result_type is an ad hoc tuple type used for
transferring data between phases of the indexer. It's a bit unwieldy
and another patch I'm working on would be somewhat nicer without it.
This patch removes the type. Now cooked_index_ephemeral objects are
transferred instead, which is handy because they already hold the
needed state.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
It was convenient to add a 'clear' method to addrmap_mutable. The
cleanest way to do this was to change the class to lazily initialize
its 'tree' member. This also makes addrmap_mutable::operator= a bit
less weird.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This updates the "See xyz.h" comments for all the methods that were
moved earlier in this series. Perhaps I should have removed them
instead.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This moves the cooked_index_worker class to cooked-index-worker.[ch].
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes cooked-index-worker.h to include the new header files.
This breaks the circular dependency that would otherwise be introduced
in the next patch.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This moves cooked_index_shard to a couple of new files,
dwarf2/cooked-index-shard.[ch]. The rationale is the same as the
previous patch: cooked-index.h had to be split to enable other
cleanups.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This moves cooked_index_entry and some related helper code to a couple
of new files, dwarf2/cooked-index-entry.[ch].
The main rationale for this is that in order to finish this series and
remove "cooked_index_worker::result_type", I had to split
cooked-index.h into multiple parts to avoid circular includes.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
language_requires_canonicalization is only called from cooked-index.c,
so mark it as static.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This renames cooked_index_storage to cooked_index_worker_result,
making its function more clear. It also updates the class comment to
as well.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
A discussion with Simon made me realize that cooked_index_storage
isn't a very clear name, especially now that it's escaped from read.c.
While it does provide some storage (I guess any object does in a
sense), it is really a helper for cooked_index_worker -- a temporary
object that is destroyed after reading has completed.
This patch renames this file. Later patches will rename the class and
move cooked_index_worker here, something I think is reasonable given
that cooked_index_storage is really something of a helper class for
cooked_index_worker.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Tom de Vries pointed out that my earlier change to
gnat_version_compare made it actually test gcc's version -- not
gnat's.
This patch changes gnat_version_compare to examine gnatmake's version,
while preserving the nicer API.
Approved-By: Tom de Vries <tdevries@suse.de>
|
|
Commit
c221b2f77080 Testsuite: Add gdb_can_simple_compile
changed the source file name extension of the test program from .s to .c
resulting in compile fails. This, in turn, causes is_aarch32_target
checks to fail.
Change the test source from an assembly program to a C program using
inline assembly.
is_amd64_regs_target had a similar problem, which was fixed by commit
224d30d39365 testsuite: fix is_amd64_regs_target
This fix — and commit message — are mostly copied from it.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
In enum gdb_syscall, there are 3 entries that do not have the gdb_sys_ prefix
...
$ grep gdb_old_ gdb/linux-record.h
gdb_old_select = 82,
gdb_old_readdir = 89,
gdb_old_mmap = 90,
...
like all the other entries:
...
gdb_sys_restart_syscall = 0,
gdb_sys_exit = 1,
gdb_sys_fork = 2,
gdb_sys_read = 3,
...
The three correspond to these entries in
arch/x86/entry/syscalls/syscall_32.tbl:
...
<number> <abi> <name> <entry point> [<compat entry point> [noreturn]]
82 i386 select sys_old_select compat_sys_old_select
89 i386 readdir sys_old_readdir compat_sys_old_readdir
90 i386 mmap sys_old_mmap compat_sys_ia32_mmap
...
As we can see, the enum uses the entry point name, but without the sys_
prefix.
There doesn't seem to be a good reason for this.
There's another enum value:
...
gdb_sys_old_getrlimit = 76,
...
corresponding to:
...
76 i386 getrlimit sys_old_getrlimit compat_sys_old_getrlimit
...
where we do use the sys_ prefix.
Fix this by consistenly using the gdb_sys_ prefix in enum gdb_syscall.
No functional changes.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Now that we've started using codespell, remove gdb/contrib/spellcheck.sh and
associated file gdb/contrib/common-misspellings.txt.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
In noticed two occurrences of "if (strpbrk (...))".
Fix this style issue by checking against nullptr.
|
|
Add a pre-commit codespell hook for directories gdbsupport and gdbserver,
which are codespell-clean:
...
$ pre-commit run codespell --all-files
codespell................................................................Passed
...
A non-trivial question is where the codespell configuration goes.
Currently we have codespell sections in gdbsupport/setup.cfg and
gdbserver/setup.cfg, but codespell doesn't automatically use those because the
pre-commit hook runs codespell at the root of the repository.
A solution would be to replace those 2 setup.cfg files with a setup.cfg in the
root of the repository. Not ideal because generally we try to avoid adding
files related to subdirectories at the root.
Another solution would be to add two codespell hooks, one using
--config gdbsupport/setup.cfg and one using --config gdbserver/setup.cfg, and
add a third one once we start supporting gdb. Not ideal because it creates
duplication, but certainly possible.
I went with the following solution: a setup.cfg file in gdb/contrib (alongside
codespell-ignore-words.txt) which is used for both gdbserver and gdbsupport.
So, what can this new setup do for us? Let's demonstrate by simulating a typo:
...
$ echo "/* aways */" >> gdbsupport/agent.cc
...
We can check unstaged changes before committing:
...
$ pre-commit run codespell --all-files
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
...
Likewise, staged changes (no need for the --all-files):
...
$ git add gdbsupport/agent.cc
$ pre-commit run codespell
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
...
Or we can try to commit, and run into the codespell failure:
...
$ git commit -a
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
isort................................................(no files to check)Skipped
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
check-include-guards.................................(no files to check)Skipped
...
which makes the commit fail.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
There are a few spots where an mmap system call is mapped onto enum
gdb_syscall value gdb_sys_mmap2.
Strictly speaking, this is incorrect.
Fix this by mapping to enum gdb_syscall value gdb_old_mmap instead.
No functional changes: both gdb_old_mmap and gdb_sys_mmap2 are handled the
same in record_linux_system_call.
Tested by rebuilding on x86_64-linux.
|
|
With the selftest register_name, we run into a few warning:
...
$ gdb -q -batch -ex "maint selftest register_name" 2>&1 \
| grep -B1 warning:
Running selftest register_name::m68hc11.
warning: No frame soft register found in the symbol table.
--
Running selftest register_name::m68hc12.
warning: No frame soft register found in the symbol table.
--
Running selftest register_name::m68hc12:HCS12.
warning: No frame soft register found in the symbol table.
...
We already filter out these architectures in other selftests because of the
same warning.
Do the same in this selftest.
Tested on x86_64-linux.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
I think there is a problem with the disable_breakpoints_in_shlibs
function: it can disable breakpoint locations without calling
notify_breakpoint_modified. This means that the Python API's
breakpoint_modified event will not trigger, nor will the MI send a
breakpoint modified event.
I started looking at disable_breakpoints_in_shlibs because of an
earlier commit:
commit 8c48ec7a6160aed0d1126c623443935e4435cd41
Date: Thu Aug 29 12:34:15 2024 +0100
gdb: handle dprintf breakpoints when unloading a shared library
Currently disable_breakpoints_in_shlibs is only called from one
location, clear_solib in solib.c. clear_solib also calls
notify_solib_unloaded for every solib in the program_space of
interest, and notify_solib_unloaded will call
disable_breakpoints_in_unloaded_shlib via the solib_unloaded
observer. These two function, disable_breakpoints_in_shlibs and
disable_breakpoints_in_unloaded_shlib are very similar in what they
do.
I think that we can remove the disable_breakpoints_in_shlibs call, and
instead, tweak how we call disable_breakpoints_in_unloaded_shlib in
order to get the same end result, except that, after this change, we
will call notify_breakpoint_modified, which means the Python API event
will trigger, and the MI events will be emitted.
All that disable_breakpoints_in_shlibs does is disable some
breakpoints.
Meanwhile, disable_breakpoints_in_unloaded_shlib, will disable the
same set of breakpoints, call notify_breakpoint_modified, and
then (for some breakpoint types) print a message telling the user that
the breakpoint has been disabled. However, this function will ignore
any breakpoints that are already disabled.
As disable_breakpoints_in_shlibs disables the same set of breakpoints,
the result of the current code is that disable_breakpoints_in_shlibs
serves only to prevent the notify_breakpoint_modified call, which I
think is wrong, and to prevent the user message being printed, which I
think is reasonable.
If we remove the disable_breakpoints_in_shlibs call without making any
additional changes, then we start to see some message printed in cases
like this:
(gdb) start
The program being debugged has been started already.
Start it from the beginning? (y or n) y
warning: Temporarily disabling breakpoints for unloaded shared library "/tmp/shared-lib-test/libfoo.so"
Temporary breakpoint 3 at 0x40113e: file test.c, line 9.
Starting program: /tmp/shared-lib-test/test.x
Notice the 'warning:' line, which is new. I think this is confusing
because, in most cases the breakpoint will be enabled again by the
time the inferior reaches `main` and stops.
In the future I'm interested in exploring if GDB could be smarter
about when to print these 'Temporarily disabling breakpoints ...'
messages so that if the 'start' command does mean a breakpoint is left
disabled, then the user would be informed. However, I don't propose
doing that work immediately, and certainly not in this commit. For
now, my intention is to leave things as they are right now, GDB
doesn't warn about disabling breakpoints during an inferior re-start.
To achieve this I think we need to pass a new argument to
disable_breakpoints_in_unloaded_shlib which controls whether we should
print a message about the breakpoint being disabled or not. With this
added we can now silence the warning when the inferior is
restarted (i.e. when disable_breakpoints_in_unloaded_shlib is called
from clear_solib), but keep the warning for cases like stepping over a
dlclose() call in the inferior.
After this commit, GDB now emits breakpoint modified events (in Python
and/or MI) when a breakpoint is disabled as a result of all shared
libraries being unloaded. This will be visible in two places that I
can thing of, the 'nosharedlibrary' command, and when an inferior is
restarted.
|
|
Consider this backtrace within GDB:
#0 notify_breakpoint_modified (b=0x57d31d0) at ../../src/gdb/breakpoint.c:1083
#1 0x00000000005b6406 in breakpoint_set_commands (b=0x57d31d0, commands=...) at ../../src/gdb/breakpoint.c:1523
#2 0x00000000005c8c63 in update_dprintf_command_list (b=0x57d31d0) at ../../src/gdb/breakpoint.c:8641
#3 0x00000000005d3c4e in dprintf_breakpoint::re_set (this=0x57d31d0) at ../../src/gdb/breakpoint.c:12476
#4 0x00000000005d6347 in breakpoint_re_set () at ../../src/gdb/breakpoint.c:13298
Whenever breakpoint_re_set is called we re-build the commands that the
dprintf b/p will execute and store these into the breakpoint. The
commands are re-built in update_dprintf_command_list and stored into
the breakpoint object in breakpoint_set_commands.
Now sometimes these commands can change, dprintf_breakpoint::re_set
explains one case where this can occur, and I'm sure there must be
others. But in most cases the commands we recalculate will not
change. This means that the breakpoint modified event which is
emitted from breakpoint_set_commands is redundant.
This commit aims to eliminate the redundant breakpoint modified events
for dprintf breakpoints. This is done by adding a commands_equal call
to the start of breakpoint_set_commands.
The commands_equal function is a new function which compares two
command_line objects and returns true if they are identical. Using
this function we can check if the new commands passed to
breakpoint_set_commands are identical to the breakpoint's existing
commands. If the new commands are equal then we don't need to change
anything on the new breakpoint, and the breakpoint modified event can
be skipped.
The test for this commit stops at a dlopen() call in the inferior,
sets up a dprintf breakpoint, then uses 'next' to step over the
dlopen() call. When the library loads GDB call breakpoint_re_set,
which calls dprintf_breakpoint::re_set. But in this case we don't
expect the calculated command string to change, so we don't expect to
see the breakpoint modified event.
|
|
With commit fb2ded33c1e519659743047ed7817166545b6d91, I added
Fedora's gstack script to gdb. Some issues have arisen since
then, and this patch addresses those issues:
. As Sam James recently noted[1], PKGVERSION and VERSION
need to be quoted.
. A Fedora user reported the misuse of --readnever, which
causes gstack to omit filename and line number information in the
backtrace[Red Hat BZ 2354997].
[1] https://inbox.sourceware.org/gdb-patches/d19d6bc17e0a160ce27fc572079f11a587c0e168.1742424869.git.sam@gentoo.org/
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=2354997
|
|
Modifying inline-frame-cycle-unwind.exp to use `bt -no-filters` produces
the following incorrect backtrace:
#0 inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:49
#1 normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
#2 0x000055555555517f in inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:50
#3 normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 1: backtrace when the unwind is broken at frame 1
The expected output, which we get with `bt`, is:
#0 inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:49
#1 normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) PASS: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 1: backtrace when the unwind is broken at frame 1
The cycle checking in `get_prev_frame_maybe_check_cycle` relies on newer
frame ids having already been computed and stashed. Unlike other
frames, frame #0's id does not get computed immediately.
The test passes with `bt` because when applying python frame filters,
the call to `bootstrap_python_frame_filters` happens to compute the id
of frame #0. When `get_prev_frame_maybe_check_cycle` later tries to
stash frame #2's id, the cycle is detected.
The test fails with `bt -no-filters` because frame #0's id has not been
stashed by the time `get_prev_frame_maybe_check_cycle` tries to stash
frame #2's id which succeeds and the cycle is only detected later when
trying to stash frame #4's id. Doing `stepi` after the incorrect
backtrace would then trigger an assertion failure when trying to stash
frame #0's id because it is a duplicate of #2's already stashed id.
In `get_prev_frame_always_1`, if this_frame is inline frame 0, then
compute and stash its frame id before returning the previous frame.
This ensures that the id of inline frame 0 has been stashed before
`get_prev_frame_maybe_check_cycle` is called on older frames.
The test case has been updated to run both `bt` and `bt -no-filters`.
Co-authored-by: Andrew Burgess <aburgess@redhat.com>
|
|
Tom Tromey mentioned [1] that the words "invokable" and "useable"
present in codespell-ignore-words.txt should be dropped.
Do so and fix the following typos:
...
$ codespell --config gdbsupport/setup.cfg gdbsupport
gdbsupport/common-debug.h:218: invokable ==> invocable
gdbsupport/event-loop.cc:84: useable ==> usable
...
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://sourceware.org/pipermail/gdb-patches/2025-March/216584.html
|
|
Ignore the following codespell detection:
...
$ codespell --config gdbserver/setup.cfg gdbserver
gdbserver/linux-aarch64-low.cc:827: SME ==> SAME, SEME, SOME, SMS
...
by adding SME to codespell-ignore-words.txt.
|
|
Use GDB/MI command "-complete" to implement.
Co-authored-by: Simon Farre <simon.farre.cx@gmail.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31140
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
|
|
In OBS (Open Build Service), with a 15.2 based gdb package, occasionally I run
into:
...
(gdb) inferior 2
[Switching to inferior 2 [process 31372] (access-mem-running-thread-exit)]
[Switching to thread 2.1 (Thread 0xf7db9700 (LWP 31372))](running)
(gdb) print global_var = 555
$1 = 555
(gdb) print global_var
$2 = 556
(gdb) FAIL: $exp: all-stop: access mem \
(print global_var after writing, inf=2, iter=1)
...
I managed to reproduce this on current trunk using a reproducer patch (posted
in the PR).
The problem is due to commit 31c21e2c13d ("[gdb/testsuite] Fix
gdb.threads/access-mem-running-thread-exit.exp with clang"), which introduced
an increment of global_var at the start of main.
This created a race between:
- gdb modifying global_var, and
- the inferior modifying global_var.
Fix this by:
- adding a new empty function setup_done,
- adding a call to setup_done after the increment of global_var, and
- rather than running to main, running to setup_done.
Tested on x86_64-linux.
PR testsuite/32822
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32822
|
|
In the AIX systems available for testing in the gcc compile farm, the
default debug information format is stabs. This is a problem for many
reasons, mainly that stabs is not as complete as dwarf and stabs is
being deprecated in the next release. In the current state, we have:
PASS: 39798
FAIL: 7405
When running these tests, I unfortunately didn't have the foresight to
save the number of unsupported and untested cases.
To improve testing there, this patch changes the gdb_compile TCL proc, so
that if we're running tests in AIX, we requested debug info, and we
haven't explicitly asked for some debuginfo format, gdb_compile will add
-gdwarf to the compilation line, forcing DWARF to be used. After this
patch, we get:
PASS: 74548
FAIL: 5963
So not only do we have fewer failures, there are tens of thousands of
tests that are no longer skipped.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
GDB's compile subsystem is deeply tied to GDB's ability to understand
DWARF. A future patch will add the option to disable DWARF at configure
time, but for that to work, the compile subsystem will need to be
entirely disabled as well, so this patch adds that possibility.
I also think there is motive for a security conscious user to disable
compile for it's own sake. Considering that the code is quite
unmaintained, and depends on an equally unmaintained gcc plugin, there
is a case to be made that this is an unnecessary increase in the attack
surface if a user knows they won't use the subsystem. Additionally, this
can make compilation slightly faster and the final binary is around 3Mb
smaller. But these are all secondary to the main goal of being able to
disable dwarf at configure time.
To be able to achieve optional compilation, some of the code that
interfaces with compile had to be changed. All parts that directly
called compile things have been wrapped by ifdefs checking for compile
support. The file compile/compile.c has been setup in a similar way to
how python's and guile's main file has been setup, still being compiled
but only for with placeholder command.
Finally, to avoid several new errors, a new TCL proc was introduced to
gdb.exp, allow_compile_tests, which checks if the "compile" command is
recognized before the inferior is started and otherwise skips the compile
tests. All tests in the gdb.compile subfolder have been updated to use
that, and the test gdb.base/filename-completion also uses this. The proc
skip_compile_feature_tests to recognize when the subsystem has been
disabled at compile time.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The following patch will add a configure option to disable the compile
subsystem at compilation time. To do that, nearly all code that
interfaces with compile should be confined to the compile sub-folder.
This commit is the first step, removing the compile-related method from
the language struct and adding 2 new functions to compile.c that do the
same job in a slightly different way. Adding things to the language
struct is a more extendable way to add support for languages, but
considering compile is quite bit-rotted and questionably supported, I
don't think it will be extended any time soon, and using ifdefs to
handle disabling compile with configure felt like a messier solution.
There should be no visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Change some parameters to be references instead of pointers, when the
value must not be nullptr. I'd like to do this more of this kind of
change, but I have to limit the scope of the change, otherwise there's
just no end (and some local variables could also be turned into
references). So for now, just do it the cutu_reader constructors.
Change-Id: I9442c6043726981d58f9b141f516c590c0a71bcc
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The comment on this constructor is really outdated. Update it to better
reflect the reality today.
I'd eventually like to change this cutu_reader constructor not to use
dwarf2_per_cu, because it seems like an abuse of dwarf2_per_cu just to
pass 3 values. But for now, just document the existing behavior.
Change-Id: Id96db020c361e64d9b0d2f25d51950b206658aa2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
lookup_dwo_unit receives the name of the DWO unit to look up, as read
from the DW_AT_dwo_name attribute of the skeleton DIE. But then, it
doesn't use it:
/* Yeah, we look dwo_name up again, but it simplifies the code. */
dwo_name = dwarf2_dwo_name (comp_unit_die, cu);
Perhaps this comment made sense at some point, but with the code we have
today, I don't understand it. It should be fine to use the name passed
as a parameter, which the caller also obtained by calling
dwarf2_dwo_name.
Change-Id: I84723e12726f77e4202d042428ee0eed9962ceb8
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Looking at `cooked_index_shard::find`, I thought that we could make a
small optimization: when finding the upper bound, we already know the
lower bound. And we know that the upper bound is >= the lower bound.
So we could pass `lower` as the first argument of the `std::upper_bound`
call to cut the part of the search space that is below `lower`.
It then occured to me that what we do is basically what
`std::equal_range` is for, so why not use it. Implementations of
`std::equal_range` are likely do to things as efficiently as possible.
Unfortunately, because `cooked_index_entry::compare` is sensitive to the
order of its parameters, we need to provide two different comparison
functions (just like we do know, to the lower_bound and upper_bound
calls). But I think that the use of equal_range makes it clear what the
intent of the code is.
Regression tested using the various DWARF target boards on Debian 12.
Change-Id: Idfad812fb9abae1b942d81ad9976aeed7c2cf762
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I believe that the `(mode == MATCH && a == munge ('<'))` part of the
condition is unnecesary. Or perhaps I don't understand the algorithm.
The use of "munge" above effectively makes it so that the template
portion of names is completely ignored for the sake of the comparison.
Then, in the condition, this:
a == munge ('<')
is functionally equivalent to
a == '\0'
If `a` is indeed '\0', and `b` is also '\0', then we would have taken
the earlier branch:
if (a == b)
return 0;
If `b` is not '\0', then we won't take this branch and we'll go into the
final comparison:
return a < b ? -1 : 1;
So, as far as I can see, there is no case where `mode == MATCH`, where
we're going to use this special `return 0`.
Regression tested using the various DWARF target boards on Debian 12.
Change-Id: I5ea0463c1fdbbc1b003de2f0a423fd0073cc9dec
Approved-By: Tom Tromey <tom@tromey.com>
|
|
We have this pattern of check in multiple places:
/* Skip dummy compilation units. */
if (m_info_ptr >= begin_info_ptr + this_cu->length ()
|| peek_abbrev_code (abfd, m_info_ptr) == 0)
m_dummy_p = true;
In all places except one (read_cutu_die_from_dwo), this is done after
reading the unit header but before potentially reading the first DIE.
The effect is that we consider dummy units that have no DIE at all.
Either the "data" portion of the unit (the portion after the header) has
a size of zero, or the first abbrev code is 0, i.e. "end of list".
According to this old commit I found [1], dummy CUs were used as filler
for incremental LTO linking. A comment reads:
WARNING: If THIS_CU is a "dummy CU" (used as filler by the incremental
linker) then DIE_READER_FUNC will not get called.
In read_cutu_die_from_dwo, however, this check is done after having read
the first DIE. So at the time of the check, m_info_ptr has already been
advanced just past the first DIE. As a result, compilations units with
a single DIE are considered (erroneously, IMO) as dummy.
In commit aab6de1613df ("gdb/dwarf: fix spurious error when encountering
dummy CU") [2], I mentioned a real world case where compilation units
with a single top-level DIE were being considered dummy. I believe that
those units should not actually have been treated as dummy. A CU with
just one DIE may not be very interesting, but I don't see any reason to
consider it dummy.
Move the dummy check above the read_toplevel_die call, and return early
if the CU is dummy.
I am 99% convinced that it's not even possible to encounter an empty
unit here, and considered turning it into an assert (it did pass the
testsuite). This function is passed a dwo_unit, and functions that
create a dwo_unit are:
- create_debug_type_hash_table (creates a dwo_unit for each type unit
found in a dwo file)
- create_cus_hash_table (creates a dwo_unit for each comp unit found in
a dwo file)
- create_dwo_unit_in_dwp_v1
- create_dwo_unit_in_dwp_v2
- create_dwo_unit_in_dwp_v5
In the first two, there are already dummy checks, so we wouldn't even
get to read_cutu_die_from_dwo for such an empty CU. However, in the
last three, there is no such checks, we just trust the dwp file's index
and create dwo_units out of that. So I guess it would be possible to
craft a broken dwp file with a CU that has no DIE. Out of caution, I
didn't switch that to an assert, but I also don't really know what would
be the mode of failure if that were to happen.
Regtested using the various DWARF target boards on Debian 12.
[1] https://gitlab.com/gnutools/binutils-gdb/-/commit/dee91e82ae87f379c90fddff8db7c4b54a116609#dd409f60ba6f9c066432dafbda7093ac5eec76d1_3434_3419
[2] https://gitlab.com/gnutools/binutils-gdb/-/commit/aab6de1613df693059a6a2b505cc8f20d479d109
Change-Id: I90e6fa205cb2d23ebebeae6ae7806461596f9ace
Approved-By: Tom Tromey <tom@tromey.com>
|
