Currently, on native Windows, GDB presents a mix of backslashes and
forward slashes to users, like e.g.:
...
attach 15044
Attaching to program: C:\msys2\home\alves\gdb\build-testsuite\outputs\gdb.base\attach\attach.exe, process 15044
...
[Switching to thread 1 (Thread 15044.0x1294)]
main () at C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:19
19 while (! should_exit)
(gdb) FAIL: gdb.base/attach.exp: do_attach_failure_tests: first attach
...
Note how above, the "attach" command uses backslashes, and the source
path uses forward slashes.
Most annoyingly, depending on compiler, you can end up with mixed
slash styles in the same path:
Temporary breakpoint 1 at 0x1400010ec: file C:/rocgdb/src/gdb/testsuite/gdb.rocm\simple.cpp, line 33.
^
A while ago, in commit a63213cd37 ("MSYS2+MinGW testing: Unix <->
Windows path conversion"), I had made the testsuite normalize
backslashes to forward slashes, among other things. I had said in the
commit log there:
"It's arguable whether GDB itself should do this sanitization. I
suspect it should. I personally dislike seeing backward slashes in
e.g., "info shared" output, or worse, mixed backward and forward
slashes. Still, I propose starting with a testsuite adjustment that
moves us forward, and handle that separately. I won't be surprised if
we need the new routine for some cases even if we adjust GDB."
After running into some other tests that would need backslash
adjustment, and also because this is something user-visible, not just
a testsuite issue, I decided to bite the bullet and make GDB itself do
the slashes normalization.
This patch fixes all the cases of backslashes that I could find:
- source files and compilation directory
- executable name, and shared library names
- cd/pwd commands
It does this by normalizing slashes at some strategic places. The
conversion is only attempted if needed -- i.e., if debugging on
Windows, or cross-debugging Windows programs from a non-Windows host.
For executable/shared library names, this is doing the backslash ->
forward slash conversion completely on the host side in common code,
without touching the target backend files in either GDB or GDBserver,
by design, so that this is all completely a client side decision, and
works the same against any Windows GDBserver version, against remote
servers other than ours (e.g., Wine's), so we can change it later if
we want, etc.
I did tweak the gdb and gdbserver backends for the other way around
(forward slash => backslash), to make sure that they pass backslashes
to CreateProcess, so that inferiors see native backslashes in their
own argv[0]. For GDBserver, it would already have been possible to
launch an executable with forward slashes before (e.g., when debugging
from a Linux or Cygwin client), so this makes Windows gdbserver
behavior more consistent, independently of client, too.
I did not add an option to make this configurable at runtime on
purpose. I don't see the point, it'd just add more complication and
testing surface. Always normalizing keeps it simple.
For the source paths, I considered doing the normalization earlier in
the DWARF reader, but decided against it, because that would force
extra copying of strings -- the DWARF reader works mostly by passing
around const char pointers. Also, the places I picked in principle
will automatically handle PDB if/when we get to it.
The buildsym.c change takes care of compilation directory paths.
The allocate_symtab change takes care of source paths. That one
required a (temporary) copy, but I think that that's acceptable there.
The gdb_realpath change is needed because just before the touched code
there's a GetFullPathName call, which itself normalizes to
backslashes.
The exec.c and solib.c changes take care of executable and shared
libraries.
The change to testsuite/lib/gdb.exp, to adjust to forward-slash style
for DOS full names, are necessary otherwise MI tests break. E.g.:
*stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x00007ff780461508",
func="main",args=[],file="C:/rocgdb/src/gdb/testsuite/gdb.mi/basics.c",
fullname="C:/rocgdb/src/gdb/testsuite/gdb.mi/basics.c",line="64",arch="i386:x86-64"},
thread-id="1",stopped-threads="all"
FAIL: gdb.mi/gdb2549.exp: mi runto main (unknown output after running)
gdb.base/set-cwd.exp was the only test I found (so far at least) that
required adjustment, but note that without this patch that testcase
fails and throws TCL errors, like:
...
FAIL: gdb.base/set-cwd.exp: test_cwd_reset: inferior cwd is correctly set
ERROR: couldn't compile regular expression pattern: invalid escape \ sequence
...
It passes cleanly with the patch.
Below's the before / after comparison of different commands / output.
Before:
Reading symbols from ./outputs/gdb.rocm/simple/simple...
(gdb)
start
Temporary breakpoint 1 at 0x1400010ec: file C:/rocgdb/src/gdb/testsuite/gdb.rocm\simple.cpp, line 33.
Starting program: C:\msys2\home\alves\gdb\build-testsuite\outputs\gdb.rocm\simple\simple
Thread 1 hit Temporary breakpoint 1, main () at C:/rocgdb/src/gdb/testsuite/gdb.rocm\simple.cpp:33
33 hipError_t error = hipMalloc (&result_ptr, sizeof (int));
Thread 1 hit Temporary breakpoint 1, main () at C:/rocgdb/src/gdb/testsuite/gdb.rocm\simple.cpp:33
33 hipError_t error = hipMalloc (&result_ptr, sizeof (int));
(gdb)
(gdb) info inferiors
Num Description Connection Executable
* 1 process 15620 1 (native) C:\msys2\home\alves\gdb\build-testsuite\outputs\gdb.rocm\simple\simple
(gdb) info sharedlibrary
From To Syms Read Shared Object Library
0x00007ffd22f91000 0x00007ffd231a69a8 Yes (*) C:\Windows\SYSTEM32\ntdll.dll
0x00007ffd21af1000 0x00007ffd21bb2520 Yes (*) C:\Windows\System32\kernel32.dll
0x00007ffd20091000 0x00007ffd20432548 Yes (*) C:\Windows\System32\KernelBase.dll
0x00007ffcc89c1000 0x00007ffcc9b145bc Yes (*) C:\Windows\SYSTEM32\amdhip64_7.dll
0x00007ffd22911000 0x00007ffd22d829b0 Yes (*) C:\Windows\System32\setupapi.dll
0x00007ffd216c1000 0x00007ffd217653f0 Yes (*) C:\Windows\System32\msvcrt.dll
0x00007ffd22431000 0x00007ffd224de5c8 Yes (*) C:\Windows\System32\advapi32.dll
0x00007ffd21a41000 0x00007ffd21ae52b8 Yes (*) C:\Windows\System32\sechost.dll
0x00007ffd20651000 0x00007ffd20676af0 Yes (*) C:\Windows\System32\bcrypt.dll
0x00007ffd21f31000 0x00007ffd22045860 Yes (*) C:\Windows\System32\rpcrt4.dll
0x00007ffd22d91000 0x00007ffd22f40e50 Yes (*) C:\Windows\System32\user32.dll
0x00007ffd20781000 0x00007ffd207a43d8 Yes (*) C:\Windows\System32\win32u.dll
0x00007ffd228d1000 0x00007ffd228f73d8 Yes (*) C:\Windows\System32\gdi32.dll
0x00007ffd207b1000 0x00007ffd208d09c8 Yes (*) C:\Windows\System32\gdi32full.dll
0x00007ffd20b91000 0x00007ffd20c28410 Yes (*) C:\Windows\System32\msvcp_win.dll
0x00007ffd20471000 0x00007ffd2057f410 Yes (*) C:\Windows\System32\ucrtbase.dll
0x00007ffd21541000 0x00007ffd215afbc8 Yes (*) C:\Windows\System32\ws2_32.dll
0x00007ffd223f1000 0x00007ffd2241fc40 Yes (*) C:\Windows\System32\imm32.dll
(gdb) info source
Current source file is C:/rocgdb/src/gdb/testsuite/gdb.rocm\simple.cpp
Compilation directory is C:\msys2\home\alves\gdb\build-testsuite
Located in C:\rocgdb\src\gdb\testsuite\gdb.rocm\simple.cpp
(gdb) info sources
C:\msys2\home\alves\gdb\build-testsuite\outputs\gdb.rocm\simple\simple:
C:\rocgdb\src\gdb\testsuite\gdb.rocm\simple.cpp, C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\ucrt\corecrt_stdio_config.h,
C:\Program Files\AMD\ROCm\7.1\include\hip\hip_runtime_api.h, C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.39.33519\include\stdint.h,
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\ucrt\stdio.h, C:\Program Files\AMD\ROCm\7.1\lib\clang\21\include\__stddef_nullptr_t.h,
C:\Program Files\AMD\ROCm\7.1\lib\clang\21\include\__stddef_size_t.h, C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.39.33519\include\cstdlib,
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\ucrt\stdlib.h, C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\ucrt\corecrt_search.h,
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\ucrt\corecrt_malloc.h,
...
(gdb) detach
Detaching from program: C:\msys2\home\alves\gdb\build-testsuite\outputs\gdb.rocm\simple\simple, process 20800
[Inferior 1 (process 20800) detached]
(gdb) cd /tmp
Working directory C:\tmp.
(gdb) pwd
Working directory C:\tmp.
After:
Reading symbols from ./outputs/gdb.rocm/simple/simple...
(gdb) start
Temporary breakpoint 1 at 0x1400010ec: file C:/rocgdb/src/gdb/testsuite/gdb.rocm/simple.cpp, line 33.
Starting program: C:/msys2/home/alves/gdb/build-testsuite/outputs/gdb.rocm/simple/simple
Thread 1 hit Temporary breakpoint 1, main () at C:/rocgdb/src/gdb/testsuite/gdb.rocm/simple.cpp:33
33 hipError_t error = hipMalloc (&result_ptr, sizeof (int));
(gdb) info sources
C:/msys2/home/alves/gdb/build-testsuite/outputs/gdb.rocm/simple/simple:
C:/rocgdb/src/gdb/testsuite/gdb.rocm/simple.cpp, C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/ucrt/corecrt_stdio_config.h,
C:/Program Files/AMD/ROCm/7.1/include/hip/hip_runtime_api.h, C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.39.33519/include/stdint.h,
C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/ucrt/stdio.h, C:/Program Files/AMD/ROCm/7.1/lib/clang/21/include/__stddef_nullptr_t.h,
C:/Program Files/AMD/ROCm/7.1/lib/clang/21/include/__stddef_size_t.h, C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.39.33519/include/cstdlib,
C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/ucrt/stdlib.h, C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/ucrt/corecrt_search.h,
C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/ucrt/corecrt_malloc.h,
...
(gdb) info inferiors
Num Description Connection Executable
* 1 process 19920 1 (native) C:/msys2/home/alves/gdb/build-testsuite/outputs/gdb.rocm/simple/simple
(gdb) info shared
From To Syms Read Shared Object Library
0x00007ffd22f91000 0x00007ffd231a69a8 Yes (*) C:/Windows/SYSTEM32/ntdll.dll
0x00007ffd21af1000 0x00007ffd21bb2520 Yes (*) C:/Windows/System32/kernel32.dll
0x00007ffd20091000 0x00007ffd20432548 Yes (*) C:/Windows/System32/KernelBase.dll
0x00007ffcc89c1000 0x00007ffcc9b145bc Yes (*) C:/Windows/SYSTEM32/amdhip64_7.dll
0x00007ffd22911000 0x00007ffd22d829b0 Yes (*) C:/Windows/System32/setupapi.dll
0x00007ffd216c1000 0x00007ffd217653f0 Yes (*) C:/Windows/System32/msvcrt.dll
0x00007ffd22431000 0x00007ffd224de5c8 Yes (*) C:/Windows/System32/advapi32.dll
0x00007ffd21a41000 0x00007ffd21ae52b8 Yes (*) C:/Windows/System32/sechost.dll
0x00007ffd20651000 0x00007ffd20676af0 Yes (*) C:/Windows/System32/bcrypt.dll
0x00007ffd21f31000 0x00007ffd22045860 Yes (*) C:/Windows/System32/rpcrt4.dll
0x00007ffd22d91000 0x00007ffd22f40e50 Yes (*) C:/Windows/System32/user32.dll
0x00007ffd20781000 0x00007ffd207a43d8 Yes (*) C:/Windows/System32/win32u.dll
0x00007ffd228d1000 0x00007ffd228f73d8 Yes (*) C:/Windows/System32/gdi32.dll
0x00007ffd207b1000 0x00007ffd208d09c8 Yes (*) C:/Windows/System32/gdi32full.dll
0x00007ffd20b91000 0x00007ffd20c28410 Yes (*) C:/Windows/System32/msvcp_win.dll
0x00007ffd20471000 0x00007ffd2057f410 Yes (*) C:/Windows/System32/ucrtbase.dll
0x00007ffd21541000 0x00007ffd215afbc8 Yes (*) C:/Windows/System32/ws2_32.dll
0x00007ffd223f1000 0x00007ffd2241fc40 Yes (*) C:/Windows/System32/imm32.dll
(*): Shared library is missing debugging information.
(gdb) detach
Detaching from program: C:/msys2/home/alves/gdb/build-testsuite/outputs/gdb.rocm/simple/simple, process 7816
[Inferior 1 (process 7816) detached]
(gdb) cd /tmp
Working directory C:/tmp.
(gdb) pwd
Working directory C:/tmp.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I6366c41d200938259330e60bc1e98b576f787518
Add new bfd wrappers:
- gdb_bfd_check_format for bfd_check_format, and
- gdb_bfd_check_format_matches for bfd_check_format_matches.
and run this command to use them:
...
$ sed -i 's/bfd_check_format/gdb_bfd_check_format/' \
$(find gdb* -type f | egrep -v "/testsuite/|ChangeLog")
...
No functional changes.
Approved-By: Tom Tromey <tom@tromey.com>
This changes a number of error messages in gdb to use command_style.
In some places I've added double quotes around the command name for
consistency with other messages.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Make it a function_view and remove the `void *` parameter. Convert the
callers (in gcore.c) to use lambda functions that capture `obfd`.
The target_debug_print_find_memory_region_ftype debug printer isn't
terribly useful, but I don't see a good way to print a function_view
with what we have right now.
Since find_memory_region_ftype needs to be seen by both gdbarch.h and
target.h, I chose to move the typedef to a new file
(find-memory-region.h), that is included by both.
Change-Id: I9d24d31da31e5fe0439124cec1a9757ad226b222
Approved-By: Tom Tromey <tom@tromey.com>
Replace all occurrences of:
strcmp (...) == 0
strcmp (...) != 0
!strcmp (...)
0 == strcmp (...)
strcmp (...) directly used as a boolean predicate
with the equivalent expression using streq.
This is for consistency (we already use streq as some places in the
testsuite) but also for clarity. I think that streq is clearer on the
intent than strcmp. It's also a bit shorter.
Change-Id: Ibbf5261b1872c240bc0c982c147f6a5477275a91
Approved-By: Andrew Burgess <aburgess@redhat.com>
Change target_ops::find_memory_region, gdbarch_find_memory_regions and
associated helpers to return bool instead of int. This is a bit
confusing, because the logic is swapped. Previously, 0 meant "success,
carry on" and non-zero meant "failure, stop". Now, true means "success,
carry on" and false means "failure, stop".
The gcore_memory_sections already used that logic, so for that one it's
a simple int to bool conversion.
I build-tested this on Solaris, for the procfs.c changes, and on
FreeBSD, for the fbsd-nat.c changes. I haven't build-tested the other
native files.
Change-Id: I27ba892bb0dc949cdb4363275c8aded73ce338bc
Approved-By: Tom Tromey <tom@tromey.com>
This updates the copyright headers to include 2026. I did this by
running gdb/copyright.py and then manually modifying a few files as
noted by the script.
Add gdb_rl_tilde_expand, a wrapper around readline's tilde_expand that
returns a gdb::unique_xmalloc_ptr<char>. Change all callers of
tilde_expand to use gdb_rl_tilde_expand (even the couple of spots that
release it immediatly, for consistency). This simplifies a few callers.
The name gdb_tilde_expand is already taken by a home-made implementation
in gdbsupport/gdb_tilde_expand.{h.cc}. I wonder if we could just use
that one instead of readline's tilde_expand, but that's an orthogonal
question. I don't know how they differ, and I don't want to introduce
behavior changes in this patch.
Change-Id: I6d34eef19f86473226df4ae56d07dc01912e3131
Approved-By: Tom Tromey <tom@tromey.com>
I noticed my IDE (VSCode) starting to automatically trim trailing
whitespaces on save, despite the setting for it being disabled. I
realized that this is because the .editorconfig file now has
trim_trailing_whitespace = true
for many file types. If we have this EditorConfig setting forcing
editors to trim trailing whitespaces, I think it would make sense to
clean up trailing whitespaces from our files. Otherwise, people will
always get spurious whitespace changes when editing these files.
I did a mass cleanup using this command:
$ find gdb gdbserver gdbsupport -type f \( \
-name "*.c" -o \
-name "*.h" -o \
-name "*.cc" -o \
-name "*.texi" -o \
-name "*.exp" -o \
-name "*.tcl" -o \
-name "*.py" -o \
-name "*.s" -o \
-name "*.S" -o \
-name "*.asm" -o \
-name "*.awk" -o \
-name "*.ac" -o \
-name "Makefile*" -o \
-name "*.sh" -o \
-name "*.adb" -o \
-name "*.ads" -o \
-name "*.d" -o \
-name "*.go" -o \
-name "*.F90" -o \
-name "*.f90" \
\) -exec sed -ri 's/[ \t]+$//' {} +
I then did an autotools regen, because we don't actually want to change
the Makefile and Makefile.in files that are generated.
Change-Id: I6f91b83e3b8c4dc7d5d51a2ebf60706120efe691
PR ada/33217 points out that gdb incorrectly calls the <ctype.h>
functions. In particular, gdb feels free to pass a 'char' like:
char *str = ...;
... isdigit (*str)
This is incorrect as isdigit only accepts EOF and values that can be
represented as 'unsigned char' -- that is, a cast is needed here to
avoid undefined behavior when 'char' is signed and a character in the
string might be sign-extended. (As an aside, I think this API seems
obviously bad, but unfortunately this is what the standard says, and
some systems check this.)
Rather than adding casts everywhere, this changes all the code in gdb
that uses any <ctype.h> API to instead call the corresponding c-ctype
function.
Now, c-ctype has some limitations compared to <ctype.h>. It works as
if the C locale is in effect, so in theory some non-ASCII characters
may be misclassified. This would only affect a subset of character
sets, though, and in most places I think ASCII is sufficient -- for
example the many places in gdb that check for whitespace.
Furthermore, in practice most users are using UTF-8-based locales,
where these functions aren't really informative for non-ASCII
characters anyway; see the existing workarounds in gdb/c-support.h.
Note that safe-ctype.h cannot be used because it causes conflicts with
readline.h. And, we canot poison the <ctype.h> identifiers as this
provokes errors from some libstdc++ headers.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33217
Approved-By: Simon Marchi <simon.marchi@efficios.com>
I wrote this as a preparatory patch while attempting to make
objfile::section_iterator use filtered_iterator. It turned out not so
easy, so I have put it aside for now. But now I have this patch, so I
thought I'd send it by itself.
Since the `obj_section *` yielded by the iterator can't be nullptr, I
think it makes sense for the iterator to yield references instead.
Just like you would get if you iterated on an std::vector<obj_section>.
Change-Id: I7bbee50ed52599e64c4f3b06bdbbde597feba9aa
This patch introduces a new macro, INIT_GDB_FILE. This is used to
replace the current "_initialize_" idiom when introducing a per-file
initialization function. That is, rather than write:
void _initialize_something ();
void
_initialize_something ()
{
...
}
... now you would write:
INIT_GDB_FILE (something)
{
...
}
The macro handles both the declaration and definition of the function.
The point of this approach is that it makes it harder to accidentally
cause an initializer to be omitted; see commit 2711e475 ("Ensure
cooked_index_entry self-tests are run"). Specifically, the regexp now
used by make-init-c seems harder to trick.
New in v2: un-did some erroneous changes made by the script.
The bulk of this patch was written by script.
Regression tested on x86-64 Fedora 41.
Currently, when attaching to a process, if the user hasn't told GDB
which executable they are going to be debugging, GDB will try to
figure out the executable from the running process.
There are two (for this patch) interesting places where this can fail,
both in exec_file_locate_attach.
First GDB calls target_pid_to_exec_file, this does target specific
"stuff" to find the name of the executable file. If this returns NULL
then GDB will give a warning and return.
After this we need to "find" the executable. This is where we apply
things like the sysroot in order to transform the executable path.
This is done by calling exec_file_find, and this too can return NULL
to indicate that the executable couldn't be found.
Currently, if exec_file_find returns NULL then GDB doesn't give a
warning, instead we push on and call try_open_exec_file passing in the
NULL pointer as the filename string. This has the effect of removing
the current executable from the current program space.
However, exec_file_locate_attach already checks there is no executable
attached to the current program space. If there was, then there would
be no need to try and lookup the executable from the running process.
So calling try_open_exec_file with a NULL string is, I claim,
pointless.
But worse, calling try_open_exec_file with a NULL string means that
GDB prints the message: "No executable file now.", which, while
correct, isn't (I think) very helpful. To me this message indicates
that we've moved from a state of having an executable to a state of
not having one, which isn't correct.
I think we should introduce a new warning in exec_file_locate_attach,
which is printed if the executable cannot be found.
So, before this patch GDB's output looked like this:
(gdb) attach 12345
Attaching to process 12345
No executable file now.
warning: Could not load vsyscall page because no executable was specified
0x00007f0978b94557 in ?? ()
(gdb)
After this patch the output now looks like this:
(gdb) attach 12345
Attaching to process 12345
No executable has been specified, and target executable /tmp/my-exec (deleted) could not be found. Try using the "file" command.
warning: Could not load vsyscall page because no executable was specified
0x00007f0978b94557 in ?? ()
(gdb)
This warning includes the name of the file that GDB was looking for,
and gives a hint that the 'file' command should be used to tell GDB
which executable is being debugged. Much better.
There's no test for this change in this commit. The next commit fixes
another (semi-related) bug, and includes a test that checks for this
warning string.
I don't think that the file solist.h is useful. It would make sense to
have `struct solib` in solib.h. And then, all that would remain is
`struct solib_ops` and some solib-related function declarations. So,
move it all to solib.h.
Change-Id: I20ecf19787c378066f2c7a6a8a737c1db7c55d9a
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
While reviewing another patch I was briefly confused by a call to
target_pid_to_exec_file coming from validate_exec_file while attaching
to a process when I had not previously set an executable.
The current order of actions in validate_exec_file is:
1. Get name of current executable.
2. Get name of executable from the current inferior.
3. If either of (1) or (2) return NULL, then there's nothing to
check, early return.
I think it would be cleaner if we instead did this:
1. Get name of current executable.
3. If (1) returned NULL then there's nothing to check, early return.
3. Get name of executable from the current inferior.
4. If (3) returned NULL then there's nothing to check, early return.
This does mean there's an extra step, but I don't think the code is
any more complex really, and we now avoid trying to extract the name
of the executable from the current inferior unless we really need it.
This avoids the target_pid_to_exec_file call that I was seeing, which
for remote targets does avoid a packet round trip (not that I'm
selling this as an "optimisation", just noting the change).
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This updates the copyright headers to include 2025. I did this by
running gdb/copyright.py and then manually modifying a few files as
noted by the script.
Approved-By: Eli Zaretskii <eliz@gnu.org>
This adds a new "command" style that is used when styling the name of
a gdb command.
Note that not every instance of a command name that is output by gdb
is changed here. There is currently no way to style error() strings,
and there is no way to mark up command help strings.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31747
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
PR gdb/31713 points out some races when using the background DWARF
reader.
This particular patch fixes some of these, namely the ones when using
the sim. In this case, the 'load' command calls reopen_exec_file,
which calls bfd_stat, which introduces a race.
BFD only locks globals -- concurrent use of a single BFD must be
handled by the application. To this end, this patch adds locked
wrappers for bfd_stat and bfd_get_mtime.
I couldn't reproduce these data races but the original reporter tested
the patch and confirms that it helps.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31713
Approved-By: Andrew Burgess <aburgess@redhat.com>
This commit changes the 'target ...' commands that accept a filename
to take a quoted or escaped filename rather than a literal filename.
What this means in practice is that if you are specifying a filename
that contains no white space or quote characters, then nothing should
change, e.g.:
target exec /path/to/some/file
works both before and after this commit.
However, if a user wishes to specify a file containing white space
then either the entire filename needs to be quoted, or the special
white space needs to be escaped. Before this patch a user could
write:
target exec /path/to a file/containing spaces
But after this commit the user would have to choose one of:
target exec "/path/to a file/containing spaces"
or
target exec /path/to\ a\ file/containing\ spaces
Obviously this is a potentially breaking change. The benefit of
making this change is consistency. Commands that take multiple
arguments (one of which is a filename) or in the future, commands that
take filename options, will always need to use quoted/escaped
filenames, so converting all unquoted filename commands to use quoting
or escaping makes the UI more consistent.
Additionally (though this is probably not a common problem), GDB
strips trailing white space from commands that the user enters. As
such it is not possible to reference any file that ends in white space
unless the quoting / escaping style is used. Though I suspect very
few users run into this problem!
The downside obviously is that this is a UI breaking change.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Following on from the previous commit, this commit marks the old
unquoted filename completion related functions as deprecated.
The aim of doing this is to make it more obvious to someone adding a
new command that they should not be using the older unquoted style
filename argument handling.
I split this change from the previous to make for an easier review.
This commit touches more files, but is _just_ function renaming.
Check out gdb/completer.{c,h} for what has been renamed. All the
other files have just been updated to use the new names.
There should be no user visible changes after this commit.
Unfortunately we have two different types of filename completion in
GDB.
The majority of commands have what I call unquoted filename
completion, this is for commands like 'set logging file ...', 'target
core ...', and 'add-auto-load-safe-path ...'. For these commands
everything after the command name (that is not a command option) is
treated as a single filename. If the filename contains white space
then this does not need to be escaped, nor does the filename need to
be quoted. In fact, the filename argument is not de-quoted, and does
not have any escaping removed, so if a user does try to add such
things, they will be treated as part of the filename. As an example:
(gdb) target core "/path/that contains/some white space"
Will look for a directory calls '"' (double quotes) in the local
directory.
A small number of commands do de-quote and remove escapes from
filename arguments. These command accept what I call quoted and
escaped filenames. Right now these are the commands that specify the
file for GDB to debug, so:
file
exec-file
symbol-file
add-symbol-file
remove-symbol-file
As an example of this in action:
(gdb) file "/path/that contains/some white space"
In this case GDB would load the file:
/path/that contains/some white space
Current filename completion always assumes that filenames can be
quoted, though escaping doesn't work in completion right now. But the
assumption that quoting is allowed is clearly wrong.
This commit splits filename completion into two. The existing
filename_completer is retained, and is used for unquoted filenames. A
second filename_maybe_quoted_completer is added which can be used for
completing quoted filenames.
The filename completion test has been extended to cover more cases.
As part of the extended testing I need to know the character that
should be used to separate filenames within a path. For this TCL 8.6+
has $::tcl_platform(pathSeparator). To support older versions of TCL
I've added some code to testsuite/lib/gdb.exp.
You might notice that after this commit the completion for unquoted
files is all done in the brkchars phase, that is the function
filename_completer_handle_brkchars calculates the completions and
marks the completion_tracker as using a custom word point. The reason
for this is that we don't want to break on white space for this
completion, but if we rely on readline to find the completion word,
readline will consider the entire command line, and with no white
space in the word break character set, readline will end up using the
entire command line as the word to complete.
For now at least, the completer for quoted filenames does generate its
completions during the completion phase, though this is going to
change in a later commit.
I did a review of lines containing "catch (gdb_exception" and found a few
where we can add const.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
I believe that the get_exec_file function is unnecessary, and the code
can be simplified if we remove it.
Consider for instance when you "run" a program on Linux with native
debugging.
1. run_command_1 obtains the executable file from
`current_program_space->exec_filename ()`
2. it passes it to `run_target->create_inferior()`, which is
`inf_ptrace_target::create_inferior()` in this case, which then
passes it to `fork_inferior()`
3. `fork_inferior()` then has a fallback, where if the passed exec file
is nullptr, it gets its from `get_exec_file()`.
4. `get_exec_file()` returns `current_program_space->exec_filename ()`
- just like the things we started with - or errors out if the
current program space doesn't have a specified executable.
If there's no exec filename passed in step 1, there's not going to be
any in step 4, so it seems pointless to call `get_exec_file()`, we could
just error out when `exec_file` is nullptr. But we can't error out
directly in `fork_inferior()`, since the error is GDB-specific, and that
function is shared with GDBserver.
Speaking of GDBserver, all code paths that lead to `fork_inferior()`
provide a non-nullptr exec file.
Therefore, to simplify things:
- Make `fork_inferior()` assume that the passed exec file is not
nullptr, don't call `get_exec_file()`
- Change some targets (darwin-nat, go32-nat, gnu-nat, inf-ptrace,
nto-procfs, procfs) to error out when the exec file passed to their
create_inferior method is nullptr. Some targets are fine with a
nullptr exec file, so we can't check that in `run_command_1()`.
- Add the `no_executable_specified_error()` function, which re-uses the
error message that `get_exec_file()` had.
- Change some targets (go32-nat, nto-procfs) to not call
`get_exec_file()`, since it's pointless for the same reason as in the
example above, if it returns, it's going the be the same value as the
`exec_file` parameter. Just rely on `exec_file`.
- Remove the final use of `get_exec_file()`, in `load_command()`.
- Remove the `get_exec_file()` implementations in GDB and GDBserver and
remove the shared declaration.
Change-Id: I601c16498e455f7baa1f111a179da2f6c913baa3
Approved-By: Tom Tromey <tom@tromey.com>
Calls of `get_exec_file (0)` are equivalent to just getting the exec
filename from the current program space. I'm looking to remove
`get_exec_file`, so replace these uses with
`current_program_space->exec_filename ()`.
Remove the `err` parameter of `get_exec_wrapper` since all the calls
that remain pass 1, meaning to error out if no executable is specified.
Change-Id: I7729ea4c7f03dbb046211cc5aa3858ab3a551965
Approved-By: Tom Tromey <tom@tromey.com>
Just like the title says... I think this makes things a bit clearer, for
instance where the exec filename is set. It also makes the read call
sites a bit nicer, avoiding the `.get ()`.
Change-Id: If8b58ae8f6270c8a34b868f6ca06128c6671ea3c
Approved-By: Tom Tromey <tom@tromey.com>
Add two versions of a new function build_id_equal which can be used to
compare build-ids, then make use of these functions in GDB. It seems
better to have a specific function for the task of comparing build-ids
rather than having a length check followed by a memcmp call.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
Most files including gdbcmd.h currently rely on it to access things
actually declared in cli/cli-cmds.h (setlist, showlist, etc). To make
things easy, replace all includes of gdbcmd.h with includes of
cli/cli-cmds.h. This might lead to some unused includes of
cli/cli-cmds.h, but it's harmless, and much faster than going through
the 170 or so files by hand.
Change-Id: I11f884d4d616c12c05f395c98bbc2892950fb00f
Approved-By: Tom Tromey <tom@tromey.com>
Now that defs.h, server.h and common-defs.h are included via the
`-include` option, it is no longer necessary for source files to include
them. Remove all the inclusions of these files I could find. Update
the generation scripts where relevant.
Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837
Approved-By: Pedro Alves <pedro@palves.net>
This commit removes deprecated_exec_file_display_hook and the
associated specify_exec_file_hook.
The specify_exec_file_hook is used to add a new hook function to
deprecated_exec_file_display_hook, but is only used from the insight
debugger.
I posted a patch to remove the use of specify_exec_file_hook from
insight, and instead use gdb::observers::executable_changed, this
patch can be found here (it has now been merged):
https://inbox.sourceware.org/insight/6abeb45e97d9004ec331e94cf2089af00553de76.1702379379.git.aburgess@redhat.com/T/#u
With this merged we can now cleanup the GDB side as this code is now
unused.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
This commit is the result of the following actions:
- Running gdb/copyright.py to update all of the copyright headers to
include 2024,
- Manually updating a few files the copyright.py script told me to
update, these files had copyright headers embedded within the
file,
- Regenerating gdbsupport/Makefile.in to refresh it's copyright
date,
- Using grep to find other files that still mentioned 2023. If
these files were updated last year from 2022 to 2023 then I've
updated them this year to 2024.
I'm sure I've probably missed some dates. Feel free to fix them up as
you spot them.
In the following commit I ran into a problem. The next commit aims to
improve GDB's handling of the main executable being a file on a remote
target (i.e. one with a 'target:' prefix).
To do this I have replaced a system 'stat' call with a bfd_stat call.
However, doing this caused a regression in gdb.base/attach.exp.
The problem is that the bfd library caches open FILE* handles for bfd
objects that it has accessed, which is great for short-lived, non
interactive programs (e.g. the assembler, or objcopy, etc), however,
for GDB this caching causes us a problem.
If we open the main executable as a bfd then the bfd library will
cache the open FILE*. If some time passes, maybe just sat at the GDB
prompt, or with the inferior running, and then later we use bfd_stat
to check if the underlying, on-disk file has changed, then the bfd
library will actually use fstat on the underlying file descriptor.
This is of course slightly different than using system stat on with
the on-disk file name.
If the on-disk file has changed then system stat will give results for
the current on-disk file. But, if the bfd cache is still holding open
the file descriptor for the original on-disk file (from before the
change) then fstat will return a result based on the original file,
and so show no change as having happened.
This is a known problem in GDB, and so far this has been solved by
scattering bfd_cache_close_all() calls throughout GDB. But, as I
said, in the next commit I've made a change and run into a
problem (gdb.base/attach.exp) where we are apparently missing a
bfd_cache_close_all() call.
Now I could solve this problem by adding a bfd_cache_close_all() call
before the bfd_stat call that I plan to add in the next commit, that
would for sure solve the problem, but feels a little crude.
Better I think would be to track down where the bfd is being opened
and add a corresponding bfd_cache_close_all() call elsewhere in GDB
once we've finished doing whatever it is that caused us to open the
bfd in the first place.
This second solution felt like the better choice, so I tracked the
problem down to elf_locate_base and fixed that. But that just exposed
another problem in gdb_bfd_map_section which was also re-opening the
bfd, so I fixed this (with another bfd_cache_close_all() call), and
that exposed another issue in gdbarch_lookup_osabi... and at this
point I wondered if I was approaching this problem the wrong way...
.... And so, I wonder, is there a _better_ way to handle these
bfd_cache_close_all() calls?
I see two problems with the current approach:
1. It's fragile. Folk aren't always aware that they need to clear
the bfd cache, and this feels like something that is easy to
overlook in review. So adding new code to GDB can innocently touch
a bfd, which populates the cache, which will then be a bug that can
lie hidden until an on-disk file just happens to change at the wrong
time ... and GDB fails to spot the change. Additionally,
2. It's in efficient. The caching is intended to stop the bfd
library from continually having to re-open the on-disk file. If we
have a function that touches a bfd then often that function is the
obvious place to call bfd_cache_close_all. But if a single GDB
command calls multiple functions, each of which touch the bfd, then
we will end up opening and closing the same on-disk file multiple
times. It feels like we would be better postponing the
bfd_cache_close_all call until some later point, then we can benefit
from the bfd cache.
So, in this commit I propose a new approach. We now clear the bfd
cache in two places:
(a) Just before we display a GDB prompt. We display a prompt after
completing a command, and GDB is about to enter an idle state
waiting for further input from the user (or in async mode, for an
inferior event). If while we are in this idle state the user
changes the on-disk file(s) then we would like GDB to notice this
the next time it leaves its idle state, e.g. the next time the user
executes a command, or when an inferior event arrives,
(b) When we resume the inferior. In synchronous mode, resuming the
inferior is another time when GDB is blocked and sitting idle, but
in this case we don't display a prompt. As with (a) above, when an
inferior event arrives we want GDB to notice any changes to on-disk
files.
It turns out that there are existing observers for both of these
cases (before_prompt and target_resumed respectively), so my initial
thought was that I should attach to these observers in gdb_bfd.c, and
in both cases call bfd_cache_close_all().
And this does indeed solve the gdb.base/attach.exp problem that I see
with the following commit.
However, I see a problem with this solution.
Both of the observers I'm using are exposed through the Python API as
events that a user can hook into. The user can potentially run any
GDB command (using gdb.execute), so Python code might end up causing
some bfds to be reopened, and inserted into the cache.
To solve this one solution would be to add a bfd_cache_close_all()
call into gdbpy_enter::~gdbpy_enter(). Unfortunately, there's no
similar enter/exit object for Guile, though right now Guile doesn't
offer the same event API, so maybe we could just ignore that
problem... but this doesn't feel great.
So instead, I think a better solution might be to not use observers
for the bfd_cache_close_all() calls. Instead, I'll call
bfd_cache_close_all() directly from core GDB after we've notified the
before_prompt and target_resumed observers, this was we can be sure
that the cache is cleared after the observers have run, and before GDB
enters an idle state.
This commit also removes all of the other bfd_cache_close_all() calls
from GDB. My claim is that these are no longer needed.
Approved-By: Tom Tromey <tom@tromey.com>
Commit 8971d2788e ("gdb: link so_list using intrusive_list") introduced
a bug in clear_solib. Instead of passing an `so_list *` to
remove_target_sections, it passed an `so_list **`. This was not caught
by the compiler, because remove_target_sections takes a `void *` as the
"owner", so you can pass it any pointer and it won't complain.
This happened because I previously had a patch to change the type of the
disposer parameter to be a reference rather than a pointer, so had to
change `so` to `&so`. When dropping that patch, I forgot to revert this
bit and / or it got re-introduced when handling subsequent merge
conflicts. And I didn't properly retest.
Fix that, but try to make things less error prone. Add a union to
represent the possible owner kinds for a target_section. Trying to pass
a pointer to another type than those will not compile.
Change-Id: I600cab5ea0408ccc5638467b760768161ca3036c
Remove this typedef. I think that hiding the real type (std::vector)
behind a typedef just hinders readability.
Change-Id: I80949da3392f60a2826c56c268e0ec6f503ad79f
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
A subsequent patch changes so_list to be linked using
intrusive_list. Iterating an intrusive_list yields some references to
the list elements. Convert some functions accepting so_list objects to
take references, to make things easier and more natural. Add const
where possible and convenient.
Change-Id: Id5ab5339c3eb6432e809ad14782952d6a45806f3
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
This commit continues the work of the previous few commits.
My goal is to expose the executable_changed observer through the
Python API as an event.
At this point adding executable_changed as an event to the Python API
is trivial, but before I do that I would like to add some additional
arguments to the observable, which currently has no arguments at all.
The new arguments I wish to add are:
1. The program_space in which the executable was changed, and
2. A boolean flag that will indicate if the executable changed to a
whole new path, or if GDB just spotted that the executable changed on
disk (e.g. the user recompiled the executable).
In this commit I change the signature of the observable and then pass
the arguments through at the one place where this observable is
notified.
As there are (currently) no users of this observable nothing else
needs updating. In the next commit I'll add a listener for this
observable in the Python code, and expose this as an event in the
Python API.
Additionally, with this change, it should be possible to update the
insight debugger to make use of this observable rather than using the
deprecated_exec_file_display_hook (as it currently does), which will
then allow this hook to be removed from GDB.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
Currently, every internal_error call must be passed __FILE__/__LINE__
explicitly, like:
internal_error (__FILE__, __LINE__, "foo %d", var);
The need to pass in explicit __FILE__/__LINE__ is there probably
because the function predates widespread and portable variadic macros
availability. We can use variadic macros nowadays, and in fact, we
already use them in several places, including the related
gdb_assert_not_reached.
So this patch renames the internal_error function to something else,
and then reimplements internal_error as a variadic macro that expands
__FILE__/__LINE__ itself.
The result is that we now should call internal_error like so:
internal_error ("foo %d", var);
Likewise for internal_warning.
The patch adjusts all calls sites. 99% of the adjustments were done
with a perl/sed script.
The non-mechanical changes are in gdbsupport/errors.h,
gdbsupport/gdb_assert.h, and gdb/gdbarch.py.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
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.
I'm trying to switch these functions to use std::string instead of char
arrays, as much as possible. Some callers benefit from it (can avoid
doing a copy of the result), while others suffer (have to make one more
copy).
Change-Id: I793aab17baaef8345488f4c40b9094e2695425bc
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.
I think it only really makes sense to call wrap_here with an argument
consisting solely of spaces. Given this, it seemed better to me that
the argument be an int, rather than a string. This patch is the
result. Much of it was written by a script.
Many otherwise ordinary commands choose to use unfiltered output
rather than filtered. I don't think there's any reason for this, so
this changes many such commands to use filtered output instead.
Note that complete_command is not touched due to a comment there
explaining why unfiltered output is believed to be used.
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.
