Commit Graph

53414 Commits

Author SHA1 Message Date
Tom de Vries
2af94d6c92 [gdb] Handle EINTR in ser-event.c
Use gdb syscall wrappers to handle EINTR in ser-event.c.

Tested on aarch64-linux.
2024-11-22 17:44:29 +01:00
Tom de Vries
4e4dfc4728 [gdb] Add gdb::wait
Add gdb::wait, and use it in gdb/procfs.c, making sure that EINTR is handled.

Tested on x86_64-linux.
2024-11-22 17:44:29 +01:00
Tom de Vries
a9791f1438 [gdb] Use gdb::waitpid more often
Use gdb::waitpid instead of plain waitpid, making sure that EINTR is handled.

Tested on x86_64-linux.
2024-11-22 17:44:29 +01:00
Tom de Vries
658a03e9e8 [gdbsupport] Add gdb::{waitpid,read,write,close}
We have gdb::handle_eintr, which allows us to rewrite:
...
  ssize_t ret;
    do
      {
        errno = 0;
        ret = ::write (pipe[1], "+", 1);
      }
    while (ret == -1 && errno == EINTR);
...
into:
...
  ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
...

However, the call to write got a bit mangled, requiring effort to decode it
back to its original form.

Instead, add a new function gdb::write that allows us to write:
...
  ssize_t ret = gdb::write (pipe[1], "+", 1);
...

Likewise for waitpid, read and close.

Tested on x86_64-linux.
2024-11-22 17:44:29 +01:00
Andrew Burgess
26522e3480 gdb/disasm: fix demangling when disassembling the current function
When disassembling function symbols in C++ code, if GDB is asked to
disassemble a function by name then the function name in the header
line can be demangled by turning on `set print asm-demangle on`, e.g.:

  (gdb) disassemble foo_type::some_function
  Dump of assembler code for function _ZN8foo_type13some_functionE7my_type:
     0x0000000000401142 <+0>:	push   %rbp
     ... etc ...
  End of assembler dump.
  (gdb) set print asm-demangle on
  (gdb) disassemble foo_type::some_function
  Dump of assembler code for function foo_type::some_function(my_type):
     0x0000000000401142 <+0>:	push   %rbp
     ... etc ...                                                        │
  End of assembler dump.                                                │

However, if GDB is disassembling the current function, then this
demangling doesn't work, e.g.:

  (gdb) break foo_type::some_function
  Breakpoint 1 at 0x401152: file mangle.cc, line 16.
  (gdb) run
  Starting program: /tmp/mangle

  Breakpoint 1, foo_type::some_function (this=0x7fffffffa597, obj=...) at mangle.cc:16
  16	    obj.update ();
  (gdb) disassemble
  Dump of assembler code for function _ZN8foo_type13some_functionE7my_type:
     0x0000000000401142 <+0>:	push   %rbp
     ... etc ...
  End of assembler dump.
  (gdb) set print asm-demangle on
  (gdb) disassemble
  Dump of assembler code for function _ZN8foo_type13some_functionE7my_type:
     0x0000000000401142 <+0>:	push   %rbp
     ... etc ...
  End of assembler dump.

This commit fixes this issue, and extends gdb.cp/disasm-func-name.exp,
which was already testing the first case (disassemble by name) to also
cover disassembling the current function.

Approved-By: Tom Tromey <tom@tromey.com>
2024-11-22 16:36:07 +00:00
Tom de Vries
8a7f13063a [gdb/python] Ensure locale is restored in do_start_initialization
I noticed in do_start_initialization:
...
  std::string oldloc = setlocale (LC_ALL, NULL);
  setlocale (LC_ALL, "");
  ...
  if (count == (size_t) -1)
    {
      fprintf (stderr, "Could not convert python path to string\n");
      return false;
    }
  setlocale (LC_ALL, oldloc.c_str ());
...
that the old locale is not restored if the "return false" is triggered.

Fix this by using SCOPE_EXIT.

Tested on aarch64-linux.

Approved-By: Tom Tromey <tom@tromey.com>
2024-11-22 17:34:50 +01:00
Tom de Vries
00386b4c68 [gdb/tdep] Simplify amd64_windows_store_arg_in_reg
Simplify amd64_windows_store_arg_in_reg by:
- replacing memset with value initialization,
- making valbuf a gdb::array_view, allowing us to:
  - replace memcpy with std::copy, and
  - use valbuf.size () instead of arg->type->size (), and
- dropping the std::min in std::min (type->length (), (ULONGEST) 8), since
  we're already asserting that type->length () <= 8.

Suggested-By: Tom Tromey <tom@tromey.com>

Tested by rebuilding on x86_64-linux.
2024-11-22 13:43:03 +01:00
Tom de Vries
2e61ad32ab [gdb/testsuite] Require local host in gdb.base/bg-exec-sigint-bp-cond.exp
I noticed that gdb.base/bg-exec-sigint-bp-cond.exp fails for remote host
(concretely, host board local-remote-host and target board
remote-gdbserver-on-localhost):
...
(gdb) c&^M
Continuing.^M
(gdb) bash: line 0: kill: (23834) - Operation not permitted^M
^M
Breakpoint 2, foo () at bg-exec-sigint-bp-cond.c:23^M
23        return 0;^M
...
due to getting gdb's pid like this:
...
    set gdb_pid [exp_pid -i [board_info host fileid]]
...

For remote host using ssh, this returns the pid of the ssh session on build.

Fix this by requiring local host.

Tested on x86_64-linux.
2024-11-22 13:37:24 +01:00
Tom de Vries
b200576fa0 [gdb/testsuite] Fix gdb.base/bg-exec-sigint-bp-cond.exp for signal merging
The test-case gdb.base/bg-exec-sigint-bp-cond.exp sends 10 SIGINTS to gdb, and
counts whether 10 have arrived.

Occasionally, less than 10 arrive due to signal merging [1].

This is more likely to happen when building gdb with -fsanitize=thread.

Fix this by instead, sending one SIGINT at a time, and waiting for it to
arrive.

This still makes the test-case fail if the fix fixing the PR that the
test-case was introduced for is reverted.

Tested on aarch64-linux and x86_64-linux.

PR testsuite/32329
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32329

[1] https://www.gnu.org/software/libc/manual/html_node/Merged-Signals.html
2024-11-22 13:37:24 +01:00
Tom de Vries
c4df8ad79c [gdb/build] Workaround tsan select bug
When building gdb with -O0 and -fsanitize-thread, I run into a large number of
timeouts caused by gdb hanging, for instance:
...
(gdb) continue^M
Continuing.^M
[Inferior 1 (process 378) exited normally]^M
FAIL: gdb.multi/stop-all-on-exit.exp: continue until exit (timeout)
...

What happens is the following:
- two inferiors are added, stopped at main
- inferior 1 is setup to exit after 1 second
- inferior 2 is setup to exit after 10 seconds
- the continue command is issued
- because of set schedule-multiple on, both inferiors continue
- the first inferior exits
- gdb sends a SIGSTOP to the second inferior
- the second inferior receives the SIGSTOP, and raises a SIGCHILD
- gdb calls select, and blocks
- the signal arrives, and interrupts select
- ThreadSanitizers signal handler is called, which marks the signal pending
  internally
- select returns -1 with errno == EINTR
- gdb calls select again, and blocks
- gdb hangs, waiting for gdb's sigchild_handler to be called

