Age | Commit message (Collapse) | Author | Files | Lines |
|
In amd64_get_insn_details, I found this code with a comment explaining why
enc_prefix_offset is not set:
...
else if (vex2_prefix_p (*insn))
{
/* Don't record the offset in this case because this prefix has
no REX.B equivalent. */
insn += 2;
}
...
which I didn't understand until I looked at the only use of enc_prefix_offset,
in fixup_riprel:
...
/* REX.B should be unset (VEX.!B set) as we were using rip-relative
addressing, but ensure it's unset (set for VEX) anyway, tmp_regno
is not r8-r15. */
if (insn_details->enc_prefix_offset != -1)
{
gdb_byte *pfx = &dsc->insn_buf[insn_details->enc_prefix_offset];
if (rex_prefix_p (pfx[0]))
pfx[0] &= ~REX_B;
else if (vex3_prefix_p (pfx[0]))
pfx[1] |= VEX3_NOT_B;
else
gdb_assert_not_reached ("unhandled prefix");
}
...
Fix this by:
- setting enc_prefix_offset for the vex2 case in amd64_get_insn_details,
making the function more regular and easier to understand, and
- handling the vex2 case in the "enc_prefix_offset != -1" clause in
fixup_riprel.
Tested on x86_64-linux.
|
|
After I posted a tentative patch for PR31952, Alexander Monakov pointed out
that the patch broke instruction decoding for instructions vzeroall and
vzeroupper.
Add selftests for these two instructions in amd64-insn-decode, both using
vex2 and vex3 prefixes.
Tested on x86_64-linux.
|
|
I noticed here [1] that the vex2 prefix is essentially a special case of the
vex3 prefix, meaning it's possible to rewrite any insn with a vex2 prefix into
an equivalent one with a vex3 prefix.
Add function vex2_to_vex3 that does precisely that, in the selftests
namespace.
Add a selftest that exercises this function.
Tested on x86_64-linux.
[1] https://en.wikipedia.org/wiki/VEX_prefix
|
|
Factor out the part of fixup_riprel that patches the insn, and use it in a
unit test.
Tested on x86_64-linux.
|
|
I wanted to add a unit test for an an rip-relative amd64 insn, so I did:
...
$ gcc -fPIE hello.c
...
and used an rip-relative insn from main:
...
4005db: 48 8d 3d 1e 00 00 00 lea 0x1e(%rip),%rdi
...
While writing the unit test, I found that amd64_get_used_input_int_reg returns
rbp as input register.
Fix this by using rip_relative_p in amd64_get_used_input_int_reg to handle
this case.
Tested on x86_64-linux.
|
|
Factor out rip_relative_p, and rewrite it to use MODRM_MOD_FIELD and
MODRM_RM_FIELD.
No functional changes.
Tested on x86_64-linux.
|
|
Add a selftest that checks the results of amd64_get_insn_details and related
functions for two basic instructions.
Add a parameter assumptions to amd64_get_used_input_int_regs, to make sure
that this selftest:
...
/* INSN: add %eax,(%rcx). */
...
SELF_CHECK (amd64_get_used_input_int_regs (&details, false)
== ((1 << EAX_REG_NUM) | (1 << ECX_REG_NUM)));
...
passes because it found the "%eax" in the insn, rather than passing because of
this assumption:
...
/* Assume RAX is used. If not, we'd have to detect opcodes that implicitly
use RAX. */
used_regs_mask |= 1 << EAX_REG_NUM;
...
Tested on x86_64-linux.
|
|
The function amd64_get_unused_input_int_reg consists of two parts:
- finding the used int registers in an insn, and
- picking an unused int register.
Factor out the first part as new function amd64_get_used_input_int_regs.
No functional changes.
Tested on x86_64-linux.
|
|
While reading amd64_get_unused_input_int_reg, I noticed that it avoids picking
RSP, which has to do with how the result of the only call to it is going to be
used.
Likewise for picking a register in the RAX ... RDI range.
Fix this by:
- adding an allowed_regs_mask parameter to amd64_get_unused_input_int_reg, and
- properly documenting the value of the corresponding argument in fixup_riprel.
No functional changes.
Tested on x86_64-linux.
|
|
I noticed that amd64_get_unused_input_int_reg uses a signed int for a bit
mask:
...
/* 1 bit for each reg */
int used_regs_mask = 0;
...
There's an assert:
...
gdb_assert (used_regs_mask < 256);
...
which is meant to assert on register numbers >= 8, but if for instance
sizeof (used_regs_mask) == 4 and used_regs_mask == (1 << 31), then that is not
caught because of the signedness.
We could fix this by changing the type to unsigned int, but that only
guarantees 16 bits in the reg mask. Intel CPUs with the APX extension support
32 int registers.
The implementation of amd64_get_unused_input_int_reg doesn't support analyzing
registers with register number >= 8 yet, but now that we're changing the type,
it seems like a good idea to anticipate this.
Fix this by using uint32_t.
Likewise, update the loop over the reg mask:
...
for (i = 0; i < 8; ++i)
{
if (! (used_regs_mask & (1 << i)))
return i;
...
to handle any used_regs_mask value rather than just those for
register number < 8.
Tested on x86_64-linux.
|
|
While reading amd64_get_unused_input_int_reg, I noticed that it first asserts,
then throws an internal_error if no unused register can be found.
Looking at the documentation of gdbarch_displaced_step_copy_insn, it seems
that a failure can be indicated less abruptly, by returning a nullptr.
Fix this by:
- returning -1 in case of failure to find an unused register in
amd64_get_unused_input_int_reg, and
- propagating this to amd64_displaced_step_copy_insn.
Tested on x86_64-linux.
|
|
Fix typos:
...
mainenance ==> maintenance
epilgoue ==> epilogue
commnds ==> commands
readibility ==> readability
informations ==> information
throwed ==> threw
compiletime ==> compile time
namepace ==> namespace
reqired ==> required
explicity ==> explicitly
reqired ==> required
...
|
|
Fix typos:
...
gdb/python/py-framefilter.c:749: indention ==> indentation
gdb/python/py-framefilter.c:837: indention ==> indentation
gdb/python/py-lazy-string.c:35: sting ==> string
gdb/python/py-progspace.c:119: Retun ==> Return
gdb/python/py-progspace.c:139: Retun ==> Return
...
|
|
Fix typos:
...
gdb/python/lib/gdb/disassembler.py:84: dissables ==> disables
gdb/python/lib/gdb/command/xmethods.py:40: experession ==> expression
...
|
|
Fix typos:
...
gdb/guile/scm-lazy-string.c:41: sting ==> string
gdb/guile/lib/gdb/iterator.scm:65: satify ==> satisfy
...
|
|
Fix typos:
...
preprend -> prepend
wth -> with
Connnections -> Connections
...
|
|
Fix typos:
...
Dependant ==> Dependent
...
|
|
Fix typos:
...
atribute ==> attribute
...
|
|
Fix typos:
...
exising ==> existing
afer ==> after
...
|
|
Fix typos:
...
gdb/tui/tui.c:64: releated ==> related
gdb/tui/tui-io.c:50: releated ==> related
...
|
|
Fix typos:
...
gdb/cli/cli-utils.h:85: fuction ==> function
gdb/cli/cli-decode.c:2457: Ambigous ==> Ambiguous
...
|
|
Fix typos in gdbarch_components.py:
...
tranformations ==> transformations
charater ==> character
Noe -> Note
...
and regenerate gdb/gdbarch-gen.h.
|
|
With the hierarchical name patches to GNAT, ada_add_block_renamings
must now be updated as well -- the comment there about the supported
forms of DW_TAG_imported_declaration is no longer correct, and now
full names must sometimes be constructed during the lookup process.
|
|
In the near future, GNAT will start emitting DWARF names in a more
standard way -- specifically, the package structure will be indicated
by nested DW_TAG_module DIEs and a given entity will be nested in its
package and only have a simple name.
This patch changes gdb to understand this style of naming, while still
supporting the existing GNAT output.
A few special cases are needed. I've commented them.
The name-computing code for the full DWARF reader is very complicated
-- much too complicated, in my opinion. There are already several
bugs in bugzilla about this (search for "physname"... but there are
others as well), so I haven't filed any new ones.
When I started this project, I thought it would solve some memory
overuse issues we sometimes see from how the index-sharding code
interacts with the GNAT-specific post-pass. However, to my surprise,
the Ada code in gdb relies on some details of symbol naming, and so
I've had to add code here to synthesize "linkage" names in some cases.
This is unfortunate, but I think can eventually be fixed; I will file
a bug to track this issue.
|
|
Unfortunately, due to some details of how the Ada support in gdb
currently works, the DWARF reader will still have to synthesize some
"full name" entries after the cooked index has been constructed.
You can see one particular finding related to this in:
https://sourceware.org/bugzilla/show_bug.cgi?id=32142
This patch adds a new flag to cooked_index_entry::full_name to enable
the construction of these names.
I hope to redo this part of the Ada support eventually, so that this
code can be removed and the full-name entries simply not created.
|
|
handle_gnat_encoded_entry might create synthetic cooked index entries
for Ada packages. These aren't currently kept in m_entries, but it
seems to me that they should be, particularly because a forthcoming
GNAT will emit explicit DW_TAG_module for these names -- with this
change, the indexes will be roughly equivalent regardless of which
compiler was used.
|
|
This updates read_module_type to turn DW_TAG_module into a
TYPE_CODE_NAMESPACE when the CU represents Ada code.
Note that the GNAT that generates this isn't generally available yet
and so this shouldn't have an impact on current code.
|
|
Currently, gdb will synthesize DW_TAG_module entries for Ada names.
These entries are treated specially by the index writer,
When GNAT starts emitting DW_TAG_module, the special case will be
incorrect, because there will be non-synthetic DW_TAG_module entries
in the index.
This patch arranges to mark the synthetic entries and changes the
index writer to follow.
|
|
In GCC we decided to use DW_TAG_module to represent Ada packages, so
make this same decision in gdb. This also updates tag_matches_domain
to handle this case.
|
|
This changes a few spots in the DWARF reader to use dwarf2_full_name
when computing the name of a type. This gives the correct name when a
type is nested in a namespace. This oddity probably wasn't noticed
before because some of the types in question are either normally
anonymous in C++ (e.g, array type) or do not appear in a namespace
(base type).
|
|
With the coming changes to GNAT, gdb must compare the unqualified
names of two enum types.
Currently, GNAT will fully-qualify enumeration constant names, so for
instance one might see "enum_with_gap__lit4" as the name.
GNAT also may emit a copy of an enumeration type when a newtype is
involved. E.g., in the arr_acc_idx_w_gap.exp test case, this can
occur for the base type of this subtype:
type Enum_Subrange is new Enum_With_Gaps range Lit1 .. Lit3;
(Note that the base type of this subrange is anonymous.)
With some forthcoming changes to GNAT, these names will no longer be
qualified -- and because the newtype is anonymous, they can't be
identically qualified. But, in gdb we still want "lit4" to resolve
without ambiguity in this scenario.
The fix is to change ada_identical_enum_types_p to compare unqualified
enum names. This will work correctly with both variants of the
compiler, and with -fgnat-encodings=all as well.
|
|
With the coming changes to GNAT, we may see two distinct but
equivalent enum types in the DWARF. In this case, it's better to use
ada_identical_enum_types_p rather than types_equal when comparing
these types... something that matters when using 'Enum_Rep.
|
|
This patch applies a few fixes to gdb.ada/fun_overload_menu.exp.
It adds some comments to the source and uses this to extract line
numbers. This is used to ensure that two otherwise-equivalent results
are in fact different, so that the test really checks that the result
is correct.
It also changes the test_menu proc to accept a list of possible
results. This lets the test work regardless of the order in which the
menu items are presented by gdb.
Finally, like an earlier patch, it changes the test to optionally
accept unqualified names from gdb.
|
|
With some forthcoming changes to GNAT, the two Get_Value functions in
this test case will end up with the same name (with the current GNAT,
one ends up with a "__2" suffix). This change will cause one test to
set multiple breakpoints; this patch changes the test to work with
either version of the compiler.
|
|
The "Rec" type in ptype-o.exp is currently named "prog__rec" by the
compiler. However, with my changes to GNAT, the type will no longer
have a prefix, as it is local to a procedure.
Changing this to just use "rec" works fine with the new compiler, but
then fails with older compilers. To allow correct operation with both
compilers, this patch simply moves the type into a new package. This
doesn't affect the meaning of the test, which is just ensuring that
ptype/o works in a certain case.
Note that the more obvious fix of just using "ptype/o rec" does not
work with the current GNAT. I haven't investigated this but I did
file a bug to track it:
https://sourceware.org/bugzilla/show_bug.cgi?id=32169
|
|
Currently, when a type is declared in a subprogram that isn't part of
a package, gdb will give this type a qualified name. E.g., in the
program for gdb.ada/arr_arr.exp:
procedure Foo is
type Array2_First is array (24 .. 26) of Integer;
gdb will name this type 'foo.array2_first'.
However, with some coming changes to GNAT (and with the remainder of
this series applied as well), this will no longer happen. Instead,
such types will be given their local name. IMO this makes more sense
anyway.
This patch updates most of the Ada tests to allow either form in the
spots where it matters. Both are accepted so that the tests continue
to work with older versions of GNAT. (A few tests are handled in
separate patches; this patch only contains the straightforward
changes.)
|
|
ada_variant_discrim_name does this:
for (discrim_end = name + strlen (name) - 6; discrim_end != name;
If NAME is too short, this will construct an invalid pointer, perhaps
causing a crash.
This patch arranges to check the length first.
|
|
With some forthcoming changes to GNAT, gdb might see a nameless enum
in ada_resolve_enum, causing a crash. This patch allows an anonymous
enum type to be considered identical to a named type when the contents
are identical.
|
|
If you have many sections, "maint print sections" can take a very long
time (due to a bug). If you happen to "c" at the pagination prompt,
this can't be interrupted. This patch adds a QUIT to the loop to at
least allow interruption.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32758
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
|
|
This changes a couple of gdbarch methods to use 'const' for an
"asymbol *" parameter. These methods shouldn't be modifying the
underlying symbol in the BFD.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
I like using `this->` when it's unclear that the method or field
accessed is within the current class, but when accessing a private
member prefixed with `m_`, it's unnecessary, as the prefix makes it
clear. Remove some instances of it (some coming from the previous
patch, other pre-existing) to de-clutter the code a bit.
Change-Id: Ia83d0bce51d222fa3ac3d756d50170ec6ed12b94
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make all fields of cutu_reader private, then add getters for whatever
needs to be accessed outside of cutu_reader. This should help spot
what's used by cutu_reader itself, and what is used by others.
Change-Id: I71cb73fffa5d70cc9c7fc68bf74db937e84c2db1
Approved-By: Tom Tromey <tom@tromey.com>
|
|
These functions don't need to receive a cutu_reader, they only use it to
obtain the contained dwarf2_cu, so change them to accept a dwarf2_cu.
This helps reduce the creep of cutu_reader a little bit.
Change-Id: Iebb3c4697a4aec638b47423b3ac59077d4fa5090
Approved-By: Tom Tromey <tom@tromey.com>
|
|
With the hope of organizing things better and spotting patterns that
could lead to simplification, move all these functions to be methods of
cutu_reader. At least, this gives a good picture of what the entry
points for DIE and attribute reading are, by looking at what methods are
public.
Right now, my vague understanding of cutu_reader is that it does 3
things:
- it provides means to navigate and read the DIE tree, abstracting
things like whether the real content is in a DWO file or not
- it builds a dwarf2_cu object, for its own use but also for the use of
the caller
- it fills in missing details in the passed in dwarf2_per_cu
In the future, I'd like to separate those concerns. I think that
cutu_reader could retain the first one of those concerns, while the
other two could be done by other classes or functions, perhaps using
cutu_reader under the hood.
Change-Id: I04e0d6c864bbc09c7071ac8e9493e1e54c093d68
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I find it much more readable this way, with one idea per paragraph.
Change-Id: Ib31b410867c8444e0f3200681881f54f1b8ebea8
Approved-By: Tom Tromey <tom@tromey.com>
|
|
init_cu_die_reader is only used inside cutu_reader, to initialize fields
of cutu_reader, so make it a private method.
Change-Id: Iaa80d4dbb8d0fa35bcac18ee70e147276874cc1b
Approved-By: Tom Tromey <tom@tromey.com>
|
|
read_cutu_die_from_dwo is only used as a helper to cutu_reader, so make
it a private method of cutu_reader.
Remove the "result_reader" parameter, because it's always "this".
Change-Id: I7df6162137451c160f0e6bf3539569fcb2421eff
Approved-By: Tom Tromey <tom@tromey.com>
|
|
When running codespell on gdbsupport, we get:
...
$ codespell gdbsupport
gdbsupport/common-debug.h:218: invokable ==> invocable
gdbsupport/osabi.h:51: configury ==> configurable
gdbsupport/ChangeLog-2020-2021:344: ro ==> to, row, rob, rod, roe, rot
gdbsupport/ChangeLog-2020-2021:356: contaning ==> containing
gdbsupport/common.m4:19: configury ==> configurable
gdbsupport/Makefile.am:97: configury ==> configurable
gdbsupport/Makefile.in:811: configury ==> configurable
gdbsupport/event-loop.cc:84: useable ==> usable
gdbsupport/configure:15904: assigment ==> assignment
...
Some of these files we want to skip in a spell check, because they're
generated. We also want to skip ChangeLogs, we don't actively maintain those.
Add a file gdbsupport/setup.cfg with a codespell section, that skips those
files. The choice for setup.cfg (rather than say .codespellrc) comes from the
presence of gdb/setup.cfg.
That leaves invokable, configury and useable. I think configury is a common
expression in our context, and for invokable and useable I don't manage to
find out whether they really need rewriting, so I'd rather leave them alone
for now.
Add these to a file gdb/contrib/codespell-ignore-words.txt, and use the file in
gdbsupport/setup.cfg.
This makes the directory codespell clean:
...
$ codespell --config gdbsupport/setup.cfg gdbsupport
$
...
Because codespell seems to interpret filenames relative to the working
directory rather than relative to the config file, and the filename used in
gdbsupport/setup.cfg is gdb/contrib/codespell-ignore-words.txt, this simple
invocation doesn't work:
...
$ cd gdbsupport
$ codespell
...
because codespell can't find gdbsupport/gdb/contrib/codespell-ignore-words.txt.
We could fix this by using ../gdb/contrib/codespell-ignore-words.txt instead, but
likewise that breaks this invocation:
...
$ codespell --config gdbsupport/setup.cfg gdbsupport
...
I can't decide which one is worse, so I'm sticking with
gdb/contrib/codespell-ignore-words.txt for now.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The lineptr variable isn't really necessary, we can just keep using
linebuffer, since the original value is linebuffer isn't needed. Remove
lineptr, and fix some comparisons to be explicit.
Change-Id: If2f7df43bf79efd40149e46d5c77f9bc0439f879
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This teaches cooked_index_entry::full_name that "::" is the separator
for Fortran. I don't know enough Fortran to write a test case for
this. However, a different series I am working on has a regression if
this patch is not applied.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|