Tom de Vries [Wed, 19 Jun 2024 15:32:55 +0000 (17:32 +0200)]
[gdb/symtab] Fix target type of complex long double on arm
When running test-case gdb.base/complex-parts.exp on arm-linux, I get:
...
(gdb) p $_cimag (z3)^M
$6 = 6.5^M
(gdb) PASS: gdb.base/complex-parts.exp: long double imaginary: p $_cimag (z3)
ptype $^M
type = double^M
(gdb) FAIL: gdb.base/complex-parts.exp: long double imaginary: ptype $
...
Given that z3 is a complex long double, the test-case expects the type of the
imaginary part of z3 to be long double, but it's double instead.
This is due to the fact that the dwarf info doesn't specify an explicit target
type:
...
<5b> DW_AT_name : z3
<60> DW_AT_type : <0xa4>
...
<1><a4>: Abbrev Number: 2 (DW_TAG_base_type)
<a5> DW_AT_byte_size : 16
<a6> DW_AT_encoding : 3 (complex float)
<a7> DW_AT_name : complex long double
...
and consequently we're guessing in dwarf2_init_complex_target_type based on
the size:
...
case 64:
tt = builtin_type (gdbarch)->builtin_double;
break;
case 96: /* The x86-32 ABI specifies 96-bit long double. */
case 128:
tt = builtin_type (gdbarch)->builtin_long_double;
break;
...
For arm-linux, complex long double is 16 bytes, so the target type is assumed
to be 8 bytes, which is handled by the "case 64", which gets us double
instead of long double.
Fix this by searching for "long" in the name_hint parameter, and using long
double instead.
Note that base types in dwarf are not allowed to contain references to other
types, and the complex types are base types, so the missing explicit target
type is standard-conformant.
A gcc PR was filed to add this as a dwarf extension (
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115272 ).
Nick Alcock [Wed, 19 Jun 2024 13:06:26 +0000 (14:06 +0100)]
libctf: fix testsuite bugs revealed by -Wall
Most of these are harmless, but some of the type confusions and especially
a missing ctf_strerror() on an error path were actual bugs that could
have resulted in test failures crashing rather than printing an error
message.
libctf/
* testsuite/libctf-lookup/enumerator-iteration.c: Fix type
confusion, signedness confusion and a missing ctf_errmsg().
* testsuite/libctf-regression/libctf-repeat-cu-main.c: Return 0 from
the test function.
* testsuite/libctf-regression/open-error-free.c: Fix signedness
confusion.
* testsuite/libctf-regression/zrewrite.c: Remove unused label.
[gdb/python] Add typesafe wrapper around PyObject_CallMethod
The error message is:
../../gdb/python/python-internal.h:151:16: error: default initialization of an object of const type 'const char'
constexpr char gdbpy_method_format;
^
= '\0'
CXX python/py-block.o
1 error generated.
make[2]: *** [Makefile:1959: python/py-arch.o] Error 1
make[2]: *** Waiting for unfinished jobs....
In file included from ../../gdb/python/py-auto-load.c:25:
../../gdb/python/python-internal.h:151:16: error: default initialization of an object of const type 'const char'
constexpr char gdbpy_method_format;
^
= '\0'
1 error generated.
make[2]: *** [Makefile:1959: python/py-auto-load.o] Error 1
In file included from ../../gdb/python/py-block.c:23:
../../gdb/python/python-internal.h:151:16: error: default initialization of an object of const type 'const char'
constexpr char gdbpy_method_format;
^
= '\0'
1 error generated.
This patch fixes this by changing gdbpy_method_format to be a templated
struct, and only have its specializations define the static constexpr
member "format". This way, we avoid having an uninitialized constexpr
expression, regardless of it being instantiated or not.
Reviewed-By: Tom de Vries <tdevries@suse.de>
Change-Id: I5bec241144f13500ef78daea30f00d01e373692d
Tom de Vries [Wed, 19 Jun 2024 08:04:22 +0000 (10:04 +0200)]
[gdb/testsuite] Fix gdb.dwarf2/shortpiece.exp on s390x
On s390x-linux, I run into:
...
(gdb) p (short []) s1^M
$3 = {0, 1, 0, <optimized out>}^M
(gdb) FAIL: gdb.dwarf2/shortpiece.exp: p (short []) s1
...
while this is expected:
...
(gdb) p (short []) s1^M
$3 = {1, 0, 0, <optimized out>}^M
(gdb) PASS: gdb.dwarf2/shortpiece.exp: p (short []) s1
...
The type of s1 is:
...
(gdb) ptype s1
type = struct S {
myint a;
myushort b;
}
...
so the difference is due the fact that viewing an int as two shorts gives
different results depending on the endianness.
Tom de Vries [Wed, 19 Jun 2024 07:52:01 +0000 (09:52 +0200)]
[gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI
In commit 1a7d840a216 ("[gdb/tdep] Fix ARM_LINUX_JB_PC_EABI"), in absense of
osabi settings for newlib and uclibc for arm, I chose a best-effort approach
using ifdefs.
Post-commit review [1] pointed out that this may be causing more problems than
it's worth.
Fix this by removing the ifdefs and simply defining ARM_LINUX_JB_PC_EABI to 1.
Rebuild on x86_64-linux with --enable-targets=all.
Tom de Vries [Tue, 18 Jun 2024 15:26:22 +0000 (17:26 +0200)]
[gdb/build] Add GPL header comment to gdb/features/feature_to_c.awk
Commit 97033da5070 ("[gdb/build] Cleanup gdb/features/feature_to_c.sh")
factored out new file gdb/features/feature_to_c.awk out of
gdb/features/feature_to_c.sh, but failed to add the GPL header comment, so add
this now.
Nick Alcock [Tue, 11 Jun 2024 19:58:00 +0000 (20:58 +0100)]
libctf, include: new functions for looking up enumerators
Three new functions for looking up the enum type containing a given
enumeration constant, and optionally that constant's value.
The simplest, ctf_lookup_enumerator, looks up a root-visible enumerator by
name in one dict: if the dict contains multiple such constants (which is
possible for dicts created by older versions of the libctf deduplicator),
ECTF_DUPLICATE is returned.
The next simplest, ctf_lookup_enumerator_next, is an iterator which returns
all enumerators with a given name in a given dict, whether root-visible or
not.
The most elaborate, ctf_arc_lookup_enumerator_next, finds all
enumerators with a given name across all dicts in an entire CTF archive,
whether root-visible or not, starting looking in the shared parent dict;
opened dicts are cached (as with all other ctf_arc_*lookup functions) so
that repeated use does not incur repeated opening costs.
All three of these return enumerator values as int64_t: unfortunately, API
compatibility concerns prevent us from doing the same with the other older
enum-related functions, which all return enumerator constant values as ints.
We may be forced to add symbol-versioning compatibility aliases that fix the
other functions in due course, bumping the soname for platforms that do not
support such things.
ctf_arc_lookup_enumerator_next is implemented as a nested ctf_archive_next
iterator, and inside that, a nested ctf_lookup_enumerator_next iterator
within each dict. To aid in this, add support to ctf_next_t iterators for
iterators that are implemented in terms of two simultaneous nested iterators
at once. (It has always been possible for callers to use as many nested or
semi-overlapping ctf_next_t iterators as they need, which is one of the
advantages of this style over the _iter style that calls a function for each
thing iterated over: the iterator change here permits *ctf_next_t iterators
themselves* to be implemented by iterating using multiple other iterators as
part of their internal operation, transparently to the caller.)
Also add a testcase that tests all these functions (which is fairly easy
because ctf_arc_lookup_enumerator_next is implemented in terms of
ctf_lookup_enumerator_next) in addition to enumeration addition in
ctf_open()ed dicts, ctf_add_enumerator duplicate enumerator addition, and
conflicting enumerator constant deduplication.
Nick Alcock [Tue, 11 Jun 2024 19:55:35 +0000 (20:55 +0100)]
include: libctf: comment improvements
Describe a bit more clearly what effects a type being non-root-
visible has. More consistently use the term non-root-visible
rather than hidden. Document ctf_enum_iter.
include/
* ctf-api.h (ctf_enum_iter): Document.
(ctf_type_iter): Hidden, not non-root. Mention that
parent dictionaries are not traversed.
Nick Alcock [Tue, 11 Jun 2024 19:33:03 +0000 (20:33 +0100)]
libctf: prohibit addition of enums with overlapping enumerator constants
libctf has long prohibited addition of enums with overlapping constants in a
single enum, but now that we are properly considering enums with overlapping
constants to be conflciting types, we can go further and prohibit addition
of enumeration constants to a dict if they already exist in any enum in that
dict: the same rules as C itself.
We do this in a fashion vaguely similar to what we just did in the
deduplicator, by considering enumeration constants as identifiers and adding
them to the core type/identifier namespace, ctf_dict_t.ctf_names. This is a
little fiddly, because we do not want to prohibit opening of existing dicts
into which the deduplicator has stuffed enums with overlapping constants!
We just want to prohibit the addition of *new* enumerators that violate that
rule. Even then, it's fine to add overlapping enumerator constants as long
as at least one of them is in a non-root type. (This is essential for
proper deduplicator operation in cu-mapped mode, where multiple compilation
units can be smashed into one dict, with conflicting types marked as
hidden: these types may well contain overlapping enumerators.)
So, at open time, keep track of all enums observed, then do a third pass
through the enums alone, adding each enumerator either to the ctf_names
table as a mapping from the enumerator name to the enum it is part of (if
not already present), or to a new ctf_conflicting_enums hashtable that
tracks observed duplicates. (The latter is not used yet, but will be soon.)
(We need to do a third pass because it's quite possible to have an enum
containing an enumerator FOO followed by a type FOO: since they're processed
in order, the enumerator would be processed before the type, and at that
stage it seems nonconflicting. The easiest fix is to run through the
enumerators after all type names are interned.)
At ctf_add_enumerator time, if the enumerator to which we are adding a type
is root-visible, check for an already-present name and error out if found,
then intern the new name in the ctf_names table as is done at open time.
(We retain the existing code which scans the enum itself for duplicates
because it is still an error to add an enumerator twice to a
non-root-visible enum type; but we only need to do this if the enum is
non-root-visible, so the cost of enum addition is reduced.)
Tested in an upcoming commit.
libctf/
* ctf-impl.h (ctf_dict_t) <ctf_names>: Augment comment.
<ctf_conflicting_enums>: New.
(ctf_dynset_elements): New.
* ctf-hash.c (ctf_dynset_elements): Implement it.
* ctf-open.c (init_static_types): Split body into...
(init_static_types_internal): ... here. Count enumerators;
keep track of observed enums in pass 2; populate ctf_names and
ctf_conflicting_enums with enumerators in a third pass.
(ctf_dict_close): Free ctf_conflicting_enums.
* ctf-create.c (ctf_add_enumerator): Prohibit addition of duplicate
enumerators in root-visible enum types.
include/
* ctf-api.h (CTF_ADD_NONROOT): Describe what non-rootness
means for enumeration constants.
(ctf_add_enumerator): The name is not a misnomer.
We now require that enumerators have unique names.
Document the non-rootness of enumerators.
Nick Alcock [Wed, 12 Jun 2024 10:08:39 +0000 (11:08 +0100)]
libctf: suppress spurious failure of malloc-counting tests under valgrind
The libctf-regression/open-error-free.c test works by interposing malloc
and counting mallocs and frees across libctf operations. This only
works under suitably-interposable mallocs on systems supporting
dlsym (RTLD_NEXT, ...), so its operation is restricted to glibc
systems for now, but also it interacts badly with valgrind, which
interposes malloc itself. Detect a running valgrind and skip the test.
Add new facilities allowing libctf lookup tests to declare themselves
unsupported, by printing "UNSUPPORTED: " and then some meaningful
message instead of their normal output.
libctf/
* configure.ac: Check for <valgrind/valgrind.h>.
* config.h.in: Regenerate.
* configure: Likewise.
* testsuite/lib/ctf-lib.exp (run_lookup_test): Add support for
UNSUPPORTED tests.
* testsuite/libctf-regression/open-error-free.c: When running
under valgrind, this test is unsupported.
Nick Alcock [Wed, 12 Jun 2024 11:28:45 +0000 (12:28 +0100)]
libctf: fix dict leak on archive-wide symbol lookup error path
If a lookup fails for a reason unrelated to a lack of type data for this
symbol, we return with an error; but we fail to close the dict we opened
most recently, which is leaked.
libctf/
* ctf-archive.c (ctf_arc_lookup_sym_or_name): Close dict.
Nick Alcock [Tue, 11 Jun 2024 19:11:29 +0000 (20:11 +0100)]
libctf: don't leak enums if ctf_add_type fails
If ctf_add_type failed in the middle of enumerator addition, the
destination would end up containing the source enum type and some
but not all of its enumerator constants.
Use snapshots to roll back the enum addition as a whole if this happens.
Before now, it's been pretty unlikely, but in an upcoming commit we will ban
addition of enumerators that already exist in a given dict, making failure
of ctf_add_enumerator and thus of this part of ctf_add_type much more
likely.
libctf/
* ctf-create.c (ctf_add_type_internal): Roll back if enum or
enumerator addition fails.
Nick Alcock [Tue, 11 Jun 2024 18:51:33 +0000 (19:51 +0100)]
libctf: dedup: enums with overlapping enumerators are conflicting
The CTF deduplicator was not considering enumerators inside enum types to be
things that caused type conflicts, so if the following two TUs were linked
together, you would end up with the following in the resulting dict:
1.c:
enum foo { A, B };
2.c:
enum bar { A, B };
linked:
enum foo { A, B };
enum bar { A, B };
This does work -- but it's not something that's valid C, and the general
point of the shared dict is that it is something that you could potentially
get from any valid C TU.
So consider such types to be conflicting, but obviously don't consider
actually identical enums to be conflicting, even though they too have (all)
their identifiers in common. This involves surprisingly little code. The
deduplicator detects conflicting types by counting types in a hash table of
hash tables:
decorated identifier -> (type hash -> count)
where the COUNT is the number of times a given hash has been observed: any
name with more than one hash associated with it is considered conflicting
(the count is used to identify the most common such name for promotion to
the shared dict).
Before now, those identifiers were all the identifiers of types (possibly
decorated with their namespace on the front for enumerator identifiers), but
we can equally well put *enumeration constant names* in there, undecorated
like the identifiers of types in the global namespace, with the type hash
being the hash of each enum containing that enumerator. The existing
conflicting-type-detection code will then accurately identify distinct enums
with enumeration constants in common. The enum that contains the most
commonly-appearing enumerators will be promoted to the shared dict.
libctf/
* ctf-impl.h (ctf_dedup_t) <cd_name_counts>: Extend comment.
* ctf-dedup.c (ctf_dedup_count_name): New, split out of...
(ctf_dedup_populate_mappings): ... here. Call it for all
* enumeration constants in an enum as well as types.
ld/
* testsuite/ld-ctf/enum-3.c: New test CTF.
* testsuite/ld-ctf/enum-4.c: Likewise.
* testsuite/ld-ctf/overlapping-enums.d: New test.
* testsuite/ld-ctf/overlapping-enums-2.d: Likewise.
Nick Alcock [Tue, 11 Jun 2024 18:37:54 +0000 (19:37 +0100)]
libctf: strtab corruption when strings are added to ctf_open()ed dicts
ctf_str_add_ref and ctf_str_add_movable_ref take a string and are supposed
to return a strtab offset. These offsets are "provisional": the ref
mechanism records the address of the location in which the ref is stored and
modifies it when the strtab is finally written out. Provisional refs in new
dicts start at 0 and go up via strlen() as new refs are added: this is fine,
because the strtab is empty and none of these values will overlap any
existing string offsets (since there are none). Unfortunately, when a dict
is ctf_open()ed, we fail to set the initial provisional strtab offset to a
higher value than any existing string offset: it starts at zero again!
It's a shame that we already *have* strings at those offsets...
This is all fixed up once the string is reserialized, but if you look up
newly-added strings before serialization, you get corrupted partial string
results from the existing ctf_open()ed dict.
Observed (and thus regtested) by an upcoming test (in this patch series).
Exposed by the recently-introduced series that permits modification of
ctf_open()ed dicts, which has not been released anywhere. Before that,
any attempt to do such things would fail with ECTF_RDONLY.
Jan Beulich [Tue, 18 Jun 2024 11:23:36 +0000 (13:23 +0200)]
readelf: rename recently added testsuite files
Files named *.0 are somewhat odd for testsuite expectations. Rename the
one such file to *.r with a suitable base name suffix, and have its
sibling follow suit in this latter regard.
Nelson Chu [Tue, 18 Jun 2024 08:15:45 +0000 (16:15 +0800)]
RISC-V: Fixed typo from smscrind to smcsrind in riscv_implicit_subsets.
bfd/
* elfxx-riscv.c (riscv_implicit_subsets): Fixed type from smscrind to
smcsrind.
gas/
* testsuite/gas/riscv/march-imply-smcsrind.d: New testcase. It fails
without applying this patch.
Felix Willgerodt [Mon, 25 Mar 2024 15:57:37 +0000 (16:57 +0100)]
gdb: rename offset to high bits in ymm registers
The xsave_ymm_avx512_offset data structure contains the xsave
offset to the upper 128 bits of a ymm register. Similarly, for zmm this
offset is described by xsave_avx512_zmm_h_offset, h indicating the
high bits. This commit renames the xsave_ymm_avx512_offset to
xsave_ymm_h_avx512_offset - as well as the associated define from
XSAVE_YMM_AVX512_ADDR to XSAVE_YMM_H_AVX512_ADDR - to make this
more consistent.
Note, that the regnum defines already included the 'h' for ymm, like
I387_YMM16H_REGNUM and I387_YMMH_AVX512_END_REGNUM.
Co-authored-by: Nils-Christian Kempke <nils-christian.kempke@intel.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
This aligns LLVM:
* https://llvm.org/docs/RISCVUsage.html
* https://github.com/llvm/llvm-project/pull/83896
bfd/ChangeLog:
* elfxx-riscv.c (riscv_supported_vendor_x_ext): Add support for
'xsfcease'.
(riscv_multi_subset_supports): Handle INSN_CLASS_XSFCEASE.
(riscv_multi_subset_supports_ext): Handle INSN_CLASS_XSFCEASE.
gas/ChangeLog:
* doc/c-riscv.texi: Updated.
* testsuite/gas/riscv/march-help.l: Updated.
* testsuite/gas/riscv/sifive-insns.d: Add test case for 'sf.cease'.
* testsuite/gas/riscv/sifive-insns.s: Likewise.
include/ChangeLog:
* opcode/riscv-opc.h (MATCH_SF_CEASE, MASK_SF_CEASE): Define match and
mask encoding for 'sf.cease'.
* opcode/riscv.h (INSN_CLASS_XSFCEASE): Add new instruction class for
'xsfcease'.
opcodes/ChangeLog:
* riscv-opc.c (riscv_opcodes): Add opcode entry for 'sf.cease'.
Cui, Lili [Tue, 18 Jun 2024 02:46:31 +0000 (10:46 +0800)]
Remove %ME and used %NE for movbe.
%ME is added specifically for movbe. Now with %NE, we can use
MOD table + %NE to indicate whether a {evex} prefix is needed.
opcodes/ChangeLog:
* i386-dis-evex-mod.h: Added movbe.
* i386-dis-evex.h: Let movbe go through the mod table.
* i386-dis.c (struct dis386): Removed %ME.
(putop): Removed case ME.
Cui, Lili [Tue, 18 Jun 2024 02:45:49 +0000 (10:45 +0800)]
Support APX CCMP and CTEST
CCMP and CTEST are two new sets of instructions for conditional CMP
and TEST, SCC and OSZC flags are given as suffixes of CCMP or CTEST
in the instruction mnemonic, e.g.:
ccmp<cc> { dfv=sf , cf , of } %eax, %ecx
also add
{evex} cmp/test %eax, %ecx
as an alias for ccmpt.
For the encoder part, add function check_Scc_OszcOperation to parse
'{ dfv=of , sf, sf, cf}', store scc in the lower 4 bits of base_opcode,
and adjust base_opcode to its normal meaning in install_template.
For the decoder part, add 'SC' and 'DF' macros to add scc and oszc flags
suffixes.
gas/ChangeLog:
* config/tc-i386.c (OSZC_CF): New.
(OSZC_ZF): Ditto.
(OSZC_SF): Ditto.
(OSZC_OF): Ditto.
(set_oszc_flags): Set oszc flags and report error for using the same oszc flags twice.
(check_Scc_OszcOperations): Handle SCC OSZC flags.
(install_template): Add scc and oszc_flags.
(build_apx_evex_prefix): Encode SCC and oszc flags bits.
(parse_insn): Handle check_Scc_OszcOperations.
* testsuite/gas/i386/x86-64-apx-evex-promoted-bad.d: Add ivalid test case.
* testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s: Ditto.
* testsuite/gas/i386/x86-64.exp: Add test for ccmp and ctest.
* testsuite/gas/i386/x86-64-apx-ccmp-ctest-intel.d: New test.
* testsuite/gas/i386/x86-64-apx-ccmp-ctest-inval.l: Ditto.
* testsuite/gas/i386/x86-64-apx-ccmp-ctest-inval.s: Ditto.
* testsuite/gas/i386/x86-64-apx-ccmp-ctest.d: Ditto.
* testsuite/gas/i386/x86-64-apx-ccmp-ctest.s: Ditto.
opcodes/ChangeLog:
* i386-dis-evex-reg.h: Add ccmp and ctest.
* i386-dis-evex.h: Ditto.
* i386-dis.c (struct instr_info): add scc.
(struct dis386): Add new micro 'NE','SC' and'DF'.
(get_valid_dis386): Get scc value and move MAP4 invalid check to print_insn.
(putop): Handle %NE, %SC and %DF.
* i386-opc.h (SCC): New.
* i386-opc.tbl: Add ccmp/ctest and evex format for cmp/test.
* i386-mnem.h: Regenerated.
* i386-tbl.h: Ditto.
GAS/testsuite: Make a copy of none.s before operating on it as output
The "Output file must be distinct from input" test in gas/all/gas.exp
operates on none.s as output. Should the test fail it may happen that
GAS will delete the output file requested in which case none.s will be
removed. Since the test operates directly on the source tree it will be
clobbered as a result. It has actually been observed in the field in
the form of intermittent:
FAIL: gas/all/none
regressions in a parallel run of many configurations.
Prevent this from happening by copying none.s first to the test object
directory and operating on it instead. It does not prevent the file
from being removed should the test fail, but the source tree won't be
clobbered in that case.
A nice side effect is that syntactically different paths will now be
used in this test for the input and the output file each, so coverage
will extend to verifying that a file is checked against itself even if
referred to via different paths. Previously "$srcdir/$subdir/none.s"
was used for both paths and now "tmpdir/none.s" is referred to directly
and via a relative path from "$srcdir/$subdir" respectively.
I note that we have no previous use of the UNRESOLVED test result in the
GAS testsuite, but it seems the correct one should copying none.s fail,
as this is an unexpected situation that requires a human intervention
and the test proper has not been evaluated.
GAS/testsuite: Add a helper for paths outside the source dir
Implement a helper to construct a relative path from $srcdir/$subdir,
where `gas_run' operates, to an arbitrary place in the filesystem, for
example a file in the test object directory.
binutils/testsuite: Add a helper for relative path construction
Implement a helper to construct a relative path between two locations in
the filesystem, for example to make a path from the source to the object
directory for the case where a tool has been set up to look at a given
path and there is a need to point it elsewhere, but an absolute path
will not work. The helper works on normalized paths internally, so the
result is correct even in the presence of symlinks as intermediate path
components.
So given "/path/to/src/gas/testsuite/gas/all" as the FROM argument and
then "/path/to/obj/gas/testsuite/tmpdir/none.s" as the TO argument the
helper will return "../../../../../obj/gas/testsuite/tmpdir/none.s" in
the absence of symlinks.
Tom de Vries [Mon, 17 Jun 2024 21:26:03 +0000 (23:26 +0200)]
[gdb/testsuite] Fix duplicates in gdb.fortran/array-{indices,repeat}.exp
When running test-case gdb.fortran/array-indices.exp on a system without
fortran compiler, I run into a duplicate:
...
Running /home/vries/gdb/src/gdb/testsuite/gdb.fortran/array-indices.exp ...
gdb compile failed, default_target_compile: Can't find gfortran.
UNTESTED: gdb.fortran/array-indices.exp: array-indices.exp
gdb compile failed, default_target_compile: Can't find gfortran.
UNTESTED: gdb.fortran/array-indices.exp: array-indices.exp
DUPLICATE: gdb.fortran/array-indices.exp: array-indices.exp
...
Fix this by adding a with_test_prefix at the toplevel.
Likewise in gdb.fortran/array-repeat.exp.
Tested on x86_64-linux.
Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Alan Modra [Fri, 14 Jun 2024 22:13:08 +0000 (07:43 +0930)]
Error messages emitted during bfd_check_format_matches
Error/warning messages are only printed for the target that
successfully matched, which makes sense for warnings, but not so much
for errors where the errors cause no target to match. I noticed this
when looking at the pr20520 testcase again with objdump, which just
reports "file format not recognized" omitting the five "SHT_GROUP
section [index n] has no SHF_GROUP sections" messages. They are
omitted because multiple ELF targets match the object file. This is
going to be true for all ELF objects due to at least the proper ELF
target and the generic ELF target matching.
* format.c (print_and_clear_messages): Print messages if all
targets with messages have exactly the same set of messages.
Tom de Vries [Sat, 15 Jun 2024 06:10:44 +0000 (08:10 +0200)]
[gdb/build] Cleanup gdb/features/feature_to_c.sh
Clean up script gdb/features/feature_to_c.sh by:
- fixing shellcheck warnings,
- moving an embedded awk script out of the file, reducing the amount of
escaping in the awk script, making it more readable and maintainable, and
- adding emacs / vi settings for local tab size 2 (copied from ./ltmain.sh).
Tom de Vries [Sat, 15 Jun 2024 06:10:44 +0000 (08:10 +0200)]
[gdb/testsuite] Clean up formatting in gdb/contrib/cc-with-tweaks.sh
In emacs, on gdb/contrib/cc-with-tweaks.sh, do:
- M-x whitespace-cleanup,
- M-x mark-whole-buffer and M-x indent-region, and
- and undo the unwanted changes in the header comment.
Tom de Vries [Sat, 15 Jun 2024 06:10:44 +0000 (08:10 +0200)]
[gdb/testsuite] Clean up gdb/contrib/expect-read1.sh
Clean up script gdb/contrib/expect-read1.sh by:
- fixing shellcheck warnings,
- using mktemp (which takes TMPDIR into account) instead of a hardcoded
"/tmp/expect-read1.$$.so",
- adding comments, and
- adding emacs / vi settings for local tab size 2 (copied from ./ltmain.sh).
Tom Tromey [Thu, 23 May 2024 16:30:16 +0000 (10:30 -0600)]
Introduce language_defn::lookup_symbol_local
This introduces the new method language_defn::lookup_symbol_local, and
then changes lookup_symbol_local to use it. This removes an explicit
language check from this function, and makes it easier for other
languages to hook into this code.
Tom Tromey [Thu, 23 May 2024 15:35:46 +0000 (09:35 -0600)]
Simplify lookup_local_symbol
This simplifies lookup_local_symbol a little, by having it check
whether the current block is the static or global block, instead of
first searching for the static block.
Tom Tromey [Thu, 23 May 2024 15:21:09 +0000 (09:21 -0600)]
Examine template symbols in lookup_local_symbol
This changes lookup_local_symbol to directly examine any attached
template symbols, rather than gating this lookup on the use of C++ or
Fortran. As mentioned in an earlier patch, these objects are not
necessarily C++-specific, and doing the search generically seems
better.
This also renames cp_lookup_symbol_imports_or_template now that the
"template" part has been removed.
Tom Tromey [Thu, 23 May 2024 15:14:58 +0000 (09:14 -0600)]
Rename is_cplus_template_function
This patch renames is_cplus_template_function to is_template_function.
There is nothing C++-specific about this code, and the code in the
DWARF reader that creates these objects is not C++-specific. In fact
this may already be used by Rust (though I didn't check).
Use aarch64_features to describe register features in target descriptions.
There has been an issue with how aarch64 target descriptions are
cached within gdbserver, and specifically, how this caching impacts
the in process agent (IPA).
The function initialize_tracepoint_ftlib (gdbserver/tracepoint.cc) is
part of the IPA, this function is a constructor function, i.e. is
called as part of the global initialisation process. We can't
guarantee the ordering of when this function is called vs when other
global state is initialised.
Now initialize_tracepoint_ftlib calls initialize_tracepoint, which
calls initialize_low_tracepoint, which for aarch64 calls
aarch64_linux_read_description.
The aarch64_linux_read_description function lives in
linux-aarch64-tdesc.cc and after the above commit, depends on a
std::unordered_map having been initialized.
Prior to the above commit aarch64_linux_read_description used a global
C style array, which obviously requires no runtime initialization.
The consequence of the above is that any inferior linked with the IPA
(for aarch64) will experience undefined behaviour (access to an
uninitialized std::unordered_map) during startup, which for me
manifests as a segfault.
I propose fixing this by moving the std::unordered_map into the
function body, but leaving it static. The map will now be initialized
the first time the function is called, which removes the undefiend
behaviour.
The same problem exists for the expedited_registers global, however
this global can just be made into a function local instead. The
expedited_registers variable is used to build a pointer list which is
then passed to init_target_desc, however init_target_desc copies the
values it is given so expedited_registers does not need to live longer
than its containing function.
On most of the AArch64 machines I have access too tracing is not
supported, and so the gdb.trace/*.exp tests that use the IPA just exit
early reporting unsupported. I've added a test which links an
inferior with the IPA and just starts the inferior. No tracing is
performed. This exposes the current issue even on hosts that don't
support tracing. After this patch the test passes.
Andrew Burgess [Tue, 30 Jan 2024 15:37:23 +0000 (15:37 +0000)]
gdb/gdbserver: share x86/linux tdesc caching
This commit builds on the previous series of commits to share the
target description caching code between GDB and gdbserver for
x86/Linux targets.
The objective of this commit is to move the four functions (2 each of)
i386_linux_read_description and amd64_linux_read_description into the
gdb/arch/ directory and combine them so we have just a single copy of
each. Then GDB, gdbserver, and the in-process-agent (IPA) will link
against these shared functions.
One curiosity with this patch is the function
x86_linux_post_init_tdesc. On the gdbserver side the two functions
amd64_linux_read_description and i386_linux_read_description have some
functionality that is not present on the GDB side, there is some
additional configuration that is performed as each target description
is created, to setup the expedited registers.
To support this I've added the function x86_linux_post_init_tdesc.
This function is called from the two *_linux_read_description
functions, but is implemented separately for GDB and gdbserver.
An alternative approach that avoids adding x86_linux_post_init_tdesc
would be to have x86_linux_tdesc_for_tid return a non-const target
description, then in x86_target::low_arch_setup we could inspect the
target description to figure out if it is 64-bit or not, and modify
the target description as needed. In the end I think that adding the
x86_linux_post_init_tdesc function is the simpler solution.
The contents of gdbserver/linux-x86-low.cc have moved to
gdb/arch/x86-linux-tdesc-features.c, and gdbserver/linux-x86-tdesc.h
has moved to gdb/arch/x86-linux-tdesc-features.h, this change leads to
some updates in the #includes in the gdbserver/ directory.
This commit also changes how target descriptions are cached.
Previously both GDB and gdbserver used static C-style arrays to act as
the tdesc cache. This was fine, except for two problems. Either the
C-style arrays would need to be placed in x86-linux-tdesc-features.c,
which would allow us to use the x86_linux_*_tdesc_count_1() functions
to size the arrays for us, or we'd need to hard code the array sizes
using separate #defines, which we'd then have to keep in sync with the
rest of the code in x86-linux-tdesc-features.c.
Given both of these problems I decided a better solution would be to
just switch to using a std::unordered_map to act as the cache. This
will resize automatically, and we can use the xcr0 value as the key.
At first inspection, using xcr0 might seem to be a problem; after all
the {i386,amd64}_create_target_description functions take more than
just the xcr0 value. However, this patch is only for x86/Linux
targets, and for x86/Linux all of the other flags passed to the tdesc
creation functions have constant values and so are irrelevant when we
consider tdesc caching.
For testing I've done the following:
- Built on x86-64 GNU/Linux for all targets, and just for the native
target,
- Build on i386 GNU/Linux for all targets, and just for the native
target,
- Build on a 64-bit, non-x86 GNU/Linux for all targets, just for the
native target, and for targets x86_64-*-linux and i386-*-linux.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
Andrew Burgess [Wed, 31 Jan 2024 11:18:34 +0000 (11:18 +0000)]
gdbserver: update target description creation for x86/linux
This commit is part of a series which aims to share more of the target
description creation between GDB and gdbserver for x86/Linux.
After some refactoring earlier in this series the shared
x86_linux_tdesc_for_tid function was added into nat/x86-linux-tdesc.c.
However, this function still relies on amd64_linux_read_description
and i386_linux_read_description which are implemented separately for
both gdbserver and GDB. Given that at their core, all these functions
do is:
1. take an xcr0 value as input,
2. mask out some feature bits,
3. look for a cached pre-generated target description and return it
if found,
4. if no cached target description is found then call either
amd64_create_target_description or
i386_create_target_description to create a new target
description, which is then added to the cache. Return the newly
created target description.
The inner functions amd64_create_target_description and
i386_create_target_description are already shared between GDB and
gdbserver (in the gdb/arch/ directory), so the only thing that
the *_read_description functions really do is add the caching layer,
and it feels like this really could be shared.
However, we have a small problem.
Despite using the same {amd64,i386}_create_target_description
functions in both GDB and gdbserver to create the target descriptions,
on the gdbserver side we cache target descriptions based on a reduced
set of xcr0 feature bits.
What this means is that, in theory, different xcr0 values can map to
the same cache entry, which could result in the wrong target
description being used.
However, I'm not sure if this can actually happen in reality. Within
gdbserver we already split the target description cache based on i386,
amd64, and x32. I suspect within a given gdbserver session we'll only
see at most one target description for each of these.
The cache conflicting problem is caused by xcr0_to_tdesc_idx, which
maps an xcr0 value to a enum x86_linux_tdesc value, and there are only
7 usable values in enum x86_linux_tdesc.
In contrast, on the GDB side there are 64, 32, and 16 cache slots for
i386, amd64, and x32 respectively.
On the GDB side it is much more important to cache things correctly as
a single GDB session might connect to multiple different remote
targets, each of which might have slightly different x86
architectures.
And so, if we want to merge the target description caching between GDB
and gdbserver, then we need to first update gdbserver so that it
caches in the same way as GDB, that is, it needs to adopt a mechanism
that allows for the same number of cache slots of each of i386, amd64,
and x32. In this way, when the caching is shared, GDB's behaviour
will not change.
Unfortunately it's a little more complex than that due to the in
process agent (IPA).
When the IPA is in use, gdbserver sends a target description index to
the IPA, and the IPA uses this to find the correct target description
to use, the IPA having first generated every possible target
description.
Interestingly, there is certainly a bug here which results from only
having 7 values in enum x86_linux_tdesc. As multiple possible target
descriptions in gdbserver map to the same enum x86_linux_tdesc value,
then, when the enum x86_linux_tdesc value is sent to the IPA there is
no way for gdbserver to know that the IPA will select the correct
target description. This bug will get fixed by this commit.
** START OF AN ASIDE **
Back in the day I suspect this approach of sending a target
description index made perfect sense. However since this commit:
I think that passing an index was probably a bad idea.
We used to pass the index, and then use that index to lookup which
target description to instantiate and use, the target description was
not generated until the index arrived.
However, the above commit fixed an issue where we can't call malloc()
within (certain parts of) the IPA (apparently), so instead we now
pre-compute _every_ possible target description within the IPA. The
index is only used to lookup which of the (many) pre-computed target
descriptions to use.
It would (I think) have been easier all around if the IPA just
self-inspected, figured out its own xcr0 value, and used that to
create the one target description that is required. So long as the
xcr0 to target description code is shared (at compile time) with
gdbserver, then we can be sure that the IPA will derive the same
target description as gdbserver, and we would avoid all this index
passing business, which has made this commit so very, very painful.
I did look at how a process might derive its own xcr0 value, but I
don't believe this is actually that simple, so for now I've just
doubled down on the index passing approach.
While reviewing earlier iterations of this patch there has been
discussion about the possibility of removing the IPA from GDB. That
would certainly make all of the code touched in this patch much
simpler, but I don't really want to do that as part of this series.
** END OF AN ASIDE **
Currently then for x86/linux, gdbserver sends a number between 0 and 7
to the IPA, and the IPA uses this to create a target description.
However, I am proposing that gdbserver should now create one of (up
to) 64 different target descriptions for i386, so this 0 to 7 index
isn't going to be good enough any more (amd64 and x32 have slightly
fewer possible target descriptions, but still more than 8, so the
problem is the same).
For a while I wondered if I was going to have to try and find some
backward compatible solution for this mess. But after seeing how
lightly the IPA is actually documented, I wonder if it is not the case
that there is a tight coupling between a version of gdbserver and a
version of the IPA? At least I'm hoping so, and that's what I've
assumed in this commit.
In this commit I have thrown out the old IPA target description index
numbering scheme, and switched to a completely new numbering scheme.
Instead of the index that is passed being arbitrary, the index is
instead calculated from the set of xcr0 features that are present on
the target. Within the IPA we can then reverse this logic to recreate
the xcr0 value based on the index, and from the xcr0 value we can
choose the correct target description.
With the gdbserver to IPA numbering scheme issue resolved I have then
update the gdbserver versions of amd64_linux_read_description and
i386_linux_read_description so that they cache target descriptions
using the same set of xcr0 features as GDB itself.
After this gdbserver should now always come up with the same target
description as GDB does on any x86/Linux target.
This commit does not introduce any new code sharing between GDB and
gdbserver as previous commits in this series have done. Instead this
commit is all about bringing GDB and gdbserver into alignment
functionally so that the next commit(s) can merge the GDB and
gdbserver versions of these functions.
Notes On The Implementation
---------------------------
Previously, within gdbserver, target descriptions were cached within
arrays. These arrays were sized based on enum x86_linux_tdesc and
xcr0_to_tdesc_idx returned the array (cache) index.
Now we need different array lengths for each of i386, amd64, and x32.
And the index to use within each array is calculated based on which
xcr0 bits are set and valid for a particular target type.
I really wanted to avoid having fixed array sizes, or having the set
of relevant xcr0 bits encoded in multiple places.
The solution I came up with was to create a single data structure
which would contain a list of xcr0 bits along with flags to indicate
which of the i386, amd64, and x32 targets the bit is relevant for. By
making the table constexpr, and adding some constexpr helper
functions, it is possible to calculate the sizes for the cache arrays
at compile time, as well as the bit masks needed to each target type.
During review it was pointed out[1] that possibly the failure to check
the SSE and X87 bits for amd64/x32 targets might be an error, however,
if this is the case then this is an issue that existed long before
this patch. I'd really like to keep this patch focused on reworking
the existing code and try to avoid changing how target descriptions
are actually created, mostly out of fear that I'll break something.
Andrew Burgess [Thu, 25 Jan 2024 14:25:57 +0000 (14:25 +0000)]
gdb/gdbserver: share some code relating to target description creation
This commit is part of a series to share more of the x86 target
description creation code between GDB and gdbserver.
Unlike previous commits which were mostly refactoring, this commit is
the first that makes a real change, though that change should mostly
be for gdbserver; I've largely adopted the "GDB" way of doing things
for gdbserver, and this fixes a real gdbserver bug.
On a x86-64 Linux target, running the test:
gdb.server/connect-with-no-symbol-file.exp
results in two core files being created. Both of these core files are
from the inferior process, created after gdbserver has detached.
In this test a gdbserver process is started and then, after gdbserver
has started, but before GDB attaches, we either delete the inferior
executable, or change its permissions so it can't be read. Only after
doing this do we attempt to connect with GDB.
As GDB connects to gdbserver, gdbserver attempts to figure out the
target description so that it can send the description to GDB, this
involves a call to x86_linux_read_description.
In x86_linux_read_description one of the first things we do is try to
figure out if the process is 32-bit or 64-bit. To do this we look up
the executable via the thread-id, and then attempt to read the
architecture size from the executable. This isn't going to work if
the executable has been deleted, or is no longer readable.
And so, as we can't read the executable, we default to an i386 target
and use an i386 target description.
A consequence of using an i386 target description is that addresses
are assumed to be 32-bits. Here's an example session that shows the
problems this causes. This is run on an x86-64 machine, and the test
binary (xx.x) is a standard 64-bit x86-64 binary:
Notice the difference between the mappings reported via GDB and those
reported directly from the kernel via /proc/PID/maps, the addresses of
every mapping is clamped to 32-bits for GDB, while the kernel reports
real 64-bit addresses.
Notice also that the $pc value is a 32-bit value. It appears to be
within one of the mappings reported by GDB, but is outside any of the
mappings reported from the kernel.
And this is where the problem arises. When gdbserver detaches from
the inferior we pass the inferior the address from which it should
resume. Due to the 32/64 bit confusion we tell the inferior to resume
from the 32-bit $pc value, which is not within any valid mapping, and
so, as soon as the inferior resumes, it segfaults.
If we look at how GDB (not gdbserver) figures out its target
description then we see an interesting difference. GDB doesn't try to
read the executable. Instead GDB uses ptrace to query the thread's
state, and uses this to figure out the if the thread is 32 or 64 bit.
If we update gdbserver to do it the "GDB" way then the above problem
is resolved, gdbserver now sees the process as 64-bit, and when we
detach from the inferior we give it the correct 64-bit address, and
the inferior no longer segfaults.
Now, I could just update the gdbserver code, but better, I think, to
share one copy of the code between GDB and gdbserver in gdb/nat/.
That is what this commit does.
The cores of x86_linux_read_description from gdbserver and
x86_linux_nat_target::read_description from GDB are moved into a new
file gdb/nat/x86-linux-tdesc.c and combined into a single function
x86_linux_tdesc_for_tid which is called from each location.
This new function does things mostly the GDB way, some changes are
needed to allow for the sharing; we now take some pointers for where
the shared code can cache the xcr0 and xsave layout values.
Another thing to note about this commit is how the functions
i386_linux_read_description and amd64_linux_read_description are
handled. For now I've left these function as implemented separately
in GDB and gdbserver. I've moved the declarations of these functions
into gdb/arch/{i386,amd64}-linux-tdesc.h, but the implementations are
left where they are.
A later commit in this series will make these functions shared too,
but doing this is not trivial, so I've left that for a separate
commit. Merging the declarations as I've done here ensures that
everyone implements the function to the same API, and once these
functions are shared (in a later commit) we'll want a shared
declaration anyway.
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com> Acked-By: John Baldwin <jhb@FreeBSD.org>
Andrew Burgess [Wed, 27 Mar 2024 14:30:48 +0000 (14:30 +0000)]
gdb: move xcr0 == 0 check into i386_linux_core_read_description
Currently, in i386_linux_core_read_description, if GDB fails to
extract an xcr0 value from the core file, then we will have a default
zero value for the xcr0 variable, we still call the
i386_linux_read_description function, which checks for this zero value
and returns nullptr.
Back in i386_linux_core_read_description we spot the nullptr return
value from i386_linux_read_description and call
i386_linux_read_description again, but this time passing a default
value for xcr0.
In the next commit I plan to rework i386_linux_read_description, and
in so doing I will remove the check for xcr0 == 0, this is inline with
how the amd64 code is written.
However, this means that the 'xcr0 == 0' check needs to move up the
stack to i386_linux_core_read_description, again, this brings the i386
code into line with the amd64 code.
This is just a refactor in preparation for the next commit, there
should be no user visible changes after this commit.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com> Approved-By: John Baldwin <jhb@FreeBSD.org>
Andrew Burgess [Sat, 27 Jan 2024 09:15:35 +0000 (09:15 +0000)]
gdb/x86: move reading of cs and ds state into gdb/nat directory
This patch is part of a series that has the aim sharing the x86 Linux
target description creation code between GDB and gdbserver.
Within GDB part of this process involves reading the cs and ds state
from the 'struct user_regs_struct' using a ptrace call.
This isn't done by gdbserver, which is part of the motivation for this
whole series; the approach gdbserver takes is inferior to the approach
GDB takes (gdbserver relies on reading the file being debugged, and
extracting similar information from the file headers).
This commit moves the reading of cs and ds, which is used to figure
out if a thread is 32-bit or 64-bit (or in x32 mode), into the gdb/nat
directory so that the code can be shared with gdbserver, but at this
point I'm not actually using the code in gdbserver, that will come
later.
As such there should be no user visible changes after this commit, GDB
continues to do things as it did before (reading cs/ds), while
gdbserver continues to use its own approach (which doesn't require
reading cs/ds).
Approved-By: John Baldwin <jhb@FreeBSD.org> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
Andrew Burgess [Wed, 8 May 2024 14:04:56 +0000 (15:04 +0100)]
gdb: move have_ptrace_getregset declaration into gdb/nat directory
In a later commit I want to access have_ptrace_getregset from a .c
file in the nat/ directory. To achieve this I need access to the
declaration of have_ptrace_getregset.
Currently have_ptrace_getregset is declared (and defined) twice, once
in GDB and once in gdbserver.
This commit moves the declaration into nat/linux-nat.h, but leaves the
two definitions where they are. Now, in my later commit, I can pull
in the declaration from nat/linux-nat.h.
There should be no user visible changes after this commit.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
Andrew Burgess [Fri, 26 Apr 2024 13:24:40 +0000 (14:24 +0100)]
gdb/x86: move have_ptrace_getfpxregs global into gdb/nat directory
The have_ptrace_getfpxregs global tracks whether GDB or gdbserver is
running on a kernel that supports the GETFPXREGS ptrace request.
Currently this global is declared twice (once in GDB and once in
gdbserver), I think it makes sense to move this global into the nat/
directory, and have a single declaration and definition.
While moving this variable I have converted it to a tribool, as that
was what it really was, if even used the same numbering as the tribool
enum (-1, 0, 1). Where have_ptrace_getfpxregs was used I have updated
in the obvious way.
However, while making this change I noticed what I think is a bug in
x86_linux_nat_target::read_description and x86_linux_read_description,
both of these functions can be called multiple times, but in both
cases we only end up calling i386_linux_read_description the first
time through in the event that PTRACE_GETFPXREGS is not supported.
This is because initially have_ptrace_getfpxregs will be
TRIBOOL_UNKNOWN, but after the ptrace call fails we set
have_ptrace_getfpxregs to TRIBOOL_FALSE. The next time we attempt to
read the target description we'll skip the ptrace call, and so skip
the call to i386_linux_read_description.
I've not tried to address this preexisting bug in this commit, this is
purely a refactor, there should be no user visible changes after this
commit. In later commits I'll merge the gdbserver and GDB code
together into the nat/ directory, and after that I'll try to address
this bug.
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
Andrew Burgess [Sat, 27 Jan 2024 09:33:14 +0000 (09:33 +0000)]
gdbserver/x86: move no-xml code earlier in x86_linux_read_description
This commit is part of a series that aims to share more of the x86
target description reading/generation code between GDB and gdbserver.
There are a huge number of similarities between the code in
gdbserver's x86_linux_read_description function and GDB's
x86_linux_nat_target::read_description function, and it is this
similarity that I plan, in a later commit, to share between GDB and
gdbserver.
However, one thing that is different in x86_linux_read_description is
the code inside the '!use_xml' block. This is the code that handles
the case where gdbserver is not allowed to send an XML target
description back to GDB. In this case gdbserver uses some predefined,
fixed, target descriptions.
First, it's worth noting that I suspect this code is not tested any
more. I couldn't find anything in the testsuite that tries to disable
XML target description support. And the idea of having a single
"fixed" target description really doesn't work well when we think
about all the various x86 extensions that exist. Part of me would
like to rip out the no-xml support in gdbserver (at least for x86),
and if a GDB connects that doesn't support XML target descriptions,
gdbserver can just give an error and drop the connection. GDB has
supported XML target descriptions for 16 years now, I think it would
be reasonable for our shipped gdbserver to drop support for the old
way of doing things.
Anyway.... this commit doesn't do that.
What I did notice was that, over time, the '!use_xml' block appears to
have "drifted" within the x86_linux_read_description function; it's
now not the first check we do. Instead we make some ptrace calls and
return a target description generated based on the result of these
ptrace calls. Surely it only makes sense to generate variable target
descriptions if we can send these back to GDB?
So in this commit I propose to move the '!use_xml' block earlier in
the x86_linux_read_description function.
The benefit of this is that this leaves the later half of
x86_linux_read_description much more similar to the GDB function
x86_linux_nat_target::read_description and sets us up for potentially
sharing code between GDB and gdbserver in a later commit.
Approved-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
Tom de Vries [Thu, 13 Jun 2024 15:29:37 +0000 (17:29 +0200)]
[gdb/testsuite] Fix kfail number in gdb.base/watchpoint-running.exp
Test-case gdb.base/watchpoint-running.exp reports the following kfail:
...
KFAIL: $exp: all-stop: software: watchpoint hit (timeout) (PRMS: gdb/111111)
...
but the referenced gdb PR doesn't exist.
MIPS/opcodes: Rework INSN_* flags into a consistent block
For historic reasons we have ended up with a random set of discontiguous
bit assignments for INSN_* flags within `membership' and `exclusions'
members of `mips_opcode'. Some of the bits were previously used for ASE
assignments and have been reused in a disorganised fashion since `ase'
has been split off as a member on its own. It makes them hard to track
and maintain, and to see how many we still have available for future
assignments.
Therefore reorder the flags using consecutive bits and matching the
order used with the switch statement in `cpu_is_member'.
Tom de Vries [Wed, 12 Jun 2024 17:15:45 +0000 (19:15 +0200)]
[gdb/testsuite] Fix error in gdb.server/server-kill-python.exp
With test-case gdb.server/server-kill-python.exp, I sometimes run into:
...
builtin_spawn gdb -nw -nx -q -iex set height 0 -iex set width 0 \
-data-directory data-directory^M
kill^M
(gdb) kill^M
file server-kill-python^M
The program is not being run.^M
(gdb) ERROR: Couldn't load server-kill-python into GDB.
...
The problem is that the spawn produces a prompt, but it's not explicitly
consumed.
This is a regression since commit 0f077fcae0f ("[gdb/testsuite] Simplify
gdb.server/server-kill-python.exp").
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.