Age | Commit message (Collapse) | Author | Files | Lines |
|
Running tclint on the test-cases in gdb.threads shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.python shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
In test-case gdb.python/py-pp-maint.exp, I came across:
...
gdb_test "disable pretty-printer global lookup_function_lookup_test" \
"1 printer disabled.*[expr $num_pp - 1] of $num_pp printers enabled"
gdb_test "disable pretty-printer global pp-test;.*" \
"[expr 5] printers disabled.*0 of $num_pp printers enabled"
...
The "[expr 5]" can simply be rewritten as "5", but instead use the construct
used in the previous gdb_test: [expr {$num_pp - 1}], given that the numbers
should match.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.btrace shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.fortran shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.reverse shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.guile shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
MI testcases currently all fail on native Windows with:
Running /c/gdb/src/gdb/testsuite/gdb.mi/mi-simplerun.exp ...
ERROR: (timeout) GDB never initialized after 10 seconds.
This is because when GDB is started in MI mode, it prints info to the
terminal before -iex options are processed. I.e., before the "maint
set console-translation-mode binary" command in
gdb -nw -nx -q -iex "set height 0" -iex "set width 0" \
-iex "set interactive-mode on" \
-iex "maint set console-translation-mode binary" \
-i=mi
... is processed. This results in GDB printing early output with
\r\r\n, like can be easily seen by passing --debug to runtest:
expect: does "=thread-group-added,id="i1"\r\r\n=cmd-param-changed,param="width",value="4294967295"\r\r\n=cmd-param-changed,param="interactive-mode",value="on"\r\r\n(gdb) \r\n" (spawn_id exp10) match regular expression "~"GNU.*\r\n~".*[(]gdb[)] \r\n$"? Gate "~"GNU*\r\n~"*gdb? \r\n"? gate=no
Fix this by adding a new Windows-only --binary-output command line
option to GDB, which is processed much earlier than -iex, and making
the testsuite pass that instead of "maint set console-translation-mode
binary".
Remove "maint set console-translation-mode" completely, since the only
reason it existed was for the testsuite, and it was never included in
any release.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Tom de Vries <tdevries@suse.de>
Change-Id: I4632707bb7c8ca573cffff9641ddeb33a0e150af
|
|
Currently, the gdb DAP implementation doesn't provide a way to filter
based on the thrown Ada exception.
There isn't really an ideal way to handle this in DAP:
* Requiring an IDE to use an expression checking $_ada_exception
exposes the IDE to any workarounds needed to get this correct (see
ada-lang.c).
* The setExceptionBreakpoint "filterOptions" field doesn't allow a
special kind of condition to be set. (We could add one but we've
generally avoided gdb-specific extensions.)
* The "exceptionOptions" approach is under-documented. It could be
used but it would have to be in a somewhat gdb-specific way anyway
-- and this approach does not allow a separate condition that is an
expression.
So, after some internal discussion, we agreed that it isn't all that
useful to have conditions on Ada exception catchpoints. This patch
changes the implementation to treat the condition as an exception name
here.
|
|
Recent Go toolchains are causing GDB to crash on a relatively recent
workaround for a GAS bug:
commit a833790a626d9620319d0ca6aee23daa584d445c
Date: Wed Nov 1 00:33:12 2023 +0100
[gdb/symtab] Work around gas PR28629
In the original GAS bug, the first directory table entry did not contain
the current directory of the compilation. So the above commit added a
workaround fix to prepend the second directory table entry.
However recent Go toolchain compilations (specifically on aarch64)
only output a single directory table entry. Looking at the workaround:
if (lh->version == 5 && lh->is_valid_file_index (1))
{
std::string dir = lh->include_dir_at (1);
fnd.set_comp_dir (std::move (dir));
}
`lh->is_valid_file_index (1)' is true, but since the directory table only
has one entry, `include_dir_at (1)' returns nullptr. Consequently the
std::string ctor will segfault. Since there are no guarantees that the file
and directory tables are the same size, a better bounds check is to simply
rely on `include_dir_at' to ensure a valid directory table entry.
I have updated the workaround commit's test, gdb.dwarf2/dw2-gas-workaround.exp
and tested on x86_64 and aarch64 RHEL 9 and Fedora 41.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
In Ada, sometimes an array is represented as a "thick" pointer -- a
structure that holds a pointer to the array data and another pointer
to the bounds structure.
A new "extended access" feature is being added to GNAT which changes
the shape of these objects. With the new feature, the bounds are
inlined into the thick pointer.
This patch changes gdb to understand this new feature. A test case is
provided; it is written in C to avoid requiring a newer GNAT just for
this test.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This changes gdb.printing.make_visualizer to treat an optimized-out
pointer as a scalar variable -- that is, one that does not advertise
any children. This makes sense because such a pointer cannot be
dereferenced.
The test case checks this case, plus it ensures that synthetic
pointers still continue to work.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
The documentation says that ThreadExitedEvent is derived from
ThreadEvent, but the code does not actually implement this.
This patch fixes the problem. I propose applying this to gdb 17 as
well.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33444
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Running tclint on the test-cases in gdb.opencl shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.perf shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.compile shows a few problems.
While we're at it, likewise in lib/compile-support.exp
Fix these.
Tested on x86_64-linux (Fedora 42).
|
|
Running tclint on gdb.dap shows these warnings:
...
bt-nodebug.exp:74:16: namespace eval received an argument with a \
substitution, unable to parse its arguments [command-args]
eof.exp:33:1: expected braced word or word without substitutions in argument \
interpreted as script [command-args]
eof.exp:34:1: expected braced word or word without substitutions in argument \
interpreted as script [command-args]
...
The first one is for:
...
set list_form [namespace eval ton::2list $last_ton]
...
I don't think this can be fixed by rewriting into something similar, so ignore
this. Likewise in lib/dap-support.exp.
The second and third ones are for:
...
catch "close -i $gdb_spawn_id"
catch "wait -i $gdb_spawn_id"
...
which can easily be fixed by using curly braces instead of double quotes, but
doing so gets us:
...
eof.exp:33:8: unrecognized argument for close: -i [command-args]
...
This is because tclint doesn't have support for expect yet [1], so ignore this.
While we're at it, fix some trailing whitespace in lib/dap-support.
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://github.com/nmoroze/tclint/issues/118
|
|
After the commit:
commit e5e76451fa82e0bc00599af96382b361c3d6ac32
Date: Fri Oct 22 07:19:29 2021 +0000
gdb/gdbserver: add a '--no-escape-args' command line option
Inferior argument handling on the GDB command line was broken:
$ gdb --args /bin/ls --foo
./gdb/gdb: unrecognized option '--foo'
./gdb/gdb: `--args' specified but no program specified
Before the above patch the definition of the '--args' argument in the
long_options array (in captured_main_1) was such that the
getopt_long_only call would directly set the 'set_args' variable to
true if '--args' was seen.
This meant that, immediately after the getopt_long_only call, we could
inspect set_args and break out of the argument processing loop if
needed.
After the above patch '--args' (and the new '--no-escape-args') no
longer set set_args directly via the getopt_long_only call. Instead
the getopt_long_only call returns an OPT_* enum value, which we then
use in the following switch statement in order to set the set_args
variable.
What this means is that, immediately after the getopt_long_only call,
set_args no longer (immediately) indicates if --args was seen. After
the switch statement, when set_args has been updated, we go around the
argument processing loop again and call getopt_long_only once more.
This extra getopt_long_only call will, if it finds another argument
that starts with a dash, update the global optind to point to this
option. At this point things have gone wrong, GDB has now lost track
of the argument containing the program name the user wanted us to
start. This leads to GDB exiting with the above error.
The solution is to move the check of set_args to either before the
getopt_long_only call, or to after the switch statement. I chose to
move it earlier as this keeps all the loop exiting checks near the
beginning.
I've added more tests that cover this issue.
Approved-By: Luis Machado <luis.machado.foss@gmail.com>
Tested-By: Luis Machado <luis.machado.foss@gmail.com>
|
|
Running tclint on the test-cases in gdb.ada shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.asm shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.disasm shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.go shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.gdb shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.dlang shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.ctf shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.server shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.debuginfod shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.linespec shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Running tclint on the test-cases in gdb.multi shows a few problems.
Fix these.
Tested on x86_64-linux.
|
|
Remove some more uses of the Tcl "eval" proc.
In most cases the {*} "splat" expansion is used instead.
The exceptions are:
- gdb.base/inferior-args.exp where we rewrite:
set cmd [format "lappend item \{ '%c' '\\%c' \}" 34 34]
eval $cmd
into:
lappend item [format { '%c' '\%c' } 34 34]
- reset_vars in lib/check-test-names.exp where we simply drop an unnecessary
eval
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The DWARF assembler treats the 'children' of a DIE as plain Tcl code,
evaluating it in the parent context.
I don't recall why, but when I wrote this code, I didn't do the same
thing for the attributes. Instead, there I implemented a special
syntax. I was looking at this today and wondered why I didn't just
use ordinary evaluation as well.
This patch implements this idea. Attributes are now evaluated as
plain code. This is a bit less "magical", is slightly shorter due to
lack of braces, and most importantly now allows comments in the
attributes section.
Note that some [subst {}] calls had to be added. This could be fixed
by changing DWARF expressions to also be plain Tcl code. I think that
would be a good idea, but I didn't want to tack it on here.
This patch requires the full ("DW_AT_...") name for attributes. I did
this to avoid any possibility of name clashes. I've long considered
that my original decision to allow short names for tags and attributes
was a mistake. It's worth noting that many existing tests already
used the long names here.
Most of this patch was written by script. The main changes are in
dwarf.exp, but as noted, there were some minor fixups needed in some
tests.
Also, after committing, 'git show' indicated some whitespace issues,
so I've gone through and "tabified" some things, which is why the
patch might be otherwise larger than it should be. (This was
discussed a bit during the v1 submission.)
v1 was here:
https://inbox.sourceware.org/gdb-patches/20250530183845.2179955-1-tromey@adacore.com/
In v2 I've rebased and fixed up various tests that either changed or
were added since v1.
Regression tested on x86-64 Fedora 41.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
When GDB is configured with --program-prefix, we see:
Running /home/pedro/gdb/src/gdb/testsuite/gdb.base/gcorebg.exp ...
FAIL: gdb.base/gcorebg.exp: detached=detached: Spawned gcore finished
FAIL: gdb.base/gcorebg.exp: detached=detached: Core file generated by gcore
FAIL: gdb.base/gcorebg.exp: detached=standard: Spawned gcore finished
FAIL: gdb.base/gcorebg.exp: detached=standard: Core file generated by gcore
The problem is here (with --program-prefix=prefix-), from gdb.log:
gcore: GDB binary (/home/pedro/gdb/build-program-prefix/gdb/testsuite/../../gdb/prefix-gdb) not found
FAIL: gdb.base/gcorebg.exp: detached=detached: Spawned gcore finished
That is gcore (the script, not the GDB command) trying to run the
installed GDB:
if [ ! -f "$binary_path/@GDB_TRANSFORM_NAME@" ]; then
echo "gcore: GDB binary (${binary_path}/@GDB_TRANSFORM_NAME@) not found"
exit 1
fi
...
"$binary_path/@GDB_TRANSFORM_NAME@" </dev/null \
...
When running the testsuite with the just-built GDB, the GDB binary is
'gdb', not @GDB_TRANSFORM_NAME@.
Fix this by adding a new '-g gdb" option to the 'gcore' script, that
lets you override the GDB binary gcore runs, and then making
gdb.base/gcorebg.exp pass it to gcore. The GDB binary we're testing
is always in the $GDB global. This is similar to how it is already
possible to specify GDB's data directory with an option to gcore, and
then gdb.base/gcorebg.exp uses it.
NEWS and documentation changes included.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Change-Id: I6c60fba8768618eeba8d8d03b131dc756b57ee78
|
|
Commit d09eba07 ("Make get_compiler_info use gdb_caching_proc")
regressed some tests when you run them in isolation (as this depends
on the order the gdb_caching_proc procs' results are cached).
E.g.:
Running /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.exp ...
ERROR: tcl error sourcing /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.exp.
ERROR: tcl error code TCL WRONGARGS
ERROR: wrong # args: should be "gdb_real__get_compiler_info_1 language"
while executing
"gdb_real__get_compiler_info_1"
("uplevel" body line 1)
invoked from within
"uplevel 2 $real_name"
(procedure "gdb_do_cache_wrap" line 3)
invoked from within
"gdb_do_cache_wrap $real_name {*}$args"
(procedure "gdb_do_cache" line 98)
invoked from within
gdb.base/attach.exp triggers it too, for example.
This is actually a latent problem in gdb_do_cache_wrap, introduced in:
commit 71f1ab80f1aabd70bce526635f84c7b849e8a0f4
CommitDate: Mon Mar 6 16:49:19 2023 +0100
[gdb/testsuite] Allow args in gdb_caching_proc
This change:
# Call proc real_name and return the result, while ignoring calls to pass.
-proc gdb_do_cache_wrap {real_name} {
+proc gdb_do_cache_wrap {real_name args} {
if { [info procs save_pass] != "" } {
return [uplevel 2 $real_name] <<<<<<<<<<<<<<<<<<<<<<< HERE
}
@@ -31,7 +31,7 @@ proc gdb_do_cache_wrap {real_name} {
rename pass save_pass
rename ignore_pass pass
- set code [catch {uplevel 2 $real_name} result]
+ set code [catch {uplevel 2 [list $real_name {*}$args]} result]
Missed updating the line marked with HERE above, to pass down $args.
So the case of a caching proc calling another caching proc with args
isn't handled correctly.
We could fix this by fixing the HERE line like so:
- return [uplevel 2 $real_name]
+ return [uplevel 2 [list $real_name {*}$args]]
However, we have with_override nowadays that we can use here which
eliminates the duplicated logic, which was what was missed originally.
A new test that exposes the problem is added to
gdb.testsuite/gdb-caching-proc.exp.
This also adds a new test to gdb.testsuite/with-override.exp that I
think was missing, making sure that the inner foo override restores
the outer foo override.
Tested-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I8b2a7366bf910902fe5f547bde58c3b475bf5133
|
|
While running tests on Windows with:
$ make check-parallel RUNTESTFLAGS="-v"
I noticed that get_compiler_info was invoking the compiler over and
over for each testcase, even though the result is supposed to be
cached.
This isn't normally very visible in gdb.log, because we suppress it
there:
# Run $ifile through the right preprocessor.
# Toggle gdb.log to keep the compiler output out of the log.
set saved_log [log_file -info]
log_file
...
I'm not sure it's a good idea to do that suppression, BTW. I was very
confused when I couldn't find the compiler invocation in gdb.log, and
it took me a while to notice that code.
The reason get_compiler_info in parallel mode isn't hitting the cache
is that in that mode each testcase runs under its own expect/dejagnu
process, and the way get_compiler_info caches results currently
doesn't handle that -- the result is simply cached in a global
variable, which is private to each expect.
So improve this by switching get_compiler_info's caching mechanism to
gdb_caching_proc instead, so that results are cached across parallel
invocations of dejagnu.
On an x86-64 GNU/Linux run with "make check-parallel -j32", before the
patch I get 2223 calls to get_compiler_info that result in a compiler
invocation. After the patch, I get 7.
On GNU/Linux, those compiler invocations don't cost much, but on
Windows, they add up. On my machine each invocation takes around
500ms to 700ms. Here is one representative run:
$ time x86_64-w64-mingw32-gcc \
/c/msys2/home/alves/gdb/build-testsuite/temp/14826/compiler.c \
-fdiagnostics-color=never -E
...
real 0m0.639s
user 0m0.061s
sys 0m0.141s
This reference to a 'compiler_info' global:
# N.B. compiler_info is intended to be local to this file.
# Call test_compiler_info with no arguments to fetch its value.
# Yes, this is counterintuitive when there's get_compiler_info,
# but that's the current API.
if [info exists compiler_info] {
unset compiler_info
}
is outdated, even before this patch, as "compiler_info" is a local
variable in get_compiler_info. Remove all that code.
Since test_compiler_info now calls get_compiler_info directly, the
"Requires get_compiler_info" comments in skip_inline_frame_tests and
skip_inline_var_tests are no longer accurate. Remove them.
test_compiler_info's intro comment is also outdated; improve it.
Changing the return value of get_compiler_info to be the
'compiler_info' string directly instead of 0/-1 was simpler. It would
be possible to support the current 0/-1 interface by making
get_compiler_info_1 still return the 'compiler_info' string, and then
having the get_compiler_info wrapper convert to 0/-1, and I considered
doing that. But the only caller of get_compiler_info outside gdb.exp
is gdb.python/py-event-load.exp, and it seems that one simply crossed
wires with:
commit 9704b8b4bc58f4f464961cca97d362fd33740ce8
gdb/testsuite: remove unneeded calls to get_compiler_info
as the test as added at roughly the same time as that commit.
So simply remove that call in gdb.python/py-event-load.exp, otherwise
we get something like:
ERROR: -------------------------------------------
ERROR: in testcase src/gdb/testsuite/gdb.python/py-event-load.exp
ERROR: expected boolean value but got "gcc-13-3-0"
ERROR: tcl error code TCL VALUE NUMBER
ERROR: tcl error info:
expected boolean value but got "gcc-13-3-0"
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ia3d3dc34f7cdcf9a2013f1054128c62a108eabfb
|
|
This patch removes a lot of uses of the Tcl "eval" proc from the gdb
test suite. In most cases the {*} "splat" expansion is used instead.
A few uses of eval remain, primarily ones that were more complicated
to untangle.
In a couple of tests I also replaced some ad hoc code with
string_to_regexp.
Tested on x86-64 Fedora 40.
Reviewed-By: Tom de Vries <tdevries@suse.de>
|
|
The current behaviour for 'show remote exec-file' is this:
(gdb) show remote exec-file
(gdb) set remote exec-file /abc
(gdb) show remote exec-file
/abc
(gdb)
The first output, the blank line, is just GDB showing the default
empty value.
This output is not really inline with GDB's more full sentence style
output, so in this commit I've updated things, the output is now:
(gdb) show remote exec-file
The remote exec-file is unset, the default remote executable will be used.
(gdb) set remote exec-file /abc
(gdb) show remote exec-file
The remote exec-file is "/abc".
(gdb)
Which I think is more helpful to the user.
I have also updated the help text for this setting. Previously we had
a set/show header line, but no body text, now we have:
(gdb) help show remote exec-file
Show the remote file name for starting inferiors.
This is the file name, on the remote target, used when starting an
inferior, for example with the \"run\", \"start\", or \"starti\"
commands. This setting is only useful when debugging a remote target,
otherwise, this setting is not used.
(gdb)
Which I think is more helpful.
Reviewed-By: Mark Wielaard <mark@klomp.org>
Tested-By: Mark Wielaard <mark@klomp.org>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This commit makes two related changes. The goal of the commit is to
update the 'remote exec-file' setting to work correctly in a
multi-inferior setup. To do this I have switched from the older
style add_setshow_* function, which uses a single backing variable, to
the newer style add_setshow_* functions that uses a get/set callback.
The get/set callbacks now directly access the state held in the
progspace which ensures that the correct value is always returned.
However, the new get/set API requires that the get callback return a
reference to the setting's value, which in this case needs to be a
std::string.
Currently the 'remote exec-file' setting is stored as a 'char *'
string, which isn't going to work.
And so, this commit also changes 'remote exec-file' to be stored as a
std::string within the progspace.
Now, when switching between multiple inferiors, GDB can correctly
inform the user about the value of the 'remote exec-file' setting.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Post-commit review [1] pointed out that this change in gdb.tui/empty.exp:
...
- eval Term::check_box [list "box $boxno"] $box
+ Term::check_box [list "box $boxno"] {*}$box
...
is incomplete because it leaves the "[list ...]" in place.
Indeed, it changes the test name like this:
...
-PASS: gdb.tui/empty.exp: src: 80x24: box 1
+PASS: gdb.tui/empty.exp: src: 80x24: {box 1}
...
Fix this by dropping the "[list ...]".
Likewise in gdb.tui/new-layout.exp.
[1] https://sourceware.org/pipermail/gdb-patches/2025-September/220863.html
|
|
Found by the codespell pre-commit hook.
Change-Id: Iafadd9485ce334c069dc8dbdab88ac3fb5fba674
|
|
Testing on the AdaCore-internal equivalent to array_long_idx.exp
showed that it failed on 32-bit targets. This patch fixes the problem
by arranging to use types that aren't target-dependent.
|
|
This patch fixes a bug when 'set schedule-multiple on' is in use and a
second inferior is started using the 'run' command (or 'start' or
'starti'). This bug was reported as PR gdb/28777.
The problem appears as the first inferior terminating with an
unexpected SIGTRAP. The bug can be reproduced like this:
gdb -ex 'set schedule-multiple on' \
-ex 'file /tmp/spin' \
-ex 'break main' \
-ex 'run' \
-ex 'add-inferior' \
-ex 'inferior 2' \
-ex 'file /tmp/spin' \
-ex 'break main' \
-ex 'run'
The final 'run' can be replaced with 'start' or 'starti'. The output
is different in the 'starti' case, but the cause is the same. For the
'run' and 'start' cases the final output is:
Starting program: /tmp/spin
Program terminated with signal SIGTRAP, Trace/breakpoint trap.
The program no longer exists.
In the 'starti' case the output is:
Starting program: /tmp/spin
Thread 2.1 "spin" stopped.
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.
0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
What's happening is that GDB is failing to clear the previous proceed
status from inferior 1 before starting inferior 2. Normally when
schedule-multiple is off, this isn't a problem as 'run' only starts
the new inferior, and the new inferior will have no previous proceed
status that needs clearing.
But when schedule-multiple is on, starting a new inferior, with 'run'
and friends, will actually start all inferiors, including those that
previous stopped at a breakpoint with a SIGTRAP signal.
By failing to clear out the proceed status for those threads, when GDB
restarts inferior 1 it arranges for the thread to receive the SIGTRAP,
which is delivered, and, as GDB isn't expecting a SIGTRAP, is allowed
to kill the process.
Fix this by calling clear_proceed_status from run_command_1.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28777
|
|
This patch fixes a bug that was exposed by a test added in the
previous commit. After writing this patch I also discovered that this
issue had been reported as PR gdb/27322.
When 'maint set target-non-stop on' is in effect, then the remote
targets will be running in non-stop mode. The previous commit
revealed a bug where, in this mode, GDB can fail to copy the thread
state from the target to the GDB frontend, this leaves the thread
marked as running in the frontend, even though the thread is actually
stopped. When this happens the user is no longer able to interrupt
the thread (it's already stopped), nor can the user resume the
thread (GDB thinks the threads is running).
To reproduce the bug:
gdb -q -ex 'maint set target-non-stop on' \
-ex 'set non-stop off' \
-ex 'set sysroot' \
-ex 'file /bin/ls' \
-ex 'run &' \
-ex 'add-inferior' \
-ex 'infer 2' \
-ex 'set sysroot' \
-ex 'target remote | gdbserver - ls' \
-ex 'info threads'
The 'info threads' output will look something like:
Id Target Id Frame
1.1 process 1746383 "ls" (running)
* 2.1 Thread 1746389.1746389 "ls" 0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
The thread 1.1 should be stopped. GDB is running in all-stop mode
after all.
The problem is that in remote_target::process_initial_stop_replies,
there is a call to stop_all_threads, however, the changes in the
thread state are never copied back to the GDB frontend. This leaves
the threads stopped, but still marked running.
Solve this by adding a scoped_finish_thread_state. This is similar to
how scoped_finish_thread_state is used in run_command_1 when we start
a new inferior running.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27322
|
|
This patch proposes a fix for PR gdb/33147. The bug can be reproduced
like this:
gdb -q -ex 'file /bin/ls' \
-ex 'run &' \
-ex 'add-inferior' \
-ex 'infer 2' \
-ex 'set sysroot' \
-ex 'target remote | gdbserver - ls'
Which will trigger an assertion failure:
target.c:3760: internal-error: target_stop: Assertion `!proc_target->commit_resumed_state' failed.
The problem is that target_stop is being called for a target when
commit_resumed_state is true, the comment on
process_stratum_target::commit_resumed_state is pretty clear:
To simplify the implementation of targets, the following methods
are guaranteed to be called with COMMIT_RESUMED_STATE set to
false:
- resume
- stop
- wait
So clearly we're breaking a precondition of target_stop. In this
example there are two target, the native target (inferior 1), and the
remote target (inferior 2). It is the first, the native target, for
which commit_resumed_state is set incorrectly.
At the point target_stop is called looks like this:
#11 0x00000000009a3c19 in target_stop (ptid=...) at ../../src/gdb/target.c:3760
#12 target_stop (ptid=...) at ../../src/gdb/target.c:3756
#13 0x00000000007042f2 in stop_all_threads (reason=<optimized out>, inf=<optimized out>) at ../../src/gdb/infrun.c:5739
#14 0x0000000000711d3a in wait_for_inferior (inf=0x2b90fd0) at ../../src/gdb/infrun.c:4412
#15 start_remote (from_tty=from_tty@entry=1) at ../../src/gdb/infrun.c:3829
#16 0x0000000000897014 in remote_target::start_remote_1 (this=this@entry=0x2c4a520, from_tty=from_tty@entry=1, extended_p=extended_p@entry=0) at ../../src/gdb/remote.c:5350
#17 0x00000000008976e7 in remote_target::start_remote (extended_p=0, from_tty=1, this=0x2c4a520) at ../../src/gdb/remote.c:5441
#18 remote_target::open_1 (name=<optimized out>, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:6312
#19 0x00000000009a815f in open_target (args=0x7fffffffa93c "| gdbserver - ls", from_tty=1, command=<optimized out>) at ../../src/gdb/target.c:838
For new inferiors commit_resumed_state starts set to false, for this
reason, if we only start a remote inferior, then when
wait_for_inferior is called commit_resumed_state will be false, and
everything will work.
Further, as target_stop is only called for running threads, if, when
the remote inferior is started, all other threads (in other targets)
are already stopped, then GDB will never need to call target_stop for
the other targets, and so GDB will not notice that
commit_resumed_state for those target is set to true.
In this case though, as the first (native) inferior is left running in
the background while the remote inferior is created, and because GDB
is running in all-stop mode (so needs to stop all threads in all
targets), then GDB does call target_stop for the other targets, and so
spots that commit_resumed_state is not set correctly and asserts.
The fix is to add scoped_disable_commit_resumed somewhere in the call
stack. Initially I planned to add the scoped_disable_commit_resumed
in `wait_for_inferior`, however, this isn't good enough. This
location would solve the problem as described in the bug, but when
writing the test I extended the problem to also cover non-stop mode,
and this runs into a second problem, the same assertion, but triggered
from a different call path. For this new case the stack looks like
this:
#1 0x0000000000fb0e50 in target_stop (ptid=...) at ../../src/gdb/target.c:3771
#2 0x0000000000a7f0ae in stop_all_threads (reason=0x1d0ff74 "remote connect in all-stop", inf=0x0) at ../../src/gdb/infrun.c:5756
#3 0x0000000000d9c028 in remote_target::process_initial_stop_replies (this=0x3e10670, from_tty=1) at ../../src/gdb/remote.c:5017
#4 0x0000000000d9cdf0 in remote_target::start_remote_1 (this=0x3e10670, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:5405
#5 0x0000000000d9d0d4 in remote_target::start_remote (this=0x3e10670, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:5457
#6 0x0000000000d9e8ac in remote_target::open_1 (name=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1, extended_p=0) at ../../src/gdb/remote.c:6329
#7 0x0000000000d9d167 in remote_target::open (name=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1) at ../../src/gdb/remote.c:5479
#8 0x0000000000f9914d in open_target (args=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1, command=0x35d1a40) at ../../src/gdb/target.c:838
So I'm now thinking that stop_all_threads would be the best place for
the scoped_disable_commit_resumed. I did leave an assert in
wait_for_inferior as, having thought about the assert some, I do still
think the logic of it is true, and it doesn't hurt to leave it in
place I think.
However, it's not quite that simple, the test throws up yet another
bug when we 'maint set target-non-stop on', but then 'set non-stop
off'. This bug leaves a stopped thread marked as "(running)" in the
'info threads' output. I have a fix for this issue, but I'm leaving
that for the next commit. For now I've just disabled part of the test
in the problem case.
I've also tagged this patch with PR gdb/27322. That bug was created
before the above assert was added, but if you follow the steps to
reproduce for that bug today you will hit the above assert. The
actual issue described in PR gdb/27322 is fixed in the next patch.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27322
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33147
|
|
This patch fixes a multi-target issue where the normal_stop function
can fail to finish the thread state of threads from a non current
target, this leaves the threads marked as running in GDB core, while
the threads is actually stopped.
For testing I used this test program:
#include <unistd.h>
int
main ()
{
while (1)
sleep (1);
return 0;
}
Compile this to make '/tmp/spin', then the bug can be shown using this
command:
$ gdb -ex 'file /tmp/spin' \
-ex 'start' \
-ex 'add-inferior' \
-ex 'inferior 2' \
-ex 'set sysroot' \
-ex 'target extended-remote | gdbserver --multi --once - /tmp/spin' \
-ex 'inferior 1' \
-ex 'continue&' \
-ex 'inferior 2' \
-ex 'search sleep' \
-ex 'break $_ inferior 2' \
-ex 'continue' \
-ex 'info threads'
The interesting part of the output is:
Id Target Id Frame
1.1 process 1610445 "spin" (running)
* 2.1 Thread 1610451.1610451 "spin" main () at spin.c:7
(gdb)
Notice that thread 1.1 is marked as running when it should be
stopped. We can see that the thread is actually stopped if we try
this:
(gdb) inferior 1
[Switching to inferior 1 [process 1610445] (/tmp/spin)]
[Switching to thread 1.1 (process 1610445)](running)
(gdb) continue
Cannot execute this command while the selected thread is running.
(gdb) interrupt
(gdb) info threads
Id Target Id Frame
* 1.1 process 1610445 "spin" (running)
2.1 Thread 1610451.1610451 "spin" main () at spin.c:7
(gdb)
We can see the expected behaviour if both inferiors run on the same
target, like this:
$ gdb -ex 'file /tmp/spin' \
-ex 'start' \
-ex 'add-inferior' \
-ex 'inferior 2' \
-ex 'file /tmp/spin' \
-ex 'start' \
-ex 'inferior 1' \
-ex 'continue&' \
-ex 'inferior 2' \
-ex 'search sleep' \
-ex 'break $_ inferior 2' \
-ex 'continue' \
-ex 'info threads'
The 'info threads' from this series of commands looks like this:
Id Target Id Frame
1.1 process 1611589 "spin" 0x00007ffff7e951e7 in nanosleep () from /lib64/libc.so.6
* 2.1 process 1611593 "spin" main () at spin.c:7
(gdb)
Now both threads are stopped as we'd expect.
The problem is in normal_stop. The scoped_finish_thread_state uses
user_visible_resume_target to select the target(s) over which GDB will
iterate to find the threads to update.
The problem with this is that when the ptid_t is minus_one_ptid,
meaning all threads, user_visible_resume_target only returns nullptr,
meaning all targets, when sched_multi is true.
This dependency on sched_multi makes sense when _resuming_ threads.
If we are resuming all threads, then when sched_multi (the
schedule-multiple setting) is off (the default), all threads actually
means all threads in the current inferior only. When sched_multi is
true (schedule-multiple is on) then this means all threads, from all
inferiors, which means GDB needs to consider every target.
However, when stopping an inferior in all-stop mode (non_stop is
false), then GDB wants to stop all threads from all inferiors,
regardless of the sched_multi setting.
What this means is that, when 'non_stop' is false, then we should be
passing nullptr as the target selection to scoped_finish_thread_state.
My proposal is that we should stop using user_visible_resume_target in
the normal_stop function for the target selection of the
scoped_finish_thread_state, instead we should manually figure out the
correct target value and pass this in.
There is precedent for this in GDB, see run_command_1, where
'finish_target' is calculated directly within the function rather than
using user_visible_resume_target.
After this commit, when using two different targets (native and
remote) as in my first example above, both threads will be correctly
stopped.
|
|
Find uses of the then keyword:
...
$ find gdb/testsuite/ -type f -name *.exp* | xargs grep "if.*then {"
...
and remove them.
See also commit d4c4542312c ("gdb/testsuite: remove use of then keyword from
library files") and related commits.
Tested on aarch64-linux.
|
|
Running tclint on the test-cases in gdb.opt shows a problem.
Fix it.
Tested on aarch64-linux.
|
|
Running tclint on the test-cases in gdb.opt shows a few problems.
Fix these.
Tested on aarch64-linux.
|
|
Running tclint on the test-cases in gdb.pascal shows a few problems.
Fix these.
Tested on aarch64-linux.
|
|
Running tclint on the test-cases in gdb.rocm shows a few problems:
...
precise-memory-multi-inferiors.exp:33:5: expected braced word or word \
without substitutions in argument interpreted as expr [command-args]
precise-memory-multi-inferiors.exp:43:5: expected braced word or word \
without substitutions in argument interpreted as expr [command-args]
precise-memory-multi-inferiors.exp:55:5: expected braced word or word \
without substitutions in argument interpreted as expr [command-args]
...
Fix these.
The gdb.rocm test-cases are unsupported for me, so I can't test this.
|
|
Running tclint on the test-cases in gdb.rust shows a few problems:
...
modules.exp:37:1: expected braced word or word without substitutions in \
argument interpreted as expr [command-args]
traits.exp:28:13: expected braced word or word without substitutions in \
argument interpreted as script [command-args]
...
Fix these.
Tested on aarch64-linux.
|