Small cleanup to use std::vector iterators rather than raw pointers.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I8d50dbb3f2d8dad7ff94066a578d523f1f31b590
When building GDB with clang and --enable-ubsan, I get:
UNRESOLVED: gdb.dwarf2/frame-inlined-in-outer-frame.exp: starti prompt
The cause being:
$ ./gdb --data-directory=data-directory -nx -q -readnow testsuite/outputs/gdb.dwarf2/frame-inlined-in-outer-frame/frame-inlined-in-outer-frame
Reading symbols from testsuite/outputs/gdb.dwarf2/frame-inlined-in-outer-frame/frame-inlined-in-outer-frame...
Expanding full symbols from testsuite/outputs/gdb.dwarf2/frame-inlined-in-outer-frame/frame-inlined-in-outer-frame...
/home/simark/src/binutils-gdb/gdb/dwarf2/read.c:11954:47: runtime error: applying non-zero offset 8 to null pointer
I found this to happen with ld-linux on at least Arch Linux and Ubuntu
22.04:
$ ./gdb --data-directory=data-directory -nx -q -readnow -iex "set debuginfod enabled on" /lib64/ld-linux-x86-64.so.2
Reading symbols from /lib64/ld-linux-x86-64.so.2...
Reading symbols from /home/simark/.cache/debuginfod_client/22bd7a2c03d8cfc05ef7092bfae5932223189bc1/debuginfo...
Expanding full symbols from /home/simark/.cache/debuginfod_client/22bd7a2c03d8cfc05ef7092bfae5932223189bc1/debuginfo...
/home/simark/src/binutils-gdb/gdb/dwarf2/read.c:11954:47: runtime error: applying non-zero offset 8 to null pointer
The problem happens when doing this:
sect_offset *offsetp = offsets.data () + 1
When `offsets` is an empty vector, `offsets.data ()` returns nullptr.
Fix it by wrapping that in a `!offsets.empty ()` check.
Change-Id: I6d29ba2fe80ba4308f68effd9c57d4ee8d67c29f
Approved-By: Tom Tromey <tom@tromey.com>
PR symtab/29694 points out a regression caused by the new DWARF
scanner when the cc-with-gdb-index target board is used.
What happens here is that an older version of gdb will make an index
describing the "A" type as:
[737] A: 1 [global, type]
whereas the new gdb says:
[1008] A: 0 [global, type]
Here the old one is correct because the A in CU 0 is just a
declaration without a size:
<1><45>: Abbrev Number: 10 (DW_TAG_structure_type)
<46> DW_AT_name : A
<48> DW_AT_declaration : 1
<48> DW_AT_sibling : <0x6d>
This patch fixes the problem by introducing the idea of a "type
declaration". I think gdb still needs to recurse into these types,
searching for methods, but by marking the type itself as a
declaration, gdb can skip this type during lookups and when writing
the index.
Regression tested on x86-64 using the cc-with-gdb-index board.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29694
A user noticed that gdb would crash when printing a packed array after
doing "set lang c". Packed arrays don't exist in C, but it's
occasionally useful to print things in C mode when working in a non-C
language -- this lets you see under the hood a little bit.
The bug here is that generic value printing does not handle packed
arrays at all. This patch fixes the bug by introducing a new function
to extract a value from a bit offset and width.
The new function includes a hack to avoid problems with some existing
test cases when using -fgnat-encodings=all. Cleaning up this code
looked difficult, and since "all" is effectively deprecated, I thought
it made sense to simply work around the problems.
A user found a bug where an array of packed arrays was printed
incorrectly. The bug here is that the packed array has a bit stride,
but the outer array does not -- and should not. However,
update_static_array_size does not distinguish between an array of
packed arrays and a multi-dimensional packed array, and for the
latter, only the innermost array will end up with a stride.
This patch fixes the problem by adding a flag to indicate whether a
given array type is a constituent of a multi-dimensional array.
PR 29654
* ld.h (struct ld_config_type): Add no_warnings field.
* ldlex.h (enum option_values): Add OPTION_NO_WARNINGS.
* lexsup.c (ld_options): Add --no-warnings.
(parse_args): Add support for -w and --no-warnings.
* ldmisc.c (vfinfo): Return early if the message is a warning and
-w has been enabled.
* ld.texi (options): Document new command line option.
* NEWS: Mention the new feature.
Currently, when using GDB to do reverse debugging, if we try to use the
command "reverse next" to skip a recursive function, instead of skipping
all of the recursive calls and stopping in the previous line, we stop at
the second to last recursive call, and need to manually step backwards
until we leave the first call. This is well documented in PR gdb/16678.
This bug happens because when GDB notices that a reverse step has
entered into a function, GDB will add a step_resume_breakpoint at the
start of the function, then single step out of the prologue once that
breakpoint is hit. The problem was happening because GDB wouldn't give
that step_resume_breakpoint a frame-id, so the first time the breakpoint
was hit, the inferior would be stopped. This is fixed by giving the
current frame-id to the breakpoint.
This commit also changes gdb.reverse/step-reverse.c to contain a
recursive function and attempt to both, skip it altogether, and to skip
the second call from inside the first call, as this setup broke a
previous version of the patch.
When GDB is stopped at a ret instruction and no debug information is
available for unwinding, GDB defaults to the amd64 epilogue unwinder, to
be able to generate a decent backtrace. However, when calculating the
frame id, the epilogue unwinder generates information as if the return
instruction was the whole frame.
This was an issue especially when attempting to reverse debug, as GDB
would place a step_resume_breakpoint from the epilogue of a function if
we were to attempt to skip that function, and this breakpoint should
ideally have the current function's frame_id to avoid other problems
such as PR record/16678.
This commit changes the frame_id calculation for the amd64 epilogue,
so that it is always the same as the dwarf2 unwinder's frame_id.
It also adds a test to confirm that the frame_id will be the same,
regardless of using the epilogue unwinder or not, thanks to Andrew
Burgess.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Similarly to booleans and following the fix for PR python/29217 make
`gdb.parameter' accept `None' for `unlimited' with parameters of the
PARAM_UINTEGER, PARAM_INTEGER, and PARAM_ZUINTEGER_UNLIMITED types, as
`None' is already returned by parameters of the two former types, so
one might expect to be able to feed it back. It also makes it possible
to avoid the need to know what the internal integer representation is
for the special setting of `unlimited'.
Expand the testsuite accordingly.
Approved-By: Simon Marchi <simon.marchi@polymtl.ca>
Also verify PARAM_UINTEGER, PARAM_INTEGER, and PARAM_ZINTEGER parameter
types, in addition to PARAM_ZUINTEGER and PARAM_ZUINTEGER_UNLIMITED
already covered, and verify a choice of existing GDB parameters. Add
verification for reading parameters via `<parameter>.value' in addition
to `gdb.parameter('<parameter>')' as this covers different code paths.
Approved-By: Simon Marchi <simon.marchi@polymtl.ca>
Do not assert that a value intended for an integer parameter, of either
the PARAM_UINTEGER or the PARAM_ZUINTEGER_UNLIMITED type, is boolean,
causing error messages such as:
ERROR: In procedure make-parameter:
ERROR: In procedure gdbscm_make_parameter: Wrong type argument in position 15 (expecting integer or #:unlimited): 3
Error while executing Scheme code.
when initialization with a number is attempted. Instead assert that it
is integer. Keep matching `#:unlimited' keyword as an alternative. Add
suitable test cases.
Approved-By: Simon Marchi <simon.marchi@polymtl.ca>
With gcc 7.5.0 and test-case gdb.base/rtld-step.exp, I run into:
...
gdb compile failed, gcc: error: unrecognized command line option \
'-static-pie'; did you mean '-static'?
...
Silence this by checking in the test-case that -static-pie is supported, and
emitting instead:
...
UNTESTED: gdb.base/rtld-step.exp: \
failed to compile (-static-pie not supported or static libc missing)
...
Tested on x86_64-linux, with:
- gcc 7.5.0: UNTESTED
- gcc 12.2.1 with static glibc not installed: UNTESTED
- gcc 12.2.1 with static glibc installed: PASS
Not only that sim/configure.ac does not AC_SUBST CXXFLAGS,
unless we need C++ compiler like CXX, substitution @CXXFLAGS@ is useless.
Because of this, this commit removes this substitution.
Since
commit 837e225ba1
Author: Jan Beulich <jbeulich@suse.com>
Date: Thu Oct 20 10:01:12 2022 +0200
x86: re-work AVX-VNNI support
moved AVX-VNNI after AVX512-VNNI, vector Disp8 is applied even when VEX
encoding is selected. Check VEX/EVEX encoding before checking vector
operands to avoid vector Disp8 with VEX encoding.
PR gas/29708
* config/tc-i386.c (match_template): Check VEX/EVEX encoding
before checking vector operands.
* testsuite/gas/i386/avx-vnni.d: Updated.
* testsuite/gas/i386/x86-64-avx-vnni.d: Likewise.
* testsuite/gas/i386/avx-vnni.s: Add a Disp32 test.
* testsuite/gas/i386/x86-64-avx-vnni.s: Likewise.
In a later commit in this series I will propose removing all of the
explicit gdbpy_initialize_* calls from python.c and replace these
calls with a more generic mechanism.
One of the side effects of this generic mechanism is that the order in
which the various Python sub-systems within GDB are initialized is no
longer guaranteed.
On the whole I don't think this matters, most of the sub-systems are
independent of each other, though testing did reveal a few places
where we did have dependencies, though I don't think those
dependencies were explicitly documented in comment anywhere.
This commit is similar to the previous one, and fixes the second
dependency issue that I found.
In this case the finish_breakpoint_object_type uses the
breakpoint_object_type as its tp_base, this means that
breakpoint_object_type must have been initialized with a call to
PyType_Ready before finish_breakpoint_object_type can be initialized.
Previously we depended on the ordering of calls to
gdbpy_initialize_breakpoints and gdbpy_initialize_finishbreakpoints in
python.c.
After this commit a new function gdbpy_breakpoint_init_breakpoint_type
exists, this function ensures that breakpoint_object_type has been
initialized, and can be called from any gdbpy_initialize_* function.
I feel that this change makes the dependency explicit, which I think
is a good thing.
There should be no user visible changes after this commit.
In a later commit in this series I will propose removing all of the
explicit gdbpy_initialize_* calls from python.c and replace these
calls with a more generic mechanism.
One of the side effects of this generic mechanism is that the order in
which the various Python sub-systems within GDB are initialized is no
longer guaranteed.
On the whole I don't think this matters, most of the sub-systems are
independent of each other, though testing did reveal a few places
where we did have dependencies, though I don't think those
dependencies were explicitly documented in a comment anywhere.
This commit removes the first dependency issue, with this and the next
commit, all of the implicit inter-sub-system dependencies will be
replaced by explicit dependencies, which will allow me to, I think,
clean up how the sub-systems are initialized.
The dependency is around the py_insn_type. This type is setup in
gdbpy_initialize_instruction and used in gdbpy_initialize_record.
Rather than depend on the calls to these two functions being in a
particular order, in this commit I propose adding a new function
py_insn_get_insn_type. This function will take care of setting up the
py_insn_type type and calling PyType_Ready. This helper function can
be called from gdbpy_initialize_record and
gdbpy_initialize_instruction, and the py_insn_type will be initialized
just once.
To me this is better, the dependency is now really obvious, but also,
we no longer care in which order gdbpy_initialize_record and
gdbpy_initialize_instruction are called.
There should be no user visible changes after this commit.
Some int to bool conversion in breakpoint.c. I've only updated the
function signatures of static functions, but I've updated some
function local variables throughout the file.
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
I noticed that we could make use of a scoped_restore in the function
unduplicated_should_be_inserted. I've also converted the function
return type from int to bool.
This change shouldn't make any difference, as I don't think anything
within should_be_inserted could throw an exception, but the change
doesn't hurt, and will help keep us safe if anything ever changes in
the future.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
I was doing some int to bool cleanup in update_watchpoint, and I
noticed a manual version of scoped_restore_selected_frame. As always
when these things are done manually, there is the chance that, in an
error case, we might leave the wrong frame selected.
This commit updates things to use scoped_restore_selected_frame, and
also converts a local variable from int to bool.
The only user visible change after this commit is in the case where
update_watchpoint throws an error - we should now correctly restore
the previously selected frame. Otherwise, this commit should be
invisible to the user.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
I spotted a few places where I could make some 'bp_location *'
arguments constant in breakpoint.c.
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
By putting the templates after their AVX512 counterparts, the AVX512
flavors will be picked by default. That way the need to always use {vex}
ceases to exist once respective CPU features (AVX512-VNNI or AVX512VL as
a whole) have been disabled. This way the need for the PseudoVexPrefix
attribute also disappears.
With test-case gdb.debuginfod/fetch_src_and_symbols.exp and check-read1, I run
into:
...
(gdb) FAIL: gdb.debuginfod/fetch_src_and_symbols.exp: local_url: \
file fetch_src_and_symbols (got interactive prompt)
...
The problem is that this output:
...
Enable debuginfod for this session? (y or [n]) y^M
...
is matched using regexp "Enable debuginfod?.*" with matches only the first two
words of the output, after which an implicit clause in gdb_test_multiple triggers
on the second part containing the interactive prompt.
Fix this by included the interactive prompt in the regexp.
Tested on x86_64-linux.
With test-case gdb.mi/mi-disassemble.exp and check-read1 I run into:
...
FAIL: gdb.mi/mi-disassemble.exp: disassemble /b main
FAIL: gdb.mi/mi-disassemble.exp: get valueof "*((unsigned char *) 0x400549)"
...
The problem for both FAILs is that the output is parsed using
gdb_test_multiple, which has implicit clauses using $gdb_prompt, which can
match before the explicit clauses using $mi_gdb_prompt.
Fix this by passing -prompt "$mi_gdb_prompt$" to gdb_test_multiple.
Tested on x86-64-linux.
See the remarks in rtld-step.exp for a description of what this
test is about.
This test case has been tested using gcc on the following x86-64 Linux
distributions/releases:
Fedora 28
Fedora 32
Fedora 33
Fedora 34
Fedora 35
Fedora 36
Fedora 37
rawhide (f38)
RHEL 9.1
Ubuntu 22.04.1 LTS
It's also been tested (and found to be working) with
RUNTESTFLAGS="CC_FOR_TARGET=clang" on all of the above expect for
Fedora 28. The (old) version of clang available on F28 did not
accept the -static-pie option.
I also tried to make this test work on FreeBSD 13.1. While I think I
made significant progress, I was ultimately stymied by this message
which occurs when attempting to run the main program which has been
set to use the fake/pretend RTLD as the ELF interpreter:
ELF interpreter /path/to/rtld-step-rtld not found, error 22
I have left one of the flags (-static) in place which I believe
to be needed for FreeBSD (though since I never got it to work, I
don't know for sure.) I've also left some declarations needed
for FreeBSD in rtld-step-rtld.c. They're currently disabled via
a #if 0; you'll need to enable them if you want to try to make
it work on FreeBSD.
At present, GDB does not allow for the debugging of the runtime loader
and/or dynamic linker. Much of the time, this makes sense. An
application programmer doesn't normally want to see symbol resolution
code when stepping into a function that hasn't been resolved yet.
But someone who wishes to debug the runtime loader / dynamic linker
might place a breakpoint in that code and then wish to debug it
as normal. At the moment, this is not possible. Attempting to step
will cause GDB to internally step (and not stop) until code
unrelated to the dynamic linker is reached.
This commit makes a minor change to infrun.c which allows the dynamic
loader / linker to be debugged in the case where a step, next, etc.
is initiated from within that code.
While developing this fix, I tried some approaches which weren't quite
right. The GDB testusite definitely contains tests which FAIL when
it's done incorrectly. (At one point, I saw 17 regressions!) This
commit has been tested on x86-64 linux with no regressions.
Unlike other substitution, this substitution of @PROGRAM@ was done in
binutils/Makefile and it was intended for binutils/cxxfilt.man. @PROGRAM@
in binutils/cxxfilt.man is removed in the commit 0285c67df1 ("Automate
generate on man pages") in 2001 and @PROGRAM@ is ineffective since then.
Because PROGRAM substitution does nothing, removing this manual
substitution should be completely safe.
binutils/ChangeLog:
* doc/local.mk: Remove unused substitution PROGRAM.
* Makefile.in: Regenerate.
With the cc-with-tweaks.sh patch submitted here (
https://sourceware.org/pipermail/gdb-patches/2022-October/192586.html ) we run
with:
...
$ export STRIP_ARGS_STRIP_DEBUG=--strip-all
$ make check RUNTESTFLAGS="gdb.base/jit-reader.exp \
--target_board cc-with-gnu-debuglink"
...
into the following assert:
...
(gdb) run ^M
Starting program: jit-reader ^M
gdb/jit.c:1247: internal-error: jit_event_handler: \
Assertion `jiter->jiter_data != nullptr' failed.^M
...
Fix this by handling the
jit_bp_sym.objfile->separate_debug_objfile_backlink != nullptr case in
handle_jit_event.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29277
Currently, every internal_error call must be passed __FILE__/__LINE__
explicitly, like:
internal_error (__FILE__, __LINE__, "foo %d", var);
The need to pass in explicit __FILE__/__LINE__ is there probably
because the function predates widespread and portable variadic macros
availability. We can use variadic macros nowadays, and in fact, we
already use them in several places, including the related
gdb_assert_not_reached.
So this patch renames the internal_error function to something else,
and then reimplements internal_error as a variadic macro that expands
__FILE__/__LINE__ itself.
The result is that we now should call internal_error like so:
internal_error ("foo %d", var);
Likewise for internal_warning.
The patch adjusts all calls sites. 99% of the adjustments were done
with a perl/sed script.
The non-mechanical changes are in gdbsupport/errors.h,
gdbsupport/gdb_assert.h, and gdb/gdbarch.py.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
When building the iq2000 simulator I see a few warnings like this:
/tmp/build/sim/../../src/sim/iq2000/iq2000.c: In function ‘fetch_str’:
/tmp/build/sim/../../src/sim/iq2000/iq2000.c:50:54: error: pointer targets in passing argument 3 of ‘sim_read’ differ in signedness [-Werror=pointer-sign]
50 | sim_read (CPU_STATE (current_cpu), CPU2DATA(addr), buf, nr);
| ^~~
| |
| char *
I've silenced these warnings by casting buf to 'unsigned char *'.
With this change I now see no warnings when compiling iq2000.c, so
I've removed the line from Makefile.in that disables -Werror.
Makefile.in was also disabling -Werror when compiling mloop.c,
however, I'm not seeing any warnings when compiling that file, so I've
removed the -Werror disable in that case too.
When building the erc32 simulator I get a few warnings like this:
/tmp/build/sim/../../src/sim/erc32/exec.c:1377:21: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
1377 | sregs->fs[rd] = *((float32 *) & ddata[0]);
| ~^~~~~~~~~~~~~~~~~~~~~~~
The type of '& ddata[0]' will be 'uint32_t *', which is what triggers
the warning.
This commit makes use of memcpy when performing the type-punning,
which resolves the above warnings.
With this change, I now see no warnings when compiling exec.c, which
means that the line in Makefile.in that disables -Werror can be
removed.
There should be no change in behaviour after this commit.
The device_error function always ends up calling the error function,
which is itself marked as ATTRIBUTE_NORETURN, so it makes sense that
device_error should also be marked ATTRIBUTE_NORETURN.
Doing this resolves a few warnings from hw_ide.c about possibly
uninitialized variables - the variables are only uninitialized after
passing through a call to device_error, which obviously means the
variables are never really used uninitialized, the simulation will
terminate with the device_error call.
This commit is a follow on to:
commit 182421c9d2
Date: Tue Oct 11 15:02:08 2022 +0100
sim/ppc: fixes for arguments to printf style functions
where commit 182421c9d2 addressed issues with printf format
arguments that were causing the compiler to give an error, this commit
addresses issues that caused the compiler to emit a warning.
This commit is mostly either changing the format string to match the
argument, or in some cases, excess, unused arguments are removed.
I see an uninitialized variable warning (with gcc 9.3.1) from
cgen-run.c, like this:
/tmp/build/sim/../../src/sim/cris/../common/cgen-run.c: In function ‘sim_resume’:
/tmp/build/sim/../../src/sim/cris/../common/cgen-run.c:259:5: warning: ‘engine_fns$’ may be used uninitialized in this function [-Wmaybe-uninitialized]
259 | (* engine_fns[next_cpu_nr]) (cpu);
| ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/build/sim/../../src/sim/cris/../common/cgen-run.c:232:14: note: ‘engine_fns$’ was declared here
232 | ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
| ^~~~~~~~~~
This is a false positive - we over allocate engine_fn, and then only
initialize the nr_cpus entries which we will later go on to use.
However, we can easily silence this warning by initializing the unused
entries in engine_fns to NULL, this might also help if anyone ever
looks at engine_fns in a debugger, it should now be obvious which
entries are in use, and which are not.
With this change the warning is gone.
There should be no change in behaviour with this commit.
* testsuite/binutils-all/addr2line.exp: Tidy. For powerpc64
arrange to pass --synthetic to nm, and extract .main and .fn
symbol address for addr2line test. Handle default executable
extension on cygwin/mingw compilers.
I spotted that the gdb.gdb/unittest.exp script causes a temporary file
inserters_extractors-2.txt to be created in build/gdb/testsuite/
instead of in build/gdb/testsuite/output/gdb.gdb/unittest/.
This is because some of the 'maint selftest' tests create temporary
files in GDB's current directory, specifically, the two source files:
gdb/unittests/basic_string_view/inserters/wchar_t/2.cc
gdb/unittests/basic_string_view/inserters/char/2.cc
both create a temporary file called inserters_extractors-2.txt, though
we only run the second of these as part of GDB's selftests.
I initially proposed just using GDB's 'cd' command in unittest.exp to
switch to the test output directory before running the selftests,
however, Pedro pointed out that there was a risk here that, if GDB
crashed during shutdown, the generated core file would be left in the
test output directory rather than in the testsuite directory. As a
result, our clever core file spotting logic would fail to spot the
core file and alert the user.
Instead, I propose this slightly more involved solution. I've added a
new with_gdb_cwd directory proc, used like this:
with_gdb_cwd $directory {
# Tests here...
}
The new proc temporarily switches to $directory and then runs the
tests within the block. After running the tests the previous current
working directory is restored.
Additionally, after switching back to the previous cwd, we check that
GDB is still responsive. This means that if GDB crashed immediately
prior to restoring the previous directory, and left the core file in
the wrong place, then the responsiveness check will fail, and a FAIL
will be emitted, this should be enough to alert the user that
something has gone wrong.
With this commit in place the unittest.exp script now leaves its
temporary file in the test output directory.