This is a bug [1] in ThreadSanitizer.  When select is called with
timeout == nullptr, it is blocking but ThreadSanitizer doesn't consider it so,
and consequently doesn't see the need to call sigchild_handler.

Work around this by:
- instead of using the blocking select variant, forcing a small timeout and
- upon timeout calling a function that ThreadSanitizer does consider
  blocking: usleep, forcing sigchild_handler to be called.

Tested on x86_64-linux.

PR build/32295
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32295

[1] https://github.com/google/sanitizers/issues/1813
2024-11-22 12:54:57 +01:00
Tom de Vries
dcc4d67866 [gdb] Add gdb_select variant for looping
In interruptible_select we run gdb_select in a loop:
...
  do
    {
      res = gdb_select (n, readfds, writefds, exceptfds, timeout);
    }
  while (res == -1 && errno == EINTR);
...
but man select tells us that:
- if using select() within a loop, the sets (readfds, writefds and
  exceptfds) must be reinitialized before each call, and
- timeout should be considered to be undefined after select() returns.

Add a gdb_select variant:
...
static int
gdb_select (int n,
	    const fd_set *req_readfds, fd_set *ret_readfds,
	    const fd_set *req_writefds, fd_set *ret_writefds,
	    const fd_set *req_exceptfds, fd_set *ret_exceptfds,
	    const struct timeval *req_timeout, struct timeval *ret_timeout)
...
that keeps requested and returned values separate, ensuring that the requested
values stay constant.

Tested on x86_64-linux.

Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
2024-11-22 12:54:57 +01:00
Tom Tromey
29e316d8b1 Don't put JIT_READER_DIR into help text
The 80-column-help-string self-test can fail if gdb's install
directory is too long, because the help for "jit-reader-load" includes
JIT_READER_DIR.

This help text is actually somewhat misleading, though.
JIT_READER_DIR is not actually used directly -- instead the relocated
variant is used.

This patch adds a new "show jit-reader-directory" command and changes
the help text to refer to this instead.  I considered adding a "set"
command as well, but since absolute paths are acceptable here, and
since this is a very niche command anyway, I figured there was no need
to bother.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32357
Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Andrew Burgess <aburgess@redhat.com>
2024-11-21 14:08:18 -07:00
Andrew Burgess
9783247189 gdb/build-id: protect against weirdly short build-ids
While looking at build_id_to_bfd_suffix (in gdb/build-id.c) I realised
that GDB would likely not do what we wanted if a build-id was ever a
single byte.

Right now, build-ids generated by the GNU linker are 32 bytes, but
there's nothing that forces this to be the case, it's pretty easy to
create a fake, single byte, build-id.  Given that the build-id is an
external input (read from the objfile), GDB should protect itself
against these edge cases.

The problem with build_id_to_bfd_suffix is that this function creates
the path used to lookup either the debug information, or an
executable, based on its build-id.  So a 3-byte build-id 0xaabbcc will
look in the path: `$DEBUG_FILE_DIRECTORY/.build-id/aa/bbcc.debug`.
However, a single byte build-id 0xaa, will look in the file:
`$DEBUG_FILE_DIRECTORY/.build-id/aa/.debug` which doesn't seem right.

Worse, when looking for an objfile given a build-id GDB will look for
a file called `$DEBUG_FILE_DIRECTORY/.build-id/aa/` with a trailing
'/' character.

I propose that, in build_id_to_bfd_suffix we just return early if the
build-id is 1 byte (or less) with a return value that indicates no
separate file was found.

For testing I made use of the DWARF assembler.  I needed to update the
build-id creation proc, the existing code assumes that the build-id is
a multiple of 4 bytes, so I added some additional padding to ensure
that the generated note was a multiple of 4 bytes, even if the
build-id was not.

I added a test with a 1 byte build-id, and also for the case where the
build-id has zero length.  The zero length case already does what
you'd expect (no debug is loaded) as the bfd library rejects the
build-id when loading it from the objfile, but adding this additional
test is pretty cheap.

Approved-By: Kevin Buettner <kevinb@redhat.com>
2024-11-21 19:38:39 +00:00
Rohr, Stephan
be740e7cc6 testsuite: skip confirmation in 'gdb_reinitialize_dir'
Some shells automatically confirm the 'dir' command:

  (gdb) dir
  Reinitialize source path to empty? (y or n)
    [answered Y; input not from terminal]
  Source directories searched: $cdir;$cwd
  (gdb) y
  dir <...>/gdb/testsuite/gdb.base
  Undefined command: "y".  Try "help".

For example, this reprdocues in a MinGW32 environment with
'TERM=dumb'.  Skip sending 'y' if the command is already confirmed.

Approved-By: Tom Tromey <tom@tromey.com>
2024-11-21 00:49:39 -08:00
Tom Tromey
b6f3ac06ee Improve choice sorting in ada-lang.c
ada-lang.c has a "sort_choices" function that claims to sort the
symbol choices, but which does not really implement sorting.  This
patch changes this code to really sort the result vector, sorting
first by filename, then line number, and finally by the symbol name.

The filename sorting is done first by comparing basenames.  It turns
out that gnatmake and gprbuild invoke the compiler a bit differently,
so depending on which one you use, the results of a naive sort might
be different (due to the use of absolute or relative paths).
2024-11-20 13:18:40 -07:00
Mohamed Bouhaouel
a5419b6f00 gdb: add Mohamed Bouhaouel to gdb/MAINTAINERS 2024-11-20 15:17:01 +01:00
Andrew Burgess
661611b9d7 gdb/python: fix reference leak in gdb.BreakpointLocation.thread_groups
While reviewing another patch which uses PyList_Append I took a look
at our other uses of PyList_Append in GDB.  I spotted something odd
about the use in bplocpy_get_thread_groups.

We do:

    gdbpy_ref<> num = gdb_py_object_from_ulongest (inf->num);

At which point `num` will own a reference to the `int` object.  But
when we add the object to the result list we do:

    if (PyList_Append (list.get (), num.release ()) != 0)
      return nullptr;

By calling `release` we pass ownership of the reference to
PyList_Append, however, PyList_Append acquires its own reference, it
doesn't take ownership of an existing reference.

The consequence of this is that we leak the reference held in `num`.

This mostly isn't a problem though.  For small (< 257) integers Python
keeps a single instance of each and just hands out new references.  By
leaking the references, these small integers will not be cleaned up as
the Python interpreter shuts down, but that is only done when GDB
exits, so hardly a disaster.  As we're dealing with GDB's internal
inferior number here, unless the user has 257+ inferiors, we'll not
actually be leaking memory.

Still, lets do things right.  Switch to using `num.get ()`.  Now when
`num` goes out of scope it will decrement the reference count as
needed.

Approved-By: Tom Tromey <tom@tromey.com>
2024-11-20 09:33:17 +00:00
Charles Baylis
9fc57f35eb gdb: Remove inappropriate comments
Remove some inappropriate comments in darwin_nat_target::attach,
gnu_nat_target::attach and inf_ptrace_target::attach.

Tested by rebuilding on x86_64-linux.

Copyright-paperwork-exempt: yes
Approved-By: Tom Tromey <tom@tromey.com>
2024-11-19 22:27:37 +01:00
Tom de Vries
f5e259f32f [gdb/contrib] Fix shellcheck warnings in spellcheck.sh
Fix shellcheck warnings in spellcheck.sh, found using shellcheck v0.10.0.

