As noted in an earlier patch, the Ada lexer does not handle multi-byte
bracket sequences. This patch adds support for these for character
literals. gdb does not generally seem to handle the Ada wide string
types, so for the time being these continue to be excluded -- but an
explicit error is added to make this more clear.
In Ada, an enum can contain character literals. GNAT encodes these
values in a special way. For example, the Unicode character U+0178
would be represented as 'QW0178' in the DWARF:
<3><112f>: Abbrev Number: 2 (DW_TAG_enumerator)
<1130> DW_AT_name : (indirect string, offset: 0x19ff): QW0178
<1134> DW_AT_const_value : 2
gdb handles this reasonably well, but failed to handle the 'QWW'
encoding, which is used for characters outside the base plane.
Also, while working on this, I noticed that gdb will print the decimal
value for an enum character constant:
(gdb) print Char_X
$2 = 1 'x'
This is a nice feature, IMO, because in this situation the 'x' enum
constant does not have its usual decimal value -- it has the value
that's assigned based on the enumeration type.
However, gdb did not do this when it decided to print the constant
using the bracket notation:
(gdb) print Char_Thorn
$3 = ["de"]
This patch changes gdb to print the decimal value here as well, and to
put the bracket notation in single quotes -- otherwise gdb will be
printing something that it can't then read. Now it looks like:
(gdb) print Char_Thorn
$3 = 4 '["de"]'
Note that gdb can't read longer bracket notations, like the other ones
printed in this test case:
(gdb) print Char_King
$4 = 3 '["01fa00"]'
While I think this is a bug, I plan to fix it separately.
Finally, in the new test case, the copyright dates are chosen this way
because this all started as a copy of an existing test.
Add a getter and a setter for a symbol's line. Remove the corresponding macro
and adjust all callers.
Change-Id: I229f2b8fcf938c07975f641361313a8761fad9a5
Add a getter and a setter for a symbol's type. Remove the corresponding
macro and adjust all callers.
Change-Id: Ie1a137744c5bfe1df4d4f9ae5541c5299577c8de
Add a getter and a setter for whether a symbol is an argument. Remove
the corresponding macro and adjust all callers.
Change-Id: I71b4f0465f3dfd2ed8b9e140bd3f7d5eb8d9ee81
Add a getter and a setter for whether a symbol is objfile owned. Remove
the corresponding macro and adjust all callers.
Change-Id: Ib7ef3718d65553ae924ca04c3fd478b0f4f3147c
Add a getter and a setter for a symbol's domain. Remove the
corresponding macro and adjust all callers.
Change-Id: I54465b50ac89739c663859a726aef8cdc6e4b8f3
Add a getter and a setter for a compunit_symtab's blockvector. Remove
the corresponding macro and adjust all callers.
Change-Id: I99484c6619dcbbea7c5d89c72aa660316ca62f64
A common pattern for string_file is to want to move out the internal
string buffer, because it is the result of the computation that we want
to return. It is the reason why string_file::string returns a non-const
reference, as explained in the comment. I think it would make sense to
have a dedicated method for that instead and make string_file::string
return a const reference.
This allows removing the explicit std::move in the typical case. Note
that compile_program::compute was missing a move, meaning that the
resulting string was copied. With the new version, it's not possible to
forget to move.
Change-Id: Ieaefa35b73daa7930b2f3a26988b6e3b4121bb79
All the Ada catchpoints use the same breakpoint_ops contents, because
the catchpoint itself records its kind. This patch simplifies the
code by removing the redundant ops structures.
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
There are some loops in gdb that use ARRAY_SIZE (or a wordier
equivalent) to loop over a static array. This patch changes some of
these to use foreach instead.
Regression tested on x86-64 Fedora 34.
On openSUSE Leap 42.3, with system compiler gcc 4.8.5 I run into:
...
(gdb) print u_one_two_three^M
src/gdb/gdbtypes.h:1050: internal-error: field: \
Assertion `idx >= 0 && idx < num_fields ()' failed.^M
...
We run into trouble while doing this in
ada_is_unconstrained_packed_array_type:
...
1953 return TYPE_FIELD_BITSIZE (type, 0) > 0;
...
which tries to get field 0 from a type without fields:
...
(gdb) p type->num_fields ()
$6 = 0
...
which is the case because the type is a typedef:
...
(gdb) p type->code ()
$7 = TYPE_CODE_TYPEDEF
...
Fix this by using the type referenced by the typedef instead.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28323
Commit 4bce7cdaf4 ("gdbsupport: add array_view copy function") caused
an internal error when running gdb.ada/packed_array_assign.exp:
print pra(1) := pr^M
/home/smarchi/src/binutils-gdb/gdb/../gdbsupport/array-view.h:217: internal-error: copy: Assertion `dest.size () == src.size ()' failed.^M
I am not sure what's the root cause of this, whether it is a GDB bug
exposed by using the array_view copy function or not. Back out the
change that triggers the internal error for now, while we investigate
it.
Change-Id: I055ab14143e4cfd3ca7ce8f4855c6c3c05db52a7
An assertion was recently added to array_view::operator[] to ensure we
don't do out of bounds accesses. However, when the array_view is copied
to or from using memcpy, it bypasses that safety.
To address this, add a `copy` free function that copies data from an
array view to another, ensuring that the destination and source array
views have the same size. When copying to or from parts of an
array_view, we are expected to use gdb::array_view::slice, which does
its own bounds check. With all that, any copy operation that goes out
of bounds should be caught by an assertion at runtime.
copy is implemented using std::copy and std::copy_backward, which, at
least on libstdc++, appears to pick memmove when copying trivial data.
So in the end there shouldn't be much difference vs using a bare memcpy,
as we do right now. When copying non-trivial data, std::copy and
std::copy_backward assigns each element in a loop.
To properly support overlapping ranges, we must use std::copy or
std::copy_backward, depending on whether the destination is before the
source or vice-versa. std::copy and std::copy_backward don't support
copying exactly overlapping ranges (where the source range is equal to
the destination range). But in this case, no copy is needed anyway, so
we do nothing.
The order of parameters of the new copy function is based on std::copy
and std::copy_backward, where the source comes before the destination.
Change a few randomly selected spots to use the new function, to show
how it can be used.
Add a test for the new function, testing both with arrays of a trivial
type (int) and of a non-trivial type (foo). Test non-overlapping
ranges as well as three kinds of overlapping ranges: source before dest,
dest before source, and dest == source.
Change-Id: Ibeaca04e0028410fd44ce82f72e60058d6230a03
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
I don't find that the bpstat typedef, which hides a pointer, is
particularly useful. In fact, it confused me many times, and I just see
it as something to remember that adds cognitive load. Also, with C++,
we might want to be able to pass bpstats objects by const-reference, not
necessarily by pointer.
So, remove the bpstat typedef and rename struct bpstats to bpstat (since
it represents one bpstat, it makes sense that it is singular).
Change-Id: I52e763b6e54ee666a9e045785f686d37b4f5f849
There's a common pattern to call add_basic_prefix_cmd and
add_show_prefix_cmd to add matching set and show commands. Add the
add_setshow_prefix_cmd function to factor that out and use it at a few
places.
Change-Id: I6e9e90a30e9efb7b255bf839cac27b85d7069cfd
The bug fixed by this [1] patch was caused by an out-of-bounds access to
a value's content. The code gets the value's content (just a pointer)
and then indexes it with a non-sensical index.
This made me think of changing functions that return value contents to
return array_views instead of a plain pointer. This has the advantage
that when GDB is built with _GLIBCXX_DEBUG, accesses to the array_view
are checked, making bugs more apparent / easier to find.
This patch changes the return types of these functions, and updates
callers to call .data() on the result, meaning it's not changing
anything in practice. Additional work will be needed (which can be done
little by little) to make callers propagate the use of array_view and
reap the benefits.
[1] https://sourceware.org/pipermail/gdb-patches/2021-September/182306.html
Change-Id: I5151f888f169e1c36abe2cbc57620110673816f3
The "add accessors for field (and call site) location" patch caused a
gdb crash when running the internal AdaCore testsuite. This turned
out to be a latent bug in ada-lang.c.
The immediate cause of the bug is that find_struct_field
unconditionally uses TYPE_FIELD_BITPOS. This causes an assert for a
dynamic type.
This patch fixes the problem by doing two things. First, it changes
find_struct_field to use a dummy value for the field offset in the
situation where the offset is not actually needed by the caller. This
works because the offset isn't used in any other way -- only as a
result.
Second, this patch assures that calls to find_struct_field use a
resolved type when the offset is needed. For
value_tag_from_contents_and_address, this is done by resolving the
type explicitly. In ada_value_struct_elt, this is done by passing
nullptr for the out parameters when they are not needed (the second
call in this function already uses a resolved type).
Note that, while we believe the parent field probably can't occur at a
variable offset, the patch still updates this code path, just in case.
I've updated an existing test case to reproduce the crash.
I'm checking this in.
This changes gdb to check the index that is passed to type::field.
This caught one bug in the Ada code when running the test suite
(actually I found the bug first, then realized that the check would
have helped), so this patch fixes that as well.
Regression tested on x86-64 Fedora 34.
Add accessors for the various location values in struct field. This
lets us assert that when we get a location value of a certain kind (say,
bitpos), the field's location indeed contains a value of that kind.
Remove the SET_FIELD_* macros, instead use the new setters directly.
Update the FIELD_* macros used to access field locations to go through
the getters. They will be removed in a subsequent patch.
There are places where the FIELD_* macros are used on call_site_target
structures, because it contains members of the same name (loc_kind and
loc). For now, I have replicated the getters/setters in
call_site_target. But we could perhaps eventually factor them in a
"location" structure that can be used at both places.
Note that the field structure, being zero-initialized, defaults to a
bitpos location with value 0. While writing this patch, I tried to make
it default to an "unset" location, to catch places where we would miss
setting a field's location. However, I found that some places relied on
the default being "bitpos 0", so I left it as-is. This change could
always be done as follow-up work, making these places explicitly set the
"bitpos 0" location.
I found two issues to fix:
- I got some failures in the gdb.base/infcall-nested-structs-c++.exp
test. They were caused by two functions in amd64-tdep.c using
TYPE_FIELD_BITPOS before checking if the location is of the bitpos
kind, which they do indirectly through `field_is_static`. Simply
move getting the bitpos below the field_is_static call.
- I got a failure in gdb.xml/tdesc-regs.exp. It turns out that in
make_gdb_type_enum, we set enum field values using SET_FIELD_BITPOS,
and later access them through FIELD_ENUMVAL. Fix that by using
set_loc_enumval to set the value.
Change-Id: I53d3734916c46457576ba11dd77df4049d2fc1e8
This makes the Ada-specific "varsize-limit" a synonym for
"max-value-size", and removes the Ada-specific checks of the limit.
I am not certain of the history here, but it seems to me that this
code is fully obsolete now. And, removing this makes it possible to
index large Ada arrays without triggering an error. A new test case
is included to demonstrate this.
I noticed that some methods in language_defn could use
unique_xmalloc_ptr<char> rather than a plain 'char *'. This patch
implements this change, fixing up the fallout and changing
gdb_demangle to also return this type. In one spot, std::string is
used to simplify some related code, and in another, an auto_obstack is
used to avoid manual management.
Regression tested on x86-64 Fedora 34.
Remove the `TYPE_FIELD_NAME` and `FIELD_NAME` macros, changing all the
call sites to use field::name directly.
Change-Id: I6900ae4e1ffab1396e24fb3298e94bf123826ca6
Add the `name` and `set_name` methods on `struct field`, in order to
remove `FIELD_NAME` and `TYPE_FIELD_NAME` macros. In this patch, the
macros are changed to use `field::name`, so all the call sites that are
used to set the field's name are changed to use `field::set_name`.
The next patch will remove the macros completely.
Note that because of the name clash between the existing field named
`name` and the new method, I renamed the field `m_name`. It is not
private per-se, because we can't make `struct field` a non-POD yet, but
it should be considered private anyway (not accessed outside `struct
field`).
Change-Id: If16ddbca4e0c39d0ff9da420bb5cdebe5b9b0896
I noticed that value_true is declared in language.h and defined in
language.c. However, as part of the value API, I think it would be
better in one of those files. And, because it is very short, I
changed it to be an inline function in value.h. I've also removed a
comment from the implementation, on the basis that it seems obsolete
-- if the change it suggests was needed, it probably would have been
done by now; and if it is needed in the future, odds are it would be
done differently anyway.
Finally, this patch also changes value_true and value_logical_not to
return a bool, and updates some uses.
With current master and gcc 7.5.0/8.5.0, we have this timeout:
...
(gdb) print s^M
Multiple matches for s^M
[0] cancel^M
[1] s at src/gdb/testsuite/gdb.ada/interface/foo.adb:20^M
[2] s at src/gdb/testsuite/gdb.ada/interface/foo.adb:?^M
> FAIL: gdb.ada/interface.exp: print s (timeout)
...
[ The FAIL doesn't reproduce with gcc 9.3.1. This difference in
behaviour bisects to gcc commit d70ba0c10de.
The FAIL with earlier gcc bisects to gdb commit ba8694b650. ]
The FAIL is caused by gcc generating this debug info describing a named
artificial variable:
...
<2><1204>: Abbrev Number: 31 (DW_TAG_variable)
<1205> DW_AT_name : s.14
<1209> DW_AT_type : <0x1213>
<120d> DW_AT_artificial : 1
<120d> DW_AT_location : 5 byte block: 91 e0 7d 23 18 \
(DW_OP_fbreg: -288; DW_OP_plus_uconst: 24)
...
An easy way to fix this would be to simply not put named artificial variables
into the symbol table. However, that causes regressions for Ada. It relies
on being able to get the value from such variables, using a named reference.
Fix this instead by marking the symbol as artificial, and:
- ignoring such symbols in ada_resolve_variable, which fixes the FAIL
- ignoring such ada symbols in do_print_variable_and_value, which prevents
them from showing up in "info locals"
Note that a fix for the latter was submitted here (
https://sourceware.org/pipermail/gdb-patches/2008-January/054994.html ), and
this patch borrows from it.
Tested on x86_64-linux.
Co-Authored-By: Joel Brobecker <brobecker@adacore.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28180
The compiler may add a suffix to a mangled name. A typical example
would be splitting a function and creating a ".cold" variant.
This patch changes Ada decoding (aka demangling) to handle these
suffixes. It also changes the encoding process to handle them as
well.
A symbol like "function.cold" will now be displayed to the user as
"function[cold]". The "." is not simply preserved because that is
already used in Ada.
Pierre-Marie noticed that the Ada expression "TYPE'(NAME)" resolved
incorrectly when "TYPE" was an enumeration type. Here, "NAME" should
be unambiguous.
This patch fixes this problem. Note that the patch is not perfect --
it does not give an error if TYPE is an enumeration type but NAME is
not an enumerator but does have some other meaning in scope. Fixing
this proved difficult, and so I've left it out.
In Ada, an enumeration type can use a character literal as one of the
enumerators. The Ada expression parser handles the appropriate
conversion.
It turns out, though, that this conversion was handled incorrectly.
For an expression like TYPE'(EXP), the conversion would be done for
any such literal appearing in EXP -- but only the outermost such
expression should really be affected.
This patch defers the conversion until the resolution phase, fixing
the bug.
In a subsequent patch, it will be convenient if an Ada expression
operation can supply its own replacement object. This patch refactors
Ada expression resolution to make this possible.
I noticed that add_symbols_from_enclosing_procs is empty, and can be
removed. The one caller, ada_add_local_symbols, can also be
simplified, removing some code that, I think, was an incorrect attempt
to handle nested functions.
Following on from the previous commit, this commit changes the API of
value_struct_elt to take gdb::optional<gdb::array_view<value *>>
instead of a pointer to the gdb::array_view.
This makes the optional nature of the array_view parameter explicit.
This commit is purely a refactoring commit, there should be no user
visible change after this commit.
I have deliberately kept this refactor separate from the previous two
commits as this is a more extensive change, and I'm not 100% sure that
using gdb::optional for the parameter type, instead of a pointer, is
going to be to everyone's taste. If there's push back on this patch
then this one can be dropped from the series.
gdb/ChangeLog:
* ada-lang.c (desc_bounds): Use '{}' instead of NULL to indicate
an empty gdb::optional when calling value_struct_elt.
(desc_data): Likewise.
(desc_one_bound): Likewise.
* eval.c (structop_base_operation::evaluate_funcall): Pass
gdb::array_view, not a gdb::array_view* to value_struct_elt.
(eval_op_structop_struct): Use '{}' instead of NULL to indicate
an empty gdb::optional when calling value_struct_elt.
(eval_op_structop_ptr): Likewise.
* f-lang.c (fortran_structop_operation::evaluate): Likewise.
* guile/scm-value.c (gdbscm_value_field): Likewise.
* m2-lang.c (eval_op_m2_high): Likewise.
(eval_op_m2_subscript): Likewise.
* opencl-lang.c (opencl_structop_operation::evaluate): Likewise.
* python/py-value.c (valpy_getitem): Likewise.
* rust-lang.c (rust_val_print_str): Likewise.
(rust_range): Likewise.
(rust_subscript): Likewise.
(eval_op_rust_structop): Likewise.
(rust_aggregate_operation::evaluate): Likewise.
* valarith.c (value_user_defined_op): Likewise.
* valops.c (search_struct_method): Change parameter type, update
function body accordingly, and update header comment.
(value_struct_elt): Change parameter type, update function body
accordingly.
* value.h (value_struct_elt): Update declaration.
When ada_decode encounters a name that it cannot decode, it simply
wraps it in <...>, which is used elsewhere in the Ada code to indicate
that a verbatim match should be done.
A subequent patch needed the ability to suppress this wrapping, so
this patch adds a new mode to ada_decode.
2021-06-25 Tom Tromey <tromey@adacore.com>
* ada-lang.c (ada_decode): Add wrap parameter.
* ada-lang.h (ada_decode): Add wrap parameter.
I spotted some indentation issues where we had some spaces followed by
tabs at beginning of line, that I wanted to fix. So while at it, I did
a quick grep to find and fix all I could find.
gdb/ChangeLog:
* Fix tab after space indentation issues throughout.
Change-Id: I1acb414dd9c593b474ae2b8667496584df4316fd
Add the breakpoint::locations method, which returns a range that can be
used to iterate over a breakpoint's locations. This shortens
for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
into
for (bp_location *loc : b->locations ())
Change all the places that I found that could use it.
gdb/ChangeLog:
* breakpoint.h (bp_locations_range): New.
(struct breakpoint) <locations>: New. Use where possible.
Change-Id: I1ba2f7d93d57e544e1f8609124587dcf2e1da037
gdb/ChangeLog:
* ui-out.h (class ui_out): Add ui_out::text accepting a constant
reference to a std::string. Fix all callers using
std::string::c_str.
* ui-out.c (ui_out::text): Ditto.
Previously, the prefixname field of struct cmd_list_element was manually
set for prefix commands. This seems verbose and error prone as it
required every single call to functions adding prefix commands to
specify the prefix name while the same information can be easily
generated.
Historically, this was not possible as the prefix field was null for
many commands, but this was fixed in commit
3f4d92ebdf by Philippe Waroquiers, so
we can rely on the prefix field being set when generating the prefix
name.
This commit also fixes a use after free in this scenario:
* A command gets created via Python (using the gdb.Command class).
The prefix name member is dynamically allocated.
* An alias to the new command is created. The alias's prefixname is set
to point to the prefixname for the original command with a direct
assignment.
* A new command with the same name as the Python command is created.
* The object for the original Python command gets freed and its
prefixname gets freed as well.
* The alias is updated to point to the new command, but its prefixname
is not updated so it keeps pointing to the freed one.
gdb/ChangeLog:
* command.h (add_prefix_cmd): Remove the prefixname argument as
it can now be generated automatically. Update all callers.
(add_basic_prefix_cmd): Ditto.
(add_show_prefix_cmd): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_abbrev_prefix_cmd): Ditto.
* cli/cli-decode.c (add_prefix_cmd): Ditto.
(add_basic_prefix_cmd): Ditto.
(add_show_prefix_cmd): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_abbrev_prefix_cmd): Ditto.
* cli/cli-decode.h (struct cmd_list_element): Replace the
prefixname member variable with a method which generates the
prefix name at runtime. Update all code reading the prefix
name to use the method, and remove all code setting it.
* python/py-cmd.c (cmdpy_destroyer): Remove code to free the
prefixname member as it's now a method.
(cmdpy_function): Determine if the command is a prefix by
looking at prefixlist, not prefixname.
Now that libiberty includes htab_eq_string, we can remove the
identical function from gdb.
gdb/ChangeLog
2021-05-07 Tom Tromey <tom@tromey.com>
* breakpoint.c (ambiguous_names_p): Use htab_eq_string.
* utils.c (streq_hash): Remove.
* utils.h (streq_hash): Don't declare.
* completer.c (completion_tracker::discard_completions): Update
comment.
* ada-lang.c (_initialize_ada_language): Use htab_eq_string.
Running the AdaCore internal test suite with -fgnat-encodings=minimal
found a gdb crash. The bug is that GDB ends up with a typedef in
ada_index_type, resulting in a NULL dereference.
This crash can be reproduced using GCC 11 with the included test case.
Tested on x86-64 Fedora 32. Because this is Ada-specific, and was
already reviewed by Joel, I am going to check it in.
2021-04-30 Tom Tromey <tromey@adacore.com>
* ada-lang.c (ada_index_type): Use ada_check_typedef.
gdb/testsuite/ChangeLog
2021-04-30 Tom Tromey <tromey@adacore.com>
* gdb.ada/enum_idx_packed/pck.ads (My_Enum, My_Array_Type)
(Confused_Array): New types.
* gdb.ada/enum_idx_packed/foo.adb (Confused_Array): New variable.
* gdb.ada/enum_idx_packed.exp: Add new tests.
