This patch started by adding comprehensive unit tests for enum_flags.
For the testing part, it adds:
- tests of normal expected uses of the API.
- checks that _invalid_ uses of the API would fail to compile. I.e.,
it validates that enum_flags really is a strong type, and that
incorrect mixing of enum types would be caught at compile time. It
pulls that off making use of SFINEA and C++11's decltype/constexpr.
This revealed many holes in the enum_flags API. For example, the f1
assignment below currently incorrectly fails to compile:
enum_flags<flags> f1 = FLAG1;
enum_flags<flags> f2 = FLAG2 | f1;
The unit tests also revealed that this useful use case doesn't work:
enum flag { FLAG1 = 1, FLAG2 = 2 };
enum_flags<flag> src = FLAG1;
enum_flags<flag> f1 = condition ? src : FLAG2;
It fails to compile because enum_flags<flag> and flag are convertible
to each other.
Turns out that making enum_flags be implicitly convertible to the
backing raw enum type was not a good idea.
If we make it convertible to the underlying type instead, we fix that
ternary operator use case, and, we find cases throughout the codebase
that should be using the enum_flags but were using the raw backing
enum instead. So it's a good change overall.
Also, several operators were missing.
These holes and more are plugged by this patch, by reworking how the
enum_flags operators are implemented, and making use of C++11's
feature of being able to delete methods/functions.
There are cases in gdb/compile/ where we need to call a function in a
C plugin API that expects the raw enum. To address cases like that,
this adds a "raw()" method to enum_flags. This way we can keep using
the safer enum_flags to construct the value, and then be explicit when
we need to get at the raw enum.
This makes most of the enum_flags operators constexpr. Beyond
enabling more compiler optimizations and enabling the new unit tests,
this has other advantages, like making it possible to use operator|
with enum_flags values in switch cases, where only compile-time
constants are allowed:
enum_flags<flags> f = FLAG1 | FLAG2;
switch (f)
{
case FLAG1 | FLAG2:
break;
}
Currently that fails to compile.
It also switches to a different mechanism of enabling the global
operators. The current mechanism isn't namespace friendly, the new
one is.
It also switches to C++11-style SFINAE -- instead of wrapping the
return type in a SFINAE-friently structure, we use an unnamed template
parameter. I.e., this:
template <typename enum_type,
typename = is_enum_flags_enum_type_t<enum_type>>
enum_type
operator& (enum_type e1, enum_type e2)
instead of:
template <typename enum_type>
typename enum_flags_type<enum_type>::type
operator& (enum_type e1, enum_type e2)
Note that the static_assert inside operator~() was converted to a
couple overloads (signed vs unsigned), because static_assert is too
late for SFINAE-based tests, which is important for the CHECK_VALID
unit tests.
Tested with gcc {4.8, 7.1, 9.3} and clang {5.0.2, 10.0.0}.
gdb/ChangeLog:
* Makefile.in (SELFTESTS_SRCS): Add
unittests/enum-flags-selftests.c.
* btrace.c (ftrace_update_caller, ftrace_fixup_calle): Use
btrace_function_flags instead of enum btrace_function_flag.
* compile/compile-c-types.c (convert_qualified): Use
enum_flags::raw.
* compile/compile-cplus-symbols.c (convert_one_symbol)
(convert_symbol_bmsym):
* compile/compile-cplus-types.c (compile_cplus_convert_method)
(compile_cplus_convert_struct_or_union_methods)
(compile_cplus_instance::convert_qualified_base):
* go-exp.y (parse_string_or_char): Add cast to int.
* unittests/enum-flags-selftests.c: New file.
* record-btrace.c (btrace_thread_flag_to_str): Change parameter's
type to btrace_thread_flags from btrace_thread_flag.
(record_btrace_cancel_resume, record_btrace_step_thread): Change
local's type to btrace_thread_flags from btrace_thread_flag. Add
cast in DEBUG call.
gdbsupport/ChangeLog:
* common/enum-flags.h: Include "traits.h".
(DEF_ENUM_FLAGS_TYPE): Declare a function instead of defining a
structure.
(enum_underlying_type): Update comment.
(namespace enum_flags_detail): New. Move struct zero_type here.
(EnumIsUnsigned, EnumIsSigned): New.
(class enum_flags): Make most methods constexpr.
(operator&=, operator|=, operator^=): Take an enum_flags instead
of an enum_type.
(operator enum_type()): Delete.
(operator&, operator|, operator^, operator~): Delete, moved out of
class.
(raw()): New method.
(is_enum_flags_enum_type_t): Declare.
(ENUM_FLAGS_GEN_BINOP, ENUM_FLAGS_GEN_COMPOUND_ASSIGN)
(ENUM_FLAGS_GEN_COMP): New. Use them to reimplement global
operators.
(operator~): Now constexpr and reimplemented.
(operator<<, operator>>): New deleted functions.
* valid-expr.h (CHECK_VALID_EXPR_5, CHECK_VALID_EXPR_6): New.
The next patch in this series will rewrites enum_flags fixing some API
holes. That would cause build failures around code using
type_instance_flags. Or rather, that should be using it, but wasn't.
This patch fixes it by using type_instance_flags throughout instead of
plain integers.
Note that we can't make the seemingly obvious change to struct
type::instance_flags:
- unsigned instance_flags : 9;
+ ENUM_BITFIELD (type_instance_flag_value) instance_flags : 9;
Because G++ complains then that 9 bits isn't sufficient for holding
all values of type_instance_flag_value.
So the patch adds a cast to TYPE_INSTANCE_FLAGS, and adds a separate
SET_TYPE_INSTANCE_FLAGS macro.
gdb/ChangeLog:
* dwarf2/read.c (read_tag_pointer_type): Use type_instance_flags.
* eval.c (fake_method::fake_method): Use SET_TYPE_INSTANCE_FLAGS.
* gdbarch.h, gdbarch.c: Regenerate.
* gdbarch.sh (address_class_type_flags): Use type_instance_flags.
(address_class_name_to_type_flags): Use type_instance_flags and
bool.
* gdbtypes.c (address_space_name_to_int)
(address_space_int_to_name, make_qualified_type): Use
type_instance_flags.
(make_qualified_type): Use type_instance_flags and
SET_TYPE_INSTANCE_FLAGS.
(make_type_with_address_space, make_cv_type, make_vector_type)
(check_typedef): Use type_instance_flags.
(recursive_dump_type): Cast type_instance_flags to unsigned for
printing.
(copy_type_recursive): Use SET_TYPE_INSTANCE_FLAGS.
* gdbtypes.h (TYPE_INSTANCE_FLAGS): Return a type_instance_flags.
(SET_TYPE_INSTANCE_FLAGS): New.
(address_space_name_to_int, address_space_int_to_name)
(make_type_with_address_space): Pass flags using
type_instance_flags instead of int.
* stabsread.c (cleanup_undefined_types_noname): Use
SET_TYPE_INSTANCE_FLAGS.
* type-stack.c (type_stack::follow_types): Use type_instance_flags.
An earlier attempt at doing this had failed (wouldn't work in GCCs
around 4.8, IIRC), but now that I try again, it works. I suspect that
my previous attempt did not use the pre C++14-safe void_t (in
traits.h).
I want to switch to this model because:
- It's the standard detection idiom that folks will learn starting
with C++17.
- In the enum_flags unit tests, I have a static_assert that triggers
a warning (resulting in build error), which GCC does not suppress
because the warning is not being triggered in the SFINAE context.
Switching to the detection idiom fixes that. Alternatively,
switching to the C++03-style expression-validity checking with a
varargs overload would allow addressing that, but I think that
would be going backwards idiomatically speaking.
- While this patch shows a net increase of lines of code, the magic
being added to traits.h can be removed in a few years when we start
requiring C++17.
gdbsupport/ChangeLog:
* traits.h (struct nonesuch, struct detector, detected_or)
(detected_or_t, is_detected, detected_t, detected_or)
(detected_or_t, is_detected_exact, is_detected_convertible): New.
* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.
As reported by Tom here [1], commit 888bdb2b74 ("gdb: change regcache
list to be a map") overlooked an important case, causing a regression.
When registers_changed_ptid is called with a pid-like ptid, it used to
clear all the regcaches for that pid. That commit accidentally removed
that behavior. We need to handle the `ptid.is_pid ()` case in
registers_changed_ptid.
The most trivial way of fixing it would be to iterate on all ptids of a
target and delete the entries where the ptid match the pid. But the
point of that commit was to avoid having to iterate on ptids to
invalidate regcaches, so that would feel like a step backwards.
The only logical solution I see is to add yet another map level, so that
we now have:
target -> (pid -> (ptid -> regcaches))
This patch implements that and adds a test for the case of calling
registers_changed_ptid with a pid-like ptid.
[1] https://sourceware.org/pipermail/gdb-patches/2020-August/171222.html
gdb/ChangeLog:
* regcache.c (pid_ptid_regcache_map): New type.
(target_ptid_regcache_map): Remove.
(target_pid_ptid_regcache_map): New type.
(regcaches): Change type to target_pid_ptid_regcache_map.
(get_thread_arch_aspace_regcache): Update.
(regcache_thread_ptid_changed): Update, handle pid-like ptid
case.
(regcaches_size): Update.
(regcache_count): Update.
(registers_changed_ptid_target_pid_test): New.
(_initialize_regcache): Register new test.
Change-Id: I4c46e26d8225c177dbac9488b549eff4c68fa0d8
The selftest `regcaches` selftest is a bit too broad for my taste,
testing the behavior of get_thread_arch_aspace_regcache and various
cases of registers_changed_ptid. Since I'll want to test even more
scenarios of registers_changed_ptid, passing different sets of
parameters, it will be difficult to do in a single test case. It is
difficult to change something at some point in the test case while make
sure it doesn't compromise what comes after, that we still test the
scenarios that we intended to test. So, split the test case in multiple
smaller ones.
- Split the test case in multiple, where each test case starts from
scratch and tests one specific scenario.
- Introduce the populate_regcaches_for_test function, which is used by
the various test cases to start with a regcache container populated
with a few regcaches for two different targets.
- populate_regcaches_for_test returns a regcache_test_data object, which
contains the test targets that were used to create the regcaches. It
also takes care to call registers_changed at the beginning and end of
the test to ensure the test isn't influenced by existing regcaches,
and cleans up after itself.
- Move the regcache_count lambda function out of
regcache_thread_ptid_changed, so it can be used in
other tests.
- For get_thread_arch_aspace_regcache, test that getting a regcache that
already exists does not increase the count of existing regcaches.
- For registers_changed_ptid, test the three cases we handle today:
(nullptr, minus_one_ptid), (target, minus_one_ptid) and (target,
ptid). The (target, minus_one_ptid) case was not tested prior to this
patch.
gdb/ChangeLog:
* regcache.c (regcache_count): New.
(struct regcache_test_data): New.
(regcache_test_data_up): New.
(populate_regcaches_for_test): New.
(regcaches_test): Remove.
(get_thread_arch_aspace_regcache_test): New.
(registers_changed_ptid_all_test): New.
(registers_changed_ptid_target_test): New.
(registers_changed_ptid_target_ptid_test): New.
(regcache_thread_ptid_changed): Remove regcache_count lambda.
(_initialize_regcache): Register new tests.
Change-Id: Id4280879fb40ff3aeae49b01b95359e1359c3d4b
Do these misc changes to test_get_thread_arch_aspace_regcache:
- Rename to get_thread_arch_aspace_regcache_and_check. The following
patch introduces a selftest for get_thread_arch_aspace_regcache, named
get_thread_arch_aspace_regcache_test. To avoid confusion between the
two functions, rename this one to
get_thread_arch_aspace_regcache_and_check, I think it describes better
what it does.
- Remove gdbarch parameter. We always pass the same gdbarch (the
current inferior's gdbarch), so having a parameter is not useful. It
would be interesting to actually test with multiple gdbarches, to
verify that the regcache container can hold multiple regcaches (with
different architectures) for a same (target, ptid). But it's not the
case as of this patch.
- Verify that the regcache's arch is correctly set.
- Remove the aspace parameter. We always pass NULL here, so it's not
useful to have it as a parameter. Also, instead of passing a NULL
aspace to get_thread_arch_aspace_regcache and verifying that we get a
NULL aspace back, pass the current inferior's aspace (just like we use
the current inferior's gdbarch).
gdb/ChangeLog:
* regcache.c (test_get_thread_arch_aspace_regcache): Rename to...
(get_thread_arch_aspace_regcache_and_check): ... this. Remove
gdbarch and aspace parameter. Use current inferior's aspace.
Validate regcache's arch value.
(regcaches_test): Update.
Change-Id: I8b4c2303b4f91f062269043d1f7abe1650232010
It currently does not work to run the `regcaches` selftest while
debugging something. This is because we expect that there exists no
regcache at the start of the test. If we are debugging something, there
might exist some regcaches.
Fix it by making the test clear regcaches at the start.
While at it, make the test clean up after it self and clear the
regcaches at the end too.
gdb/ChangeLog:
* regcache.c (regcaches_test): Call registers_changed.
Change-Id: I9d4f83ecb0ff9721a71e2c5cbd19e6e6d4e6c30c
PR 26428
bfd * bfd.c (bfd_update_compression_header): Also set the sh_addralign
field in the ELF header of the compressed sections.
ld * testsuite/ld-elf/zlibbegin.rS: Update expected output.
* testsuite/ld-elf/zlibnormal.rS: Likewise.
PR 26406
* elf-bfd.h (struct bfd_elf_section_data): Add
has_secondary_relocs field.
* elf.c (_bfd_elf_copy_special_section_fields): Set the
has_secondary_relocs field for sections which have associated
secondary relocs.
* elfcode.h (elf_write_relocs): Only call write_secondary_relocs
on sections which have associated secondary relocs.
Currently, GDB is not able to set a breakpoint at subprogram post
prologue for flang generated binaries. This is due to clang having
two line notes one before and another after the prologue.
Now the end of prologue is determined using symbol table, which was
the way for clang generated binaries already. Since clang and flang
both share same back-end it is true for flang as well.
gdb/ChangeLog
* amd64-tdep.c (amd64_skip_prologue): Using symbol table
to find the end of prologue for flang compiled binaries.
* arm-tdep.c (arm_skip_prologue): Likewise.
* i386-tdep.c (i386_skip_prologue): Likewise.
* producer.c (producer_is_llvm): New function.
(producer_parsing_tests): Added new tests for clang/flang.
* producer.h (producer_is_llvm): New declaration.
gdb/testsuite/ChangeLog
* gdb.fortran/vla-type.exp: Skip commands not required for
the Flang compiled binaries after prologue fix.
* ecoff.c (INIT_VARRAY): Use htab_t.
(add_string): Likewise.
(ecoff_read_begin_hook): Use new str_htab_create.
(get_tag): Use htab_t.
(add_file): Likewise.
The first of a patch series deleting the gas/hash.c hash table
implementation and instead using libiberty/hashtab.c hash tables in
gas.
* as.h: Include hashtab.h.
* hash.c (htab_insert): New.
(htab_print_statistics): Likewise.
* hash.h (htab_insert): Likewise.
(htab_print_statistics): Likewise.
The 't' length modifier isn't in SUSv2, unsurprisingly %tx isn't
recognized by older printf implementations. So even though 't' is
correct for ptrdiff_t we can't use it. Also, _bfd_int64_high and
_bfd_int64_low disappeared in 2008.
PR 26349
* readelf.c (dump_relocations): Use BFD_VMA_FMT to print offset
and info fields.
(dump_section_as_strings): Don't use %tx to display offset.
The debug prints inside linux-nat.c almost all have a prefix that
indicate in which function they are located. This prefix is an
abbreviation of the function name. For example, this print is in the
`linux_nat_post_attach_wait` function:
if (debug_linux_nat)
fprintf_unfiltered (gdb_stdlog,
"LNPAW: Attaching to a stopped process\n");
Over time, the code has changed, things were moved, and many of these
prefixes are not accurate anymore. Also, unless you know the
linux-nat.c file by heart, it's a bit cryptic what LLR, LNW, RSRL, etc,
all mean.
To address both of these issues, I suggest adding this macro for
printing debug statements, which automatically includes the function
name. It also includes the `[linux-nat]` prefix to clarify which part
of GDB printed this (I think that, ideally, all debug prints would
include such a tag).
The `__func__` magic symbol is used to get the function name.
Unfortunately, in the case of methods, it only contains the method name,
not the class name. So we'll get "wait", where I would have liked to
get "linux_nat_target::wait". But at least with the `[linux-nat]` tag
in the front, it's not really ambiguous.
I've made the macro automatically include the trailing newline, because
it wouldn't make sense to call it twice to print two parts of one line,
the `[linux-nat]` tag would be printed in the middle.
An advantage of this (IMO) is that it's less verbose, we don't have to
check for `if (debug_linux_nat)` everywhere.
Another advantage is that it's easier to customize the output later,
without having to touch all call sites.
Here's an example of what it looks like in the end:
[linux-nat] linux_nat_wait_1: enter
[linux-nat] wait: [process -1], [TARGET_WNOHANG]
gdb/ChangeLog:
* linux-nat.c (linux_nat_debug_printf): New function.
(linux_nat_debug_printf_1): New macro. Use throughout the file.
Change-Id: Ifcea3255b91400d3ad093cd0b75d3fac241cb998
These shouldn't be optional. The record form of vector instructions
set CR6, giving an expectation that omitting BF should be the same as
specifying CR6.
opcodes/
* ppc-opc.c (powerpc_opcodes): Replace OBF with BF for vcmpsq,
vcmpuq and xvtlsbb.
gas/
* testsuite/gas/ppc/int128.s: Correct vcmpuq.
* testsuite/gas/ppc/int128.d: Update.
* testsuite/gas/ppc/xvtlsbb.d: Update.
Introduce Makefile variables DEBUGINFOD_CFLAGS and DEBUGINFOD_LIBS
that map to the configuration variables of the same names.
Replace @DEBUGINFOD_LIBS@ with $(DEBUGINFOD_LIBS) in the definition
of CLIBS in order to conform to the usage of other *_LIBS variables
in Makefile.in.
Add DEBUGINFOD_CFLAGS to INTERNAL_CFLAGS_BASE. This fixes an issue
where GDB would fail to find debuginfod.h if it was not installed
in a default location searched by the compiler.
gdb/ChangeLog:
* Makefile.in (DEBUGINFOD_CFLAGS, DEBUGINFOD_LIBS): New variables.
(INTERNAL_CFLAGS_BASE): Add DEBUGINFOD_CFLAGS.
(CLIBS): Add DEBUGINFOD_LIBS.
The xvcvbf16sp mnemonic has been renamed to xvcvbf16spn, to be consistent
with the other non-signaling conversion instructions which all end with "n".
opcodes/
* ppc-opc.c (powerpc_opcodes) <xvcvbf16sp>: Rename from this...
<xvcvbf16spn>: ...to this.
gas/
* testsuite/gas/ppc/vsx4.s: Update test to use new mnemonic.
* testsuite/gas/ppc/vsx4.d: Likewise.
On IA64 built failed as:
```
ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope
352 | if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))
| ^~~~~~~~~~~~~~~~
```
The fix includes "gdbarch.h" header where symbol is declared.
* ia64-linux-nat.c: Include "gdbarch.h" to declare used
'gdbarch_num_regs'.
Signed-off-by: Sergei Trofimovich <siarheit@google.com>
In PR rust/26197, Tom de Vries notes that the variant part rewrite
caused some regressions for the Rust compiler he has. This compiler
is unusual in that it combines a relatively recent rustc with a
relatively old LLVM -- so variant parts are not emitted using DWARF.
Most of the bugs in that PR were already fixed by earlier patches, but
some lingered. After some research we found that some of these never
did work -- which is consistent with the investigations we did into
the debug info -- but instead were xfail'd. This patch updates the
xfails to cope with the new output. (After this, just one failure
remains.)
Tom de Vries tested this using his rustc and suggested a fix that
appears in this version.
gdb/testsuite/ChangeLog
2020-08-17 Tom de Vries <tdevries@suse.de>
Tom Tromey <tromey@adacore.com>
PR rust/26197:
* gdb.rust/simple.exp (xfail_pattern): Update for new failure.
I happened to notice that using -var-create at a certain spot in an
Ada program caused a crash. This happens because
ada_get_decoded_value can return NULL -- in particular, deeper in the
code it can hit this return in ada_type_of_array:
descriptor = desc_bounds (arr);
if (value_as_long (descriptor) == 0)
return NULL;
This patch avoids the crash by handling this NULL return.
gdb/ChangeLog
2020-08-17 Tom Tromey <tromey@adacore.com>
* ada-varobj.c (ada_varobj_decode_var): Handle case where
ada_get_decoded_value returns NULL.
gdb/testsuite/ChangeLog
2020-08-17 Tom Tromey <tromey@adacore.com>
* gdb.ada/mi_var_access.exp: New file.
* gdb.ada/mi_var_access/mi_access.adb: New file.
* gdb.ada/mi_var_access/pck.adb: New file.
* gdb.ada/mi_var_access/pck.ads: New file.
An internal test failed on a riscv64-elf cross build because
Inferior.search_memory returned a negative value. I tracked this down
to to use of PyLong_FromLong in infpy_search_memory. Then, I looked
at other conversions of CORE_ADDR to Python and fixed these as well.
I don't think there is a good way to write a test for this.
gdb/ChangeLog
2020-08-17 Tom Tromey <tromey@adacore.com>
* python/py-inferior.c (infpy_search_memory): Use
gdb_py_object_from_ulongest.
* python/py-infevents.c (create_inferior_call_event_object)
(create_memory_changed_event_object): Use
gdb_py_object_from_ulongest.
* python/py-linetable.c (ltpy_entry_get_pc): Use
gdb_py_object_from_ulongest.
This variable is really a boolean, so use the bool type.
gdb/ChangeLog:
* dwarf2/loc.c (dwarf2_loc_desc_get_symbol_read_needs): Use
bool.
Change-Id: I814a47d1200f3b88722c54c822fd49607a6b77be
This patch fixes an internal error in GAS when defining a section using
a symbol that has already been named but not defined. For a minimal
reproducer, try the following input:
a=b
.sect a
The problem is that obj_elf_change_section() happily reuses the symbol
"a" created by equals() without clearing the sy_value field: prior to
this patch, it just set bsym. This caused a problem when attempting to
resolve the section symbol, since resolve_symbol_value() ended up
resolving the symbol as if it were the original symbol created by
equals(), which ends up leaving the section symbol in the undefined
section instead of in section a, hence the call to abort() in
S_SET_SEGMENT().
gas/ChangeLog:
* config/obj-elf.c (obj_elf_change_section): When repurposing an
existing symbol, ensure that we set sy_value as per other (fresh)
section symbols.
* testsuite/gas/elf/elf.exp: Add new test.
* testsuite/gas/elf/section-symbol-redef.d: New test.
* testsuite/gas/elf/section-symbol-redef.s: Input for test.
When debugging gdb in batch mode with executable mixed-lang-stack and doing a
backtrace at breakpt:
...
$ gdb --args gdb \
-batch \
outputs/gdb.fortran/mixed-lang-stack/mixed-lang-stack \
-ex "b breakpt" \
-ex r \
-ex bt
...
and stopping at resolve_dynamic_type to print the type:
...
(gdb) b resolve_dynamic_type
Breakpoint 1 at 0x6b020c: file gdbtypes.c, line 2633.
(gdb) commands
Type commands for breakpoint(s) 1, one per line.
End with a line saying just "end".
>call recursive_dump_type (type, 0)
>continue
>end
(gdb) run
...
we eventually run into an assert for the dynamic type of "str":
...
Thread 1 "gdb" hit Breakpoint 1, resolve_dynamic_type (type=0x22204f0, \
valaddr=..., addr=4199408) at gdbtypes.c:2633
2633 = {check_typedef (type), valaddr, addr, NULL};
type node 0x22204f0
name '<NULL>' (0x0)
code 0xd (TYPE_CODE_STRING)
length 0
...
nfields 0 0x22204b0
gdbtypes.h:526: internal-error: LONGEST dynamic_prop::const_val() const: \
Assertion `m_kind == PROP_CONST' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
...
when trying to print the high bound of a TYPE_CODE_RANGE, which has m_kind
PROP_LOCEXPR, while the code in resolve_dynamic_type assumes PROP_CONST.
Fix this by extending the printing of TYPE_CODE_RANGE to allow
PROP_LOCEXPR/PROP_LOCLIST as well, such that we have instead:
...
nfields 0 0x1fbc020
low 1 high (dynamic)
...
Tested on x86_64-linux.
gdb/ChangeLog:
2020-08-17 Tom de Vries <tdevries@suse.de>
PR gdb/26393
* gdbtypes.c (dump_dynamic_prop): New function.
(recursive_dump_type): Use dump_dynamic_prop for TYPE_CODE_RANGE.
In PR25350, an internal error was reported:
...
(gdb) break *eh2+0x7e
Breakpoint 1 at 0x13e2: file small.c, line 38.
(gdb) run
Starting program: a.out
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
Breakpoint 1, 0x00005555555553e2 in eh2 (
frame.c:558: internal-error: frame_id get_frame_id(frame_info*): \
Assertion `stashed' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
...
The internal error does not reproduce after recent commit 547ce8f00b
"[gdb/backtrace] Fix printing of fortran string args".
Add the corresponding test-case as regression test, given that the code is
rather atypical.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-08-16 Tom de Vries <tdevries@suse.de>
PR gdb/25350
* gdb.base/eh_return.c: New test.
* gdb.base/eh_return.exp: New file.
In gdb.fortran/mixed-lang-stack.f90, we have fortran function mixed_func_1d:
...
subroutine mixed_func_1d(a, b, c, d, str)
use, intrinsic :: iso_c_binding, only: c_int, c_float, c_double
use, intrinsic :: iso_c_binding, only: c_float_complex
implicit none
integer(c_int) :: a
real(c_float) :: b
real(c_double) :: c
complex(c_float_complex) :: d
character(len=*) :: str
...
which we declare in C in gdb.fortran/mixed-lang-stack.c like this:
...
extern void mixed_func_1d_ (int *, float *, double *, complex float *,
char *, size_t);
...
The fortran string parameter str is passed as a char *, and an additional
argument str_ for the string length. The type used for the string length
argument is size_t, but for gcc 7 and earlier, the actual type is int
instead ( see
https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html ).
Fix this by declaring the string length type depending on the gcc version:
...
#if !defined (__GNUC__) || __GNUC__ > 7
typedef size_t fortran_charlen_t;
#else
typedef int fortran_charlen_t;
...
Tested on x86_64-linux, with gcc-7 and gcc-8.
gdb/testsuite/ChangeLog:
2020-08-15 Tom de Vries <tdevries@suse.de>
* gdb.fortran/mixed-lang-stack.c (fortran_charlen_t): New type.
(mixed_func_1d_): Use fortran_charlen_t in decl.
When running test-case gdb.fortran/mixed-lang-stack.exp, it passes, but we
find in gdb.log:
...
(gdb) bt^M
...
#7 0x000000000040113c in mixed_func_1b (a=1, b=2, c=3, d=(4,5), \
e=<error reading variable: value requires 140737488341744 bytes, which \
is more than max-value-size>, g=..., _e=6) at mixed-lang-stack.f90:87^M
...
while a bit later in gdb.log, we have instead for the same frame (after
adding a gdb_test_no_output "set print frame-arguments all" to prevent
getting "e=..."):
...
(gdb) up^M
#7 0x000000000040113c in mixed_func_1b (a=1, b=2, c=3, d=(4,5), \
e='abcdef', g=( a = 1.5, b = 2.5 ), _e=6) at mixed-lang-stack.f90:87^M
...
The difference is that in the latter case, we print the frame while it's
selected, while in the former, it's not.
The problem is that while trying to resolve the dynamic type of e in
resolve_dynamic_type, we call dwarf2_evaluate_property with a frame == NULL
argument, and then use the selected frame as the context in which to evaluate
the dwarf property, effectively evaluating a DW_OP_fbreg operation in the
wrong frame context.
Fix this by temporarily selecting the frame of which we're trying to print the
arguments in print_frame_args, borrowing code from print_frame_local_vars that
was added to fix a similar issue in commit 16c3b12f19 "error/internal-error
printing local variable during "bt full".
Build and tested on x86_64-linux.
gdb/ChangeLog:
2020-08-15 Tom de Vries <tdevries@suse.de>
PR backtrace/26390
* stack.c (print_frame_args): Temporarily set the selected
frame to FRAME while printing the frame's arguments.
gdb/testsuite/ChangeLog:
2020-08-15 Tom de Vries <tdevries@suse.de>
PR backtrace/26390
* gdb.fortran/mixed-lang-stack.exp: Call bt with -frame-arguments all.
Update expected pattern.
git commit 49d9fd42ac chose to make nm print 'C' for the normal
common section, and 'c' for other commons. This was an attempt to
make common symbols in .scommon and other small common sections show
a 'c' type without a section name comparison, but it failed for
nm --plugin on lto objects where normal common symbols are stashed in
a "plug" section. It's also wrong for large common symbols. So
instead set SEC_SMALL_DATA on sections created for small commons, and
key off that flag to show 'c' type. If your ELF target doesn't have
an elf_backend_symbol_processing function, then you won't see 'c' for
symbols in .scommon.
Note that due to bfd_decode_symclass decoding common symbols without
a chance for coff_section_type to treat .scommon specially, then
having .scommon in the array of special sections handled by
coff_section_type prior to 49d9fd42ac was entirely ineffective.
That fact escaped me when writing 49d9fd42ac. Unless .scommon
didn't have SEC_IS_COMMON set, which would be a little weird.
PR 26389
* syms.c (bfd_decode_symclass): Choose 'c' for commons only when
SEC_SMALL_DATA.
* elf32-m32r.c (_bfd_m32r_elf_symbol_processing): Set SEC_SMALL_DATA
on small common section.
* elf32-score.c (s3_bfd_score_elf_symbol_processing): Likewise.
* elf32-score7.c (s7_bfd_score_elf_symbol_processing): Likewise.
* elf32-tic6x.c (elf32_tic6x_symbol_processing): Likewise.
* elf32-v850.c (v850_elf_symbol_processing): Likewise.
* elfxx-mips.c (_bfd_mips_elf_symbol_processing): Likewise.
* ecoff.c (ecoff_set_symbol_info, ecoff_link_add_externals): Likewise.
This patches changes low_prepare_to_resume in the ppc linux native target
to always clear the watchpoint when the old PTRACE_SET_DEBUGREG interface
is used, even if another watchpoint GDB requested to the target is
written right after using the same call.
The reason for this is that there were some older kernel versions for
which overwriting a watchpoint with PTRACE_SET_DEBUGREG would not
re-activate the watchpoint if it was previouly disabled following a hit.
This happened when the kernel was configured with CONFIG_HW_BREAKPOINT on
and uses perf events to install watchpoints.
Previously, the ppc linux native target would immediately remove or
insert watchpoints following a request from the upper layers. This was
changed in commit 227c0bf4b3 to fix other
issues, which caused watchpoint requests to be applied to the inferior
only in low_prepare_to_resume, right before the inferior is resumed.
Usually, but maybe not always, after a hit, GDB will remove the
watchpoint, resume the inferior for a single-step, possibly report the
watchpoint hit to the user, and then re-insert the watchpoint before the
inferior is next resumed. In this case there would be no problems, but
since I can't guarantee that there aren't other paths in GDB that allow
the user to set a new watchpoint after the first one hit, and after its
deletion by GDB, but before the inferior is resumed, there is a chance
that PTRACE_SET_DEBUGREG could be called directly without the watchpoint
first having been cleared, which could cause a false negative with the
older kernel versions.
This issue would affect kernel versions starting from this commit:
5aae8a53708025d4e718f0d2e7c2f766779ddc71
Up to the fix in this commit:
a53fd61ac2f411745471c1c877d5e072fbbf0e5c
gdb/ChangeLog:
PR breakpoints/26385
* ppc-linux-nat.c (ppc_linux_nat_target::low_prepare_to_resume):
Always clear watchpoint with PTRACE_SET_DEBUGREG.
In commit 227c0bf4b3, which fixed some
watchpoint bugs, I compared the return value of some ptrace calls with ==
-1 and != -1. Althought this should be correct, since the rest of the
file uses < 0 and >= 0, I have modified this for consistency.
gdb/ChangeLog:
* ppc-linux-nat.c (ppc_linux_dreg_interface::detect)
(ppc_linux_nat_target::low_prepare_to_resume): Use ptrace () < 0
and >= to check return value instead of == -1 and != -1.
Introduce the gdb_argv::as_array_view method, as a way to easily pass
the parsed arguments array to a function taking an array view. There is
currently one caller where we can use this (which prompted the
suggestion to implement this method).
Add some selftests for the new method, which at the same time test a
little bit gdb_argv. As far as I know, it's not tested currently.
gdb/ChangeLog:
* utils.h (class gdb_argv) <as_array_view>: New method.
* utils.c (gdb_argv_as_array_view_test): New.
(_initialize_utils): Register selftest.
* maint.c (maintenance_selftest): Use the new method.
Change-Id: I0645037613ed6549aabe60f14a36f3494513b177