Ran shellcheck v0.10.0 (on a system with shellcheck version 0.8.0) using this
command from an RFC patch [1]:
...
$ ./gdb/contrib/pre-commit-shellcheck.sh ./gdb/contrib/spellcheck.sh
...

Tested on x86_64-linux

[1] https://sourceware.org/pipermail/gdb-patches/2024-November/213400.html
2024-11-19 12:32:40 +01:00
Christina Schimpe
27e82ad68b LAM: Enable tagged pointer support for watchpoints.
The Intel (R) linear address masking (LAM) feature modifies the checking
applied to 64-bit linear addresses.  With this so-called "modified
canonicality check" the processor masks the metadata bits in a pointer
before using it as a linear address.  LAM supports two different modes that
differ regarding which pointer bits are masked and can be used for
metadata: LAM 48 resulting in a LAM width of 15 and LAM 57 resulting in a
LAM width of 6.

This patch adjusts watchpoint addresses based on the currently enabled
LAM mode using the untag mask provided in the /proc/<pid>/status file.
As LAM can be enabled at runtime or as the configuration may change
when entering an enclave, GDB checks enablement state each time a watchpoint
is updated.

In contrast to the patch implemented for ARM's Top Byte Ignore "Clear
non-significant bits of address on memory access", it is not necessary to
adjust addresses before they are passed to the target layer cache, as
for LAM tagged pointers are supported by the system call to read memory.
Additionally, LAM applies only to addresses used for data accesses.
Thus, it is sufficient to mask addresses used for watchpoints.

The following examples are based on a LAM57 enabled program.
Before this patch tagged pointers were not supported for watchpoints:
~~~
(gdb) print pi_tagged
$2 = (int *) 0x10007ffffffffe004
(gdb) watch *pi_tagged
Hardware watchpoint 2: *pi_tagged
(gdb) c
Continuing.
Couldn't write debug register: Invalid argument.
~~~~

Once LAM 48 or LAM 57 is enabled for the current program, GDB can now
specify watchpoints for tagged addresses with LAM width 15 or 6,
respectively.

Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
2024-11-18 13:36:16 +00:00
Christina Schimpe
86bb38cee9 gdb: Make tagged pointer support configurable.
The gdbarch function gdbarch_remove_non_address_bits adjusts addresses to
enable debugging of programs with tagged pointers on Linux, for instance for
ARM's feature top byte ignore (TBI).
Once the function is implemented for an architecture, it adjusts addresses for
memory access, breakpoints and watchpoints.

Linear address masking (LAM) is Intel's (R) implementation of tagged
pointer support.  It requires certain adaptions to GDB's tagged pointer
support due to the following:
- LAM supports address tagging for data accesses only.  Thus, specifying
  breakpoints on tagged addresses is not a valid use case.
- In contrast to the implementation for ARM's TBI, the Linux kernel supports
  tagged pointers for memory access.

