Some SME2 instructions operate on a range of consecutive ZA vectors.
This is indicated by syntax such as:
za[<Wv>, <imml>:<immh>]
Like with the earlier vgx2 and vgx4 support, we get better error
messages if the parser allows all ZA indices to have a range.
We can then reject invalid cases during constraint checking.
Many SME2 instructions operate on groups of 2 or 4 ZA vectors.
This is indicated by adding a "vgx2" or "vgx4" group size to the
ZA index. The group size is optional in assembly but preferred
for disassembly.
There is not a binary distinction between mnemonics that have
group sizes and mnemonics that don't, nor between mnemonics that
take vgx2 and mnemonics that take vgx4. We therefore get better
error messages if we allow any ZA index to have a group size
during parsing, and wait until constraint checking to reject
invalid sizes.
A quirk of the way errors are reported means that if an instruction
is wrong both in its qualifiers and its use of a group size, we'll
print suggested alternative instructions that also have an incorrect
group size. But that's a general property that also applies to
things like out-of-range immediates. It's also not obviously the
wrong thing to do. We need to be relatively confident that we're
looking at the right opcode before reporting detailed operand-specific
errors, so doing qualifier checking first seems resonable.
aarch64: Add _off4 suffix to AARCH64_OPND_SME_ZA_array
SME2 adds various new fields that are similar to
AARCH64_OPND_SME_ZA_array, but are distinguished by the size of
their offset fields. This patch adds _off4 to the name of the
field that we already have.
aarch64: Prefer register ranges & support wrapping
Until now, binutils has supported register ranges such
as { v0.4s - v3.4s } as an unofficial shorthand for
{ v0.4s, v1.4s, v2.4s, v3.4s }. The SME2 ISA embraces this form
and makes it the preferred disassembly. It also embraces wrapped
lists such as { z31.s - z2.s }, which is something that binutils
didn't previously allow.
The range form was already binutils's preferred disassembly for 3- and
4-register lists. This patch prefers it for 2-register lists too.
The patch also adds support for wrap-around.
SME2 has instructions that accept strided register lists,
such as { z0.s, z4.s, z8.s, z12.s }. The purpose of this
patch is to extend binutils to support such lists.
The parsing code already had (unused) support for strides of 2.
The idea here is instead to accept all strides during parsing
and reject invalid strides during constraint checking.
The SME2 instructions that accept strided operands also have
non-strided forms. The errors about invalid strides therefore
take a bitmask of acceptable strides, which allows multiple
possibilities to be summed up in a single message.
I've tried to update all code that handles register lists.
Some FLD_imm* suffixes used a counting scheme such as FLD_immN,
FLD_immN_2, FLD_immN_3, etc., while others used the lsb as the
suffix. The latter seems more mnemonic, and was a big help
in doing the SME2 work.
Similarly, the _10 suffix on FLD_SME_size_10 was nonobvious.
Presumably it indicated a 2-bit field, but it actually starts
in bit 22.
In GAS, the vector and predicate registers are identified by
REG_TYPE_VN, REG_TYPE_ZN and REG_TYPE_PN. This "N" is obviously
a placeholder for the register number. However, we don't use that
convention for integer and FP registers, and (more importantly)
SME2 adds "predicate-as-counter" registers that are denoted PN.
This patch therefore drops the "N" suffix from the existing
registers. The main hitch is that Z was also used for the
zero register in things like R_Z, but using ZR seems more
consistent with the SP-based names.
Quite a lot of SME2 instructions have an opcode bit that selects
between 32-bit and 64-bit forms of an instruction, with the 32-bit
forms being part of base SME2 and with the 64-bit forms being part
of an optional extension. It's nevertheless useful to have a single
opcode entry for both forms since (a) that matches the ISA definition
and (b) it tends to improve error reporting.
This patch therefore adds a libopcodes function called
aarch64_cpu_supports_inst_p that tests whether the target
supports a particular instruction. In future it will depend
on internal libopcodes routines.
This patch renames the OP_SME_* macros in aarch64-tbl.h so that
they follow the same scheme as the OP_SVE_* ones. It also uses
OP_SVE_ as the prefix, since there is no real distinction between
the SVE and SME uses of qualifiers: a macro defined for one can
be useful for the other too.
aarch64: Tweak priorities of parsing-related errors
There are three main kinds of error reported during parsing,
in increasing order of priority:
- AARCH64_OPDE_RECOVERABLE (register seen instead of immediate)
- AARCH64_OPDE_SYNTAX_ERROR
- AARCH64_OPDE_FATAL_SYNTAX_ERROR
This priority makes sense when comparing errors reported against the
same operand. But if we get to operand 3 (say) and see a register
instead of an immediate, that's likely to be a better match than
something that fails with a syntax error at operand 1.
The idea of this patch is to prioritise parsing-related errors
based on operand index first, then by error code. Post-parsing
errors still win over parsing errors, and their relative priorities
don't change.
aarch64: Try to report invalid variants against the closest match
If an instruction has invalid qualifiers, GAS would report the
error against the final opcode entry that got to the qualifier-
checking stage. It seems better to report the error against
the opcode entry that had the closest match, just like we
pick the closest match within an opcode entry for the
"did you mean this?" message.
This patch adds the number of invalid operands as an
argument to AARCH64_OPDE_INVALID_VARIANT and then picks the
AARCH64_OPDE_INVALID_VARIANT with the lowest argument.
aarch64: Make AARCH64_OPDE_REG_LIST take a bitfield
AARCH64_OPDE_REG_LIST took a single operand that specified the
expected number of registers. However, there are quite a few
SME2 instructions that have both 2-register forms and (separate)
4-register forms. If the user tries to use a 3-register list,
it isn't obvious which opcode entry they meant. Saying that we
expect 2 registers and saying that we expect 4 registers would
both be wrong.
This patch therefore switches the operand to a bitfield. If a
AARCH64_OPDE_REG_LIST is reported against multiple opcode entries,
the patch ORs up the expected lengths.
This has no user-visible effect yet. A later patch adds more error
strings, alongside tests that use them.
aarch64: Add an operand class for SVE register lists
SVE register lists were classified as SVE_REG, since there had been
no particular reason to separate them out. However, some SME2
instructions have tied register list operands, and so we need to
distinguish registers and register lists when checking whether two
operands match.
Also, the register list operands used a general error message,
even though we already have a dedicated error code for register
lists that are the wrong length.
aarch64: Add an error code for out-of-range registers
libopcodes currently reports out-of-range registers as a general
AARCH64_OPDE_OTHER_ERROR. However, this means that each register
range needs its own hard-coded string, which is a bit cumbersome
if the range is determined programmatically. This patch therefore
adds a dedicated error type for out-of-range errors.
SME2 has many instructions that take a list of SVE registers.
There are often multiple forms, with different forms taking
different numbers of registers.
This means that if, after a successful parse and qualifier match,
we find that the number of registers does not match the opcode entry,
the associated error should have a lower priority/severity than other
errors reported at the same stage. For example, if there are 2-register
and 4-register forms of an instruction, and if the assembly code uses
the 2-register form with an out-of-range value, the out-of-range value
error against the 2-register instruction should have a higher priority
than the "wrong number of registers" error against the 4-register
instruction.
This is tested by the main SME2 patches, but seemed worth splitting out.
aarch64: Rework reporting of failed register checks
There are many opcode table entries that share the same mnemonic.
Trying to parse an invalid assembly line will trigger an error for
each of these entries, but the specific error might vary from one
entry to another, depending on the exact nature of the problem.
GAS has quite an elaborate system for picking the most appropriate
error out of all the failed matches. And in many cases it works well.
However, one of the limitations is that the error is always reported
against a single opcode table entry. If that table entry isn't the
one that the user intended to use, then the error can end up being
overly specific.
This is particularly true if an instruction has a typoed register
name, or uses a type of register that is not accepted by any
opcode table entry. For example, one of the expected error
matches for an attempted SVE2 instruction is:
Error: operand 1 must be a SIMD scalar register -- `addp z32\.s,p0/m,z32\.s,z0\.s'
even though the hypothetical user was presumably attempting to use
the SVE form of ADDP rather than the Advanced SIMD one. There are
many other instances of this in the testsuite.
The problem becomes especially acute with SME2, since many SME2
instructions reuse existing mnemonics. This could lead to us
reporting an SME-related error against a non-SME instruction,
or a non-SME-related error against an SME instruction.
This patch tries to improve things by collecting together all
the register types that an opcode table entry expected for a
given operand. It also records what kind of register was
actually seen, if any. It then tries to summarise all this
in a more directed way, falling back to a generic error if
the combination defies a neat summary.
The patch includes tests for all new messages except REG_TYPE_ZA,
which only triggers with SME2.
To test this, I created an assembly file that contained the cross
product of all known mnemonics and one example from each register
class. I then looked for cases where the new routines fell back on the
generic errors ("expected a register" or "unexpected register type").
I locally added dummy messages for each one until there were no
more hits. The patch adds a specimen instruction to diagnostics.s
for each of these combinations. In each case, the combination didn't
seem like something that could be summarised in a natural way, so the
generic messages seemed better. There's always going to be an element
of personal taste around this kind of thing though.
Adding more register types made 1<<REG_TYPE_MAX exceed the range
of the type, but we don't actually need/want 1<<REG_TYPE_MAX.
aarch64: Try to avoid inappropriate default errors
After parsing a '{' and the first register, parse_typed_reg would
report errors in subsequent registers in the same way as for the
first register. It used set_default_error, which reports errors
of the form "operand N must be X".
The problem is that if there are multiple opcode entries for the
same mnemonic, there could be several matches that lead to a
default error. There's no guarantee that the default error for
the register list is the one that will be chosen.
To take an example from the testsuite:
ext z0.b,{z31.b,z32.b},#0
gave:
operand 2 must be an SVE vector register
with the error being reported against the single-vector version
of ext, even though the operand is clearly a list.
This patch uses set_fatal_syntax_error to bump the priority of the
error once we're sure that the operand is a list of the right type.
aarch64: Improve errors for malformed register lists
parse_typed_reg is used for parsing both bare registers and
registers that occur in lists. If it doesn't see a register,
or sees an unexpected kind of register, it queues a default
error to report the problem. These default errors have the form
"operand N must be an X", where X comes from the operand table.
If there are multiple opcode entries that report default errors,
GAS tries to pick the most appropriate one, using the opcode
table order as a tiebreaker. But this can lead to cases where
a syntax error in a register list is reported against an opcode
that doesn't accept register lists. For example, the unlikely
error:
ext z0.b,{,},#0
is reported as:
operand 2 must be an SVE vector register -- `ext z0.b,{,},#0'
even though operand 2 can be a register list.
If we've parsed the opening '{' of a register list, and then see
something that isn't remotely register-like, it seems better to
report that directly as a syntax error, rather than rely on the
default error. The operand won't be a valid list of anything,
so there's no need to pick a specific Y in "operand N must be
a list of Y".
Integer registers were parsed indirectly through
aarch64_reg_parse_32_64 (and thus aarch64_addr_reg_parse) rather
than directly through parse_reg. This was because we need the
qualifier associated with the register, and the logic to calculate
that was buried in aarch64_addr_reg_parse.
The code that parses FP registers had the same need, but it
open-coded the calculation of the qualifier.
This patch tries to handle both cases in the same way. It is
needed by a later patch that tries to improve the register-related
diagnostics.
parse_address_main currently uses get_reg_expected_msg to
report invalid base and offset registers, but the disadvantage
of doing that is that it isn't immediately clear which register
is wrong (the base or the offset).
A later patch moves away from using get_reg_expected_msg for failed
type checks, but doing that here didn't seem like the best approach.
The patch tries to use more tailored messages instead.
In SME, the vector select register had to be in the range
w12-w15, so it made sense to enforce that during parsing.
However, SME2 adds instructions for which the range is
w8-w11 instead.
This patch therefore moves the range check from the parsing
stage to the constraint-checking stage.
Also, the previous error used a capitalised range W12-W15,
whereas other register range errors used lowercase ranges
like p0-p7. A quick internal poll showed a preference for
the lowercase form, so the patch uses that.
The patch uses "selection register" rather than "vector
select register" so that the terminology extends more
naturally to PSEL.
Now that parse_typed_reg checks the range of tile register numbers
and libopcodes checks the range of vector select offsets, there's
very little difference between the parsing of ZA tile indices,
ZA array indices, and PSEL indices. The main one is that ZA
array indices don't currently allow "za" to be qualified,
but we need to remove that restriction for SME2.
This patch therefore consolidates all three parsers into a single
routine, parameterised by the type of register that they expect.
This patch moves the range checks on ZA vector select offsets from
gas to libopcodes. Doing the checks there means that the error
messages contain the expected range. It also fits in better
with the error severity scheme, which becomes important later.
(This is because out-of-range indices are treated as more severe than
syntax errors, on the basis that parsing must have succeeded if we get
to the point of checking the completed opcode.)
The patch also adds a new check_za_access function for checking
ZA accesses. That's a bit over the top for one offset check, but the
function becomes more complex with later patches.
sme-9-illegal.s checked for an invalid .q suffix using:
psel p1, p15, p3.q[w15]
but this is doubly invalid because it misses the immediate part
of the index. The patch keeps that test but adds another with
a zero index, so that .q is the only thing wrong.
The aarch64-tbl.h change includes neatening up the backslash
positions.
ZA indices have more parts than most operands, so passing these
parts around individually is more awkward than for other operand
types. Things aren't too bad at the moment, but SME2 adds two
further pieces: an offset range and a vector group size.
This patch therefore replaces arguments for the individual pieces
with a single argument for the index as a whole.
A later patch moves the range checking for ZA vector select
offsets from gas to libopcodes. That in turn requires the
immediate field to be big enough to support all parsed values.
This shouldn't be a particularly size-sensitive structure,
so there should be no memory problems with doing this.
za_tile_vector is also used for indexing ZA as a whole, rather than
just for indexing tiles. The former is more common than the latter
in SME2, so this patch generalises the name to "indexed_za".
The patch also names the associated structure, so that later patches
can reuse it during parsing.
We already treat the ZA tiles ZA0-ZA15 as registers. This patch
does the same for ZA itself. parse_sme_zero_mask can then parse
ZA tiles and ZA in the same way, through parsed_type_reg.
One important effect of going through parsed_type_reg (in general)
is that it allows ZA to take qualifiers. This is necessary for many
SME2 instructions.
However, to support existing unqualified uses of ZA, parse_reg_with_qual
needs to treat the qualiier as optional. Hopefully the net effect is
to give better error messages, since now that SME2 makes "za.<T>"
valid in some contexts, it might be natural to use it (incorrectly)
in ZERO too.
While there, the patch also tweaks the error messages for invalid
ZA tiles, to try to make some cases more specific.
For now, parse_sme_za_array just uses parse_reg, rather than
parse_typed_reg/parse_reg_with_qual. A later patch consolidates
the parsing further.
Now that all parsing of ZA tile names goes through parse_typed_reg,
we can check there for out-of-range tile numbers. The other check
performed by parse_sme_zada_operand was to reject .q, but that can
now be done via F_STRICT instead. (.q tiles are valid in other
contexts, so they shouldn't be rejected in parse_typed_reg.)
This patch reuses the general parse_typed_reg for ZA tiles.
This involves adding a way of suppressing the usual treatment
of register indices, since ZA indices look very different from
Advanced SIMD and SVE vector indices.
parse_typed_reg returned a register number and passed the
register type back using a pointer parameter. It seems simpler
to return the register entry instead, since that has both pieces
of information in one place.
The patch also replaces the boolean in_reg_list parameter with
a mask of flags. This hopefully makes calls easier to read
(more self-documenting than "true" or "false"), but more
importantly, it allows a later patch to add a second flag.
This patch just moves vectype_to_qualifier further up, so that
a later patch can call it at an earlier point in the file.
No behavioural change intended.
This patch adds a multi-register type that includes both REG_TYPE_ZATH
and REG_TYPE_ZATV. This slightly simplifies the existing code, but the
main purpose is to enable later patches.
The ZA tile registers were called REG_TYPE_ZA, REG_TYPE_ZAH and
REG_TYPE_ZAV. However, a later patch wants to make plain "za"
a register type too, and REG_TYPE_ZA is the obvious name for that.
This patch therefore adds "T" (tile) to the existing names.
GAS's aarch64_instruction had its own cut-down error record,
but it's better for later patches if it reuses the binutils-wide
aarch64_operand_error instead. The main difference is that
aarch64_operand_error can store arguments to the error while
aarch64_instruction couldn't.
This patch makes all SME instructions use F_STRICT, so that qualifiers
have to be provided explicitly rather than being inferred from other
operands. The main change is to move the qualifier setting from the
operand-level decoders to the opcode level.
This is one step towards consolidating the ZA parsing code and
extending it to handle SME2.
GAS refuses to interpret register names like x0 as unadorned
immediates, due to the obvious potential for confusion with
register operands. (An explicit #x0 is OK.)
For compatibility reasons, we can't extend the set of registers
that GAS rejects for existing instructions. For example:
mov x0, z0
was valid code before SVE was added, so it needs to stay valid
code even when SVE is enabled. But we can make GAS reject newer
registers in newer instructions. The SVE instruction:
and z0.s, z0.s, z0.h
is therefore invalid, rather than z0.h being an immediate.
This patch extends the SVE behaviour to SVE2. The old call
to AARCH64_CPU_HAS_FEATURE was technically the wrong way around,
although it didn't matter in practice for base SVE instructions
since their avariants only set SVE.
In the register-index forms of PRFM, the unallocated prefetch opcodes
24-31 have been reused for the encoding of the new RPRFM instruction.
The PRFM opcode space is now capped at 23 for these forms. The other
forms of PRFM are unaffected.
Most extension flags are named after the associated architectural
FEAT_* flags, but sme-i64 and sme-f64 were exceptions. This patch
adds sme-i16i64 and sme-f64f64 aliases, but keeps the old names too
for compatibility.
Andrew Burgess [Fri, 10 Mar 2023 12:29:58 +0000 (12:29 +0000)]
gdb/python: Add new gdb.unwinder.FrameId class
When writing an unwinder it is necessary to create a new class to act
as a frame-id. This new class is almost certainly just going to set a
'sp' and 'pc' attribute within the instance.
This commit adds a little helper class gdb.unwinder.FrameId that does
this job. Users can make use of this to avoid having to write out
standard boilerplate code any time they write an unwinder.
Of course, if the user wants their FrameId class to be more
complicated in some way, then they can still write their own class,
just like they could before.
I've simplified the example code in the documentation to now use the
new helper class, and I've also made use of this helper within the
testsuite.
Any existing user code will continue to work just as it did before
after this change.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Reviewed-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Fri, 10 Mar 2023 11:29:39 +0000 (11:29 +0000)]
gdb/python: Allow gdb.UnwindInfo to be created with non gdb.Value args
Currently when creating a gdb.UnwindInfo object a user must call
gdb.PendingFrame.create_unwind_info and pass a frame-id object.
The frame-id object should have at least a 'sp' attribute, and
probably a 'pc' attribute too (it can also, in some cases have a
'special' attribute).
Currently all of these frame-id attributes need to be gdb.Value
objects, but the only reason for that requirement is that we have some
code in py-unwind.c that only handles gdb.Value objects.
If instead we switch to using get_addr_from_python in py-utils.c then
we will support both gdb.Value objects and also raw numbers, which
might make things simpler in some cases.
So, I started rewriting pyuw_object_attribute_to_pointer (in
py-unwind.c) to use get_addr_from_python. However, while looking at
the code I noticed a problem.
The pyuw_object_attribute_to_pointer function returns a boolean flag,
if everything goes OK we return true, but we return false in two
cases, (1) when the attribute is not present, which might be
acceptable, or might be an error, and (2) when we get an error trying
to extract the attribute value, in which case a Python error will have
been set.
Now in pending_framepy_create_unwind_info we have this code:
if (!pyuw_object_attribute_to_pointer (pyo_frame_id, "sp", &sp))
{
PyErr_SetString (PyExc_ValueError,
_("frame_id should have 'sp' attribute."));
return NULL;
}
Notice how we always set an error. This will override any error that
is already set.
So, if you create a frame-id object that has an 'sp' attribute, but
the attribute is not a gdb.Value, then currently we fail to extract
the attribute value (it's not a gdb.Value) and set this error in
pyuw_object_attribute_to_pointer:
rc = pyuw_value_obj_to_pointer (pyo_value.get (), addr);
if (!rc)
PyErr_Format (
PyExc_ValueError,
_("The value of the '%s' attribute is not a pointer."),
attr_name);
Then we return to pending_framepy_create_unwind_info and immediately
override this error with the error about 'sp' being missing.
This all feels very confused.
Here's my proposed solution: pyuw_object_attribute_to_pointer will now
return a tri-state enum, with states OK, MISSING, or ERROR. The
meanings of these states are:
OK - Attribute exists and was extracted fine,
MISSING - Attribute doesn't exist, no Python error was set.
ERROR - Attribute does exist, but there was an error while
extracting it, a Python error was set.
We need to update pending_framepy_create_unwind_info, the only user of
pyuw_object_attribute_to_pointer, but now I think things are much
clearer. Errors from lower levels are not blindly overridden with the
generic meaningless error message, but we still get the "missing 'sp'
attribute" error when appropriate.
This change also includes the switch to get_addr_from_python which was
what started this whole journey.
For well behaving user code there should be no visible changes after
this commit.
For user code that hits an error, hopefully the new errors should be
more helpful in figuring out what's gone wrong.
Additionally, users can now use integers for the 'sp' and 'pc'
attributes in their frame-id objects if that is useful.
Andrew Burgess [Fri, 10 Mar 2023 10:49:05 +0000 (10:49 +0000)]
gdb: have value_as_address call unpack_pointer
While refactoring some other code in gdb/python/* I wanted to merge
two code paths. One path calls value_as_address, while the other
calls unpack_pointer.
I suspect calling value_as_address is the correct choice, but, while
examining the code I noticed that value_as_address calls unpack_long
rather than unpack_pointer.
Under the hood, unpack_pointer does just call unpack_long so there's
no real difference here, but it feels like value_as_address should
call unpack_pointer.
I've updated the code to use unpack_pointer, and changed a related
comment to say that we call unpack_pointer. I've also adjusted the
header comment on value_as_address. The existing header refers to
some code that is now commented out.
Rather than trying to describe the whole algorithm of
value_as_address, which is already well commented within the function,
I've just trimmed the comment on value_as_address to be a brief
summary of what the function does.
There should be no user visible changes after this commit.
Andrew Burgess [Fri, 10 Mar 2023 10:28:21 +0000 (10:28 +0000)]
gdb/python: remove Py_TPFLAGS_BASETYPE from gdb.UnwindInfo
It is not currently possible to directly create gdb.UnwindInfo
instances, they need to be created by calling
gdb.PendingFrame.create_unwind_info so that the newly created
UnwindInfo can be linked to the pending frame.
As such there's no tp_init method defined for UnwindInfo.
A consequence of all this is that it doesn't really make sense to
allow sub-classing of gdb.UnwindInfo. Any sub-class can't call the
parents __init__ method to correctly link up the PendingFrame
object (there is no parent __init__ method). And any instances that
sub-classes UnwindInfo but doesn't call the parent __init__ is going
to be invalid for use in GDB.
This commit removes the Py_TPFLAGS_BASETYPE flag from the UnwindInfo
class, which prevents the class being sub-classed. Then I've added a
test to check that this is indeed prevented.
Any functional user code will not have any issues with this change.
Andrew Burgess [Fri, 10 Mar 2023 10:19:58 +0000 (10:19 +0000)]
gdb/python: add __repr__ for PendingFrame and UnwindInfo
Having a useful __repr__ method can make debugging Python code that
little bit easier. This commit adds __repr__ for gdb.PendingFrame and
gdb.UnwindInfo classes, along with some tests.
Andrew Burgess [Wed, 8 Mar 2023 16:11:45 +0000 (16:11 +0000)]
gdb/python: add some additional methods to gdb.PendingFrame
The gdb.Frame class has far more methods than gdb.PendingFrame. Given
that a PendingFrame hasn't yet been claimed by an unwinder, there is a
limit to which methods we can add to it, but many of the methods that
the Frame class has, the PendingFrame class could also support.
In this commit I've added those methods to PendingFrame that I believe
are safe.
In terms of implementation: if I was starting from scratch then I
would implement many of these (or most of these) as attributes rather
than methods. However, given both Frame and PendingFrame are just
different representation of a frame, I think there is value in keeping
the interface for the two classes the same. For this reason
everything here is a method -- that's what the Frame class does.
The new methods I've added are:
- gdb.PendingFrame.is_valid: Return True if the pending frame
object is valid.
- gdb.PendingFrame.name: Return the name for the frame's function,
or None.
- gdb.PendingFrame.pc: Return the $pc register value for this
frame.
- gdb.PendingFrame.language: Return a string containing the
language for this frame, or None.
- gdb.PendingFrame.find_sal: Return a gdb.Symtab_and_line object
for the current location within the pending frame, or None.
- gdb.PendingFrame.block: Return a gdb.Block for the current
pending frame, or None.
- gdb.PendingFrame.function: Return a gdb.Symbol for the current
pending frame, or None.
In every case I've just copied the implementation over from gdb.Frame
and cleaned the code slightly e.g. NULL to nullptr. Additionally each
function required a small update to reflect the PendingFrame type, but
that's pretty minor.
There are tests for all the new methods.
For more extensive testing, I added the following code to the file
gdb/python/lib/command/unwinders.py:
from gdb.unwinder import Unwinder
class TestUnwinder(Unwinder):
def __init__(self):
super().__init__("XXX_TestUnwinder_XXX")
def __call__(self,pending_frame):
lang = pending_frame.language()
try:
block = pending_frame.block()
assert isinstance(block, gdb.Block)
except RuntimeError as rte:
assert str(rte) == "Cannot locate block for frame."
function = pending_frame.function()
arch = pending_frame.architecture()
assert arch is None or isinstance(arch, gdb.Architecture)
name = pending_frame.name()
assert name is None or isinstance(name, str)
valid = pending_frame.is_valid()
pc = pending_frame.pc()
sal = pending_frame.find_sal()
assert sal is None or isinstance(sal, gdb.Symtab_and_line)
return None
This registers a global unwinder that calls each of the new
PendingFrame methods and checks the result is of an acceptable type.
The unwinder never claims any frames though, so shouldn't change how
GDB actually behaves.
I then ran the testsuite. There was only a single regression, a test
that uses 'disable unwinder' and expects a single unwinder to be
disabled -- the extra unwinder is now disabled too, which changes the
test output. So I'm reasonably confident that the new methods are not
going to crash GDB.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Reviewed-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Thu, 9 Mar 2023 10:58:54 +0000 (10:58 +0000)]
gdb/python: add PENDING_FRAMEPY_REQUIRE_VALID macro in py-unwind.c
This commit copies the pattern that is present in many other py-*.c
files: having a single macro to check that the Python object is still
valid.
This cleans up the code a little throughout the py-unwind.c file.
Some of the exception messages will change slightly with this commit,
though the type of the exceptions is still ValueError in all cases.
I started writing some tests for this change and immediately ran into
a problem: GDB would crash. It turns out that the PendingFrame
objects are not being marked as invalid!
In pyuw_sniffer where the pending frames are created, we make use of a
scoped_restore to invalidate the pending frame objects. However, this
only restores the pending_frame_object::frame_info field to its
previous value -- and it turns out we never actually give this field
an initial value, it's left undefined.
So, when the scoped_restore (called invalidate_frame) performs its
cleanup, it actually restores the frame_info field to an undefined
value. If this undefined value is not nullptr then any future
accesses to the PendingFrame object result in undefined behaviour and
most likely, a crash.
As part of this commit I now initialize the frame_info field, which
ensures all the new tests now pass.
Andrew Burgess [Thu, 9 Mar 2023 09:11:35 +0000 (09:11 +0000)]
gdb/python: remove unneeded nullptr check in frapy_block
Spotted a redundant nullptr check in python/py-frame.c in the function
frapy_block. This was introduced in commit 57126e4a45e3000e when we
expanded an earlier check in return early if the pointer in question
is nullptr.
There should be no user visible changes after this commit.
Andrew Burgess [Wed, 8 Mar 2023 16:11:30 +0000 (16:11 +0000)]
gdb/python: make the gdb.unwinder.Unwinder class more robust
This commit makes a few related changes to the gdb.unwinder.Unwinder
class attributes:
1. The 'name' attribute is now a read-only attribute. This prevents
user code from changing the name after registering the unwinder. It
seems very unlikely that any user is actually trying to do this in
the wild, so I'm not very worried that this will upset anyone,
2. We now validate that the name is a string in the
Unwinder.__init__ method, and throw an error if this is not the
case. Hopefully nobody was doing this in the wild. This should
make it easier to ensure the 'info unwinder' command shows sane
output (how to display a non-string name for an unwinder?),
3. The 'enabled' attribute is now implemented with a getter and
setter. In the setter we ensure that the new value is a boolean,
but the real important change is that we call
'gdb.invalidate_cached_frames()'. This means that the backtrace
will be updated if a user manually disables an unwinder (rather than
calling the 'disable unwinder' command). It is not unreasonable to
think that a user might register multiple unwinders (relating to
some project) and have one command that disables/enables all the
related unwinders. This command might operate by poking the enabled
attribute of each unwinder object directly, after this commit, this
would now work correctly.
There's tests for all the changes, and lots of documentation updates
that both cover the new changes, but also further improve (I think)
the general documentation for GDB's Unwinder API.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Reviewed-By: Tom Tromey <tom@tromey.com>
Alan Modra [Thu, 30 Mar 2023 01:40:16 +0000 (12:10 +1030)]
Setting sh_link for SHT_REL/SHT_RELA
It's wrong to have an alloc reloc section trying to use a non-alloc
symbol table.
* elf.c (assign_section_numbers <SHT_REL, SHT_RELA>): Correct
comment. Always set sh_link to .dynsym for alloc reloc
sections and to .symtab for non-alloc.
Alan Modra [Wed, 29 Mar 2023 12:01:15 +0000 (22:31 +1030)]
Fix memory leak in bfd_get_debug_link_info_1
* opncls.c (bfd_get_alt_debug_link_info): Don't bother freeing
after bfd_malloc_and_get_section failure.
(get_build_id): Likewise.
(bfd_get_debug_link_info_1): Likewise. Free section contents
when crc not present.
* section.c (bfd_malloc_and_get_section): Document that the
buffer is NULL on error return.
Alan Modra [Wed, 29 Mar 2023 11:35:59 +0000 (22:05 +1030)]
Tidy leaked objcopy memory
* objcopy.c (delete_symbol_htabs): Also free symbols.
(write_debugging_info): Free strings and syms once written.
* wrstabs.c (write_stabs_in_sections_debugging_info): memset
entire info struct. Free hash tables before returning. Free
syms on error return.
Palmer Dabbelt [Sat, 25 Mar 2023 00:41:13 +0000 (08:41 +0800)]
RISC-V: PR28789, Reject R_RISCV_PCREL relocations with ABS symbol in PIC/PIE.
The non-preemptible SHN_ABS symbol with a pc-relative relocation should be
disallowed when generating shared object (pic and pie). Generally, the
following cases, which refer to pr25749, will cause a symbol be
non-preemptible,
* -pie, or -shared with -symbolic
* STV_HIDDEN, STV_INTERNAL, STV_PROTECTED
* Have dynamic symbol table, but without the symbol
* VER_NDX_LOCAL
However, PCREL_HI20/LO12 relocs are always bind locally when generating
shared object, so not only the non-preemptible absolute symbol need to
be disallowed, all absolute symbol references need but except that they
are defined in linker script. If we also disallow the absolute symbol
in linker script, then the glibc-linux toolchain build failed, so regard
them as pc-relative symbols, just like what x86 did.
Maybe we should add this check for all pc-relative relocations, rather
than just handle in R_RISCV_PCREL relocs. Ideally, since the value of
SHN_ABS symbol is a constant, only S - A relocations should be allowed
in the shared object, so only BFD_RELOC_8/16/32/64 are allowed, which
means R_RISCV_32/R_RISCV_64.
bfd/
PR 28789
* elfnn-riscv.c (riscv_elf_check_relocs): The absolute symbol cannot be
referneced with pc-relative relocation when generating shared object.
ld/
PR 28789
* ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
* ld/testsuite/ld-riscv-elf/pcrel-reloc*: New testcases.
Nelson Chu [Sat, 25 Mar 2023 00:41:12 +0000 (08:41 +0800)]
RISC-V: Clarify link behaviors of R_RISCV_32/64 relocations with ABS symbol.
There are two improvements, which are all referenced to aarch64,
* R_RISCV_32 with non ABS symbol cannot be used under RV64 when making
shard objects.
* Don't need dynamic relocation for R_RISCV_32/64 under RV32/RV64 when
making shared objects, if the referenced symbol is local ABS symbol.
However, considering this link,
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/341
Seems like we should makes all R_RISCV_32/64 relocs with ABS symbol
that don't need any dynamic relocations when making the shared objects.
But anyway, I just sync the current behavior as aarch64 ld, in case
there are any unexpected behaviors happen.
Passed the gcc/binutils regressions in riscv-gnu-toolchain.
bfd/
* elfnn-riscv.c (riscv_elf_check_relocs): Only allow R_RISCV_32 with ABS
symbol under RV64.
(riscv_elf_relocate_section): R_RISCV_32/64 with local ABS symbol under
RV32/RV64 doesn't need any dynamic relocation when making shared objects.
I just make the implementations similar to other targets, so that will be
more easy to mainatain.
ld/
* testsuite/ld-riscv-elf/data-reloc*: New testcases.
* testsuite/ld-riscv-elf/ld-riscv-elf.exp: Added new data-reloc* testcases,
and need to make ifunc-seperate* testcases work for rv32.
* testsuite/ld-riscv-elf/ifunc-seperate-caller-nonplt.s: Likewise.
* testsuite/ld-riscv-elf/ifunc-seperate-caller-plt.s: Likewise.
Nelson Chu [Sat, 25 Mar 2023 00:41:11 +0000 (08:41 +0800)]
RISC-V: Extract the ld code which are too complicated, and may be reused.
These types of codes are different for each target, I am not sure what are the
best for RISC-V, so extract them out may be more easy to compare what's the
difference.
bfd/
* elfnn-riscv.c (RISCV_NEED_DYNAMIC_RELOC): New defined. Extracted
from riscv_elf_check_relocs, to see if dynamic reloc is needed for the
specific relocation.
(RISCV_GENERATE_DYNAMIC_RELOC): New defined. Extracted from
riscv_elf_relocate_section, to see if R_RISCV_32/64 need to generate
dynamic relocation.
(RISCV_COPY_INPUT_RELOC): New defined. Extracted from
riscv_elf_relocate_section, to see if R_RISCV_32/64 need to copy itslef
tp output file.
(RISCV_RESOLVED_LOCALLY): New defined. Extracted from
riscv_elf_relocate_section, to see if R_RISCV_GOT_HI20 can be resolved
locally.
Tom Tromey [Wed, 1 Mar 2023 17:33:27 +0000 (10:33 -0700)]
Use the correct frame when evaluating a dynamic property
The test case in this patch shows an unusual situation: an Ada array
has a dynamic bound, but the bound comes from a frame that's referred
to by the static link. This frame is correctly found when evaluating
the array variable itself, but is lost when evaluating the array's
bounds.
This patch fixes the problem by passing this frame through to
value_at_lazy in the DWARF expression evaluator.
Tom Tromey [Wed, 1 Mar 2023 17:33:20 +0000 (10:33 -0700)]
Pass a frame to value_at_lazy and value_from_contents_and_address
This patch adds a 'frame' parameter to value_at_lazy and ensures that
it is passed down to the call to resolve_dynamic_type. This required
also adding a frame parameter to value_from_contents_and_address.
Nothing passes this parameter to value_at_lazy yet, so this patch
should have no visible effect.
Tom Tromey [Wed, 1 Mar 2023 16:54:04 +0000 (09:54 -0700)]
Add frame parameter to resolve_dynamic_type
This adds a frame parameter to resolve_dynamic_type and arranges for
it to be passed through the call tree and, in particular, to all calls
to dwarf2_evaluate_property.
Nothing passes this parameter yet, so this patch should have no
visible effect.
A 'const frame_info_ptr *' is used here to avoid including frame.h
from gdbtypes.h.
Tom Tromey [Mon, 27 Feb 2023 20:23:35 +0000 (13:23 -0700)]
Introduce rust_at_least helper proc
This adds a 'rust_at_least' helper proc, for checking the version of
the Rust compiler in use. It then changes various tests to use this
with 'require'.
Tom de Vries [Wed, 29 Mar 2023 14:09:44 +0000 (16:09 +0200)]
[gdb/testsuite] Require gnatmake 11 for gdb.ada/verylong.exp
With test-case gdb.ada/verylong.exp and gnatmake 7.5.0 I run into:
...
compilation failed: gcc ... $src/gdb/testsuite/gdb.ada/verylong/prog.adb
prog.adb:16:11: warning: file name does not match unit name, should be "main.adb"
prog.adb:17:08: "Long_Long_Long_Integer" is undefined (more references follow)
gnatmake: "prog.adb" compilation error
gdb, infcmd: remove redundant ERROR_NO_INFERIOR in continue_command
The ERROR_NO_INFERIOR macro is already called at the beginning of the
function continue_command. Since target/inferior are not switched in-between,
the second call to it is redundant.
Co-Authored-By: Christina Schimpe <christina.schimpe@intel.com>
Andrew Burgess [Wed, 22 Mar 2023 17:22:51 +0000 (17:22 +0000)]
gdb: move displaced_step_dump_bytes into gdbsupport (and rename)
It was pointed out during review of another patch that the function
displaced_step_dump_bytes really isn't specific to displaced stepping,
and should really get a more generic name and move into gdbsupport/.
This commit does just that. The function is renamed to
bytes_to_string and is moved into gdbsupport/common-utils.{cc,h}. The
function implementation doesn't really change. Much...
... I have updated the function to take an array view, which makes it
slightly easier to call in a couple of places where we already have a
gdb::bytes_vector. I've then added an inline wrapper to convert a raw
pointer and length into an array view, which is used in places where
we don't easily have a gdb::bytes_vector (or similar).
Updated all users of displaced_step_dump_bytes.
There should be no user visible changes after this commit.
Finally, I ended up having to add an include of gdb_assert.h into
array-view.h. When I include array-view.h into common-utils.h I ran
into build problems because array-view.h calls gdb_assert.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Andrew Burgess [Tue, 21 Feb 2023 12:38:03 +0000 (12:38 +0000)]
gdb: more debug output for displaced stepping
While investigating a displaced stepping issue I wanted an easy way to
see what GDB thought the original instruction was, and what
instruction GDB replaced that with when performing the displaced step.
We do print out the address that is being stepped, so I can track down
the original instruction, I just need to go find the information
myself.
And we do print out the bytes of the new instruction, so I can figure
out what the replacement instruction was, but it's not really easy.
Also, the code that prints the bytes of the replacement instruction
only prints 4 bytes, which clearly isn't always going to be correct.
In this commit I remove the existing code that prints the bytes of the
replacement instruction, and add two new blocks of code to
displaced_step_prepare_throw. This new code prints the original
instruction, and the replacement instruction. In each case we print
both the bytes that make up the instruction and the completely
disassembled instruction.
Here's an example of what the output looks like on x86-64 (this is
with 'set debug displaced on'). The two interesting lines contain the
strings 'original insn' and 'replacement insn':
(gdb) step
[displaced] displaced_step_prepare_throw: displaced-stepping 2892655.2892655.0 now
[displaced] displaced_step_prepare_throw: original insn 0x401030: ff 25 e2 2f 00 00 jmp *0x2fe2(%rip) # 0x404018 <puts@got.plt>
[displaced] prepare: selected buffer at 0x401052
[displaced] prepare: saved 0x401052: 1e fa 31 ed 49 89 d1 5e 48 89 e2 48 83 e4 f0 50
[displaced] fixup_riprel: %rip-relative addressing used.
[displaced] fixup_riprel: using temp reg 2, old value 0x7ffff7f8a578, new value 0x401036
[displaced] amd64_displaced_step_copy_insn: copy 0x401030->0x401052: ff a1 e2 2f 00 00 68 00 00 00 00 e9 e0 ff ff ff
[displaced] displaced_step_prepare_throw: prepared successfully thread=2892655.2892655.0, original_pc=0x401030, displaced_pc=0x401052
[displaced] displaced_step_prepare_throw: replacement insn 0x401052: ff a1 e2 2f 00 00 jmp *0x2fe2(%rcx)
[displaced] finish: restored 2892655.2892655.0 0x401052
[displaced] amd64_displaced_step_fixup: fixup (0x401030, 0x401052), insn = 0xff 0xa1 ...
[displaced] amd64_displaced_step_fixup: restoring reg 2 to 0x7ffff7f8a578
0x00007ffff7e402c0 in puts () from /lib64/libc.so.6
(gdb)
One final note. For many targets that support displaced stepping (in
fact all targets except ARM) the replacement instruction is always a
single instruction. But on ARM the replacement could actually be a
series of instructions.
The debug code tries to handle this by disassembling the entire
displaced stepping buffer. Obviously this might actually print more
than is necessary, but there's (currently) no easy way to know how
many instructions to disassemble; that knowledge is all locked in the
architecture specific code. Still I don't think it really hurts, if
someone is looking at this debug then hopefully they known what to
expect.
Obviously we can imagine schemes where the architecture specific
displaced stepping code could communicate back how many bytes its
replacement sequence was, and then our debug print code could use this
to limit the disassembly. But this seems like a lot of effort just to
save printing a few additional instructions in some debug output.
I'm not proposing to do anything about this issue for now.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Alan Modra [Wed, 29 Mar 2023 02:15:16 +0000 (12:45 +1030)]
Sanity check section size in bfd_init_section_compress_status
This function doesn't just initialise for compression, it actually
compresses. This patch sanity checks section size before allocating
buffers for the uncompressed contents.
Alan Modra [Tue, 28 Mar 2023 10:32:36 +0000 (21:02 +1030)]
ld testsuite CFLAGS_FOR_TARGET
run_host_cmd adds $gcc_B_opt and $ld_L_opt to the command line if it
detects the program being run is a compiler. Since the program being
run in lto.exp linking pr28138 is "sh", we need to add these by hand.
This isn't exactly as run_host_cmd does, as it lacks reordering of
any user -B option in $CC_FOR_TARGET, but it's better than ignoring
gcc_B_opt. This fixes a mips64 testsuite fail.
ld_compile adds CFLAGS_FOR_TARGET and other flags as well, so there
is no need for the ld_compile command line to include
CFLAGS_FOR_TARGET. Fixing this is just a tidy.
* testsuite/ld-plugin/lto.exp: Add gcc_B_opt, CFLAGS_FOR_TARGET
and $ld_L_opt to pr28138 link line.
* testsuite/lib/ld-lib.exp (run_ld_link_tests): Don't pass
unnecessary flags to ld_compile.
(run_ld_link_exec_tests, run_cc_link_tests): Likewise.
Tom Tromey [Fri, 17 Mar 2023 22:37:15 +0000 (16:37 -0600)]
Use unrelocated_addr in partial symbol tables
This changes partial symbol tables to use unrelocated_addr for the
text_high and text_low members. This revealed some latent bugs in
ctfread.c, which are fixed here.
projects / thirdparty / binutils-gdb.git / log
