Tom de Vries [Tue, 23 Jul 2024 12:38:16 +0000 (14:38 +0200)]
[gdb/testsuite] Add xfail in gdb.base/hbreak.exp
On an aarch64-linux system with 32-bit userland running in a chroot, and using
target board unix/mthumb I get:
...
(gdb) hbreak hbreak.c:27^M
Hardware assisted breakpoint 2 at 0x4004e2: file hbreak.c, line 27.^M
(gdb) PASS: gdb.base/hbreak.exp: hbreak
continue^M
Continuing.^M
Unexpected error setting breakpoint: Invalid argument.^M
(gdb) FAIL: gdb.base/hbreak.exp: continue to break-at-exit after hbreak
...
due to this call in arm_linux_nat_target::low_prepare_to_resume:
...
if (ptrace (PTRACE_SETHBPREGS, pid,
(PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
perror_with_name (_("Unexpected error setting breakpoint"));
...
This problem does not happen if instead we use a 4-byte aligned address.
I'm not sure if this is simply unsupported or if there's a kernel bug of some
sort, but I don't see what gdb can do about this.
Tom de Vries [Tue, 23 Jul 2024 12:27:36 +0000 (14:27 +0200)]
[gdb/testsuite] Fix gdb.ada/mi_task_arg.exp on arm-linux
On arm-linux, I run into:
...
PASS: gdb.ada/mi_task_arg.exp: mi runto task_switch.break_me
Expecting: ^(-stack-list-arguments 1[^M
]+)?(\^done,stack-args=\[frame={level="0",args=\[\]},frame={level="1",args=\[{name="<_task>",value="0x[0-9A-Fa-f]+"}(,{name="<_taskL>",value="[0-9]+"})?\]},frame={level="2",args=\[({name="self_id",value="(0x[0-9A-Fa-f]+|<optimized out>)"})?\]},.*[^M
]+[(]gdb[)] ^M
[ ]*)
-stack-list-arguments 1^M
^done,stack-args=[frame={level="0",args=[]},frame={level="1",args=[{name="<_task>",value="0x40bc48"}]},frame={level="2",args=[]}]^M
(gdb) ^M
FAIL: gdb.ada/mi_task_arg.exp: -stack-list-arguments 1 (unexpected output)
...
The problem is that the test-case expects a level 3 frame, but there is none.
This can be reproduced using cli bt:
...
$ gdb -q -batch outputs/gdb.ada/mi_task_arg/task_switch \
-ex "b task_switch.break_me" \
-ex run \
-ex bt
Breakpoint 1 at 0x34b4: file task_switch.adb, line 57.
Thread 3 "my_caller" hit Breakpoint 1, task_switch.break_me () \
at task_switch.adb:57
57 null;
#0 task_switch.break_me () at task_switch.adb:57
#1 0x00403424 in task_switch.caller (<_task>=0x40bc48) at task_switch.adb:51
#2 0xf7f95a08 in ?? () from /lib/arm-linux-gnueabihf/libgnarl-12.so
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
...
The purpose of the test-case is printing the frame at level 1, so I don't
think we should bother about the presence of the frame at level 3.
Fix this by allowing the backtrace to stop at level 2.
Tested on arm-linux.
Approved-By: Luis Machado <luis.machado@arm.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
The comment relates to how 'E.msg' style remote replies are not
supported by every packet, but after the above commit they are
supported in all cases (that call packet_check_result), and the
comment should have been removed.
Alan Modra [Sun, 21 Jul 2024 11:33:22 +0000 (21:03 +0930)]
Don't trim trailing newline in run_host_cmd
Testcases like ld-elf/pr19719a.c that
printf ("PASS\n");
on success ought to see the whole output for "string match".
Similarly, the ld-pe/ pdb*.d files shouldn't need to remove the last
newline to match. For most of the testsuite it doesn't matter whether
the trailing newline is present or not, and there are only a few cases
where we need to remove it.
* testsuite/lib/ld-lib.exp (run_host_cmd): Don't regsub away
output trailing newline. Do string trim for gcc/ld version checks.
* testsuite/config/default.exp (plug_so): Do string trim output of
run_host_cmd.
* testsuite/ld-elf/shared.exp (mix_pic_and_non_pic): Adjust
string match to include trailing newline.
* testsuite/ld-i386/i386.exp (undefined_weak): Likewise.
* testsuite/ld-x86-64/x86-64.exp (undefined_weak): Likewise.
* testsuite/ld-plugin/libdep.exp (run_test): Likewise.
* testsuite/ld-plugin/lto.exp (PR ld/28138 run): Likewise.
* testsuite/ld-pe/pdb-strings.d,
* testsuite/ld-pe/pdb-syms1-globals.d,
* testsuite/ld-pe/pdb-syms1-records.d,
* testsuite/ld-pe/pdb-syms1-symbols1.d,
* testsuite/ld-pe/pdb-syms1-symbols2.d,
* testsuite/ld-pe/pdb-syms2-symbols1.d,
* testsuite/ld-pe/pdb-types1-hashlist.d,
* testsuite/ld-pe/pdb-types1-skiplist.d,
* testsuite/ld-pe/pdb-types1-typelist.d,
* testsuite/ld-pe/pdb-types2-hashlist.d,
* testsuite/ld-pe/pdb-types2-skiplist.d,
* testsuite/ld-pe/pdb-types2-typelist.d,
* testsuite/ld-pe/pdb-types3-hashlist.d,
* testsuite/ld-pe/pdb-types3-skiplist.d,
* testsuite/ld-pe/pdb-types3-typelist.d,
* testsuite/ld-pe/pdb1-publics.d,
* testsuite/ld-pe/pdb1-sym-record.d,
* testsuite/ld-pe/pdb2-section-contrib.d,
* testsuite/ld-pe/pdb3-c13-info1.d,
* testsuite/ld-pe/pdb3-c13-info2.d,
* testsuite/ld-pe/pdb3-source-info.d: Add trailing newline.
Tom de Vries [Sun, 21 Jul 2024 08:03:09 +0000 (10:03 +0200)]
[gdb/testsuite] Fix scopes check in gdb.dap/rust-slices.exp
When running test-case gdb.dap/rust-slices.exp on aarch64-linux
(debian 12/bookworm), I run into:
...
{"request_seq": 6, "type": "response", "command": "scopes", "body": {"scopes": [{"variablesReference": 1, "name": "Locals", "presentationHint": "locals", "expensive": false, "namedVariables": 3, "line": 28, "source": {"name": "rust-slices.rs", "path": "/home/linux/gdb/binutils-gdb.git/gdb/testsuite/gdb.dap/rust-slices.rs"}}, {"variablesReference": 2, "name": "Registers", "presentationHint": "registers", "expensive": false, "namedVariables": 261, "line": 28, "source": {"name": "rust-slices.rs", "path": "/home/linux/gdb/binutils-gdb.git/gdb/testsuite/gdb.dap/rust-slices.rs"}}]}, "success": true, "seq": 20}PASS: gdb.dap/rust-slices.exp: get scopes success
FAIL: gdb.dap/rust-slices.exp: three scopes
PASS: gdb.dap/rust-slices.exp: scope is locals
PASS: gdb.dap/rust-slices.exp: locals presentation hint
PASS: gdb.dap/rust-slices.exp: three vars in scope
...
The test-case expects three scopes due to a rust compiler issue:
...
# There are three scopes because an artificial symbol ends up in the
# DWARF. See https://github.com/rust-lang/rust/issues/125126.
gdb_assert {[llength $scopes] == 3} "three scopes"
...
but it seems that the version used here (rustc 1.63.0, llvm 14.0.6) doesn't
have this issue.
Fix this by allowing two or three scopes, and changing the test name to
"two scopes".
Tested on aarch64-linux.
Approved-by: Kevin Buettner <kevinb@redhat.com>
PR testsuite/31983
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31983
Mark Harmstone [Sat, 13 Jul 2024 14:30:46 +0000 (15:30 +0100)]
ld/testsuite: add missing definition to PDB test
The file pdb-syms1a.s was missing a definition for T_VOID, which was
causing some types not to be deduplicated. It also meant that the test
couldn't be run against LLVM's lld, which throws an error for this.
This particular test only tests the symbols stream, not the types
stream, which is why the deduplication doesn't result in a change in the
file size.
Mark Harmstone [Sat, 13 Jul 2024 14:36:34 +0000 (15:36 +0100)]
ld/PDB: use correct hashing algorithm in add_globals_ref
add_globals_ref was hashing using CRC32 rather than the hashing
algorithm used for symbols, which meant that windbg was unable to put
breakpoints against unmangled names.
The tracepoint_probe_create_sals_from_location_spec function just
forwards all its arguments to
bkpt_probe_create_sals_from_location_spec, and is only used in one
place.
Lets delete tracepoint_probe_create_sals_from_location_spec and
replace it with bkpt_probe_create_sals_from_location_spec.
There should be no user visible changes after this commit.
Andrew Burgess [Fri, 14 Apr 2023 13:45:38 +0000 (14:45 +0100)]
gdb: remove breakpoint_re_set_one
During a later patch I wanted to reset a single breakpoint, so I
called breakpoint_re_set_one. However, this is not the right thing to
do. If we look at breakpoint_re_set then we see that there's a whole
bunch of state that needs to be preserved prior to calling
breakpoint_re_set_one, and after calling breakpoint_re_set_one we
still need to call update_global_location_list.
I could just update the comment on breakpoint_re_set_one to make it
clearer how the function should be used -- or more likely to warn that
the function should only be used as a helper from breakpoint_re_set.
However, breakpoint_re_set_one is only 3 lines long. So I figure it
might actually be easier to just fold breakpoint_re_set_one into
breakpoint_re_set, then there's no risk of accidentally calling
breakpoint_re_set_one when we shouldn't.
There should be no user visible changes after this commit.
Andrew Burgess [Thu, 13 Apr 2023 14:12:17 +0000 (15:12 +0100)]
gdb: don't display inferior list for pending breakpoints
I noticed that in the 'info breakpoints' output, GDB sometimes prints
the inferior list for pending breakpoints, this doesn't seem right to
me. A pending breakpoint has no locations (at least, as far as we
display things in the 'info breakpoints' output), so including an
inferior list seems odd.
Here's what I see right now:
(gdb) info breakpoint 5
Num Type Disp Enb Address What
5 breakpoint keep y <PENDING> foo inf 1
(gdb)
It's the 'inf 1' at the end of the line that I'm objecting too.
To trigger this behaviour we need to be in a multi-inferior debug
session. The breakpoint must have been non-pending at some point in
the past, and so have a location assigned to it.
The breakpoint becomes pending again as a result of a shared library
being unloaded. When this happens the location itself is marked
pending (via bp_location::shlib_disabled).
In print_one_breakpoint_location, in order to print the inferior list
we check that the breakpoint has a location, and that we have multiple
inferiors, but we don't check if the location itself is pending.
This commit adds that check, which means the output is now:
(gdb) info breakpoint 5
Num Type Disp Enb Address What
5 breakpoint keep y <PENDING> foo
(gdb)
Which I think makes more sense -- indeed, the format without the
inferior list is what we display for a pending breakpoint that has
never had any locations assigned, so I think this change in behaviour
makes GDB more consistent.
Alan Modra [Sat, 20 Jul 2024 02:53:59 +0000 (12:23 +0930)]
Re: binutils: Add a test for strip with build notes
The new test wasn't being run, and failed due to relocations against
.gnu.build.attributes being stripped by default strip behaviour.
We probably should be keeping these relocations, but I haven't made
that change here.
BTW, the new test fails on ia64-hpux but that's just a repeat of the
existing note-5 fail.
PR 31999
* testsuite/binutils-all/strip-16.d: strip with --strip-unneeded
and --merge-notes.
* testsuite/binutils-all/objcopy.exp: Run the new test. Sort
other strip tests.
MIPS/opcodes: Mark MT thread context move assembly idioms as aliases
A number of instructions in the regular MIPS opcode table are assembly
idioms for the MT thread context move MFTR and MTTR instructions, so
mark them as aliases accordingly. Add suitable test cases, which also
cover the PAUSE assembly idiom.
PAUSE is an assembly idiom for 'sll $0,$0,5', so mark it as an alias in
the regular MIPS opcode table, matching the microMIPS opcode table. A
test case will be supplied separately.
A number of coprocessor move encodings have been randomly sprinkled over
the regular MIPS and microMIPS opcode tables rather than where they'd be
expected following the alphabetic order. Fix the ordering, taking into
account precedence where it has to be observed for correct disassembly.
No functional change.
Make AL a shorthand for INSN2_ALIAS with the regular MIPS and microMIPS
opcode tables, just as with the MIPS16 opcode table, and use it
throughout. No functional change.
YunQiang Su [Fri, 19 Jul 2024 18:01:52 +0000 (19:01 +0100)]
MIPS/opcodes: Remove the regular MIPS "+t" operand code
The semantics of the regular MIPS "+t" operand code is exactly the same
as that of the "E" operand code, so replace the former with the latter
in the single MFTC0 instruction with implicit 'sel' == 0 encoding where
it's used, matching the encoding with explicit 'sel' as well as other
instructions.
MIPS/opcodes: Output thread context registers numerically with MFTR/MTTR
We print MFTR and MTTR instructions' thread context register operand in
disassembly using the ABI name the register number would correspond to
should the targeted register be a general-purpose register.
However in most cases it is wrong, because general-purpose registers are
only referred when the 'u' and 'sel' operands are 1 and 0 respectively.
And even in these cases the MFGPR and MTGPR aliases take precedence over
the corresponding generic instruction encodings, so you won't see the
valid case to normally trigger.
Conversely decoding the thread context register operand numerically is
always valid, so switch to using it. Adjust test coverage accordingly.
MIPS/opcodes: Discard unused OP_SH, OP_MASK, and OP_OP macros
As from commit ab90248154ba ("Add structures to describe MIPS
operands"), <https://sourceware.org/ml/binutils/2013-07/msg00135.html>,
the use of numerous regular MIPS and microMIPS OP_SH and OP_MASK macros
has been removed.
Similarly as from commit c3c0747817f4 ("Use operand structures for
MIPS16"), <https://sourceware.org/ml/binutils/2013-07/msg00136.html>,
the use of numerous MIPS16 OP_SH and OP_MASK macros has been removed.
And as from commit 9e12b7a2b022 ("Rewrite main mips_ip parsing loop"),
<https://sourceware.org/ml/binutils/2013-07/msg00139.html>, none of the
OP_OP macros are used anymore.
Discard all the unused macros then and only keep the small subset that
is still referred. This simplifies maintenance and removes the need to
keep the artificial arrangement where some regular MIPS and microMIPS
macros expand to 0 and are kept for compatibility with the opposite ISA
mode only, as it used to be required before the commit referred.
MIPS/opcodes: Correct documentation for R6 operand types
The "-t", "-u", "-v", and "-w" operand types refer 'rt' operand, which
is the target register rather than the source register. Additionally
the "-x" and "-y" R6 operand types refer 'rs' rather than 'rt' operand
of the BOVC/BNVC and the BEQC/BNEC instructions respectively. Also the
"-x" operand type does not permit 'rs' to be the same as 'rt'.
Correct inline documentation in opcode/mips.h accordingly.
MIPS/opcodes: Exclude $0 from "-x" R6 operand type
The "-x" operand type is used for the reverse encoding of the BOVC and
BNVC instructions, where 'rs' and 'rt' have been supplied as the second
and the first operand respectively rather than the order the instruction
expects.
In this case we require the register associated with the "-x" operand to
have a higher number than the register associated with the preceding "t"
operand, which precludes the use of $0. The case where 'rs' and 'rt'
both refer to the same register is handled by the straight encoding of
the BOVC and BNVC instructions, which come in the opcode table ahead of
the corresponding reverse encoding.
Therefore clear the ZERO_OK flag for the "-x" operand. No need for an
extra test case as the encodings involved are already covered by "r6"
and its associated GAS tests.
Jan Beulich [Fri, 19 Jul 2024 09:58:58 +0000 (11:58 +0200)]
Sparc: relax gas testsuite whitespace expectations
In a subsequent change the scrubber is going to be changed to retain
further whitespace. Test case expectations generally would better not
depend on the specific whitespace treatment by the scrubber, unless of
course a test is specifically about it. Adjust relevant test cases to
permit blanks where those will subsequently appear.
Jan Beulich [Fri, 19 Jul 2024 09:58:26 +0000 (11:58 +0200)]
TilePro: correct macro use in gas testsuite
Whitespace in macro arguments either needs quoting / parenthesizing to
reliably not be mistaken for an argument separator, or respective macro
parameters need to be marked as covering all remaining arguments. The
latter appears more appropriate (and far less intrusive) here.
Jan Beulich [Fri, 19 Jul 2024 09:58:04 +0000 (11:58 +0200)]
MIPS: correct macro use in gas and ld testsuites
Whitespace in macro arguments either needs quoting / parenthesizing to
reliably not be mistaken for an argument separator, or respective macro
parameters need to be marked as covering all remaining arguments. The
former appears more appropriate here, as the macro parameters already
have ":req".
Jan Beulich [Fri, 19 Jul 2024 09:57:12 +0000 (11:57 +0200)]
ia64: correct macro use in gas testsuite
Whitespace in macro arguments either needs quoting / parenthesizing to
reliably not be mistaken for an argument separator, or respective macro
parameters need to be marked as covering all remaining arguments. The
latter appears more appropriate here.
Jan Beulich [Fri, 19 Jul 2024 09:56:46 +0000 (11:56 +0200)]
bfin: drop _ASSIGN_BANG
A few testcases demonstrate that "=!" isn't supposed to be an
individual token, since "= !" is used in a number of places. So far
lexing that to a single token worked because of the scrubber being
overly aggressive in removing whitespace. As that's going to change,
replace uses by separate ASSIGN and BANG.
Jan Beulich [Fri, 19 Jul 2024 09:56:20 +0000 (11:56 +0200)]
bfin: correct macro use in gas testsuite
Whitespace in macro arguments either needs quoting / parenthesizing to
reliably not be mistaken for an argument separator, or respective macro
parameters need to be marked as covering all remaining arguments. The
latter really isn't an option here.
Jan Beulich [Fri, 19 Jul 2024 09:55:54 +0000 (11:55 +0200)]
Arm: correct macro use in gas testsuite
The way the inner macro invocations are written doesn't quite work as
expected (and would actually break subsequently): Due to overly
aggressive removal of whitespace by the scrubber, the incoming \sym and
\offset arguments actually get concatenated; an empty 3rd argument is
being passed to ldrtest2. That just so happened to work as intended; any
use of \offset alone would have exposed the problem. Quote the 3rd
argument, thus retaining enough whitespace to be independent of scrubber
internals.
Jan Beulich [Fri, 19 Jul 2024 09:54:45 +0000 (11:54 +0200)]
gas: adjust impossible/bogus M68K/MRI special case when scrubbing
State 1 is uniformly handled further up. And it is highly questionable
that in state 10 (i.e. after having seen not only a possible label, but
also an opcode), which is about to go away anyway, a line comment char
could still be meant to take effect. With the state checking dropped,
the immediately preceding logic can then also be simplified.
Jan Beulich [Fri, 19 Jul 2024 09:54:09 +0000 (11:54 +0200)]
gas: consistently drop trailing whitespace when scrubbing
From especially the checks for the two separator forms it appears to
follow that the construct being touched is about trailing whitespace. In
such a case, considering that for many targets ordinary and line comment
chars overlap, take into account that line comment chars override
ordinary ones in lex[] (logic elsewhere in do_scrub_chars() actually
depends on that ordering, and also accounts for this overriding).
Plus of course IS_NEWLINE() would better also be consulted. Note also
that the DOUBLESLASH_LINE_COMMENTS change should generally have no
effect just yet; it's a prereq for a later change but better fits here.
Leave respective comments as well, and update documentation to correct
which comment form is actually replaced by a single blank (i.e. neither
the ones starting with what {,tc_}comment_chars[] has nor the ones
starting with what line_comment_chars[] has).
Jan Beulich [Fri, 19 Jul 2024 09:53:24 +0000 (11:53 +0200)]
gas: drop tic6x scrubber special case
Two successive PUT() without a state change in between can't be right:
The first PUT() may take the "goto tofull" path, leading to the
subsequent character being processed later in the previously set state
(1 in this case), rather than the state we were in upon entry to the
switch() (13 in this case).
However, the original purpose of that logic appears to be to not mistake
"|| ^" for "||^". This effect, sadly, looks to not have been achieved.
Therefore drop the special case altogether; something that actually
achieves the (presumably) intended effect may then be introduced down
the road.
Jan Beulich [Fri, 19 Jul 2024 09:52:21 +0000 (11:52 +0200)]
x86: accept whitespace inside curly braces
Other than documented /**/ comments currently aren't really converted to
a single space, at least not for x86 in its most common configurations.
That'll be fixed subsequently, at which point blanks may appear where so
far none were expected. Furthermore not permitting blanks immediately
inside curly braces wasn't quite logical anyway - such constructs are
composite ones, and hence components ought to have been permitted to be
separated by whitespace from the very beginning.
With this we also don't care anymore whether the scrubber would remove
whitespace around curly braces, so move them from extra_symbol_chars[]
to operand_special_chars[].
Note: The new testcase doesn't actually exercise much (if any) of the
added code. It is being put in place to ensure that subsequently, when
that code actually comes into play, behavior remains the same.
Jan Beulich [Fri, 19 Jul 2024 09:44:07 +0000 (11:44 +0200)]
x86: undo '{' being a symbol-start character
Having it that way has undue side effects, in permitting not only
pseudo-prefixes to be parsed correctly, but also permitting odd symbol
names which ought to be possible only when quoted. Borrow what other
architectures do: Put in place an "unrecognized line" hook to parse off
any pseudo prefixes, while using the "start of line" hook to reject ones
not actually followed by an insn. For that parsing re-use parse_insn()
in yet a slightly different mode (dealing with only pseudo-prefixes).
With that, pp may no longer be cleared from init_globals(), but instead
needs clearing after a line was fully processed. Since md_assemble() has
pretty many return paths, convert that into a local helper, with a
trivial wrapper around it.
Similarly pp may no longer be updated (by check_register()) when
processing anything other than insn operands. To be able to (easily)
recognize the case, clear current_templates.start when done with an insn
(or with .insn).
Jan Beulich [Fri, 19 Jul 2024 09:43:37 +0000 (11:43 +0200)]
x86: split pseudo-prefix state from i386_insn
Subsequently we will want to update that ahead of md_assemble(), with
that function needing to take into account such earlier updating.
Therefore it'll want resetting separately from i.
it is not logical why the last one shouldn't be permitted. Bypassing
that check requires other adjustments, though, to actually properly
consume (and then squash) the prefix.
MIPS/GAS/testsuite: Split mul test into 32-bit and 64-bit parts
Enable full 32-bit and 64-bit multiplication macro verification, by
splitting the 'mul' test into two parts respectively, and run them
across all the compatible architectures.
MIPS/GAS/testsuite: Run the mul macro test across architectures
The multiplication macros expand differently based on the ISA chosen, so
run the 'mul' macro test across compatible architectures, adopting the
'mul-ilocks' test orphaned by commit 23fce1e31156 ("MIPS16 intermix test
failure"), <https://sourceware.org/ml/binutils/2009-01/msg00335.html>,
and providing coverage for the expansion variants.
Only run from MIPS III up for now and remove the ISA override from the
source, so that the 64-bit instructions are covered for individual
64-bit architectures.
MIPS/GAS/testsuite: Split div test into 32-bit and 64-bit parts
Enable full 32-bit and 64-bit division macro verification, by splitting
the 'div' test into two parts respectively, and run them across all the
compatible architectures.
MIPS/GAS/testsuite: Run the div macro test across architectures
The division macros expand differently depending on the ISA selected, so
run the 'div' macro test across compatible architectures, adopting the
'div-ilocks' test orphaned by commit 23fce1e31156 ("MIPS16 intermix test
failure"), <https://sourceware.org/ml/binutils/2009-01/msg00335.html>,
and providing coverage for the expansion variants.
Only run from MIPS III up for now and remove the ISA override from the
source, so that the 64-bit instructions are covered for individual
64-bit architectures.
We have an ISA check for the '--trap' command-line option that reports
its incompatibility with the MIPS I architecture. It doesn't prevent
trap instructions from being enabled though, so when attempt is made to
emit one in an expansion of one of the division or multiplication macros
an assertion failure triggers:
.../gas/testsuite/gas/mips/brtr-opt.s: Assembler messages:
.../gas/testsuite/gas/mips/brtr-opt.s:3: Error: trap exception not supported at ISA 1
.../gas/testsuite/gas/mips/brtr-opt.s:9: Warning: divide by zero
.../gas/testsuite/gas/mips/brtr-opt.s:9: Internal error in macro_build at .../gas/config/tc-mips.c:9064.
Please report this bug.
The same assertion failure triggers without an earlier error message
when the initial ISA is compatible with the '--trap', however at the
time an attempt is made to emit a trap instruction from a division or
multiplication macro the ISA has been changed by a '.set' pseudo-op to
an incompatible one.
With the way the situations are mishandled it seems unlikely that anyone
relies on the current semantics and a sane approach is to decide on the
fly according to the currently selected ISA as to whether to emit trap
or breakpoint instructions in the case where '--trap' has been used.
Change our code to do so then and clarify that in the manual, which is
not explicit about how '--trap' is handled with a changing ISA. Mention
the change in NEWS too since it's a applies to a user option.
Add a fully interlocked MIPS IV CPU so that we can have coverage for
MIPS IV instruction sequences with and without instruction separation
required for a HI/LO data anti-dependency.
Similar to the x86_64 testcases, some .s files contain the corresponding
CFI directives. This helps in validating the synthesized CFI by running
those tests with and without the --scfi=experimental command line
option.
GAS issues some diagnostics, enabled by default, with
--scfi=experimental. The diagnostics have been added with an intent to
help user correct inadvertent errors in their hand-written asm. An
error is issued when GAS finds that input asm is not amenable to
accurate CFI synthesis. The existing scfi-diag-*.s tests in the
gas/testsuite/gas/scfi/x86_64 directory test some SCFI diagnostics
already:
- (#1) "Warning: SCFI: Asymetrical register restore"
- (#2) "Error: SCFI: usage of REG_FP as scratch not supported"
- (#3) "Error: SCFI: unsupported stack manipulation pattern"
- (#4) "Error: untraceable control flow for func 'XXX'"
In the newly added aarch64 testsuite, further tests for additional
diagnostics have been added:
- scfi-diag-1.s in this patch highlights an aarch64-specific diagnostic:
(#5) "Warning: SCFI: ignored probable save/restore op with reg offset"
Additionally, some testcases are added to showcase the (currently)
unsupported patterns, e.g., scfi-unsupported-1.s
mov x16, 4384
sub sp, sp, x16
gas/testsuite/:
* gas/scfi/README: Update comment to include aarch64.
* gas/scfi/aarch64/scfi-aarch64.exp: New file.
* gas/scfi/aarch64/ginsn-arith-1.l: New test.
* gas/scfi/aarch64/ginsn-arith-1.s: New test.
* gas/scfi/aarch64/ginsn-cofi-1.l: New test.
* gas/scfi/aarch64/ginsn-cofi-1.s: New test.
* gas/scfi/aarch64/ginsn-ldst-1.l: New test.
* gas/scfi/aarch64/ginsn-ldst-1.s: New test.
* gas/scfi/aarch64/scfi-callee-saved-fp-1.d: New test.
* gas/scfi/aarch64/scfi-callee-saved-fp-1.l: New test.
* gas/scfi/aarch64/scfi-callee-saved-fp-1.s: New test.
* gas/scfi/aarch64/scfi-callee-saved-fp-2.d: New test.
* gas/scfi/aarch64/scfi-callee-saved-fp-2.l: New test.
* gas/scfi/aarch64/scfi-callee-saved-fp-2.s: New test.
* gas/scfi/aarch64/scfi-cb-1.d: New test.
* gas/scfi/aarch64/scfi-cb-1.l: New test.
* gas/scfi/aarch64/scfi-cb-1.s: New test.
* gas/scfi/aarch64/scfi-cfg-1.d: New test.
* gas/scfi/aarch64/scfi-cfg-1.l: New test.
* gas/scfi/aarch64/scfi-cfg-1.s: New test.
* gas/scfi/aarch64/scfi-cfg-2.d: New test.
* gas/scfi/aarch64/scfi-cfg-2.l: New test.
* gas/scfi/aarch64/scfi-cfg-2.s: New test.
* gas/scfi/aarch64/scfi-cfg-3.d: New test.
* gas/scfi/aarch64/scfi-cfg-3.l: New test.
* gas/scfi/aarch64/scfi-cfg-3.s: New test.
* gas/scfi/aarch64/scfi-cfg-4.l: New test.
* gas/scfi/aarch64/scfi-cfg-4.s: New test.
* gas/scfi/aarch64/scfi-cond-br-1.d: New test.
* gas/scfi/aarch64/scfi-cond-br-1.l: New test.
* gas/scfi/aarch64/scfi-cond-br-1.s: New test.
* gas/scfi/aarch64/scfi-diag-1.l: New test.
* gas/scfi/aarch64/scfi-diag-1.s: New test.
* gas/scfi/aarch64/scfi-diag-2.l: New test.
* gas/scfi/aarch64/scfi-diag-2.s: New test.
* gas/scfi/aarch64/scfi-diag-3.l: New test.
* gas/scfi/aarch64/scfi-diag-3.s: New test.
* gas/scfi/aarch64/scfi-ldrp-1.d: New test.
* gas/scfi/aarch64/scfi-ldrp-1.l: New test.
* gas/scfi/aarch64/scfi-ldrp-1.s: New test.
* gas/scfi/aarch64/scfi-ldrp-2.d: New test.
* gas/scfi/aarch64/scfi-ldrp-2.l: New test.
* gas/scfi/aarch64/scfi-ldrp-2.s: New test.
* gas/scfi/aarch64/scfi-ldstnap-1.d: New test.
* gas/scfi/aarch64/scfi-ldstnap-1.l: New test.
* gas/scfi/aarch64/scfi-ldstnap-1.s: New test.
* gas/scfi/aarch64/scfi-strp-1.d: New test.
* gas/scfi/aarch64/scfi-strp-1.l: New test.
* gas/scfi/aarch64/scfi-strp-1.s: New test.
* gas/scfi/aarch64/scfi-strp-2.d: New test.
* gas/scfi/aarch64/scfi-strp-2.l: New test.
* gas/scfi/aarch64/scfi-strp-2.s: New test.
* gas/scfi/aarch64/scfi-unsupported-1.l: New test.
* gas/scfi/aarch64/scfi-unsupported-1.s: New test.
* gas/scfi/aarch64/scfi-unsupported-2.l: New test.
* gas/scfi/aarch64/scfi-unsupported-2.s: New test.
For synthesizing CFI (SCFI) for hand-written asm, the SCFI machinery in
GAS works on the generic GAS insns (ginsns). This patch adds support in
the aarch64 backend to create ginsns for a subset of the supported
machine instructions. The subset includes the minimal necessary
instructions to ensure SCFI correctness:
- Any potential register saves and unsaves. Hence, process instructions
belonging to a variety of iclasses involving str, ldr, stp, ldp.
- Any change of flow instructions. This includes all conditional and
unconditional branches, call (bl, blr, etc.) and return.
- Most importantly, any instruction that could affect the two registers
of interest: REG_SP, REG_FP. This set includes all pre-indexed and
post-indexed memory operations, with writeback, on the stack. This
set must also include other instructions (e.g., arithmetic insns)
where the destination register is one of the afore-mentioned registers.
With respect to callee-saved registers in Aarch64, FP/Advanced SIMD
registers D8-D15 are included along with the relevant GPRs. Calculating
offsets for loads and stores especially for Q registers needs special
attention here.
As an example,
str q8, [sp, #16]
On big-endian:
STR Qn stores as a 128-bit integer (MSB first), hence, should record
D8 as being saved at sp+24 rather than sp+16.
On little-endian:
should record D8 as being saved at sp+16
D8-D15 are the low 64 bits of Q8-Q15, and of Z8-Z15 if SVE is used;
hence, they remain "interesting" for SCFI purposes in such cases. A CFI
save slot always represents the low 64 bits, regardless of whether a
save occurs on D, Q or Z registers. Currently, the ginsn creation
machinery can handle D and Q registers on little-endian and big-endian.
Apart from creating ginsn, another key responsibility of the backend is
to make sure there are safeguards in place to detect and alert if an
instruction of interest may have been skipped. This is done via
aarch64_ginsn_unhandled () (similar to the x86 backend). This function
, hence, is also intended to alert when future ISA changes may otherwise
render SCFI results incorrect, because of missing ginsns for the newly
added machine instructions.
At this time, becuase of the complexities wrt endianness in handling Z
register usage, skip sve_misc opclass altogether for now. The SCFI
machinery will error out (using the aarch64_ginsn_unhandled () code
path) though if Z register usage affects correctness.
The current SCFI machinery does not currently synthesize the
PAC-related, aarch64-specific CFI directives: .cfi_b_key_frame. The
support for this is planned for near future.
SCFI is enabled for ELF targets only.
gas/
* config/tc-aarch64-ginsn.c: New file.
* config/tc-aarch64.c (md_assemble): Include tc-aarch64-ginsn.c
file. Invoke aarch64_ginsn_new.
* config/tc-aarch64.h (TARGET_USE_GINSN): Define for SCFI
enablement.
(TARGET_USE_SCFI): Likewise.
(SCFI_MAX_REG_ID): New definition.
(REG_FP): Likewise.
(REG_LR): Likewise.
(REG_SP): Likewise.
(SCFI_INIT_CFA_OFFSET): Likewise.
(SCFI_CALLEE_SAVED_REG_P): Likewise.
(aarch64_scfi_callee_saved_p): New declaration.
opcodes: aarch64: enforce checks on subclass flags in aarch64-gen.c
Enforce some checks on the newly added subclass flags:
- If a subclass is set of one insn of an iclass, every insn of that
iclass must have non-zero subclass field.
- For all other iclasses, the subclass bits are zero for all insns.
include/
* opcode/aarch64.h (enum aarch64_insn_class): Identify the
maximum iclass enum value.
opcodes/
* aarch64-gen.c (iclass_has_subclasses_p): New array of bool.
(read_table): Enforce checks on subclass flags.
opcodes: aarch64: add flags to denote subclasses of arithmetic insns
Use the three new subclass flags: F_ARITH_ADD, F_ARITH_SUB,
F_ARITH_MOV, to indicate add, sub and mov ops respectively.
These flags for subclasses will later be used for SCFI purposes to
create appropriate ginsns. At this time, only those iclasses relevant
to SCFI have the new subclass flags specified.
For addg and subg insns, F_SUBCLASS_OTHER is more suitable because these
operations do more than just simple add or sub.
opcodes/
* aarch64-tbl.h: Use the new F_ARITH_* flags.
opcodes: aarch64: add flags to denote subclasses of ldst insns
The existing iclass information tells us the general shape and purpose
of the instructions. In some cases, however, we need to further disect
the iclass on the basis of other finer-grain information. E.g., for the
purpose of SCFI, we need to know whether a given insn with iclass
of ldst_* is a load or a store.
At the moment, specify subclasses for only those iclasses relevant to
SCFI: ldst_imm9, ldst_pos, ldstpair_indexed, ldstpair_off and
ldstnapair_offs.
Some insns are best tagged with F_SUBCLASS_OTHER rather than F_LDST_LOAD
or F_LDST_STORE:
- stg* ops (as they store tag only),
- prfm,
- ldpsw, ldrsw (32-bit loads with signed extended value. Not useful
for restore operations in context of SCFI.)
- Use F_SUBCLASS_OTHER for all QL_LDST_R8 and QL_LDST_R16 operands.
Also use F_SUBLASS_OTHER for strb/ldrb, strh/ldrh opcodes.
These are not full loads and stores and cannot be allowed for
register save / restore for the purpose of SCFI.
opcodes/
* aarch64-tbl.h: Use the new F_LDST_* flags.
The existing iclass information tells us the general shape and purpose
of the instructions. In some cases, however, we need to further disect
the iclass on the basis of other finer-grain information. E.g., for the
purpose of SCFI, we need to know whether a given insn with iclass of
ldst_* is a load or a store. Similarly, whether a particular arithmetic
insn is an add or sub or mov, etc.
This patch defines new flags to demarcate the insns. Also provide an
access function for subclass lookup.
Later, we will enforce (in aarch64-gen.c) that if an iclass has at least
one instruction with a non-zero subclass, all instructions of the iclass
must have a non-zero subclass information. If none of the defined
subclasses are applicable (or not required for SCFI purposes),
F_SUBCLASS_OTHER can be used for such instructions.
gas: scfi: make scfi_state_restore_reg function more precise
When the SCFI machinery detects that a register has been restored from
stack, it makes some state changes in the SCFI state object.
Prior to the patch, scfi_state_restore_reg () was setting a value of
(reg, CFI_IN_REG) for (base, state) respectively. This was causing
issues in the cmp_scfi_state () function:
- The default state of all (callee-saved) regs at the beginning of
function is set to (0, CFI_UNDEFINED).
- If a register is saved and restored on some control path, the state
of reg is (reg, CFI_IN_REG) on that path.
- On another control path where the register was perhaps not
used (or saved/restored on stack) remains (0, CFI_UNDEFINED).
- The two states should be treated equal, however, at the point in
program after the register has been restored.
Fix this by resetting the state to (0, CFI_UNDEFINED) in
scfi_state_restore_reg ().
A testcase (scfi-cfg-4.s) for this is added in a subsequent commit.
gas/
* scfi.c (scfi_state_restore_reg): Reset to 0, CFI_UNDEFINED
for base, state.
Matthieu Longo [Thu, 16 May 2024 11:12:24 +0000 (12:12 +0100)]
gas: minor reformatting in command line help and doc
- help message: add a comma between the short and long option
- as doc:
- brief summary of how to invoke gas: separate [-w] [-x] on a new line as those
two options have nothing to do with the warning options.
- reordering of the warning options to have the same order as the listing.
- no-warn option description: change an "and" to a "or", as it is either the short
or long option to use, but not both at the same time.
- remove trailing whitespaces.
There appear to be two strategies for populating the .build-id
directory. Ubuntu takes the approach of placing the actual debug
information in this directory, so 4b4142d62b399499844924d53e33d4028380db.debug is an actual file
containing the debug information.
Fedora, RHEL, and SUSE take a slightly different approach, placing the
debug information elsewhere, and then creating symlinks in the
.build-id directory back to the original debug information file. The
actual debug information is arranged in a mirror of the filesystem
within the debug directory, as an example, if the debug-file-directory
is /usr/lib/debug, then the debug information for /bin/foo can be
found in /usr/lib/debug/bin/foo.debug.
Where this gets interesting is that in some cases a package will
install a single binary with multiple names, in this case a single
binary will be install with either hard-links, or symlinks providing
the alternative names.
The debug information for these multiple binaries will then be placed
into the /usr/lib/debug/ tree, and again, links are created so a
single file can provide debug information for each of the names that
binary presents as. An example file system might look like this (the
[link] could be symlinks, but are more likely hard-links):
/bin/
foo
bar -> foo [ HARD LINK ]
baz -> foo [ HARD LINK ]
/usr/
lib/
debug/
bin/
foo.debug
bar.debug -> foo.debug [ HARD LINK ]
baz.debug -> foo.debug [ HARD LINK ]
In the .build-id tree though we have a problem. Do we have a single
entry that links to one of the .debug files? This would work; a user
debugging any of the binaries will find the debug information based on
the build-id, and will get the correct information, after all the
.debug files are identical (same file linked together). But there is
one problem with this approach.
Sometimes, for *reasons* it's possible that one or more the linked
binaries might get removed, along with its associated debug
information. I'm honestly not 100% certain under what circumstances
this can happen, but what I observe is that sometime a single name for
a binary, and its corresponding .debug entry, can be missing. If this
happens to be the entry that the .build-id link is pointing at, then
we have a problem. The user can no longer find the debug information
based on the .build-id link.
The solution that Fedora, RHEL, & SUSE have adopted is to add multiple
entries in the .build-id tree, with each entry pointing to a different
name within the debug/ tree, a sequence number is added to the
build-id to distinguish the multiple entries. Thus, we might end up
with a layout like this:
With current master GDB, debug information will only ever be looked up
via the 4b4142d62b399499844924d53e33d4028380db.debug link. But if
'foo' and its corresponding 'foo.debug' are ever removed, then master
GDB will fail to find the debug information.
Ubuntu seems to have a much better approach for debug information
handling; they place the debug information directly into the .build-id
tree, so there only ever needs to be a single entry for any one
build-id. I wonder if/how they handle the case where multiple names
might share a single .debug file, if one of those names is then
uninstalled, how do they know the .debug file should be retained or
not ... but I assume that problem either doesn't exist or has been
solved.
Anyway, for a while Fedora has carried a patch that handles the
build-id sequence number logic. What's presented here is inspired by
the Fedora patch, but has some changes to fix some issues.
I'm aware that this is a patch that applies to only some (probably a
minority) of distros. However, the logic is contained to only a
single function in build-id.c, and isn't too complex, so I'm hoping
that there wont be too many objections.
For distros that don't have build-id sequence numbers there should be
no impact. The sequence number approach still leaves the first file
without a sequence number, and this is the first file that GDB (after
this patch) checks for. The new logic only kicks in if the
non-sequence numbered first file exists, but is a symlink to a non
existent file; in this case GDB checks for the sequence numbered files
instead.
Tests are included.
There is a small fix needed for gdb.base/sysroot-debug-lookup.exp,
after this commit GDB now treats a target: sysroot where the target
file system is local to GDB the same as if the sysroot had no target:
prefix. The consequence of this is that GDB now resolves a symlink
back to the real filename in the sysroot-debug-lookup.exp test where
it didn't previously. As this behaviour is inline with the case where
there is no target: prefix I think this is fine.
Andrew Burgess [Tue, 21 May 2024 14:58:41 +0000 (15:58 +0100)]
gdbserver: add gdbserver support for vFile::stat packet
After the previous two commits, this commit adds support for the
vFile::stat packet to gdbserver. This is pretty similar to the
handling for vFile::fstat, but instead calls 'lstat'.
There's still no users of target_fileio_stat in GDB, that will come in
a later commit.
Andrew Burgess [Tue, 21 May 2024 14:58:02 +0000 (15:58 +0100)]
gdb: add GDB side target_ops::fileio_stat implementation
This commit adds the GDB side of target_ops::fileio_stat. There's an
implementation for inf_child_target, which just calls 'lstat', and
there's an implementation for remote_target, which sends a new
vFile:stat packet.
The new packet is documented.
There's still no users of target_fileio_stat as I have not yet added
support for vFile::stat to gdbserver. If these packets are currently
sent to gdbserver then they will be reported as not supported and the
ENOSYS error code will be returned.
Andrew Burgess [Tue, 21 May 2024 14:39:37 +0000 (15:39 +0100)]
gdb: add target_fileio_stat, but no implementations yet
In a later commit I want target_fileio_stat, that is a call that
operates on a filename rather than an open file descriptor as
target_fileio_fstat does.
This commit adds the initial framework for target_fileio_stat, I've
added the top level target function and the virtual target_ops methods
in the target_ops base class.
At this point no actual targets override target_ops::fileio_stat, so
any attempts to call this function will return ENOSYS error code.
Tom de Vries [Wed, 17 Jul 2024 15:04:02 +0000 (17:04 +0200)]
[gdb/testsuite] Fix gdb.arch/arm-pseudo-unwind.exp with unix/mthumb
When running test-case gdb.arch/arm-pseudo-unwind.exp with target board
unix/mthumb, we run into:
...
(gdb) continue^M
Continuing.^M
^M
Program received signal SIGILL, Illegal instruction.^M
0x00400f38 in ?? ()^M
(gdb) FAIL: $exp: continue to breakpoint: continue to callee
...
The test-case attempts to force arm-pseudo-unwind.c to be compiled in arm mode
using additional_flags=-marm, but that's overridden by using target board
unix/mthumb.
This causes function main to be in thumb mode, and consequently function
caller (which is called from main) is is executed as if it's in thumb mode,
while it's actually in arm mode.
Fix this by adding an intermediate function caller_trampoline in
arm-pseudo-unwind.c, and hardcoding it to arm mode using
__attribute__((target("arm"))).
Likewise for test-case gdb.arch/arm-pseudo-unwind-legacy.exp.
Simon Marchi [Thu, 16 May 2024 20:13:20 +0000 (16:13 -0400)]
gdb: make `program_space::free_all_objfiles` use `this`
Use `this` instead of `current_program_space`. Presumably, the method
wants to check the solibs of "this" program space, not the current
global program space (although they are likely always the same at the
moment).