This patch makes GDB's tagged pointer support configurable such that it is
possible to enable the address adjustment for a specific feature only (e.g
memory access, breakpoints or watchpoints).  This way, one can make sure
that addresses are only adjusted when necessary.  In case of LAM, this
avoids unnecessary parsing of the /proc/<pid>/status file to get the
untag mask.

Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
(AArch64) Tested-By: Luis Machado <luis.machado@arm.com>
Approved-By: Luis Machado <luis.machado@arm.com>
2024-11-18 13:35:52 +00:00
Tom de Vries
c6f2bd9d10 [gdb/contrib] Fix spellcheck.sh for bash < 5.1
Since commit 5cb0406bb6 ("[gdb/contrib] Handle capitalized words in
spellcheck.sh"), spellcheck.sh uses '${pat@u}' which is available starting
bash 5.1, and consequently the script breaks with bash 4.4.

Fix this by checking for the bash version, and using an alternative
implementation for bash < 5.1.

Tested on x86_64-linux.
2024-11-18 11:42:44 +01:00
Tom de Vries
8b2ea4bcbc [gdb] Fix some typos
Run gdb/contrib/spellcheck.sh on directories gdb*.

Fix typo:
...
unkown -> unknown
...

Tested on x86_64-linux.
2024-11-18 09:46:31 +01:00
Tom de Vries
57e43f6ea5 [gdb/contrib] Add spellcheck.sh --print-dictionary
Add an option --print-dictionary to spellcheck.sh that allows us to inspect
the effective dictionary.

Verified with shellcheck.
2024-11-18 09:42:04 +01:00
Tom de Vries
92a5cfde2f [gdb/contrib] Allow thru in spellcheck.sh
Eli mentioned that "thru" is a widely-accepted shorthand [1].

Skip the "thru->through" rule by adding an overriding identity rule
"thru->thru".

Verified with shellcheck.

[1] https://sourceware.org/pipermail/gdb-patches/2024-November/213380.html
2024-11-18 09:42:03 +01:00
Ijaz, Abdul B
9dc89f2b27 gdb: Update linkage name lookup function to allow mst_file_data/bss types.
From the commit 667ed4b14d onward, instead
of normal name GDB looks for the "jit_descriptor" linkage name in the JIT
code initialization.  Without this change, the function
"lookup_minimal_symbol_linkage", only matches the non-static data.  So in
case jit_debugger is static type then setting up breakpoint in the JIT code
fails.  Issue is seen for the intel compilers, where jit_debug_descriptor has
static type i.e. "mst_file_data".  Hence lookup_minimal_symbol_linkage returns
nullptr for it.  So, in this case breakpoint does not hit in the JIT code.
To resolve this, the commit introduces a new boolean argument to the
lookup_minimal_symbol_linkage function.  This argument allows the function to
also match mst_file_data and mst_file_bss types when set to true.  The
function is called with this new argument set to true only from JIT code
initialization handling, ensuring that the current behavior remains unchanged
for other cases.  Because handling of static types of data symbols for all cases
result in regression for "gdb.base/print-file-var.exp" test.

Example of minsym for the JIT code emitted by the intel compilers where
lookup_minimal_symbol_linkage fails without this change because jit_debugger
type is "mst_file_data".

(top-gdb) p *msymbol
$1 = {<general_symbol_info> =
{m_name = 0x7fffcc77dc95 "__jit_debug_descriptor",
m_value = {ivalue = 84325936, block = 0x506b630,
bytes = 0x506b630 <error: Cannot access memory at address 0x506b630>,
address = 0x506b630, unrel_addr = (unknown: 0x506b630),
common_block = 0x506b630, chain = 0x506b630},
language_specific = {obstack = 0x0, demangled_name = 0x0},
m_language = language_unknown, ada_mangled = 0, m_section = 29},
m_size = 24, filename = 0x55555a751b70 "JITLoaderGDB.cpp",
m_type = mst_file_data, created_by_gdb = 0,
m_target_flag_1 = 0, m_target_flag_2 = 0, m_has_size = 1,
name_set = 1, hash_next = 0x55555b86e4f0, demangled_hash_next = 0x0}

Updated the test "jit-elf-so.exp" to test the static type of jit_descriptor
object.

Approved-By: Tom Tromey <tom@tromey.com>
2024-11-17 02:02:11 +01:00
Tom Tromey
2abed72b2c Use bool for solib::symbols_loaded
This changes solib::symbols_loaded to be of type 'bool'.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2024-11-16 08:45:57 -07:00
Tom de Vries
f80d7a791b [gdb/symtab] Fix segfault with dwp file
Consider the following test-case:
...
$ cat test.c
int main (void) { return 0; }
$ clang -g -gsplit-dwarf test.c -o test
$ llvm-dwp -e test -o test.dwp
...

This runs into a segmentation fault:
...
$ gdb -q -batch test
Fatal signal: Segmentation fault
...

The segmentation fault happens because in read_dwo_str_index this line sets p
to nullptr:
...
  const gdb_byte *p = reader->dwo_file->sections.str_offsets.buffer;
...
while the following code expects it to point to some data.

The section we're trying to read is:
...
(gdb) p reader->dwo_file->sections.str_offsets
$4 = {s = {section = 0xffffcc00a9d0, containing_section = 0xffffcc00a9d0},
  buffer = 0x0, size = 28, virtual_offset = 0, readin = false, is_virtual = true}
...

At first glance, the section is not readin, but actually it is.

This is a virtual section, meaning part of a containing section:
...
(gdb) p *reader->dwo_file->sections.str_offsets.s.containing_section
$8 = {s = {section = 0xffffcc00cde8, containing_section = 0xffffcc00cde8},
  buffer = 0xffffcc009650 "\030", size = 28, virtual_offset = 0, readin = true,
  is_virtual = false}
...
which is readin.

Fix this in create_dwp_v2_or_v5_section by initializing the buffer of the
virtual section using the buffer of the containing section:
...
  result.buffer = section->buffer + offset;
...

Unfortunately it's difficult to write a test-case for this.  We'll have to
teach the dwarf assembler to generate dwp files.

Tested on aarch64-linux.

This is a partial fix for PR symtab/31497.

Approved-By: Tom Tromey <tom@tromey.com>

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31497
2024-11-15 22:48:37 +01:00
Tom Tromey
a7d1f26ec3 Improvements to gdb.LazyString documentation
I noticed the gdb.LazyString documentation did not mention how to
create one.  Then, while adding this, I found a couple other ways that
this documentation could be clarified.

Approved-By: Eli Zaretskii <eliz@gnu.org>
2024-11-15 13:14:37 -07:00
Andrew Burgess
82eff6743b gdb/testsuite: skip gdb.opt/inline-entry.exp for gcc 7 and older
It was pointed out that the recently added gdb.opt/inline-entry.exp
test would fail when run using gcc 7 and earlier, on an x86-64 target:

  https://inbox.sourceware.org/gdb-patches/9fe35ea1-d99b-444d-bd1b-e3a1f108dd77@suse.de

Bernd Edlinger points out that, for gcc, the test relies on the
-gstatement-frontiers work which was added in gcc 8.x:

  https://inbox.sourceware.org/gdb-patches/DU2PR08MB10263357597688D9D66EA745CE4242@DU2PR08MB10263.eurprd08.prod.outlook.com

For gcc 7.x and older, without the -gstatement-frontiers work, the
compiler uses DW_AT_entry_pc differently, which leads to a poorer
debug experience.

Here is the interesting source line from inline-entry.c:

  if ((global && bar (1)) || bar (2))

And here's some of the relevant disassembly output:

  Dump of assembler code for function main:
     0x401020 <+0>:	mov    0x3006(%rip),%eax	(1)
     0x401026 <+6>:	test   %eax,%eax		(2)
     0x401028 <+8>:	mov    0x2ffe(%rip),%eax	(3)
     0x40102e <+14>:	je     0x401038 <main+24>	(4)
     0x401030 <+16>:	sub    $0x1,%eax		(5)
     0x401033 <+19>:	jne    0x40103d <main+29>	(6)

Lines (1), (2), and (4) represent the check of 'global'.  However,
line (3) is actually the first instruction for 'bar' which has been
inlined.  Lines (5) and (6) are also part of the first inlined 'bar'
function.

If the check of 'global' returns false then the first call to 'bar'
should never happen, this is accomplished by the branch at (4) being
taken.

For gcc 8+, gcc generates a DW_AT_entry_pc with the value 0x401030,
this is where GDB places a breakpoint for 'bar', and this address is
after the branch at line (4), and so, if the call to 'bar' never
happens, the breakpoint is never hit.

For gcc 7 and older, gcc generates a DW_AT_entry_pc with the value
0x401028, which is the first address associated with the inline 'bar'
function.  Unfortunately, this address is also before the check of
'global' has completed, this means that GDB hits the 'bar' breakpoint
before the inferior has decided if 'bar' should actually be called or
not.

I don't think there's really much GDB can do in the older gcc
versions, we are placing the breakpoint at the entry point, and this
is within bar.  Given that this test does really depend on the newer
gcc behaviour, I think the only sensible solution is to skip this test
when an older version of gcc is being used.

I've incorporated the check for -gstatement-frontiers support that
Bernd suggested and now the test will be skipped for older versions of
GCC.

Approved-By: Tom de Vries <tdevries@suse.de>
2024-11-15 19:22:13 +00:00
Andrew Burgess
8518ce5fc2 gdb/python: missing PyObject_IsTrue error check in bppy_init
As with the previous two commits, this commit fixes a location where
we called PyObject_IsTrue without including an error check, this time
in bppy_init.

The 'qualified' argument is supposed to be a bool, the docs say:

  The optional QUALIFIED argument is a boolean that allows
  interpreting the function passed in 'spec' as a fully-qualified
  name.  It is equivalent to 'break''s '-qualified' flag (*note
  Linespec Locations:: and *note Explicit Locations::).

It's not totally clear that the only valid values are True or False,
but I'm choosing to interpret the docs that way, and so I've added a
PyBool_Type check during argument parsing.  Now, if a non-bool is
passed the user will get a TypeError during argument parsing.  I've
added a test to cover this case.

This is a potentially breaking change to the Python API, but hopefully
this will not impact too many people.  I've added a NEWS entry to
highlight this change.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
2024-11-14 19:34:44 +00:00
Andrew Burgess
ad39b4aae8 gdb/python: missing PyObject_IsTrue error check in micmdpy_set_installed
Like the previous commit, I discovered that in micmdpy_set_installed
we were calling PyObject_IsTrue, but not checking for a possible error
value being returned.

The micmdpy_set_installed function implements the
gdb.MICommand.installed attribute, and the documentation indicates that
this attribute should only be assigned a bool:

  This attribute is read-write, setting this attribute to 'False'
  will uninstall the command, removing it from the set of available
  commands.  Setting this attribute to 'True' will install the
  command for use.

So I propose that instead of using PyObject_IsTrue we use
PyBool_Check, and if the new value fails this check we raise an
error.  We can then compare the new value to Py_True directly instead
of calling PyObject_IsTrue.

This is a potentially breaking change to the Python API, but hopefully
this will not impact too many people, and the fix is pretty
easy (switch to using a bool).  I've added a NEWS entry to draw
attention to this change.

Approved-By: Tom Tromey <tom@tromey.com>
2024-11-14 19:34:44 +00:00
Andrew Burgess
5209b83f53 gdb/python: missing PyObject_IsTrue error check in py-arch.c
Building on the previous two commits, I was auditing our uses of
PyObject_IsTrue looking for places where we were missing an error
check.

The gdb.Architecture.integer_type() function takes a 'signed' argument
which should be a 'bool', and the docs do say:

  If SIGNED is not specified, it defaults to 'True'.  If SIGNED is
  'False', the returned type will be unsigned.

