Tom Tromey [Wed, 12 Jun 2024 16:58:49 +0000 (18:58 +0200)]
[gdb/python] Add typesafe wrapper around PyObject_CallMethod
In gdb/python/py-tui.c we have code like this:
...
gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll",
"i", num_to_scroll, nullptr));
...
The nullptr is superfluous, the format string already indicates that there's
only one method argument.
OTOH, passing no method args does use a nullptr:
...
gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render",
nullptr));
...
Furthermore, choosing the right format string chars can be tricky.
Add a typesafe wrapper around PyObject_CallMethod that hides these
details, such that we can use the more intuitive:
...
gdbpy_ref<> result (gdbpy_call_method (m_window.get(), "hscroll",
num_to_scroll));
...
and:
...
gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "render"));
...
Tested on x86_64-linux.
Co-Authored-By: Tom de Vries <tdevries@suse.de> Approved-By: Tom Tromey <tom@tromey.com>
aarch64: add Branch Record Buffer extension instructions
The FEAT_BRBE extension provides two aliases of sys:
- brb iall (Invalidates all Branch records in the Branch Record Buffer)
- brb inj (Injects the Branch Record held in BRBINFINJ_EL1,
BRBSRCINJ_EL1, and BRBTGTINJ_EL1 into the Branch Record Buffer)
This patch adds:
- the feature option "brbe" that must be added for the aliases to be available
- a new operand flag AARCH64_OPND_Rt_IN_SYS_ALIASES that warns in a comment
when Rt is set to the non default value 0b11111 (it is constrained
unpredictable whether the instruction is undefined or behaves as if the Rt
field is set to 0b11111).
- a new operand flag AARCH64_OPND_BRBOP that encodes and decodes Op2 values
from bit 5
- support for the two brb aliases above
Hannes Domani [Wed, 12 Jun 2024 13:24:02 +0000 (15:24 +0200)]
Allow calling of user-defined function call operators
Currently it's not possible to call user-defined function call
operators, at least not without specifying operator() directly:
```
(gdb) l 1
1 struct S {
2 int operator() (int x) { return x + 5; }
3 };
4
5 int main () {
6 S s;
7
8 return s(23);
9 }
(gdb) p s(10)
Invalid data type for function to be called.
(gdb) p s.operator()(10)
$1 = 15
```
This now looks if an user-defined call operator is available when
trying to 'call' a struct value, and calls it instead, making this
possible:
```
(gdb) p s(10)
$1 = 15
```
The change in operation::evaluate_funcall is to make sure the type
fields are only used for function types, only they use them as the
argument types.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213 Approved-By: Tom Tromey <tom@tromey.com>
Add a new 'error_message' feature to the qSupported packet. When GDB
supports this feature then gdbserver is able to send
errors in the E.errtext format for the qRcmd and m packets.
Update qRcmd packet and m packets documentation as qRcmd newly
accepts errors in a E.errtext format.
Previously these two packets didn't support E.errtext style errors.
Approved-By: Tom Tromey <tom@tromey.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
Andrew Burgess [Tue, 21 May 2024 14:38:23 +0000 (15:38 +0100)]
gdb: warn of slow remote file reading only after a successful open
While working on a later patch in this series, I noticed that GDB
would print the message:
Reading /path/to/file from remote target...
Even when /path/to/file doesn't exist on the remote target.
GDB does indeed try to open /path/to/file, but I'm not sure we really
need to tell the user unless we actually manage to open the file, and
plan to read content from it.
If we consider how GDB probes for separate debug files, we can attempt
to open multiple possible files, most of them will not exist. When we
are native debugging we don't bother telling the user about each file
we're checking for, we just announce any file we finally use.
I think it makes sense to do a similar thing for remote files.
So, in remote_target::remote_hostio_open(), I'd like to move the block
of code that prints the above message to after the open call has been
made, and we should only print the message if the open succeeds.
Now GDB only tells the user about files that we actually open and read
from the remote.
Andrew Burgess [Thu, 23 May 2024 17:11:55 +0000 (18:11 +0100)]
gdb: avoid duplicate search in build_id_to_bfd_suffix
In build_id_to_bfd_suffix we look in each debug-file-directory for a
file matching the required build-id.
If we don't find one then we add the sysroot and perform the search
again.
However, the default sysroot is 'target:', and for a native target
this just means to search on the local machine. So by default, if the
debug information is not present, then we end up searching each
location twice.
I think we only need to perform the "with sysroot" check if either:
1. The sysroot is something other than 'target:'. If the user has
set it to '/some/directory' then we should check this sysroot. Or if
the user has set it to 'target:/some/other_directory', this is also
worth checking.
2. If the sysroot is 'target:', but the target's filesystem is not
local (i.e. the user is connected to a remote target), then we should
check using the sysroot as this will be looking on the remote
machine.
There's no tests for this as the whole point here is that I'm removing
duplicate work. No test regressions were seen though.
There should be no user visible changes after this commit.
Andrew Burgess [Tue, 21 May 2024 10:22:26 +0000 (11:22 +0100)]
gdb/fileio: fix errno for packets where an attachment is expected
In remote.c lives remote_target::remote_hostio_send_command(), which
is used to handle sending a fileio packet to the remote, and for
parsing the reply packet.
Some commands, e.g. open, pwrite, close, send some arguments to the
remote, and then get back a single integer return value.
Other commands though, e.g. pread, readlink, fstat, send some
arguments and get back an integer return value and some additional
data. This additional data is called the attachment.
Except, we only get the attachment if the command completes
successfully. For example, calling readlink with a non existent path
will result in a return packet: 'F-1,2' with no attachment. This is
as expected.
Within remote_hostio_send_command we call remote_hostio_parse_result,
this parses the status code (-1 in our example above) and
then parses the errno value (2 in our example above).
Back in remote_hostio_parse_result we then hit this block:
/* Make sure we saw an attachment if and only if we expected one. */
if ((attachment_tmp == NULL && attachment != NULL)
|| (attachment_tmp != NULL && attachment == NULL))
{
*remote_errno = FILEIO_EINVAL;
return -1;
}
Which ensures that commands that expect an attachment, got an
attachment.
The problem is, we'll only get an attachment if the command
succeeded. If it didn't, then there is no attachment, and that is as
expected.
As remote_hostio_parse_result always sets the returned error number to
FILEIO_SUCCESS unless the packet contained an actual error
number (e.g. 2 in our example above), I suggest we should return early
if remote_hostio_parse_result indicates an error packet.
I ran into this issue while working on another patch. In that patch I
was checking the error code returned by a remote readlink call and
spotted that when I passed an invalid path I got EINVAL instead of
ENOENT. This patch fixes this issue.
Unfortunately the patch I was working on evolved, and my need to check
the error code went away, and so, right now, I have no way to reveal
this bug. But I think this is an obviously correct fix, and worth
merging even without a test.
Hannes Domani [Tue, 11 Jun 2024 19:30:45 +0000 (21:30 +0200)]
Fix cast types for opencl
The bitshift tests for opencl have these failures:
print /x (signed char) 0x0f << 8
No type named signed char.
(gdb) FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, promoted: print /x (signed char) 0x0f << 8
print (signed char) 0x0f << 8
No type named signed char.
(gdb) FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, promoted: print (signed char) 0x0f << 8
Apparently opencl doesn't have the 'signed' modifier for types, only
the 'unsigned' modifier.
Even 'char' is guaranteed to be signed if no modifier is used, so
this changes the casts to match this logic.
Hannes Domani [Tue, 11 Jun 2024 18:36:51 +0000 (20:36 +0200)]
Fix 64-bit shifts where long only has 32-bit size
On systems where long has 32-bit size you get these failures:
print 1 << (unsigned long long) 0xffffffffffffffff
Cannot export value 18446744073709551615 as 32-bits unsigned integer (must be between 0 and 4294967295)
(gdb) FAIL: gdb.base/bitshift.exp: lang=c: max-uint64: print 1 << (unsigned long long) 0xffffffffffffffff
print 1 >> (unsigned long long) 0xffffffffffffffff
Cannot export value 18446744073709551615 as 32-bits unsigned integer (must be between 0 and 4294967295)
(gdb) FAIL: gdb.base/bitshift.exp: lang=c: max-uint64: print 1 >> (unsigned long long) 0xffffffffffffffff
print -1 << (unsigned long long) 0xffffffffffffffff
Cannot export value 18446744073709551615 as 32-bits unsigned integer (must be between 0 and 4294967295)
(gdb) FAIL: gdb.base/bitshift.exp: lang=c: max-uint64: print -1 << (unsigned long long) 0xffffffffffffffff
print -1 >> (unsigned long long) 0xffffffffffffffff
Cannot export value 18446744073709551615 as 32-bits unsigned integer (must be between 0 and 4294967295)
(gdb) FAIL: gdb.base/bitshift.exp: lang=c: max-uint64: print -1 >> (unsigned long long) 0xffffffffffffffff
Fixed by changing the number-of-bits variable to ULONGEST.
The problem was introduced with the change to gmp functions in
commit 303a881f87.
It's wrong because gdb_mpz::operator>> uses mpz_tdif_q_2exp, which
always rounds toward zero, and the gmp docu says this:
For positive n both mpz_fdiv_q_2exp and mpz_tdiv_q_2exp are simple
bitwise right shifts.
For negative n, mpz_fdiv_q_2exp is effectively an arithmetic right shift
treating n as two's complement the same as the bitwise logical functions
do, whereas mpz_tdiv_q_2exp effectively treats n as sign and magnitude.
So this changes mpz_tdiv_q_2exp to mpz_fdiv_q_2exp, since it
does right shifts for both positive and negative numbers.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31590 Approved-By: Tom Tromey <tom@tromey.com>
Kilian Kilger [Sun, 26 May 2024 08:41:12 +0000 (10:41 +0200)]
fix division by zero in target_read_string()
Under certain circumstances, a floating point exception in
target_read_string() can happen when the type has been obtained
by a call to stpy_lazy_string_elt_type(). In the latter function,
a call to check_typedef() has been forgotten. This makes
type->length = 0 in this case.
Tom Tromey [Sun, 17 Dec 2023 20:22:10 +0000 (13:22 -0700)]
Remove useless call to wnoutrefresh
This call to wnoutrefresh is not useful. It's based on the
misunderstanding that wnoutrefresh somehow prevents display, whereas
actually what it does is copy from one curses buffer to another.
I'm working on a larger patch to clean up this area, but this
particular call can be removed immediately without consequence.
Ciaran Woodward [Mon, 10 Jun 2024 15:52:37 +0000 (16:52 +0100)]
Fix printing strings on macOS Sonoma
On macOS sonoma, printing a string would only print the first
character. For instance, if there was a 'const char *s = "foobar"',
then the 'print s' command would print '$1 = "f"' rather than the
expected '$1 = "foobar"'.
It seems that this is due to Apple silently replacing the version
of libiconv they ship with the OS to one which silently fails to
handle the 'outbytesleft' parameter correctly when using 'wchar_t'
as a target encoding.
This specifically causes issues when using iterating through a
string as wchar_iterator does.
This bug is visible even if you build for an old version of macOS,
but then run on Sonoma. Therefore this fix in the code applies
generally to macOS, and not specific to building on Sonoma. Building
for an older version and expecting forwards compatibility is a
common situation on macOS.
Ilya Leoshkevich [Mon, 10 Jun 2024 13:33:48 +0000 (15:33 +0200)]
IBM zSystems: Rewrite l(g)rl @GOTENT to larl for --no-pie
Regtested on s390x-redhat-linux.
Rewriting l(g)rl @GOTENT to larl is unnecessarily guarded by
bfd_link_pic(). There were no use cases for this in the past, but
since recently the Linux Kernel on s390x is compiled with -fPIE
and linked with --no-pie. Remove the unnecessary bfd_link_pic()
check.
bfd/ChangeLog:
* elf32-s390.c (elf_s390_relocate_section): Don't check for
bfd_link_pic() when rewriting lrl@GOTENT to larl.
(elf_s390_finish_dynamic_symbol): Emit a relative reloc for
the above case.
* elf64-s390.c (elf_s390_relocate_section): Don't check for
bfd_link_pic() when rewriting lgrl@GOTENT to larl.
(elf_s390_finish_dynamic_symbol): Emit a relative reloc for
the above case.
ld/ChangeLog:
* testsuite/ld-s390/s390.exp: Hook up the new tests.
* testsuite/ld-s390/gotreloc_31-no-pie-1.dd: New test.
* testsuite/ld-s390/gotreloc_64-no-pie-1.dd: New test.
Tom Tromey [Fri, 24 May 2024 18:31:45 +0000 (12:31 -0600)]
Make global_symbol_searcher::filenames private
This patch renames global_symbol_searcher::filenames and makes it
private, adding a new method to append a filename to the vector. This
also cleans up memory management here, removing an alloca from rbreak,
and removing a somewhat ugly SCOPE_EXIT from the Python code, in favor
of having global_symbol_searcher manage the memory itself.
Tom de Vries [Mon, 10 Jun 2024 15:53:30 +0000 (17:53 +0200)]
[gdb/python] Fix GDB_PY_{LL,LLU}_ARG on platform without long long
If in gdb/python/python-internal.h, we pretend to have a platform that doesn't
support long long:
...
-#ifdef HAVE_LONG_LONG
+#if 0
...
I get on arm-linux:
...
(gdb) placement_candidate()
disassemble test^M
Dump of assembler code for function test:^M
0x004004d8 <+0>: push {r11} @ (str r11, [sp, #-4]!)^M
0x004004dc <+4>: Python Exception <class 'ValueError'>: \
Buffer returned from read_memory is sized 0 instead of the expected 4^M
^M
unknown disassembler error (error = -1)^M
(gdb) FAIL: $exp: memory source api: second disassembler pass
...
The problem is that gdb_py_longest is typedef-ed to long, but the
corresponding format character GDB_PY_LL_ARG is defined to "L", meaning
"long long" [1].
Fix this by using "l", meaning long instead. Likewise for GDB_PY_LLU_ARG.
Tested on arm-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR python/31845
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31845
Tom de Vries [Mon, 10 Jun 2024 15:53:30 +0000 (17:53 +0200)]
[gdb/python] Fix gdb.python/py-disasm.exp on arm-linux
After fixing test-case gdb.python/py-disasm.exp to recognize the arm nop:
...
nop {0}
...
we run into:
...
disassemble test^M
Dump of assembler code for function test:^M
0x004004d8 <+0>: push {r11} @ (str r11, [sp, #-4]!)^M
0x004004dc <+4>: add r11, sp, #0^M
0x004004e0 <+8>: nop {0}^M
=> 0x004004e4 <+12>: Python Exception <class 'ValueError'>: Buffer \
returned from read_memory is sized 0 instead of the expected 4^M
^M
unknown disassembler error (error = -1)^M
(gdb) FAIL: $exp: global_disassembler=ShowInfoRepr: disassemble test
...
This is caused by this code in gdbpy_disassembler::read_memory_func:
...
gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
"read_memory",
"KL", len, offset));
...
where len has type "unsigned int", while "K" means "unsigned long long" [1].
Fix this by using "I" instead, meaning "unsigned int".
Also, offset has type LONGEST, which is typedef'ed to int64_t, while "L" means
"long long".
Fix this by using type gdb_py_longest for offset, in combination with format
character "GDB_PY_LL_ARG". Likewise in disasmpy_info_read_memory.
Tested on arm-linux.
Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com> Approved-By: Tom Tromey <tom@tromey.com>
PR python/31845
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31845
Matthieu Longo [Tue, 21 May 2024 16:37:07 +0000 (17:37 +0100)]
aarch64: warn on unpredictable results for new rcpc3 instructions
The previous patch for the feature rcpc3 introduced 4 new operations
(ldiapp, stilp, ldapr, stlr).
The specification mentions some cases of inputs causing unpredictable
results. gas currently fails to diagnose them, and does not emit
warnings. Even if the instruction encoding is valid, the developer
probably wants to know for those cases that the instruction won't have
the expected effect.
- ldiapp & stilp:
* unpredictable load pair transfer with register overlap
* unpredictable transfer with writeback
- ldapr & stlr:
* unpredictable transfer with writeback
This patch also completes the existing relevant tests.
Jan Beulich [Mon, 10 Jun 2024 08:46:21 +0000 (10:46 +0200)]
x86/APX: convert ZU to operand constraint
Extremely rarely used attributes are inefficient when represented by a
separate attribute. Convert it to an operand constraint, as already
suggested during review. The collision with RegKludge is pretty simple
to resolve.
Jan Beulich [Mon, 10 Jun 2024 08:45:56 +0000 (10:45 +0200)]
x86: disassembler macro for condition code
Both CMPccXADD and APX'es {,CF}CMOVcc have almost identical entries
replicated 16 times each. Fold those to just one each by introducing a
%CC macro. (Note that the recording of ->condition_code in print_insn()
is merely for completeness for now; it's not used as long as only
VEX/EVEX encodings would consume it.)
This then also renders condition codes printed consistent across all
respective insns; CMPxxXADD had a number of outliers so far.
Jan Beulich [Mon, 10 Jun 2024 08:45:16 +0000 (10:45 +0200)]
x86/APX: support extended SETcc form
As indicated during review, spelling/readability-wise
setz %eax
is easier than
setzuz %al
_and_ properly specifies the full register that's being modified. Permit
that form to be used, even if the spec writers are unwilling to formally
mention it.
While there also correct the non-ZU EVEX form: That ought to also permit
memory operands.
Simon Marchi [Mon, 10 Jun 2024 08:43:10 +0000 (10:43 +0200)]
gdb: re-add necessary includes in tui/tui-win.c
Commit 9102a6c15c75 ("gdb/tui: cleanup includes") broke
gdb.python/tui-window.exp on Linux:
Running /data/vries/gdb/src/gdb/testsuite/gdb.python/tui-window.exp ...
WARNING: timeout in accept_gdb_output
WARNING: timeout in accept_gdb_output
FAIL: gdb.python/tui-window.exp: Window was updated
and caused a build failure on AIX:
CXX tui/tui-win.o
tui/tui-win.c: In function 'void tui_sigwinch_handler(int)':
tui/tui-win.c:532:3: error: 'mark_async_signal_handler' was not declared in this scope; did you mean 'async_signal_handler'?
532 | mark_async_signal_handler (tui_sigwinch_token);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| async_signal_handler
tui/tui-win.c: At global scope:
tui/tui-win.c:538:1: error: variable or field 'tui_async_resize_screen' declared void
538 | tui_async_resize_screen (gdb_client_data arg)
| ^~~~~~~~~~~~~~~~~~~~~~~
tui/tui-win.c:538:26: error: 'gdb_client_data' was not declared in this scope
538 | tui_async_resize_screen (gdb_client_data arg)
| ^~~~~~~~~~~~~~~
tui/tui-win.c: In function 'void tui_initialize_win()':
tui/tui-win.c:579:36: error: 'tui_async_resize_screen' was not declared in this scope
579 | = create_async_signal_handler (tui_async_resize_screen, NULL,
| ^~~~~~~~~~~~~~~~~~~~~~~
tui/tui-win.c:579:7: error: 'create_async_signal_handler' was not declared in this scope; did you mean 'async_signal_handler'?
579 | = create_async_signal_handler (tui_async_resize_screen, NULL,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
| async_signal_handler
gmake: *** [Makefile:1947: tui/tui-win.o] Error 1
On Linux, the removal of the signal.h include causes the `#ifdef
SIGWINCH` sections to become inactive. The code builds, but then the
TUI fails to react to terminal size changes. When we add back the
inclusion of signal.h, then we see the same build error as on AIX.
On AIX, I suppose that the SIGWINCH define is still seen by some other
transitive include.
When I go back to before 9102a6c15c75, I don't see async-event.h and
signal.h being marked as unneeded by clangd, so I'm not sure why I
removed them in the first place... I'll be more careful in the future
(files with #ifdef/#ifndef can be tricky w.r.t. determining necessary
includes).
So, re-add the inclusion of signal.h and async-event.h
Change-Id: I3ed385c2dc1726ade2118a5186ea7cccffd12635 Reported-By: Aditya Kamath1 <Aditya.Kamath1@ibm.com> Reported-By: Tom de Vries <tdevries@suse.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31865
Tom de Vries [Mon, 10 Jun 2024 08:43:10 +0000 (10:43 +0200)]
[gdb/testsuite] Don't use set auto-solib-add off
In test-case gdb.mi/mi-var-child-f.exp, we have:
...
mi_gdb_test "-gdb-set auto-solib-add off" "\\^done"
mi_runto prog_array
mi_gdb_test "nosharedlibrary" ".*\\^done"
...
This was added to avoid a name clash between the array variable as defined in
gdb.mi/array.f90 and debug info in shared libraries, and used in other places
in the testsuite.
The same workaround is also used to ignore symbols from shared libraries when
excercising for instance a command that prints all symbols.
However, this approach can cause problems for targets like arm that require
symbol info for some libraries like ld.so and libc to fully function.
While absense of debug info for shared libraries should be handled gracefully
(which does need fixing, see PR31817), failure to do so should not result
in failures in unrelated test-cases.
Fix this by removing "set auto-solib-add off".
This ensures that we don't run into PR31817, while the presence of
nosharedlibrary still ensures that in the rest of the test-case we're not
bothered by shared library symbols.
Likewise in other test-cases.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Tested on arm-linux.
Jan Beulich [Mon, 10 Jun 2024 07:06:37 +0000 (09:06 +0200)]
gas: extend \+ support to .rept
PR gas/31752
While not quite as macro-like as .irp / .irpc, this perhaps benefits from
supporting \+ even more than those: It allows, where desired, to get away
without maintaining an explicit count variable in source code.
Matthieu Longo [Tue, 28 May 2024 09:49:48 +0000 (10:49 +0100)]
autoconf: delete obsolete unused m4 file
config/uintmax_t.m4 contains only one function: jm_AC_TYPE_UINTMAX_T.
After a grep, I could not find any usage of it.
jm_AC_TYPE_UINTMAX_T seems to be an old implementation of the officially
suppported AC_TYPE_UINTMAX_T.
Doc: https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/
autoconf-2.72/autoconf.html#index-AC_005fTYPE_005fUINTMAX_005fT-1
config/stdint.m4 seems to have taken over. The usage of
AC_REQUIRE([AC_TYPE_UINTMAX_T]) is not garded or inside a function, so it
will also automatically be run for the configure.ac files using
AC_CONFIG_MACRO_DIRS([../config]) instead of m4_include([../config/stdint.m4]).
It seems that people replaced jm_AC_TYPE_UINTMAX_T occurences by
AC_TYPE_UINTMAX_T, and forgot to delete uintmax_t.m4.
Deleting the file and regenerating the build scripts results in no diff,
so it looks safe to delete it from the repository.
Matthieu Longo [Tue, 28 May 2024 09:49:45 +0000 (10:49 +0100)]
autoupdate: replace old version of AC_INIT by the new one
- old AC_INIT by AC_INIT + AC_CONFIG_SRC_DIR
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.72/autoconf.html#index-AC_005fINIT-3
- AC_CONFIG_HEADER by AC_CONFIG_HEADERS
https://www.gnu.org/software/automake/manual/1.12.2/html_node/Obsolete-Macros.html#index-AM_005fCONFIG_005fHEADER
- AC_CANONICAL_SYSTEM by:
* AC_CANONICAL_HOST where host, and host_alias are needed
* AC_CANONICAL_TARGET where target_alias is needed
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.72/autoconf.html#index-AC_005fCANONICAL_005fTARGET-1
Matthieu Longo [Tue, 28 May 2024 09:49:41 +0000 (10:49 +0100)]
autoupdate: replace obsolete macros AC_AIX, AC_MINIX, and AC_GNU_SOURCE
- AC_AIX, AC_MINIX, and AC_GNU_SOURCE by AC_USE_SYSTEM_EXTENSIONS
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.72/autoconf.html#index-AC_005fAIX
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.72/autoconf.html#index-AC_005fMINIX-1
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.72/autoconf.html#index-AC_005fGNU_005fSOURCE-1
- AC_HELP_STRING by AS_HELP_STRING
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.72/autoconf.html#index-AC_005fHELP_005fSTRING-1
Except for the ifdef in lib-prefix.m4, make the defun of AC_LIB_ARG_WITH
unconditional.
H.J. Lu [Sat, 1 Jun 2024 04:30:34 +0000 (21:30 -0700)]
gold: Properly remove the versioned symbol
When the versioned symbol foo is removed from the shared library, the
".symver foo,foo@VER" directive provides binary compatibility for foo@VER.
In this case, the unversioned symbol foo should be hidden and shouldn't
generate a multiple definition error.
PR gold/31830
* resolve.cc (Symbol_table::resolve): Move symbol version handling
to ...
* symtab.cc (Symbol_table::add_from_object): Here. If the hidden
version from .symver is the same as the default version from the
unversioned symbol, don't make the unversioned symbol the default
versioned
symbol.
* testsuite/Makefile.am (check_SCRIPTS): Add ver_test_pr31830.sh.
(check_DATA): ver_test_pr31830_a.syms and ver_test_pr31830_b.syms.
(ver_test_pr31830_a.syms): New.
(ver_test_pr31830_b.syms): Likewise.
(ver_test_pr31830_a.so): Likewise.
(ver_test_pr31830_b.so): Likewise.
* testsuite/Makefile.in: Regenerated.
* testsuite/ver_test_pr31830.script: New file.
* testsuite/ver_test_pr31830.sh: Likewise.
* testsuite/ver_test_pr31830_a.c: Likewise.
* testsuite/ver_test_pr31830_b.c: Likewise.
* testsuite/ver_test_pr31830_lto.c: Likewise.
* testsuite/ver_test_pr31830_lto.sh: Likewise.
Simon Marchi [Thu, 30 May 2024 18:53:56 +0000 (14:53 -0400)]
gdb: remove get_exec_file
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>
Simon Marchi [Thu, 30 May 2024 18:53:54 +0000 (14:53 -0400)]
gdb: replace `get_exec_file (0)` calls with `current_program_space->exec_filename ()`
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>
Simon Marchi [Thu, 30 May 2024 18:53:53 +0000 (14:53 -0400)]
gdb: make progspace::exec_filename private, add getter / setter
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>
Simon Marchi [Fri, 31 May 2024 02:54:07 +0000 (22:54 -0400)]
gdb/tui: cleanup includes
Remove includes reported as unused by clangd. Then, add any includes
necessary to get rid of errors (includes possibly relying on previous
includes)..
I didn't remove the includes of gdb-safe-ctypes.h, because it appears to
do some some preprocessor magic, redefining standard macros. I'm afraid
that removing these includes could change the behavior unintentionally.
Change-Id: I4c5b652355c3bbce022fe0d447a72dc4e1d17d34 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Fri, 31 May 2024 02:54:06 +0000 (22:54 -0400)]
gdb: add IWYU export pragams to gdb_curses.h
It seems like gdb_curses.h is included whenever we want to access
ncurses functionality, instead of including directly ncurses.h. As a
result, clangd often erroneously shows that gdb_curses.h inclusions are
unused.
By adding those pragmas, clangd (and the include-what-you-use tool)
understands that gdb_curses.h is a valid provider for whatever these
ncurses.h files provide.
Change-Id: Ia8acd761dae1577f7151d5fb558f35514b4e4ea2 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Fri, 31 May 2024 02:54:05 +0000 (22:54 -0400)]
gdb/tui: change some macros to functions
Change the `TUI_*` macros to access known windows to functions. Define
them in their respective files, because trying to define them in
tui-data.h would end up causing include cycles.
This makes static analysis (detection of unused include files in this
case) more accurate, and I think in general we should avoid hiding
code behind macros if not necessary.
Change-Id: I1e38cee843984c48ab34030b19dac0d726f851af Approved-By: Tom Tromey <tom@tromey.com>
Test behaviour of watchpoints triggered by MOPS instructions. This test
is similar to gdb.base/memops-watchpoint.exp, but specifically for MOPS
instructions rather than whatever instructions are used in the libc's
implementation of memset/memcpy/memmove.
There's a separate watched variable for each set of instructions so that
the testcase can test whether GDB correctly identified the watchpoint
that triggered in each case.
Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com>
gdb/aarch64: Add record support for MOPS instructions.
There are two kinds of MOPS instructions: set instructions and copy
instructions. Within each group there are variants with minor
differences in how they read or write to memory — e.g., non-temporal
read and/or write, unprivileged read and/or write and permutations of
those — but they work in the same way in terms of the registers and
regions of memory that they modify.
The new gdb.reverse/aarch64-mops.exp testcase verifies that MOPS
instructions are recorded and correctly reversed. Not all variants of the
copy and set instructions are tested, since there are many and the record
and replay target processes them in the same way.
PR tdep/31666
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31666 Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com>
gdb/aarch64: Disable displaced single-step for MOPS instructions
The AArch64 MOPS (Memory Operation) instructions provide a standardised
instruction sequence to perform a memset, memcpy or memmove. A sequence is
always composed of three instructions: a prologue instruction, a main
instruction and an epilogue instruction. As an illustration, here are the
implementations of these memory operations in glibc 2.39:
(gdb) disassemble/r
Dump of assembler code for function __memset_mops:
=> 0x0000fffff7e8d780 <+0>: d503201f nop
0x0000fffff7e8d784 <+4>: aa0003e3 mov x3, x0
0x0000fffff7e8d788 <+8>: 19c10443 setp [x3]!, x2!, x1
0x0000fffff7e8d78c <+12>: 19c14443 setm [x3]!, x2!, x1
0x0000fffff7e8d790 <+16>: 19c18443 sete [x3]!, x2!, x1
0x0000fffff7e8d794 <+20>: d65f03c0 ret
End of assembler dump.
(gdb) disassemble/r
Dump of assembler code for function __memcpy_mops:
=> 0x0000fffff7e8c580 <+0>: d503201f nop
0x0000fffff7e8c584 <+4>: aa0003e3 mov x3, x0
0x0000fffff7e8c588 <+8>: 19010443 cpyfp [x3]!, [x1]!, x2!
0x0000fffff7e8c58c <+12>: 19410443 cpyfm [x3]!, [x1]!, x2!
0x0000fffff7e8c590 <+16>: 19810443 cpyfe [x3]!, [x1]!, x2!
0x0000fffff7e8c594 <+20>: d65f03c0 ret
End of assembler dump.
(gdb) disassemble/r
Dump of assembler code for function __memmove_mops:
=> 0x0000fffff7e8d180 <+0>: d503201f nop
0x0000fffff7e8d184 <+4>: aa0003e3 mov x3, x0
0x0000fffff7e8d188 <+8>: 1d010443 cpyp [x3]!, [x1]!, x2!
0x0000fffff7e8d18c <+12>: 1d410443 cpym [x3]!, [x1]!, x2!
0x0000fffff7e8d190 <+16>: 1d810443 cpye [x3]!, [x1]!, x2!
0x0000fffff7e8d194 <+20>: d65f03c0 ret
End of assembler dump.
The Arm Architecture Reference Manual says that "the prologue, main, and
epilogue instructions are expected to be run in succession and to appear
consecutively in memory". Therefore this patch disables displaced stepping
on them.
The testcase verifies that MOPS sequences are correctly single-stepped.
PR tdep/31666
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31666 Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com>
Alan Modra [Thu, 6 Jun 2024 22:57:31 +0000 (08:27 +0930)]
Re: Yet another ecoff fuzzed object fix
In commit 6fc018e9e593 I replaced the fdr_ptr csym check against the
header isymMax count with a check against bfd symcount. In fact, both
checks are needed. The isymMax check sanity checks accesses against
the external sym array, the symcount one against the internal array.
* ecoff.c (_bfd_ecoff_slurp_symbol_table): Reinstate fdr_ptr
csym check against isymMax.
Szabolcs Nagy [Mon, 3 Jun 2024 16:20:32 +0000 (17:20 +0100)]
aarch64: Fix DT_RELR support with discarded sections
In case of discarded sections, via /DISCARD/ or .gnu.linkonce,
relr relocation accounting was wrong. This broke building linux.
The issue was that the *_relocate_section logic was copied to
record_relr_non_got_relocs to find the relative relocs that can
be packed, however *_relocate_section is not called on sections
that are discarded, while record_relr_non_got_relocs is called
for all input sections. The fix is to filter out the discarded
sections with the same logic that is used to count non-GOT
relocs in *_late_size_sections for local symbols earlier.
Use the discarded_section helper in both cases to clarify the
intent and handle all corner-cases consistently.
GOT relocations are affected too if all sections are discarded
that reference the GOT entry of a particular symbol, however
this can cause unused GOT entries independently of DT_RELR, and
the only difference with DT_RELR is that a relative reloc may be
emitted instead of a R_AARCH64_NONE for the unused GOT entry
which is acceptable. A proper fix would require redoing the GOT
refcounting after we know the discarded sections, see bug 31850.
I rewrote the rules for building the man pages. While doing this I
accidentally switched from using MAN2POD5 to MAN2POD1 for generating
the file gdbinit.5.
Johan Sternerup [Sat, 1 Jun 2024 16:16:32 +0000 (18:16 +0200)]
DAP: Handle "stepOut" request in outermost frame
Previously a "stepOut" request when in the outermost frame would result
in a sucessful response even though gdb internally would reject the
associated "finish" request, which means no stoppedEvent would ever be
sent back to the client. Thus the client would believe the inferior was
still running and as a consequence reject subsequent "next" and "stepIn"
requests from the user.
The solution is to execute the underlying finish command as a background
command, i.e. `finish &`. If we're in the outermost frame an exception
will be raised immediately, which we can now capture and report back to
the client as success=False so then the absence of a `stopped` event is
no longer a problem.
We also make use of the `defer_stop_event` option to prevent a stop
event from reaching the client until the response has been sent.
Johan Sternerup [Sat, 1 Jun 2024 16:16:31 +0000 (18:16 +0200)]
DAP: Allow gdb exception in exec_and_log to propagate
This allows a request to specify that any gdb exception raised in
exec_and_log within the gdb thread to be propagated back to the DAP
thread (using the Canceller object as the orchestrator).
Johan Sternerup [Sat, 1 Jun 2024 16:16:30 +0000 (18:16 +0200)]
DAP: Allow for deferring stop events from gdb thread
The existing `send_event_later()` method allows commands processed on
the DAP thread to queue an event for execution until after the response
has been sent to the client.
We now introduce a corresponding method for use by the gdb thread. This
method `send_event_maybe_later()` will queue the event just like
`send_event_later()`, but only if it has been configured to do so by a
new @request option `defer_stop_events`. As the name implies the
functionality is currently only used for handling stop events.
arm: fix testsuite fallout on arm-elf and arm-nto from FPA removal
Removing FPA means that in some cases we default to 'no-fpu' in the
assembler when previously we would have picked FPA-format floating
numbers. This patch fixes the testsuite fallout on a couple of
targets that are affected by this change. Where possible we do this
by adding an option to set the floating-point format, but for bad-bss
we just skip the test.
Andrew Burgess [Wed, 5 Jun 2024 12:12:42 +0000 (13:12 +0100)]
opcodes/riscv: prevent future use of disassemble_info::fprintf_func
The previous commit removed a use of disassemble_info::fprintf_func
which had been added to the RISC-V disassembler after the disassembler
had been switched to use ::fprintf_styled_func, for styled output.
To prevent future mistakes, I propose adding a #define to rename
fprintf_func to something which does not exist. If this had been in
place then the before the previous commit libopcodes would have failed
to compile, like this:
../../src/opcodes/riscv-dis.c: In function ‘print_reg_list’:
../../src/opcodes/riscv-dis.c:229:7: error: ‘disassemble_info’ {aka ‘struct disassemble_info’} has no member named ‘please_use_fprintf_styled_func_instead’
229 | info->fprintf_func (info->stream, "%s", riscv_gpr_names[X_RA]);
| ^~
If this commit is accepted then I'll follow up with another commit
that adds the same #define to every disassembler that has been
converted to use styled output.
As the RISC-V disassembler is now fully styled, this commit should
make no difference at all.
Xiao Zeng [Thu, 6 Jun 2024 07:59:53 +0000 (15:59 +0800)]
RISC-V: Add support for Zvfbfwma extension
This implements the Zvfbfwma extension, as of version 1.0.
View detailed information in:
<https://github.com/riscv/riscv-isa-manual/blob/main/src/bfloat16.adoc#zvfbfwma---vector-bf16-widening-mul-add>
1 In spec: "Zvfbfwma requires the Zvfbfmin extension and the Zfbfmin extension."
1.1 In Embedded Processor: Zvfbfwma -> Zvfbfmin -> Zve32f
1.2 In Application Processor: Zvfbfwma -> Zvfbfmin -> V
1.3 In both scenarios, there are: Zvfbfwma -> Zfbfmin
2 Depending on different usage scenarios, the Zvfbfwma extension may
depend on 'V' or 'Zve32f'. This patch only implements dependencies in
scenario of Embedded Processor. This is consistent with the processing
strategy in Zvfbfmin. In scenario of Application Processor, it is
necessary to explicitly indicate the dependent 'V' extension.
Xiao Zeng [Thu, 6 Jun 2024 07:59:52 +0000 (15:59 +0800)]
RISC-V: Add support for Zvfbfmin extension
This implements the Zvfbfmin extension, as of version 1.0.
View detailed information in:
<https://github.com/riscv/riscv-isa-manual/blob/main/src/bfloat16.adoc#zvfbfmin---vector-bf16-converts>
Depending on different usage scenarios, the Zvfbfmin extension may
depend on 'V' or 'Zve32f'. This patch only implements dependencies
in scenario of Embedded Processor. In scenario of Application
Processor, it is necessary to explicitly indicate the dependent
'V' extension.
Xiao Zeng [Thu, 6 Jun 2024 07:59:51 +0000 (15:59 +0800)]
RISC-V: Add support for Zfbfmin extension
This implements the Zfbfmin extension, as of version 1.0.
View detailed information in:
<https://github.com/riscv/riscv-isa-manual/blob/main/src/bfloat16.adoc#zfbfmin---scalar-bf16-converts>
1 The Zfbfmin extension depend on 'F', and the FLH, FSH, FMV.X.H, and
FMV.H.X instructions as defined in the Zfh extension.
2 The Zfhmin extension includes the following instructions from the Zfh
extension: FLH, FSH, FMV.X.H, FMV.H.X... View detailed information in:
<https://github.com/riscv/riscv-isa-manual/blob/main/src/zfh.adoc>
3 Zfhmin extension depend on 'F'.
4 Simply put, just make Zfbfmin dependent on Zfhmin.
Perhaps in the future, we could propose making the FLH, FSH, FMV.X.H, and
FMV.H.X instructions an independent extension to achieve precise dependency
relationships for the Zfbfmin.
Alan Modra [Mon, 3 Jun 2024 23:30:57 +0000 (09:00 +0930)]
Re: aarch64: Add some DT_RELR ld tests
aarch64-elf fails these tests due to .rela.dyn being at a different
address to that expected, and due to the symbol table being different.
Unexpected symbol numbering results in a mismatch of reloc r_info
field, but these are shown decoded so the raw field doesn't really add
anything to the test.
* testsuite/ld-aarch64/relr-align.d: Accept any address for
.relr.dyn section. Don't match raw r_info field.
* testsuite/ld-aarch64/relr-data-shared.d: Likewise.
* testsuite/ld-aarch64/relr-got-shared.d: Likewise.
* testsuite/ld-aarch64/relr-text-shared.d: Likewise.
arm: remove disassembly support for the FPA co-processor
Remove the FPA support from the disassembler. This entails a couple
of testsuite fixes where we were (probably incorrectly) disassembling
a generic co-processor instruction using the legacy FPA opcodes.
Remove the command-line options to choose the FPA (or FPE - an
emulated FPA). From this point on it should be impossible to assemble
the old FPA instructions.
Change the cases where the default FPU was FPA to none. This should
ensure that any code that used settings to pick the floating-point
order will not silently produce a different output. The options that
explicitly set the FPA remain for the moment.
arm: redirect fp constant data directives through a wrapper
Assembler directives such as .float, or .double are handled by generic
code, but on Arm, their output can vary depeding on the type of FPU
begin targetted. When we remove FPA support we don't want to silently
generate different code for processors that previously defaulted to
the FPA, so redirect these directives through a wrapper function that
checks the FPU is enabled; we use the legacy -mno-fpu in the test to
catch this.
Also fix a few tests so that they won't start to fail on targets (eg
arm-wince-pe) where there is no default format for the FPU and we pick
this from the default processor type.
The logic here seems to be overly complex, so simplify it a bit. One
particular problem was that using the legacy -mno-fpu option was not
working properly, as this has all the feature bits set to zero causing
the code to then pick a different FPU as the default. Fix this by
only selecting an FPU as a fallback if the code has not otherwise
selected one: there was only one route by which this could happen.
This patch is really a pre-cursor to the following one where we want
to make no-fpu internally a fall-back position for some legacy
processors where previously we would have dropped back to the FPA.
From armv6 onwards a lot of cores started to come with a physical VFP
implementation; but many still did not and in some cases there are
both variants. For the cores that lacked a physical VFP we would fall
back to FPU_NONE if the platform/ABI did not mandate something else.
To make matters worse, FPU_NONE is internal state used to imply
soft-fpa (ie a mixed-endian double format), so any use of .double in
hand-written assembly is almost certainly generating incorrect output.
That's undesirable, all these cores should really default to a softvfp
model.