Even after the previous patches reworking the inheritance of several
breakpoint types, the present breakpoint hierarchy looks a bit
surprising, as we have "breakpoint" as the superclass, and then
"base_breakpoint" inherits from "breakpoint". Like so, simplified:
breakpoint
base_breakpoint
ordinary_breakpoint
internal_breakpoint
momentary_breakpoint
ada_catchpoint
exception_catchpoint
tracepoint
watchpoint
catchpoint
exec_catchpoint
...
The surprising part to me is having "base_breakpoint" being a subclass
of "breakpoint". I'm just refering to naming here -- I mean, you'd
expect that it would be the top level baseclass that would be called
"base".
Just flipping the names of breakpoint and base_breakpoint around
wouldn't be super great for us, IMO, given we think of every type of
*point as a breakpoint at the user visible level. E.g., "info
breakpoints" shows watchpoints, tracepoints, etc. So it makes to call
the top level class breakpoint.
Instead, I propose renaming base_breakpoint to code_breakpoint. The
previous patches made sure that all code breakpoints inherit from
base_breakpoint, so it's fitting. Also, "code breakpoint" contrasts
nicely with a watchpoint also being typically known as a "data
breakpoint".
After this commit, the resulting hierarchy looks like:
breakpoint
code_breakpoint
ordinary_breakpoint
internal_breakpoint
momentary_breakpoint
ada_catchpoint
exception_catchpoint
tracepoint
watchpoint
catchpoint
exec_catchpoint
...
... which makes a lot more sense to me.
I've left this patch as last in the series in case people want to
bikeshed on the naming.
"code" has a nice property that it's exactly as many letters as
"base", so this patch didn't require any reindentation. :-)
Change-Id: Id8dc06683a69fad80d88e674f65e826d6a4e3f66
To look for code paths that lead to create_breakpoints_sal creating
multiple breakpoints, I ran the whole testsuite with this hack:
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8377,8 +8377,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
int from_tty,
int enabled, int internal, unsigned flags)
{
- if (canonical->pre_expanded)
- gdb_assert (canonical->lsals.size () == 1);
+ gdb_assert (canonical->lsals.size () == 1);
surprisingly, the assert never failed...
The way to get to create_breakpoints_sal with multiple lsals is to use
"set multiple-symbols ask" and then select multiple options from the
menu, like so:
(gdb) set multiple-symbols ask
(gdb) b overload1arg
[0] cancel
[1] all
[2] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg()
[3] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(char)
[4] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(double)
[5] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(float)
[6] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(int)
[7] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(long)
[8] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(short)
[9] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(signed char)
[10] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(unsigned char)
[11] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(unsigned int)
[12] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(unsigned long)
[13] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(unsigned short)
> 2-3
Breakpoint 2 at 0x1532: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc, line 107.
Breakpoint 3 at 0x154b: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc, line 110.
warning: Multiple breakpoints were set.
Use the "delete" command to delete unwanted breakpoints.
... which would trigger the assert.
This commit makes gdb.cp/ovldbreak.exp test this scenario. It does
that by making set_bp_overloaded take a list of expected created
breakpoints rather than just one breakpoint. It converts the
procedure to use gdb_test_multiple instead of send_gdb/gdb_expect
along the way.
Change-Id: Id87d1e08feb6670440d926f5344e5081f5e37c8e
This adds a new ctor to momentary_breakpoints with a few parameters
that are always necessary for momentary breakpoints.
In particular, I noticed that set_std_terminate_breakpoint doesn't
make the breakpoint be thread specific, which looks like a bug to me.
The point of that breakpoint is to intercept std::terminate calls that
happen as result of the called thread throwing an exception that won't
be caught by the dummy frame. If some other thread calls
std::terminate, IMO, it's no different from some other thread calling
exit/_exit, for example.
Change-Id: Ifc5ff4a6d6e58b8c4854d00b86725382d38a1a02
Momentary breakpoints have no breakpoint number, their breakpoint
number should be always 0, to avoid constantly incrementing (or
decrementing) the internal breakpoint count.
Indeed, set_momentary_breakpoint installs the created breakpoint
without a number.
However, momentary_breakpoint_from_master incorrectly gives an
internal breakpoint number to the new breakpoint. This commit fixes
that.
Change-Id: Iedcae5432cdf232db9e9a6e1a646d358abd34f95
This tweaks the intro comments of the following classes:
internal_breakpoint
momentary_breakpoint
breakpoint
base_breakpoint
watchpoint
catchpoint
Change-Id: If6b31f51ebbb81705fbe5b8435f60ab2c88a98c8
After the previous patches, only base_breakpoint subclasses use
add_location(sal), so we can move it to base_breakpoint (a.k.a. base
class for code breakpoints).
This requires a few casts here and there, but always at spots where
you can see from context what the breakpoint's type actually is.
I inlined new_single_step_breakpoint into its only caller exactly for
this reason.
I did try to propagate more use of base_breakpoint to avoid casts, but
that turned out unwieldy for this patch.
Change-Id: I49d959322b0fdce5a88a216bb44730fc5dd7c6f8
Move common bits of catchpoint and exception_catchpoint to
breakpoint's ctor, to avoid duplicating code.
Change-Id: I3a115180f4d496426522f1d89a3875026aea3cf2
struct catchpoint's ctor currently calls init_raw_breakpoint, which is
a bit weird, as that ctor-like function takes a sal argument, but
catchpoints don't have code locations.
Instead, make struct catchpoint's ctor add the catchpoint's dummy
location using add_dummy_location.
init_raw_breakpoint uses add_location under the hood, and with a dummy
sal it would ultimately use the breakpoint's gdbarch for the
location's gdbarch, so replace the references to loc->gdbarch (which
is now NULL) in syscall_catchpoint to references to the catchpoint's
gdbarch.
struct catchpoint's ctor was the last user of init_raw_breakpoint, so
this commit eliminates the latter.
Since catchpoint locations aren't code locations, make struct
catchpoint inherit struct breakpoint instead of base_breakpoint. This
let's us delete the tracepoint::re_set override too.
Change-Id: Ib428bf71efb09fdaf399c56e4372b0f41d9c5869
Software watchpoints allocate a special dummy location using
software_watchpoint_add_no_memory_location, and then
breakpoint_address_bits checks whether the location is that special
location to decide whether the location has a meaninful address to
print.
Introduce a new bp_loc_software_watchpoint location kind, and make
breakpoint_address_bits use bl_address_is_meaningful instead, which
returns false for bp_loc_other, which is in accordance with we
document for bp_location::address:
/* (... snip ...) Valid for all types except
bp_loc_other. */
CORE_ADDR address = 0;
Rename software_watchpoint_add_no_memory_location to
add_dummy_location, and simplify it. This will be used by catchpoints
too in a following patch.
Note that neither "info breakpoints" nor "maint info breakpoints"
actually prints the addresses of watchpoints, but I think it would be
useful to do so in "maint info breakpoints". This approach let's us
implement that in the future.
Change-Id: I50e398f66ef618c31ffa662da755eaba6295aed7
exception_catchpoint is really a code breakpoint, with locations set
by sals, re-set like other code breakpoints, etc., so make it inherit
base_breakpoint.
This adds a bit of duplicated code to exception_catchpoint's ctor
(copied from struct catchpoint's ctor), but it will be eliminated in a
following patch.
Change-Id: I9fbb2927491120e9744a4f5e5cb5e6870ca07009
This commit makes set_momentary_breakpoint allocate the breakpoint
type without relying on set_raw_breakpoint, and similarly,
momentary_breakpoint_from_master not rely on
set_raw_breakpoint_without_location. This will let us convert
init_raw_breakpoint to a ctor in a following patch.
The comment about set_raw_breakpoint being used in gdbtk sources is
stale. gdbtk no longer uses it.
Change-Id: Ibbf77731e4b22e18ccebc1b5799bbec0aff28c8a
This moves initialization of internal_breakpoint's breakpoint fields
to internal_breakpoint's ctor, and stops using
new_breakpoint_from_type for internal_breakpoint breakpoints.
Change-Id: I898ed0565f47cb00e4429f1c6446e6f9a385a78d
Currently, init_ada_exception_catchpoint is defined in breakpoint.c, I
presume so it can call the static describe_other_breakpoints function.
I think this is a dependency inversion.
init_ada_exception_catchpoint, being code specific to Ada catchpoints,
should be in ada-lang.c, and describe_other_breakpoints, a core
function, should be exported.
And then, we can convert init_ada_exception_catchpoint to an
ada_catchpoint ctor.
Change-Id: I07695572dabc5a75d3d3740fd9b95db1529406a1
This commit changes ada_catchpoint_location's ctor from:
ada_catchpoint_location (breakpoint *owner)
to:
ada_catchpoint_location (ada_catchpoint *owner)
just to make the code better document intention.
To do this, we need to move the ada_catchpoint_location type's
definition to after ada_catchpoint is defined, otherwise the compiler
doesn't know that ada_catchpoint is convertible to struct breakpoint.
Change-Id: Id908b2e38bde30b262381e00c5637adb9bf0129d
This converts init_breakpoint_sal to a base_breakpoint constructor.
It removes a use of init_raw_breakpoint.
To avoid manually adding a bunch of parameters to
new_breakpoint_from_type, and manually passing them down to the
constructors of a number of different base_breakpoint subclasses, make
new_breakpoint_from_type a variable template function.
Change-Id: I4cc24133ac4c292f547289ec782fc78e5bbe2510
None of init_breakpoint_sal, create_breakpoint_sal, and
strace_marker_create_breakpoints_sal make use of their "internal"
parameter, so remove it.
Change-Id: I943f3bb44717ade7a7b7547edf8f3ff3c37da435
This makes tracepoints inherit from base_breakpoint, since their
locations are code locations. If we do that, then we can eliminate
tracepoint::re_set and tracepoint::decode_location, as they are doing
the same as the base_breakpoint implementations.
With this, all breakpoint types created by new_breakpoint_from_type
are code breakpoints, i.e., base_breakpoint subclasses, and thus we
can make it return a base_breakpoint pointer.
Finally, init_breakpoint_sal can take a base_breakpoint pointer as
"self" pointer too. This will let us convert this function to a
base_breakpoint ctor in a following patch.
Change-Id: I3a4073ff1a4c865f525588095c18dc42b744cb54
This commit replaces a chunk of code in break_range_command by an
equivalent call to install_breakpoint.
Change-Id: I31c06cabd36f5be91740aab029265f678aa78e35
ranged_breakpoint's ctor already sets the breakpoint's type to
bp_hardware_breakpoint.
Since this is a "regular" breakpoint, b->pspace should remain NULL.
Thus, the only thing init_raw_breakpoint is needed for, is to add the
breakpoint's location. Do that directly.
Change-Id: I1505de94c3919881c2b300437e2c0da9b05f76bd
Make add_location_to_breakpoint be a method of struct breakpoint.
A patch later in the series will move this to base_breakpoint, but for
now, it needs to be here.
Change-Id: I5bdc2ec1a7c2d66f26f51bf6f6adc8384a90b129
The .quad statement stores the 64-bit hex value in Endian order. When used
to store a 64-bit prefix instructions on Big Endian (BE) systems, the .quad
statement stores the 32-bit suffix followed by the 32-bit prefix rather
than the expected order of prefix word followed by the suffix word. GDB
fetches 32-bits at a time when disassembling instructions. The disassembly
on BE gets messed up since GDB fetches the suffix first and interprets it
as a word instruction not a prefixed instruction. When gdb fetches the
prefix part of the instruction, following the initial suffix word, gdb
associates the prefix word incorrectly with the following 32-bits as the
suffix for the instruction when in fact it is the following instruction.
For example on BE we have two prefixed instructions stored using the
.quad statement as follows:
addr word GDB action
---------------------------------------------
1 suffix inst A <- GDB interprets as a word instruction
2 prefix inst A <- GDB uses this prefix with
3 suffix inst B <- this suffix rather than the suffix at addr 1.
4 prefix inst B
This patch changes the .quad statement into two .longs to explicitly store
the prefix followed by the suffix of the instruction.
The patch rearranges the instructions to put all of the word instructions
together followed by the prefix instructions for clarity.
The patch has been tested on Power 10 and Power 7 BE and LE to verify
the change works as expected.
The documentation says that -enable-pretty-printing is experimental in
7.0 and may change -- that's long enough ago that I think we can say
that this text is no longer correct or useful.
* dwarf.c (dwarf_select_sections_by_names): Return zero if no
sections were selected.
(dwarf_select_sections_by_letters): Likewise.
* dwarf.h: (dwarf_select_sections_by_names): Update prototype.
(dwarf_select_sections_by_letters): Update prototype.
* objdump.c (might_need_separate_debug_info): New function.
(dump_bfd): Call new function before attempting to load separate
debug info files.
(main): Do not enable dwarf section dumping for -WK or -WN.
* readelf.c (parse_args): Do not enable dwarf section dumping for
-wK or -wN.
(might_need_separate_debug_info): New function.
(process_object): Call new function before attempting to load
separate debug info files.
* testsuite/binutils-all/debuginfo.exp: Expect -WE and -wE
debuginfod tests to pass.
* testsuite/binutils-all/objdump.Wk: Add extra regexps.
* testsuite/binutils-all/readelf.k: Add extra regexps.
As fmv.x.q and fmv.q.x instructions are RV128-only (not RV64-only),
it should be removed until RV128 support for GNU Binutils is required
again.
gas/ChangeLog:
* testsuite/gas/riscv/fmv.x.q-rv64-fail.d: New failure test.
* testsuite/gas/riscv/fmv.x.q-rv64-fail.l: Likewise.
* testsuite/gas/riscv/fmv.x.q-rv64-fail.s: Likewise.
include/ChangeLog:
* opcode/riscv-opc.h (MATCH_FMV_X_Q, MASK_FMV_X_Q,
MATCH_FMV_Q_X, MASK_FMV_Q_X): Remove RV128-only instructions.
opcodes/ChangeLog:
* riscv-opc.c (riscv_opcodes): Remove RV128-only instructions.
In aix-thread.c we use ms->value_address () to get the symbol address.
This triggers the following compiler error...
base operand of '->' has non-pointer type 'bound_minimal_symbol'
... because ms is not a pointer.
This commit fixes this error by using ms.value_address () instead.
When using perf to profile large binaries, _bfd_dwarf2_find_nearest_line()
becomes a hotspot, as perf wants to get line number information
(for inline-detection purposes) for each and every sample. In Chromium
in particular (the content_shell binary), this entails going through
475k address ranges, which takes a long time when done repeatedly.
Add a radix-256 trie over the address space to quickly map address to
compilation unit spaces; for content_shell, which is 1.6 GB when some
(but not full) debug information turned is on, we go from 6 ms to
0.006 ms (6 µs) for each lookup from address to compilation unit, a 1000x
speedup.
There is a modest RAM increase of 180 MB in this binary (the existing
linked list over ranges uses about 10 MB, and the entire perf job uses
between 2–3 GB for a medium-size profile); for smaller binaries with few
ranges, there should be hardly any extra RAM usage at all.
Make ld and bfd values consistent by swapping values 0 and 2 in
link_info.warn_execstack. This has the benefit of making the value an
"extended" boolean, with 0 meaning no warning, 1 meaning warn, other
values a conditional warning.
Yes, this patch introduces fails on arm/aarch64. Not a problem with
this patch but an arm/aarch64 before_parse problem.
bfd/
* elflink.c (bfd_elf_size_dynamic_sections): Adjust
warn_execstack test.
include/
* bfdlink.h (warn_execstack): Swap 0 and 2 meaning.
ld/
* configure.ac (DEFAULT_LD_WARN_EXECSTACK): Use values of 0,
1, 2 consistent with link_info.warn_execstack.
* ld.texi: Typo fixes.
* lexsup.c (parse_args): Adjust setting of link_info.warn_execstack.
(elf_static_list_options): Adjust help message conditions.
* configure: Regenerate.
The current assembler accepts system registers FPCXTNS and FPCXTS for Armv8.1-M
Mainline Instructions VSTR, VLDR, VMRS and VMSR.
Assembler should be also allowing FPCXT_NS, fpcxt_ns, fpcxtns, FPCXT_S, fpcxt_s
and fpcxts. This patch fixes the issue.
The 'info pretty-printers' example is pretty long and consists of many
commands and their output.
Currently, when the pdf manual is generated this example spans a
page-break, with the page-break falling part way through some example
output from GDB.
This commit breaks up the example using @group .... @end group, within
each group is a single GDB command and all its output.
Now, when the pdf manual is created, the page-break is placed after
the output of one GDB command, and before the subsequent command, this
looks much nicer.
The example for 'info pretty-printer' in the manual passes an
object-regexp in some cases, but presents output as though no
object-regexp was passed.
This commit fixes the two mistakes, in one case, fixing the output to
filter based on object-regexp, and in the other, to remove the
object-regexp from the command and leave all the output.
When building GDB with -std=c++17 and -D_GLIBCXX_DEBUG=1, I get:
$ ./gdb -nx --data-directory=data-directory -q -ex "maint selftest path_join"
/usr/include/c++/11.2.0/string_view:233: constexpr const value_type& std::basic_string_view<_CharT, _Traits>::operator[](std::basic_string_view<_CharT, _Traits>::size_type) const [with _CharT = char; _Traits = std::char_traits<char>; std::basic_string_view<_CharT, _Traits>::const_reference = const char&; std::basic_string_view<_CharT, _Traits>::size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed.
The problem is that we're passing an empty string_view to
IS_ABSOLUTE_PATH. IS_ABSOLUTE_PATH accesses [0] on that string_view,
which is out-of-bounds.
The reason this is not seen with -std less than c++17 is that our local
copy of string_view (used with C++ < 17) does not have the assert in
operator[], as that wouldn't work in a constexpr method:
5890af36e5/gdbsupport/gdb_string_view.h (L180)
IS_ABSOLUTE_PATH is normally used with null-terminated string. It's
fine to pass an empty null-terminated string to IS_ABSOLUTE_PATH,
because index 0 in such a string is valid. But not with an empty
string_view.
Fix that by avoiding the "call" to IS_ABSOLUTE_PATH if the string_view
is empty.
Change-Id: Idf4df961b63f513b3389235e93814c02b89ea32e
Like the placeholder types added in 04dfe7aa52 ("Arm64: follow-on to
PR gas/27217 fix"), these are also placeholders which are subsequently
resolved (albeit later, hence this being a separate issue). As for the
resolved types 1 is returned, these pseudo-relocs should also have 1
returned to force retaining of the [eventual] relocations. This is also
spelled out individually for each of them in md_apply_fix().
Otherwise the string table may grow and hence e.g. change a final binary
(observed with PE/COFF ones) even if really there's no change. Doing so
in fact reduces the overall amount of code, and in particular the number
of places which need to remain in sync.
Afaics there's no real equivalent to the "traditional_format" field used
when linking, so hashing is always enabled when copying / stripping.
Neither of the tools is really a linker, so whatever was originally
recorded should be retained rather than being overwritten by these
tools' versions.
Fill the timestamp field suitably for _bfd_XXi_only_swap_filehdr_out().
Instead of re-arranging the present if(), fold this logic with that of
copying the optional header.
So far this option had no effect when used together with e.g.
--strip-debug. Set BSF_FILE on these symbols to change that.
While altering this also join two adjacent blocks of case labeled
statements with identical code.
When a sufficiently small alignment was specified via --file-alignment,
individual section alignment shouldn't affect placement within the file.
This involves first of all clearing D_PAGED for images when section and
file alignment together don't permit paging of the image. The involved
comparison against COFF_PAGE_SIZE in turn helped point out (through a
compiler warning) that 'page_size' should be of unsigned type (as in
particular FileAlignment is). This yet in turn pointed out a dubious
error condition (which is being deleted).
For the D_PAGED case I think the enforced file alignment may still be
too high, but I'm wary of changing that logic without knowing of
possible corner cases.
Furthermore file positions in PE should be independent of the alignment
recorded in section headers anyway. Otherwise there are e.g. anomalies
following commit 6f8f6017a0 ("PR27567, Linking PE files adds alignment
section flags to executables") in that linking would use information a
subsequent processing step (e.g. stripping) wouldn't have available
anymore, and hence a binary could change in that 2nd step for no actual
reason. (Similarly stripping a binary linked with a linker pre-dating
that commit would change the binary again when stripping it a 2nd time.)
This commit fixes canonical extension order to follow the RISC-V ISA
Manual draft-20210402-1271737 or later.
bfd/ChangeLog:
* elfxx-riscv.c (riscv_recognized_prefixed_ext): Fix "K" extension
prefix to be placed before "J".
Replace the sve bool member of aarch64_features with a vq member that
holds the vector quotient. It is zero if SVE is not present.
Add std::hash<> specialization and operator== so that aarch64_features
can be used as a key with std::unordered_map<>.
Change the various functions that create or lookup aarch64 target
descriptions to accept a const aarch64_features object rather than a
growing number of arguments.
Replace the multi-dimension tdesc_aarch64_list arrays used to cache
target descriptions with unordered_maps indexed by aarch64_feature.
PR gas/27217
Prior to trying to address PR gas/28888 I noticed anomalies in how
certain insns would / wouldn't be affected in similar ways.
Commit eac4eb8ecb ("Fix a problem assembling AArch64 sources when a
relocation is generated against a symbol that has a defined value") had
two copy-and-paste mistakes, passing the wrong type to
aarch64_force_reloc().
It further failed to add placeholder relocation types to that function's
block of case labels leading to a return of 1. While not of interest for
aarch64_force_relocation() (these placeholders are resolved right in
parse_operands()), calls to aarch64_force_reloc() happen before that
resolution would take place.