Currently we use PyObject_IsTrue, but we are missing an error check.

To fix this I've tightened the code to enforce the bool requirement at
the point that the arguments are parsed.  With that done I can remove
the call to PyObject_IsTrue and instead compare to Py_True directly,
the object in question will always be a PyBool_Type.

However, we were testing that passing a non-bool argument for 'signed'
is treated as Py_False, this was added with this commit:

  commit 90fe61ced1
  Date:   Mon Nov 29 13:53:06 2021 +0000

      gdb/python: don't use the 'p' format for parsing args

which is when the PyObject_IsTrue call was added.  Given that the docs
do seem pretty clear that only True or False are suitable argument
values, my proposal is that we just remove these tests and instead
test that any non-bool argument value for 'signed' gives a TypeError.

This is a breaking change to the Python API, however, my hope is that
this is such a edge case that it will not cause too many problem.
I've added a NEWS entry to highlight this change though.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
2024-11-14 19:34:43 +00:00
Andrew Burgess
d8a2c719da gdb/python: remove some additional PyObject_IsTrue calls
After the previous commit I audited all our uses of PyObject_IsTrue
looking for places where we were missing an error check.  I did find
some that are missing error checks in places where we really should
have error checks, and I'll fix those in later commits.

This commit however, focuses on those locations where PyObject_IsTrue
is called, there is no error check, and the error check isn't really
necessary because we already know that the object we are dealing with
is of type PyBool_Type.

Inline with the previous commit, in these cases I have removed the
PyObject_IsTrue call, and replaced it with a comparison against
Py_True.  In one location where it is not obvious that the object we
have is PyBool_Type I've added an assert, but in the other cases the
comparison to Py_True immediately follows a PyBool_Check call, so an
assert would be redundant.

I've added a test for the gdb.Value.format_string styling argument
being passed a non-bool value as this wasn't previously being tested,
though this new test will pass before and after this commit.

There should be no functional change after this commit.

Approved-By: Tom Tromey <tom@tromey.com>
2024-11-14 19:34:43 +00:00
Andrew Burgess
dcf4c48453 gdb/python: remove PyObject_IsTrue call in gdbpy_handle_missing_debuginfo
In this review:

  https://inbox.sourceware.org/gdb-patches/87wmirfzih.fsf@tromey.com

Tom pointed out that using PyObject_IsTrue as I was doing, though
technically fine, at least appears to be missing an error check, and
that it would be better to compare to Py_True directly.  I made that
change in the patch Tom was commenting on, but I'd actually copied
that code from elsewhere in python/python.c, so this commit updates
the original code inline with Tom's review feedback.

There should be no functional change after this commit.

Approved-By: Tom Tromey <tom@tromey.com>
2024-11-14 19:34:43 +00:00
Tom de Vries
580f291cb8 [gdb/tdep] Handle syscall clock_gettime64 for arm-linux
When running test-case gdb.reverse/time-reverse.exp on arm-linux, I run into:
...
(gdb) continue^M
Continuing.^M
Process record and replay target doesn't support syscall number 403^M
Process record does not support instruction 0xdf00 at address 0xf7ebf774.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
0xf7ebf774 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
(gdb) FAIL: $exp: mode=c: continue to breakpoint: marker2
...

Syscall number 403 stands for clock_gettime64 on arm-linux.

Fix this by handling 403 in arm_canonicalize_syscall, and handling
gdb_sys_clock_gettime64 elsewhere.

Since i386_canonicalize_syscall is the identity function, enum value
gdb_sys_clock_gettime64 gets a value to match i386, which also happens to be
403.

Tested on arm-linux.

Approved-By: Guinevere Larsen <guinevere@redhat.com> (record-full)
2024-11-13 22:41:35 +01:00
Tom de Vries
5cb0406bb6 [gdb/contrib] Handle capitalized words in spellcheck.sh
The dictionary contains a few entries with capital letters:
...
$ grep -E '[A-Z]' .git/wikipedia-common-misspellings.txt | wc -l
143
...
but they don't look too interesting in the gdb context (for instance,
Habsbourg->Habsburg), so filter them out.

That leaves us with entries looking only like "foobat->foobar", so add
handling of capitalized words, such that we also rewrite "Foobat" to "Foobar".

Tested on aarch64-linux.  Verified with shellcheck.

Approved-by: Kevin Buettner <kevinb@redhat.com>
2024-11-13 22:38:19 +01:00
Tom de Vries
74b9033e6f [gdb/contrib] Add "doens't->doesn't" to common-misspellings.txt
Add "doens't->doesn't" to gdb/contrib/common-misspellings.txt, and run
gdb/contrib/spellcheck.sh to fix this in a few files.

Tested on x86_64-linux.

Approved-by: Kevin Buettner <kevinb@redhat.com>
2024-11-13 21:06:58 +01:00
Tom de Vries
ee126995f5 [gdb/contrib] Handle double quotes in spellcheck.sh
Add handling of double quotes in gdb/contrib/spellcheck.sh, and fix the
following typos:
...
inheritence -> inheritance
psuedo -> pseudo
succeded -> succeeded
...

Tested on x86_64-linux.

Approved-by: Kevin Buettner <kevinb@redhat.com>
2024-11-13 21:06:58 +01:00
Tom de Vries
0d1f7e2765 [gdb/contrib] Handle parentheses in spellcheck.sh
Currently, text adjacent to parentheses is not spell-checked:
...
$ cat tmp.txt
(upto)
$ ./gdb/contrib/spellcheck.sh tmp.txt
$
...

Add handling of parentheses, such that we get:
...
$ ./gdb/contrib/spellcheck.sh tmp.txt
upto -> up to
$
...

Rerun spellcheck.sh, resulting in a few "thru->through" replacements.

Tested on x86_64-linux.

Approved-by: Kevin Buettner <kevinb@redhat.com>
2024-11-13 21:06:58 +01:00
Tom de Vries
6e4577fc29 [gdb/tdep] Fix recording of T1 push
When running test-case gdb.reverse/recursion.exp on arm-linux with target
board unix/-mthumb, I run into:
...
(gdb) PASS: gdb.reverse/recursion.exp: Skipping recursion from inside
reverse-next^M
bar (x=4195569) at /home/linux/gdb/src/gdb/testsuite/gdb.reverse/recursion.c:34^M
34        int r = foo (x);^M
(gdb) FAIL: gdb.reverse/recursion.exp: print frame when stepping out
...

The problem is the recording of the T1 push instruction [1,2], specifically:
...
000004d8 <foo>:
 4d8:   b580            push    {r7, lr}
...

The current code fails to add a memory record for the memory written with the
value of the lr register.

Fix this by adding the missing memory record.

Tested on arm-linux.

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Luis Machado <luis.machado@arm.com>

[1] https://developer.arm.com/documentation/ddi0406/c/Application-Level-Architecture/Instruction-Details/Encoding-of-lists-of-ARM-core-registers
[2] https://developer.arm.com/documentation/ddi0597/2024-09/T32-Instructions-by-Encoding/16-bit?lang=en#pushpop16
2024-11-13 19:44:21 +01:00
Tom de Vries
76b9b6eb9c [gdb/tdep] Handle sycall statx for arm-linux
When running test-case gdb.reverse/fstatat-reverse.exp on arm-linux, I run
into:
...
(gdb) continue^M
Continuing.^M
Process record and replay target doesn't support syscall number 397^M
Process record does not support instruction 0xdf00 at address 0xf7ebf774.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
0xf7ebf774 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
(gdb) FAIL: gdb.reverse/fstatat-reverse.exp: continue to breakpoint: marker2
...

