This implements target async for Windows. The basic idea is to have
the worker thread block in WaitForDebugEvent, then notify the event
loop when an event is seen. In a few situations, this blocking
behavior is undesirable, so the functions passed to do_synchronously
are changed to return a boolean indicating which behavior is needed.
On Windows, certain debugging APIs can only be called from the thread
that started (or attached) to the inferior. Also, there is no way on
Windows to wait for a debug event in addition to other events.
Therefore, in order to implement target async for Windows, gdb will
have to call some functions in a worker thread.
This patch implements the worker thread and moves the necessary
operations there. Target async isn't yet implemented, so this patch
does not cause any visible changes.
When working on windows-nat.c, it's useful to see an error message in
addition to the error number given by GetLastError. This patch moves
strwinerror from gdbserver to gdbsupport, and then updates
windows-nat.c to use it. A couple of minor changes to strwinerror
(constify the return type and use the ARRAY_SIZE macro) are also
included.
windows_nat_target::detach has a variable 'detached' that is only set
after a call to 'error'. However, this can't happen because 'error'
throws an exception.
This patch removes the dead code.
I built GDB for all targets on a x86-64/GNU-Linux system, and
then (accidentally) passed GDB a RISC-V binary, and asked GDB to "run"
the binary on the native target. I got this error:
(gdb) show architecture
The target architecture is set to "auto" (currently "i386").
(gdb) file /tmp/hello.rv32.exe
Reading symbols from /tmp/hello.rv32.exe...
(gdb) show architecture
The target architecture is set to "auto" (currently "riscv:rv32").
(gdb) run
Starting program: /tmp/hello.rv32.exe
../../src/gdb/i387-tdep.c:596: internal-error: i387_supply_fxsave: Assertion `tdep->st0_regnum >= I386_ST0_REGNUM' failed.
What's going on here is this; initially the architecture is i386, this
is based on the default architecture, which is set based on the native
target. After loading the RISC-V executable the architecture of the
current inferior is updated based on the architecture of the
executable.
When we "run", GDB does a fork & exec, with the inferior being
controlled through ptrace. GDB sees an initial stop from the inferior
as soon as the inferior comes to life. In response to this stop GDB
ends up calling save_stop_reason (linux-nat.c), which ends up trying
to read register from the inferior, to do this we end up calling
target_ops::fetch_registers, which, for the x86-64 native target,
calls amd64_linux_nat_target::fetch_registers.
After this I eventually end up in i387_supply_fxsave, different x86
based targets will end in different functions to fetch registers, but
it doesn't really matter which function we end up in, the problem is
this line, which is repeated in many places:
i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (arch);
The problem here is that the ARCH in this line comes from the current
inferior, which, as we discussed above, will be a RISC-V gdbarch, the
tdep field will actually be of type riscv_gdbarch_tdep, not
i386_gdbarch_tdep. After this cast we are relying on undefined
behaviour, in my case I happen to trigger an assert, but this might
not always be the case.
The thing I tried that exposed this problem was of course, trying to
start an executable of the wrong architecture on a native target. I
don't think that the correct solution for this problem is to detect,
at the point of cast, that the gdbarch_tdep object is of the wrong
type, but, I did wonder, is there a way that we could protect
ourselves from incorrectly casting the gdbarch_tdep object?
I think that there is something we can do here, and this commit is the
first step in that direction, though no actual check is added by this
commit.
This commit can be split into two parts:
(1) In gdbarch.h and arch-utils.c. In these files I have modified
gdbarch_tdep (the function) so that it now takes a template argument,
like this:
template<typename TDepType>
static inline TDepType *
gdbarch_tdep (struct gdbarch *gdbarch)
{
struct gdbarch_tdep *tdep = gdbarch_tdep_1 (gdbarch);
return static_cast<TDepType *> (tdep);
}
After this change we are no better protected, but the cast is now
done within the gdbarch_tdep function rather than at the call sites,
this leads to the second, much larger change in this commit,
(2) Everywhere gdbarch_tdep is called, we make changes like this:
- i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (arch);
+ i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (arch);
There should be no functional change after this commit.
In the next commit I will build on this change to add an assertion in
gdbarch_tdep that checks we are casting to the correct type.
Prior to c6ca3dab dropping support for Cygwin 1.5, __USEWIDE was not
defined for Cygwin 1.5. After that, it's always defined if __CYGWIN__
is, so remove __USEWIDE conditionals inside __CYGWIN__ conditionals.
This changes windows_process_info to use virtual methods for its
callbacks, and then changes the two clients of this code to subclass
this class to implement the methods.
I considered using CRTP here, but that would require making the new
structures visible to the compilation of of nat/windows-nat.c. This
seemed like a bit of a pain, so I didn't do it.
This change then lets us change all the per-inferior globals to be
members of the new subclass. Note that there can still only be a
single inferior -- currently there's a single global of the new type.
This is just another step toward possibly implementing multi-inferior
for Windows.
It's possible this could be cleaned up further... ideally I'd like to
move more of the data into the base class. However, because gdb
supports Cygwin and gdbserver does not, and because I don't have a way
to build or test Cygwin, larger refactorings are difficult.
This patch turns some windows-nat.c static functions into methods on
windows_nat_target. This avoids having to reference the
windows_nat_target singleton in some more spots -- a minor code
cleanup.
On Windows, it is possible to disable ASLR when creating a process.
This patch adds code to do this, and hooks it up to gdb's existing
disable-randomization feature. Because the Windows documentation
cautions that this isn't available on all versions of Windows, the
CreateProcess wrapper function is updated to make the attempt, and
then fall back to the current approach if it fails.
Fix Cygwin build after 0578e87f ("Remove some globals from
nat/windows-nat.c"). Update code under ifdef __CYGWIN__ for globals
moved to members of struct windows_process_info.
Fix Cygwin build after fcab5839 ("Implement pid_to_exec_file for Windows
in gdbserver"). That change moves code from gdb/windows-nat.c to
gdb/nat/windows-nat.c, but doesn't add the required typedefs and
includes for parts of that code under ifdef __CYGWIN__.
I noticed that gdbserver did not implement pid_to_exec_file for
Windows, while gdb did implement it. This patch moves the code to
nat/windows-nat.c, so that it can be shared. This makes the gdbserver
implementation trivial.
This changes target_pid_to_exec_file and target_ops::pid_to_exec_file
to return a "const char *". I couldn't build many of these targets,
but did examine the code by hand -- also, as this only affects the
return type, it's normally pretty safe. This brings gdb and gdbserver
a bit closer, and allows for the removal of a const_cast as well.
Windows 10 introduced SetThreadDescription and GetThreadDescription, a
simpler way to set a thread's name. This changes gdb and gdbserver to
use this convention when it is available.
This is part of PR win32/29050.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29050
Currently, gdb's native Windows target implements the exception-based
approach for setting thread names, but gdbserver does not. This patch
moves handle_ms_vc_exception to the shared nat/windows-nat.c code, as
preparation for adding this support to gdbserver.
I don't think there's any need to call init_thread_list in
windows-nat.c. This patch removes it. I tested this using the
internal AdaCore test suite on Windows, which FWIW does include some
multi-threaded inferiors.
nat/windows-nat.c has a number of globals that it uses to communicate
with its clients (gdb and gdbserver). However, if we ever want the
Windows ports to be multi-inferior, globals won't work.
This patch takes a step toward that by moving most nat/windows-nat.c
globals into a new struct windows_process_info. Many functions are
converted to be methods on this object.
A couple of globals remain, as they are needed to truly be global due
to the way that the Windows debugging APIs work.
The clients still have a global for the current process. That is,
this patch is a step toward the end goal, but doesn't implement the
goal itself.
windows-nat.c uses some manual memory management when manipulating the
thread_list global. Changing this to use unique_ptr simplifies the
code, in particular windows_init_thread_list. (Note that, while I
think the the call to init_thread_list in there is wrong, I haven't
removed it in this patch.)
Currently windows-nat.c uses struct so_list to record its local idea
of which shared libraries have been loaded. However, many fields in
this are not needed, and furthermore I found this quite confusing at
first -- Windows actually uses solib-target and so the use of so_list
here is weird.
This patch simplifies this code by changing it to use a std::vector
and a new type that holds exactly what's needed for the Windows code.
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
Now that filtered and unfiltered output can be treated identically, we
can unify the puts family of functions. This is done under the name
"gdb_puts". Most of this patch was written by script.
A number of spots call printf_unfiltered only because they are in code
that should not be interrupted by the pager. However, I believe these
cases are all handled by infrun's blanket ban on paging, and so can be
converted to the default (_filtered) API.
After this patch, I think all the remaining _unfiltered calls are ones
that really ought to be. A few -- namely in complete_command -- could
be replaced by a scoped assignment to pagination_enabled, but for the
remainder, the code seems simple enough like this.
target_announce_detach was added in commit 0f48b757 ("Factor out
"Detaching from program" message printing"). There, Pedro wrote:
(For now, I left the couple targets that print this a bit differently
alone. Maybe this could be further pulled out into infcmd.c. If we
did that, and those targets want to continue printing differently,
this new function could be converted to a target method.)
It seems to me that the differences aren't very big, and in some cases
other targets handled the output a bit more nicely. In particular,
some targets will print a different message when exec_file==NULL,
rather than printing the same output with an empty string as
exec_file.
This patch incorporates the nicer output into target_announce_detach,
then changes the remaining ports to use this function.
This introduces target_announce_attach, by analog with
target_announce_detach. Then it converts existing targets to use
this, rather than emitting their own output by hand.
This changes the implementations of the target files_info method to
use filtered output. This makes sense because the sole caller of this
method is an ordinary command (info_program_command). This patch
changes this command to use filtered output as well.
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
Commit 345bd07cce ("gdb: fix gdbarch_tdep ODR violation") forgot to
update the gdbarch_tdep calls in the native files other than x86-64
Linux. This patch updates them all (to the best of my knowledge).
These are the files I was able to build-test:
aarch64-linux-nat.c
amd64-bsd-nat.c
arm-linux-nat.c
ppc-linux-nat.c
windows-nat.c
xtensa-linux-nat.c
And these are the ones I could not build-test:
aix-thread.c
arm-netbsd-nat.c
ppc-fbsd-nat.c
ppc-netbsd-nat.c
ia64-tdep.c (the part that needs libunwind)
ppc-obsd-nat.c
rs6000-nat.c
If there are still some build problems related to gdbarch_tdep in them,
they should be pretty obvious to fix.
Change-Id: Iaa3d791a850e4432973757598e634e3da6061428
A recent internal change pointed out that watchpoints were not working
on Windows when the inferior was multi-threaded. This happened
because the debug registers were only updated for certain threads --
in particular, those that were being resumed and that were not marked
as suspended. In the case of single-stepping, the need to update the
debug registers in other threads could also be "forgotten".
This patch changes windows-nat.c to mark all threads needing a debug
register update. This brings the code closer to what gdbserver does
(though, unfortunately, it still seems more complicated than needed).
I stumbled on a bug caused by the fact that a code path read
target_waitstatus::value::sig (expecting it to contain a gdb_signal
value) while target_waitstatus::kind was TARGET_WAITKIND_FORKED. This
meant that the active union field was in fact
target_waitstatus::value::related_pid, and contained a ptid. The read
signal value was therefore garbage, and that caused GDB to crash soon
after. Or, since that GDB was built with ubsan, this nice error
message:
/home/simark/src/binutils-gdb/gdb/linux-nat.c:1271:12: runtime error: load of value 2686365, which is not a valid value for type 'gdb_signal'
Despite being a large-ish change, I think it would be nice to make
target_waitstatus safe against that kind of bug. As already done
elsewhere (e.g. dynamic_prop), validate that the type of value read from
the union matches what is supposed to be the active field.
- Make the kind and value of target_waitstatus private.
- Make the kind initialized to TARGET_WAITKIND_IGNORE on
target_waitstatus construction. This is what most users appear to do
explicitly.
- Add setters, one for each kind. Each setter takes as a parameter the
data associated to that kind, if any. This makes it impossible to
forget to attach the associated data.
- Add getters, one for each associated data type. Each getter
validates that the data type fetched by the user matches the wait
status kind.
- Change "integer" to "exit_status", "related_pid" to "child_ptid",
just because that's more precise terminology.
- Fix all users.
That last point is semi-mechanical. There are a lot of obvious changes,
but some less obvious ones. For example, it's not possible to set the
kind at some point and the associated data later, as some users did.
But in any case, the intent of the code should not change in this patch.
This was tested on x86-64 Linux (unix, native-gdbserver and
native-extended-gdbserver boards). It was built-tested on x86-64
FreeBSD, NetBSD, MinGW and macOS. The rest of the changes to native
files was done as a best effort. If I forgot any place to update in
these files, it should be easy to fix (unless the change happens to
reveal an actual bug).
Change-Id: I0ae967df1ff6e28de78abbe3ac9b4b2ff4ad03b7
get_ada_task_ptid currently takes a 'long' as its 'thread' parameter
type. However, on some platforms this is actually a pointer, and
using 'long' can sometimes end up with the value being sign-extended.
This sign extension can cause problems later, if the tid is then later
used as an address again.
This patch changes the parameter type to ULONGEST and updates all the
uses. This approach preserves sign extension on the targets where it
is apparently intended, while avoiding it on others.
Co-Authored-By: John Baldwin <jhb@FreeBSD.org>
Add cwd/set_cwd to the inferior class, remove set_inferior_args.
Keep get_inferior_args, because it is used from fork_inferior, in shared
code. The cwd could eventually be passed as a parameter eventually,
though, I think that would be cleaner.
Change-Id: Ifb72ea865d7e6f9a491308f0d5c1595579d8427e
I spotted some indentation issues where we had some spaces followed by
tabs at beginning of line, that I wanted to fix. So while at it, I did
a quick grep to find and fix all I could find.
gdb/ChangeLog:
* Fix tab after space indentation issues throughout.
Change-Id: I1acb414dd9c593b474ae2b8667496584df4316fd
This moves the new DLL-loading code into nat/windows-nat.c, and
changes both gdb and gdbserver to use the shared code. One
client-provided callback, handle_load_dll, is changed to allow the
code to be shared. This callback was actually never called from
nat/windows-nat.c; maybe I had planned to share more here and then
didn't finish... I'm not sure.
gdb/ChangeLog
2021-04-30 Tom Tromey <tromey@adacore.com>
* windows-nat.c (windows_nat::handle_load_dll): Update.
(windows_nat_target::get_windows_debug_event): Call
dll_loaded_event.
(windows_add_all_dlls, windows_add_dll): Move to
nat/windows-nat.c.
* nat/windows-nat.h (handle_load_dll): Change parameters.
(dll_loaded_event, windows_add_all_dlls): Declare.
* nat/windows-nat.c (windows_add_dll, windows_add_all_dlls): Move
from windows-nat.c.
(dll_loaded_event): New function.
gdbserver/ChangeLog
2021-04-30 Tom Tromey <tromey@adacore.com>
* win32-low.cc (do_initial_child_stuff): Update.
(windows_nat::handle_load_dll): Rename from win32_add_one_solib.
Change parameter type.
(win32_add_dll, win32_add_all_dlls)
(windows_nat::handle_load_dll): Remove.
(get_child_debug_event): Call dll_loaded_event.
gdb and gdbserver both look for functions in some Windows DLLs at
runtime. This patch moves this code out of gdb and into
nat/windows-nat, so it can be shared by both programs.
gdb/ChangeLog
2021-04-30 Tom Tromey <tromey@adacore.com>
* windows-nat.c: Move code to nat/windows-nat.[ch].
(_initialize_windows_nat): Call initialize_loadable.
* nat/windows-nat.h (AdjustTokenPrivileges)
(DebugActiveProcessStop, DebugBreakProcess)
(DebugSetProcessKillOnExit, EnumProcessModules)
(EnumProcessModulesEx, GetModuleInformation)
(GetModuleFileNameExA, GetModuleFileNameExW)
(LookupPrivilegeValueA, OpenProcessToken, GetConsoleFontSize)
(GetCurrentConsoleFont, Wow64SuspendThread)
(Wow64GetThreadContext, Wow64SetThreadContext)
(Wow64GetThreadSelectorEntry): Move from windows-nat.c.
(AdjustTokenPrivileges_ftype)
(DebugActiveProcessStop_ftype, DebugBreakProcess_ftype)
(DebugSetProcessKillOnExit_ftype, EnumProcessModules_ftype)
(EnumProcessModulesEx_ftype, GetModuleInformation_ftype)
(GetModuleFileNameExA_ftype, GetModuleFileNameExW_ftype)
(LookupPrivilegeValueA_ftype, OpenProcessToken_ftype)
(GetConsoleFontSize_ftype)
(GetCurrentConsoleFont_ftype, Wow64SuspendThread_ftype)
(Wow64GetThreadContext_ftype, Wow64SetThreadContext_ftype)
(Wow64GetThreadSelectorEntry_ftype): Likewise.
(initialize_loadable): Declare.
* nat/windows-nat.c (AdjustTokenPrivileges)
(DebugActiveProcessStop, DebugBreakProcess)
(DebugSetProcessKillOnExit, EnumProcessModules)
(EnumProcessModulesEx, GetModuleInformation, GetModuleFileNameExA)
(GetModuleFileNameExW, LookupPrivilegeValueA, OpenProcessToken)
(GetCurrentConsoleFont, GetConsoleFontSize, Wow64SuspendThread)
(Wow64GetThreadContext, Wow64SetThreadContext)
(Wow64GetThreadSelectorEntry): Define.
(bad, bad_GetCurrentConsoleFont, bad_GetConsoleFontSize): Move
from windows-nat.c.
(initialize_loadable): Likewise, and rename.
windows-nat.c defines a number of replacement functions that simply
return zero. This patch removes these in favor of a couple of
template functions.
gdb/ChangeLog
2021-04-30 Tom Tromey <tromey@adacore.com>
* windows-nat.c (bad_GetModuleFileNameEx): Remove define.
(bad_DebugActiveProcessStop, bad_DebugBreakProcess)
(bad_DebugSetProcessKillOnExit, bad_EnumProcessModules)
(bad_GetModuleFileNameExW, bad_GetModuleFileNameExA)
(bad_GetModuleInformation, bad_OpenProcessToken): Remove.
(bad): New template functions.
(_initialize_loadable): Update.
This patch makes handling a DLL load at run time (using LoadLibrary)
much more reliable when its file name cannot be obtained using the
lpImageName pointer provided by the DLL load debug event. The
solution is to enumerate all the DLLs loaded by the inferior, looking
for the DLL that's loaded at base address provided by the lpBaseOfDll
pointer of the debug event. Correctly resolving the DLL file name is
important, because without that GDB doesn't record the DLL in the list
of solibs, and then later is unable to show functions in that DLL in
the backtraces, which produces corrupted and truncated backtraces.
See this thread for the problems that causes:
https://sourceware.org/pipermail/gdb-patches/2021-March/177022.html
gdb/ChangeLog:
2021-04-10 Eli Zaretskii <eliz@gnu.org>
* windows-nat.c (windows_nat::handle_load_dll): Call
windows_add_dll if get_image_name failed to glean the name of the
DLL by using the lpImageName pointer.
(windows_add_all_dlls): Now a thin wrapper around windows_add_dll.
(windows_add_dll): Now does what windows_add_all_dlls did before,
but also accepts an argument LOAD_ADDR, which, if non-NULL,
specifies the address where the DLL was loaded into the inferior,
and looks for the single DLL loaded at that address.
Same principle as the previous patches.
gdb/ChangeLog:
* target.h (target_is_pushed): Remove, update callers to use
inferior::target_is_pushed instead.
* target.c (target_is_pushed): Remove.
Change-Id: I9862e6205acc65672da807cbe4b46cde009e7b9d
Same as the previous patch, but for the push_target functions.
The implementation of the move variant is moved to a new overload of
inferior::push_target.
gdb/ChangeLog:
* target.h (push_target): Remove, update callers to use
inferior::push_target.
* target.c (push_target): Remove.
* inferior.h (class inferior) <push_target>: New overload.
Change-Id: I5a95496666278b8f3965e5e8aecb76f54a97c185
With "gcc version 10.2.0 (GCC)" on cygwin, I get this build error:
CXX windows-nat.o
In file included from ../../gdb/../gdbsupport/common-defs.h:129,
from ../../gdb/defs.h:28,
from ../../gdb/windows-nat.c:24:
../../gdb/windows-nat.c: In function 'void windows_init_thread_list()':
../../gdb/windows-nat.c:513:17: error: zero-length gnu_printf format string [-Werror=format-zero-length]
513 | DEBUG_EVENTS ("");
| ^~
../../gdb/../gdbsupport/common-debug.h:65:43: note: in definition of macro 'debug_prefixed_printf_cond'
65 | debug_prefixed_printf (module, __func__, fmt, ##__VA_ARGS__); \
| ^~~
../../gdb/windows-nat.c:513:3: note: in expansion of macro 'DEBUG_EVENTS'
513 | DEBUG_EVENTS ("");
| ^~~~~~~~~~~~
cc1plus: all warnings being treated as errors
This was introduced in 4ef367bffd, which removed
the function name from this debug message:
- DEBUG_EVENTS (("gdb: windows_init_thread_list\n"));
+ DEBUG_EVENTS ("");
DEBUG_EVENTS now always includes the function name, so just add a "called"
message to fix the compile error.
gdb/ChangeLog:
2021-03-16 Christian Biesinger <cbiesinger@google.com>
* windows-nat.c (windows_init_thread_list): Add message to
debug log.
