| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
|
|
function-view.h uses gdb::Or but does not include gdbsupport/traits.h.
This patch corrects the oversight.
|
|
When executed with the native-extended-gdbserver board file, we get
FAIL: gdb.base/save-history.exp: check history contents
This is because the history file contains an extra line for the
command 'target extended-remote ...'. Address this in the test.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
|
|
Prevent gdb.threads/schedlock.exp from crashing due to a premature exit
of the debuggee causing an attempt to use a nil value as an arithmetic
operand:
[...]
(gdb) FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: next to increment, 9
bt
The current thread has terminated
(gdb) FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: find current thread, after
FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: next does not change thread (switched to thread )
print args
Cannot access memory at address 0x410ab0
(gdb) FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: listed args, after
ERROR: tcl error sourcing .../gdb/testsuite/gdb.threads/schedlock.exp.
ERROR: can't use empty string as operand of "-"
while executing
"if {$cmd == "continue"
|| [lindex $before_args $i] == [lindex $after_args $i] - 10} {
pass "$test"
} else {
fail "$test (wrong amo..."
(procedure "check_result" line 31)
invoked from within
"check_result $cmd $curthread $before_args $locked"
(procedure "test_step" line 26)
invoked from within
"test_step $schedlock "next" $call_function"
("uplevel" body line 2)
invoked from within
"uplevel 1 $body"
invoked from within
"with_test_prefix "call_function=$call_function" {
test_step $schedlock "next" $call_function
}"
("foreach" body line 2)
invoked from within
"foreach call_function {0 1} {
with_test_prefix "call_function=$call_function" {
test_step $schedlock "next" $call_function
}
}"
("uplevel" body line 6)
invoked from within
"uplevel 1 $body"
invoked from within
"with_test_prefix "cmd=next" {
# In GDB <= 7.9, with schedlock "step", "next" would
# unlock threads when stepping over a function call. Thi..."
("uplevel" body line 5)
invoked from within
"uplevel 1 $body"
invoked from within
"with_test_prefix "schedlock=$schedlock" {
with_test_prefix "cmd=step" {
test_step $schedlock "step" 0
}
with_test_prefix "cmd=next" {
# I..."
("foreach" body line 2)
invoked from within
"foreach schedlock {"off" "step" "on"} {
with_test_prefix "schedlock=$schedlock" {
with_test_prefix "cmd=step" {
test_step $schedlock "step" ..."
(file ".../gdb/testsuite/gdb.threads/schedlock.exp" line 297)
invoked from within
"source .../gdb/testsuite/gdb.threads/schedlock.exp"
("uplevel" body line 1)
invoked from within
"uplevel #0 source .../gdb/testsuite/gdb.threads/schedlock.exp"
invoked from within
"catch "uplevel #0 source $test_file_name""
Remote debugging from host xx.xx.xx.xx, port 56596
monitor exit
(gdb) Killing process(es): 22658
testcase .../gdb/testsuite/gdb.threads/schedlock.exp completed in 32 seconds
Here `print args' has failed to produce output matching the pattern
expected by `get_args' and consequently an empty value has been assigned
to $after_args. Subsequently a calculation is attempted on an element
of said value treated as a list: `[lindex $after_args $i] - 10' and that
has caused the crash because the resulting minuend is nil.
There are various expressions $after_args and other variables set from
the result of `get_args' are used in, however the majority are equality
operations, which succeed producing a result even where a nil operand is
involved. Given that this is a test failure scenario anyway follow the
path of least resistance, ignore the other expressions and just prevent
the crash from triggering here by checking for an attempt to retrieve an
inexistent element of $after_args for this calculation, and report it as
a test failure outright letting the script proceed:
[...]
(gdb) FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: next to increment, 9
bt
The current thread has terminated
(gdb) FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: find current thread, after
FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: next does not change thread (switched to thread )
print args
Cannot access memory at address 0x410ab0
(gdb) FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: listed args, after
FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: current thread advanced - unlocked (no arg #1)
PASS: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=0: other threads ran - unlocked
set scheduler-locking off
(gdb) PASS: gdb.threads/schedlock.exp: schedlock=off: cmd=next: call_function=1: set scheduler-locking off
[...]
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
The callers of the procedure run_lang_tests expect it to return -1 in
case of error but in the case where runto_main fails, it will return
without any value. Make it return -1 in that case as well.
Found by code inspection.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Factor out a new_symbol variant out of new_symbol, containing mostly the
die->tag switch.
While we're at it, modernize the code using bool and nullptr. Also, remove
some unnecessary braces, and apply this simplification:
...
if (c)
foo (a);
else
foo (b);
->
foo (c
? a
: b);
...
I wondered about naming the new variant new_symbol_tag or new_symbol_1, but
I stuck with new_symbol.
Refactoring made we wonder why we use linkagename instead of sym->linkagename ()
after sym->set_linkage_name (), but since there are cornercases where these are
different, I've left it as is.
Also, I suspect the cp_scan_for_anonymous_namespaces can be hoisted, but I
also left that as is.
Finally, I noticed that physname == linkagename uses a pointer equivalence
test, which seems fragile to me. Also that I left as is.
The patch is easier to view with git show -w.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Say we simulate an error:
...
static void error_once () {
static int v = 0;
if (v == 1)
return;
v = 1;
error (_("Oh no!!!"));
}
...
in the call to tui_set_initial_layout in tui_enable:
...
tui_show_frame_info (deprecated_safe_get_selected_frame ());
+ error_once ();
tui_set_initial_layout ();
...
After doing "tui enable"
...
$ gdb
(gdb) tui enable
...
the screen is cleared, and we have:
...
❌️ Oh no!!!
(gdb) <blinking cursor>
...
After typing "apropos tui" (which is not echoed) and pressing enter, I run into
a segmentation violation here in tui_inject_newline_into_command_window:
...
at /data/vries/gdb/leap-16-0/build/../../src/gdb/tui/tui-io.c:1084
1084 WINDOW *w = tui_cmd_win ()->handle.get ();
...
because:
...
$2 = (tui_cmd_window *) 0x0
(gdb) p tui_cmd_win ()
...
The problem is that tui_active is true, and so
tui_inject_newline_into_command_window get called:
...
static void
tui_command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
{
...
if (tui_active)
tui_inject_newline_into_command_window ();
...
}
...
Fix this by catching the error in tui_enable, and resetting tui_active back to
false.
While this fixes the segmentation fault, and does allow "apropos tui" to run,
still the command is not echoed, and the output of the command is garbled by
runaway indentation.
Fix this by using endwin / delscreen, as borrowed from earlier in tui_enable:
...
if (cap == NULL || cap == (char *) -1 || *cap == '\0')
{
endwin ();
delscreen (s);
error (_("Cannot enable the TUI: "
"terminal doesn't support cursor addressing [TERM=%s]"),
gdb_getenv_term ());
}
...
Note that this doesn't allow a second attempt:
...
$ gdb -q
(gdb) tui enable
❌️ Oh no!!!
(gdb) tui enable
❌️ Cannot enable the TUI
(gdb)
...
because tui_finish_init is stuck at TRIBOOL_UNKNOWN.
IWBN to fix this in a way that allows the second "tui enable" to succeed. I
tried this for a bit, but didn't get it to work.
Tested on x86_64-linux.
Suggested-By: Tom Tromey <tom@tromey.com> [1]
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34100
[1] https://sourceware.org/pipermail/gdb-patches/2025-May/217660.html
|
|
I found two files in which there were two copyright notices, in both cases
with overlapping year ranges.
Fix this by merging the copyright notices, using a simple startyear-endyear
range, as per this text in gdb/copyright.py:
...
# We want to use year intervals in the copyright notices, and
# all years should be collapsed to one single year interval,
# even if there are "holes" in the list of years found in the
# original copyright notice (OK'ed by the FSF, case [gnu.org #719834]).
...
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I ran gdb/copyright.py, and encountered two files that were added in 2026 but
listed an older copyright year. Fix this.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
PR breakpoints/34112 reports that "rbreak <file>:<regexp>" sets breakpoints in
files other than <file>.
This is a regression since commit c4c093a31f6 ("Make
global_symbol_searcher::filenames private"), which did:
...
if (file_name != nullptr)
- spec.filenames.push_back (file_name);
+ spec.add_filename (std::move (file_name));
...
The std::move nullifies file_name, so a subsequent file_name check:
...
if (file_name != nullptr)
...
now always evaluates to false.
Fix this by:
- introducing a variable bool file_name_p, initialized before the
std::move, and
- using that instead.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34112
|
|
Fix two "expected braced word or word without substitutions in argument
interpreted as expr [command-args]" in gdb.dwarf2/fission-dw-form-strx.exp.
|
|
|
|
Add a note to gdb/NEWS mentioning Windows native target
scheduler-locking support.
Approved-By: Eli Zaretskii <eliz@gnu.org>
Change-Id: Id0e28525c06e57120c47b07f978581d1627d70f3
commit-id:ba1d5868
|
|
Hannes noticed that commit db040a86c8 ("Windows gdb: Simplify
windows_nat_target::wait") inadvertently reverted the earlier
is_sw_breakpoint change in windows_nat_target::wait from 6fc89bae17
("Move software breakpoint recognition code into x86-windows-nat.c")
for Aarch64 support. This commit restores it.
Change-Id: Ice2181bf4a6c8dcefd127a85ebfc660a3f430517
commit-id:546f39de
|
|
The test was using gdb_compile_shlib to create the .dwo file, which
runs the linker. This is incorrect for DWARF fission - .dwo files
should be created by objcopy extraction, not linking.
Per commit 6a29913eeb9, "it was a bad idea to generate a .dwo file
using the linker, since the idea behind .dwo files is that they do
not need to be linked."
GCC's assembler doesn't set the SHF_EXCLUDE flag on .debug_*.dwo
sections, so the linker preserves them in the output. Clang's
assembler sets SHF_EXCLUDE, causing the linker to exclude these
sections. Both approaches are valid, but only the GCC approach
happens to work with this test's incorrect use of the linker.
Rewrite the test to use build_executable_and_dwo_files with the
split-dwo option, matching all other fission-*.exp tests. This uses
objcopy to properly extract .dwo sections without involving the
linker.
Changes:
- Use build_executable_and_dwo_files instead of gdb_compile_shlib
- Generate both skeleton and DWO CUs in single Dwarf::assemble block
- Add debug_str_offsets section for DW_FORM_strx support
- Add is_remote host check (required for objcopy-based workflow)
- Fix DW_AT_dwo_name to match generated filename
Tested: GCC 13.3.0, Clang-23 (passes)
|
|
Running gdb.dwarf2/pr13961.exp with clang fails with an internal error:
gdb/dwarf2/read.c: internal-error: decode_line_header_for_cu:
Assertion `!cu->per_cu->is_debug_types ()' failed.
pr13961 is a legacy .S test that references a .debug_line label from
both a CU and a TU. The assembly includes an empty .debug_line section
declaration:
.section .debug_line,"",%progbits
.Ldebug_line0:
With gcc this results in a dummy (valid, but content-less) .debug_line
header being emitted, so GDB can build a line header and resolve file
indices. With clang the .debug_line section can be missing/empty,
leaving the line header unset.
During symbol creation, new_symbol may then try to lazily decode the
CU-only line header while processing a type unit, which triggers the
assertion above.
Fix this by only decoding the line header for non-type units. If no
line header is available, emit a complaint and continue without setting
the symtab rather than attempting CU-only line decoding from a TU.
Tested: gdb.dwarf2/pr13961.exp (CC_FOR_TARGET=clang-23)
|
|
If no alignment is specified in the assembly file, LoongArch will not
perform forced alignment. When this object file (.o) is linked into an
executable, instructions may not be 4-byte aligned, which will
eventually cause instruction fetch errors.
For example, the above issue can occur when generating a shared object
file (.so) with the -nostdlib option.
|
|
|
|
|
|
Ran "pre-commit autoupdate". No changes in formatting.
|
|
|
|
Several watchpoint-related testcases, such as
gdb.threads/watchthreads.exp for example, when tested with the backend
in non-stop mode, exposed an interesting detail of the Windows debug
API that wasn't considered before. The symptom observed is spurious
SIGTRAPs, like:
Thread 1 "watchthreads" received signal SIGTRAP, Trace/breakpoint trap.
0x00000001004010b1 in main () at .../src/gdb/testsuite/gdb.threads/watchthreads.c:48
48 args[i] = 1; usleep (1); /* Init value. */
After a good amount of staring at logs and headscratching, I realized
the problem:
#0 - It all starts with the fact that multiple threads can hit an
event at the same time. Say, a watchpoint for thread A, and a
breakpoint for thread B.
#1 - Say, WaitForDebugEvent reports the breakpoint hit for thread B
first, then GDB for some reason decides to update debug
registers, and continue. Updating debug registers means writing
the debug registers to _all_ threads, with SetThreadContext.
#2 - WaitForDebugEvent reports the watchpoint hit for thread A.
Watchpoint hits are reported as EXCEPTION_SINGLE_STEP.
#3 - windows-nat checks the Dr6 debug register to check if the step
was a watchpoint or hardware breakpoint stop, and finds that Dr6
is completely cleared. So windows-nat reports a plain SIGTRAP
(given EXCEPTION_SINGLE_STEP) to the core.
#4 - Thread A was not supposed to be stepping, so infrun reports the
SIGTRAP to the user as a random signal.
The strange part is #3 above. Why was Dr6 cleared?
Turns out that (at least in Windows 10 & 11), writing to _any_ debug
register has the side effect of clearing Dr6, even if you write the
same values the registers already had, back to the registers.
I confirmed it clearly by adding this hack to GDB:
if (th->context.ContextFlags == 0)
{
th->context.ContextFlags = CONTEXT_DEBUGGER_DR;
/* Get current values of debug registers. */
CHECK (GetThreadContext (th->h, &th->context));
DEBUG_EVENTS ("For 0x%x (once), Dr6=0x%llx", th->tid, th->context.Dr6);
/* Write debug registers back to thread, same values,
and re-read them. */
CHECK (SetThreadContext (th->h, &th->context));
CHECK (GetThreadContext (th->h, &th->context));
DEBUG_EVENTS ("For 0x%x (twice), Dr6=0x%llx", th->tid, th->context.Dr6);
}
Which showed Dr6=0 after the write + re-read:
[windows events] fill_thread_context: For 0x6a0 (once), Dr6=0xffff0ff1
[windows events] fill_thread_context: For 0x6a0 (twice), Dr6=0x0
This commit fixes the issue by detecting that a thread has a pending
watchpoint hit to report (Dr6 has interesting bits set), and if so,
avoid modifying any debug register. Instead, let the pending
watchpoint hit be reported by WaitForDebugEvent. If infrun did want
to modify watchpoints, it will still be done when the thread is
eventually re-resumed after the pending watchpoint hit is reported.
(infrun knows how to gracefully handle the case of a watchpoint hit
for a watchpoint that has since been deleted.)
Move the fill_thread_context method from windows_nat_target to
windows_per_inferior so it can be used by gdbserver too.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I21a3daa9e34eecfa054f0fea706e5ab40aabe70a
commit-id:a28f8d4e
|
|
Move the Windows native debug macros to gdb/windows-nat.h, so that
they can by used in x86-windows-nat.c.
Change-Id: I002b3e3e319bcb005b2c6424affd750a0fcd1a83
commit-id:5ec9181a
|
|
The Windows backend functions that manipulate the x86 debug registers
are called "cygwin_foo", which is outdated, because native MinGW gdb
also uses those functions, they are not Cygwin-specific. Rename them
to "windows_foo" to avoid confusion.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I46df3b44f5272adadf960da398342a3cbdb98533
commit-id:896523e0
|
|
windows_nat_target::windows_continue, when it finds a resumed thread
that has a pending event, does:
/* There's no need to really continue, because there's already
another event pending. However, we do need to inform the
event loop of this. */
serial_event_set (m_wait_event);
return TRUE;
If we have more than one pending event ready to be consumed, and,
windows_nat_target::wait returns without calling
windows_nat_target::windows_continue, which it will with the non-stop
support in a later patch, then we will miss waking up the event loop.
This patch makes windows-nat.c manage the serial_event similarly to
how linux-nat.c does it. Clear it on entry to
windows_nat_target::wait, and set it if there may be more events to
process. With this, there's no need to set it from
windows_nat_target::wait_for_debug_event_main_thread, so the patch
also makes us not do it.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I44e1682721aa4866f1dbb052b3cfb4870fb13579
commit-id:669a42f6
|
|
After the previous patches, struct pending_stop only contains one
field. So move that field into the windows_thread_info structure
directly, and eliminate struct pending_stop.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I7955884b3f378d8b39b908f6252d215f6568b367
commit-id:fb68c808
|
|
Both GDB and GDBserver have similar code to read the $_siginfo data.
This patch moves the bulk of it to gdb/nat/windows-nat.c so it can be
shared.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I58f9074caf6362274453c78ed1fc9e31249f6854
|
|
The next patch will move some duplicated code in gdb and gdbserver to
gdb/nat/windows-nat.c, where it would be convenient to get at the
Windows process info of a given Windows thread info, from within a
windows_thread_info method.
I first thought of passing down the windows_process_info pointer as
argument to the windows_thread_info method, but that looked a bit odd.
I think it looks better to just add a back pointer, so that's what
this patch does. The following patch will then add a use of it.
I suspect this will help moving more duplicated code to
gdb/nat/windows-nat.c in the future, too.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I47fc0d3323be5b6f6fcfe912b768051a41910666
|
|
With non-stop mode support, each thread has its own "last event", and
so printing $_siginfo should print the siginfo of the selected thread.
Likewise, with all-stop and scheduler-locking.
This patch reworks the siginfo functions in gdb/windows-nat.c and
gdbserver/win32-low.cc to reuse the exception record already saved
within each thread's 'last_event' field.
Here's an example of what you'll see after the whole non-stop series:
(gdb) thread apply all p -pretty -- $_siginfo
Thread 3 (Thread 2612.0x1470):
$1 = {
ExceptionCode = DBG_CONTROL_C,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ffd0583e929 <KERNELBASE!EncodeRemotePointer+8249>,
NumberParameters = 0,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
Thread 2 (Thread 2612.0x1704):
$2 = {
ExceptionCode = SINGLE_STEP,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ffd080ad6e4 <ntdll!ZwDelayExecution+20>,
NumberParameters = 0,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
Thread 1 (Thread 2612.0x434):
$3 = {
ExceptionCode = BREAKPOINT,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ff6f691174c <main+185>,
NumberParameters = 1,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
(gdb)
This was in non-stop mode, and the program originally had two threads.
Thread 1 stopped for a breakpoint, then thread 2 was manually
interrupted/paused and then single-stepped. And then I typed Ctrl-C
in the inferior's terminal, which made Windows inject thread 3 in the
inferior, and report a DBG_CONTROL_C exception for it.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I5d4f1b62f59e8aef3606642c6524df2362b0fb7d
commit-id:e0f75dea
|
|
With non-stop mode, each thread is controlled independently of the
others, and each thread has its own independent reason for its last
stop.
Thus, any thread-specific state that is currently per-process must be
converted to per-thread state.
This patch converts windows_process_info::last_sig to per-thread
state, moving it to windows_thread_info instead.
This adjusts both native gdb and gdbserver.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ie8c673a819be445753d967afd3a6084565648448
commit-id:d7dd0d0e
|
|
With non-stop mode, each thread is controlled independently of the
others, and each thread has its own independent reason for its last
stop.
Thus, any thread-specific state that is currently per-process must be
converted to per-thread state.
This patch converts windows_process_info::current_event, moving it to
windows_thread_info instead, renamed to last_event.
Since each thread will have its own copy of its last Windows debug
event, we no longer need the same information stored in struct
pending_stop.
Since windows_process.current_event no longer exists, we need to pass
the current event as parameter to a number of methods.
This adjusts both native gdb and gdbserver.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ice09a5d932c912210608d5af25e1898f823e3c99
commit-id:ca7e9182
|
|
I noticed that faked_breakpoint is write only. And then I hacked
win32_process_target::request_interrupt to force it to stop threads
using the soft_interrupt_requested mechanism (which suspends threads,
and then fakes a breakpoint event in the main thread), and saw that it
no longer works -- gdbserver crashes accessing a NULL current_thread,
because fake_breakpoint_event does not switch to a thread.
This code was originally added for Windows CE, as neither
GenerateConsoleCtrlEvent nor DebugBreakProcess worked there. Windows
CE support has since been removed.
We nowadays require Windows XP or later, and XP has DebugBreakProcess.
The soft_interrupt_requested mechanism has other problems, like for
example faking the event in the main thread, even if that thread was
previously stopped, due to scheduler-locking.
A following patch will add a similar mechanism stopping all threads
with SuspendThread to native GDB, for non-stop mode, which doesn't
have these problems. It's different enough from this old code that I
think we should just rip the old code out, and reimplement it from
scratch (based on gdb's version) when we need it.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I89e98233a9c40c6dcba7c8e1dacee08603843fb1
|
|
Surprisingly (to me), enabling scheduler locking on Windows currently
fails:
(gdb)
set scheduler-locking on
Target 'native' cannot support this command.
The backend itself does support scheduler-locking. This patch
implements windows_nat_target::get_thread_control_capabilities so that
the core knows schedlocking works for this target.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ie762d3768fd70e4ac398c8bcc03c3213bfa26a6a
|
|
This rewrites win32_process_target::resume such that scheduler-locking
is implemented properly.
It also uses the new get_last_debug_event_ptid function to avoid
considering passing a signal to the wrong thread, like done for the
native side in a previous patch.
Note this code/comment being removed:
- /* Yes, we're ignoring resume_info[0].thread. It'd be tricky to make
- the Windows resume code do the right thing for thread switching. */
- tid = windows_process.current_event.dwThreadId;
This meant that scheduler-locking currently is broken badly unless you
stay in the thread that last reported an event. If you switch to a
different thread from the one that last reported an event and step,
you get a spurious SIGTRAP in the thread that last reported a stop,
not the one that you tried to step:
(gdb) t 1
[Switching to thread 1 (Thread 3908)]
#0 0x00007fffc768d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) set scheduler-locking on
(gdb) set disassemble-next-line on
(gdb) frame
#0 0x00007fffc768d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
=> 0x00007fffc768d6e4 <ntdll!ZwDelayExecution+20>: c3 ret
(gdb) si
Thread 3 received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 2660]
0x00007fffc4e4e92e in KERNELBASE!EncodeRemotePointer () from target:C:/Windows/System32/KernelBase.dll
=> 0x00007fffc4e4e92e <KERNELBASE!EncodeRemotePointer+8254>: eb 78 jmp 0x7fffc4e4e9a8 <KERNELBASE!EncodeRemotePointer+8376>
(gdb)
Note how we switched to thread 1, stepped, and GDBserver still stepped
thread 3... This is fixed by this patch. We now get:
(gdb) info threads
Id Target Id Frame
1 Thread 920 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
2 Thread 8528 0x00007ffe03730ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from target:C:/Windows/SYSTEM32/ntdll.dll
3 Thread 3128 0x00007ffe03730ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from target:C:/Windows/SYSTEM32/ntdll.dll
* 4 Thread 7164 0x00007ffe0102e929 in KERNELBASE!EncodeRemotePointer () from target:C:/Windows/System32/KernelBase.dll
5 Thread 8348 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
6 Thread 2064 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) t 1
[Switching to thread 1 (Thread 920)]
#0 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) set scheduler-locking on
(gdb) si
0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) si
0x00007ffe00f9b44e in SleepEx () from target:C:/Windows/System32/KernelBase.dll
(gdb) si
0x00007ffe00f9b453 in SleepEx () from target:C:/Windows/System32/KernelBase.dll
I.e., we kept stepping the right thread, thread 1.
Note we stopped again at 0x00007ffe0372d6e4 the first time (same PC
the thread already was at before the first stepi) because the thread
had been stopped at a syscall, so that's normal:
(gdb) disassemble
Dump of assembler code for function ntdll!ZwDelayExecution:
0x00007ffe0372d6d0 <+0>: mov %rcx,%r10
0x00007ffe0372d6d3 <+3>: mov $0x34,%eax
0x00007ffe0372d6d8 <+8>: testb $0x1,0x7ffe0308
0x00007ffe0372d6e0 <+16>: jne 0x7ffe0372d6e5 <ntdll!ZwDelayExecution+21>
0x00007ffe0372d6e2 <+18>: syscall
=> 0x00007ffe0372d6e4 <+20>: ret
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I44f4fe4cb98592517569c6716b9d189f42db25a0
commit-id:2a7b7d8e
|
|
Passing a signal to a thread other than the one that last reported an
event will be later possible with DBG_REPLY_LATER and the Windows
backend working in non-stop mode.
With an all-stop backend that isn't possible, so at least don't
incorrectly consider passing DBG_EXCEPTION_NOT_HANDLED if the thread
that we're going to call ContinueDebugEvent for is not the one that
the user issued "signal SIG" on.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I27092ecfbf0904ebce02dff07d9104d22f3d8f0e
commit-id:30c8d0ce
|
|
This will be used in subsequent patches to avoid using
DBG_EXCEPTION_NOT_HANDLED on the wrong thread.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I32915623b5036fb902f9830ce2d6f0b1ccf1f5cf
commit-id:1cf51152
|
|
windows_process.desired_stop_thread_id doesn't work for non-stop, as
in that case every thread will have its own independent
WaitForDebugEvent event.
Instead, detect whether we have been reported a stop that was not
supposed to be reported by simply checking whether the thread that is
reporting the event is suspended. This is now easilly possible since
each thread's suspend state is kept in sync with whether infrun wants
the thread executing or not.
windows_process.desired_stop_thread_id was also used as thread to pass
to windows_continue. However, we don't really need that either.
windows_continue is used to update the thread's registers, unsuspend
them, and then finally call ContinueDebugEvent. In most cases, we
only need the ContinueDebugEvent step, so we can convert the
windows_continue calls to continue_last_debug_event_main_thread calls.
The exception is when we see a thread creation event -- in that case,
we need to update the debug registers of the new thread. We can use
continue_one_thread for that.
Since the pending stop is now stored in windows_thread_info,
get_windows_debug_event needs to avoid reaching the bottom code if
there's no thread associated with the event anymore (i.e.,
EXIT_THREAD_DEBUG_EVENT / EXIT_PROCESS_DEBUG_EVENT).
I considered whether it would be possible to keep the pending_stop
handling code shared in gdb/nat/windows-nat.c, in this patch and
throughout the series, but I conclused that it isn't worth it, until
gdbserver is taught about async and non-stop as well.
The pending_stop struct will eventually be eliminated later down the
series.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ib7c8e8d16edc0900b7c411976c5d70cf93931c1c
commit-id:0b93b3f0
|
|
I noticed that windows_nat_target::get_windows_debug_event does not
copy the event recorded in pending stop to
windows_process.current_event. This seems like an oversight. The
equivalent code in gdbserver/win32-low.cc does copy it.
This change will become moot later in the series, but I figure its
still clearer to correct the buglet as preparatory patch.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ic8935d854cf67a3a3c4edcbc1a1e8957b800d907
|
|
This factors some code out of windows_nat_target::windows_continue
into a new windows_continue_one function. This will make the
following patch easier to read (as well as the resulting code itself).
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I14a0386b1b8b03015e86273060af173b5130e375
|
|
windows_continue already has two boolean parameters:
(..., int killed, bool last_call = false)
A patch later in the series would need a third. Instead, convert
windows_continue to use an optional enum-flags parameter instead of
multiple booleans.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I17c4d8a12b662190f972c380f838cb3317bd2e1e
commit-id:e669e7de
|
|
We have code using do_synchronously to call continue_last_debug_event,
and later patches in the series would need to add the same code in few
more places. Factor it out to a continue_last_debug_event_main_thread
function so these other places in future patches can just call it.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I945e668d2b3daeb9de968219925a7b3c7c7ce9ed
commit-id:ee04461a
|
|
The current code suspends a thread just before calling
GetThreadContext. You can only call GetThreadContext if the thread is
suspended. But, after WaitForDebugEvent, all threads are implicitly
suspended. So I don't think we even needed to call SuspendThread
explictly at all before our GetThreadContext calls.
However, suspending threads when we're about to present a stop to gdb
simplifies adding non-stop support later. This way, the windows
SuspendThread state corresponds to whether a thread is suspended or
resumed from the core's perspective. Curiously, I noticed that Wine's
winedbg does something similar:
https://github.com/wine-mirror/wine/blob/234943344f7495d1e072338f0e06fa2d5cbf0aa1/programs/winedbg/gdbproxy.c#L651
This makes it much easier to reason about a thread's suspend state,
and simplifies adding non-stop mode later on.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ifd6889a8afc041fad33cd1c4500e38941da6781b
commit-id:c4d2c92e
|
|
The logic in windows_nat_target::wait, where we decide what to do
depending on the result from get_windows_debug_event is harder to
grasp than it looks.
It is not easy to tell what should happen when in async mode
get_windows_debug_event returns that there's no event to process.
And then, if get_windows_debug_event returns null_ptid /
TARGET_WAITKIND_SPURIOUS, then we need to issue a ContinueDebugEvent.
There's also this comment in windows_nat_target::wait, which we're not
really implementing today:
~~~~
/* We loop when we get a non-standard exception rather than return
with a SPURIOUS because resume can try and step or modify things,
which needs a current_thread->h. But some of these exceptions mark
the birth or death of threads, which mean that the current thread
isn't necessarily what you think it is. */
~~~~
This patch changes things a bit so that the code is more obvious:
- look at the status kind, instead of ptid_t.
- add an explicit early return case for no-event.
- add an explicit case for TARGET_WAITKIND_SPURIOUS.
- with those, we no longer need to handle the case of find_thread not
finding a thread, so we can drop one indentation level.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I76c41762e1f893a7ff23465856ccf6a44af1f0e7
commit-id:aff7fc4a
|
|
After the previous patches, thread_rec is no longer called anywhere.
Delete it.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ib14e5807fc427e1c3c4a393a9ea7b36b6047a2d7
|
|
There's a single call to thread_rec(DONT_SUSPEND), in
windows_process_info::handle_exception.
In GDB, the windows-nat.c thread_rec implementation avoids actually
calling SuspendThread on the event thread by doing:
th->suspended = -1;
I am not exactly sure why, but it kind of looks like it is done as an
optimization, avoiding a SuspendThread call? It is probably done for
the same reason as the code touched in the previous patch avoided
suspending the event thread.
This however gets in the way of non-stop mode, which will really want
to SuspendThread the event thread for DBG_REPLY_LATER.
In gdbserver's thread_rec implementation DONT_SUSPEND is ignored, and
thread_rec actually always suspends, which really suggests that
SuspendThread on the event thread is really not a problem. I really
can't imagine why it would be.
DONT_SUSPEND invalidates the thread's context, but there is no need to
invalidate the context when we get an event for a thread, because we
invalidate it when we previously resumed the thread.
So, we can just remove the thread_rec call from
windows_process_info::handle_exception. That's what this patch does.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I0f328542bda6d8268814ca1ee4ae7a478098ecf2
|
|
Replace thread_rec(INVALIDATE_CONTEXT) calls with find_thread, and
invalidate_context / suspend calls in the spots that might need those.
I don't know why does the INVALIDATE_CONTEXT implementation in GDB
avoid suspending the event thread:
case INVALIDATE_CONTEXT:
if (ptid.lwp () != current_event.dwThreadId)
th->suspend ();
Checks for a global "current_event" get in the way of non-stop support
later in the series, as each thread will have its own "last debug
event". Regardless, it should be fine to suspend the event thread.
As a data point, the GDBserver implementation always suspends. So
this patch does not try to avoid suspending the event thread on the
native side either.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I8d2f0a749d23329956e62362a7007189902dddb5
|
|
We don't need reload_context, because we can get the same information
out of th->context.ContextFlags. If ContextFlags is zero, then we
need to fetch the context out of the inferior thread. This is what
gdbserver does too.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ied566037c81383414c46c77713bdd1aec6377b23
|
|
handle_output_debug_string returns a Windows thread id, so it should
return a DWORD instead of an int.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Icbd071a1a37de8a0fc8918bd13254a8d40311e32
|
|
thread_rec(DONT_INVALIDATE_CONTEXT)
The goal of the next few patches is to eliminate thread_rec
completely. This is the first patch in that effort.
thread_rec(DONT_INVALIDATE_CONTEXT) is really just a thread lookup
with no side effects, so this adds a find_thread function that lets
you do that.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ie486badce00e234b10caa478b066c34537103e3f
|