Syscall number 397 stands for statx on arm-linux.

Fix this by handling 397 in arm_canonicalize_syscall.

Tested on arm-linux.

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Luis Machado <luis.machado@arm.com>
2024-11-13 19:37:04 +01:00
Bernd Edlinger
5d9887ffa2 gdb: stepping between inline functions with multiple ranges
I (Andrew) have split this small change from a larger patch which was
posted here:

  https://inbox.sourceware.org/gdb-patches/AS1PR01MB9465608EBD5D62642C51C428E4922@AS1PR01MB9465.eurprd01.prod.exchangelabs.com

And I have written the stand alone test for this issue.  The original
patch included this paragraph to explain this change (I've fixed one
typo in this text replacing 'program' with 'function'):

  ... it may happen that the infrun machinery steps from one inline
  range to another inline range of the same inline function.  That can
  look like jumping back and forth from the calling function to the
  inline function, while really the inline function just jumps from a
  hot to a cold section of the code, i.e. error handling.

The important thing that happens here is that both the outer function
and the inline function must both have multiple ranges.  When the
inferior is within the inline function and moves from one range to
another it is critical that the address we stop at is the start of a
range in both the outer function and the inline function.

The diagram below represents how the functions are split and aligned:

                           (A)       (B)
  bar:         |------------|         |---|
  foo:   |------------------|         |--------|

The inferior is stepping through 'bar' and eventually reaches
point (A) at which point control passes to point (B).

Currently, when the inferior stops, GDB notices that both 'foo' and
'bar' start at address (B), and so GDB uses the inline frame mechanism
to skip 'bar' and tells the user that the inferior is in 'foo'.

However, as we were in 'bar' before the step then it makes sense that
we should be in 'bar' after the step, and this is what the patch does.

There are two tests using the DWARF assembler, the first checks the
above situation and ensures that GDB reports 'bar' after the step.

The second test is similar, but after the step we enter a new range
where a different inline function starts, something like this:

                           (A)       (B)
  bar:         |------------|
  baz:                                |---|
  foo:   |------------------|         |--------|

In this case as we step at (A) and land at (B) we leave 'bar' and
expect to stop in 'foo', GDB shouldn't automatically enter 'baz' as
that is a completely different inline function.  And this is, indeed,
what we see.

Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
2024-11-13 13:50:21 +00:00
Andrew Burgess
b9de07a5ff gdb: fix handling of DW_AT_entry_pc of inlined subroutines
The entry PC for a DIE, e.g. an inline function, might not be the base
address of the DIE.  Currently though, in block::entry_pc(), GDB
always returns the base address (low-pc or the first address of the
first range) as the entry PC.

This commit extends the block class to carry the entry PC as a
separate member variable.  Then the DWARF reader is extended to read
and set the entry PC for the block.  Now in block::entry_pc(), if the
entry PC has been set, this is the value returned.

If the entry-pc has not been set to a specific value then the old
behaviour of block::entry_pc() remains, GDB will use the block's base
address.  Not every DIE will set the entry-pc, but GDB still needs to
have an entry-pc for every block, so the existing logic supplies the
entry-pc for any block where the entry-pc was not set.

The DWARF-5 spec for reading the entry PC is a super-set of the spec
as found in DWARF-4.  For example, if there is no DW_AT_entry_pc then
DWARF-4 says to use DW_AT_low_pc while DWARF-5 says to use the base
address, which is DW_AT_low_pc or the first address in the first range
specified by DW_AT_ranges if there is no DW_AT_low_pc.

I have taken the approach of just implementing the DWARF-5 spec for
everyone.  There doesn't seem to be any benefit to deliberately
ignoring a ranges based entry PC value for DWARF-4.  If some naughty
compiler has emitted that, then lets use it.

Similarly, DWARF-4 says that DW_AT_entry_pc is an address.  DWARF-5
allows an address or a constant, where the constant is an offset from
the base address.  I allow both approaches for all DWARF versions.
There doesn't seem to be any downsides to this approach.

I ran into an issue when testing this patch where GCC would have the
DW_AT_entry_pc point to an empty range.  When GDB parses the ranges
any empty ranges are ignored.  As a consequence, the entry-pc appears
to be outside the address range of a block.

The empty range problem is certainly something that we can, and should
address, but that is not the focus of this patch, so for now I'm
ignoring that problem.  What I have done is added a check: if the
DW_AT_entry_pc is outside the range of a block then the entry-pc is
ignored, GDB will then fall-back to its default algorithm for
computing the entry-pc.

If/when in the future we address the empty range problem, these
DW_AT_entry_pc attributes will suddenly become valid and GDB will
start using them.  Until then, GDB continues to operate as it always
has.

An early version of this patch stored the entry-pc within the block
like this:

  std::optional<CORE_ADDR> m_entry_pc;

However, a concern was raised that this, on a 64-bit host, effectively
increases the size of block by 16-bytes (8-bytes for the CORE_ADDR,
and 8-bytes for the std::optional's bool plus padding).

If we remove the std::optional part and just use a CORE_ADDR then we
need to have a "special" address to indicate if m_entry_pc is in use
or not.  I don't really like using special addresses; different
targets can access different address ranges, even zero is a valid
address on some targets.

However, Bernd Edlinger suggested storing the entry-pc as an offset,
and I think that will resolve my concerns.  So, we store the entry-pc
as a signed offset from the block's base address (the first address of
the first range, or the start() address value if there are now
ranges).  Remember, ranges can be out of order, in which case the
first address of the first range might be greater than the entry-pc.

When GDB needs to read the entry-pc we can add the offset onto the
blocks base address to recalculate it.

With this done, on a 64-bit host, block only needs to increase by
8-bytes.

The inline-entry.exp test was originally contributed by Bernd here:

  https://inbox.sourceware.org/gdb-patches/AS1PR01MB94659E4D9B3F4A6006CC605FE4922@AS1PR01MB9465.eurprd01.prod.exchangelabs.com

though I have made some edits, making more use of lib/gdb.exp
functions, making the gdb_test output patterns a little tighter, and
updating the test to run with Clang.  I also moved the test to
gdb.opt/ as that seemed like a better home for it.

Co-Authored-By: Bernd Edlinger <bernd.edlinger@hotmail.de>
2024-11-13 13:41:27 +00:00
Andrew Burgess
06b8b0ad97 gdb/testsuite: some cleanups in gdb.base/annota{1,3}.exp tests
Fedora GDB has, for years, carried an out of tree patch for the
gdb.base/annota{1,3}.exp tests.  The patch simply adds a call to 'set
breakpoint pending off' near the start of each test.

Normally GDB tests are written using gdb_test or gdb_test_multiple,
with gdb_test being a wrapper around gdb_test_multiple.  Inside
gdb_test_multiple we add a test pattern which detects if GDB offers
the user an interactive yes/no prompt and immediately fails the test.

What this means is that if something goes wrong with a test like:

  gdb_test "break main" ".*"

and GDB ends up offering this prompt:

  Make breakpoint pending on future shared library load? (y or [n])

then instead of hanging waiting for the test to timeout, DejaGNU will
spot the interactive prompt and immediately fail the test.

In the gdb.base/annota{1,3}.exp tests we turn on GDB's annotation
mode, and many of the tests in these scripts are written using
send_gdb and gdb_expect or gdb_expect_list.  This is done because in
the past, gdb_test_multiple and friends were hard-coded to use the
"normal" GDB prompt, and these tests instead expect the annotated
prompt.  Specifically, gdb_test_multiple expects '$gdb_prompt $' as
the full prompt, that is the value of $gdb_prompt followed by a single
white space.  The annotation tests do update the value of $gdb_prompt,
but with annotations on there is no trailing whitespace, so
gdb_test_multiple's default behaviour doesn't work, which is why the
test scripts originally avoided using gdb_test_multiple.

As a result none of the tests in these files would early exit if we
got an interactive yes/no prompt, and instead we'd be relying on each
test to timeout, which could take a while.

However, gdb_test_multiple now accepts a -prompt argument, so we can
modify the prompt we are looking for, which is neat.

So, I started off by going through these tests an changing all of the
tests that create a breakpoint to use gdb_test, passing the -prompt
option to change the prompt.

While doing that I noticed some other things that I could improve in
these tests, I've cleaned up the use of standard_testfile, switched to
use prepare_for_testing, and removed some redundant clean_restart and
'set height 0' calls (set height 0 is done within clean_restart, which
is called by prepare_for_testing).

I've updated some comments which said "we can't use gdb_test" to say
"we can use gdb_test with -prompt option", and I've removed some
commented out debug code (e.g. setting 'exp_internal').

Finally, I ran these two tests through check-all-boards, and spotted
that annota1.exp failed when using a remote host.  This is because one
test checks for a full path to the binary in some output, and with a
remote host the binary ends up being copied and the path is not as
expected.

I'm assuming that checking the full path is important, so I've
disabled this test on remote host boards.

After all these changes I checked using 'make check-all-boards' and
there are no unexpected results on either of these tests.

Tested-By: Tom de Vries <tdevries@suse.de>
Acked-By: Tom de Vries <tdevries@suse.de>
2024-11-12 14:41:23 +00:00
Andrew Burgess
26885b2b6c gdb/testsuite: fix duplicate test names in gdb.trace/
After this commit:

  commit 35f09cd5d7
  Date:   Wed Jul 31 15:04:25 2024 +0200

      [gdb/testsuite] Detect trailing-text-in-parentheses duplicates

we are now seeing some duplicate test names in gdb.trace/ tests when
using native-gdbserver or native-extended-gdbserver boards.  This is
all due to tests that use some text in trailing parenthesis to make
the test name unique.

I've gone through and edited the test names as best I could to make
them all unique.  Hopefully the updated test names should all make
sense.

On my machine I'm no longer seeing any duplicates with either of the
boards listed above.

Acked-By: Tom de Vries <tdevries@suse.de>
2024-11-12 14:35:38 +00:00
Andrew Burgess
31fa6221bf gdb/readline: don't get stuck thinking an EOF arrived
It was brought to my attention[1] that if a user makes use of Ctrl+d
to quit from a secondary prompt (e.g. the prompt used to enter lines
for the 'commands' command) then GDB will start displaying some
unexpected blank lines.  Here's an example:

  Reading symbols from /tmp/hello.x...
  (gdb) break main
  Breakpoint 1 at 0x401198: file hello.c, line 18.
  (gdb) commands
  Type commands for breakpoint(s) 1, one per line.
  End with a line saying just "end".
  >quit			# <----------- Use Ctrl+d to quit here.
  (gdb) show architecture
			# <----------- This blank line is unexpected.
  The target architecture is set to "auto" (currently "i386:x86-64").
  (gdb)

I've marked up where I press 'Ctrl+d', and also the unexpected blank
line.

This issue will only happen if bracketed-paste-mode is in use.  If
this has been disabled (e.g. in ~/.inputrc) then this issue will not
occur.

The blank line is not just emitted for the 'show architecture'
command.  The blank line is actually caused by an extra '\n' character
emitted by readline after it has gathered a complete line of input,
and so will occur for any command.

The problem is caused by readline getting "stuck" in a state where it
thinks that an EOF has just been delivered.  This state is set when
the 'Ctrl+d' does deliver an EOF, but then this state is never fully
reset.  As a result, every time readline completes a line, it thinks
that the line was completed due to an EOF and so adds an extra '\n'
character.

Obviously the real fix for this issue is to patch readline, and I do
have a patch for that[2], however, version 8.2 of readline has been
released, and contains this issue.  As such, if GDB is linked against
the system readline, and that system readline is 8.2, then we can
expect to see this issue.

There's a pretty simple, and cheap workaround that we can add to GDB
that will mitigate this issue.

I propose that we add this workaround to GDB.  If/when the readline
patch is accepted then I'll back-port this to our local readline copy,
but retaining the workaround will be harmless, and will make GDB play
nicer with system readline libraries (version 8.2).

[1] https://inbox.sourceware.org/gdb-patches/34ef5438-8644-44cd-8537-5068e0e5e434@redhat.com
[2] https://lists.gnu.org/archive/html/bug-readline/2024-10/msg00014.html

Reviewed-By: Keith Seitz <keiths@redhat.com>
2024-11-12 14:07:35 +00:00
Andrew Burgess
b0c6153ccf gdb/readline: add readline library version to 'show configuration'
When debugging readline issues I'd like an easy way to know (for sure)
what version of readline GDB is using.  This could also be useful when
writing readline tests, knowing the precise readline version will
allow us to know if we expect a test to pass or not.

Add the readline library version to the output of the 'show
configuration' command.  Also include a suffix indicating if we are
using the system readline, or the statically linked in readline.

The information about static readline vs shared readline can be
figured out from the configure command output, but having it repeated
in the readline version line makes it super easy to grok within tests,
and it's super cheap, so I don't see this as a problem.
2024-11-12 13:58:19 +00:00
Andrew Burgess
1048062a3f gdbserver: pass osabi to GDB in more target descriptions
Problem Description
-------------------

On a Windows machine I built gdbserver, configured for the target
'x86_64-w64-mingw32', then on a GNU/Linux machine I built GDB with
support for all target (--enable-targets=all).

On the Windows machine I start gdbserver with a small test binary:

  $ gdbserver 192.168.129.25:54321 C:\some\directory\executable.exe

On the GNU/Linux machine I start GDB without the test binary, and
connect to gdbserver.

As I have not given GDB the test binary, my expectation is that GDB
would connect to gdbserver and then download the file over the remote
protocol, but instead I was presented with this message:

  (gdb) target remote 192.168.129.25:54321
  Remote debugging using 192.168.129.25:54321
  warning: C:\some\directory\executable.exe: No such file or directory.
  0x00007ffa3e1e1741 in ?? ()
  (gdb)

What I found is that if I told GDB where to find the binary, like
this:

  (gdb) file target:C:/some/directory/executable.exe
  A program is being debugged already.
  Are you sure you want to change the file? (y or n) y
  Reading C:/some/directory/executable.exe from remote target...
  warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
  Reading C:/some/directory/executable.exe from remote target...
  Reading symbols from target:C:/some/directory/executable.exe...
  (gdb)

then GDB would download the executable.

The Actual Issue
----------------

I tracked the problem down to exec_file_find (solib.c).  The remote
target was passing an absolute Windows filename (beginning with "C:/"
in this case), but in exec_file_find GDB was failing the
IS_TARGET_ABSOLUTE_PATH call, and so was treating the filename as
relative.

The IS_TARGET_ABSOLUTE_PATH call was failing because GDB thought that
the file system kind was "unix", and as the filename didn't start with
a "/" it assumed the filename was not absolute.

But I'm connecting to a Windows target and 'target-file-system-kind'
was set to "auto", so GDB should be figuring out that the target
file-system is "dos-based".

Looking in effective_target_file_system_kind (filesystem.c), we find
that the logic of "auto" is delegated to the current gdbarch.  However
in windows-tdep.c we see:

  set_gdbarch_has_dos_based_file_system (gdbarch, 1);

So if we are using a Windows gdbarch we should have "dos-based"
filesystems.  What this means is that after connecting to the remote
target GDB has selected the wrong gdbarch.

What's happening is that the target description sent back by the
remote target only includes the x86-64 registers.  There's no
information about which OS we're on.  As a consequence, GDB picks the
first x86-64 gdbarch which can handle the provided register set, which
happens to be a GNU/Linux gdbarch.

And indeed, there doesn't appear to be anywhere in gdbserver that sets
the osabi on the target descriptions. Some target descriptions do have
their osabi set when the description is created, e.g. in:

  gdb/arch/amd64.c	- Sets GNU/Linux osabi when appropriate.
  gdb/arch/i386.c	- Likewise.
  gdb/arch/tic6x.c	- Always set GNU/Linux osabi.

There are also some cases in gdb/features/*.c where the tdesc is set,
but these locations are only called from GDB, not from gdbserver.

This means that many target descriptions are created without an osabi,
gdbserver does nothing to fix this, and the description is returned to
GDB without an osabi included.  This leaves GDB having to guess what
the target osabi is, and in some cases, GDB can get this wrong.

Proposed Solution
-----------------

I propose to change init_target_desc so that it requires an gdb_osabi
to be passed in, this will then be used to set the target_desc osabi
field.

I believe that within gdbserver init_target_desc is called for every
target_desc, so this should mean that every target_desc has an
opportunity to set the osabi to something sane.

I did consider passing the osabi into the code which creates the
target_desc objects, but that would require updating far more code, as
each target has its own code for creating target descriptions.
The approach taken here requires minimal changes and forces every
user of init_target_desc to think about what the correct osabi is.

In some cases, e.g. amd64, where the osabi is already set when the
target_desc is created, the init_target_desc call will override the
current value, however, we should always be replacing it with the same
actual value.  i.e. if the target_desc is created with the osabi set
to GNU/Linux, then this should only happen when gdbserver is built for
GNU/Linux, in which case the init_target_desc should also be setting
the osabi to GNU/Linux.

The Tricky Bits
---------------

Some targets, like amd64, use a features based approach for creating
target_desc objects, there's a function in arch/amd64.c which creates
a target_desc, adds features too it, and returns the new target_desc.
This target_desc is then passed to an init_target_desc call within
gdbserver.  This is the easy case to handle.

Then there are other targets which instead have a fixed set of xml
files, each of which is converted into a .dat file, which is then used
to generate a .cc file, which is compiled into gdbserver.  The
generated .cc file creates the target_desc object and calls
init_target_desc on it.  In this case though the target description
that is sent to GDB isn't generated from the target_desc object, but
is instead the contents of the fixed xml file.  For this case the
osabi which we pass to init_target_desc should match the osabi that
exists in the fixed xml file.

Luckily, in the previous commit I copied the osabi information from
the fixed xml files into the .dat files.  So in this commit I have
extended regdat.sh to read the osabi from the .dat file and use it in
the generated init_target_desc call.

The problem with some of these .dat base targets is that their fixed
xml files don't currently contain any osabi information, and the file
names don't indicate that they are Linux only (despite them currently
only being used from gdbserver for Linux targets), so I don't
currently feel confident adding any osabi information to these files.
An example would be features/rs6000/powerpc-64.xml.  For now I've just
ignored these cases.  The init_target_desc will use GDB_OSABI_UNKNOWN
which is the default.  This means that for these targets nothing
changes from the current behaviour.  But many other targets do now
pass the osabi back.  Targets that do pass the osabi back are
improved with this commit.

Conclusion
----------

Now when I connect to the Windows remote the target description
returned includes the osabi name.  With this extra information GDB
selects the correct gdbarch object, which means that GDB understands
the target has a "dos-based" file-system.  With that correct GDB
understands that the filename it was given is absolute, and so fetches
the file from the remote as we'd like.

Reviewed-By: Kevin Buettner <kevinb@redhat.com>
2024-11-12 12:51:36 +00:00
Andrew Burgess
06e9986d02 gdb/regformats: add osabi information to generated .dat files
Some gdbserver targets generate their target description based on the
gdb/regformats/*.dat files.  These .dat files are generated from a
matching xml file in gdb/features/.

Lets consider a concrete example:

Take gdb/features/or1k-linux.xml, this file is processed by
gdb/features/Makefile to create gdb/regformats/or1k-linux.dat.

When gdbserver is built for the or1k target the file
or1k-linux-generated.cc is generated using the
gdb/regformats/regdat.sh script.  This .cc file is then compiled and
linked into gdbserver.

The or1k-linux-generated.cc file contains the function
init_registers_or1k_linux which is called from within gdbserver, this
function creates a target_desc object and sets its xmltarget field to
a fixed string.  This fixed string is the xml filename that was
originally used to generate the xml file, in this case or1k-linux.xml.

Additionally, as part of the gdbserver build the file or1k-linux.xml
is converted to a string and placed in the file
xml-builtin-generated.cc which is then built into gdbserver.

Now when GDB asks gdbserver for the target description, gdbserver
returns the fixed xmltarget string, which is the name of an xml file.
GDB will then ask gdbserver for that file and gdbserver will return
the contents of that file thanks to the xml-builtin-generated.cc
file's contents.

This is all rather complicated, but it does work.  So what's the
problem that I'm fixing?

Well or1k-linux.xml does contain the osabi information, so this will
be returned from gdbserver to GDB.  That's good.

However, the target_desc object created in init_registers_or1k_linux
will not have its osabi set correctly.

Now this doesn't really matter too much except
init_registers_or1k_linux includes a call to init_target_desc.

In the next commit I want to extend init_target_desc to require an
osabi to be passed in.  The motivation for this will be explained in
the next commit, but if we accept for a moment that this is something
that should be done, then the question is what osabi should we use in
init_registers_or1k_linux?

Ideally we'd use the osabi which is set in or1k-linux.xml.  If we do
that then everything will remain consistent, which is a good thing.

And so, to get the osabi from or1k-linux.xml into
init_registers_or1k_linux, we first need to get the osabi information
into or1k-linux.dat file, and this is what this commit does.

I've added a new xsl script print-osabi.xsl and updated
gdb/features/Makefile to make use of this script.  Then I regenerated
all of the .dat files.  Now every .dat file contains either:

  osabi:GNU/Linux
  osabi:unknown

The first is for xml files containing <osabi>GNU/Linux</osabi> and the
second is for xml files that don't contain an osabi element.

This commit doesn't attempt to make use of the osabi information in
the .dat files, that will come in the next commit.  There should be no
user visible changes after this commit.

Approved-By: Kevin Buettner <kevinb@redhat.com>
2024-11-12 12:51:36 +00:00