| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
For relocatable links the output .sframe section size may be wrong.
This can be observed when dumping the SFrame information from the x86-64
sframe-reloc-1 test:
Name Address Off Size
.sframe 0000000000000000 000110 00007f
Offset Type Symbol's Value Symbol's Name + Addend
000000000000001c R_X86_64_PC32 0000000000000000 .text + 1c
0000000000000030 R_X86_64_PC32 0000000000000000 .text + 65
0x00000000 e2de0201 0300f800 02000000 08000000 ................
0x00000010 1e000000 00000000 28000000 00000000 ........(.......
0x00000020 35000000 00000000 04000000 00000000 5...............
0x00000030 00000000 25000000 0f000000 04000000 ....%...........
offset 1st FRE---^^^^^^^^ ^^^^^^^^---number of FREs
0x00000040 00000000 00030801 0510f004 0410f034 ...............4
FDE info---^^ | begin of FDEs
0x00000050 0508f000 03080105 10f00404 10f02405 ..............$.
11111112222222223333333334444---FRE 1, 2, 3, 4
0x00000060 08f00000 00000000 00000000 00000000 ................
4444^^^^...
0x00000070 00000000 00000000 00000000 000000 ...............
...^^^^^^---excessive section
When running the x86-64 test cross build on a big-endian system, such
as s390x, objdump and readelf fail to dump the SFrame information with
the following error message:
Error: SFrame decode failure: Buffer does not contain SFrame data.
This is because the following check in flip_sframe() fails, which gets
only invoked if the endianness of the SFrame data is different from the
host system one:
/* All FDEs and FREs must have been endian flipped by now. */
if ((j != ihp->sfh_num_fres) || (bytes_flipped != (buf_size - hdrsz)))
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
With:
j=8, ihp->sfh_num_fres=8, bytes_flipped=70, buf_size=127, hdrsz=28
bfd/
* elf-sframe.c (_bfd_elf_write_section_sframe): Update section
size also for relocatable links.
---
TBD include Jens Signed off by
|
|
The SFrame FDE's function start address is always emitted as follows by
GAS and ld: it is the offset of the start PC of the respective function
from the FDE field itself.
GAS and ld will emit a flag SFRAME_F_FDE_FUNC_START_ADDR_PCREL set to 1
when emitting the field in this encoding.
binutils/
* NEWS: Announce the change of encoding for SFrame FDE func
start addr field.
|
|
PR/32666 Incorrect .rela.sframe when using ld -r
Input SFrame sections are merged using _bfd_elf_merge_section_sframe (),
which clubs all SFrame FDEs together in one blob and all SFrame FREs in
another. This, of course, means the offset of an SFrame FDE in the output
section cannot be simply derived from the output_offset of the sections.
Fix this by providing _bfd_elf_sframe_section_offset () which returns
the new offset of the SFrame FDE in the merged SFrame section.
Unlike EH_Frame sections, which also use the _bfd_elf_section_offset (),
to update the r_offset, SFrame sections have distinct merging semantics.
In case of SFrame, the SFrame FDE will not simply sit at location
"sec->output_offset + offset of SFrame FDE in sec". Recall that information
layout in an SFrame section is as follows:
SFrame Header
SFrame FDE 1
SFrame FDE 2
...
SFrame FDEn
SFrame FREs (Frame Row Entries)
Note how the SFrame FDEs and SFrame FREs are clubber together in groups
of their own.
Next, also note how the elf_link_input_bfd () does a:
irela->r_offset += o->output_offset;
This, however, needs to be avoided for SFrame sections because the
placement of all FDEs is at the beginning of the section. So, rather than
conditionalizing this as follows:
if (o->sec_info_type != SEC_INFO_TYPE_SFRAME)
irela->r_offset += o->output_offset;
the implementation in _bfd_elf_sframe_section_offset () does a reverse
adjustment, so that the generic parts of the linking process in
elf_link_input_bfd () are not made to do SFrame specific adjustments.
Add a new enum to track the current state of the SFrame input section
during the linking process (SFRAME_SEC_DECODED, SFRAME_SEC_MERGED) for
each input SFrame section. This is then used to assert an assumption
that _bfd_elf_sframe_section_offset () is being used on an input SFrame
sections which have not been merged (via
_bfd_elf_merge_section_sframe ()) yet.
bfd/
* elf-bfd.h: New declaration.
* elf-sframe.c (_bfd_elf_sframe_section_offset): New definition.
* elf.c (_bfd_elf_section_offset): Adjust offset if SFrame
section.
ld/testsuite/
* ld-x86-64/x86-64.exp: New test.
* ld-x86-64/sframe-reloc-1.d: New test.
---
Notes:
This patch was previously reviewed.
https://inbox.sourceware.org/binutils/d914ea1f-6c11-4ef5-ac15-404fd7afd26d@suse.com/
There was a comment on moving this
if (o->sec_info_type != SEC_INFO_TYPE_SFRAME)
irela->r_offset += o->output_offset;
to inside the _bfd_elf_sframe_section_offset () or keep it in
elf_link_input_bfd () with some code comments. Although, I think
keeping it deliberatly out of _bfd_elf_sframe_section_offset () was
clearer as that helped keep the contract of _bfd_elf_section_offset ()
symmetrical across section types. That said, taking the overall
opinions shared in the previous review, I have moved the stub inside
_bfd_elf_sframe_section_offset (). But if opinions have changed, I am
happy to bring the conditional back to elf_link_input_bfd ().
|
|
This patch convenes a set of changes in bfd, gas, ld, libsframe towards
moving to the new encoding for the 'sfde_func_start_address' field in
SFrame FDE.
First, gas must now mark all SFrame sections with the new flag
SFRAME_F_FDE_FUNC_START_ADDR_PCREL. gas was already emitting the field
in the said encoding.
* gas/gen-sframe.c (output_sframe_internal): Emit the flag
SFRAME_F_FDE_FUNC_START_ADDR_PCREL.
Similarly for ld, adopt the new semantics of sfde_func_start_address
consistently. This means:
- When merging SFrame sections, check that all input SFrame sections
have the SFRAME_F_FDE_FUNC_START_ADDR_PCREL flag set. If the check
fails, ld errors out.
- When merging SFrame sections, keep even the in-memory contents of
the FDE function start address (buffer passed to libsframe
sframe_encoder_write () for writing out) are encoded in the new
semantics. While it is, in theory, possible that instead of doing this
change here, we adjust the value of sfde_func_start_address at the final
write (sframe_encoder_write) time. But latter is not favorable for
maintenanance and may be generally confusing for developers.
- When creating SFrame for PLT entries, emit flag
SFRAME_F_FDE_FUNC_START_ADDR_PCREL.
bfd/
* elf-sframe.c (_bfd_elf_merge_section_sframe): Check for flag
SFRAME_F_FDE_FUNC_START_ADDR_PCREL set for all input bfds. If
not, error out. Also, adopt the new semantics of function start
address encoding.
* bfd/elfxx-x86.c (_bfd_x86_elf_create_sframe_plt): Emit flag
SFRAME_F_FDE_FUNC_START_ADDR_PCREL.
Next, for dumping SFrame sections, now that we are emitting the same
encoding in GAS, non-relocatable and relocatable SFrame links, it is the
time to set relocate to TRUE in debug_displays[].
binutils/
* dwarf.c (struct dwarf_section_display): Allow sframe sections
to now be relocated.
gas/testsuite/
* gas/cfi-sframe/cfi-sframe-aarch64-pac-ab-key-1.d: Update the
test. Relocatable SFrame sections now display non-zero value
(appropriate function start address).
Now, as the SFrame sections on-disk and in-memory use the new semantics of
sfde_func_start_address encoding (i.e., function start address is the
offset from the sfde_func_start_address field to the start PC), the
calculation to make it human readable (i.e., relatable to the addresses
in .text sections) needs adjustment.
libsframe/
* sframe-dump.c (dump_sframe_func_with_fres): Adjust the
function start address for dumping.
Now that both the emission of the new encoding, and the relocation of
sections before dumping them is in place, it is time to adjust the
testcases.
gas/testsuite/
* gas/cfi-sframe/cfi-sframe-aarch64-1.d: Update expected output
to include SFRAME_F_FDE_FUNC_START_ADDR_PCREL instead of NONE.
* gas/cfi-sframe/cfi-sframe-aarch64-2.d: Likewise.
* gas/cfi-sframe/cfi-sframe-aarch64-3.d: Likewise.
* gas/cfi-sframe/cfi-sframe-aarch64-4.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-1.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-10.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-11.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-2.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-3.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-4.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-5.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-6.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-7.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-8.d: Likewise.
* gas/cfi-sframe/cfi-sframe-common-9.d: Likewise.
* gas/cfi-sframe/cfi-sframe-x86_64-1.d: Likewise.
* gas/cfi-sframe/cfi-sframe-x86_64-2.d: Likewise.
* gas/cfi-sframe/cfi-sframe-x86_64-empty-1.d: Likewise.
* gas/cfi-sframe/cfi-sframe-x86_64-empty-2.d: Likewise.
* gas/cfi-sframe/cfi-sframe-x86_64-empty-3.d: Likewise.
* gas/cfi-sframe/cfi-sframe-x86_64-empty-4.d: Likewise.
* gas/cfi-sframe/common-empty-1.d: Likewise.
* gas/cfi-sframe/common-empty-2.d: Likewise.
* gas/cfi-sframe/common-empty-3.d: Likewise.
* gas/scfi/x86_64/scfi-cfi-sections-1.d: Likewise.
* gas/scfi/x86_64/scfi-dyn-stack-1.d: Likewise.
ld/testsuite/
* ld-aarch64/sframe-simple-1.d: Update expected output to
include SFRAME_F_FDE_FUNC_START_ADDR_PCREL.
* ld-x86-64/sframe-ibt-plt-1.d: Likewise.
* ld-x86-64/sframe-plt-1.d: Likewise.
* ld-x86-64/sframe-pltgot-1.d: Likewise.
* ld-x86-64/sframe-pltgot-2.d: Likewise.
* ld-x86-64/sframe-simple-1.d: Likewise.
Naturally, the change of semantics for 'SFrame FDE function start address'
has consequences on the implementation in libsframe. As per the new
semantics:
- Function start address in the SFrame FDE (sfde_func_start_address)
is an offset from the FDE function start address field to the start
PC of the associated function.
Note that, the libsframe library brings the SFrame section contents into
its own memory to create a sframe_decoder_ctx object via sframe_decode
(). Many internal and user-interfacing APIs then use sframe_decoder_ctx
object to interact and fulfill the work.
In context of changing semantics for sfde_func_start_address, following
relevant examples may help understand the impact:
- sframe_find_fre () finds a the SFrame stack trace data (SFrame FRE)
given a lookup offset (offset of lookup_pc from the start of SFrame
section). Now that the sfde_func_start_address includes the
distance from the sfde_func_start_address field to the start of
SFrame section itself, the comparison checks of
sfde_func_start_address with the incoming lookup offset need
adjustment.
- Some internal functions (sframe_get_funcdesc_with_addr_internal ()
finds SFrame FDE by using binary seach comparing
sfde_func_start_address fields, etc.) need adjustments.
- sframe_encoder_write () sorts the SFrame FDEs before writing out
the SFrame data. Sorting of SFrame FDE via the internal function
sframe_sort_funcdesc() needs adjustments: the new encoding of
sfde_func_start_address means the distances are not from the same
anchor, so cannot be sorted directly.
This patch takes the approach of adding a new internal function:
- sframe_decoder_get_secrel_func_start_addr (): This function returns
the offset of the start PC of the function from the start of SFrame
section, i.e., it gives a section-relative offset.
As the sframe_decoder_get_secrel_func_start_addr () API needs the value
of the function index in the FDE list, another internal API needs
sframe_fre_check_range_p () adjustments too.
Sorting the FDEs (via sframe_sort_funcdesc ()) is done by first bringing
all offsets in sfde_func_start_address relative to start of SFrame
section, followed by sorting, and then readjusting the offsets accroding
to the new position in the FDE list.
TBD:
- Version bump libsframe. The change in encoding of
sfde_func_start_address means the APIs sframe_encoder_add_funcdesc ()
and sframe_find_fre () etc. are now backwards incompatible with previous
releases. If this change is backported, we need to reserve a version
bump for a backport too I think.
libsframe/
* sframe.c (sframe_decoder_get_secrel_func_start_addr): New
static function.
(sframe_fre_check_range_p): Adjust the interface a bit.
(sframe_get_funcdesc_with_addr_internal): Use
sframe_decoder_get_secrel_func_start_addr () when comparing
sfde_func_start_address with user input offset.
(sframe_find_fre): Adopt the new semantics.
(sframe_sort_funcdesc): Likewise.
For the libsframe testsuite, use the new encoding for FDE func start
addr: distance between the FDE sfde_func_start_address field and the
start PC of the function itself.
Use SFRAME_F_FDE_FUNC_START_ADDR_PCREL flag, though the sframe_encode ()
interface in libsframe applies no sanity checks for the encoding itself.
libsframe/testsuite/
* libsframe.find/findfre-1.c: Adjust to use the new
SFRAME_F_FDE_FUNC_START_ADDR_PCREL specific encoding.
* libsframe.find/findfunc-1.c: Likewise.
* libsframe.find/plt-findfre-1.c: Likewise.
|
|
PR libsframe/32589 - function start address is zero in SFrame section dump
Currently, readelf and objdump display the SFrame sections in ET_REL
object files with function start addresses of each function as 0. This
makes it difficult to correlate SFrame stack trace information with the
individual functions in the object file.
For objdump, use the dump_dwarf () interface to dump SFrame section.
Similarly, for readelf, use the display_debug_section () interface to
dump SFrame section. These existing interfaces (for DWARF debug
sections) already support relocating the section contents before
dumping, so lets use them for SFrame sections as well.
When adding a new entry for SFrame in debug_option_table[], use char
'nil' and the option name of "sframe-internal-only". This is done so
that there is no additional (unnecessary) user-exposed ways of dumping
SFrame sections. Additionally, we explicitly disallow the
"sframe-internal-only" from external/user input in --dwarf (objdump).
Similarly, "sframe-internal-only" is explicitly matched and disallowed
from --debug-dump (readelf).
For objdump and readelf, we continue to keep the same error messaging as
earlier:
$ objdump --sframe=sframe bubble_sort.o
...
No sframe section present
$ objdump --sframe=.sfram bubble_sort.o
...
No .sfram section present
$ objdump --sframe=sframe-internal-only sort
...
No sframe-internal-only section present
Similarly for readelf:
$ readelf --sframe= bubble_sort.o
readelf: Error: Section name must be provided
$ readelf --sframe=.sfram bubble_sort.o
readelf: Warning: Section '.sfram' was not dumped because it does not exist
$ readelf --sframe=sframe bubble_sort.o
readelf: Warning: Section 'sframe' was not dumped because it does not exist
PS: Note how this patch adds a new entry to debug_displays[] with a
relocate value set to FALSE. This will be set to TRUE in a subsequent
patch ("bfd: gas: ld: libsframe: emit func start addr field as an offset
from FDE") when fixes are made to emit the value of the
'sfde_func_start_address' field in the new encoding
SFRAME_F_FDE_FUNC_START_ADDR_PCREL across gas and ld.
binutils/
* dwarf.c (display_sframe): New definition.
(dwarf_select_sections_all): Enable SFrame section too.
(struct dwarf_section_display): Add entry for SFrame section.
* dwarf.h (enum dwarf_section_display_enum): Add enumerator for
SFrame.
* objdump.c (dump_section_sframe): Remove.
(dump_sframe_section): Add new definition.
(dump_bfd): Use dump_sframe_section.
* binutils/readelf.c (dump_section_as_sframe): Remove.
---
This patch was previously reviewed at part of other series previously:
https://inbox.sourceware.org/binutils/20250308073853.78738-3-indu.bhagat@oracle.com/
The review comments have been addressed in this patch. The setting of
relocate to FALSE for the new record is the new diff.
{ { ".sframe", "", "", NO_ABBREVS }, display_sframe, &do_sframe, false },
This is necessary to keep each patch in the series bisectable and reviewable.
|
|
libsframe/doc/
* sframe-spec.texi: Add details about sfde_func_start_address
encoding.
|
|
Also, update the section "Changes from Version 1 to Version 2" to
include the specification of the new flag
SFRAME_F_FDE_FUNC_START_ADDR_PCREL as an errata release to the SFrame
Version 2 specification.
libsframe/doc/
* sframe-spec.texi: Add details about the new flag.
|
|
Add a new flag SFRAME_F_FDE_FUNC_START_ADDR_PCREL to SFrame stack trace
format. If set, this flag indicates that the function start address
field (sfde_func_start_address) is the offset to the function start
address from the SFrame FDE function start address field itself.
Such an encoding is friendlier to the exisitng PC-REL relocations
available in the ABIs supported in SFrame: AMD64 (R_X86_64_PC32) and
AArch64 (R_AARCH64_PREL32). In subsequent patches, we will make the
implementation in gas and ld to both:
- emit the values in the same (above-mentioned) encoding uniformly.
- set the flag SFRAME_F_FDE_FUNC_START_ADDR_PCREL in the SFrame header
for consumers to be able to distinguish.
include/
* sframe.h (SFRAME_F_FDE_FUNC_START_ADDR_PCREL): New definition.
libsframe/
* sframe-dump.c (MAX_NUM_FLAGS, flags_helper): Update to include
the new flag.
* sframe.c (sframe_header_sanity_check_p): Use uint8_t.
|
|
These APIs will be later used by the linker to arrange SFrame FDEs in
the output SFrame section.
include/
* sframe-api.h (sframe_decoder_get_offsetof_fde_start_addr): New
declaration.
(sframe_encoder_get_offsetof_fde_start_addr): Likewise.
libsframe/
* libsframe.ver: List the new APIs.
* sframe.c (sframe_decoder_get_offsetof_fde_start_addr): New
definition.
(sframe_encoder_get_offsetof_fde_start_addr): Likewise.
|
|
To prepare code for accommodating new flag additions easily as the
format evolves.
libsframe/
* sframe-dump.c (SFRAME_HEADER_FLAGS_STR_MAX_LEN): Rename from.
(struct dump_flags_helper): New helper struct definition.
(dump_sframe_header_flags): .. to here. New definition.
(SFRAME_FLAGS_STR_MAX_LEN): Rename to.
(MAX_NUM_FLAGS): New definition.
(dump_sframe_header): Move some implementation from here ..
|
|
Add new APIs, one each for getting flags from the SFrame decoder and
SFrame encoder context objects respectively.
These will later be used by the linker to uniformly access the flags,
given the SFrame decoder and SFrame encoder objects.
Use the new API, where applicable, within libsframe.
include/
* sframe-api.h (sframe_decoder_get_flags): New declaration.
(sframe_encoder_get_flags): Likewise.
libsframe/
* libsframe.ver: List new APIs.
* sframe.c (sframe_decoder_get_flags): New definition.
(sframe_encoder_get_flags): Likewise.
(sframe_get_funcdesc_with_addr_internal): Use the new API.
(sframe_encoder_get_flags): Likewise.
(sframe_encoder_write_sframe): Likewise.
|
|
libsframe/doc/
* sframe-spec.texi: Include date with each publication.
|
|
|
|
They seem like good candidates to become methods, just like
objfile::has_partial_symbols.
Change-Id: Ic6df83012629c1137708b8ceb327f9868d8abcb5
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
|
|
I was a bit confused about the -lbl option in gdb_test_multiple, and needed
to read its implementation to determine that it would be useful for my
needs. Explicitly mention what the option does and why it's useful to
hopefully help other developers.
Reviewed-By: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
The Linaro CI runs the GDB testsuite using the read1 tool, which
significantly increases the time it takes DejaGNU to read inferior output.
On top of that sometimes the test machine has higher than normal load,
which causes tests to run even slower.
Because gdb.base/default.exp tests some verbose commands such as "info
set", it sometimes times out while waiting for the complete command
output when running in the Linaro CI environment.
Fix this problem by consuming each line of output from the most verbose
commands with gdb_test_multiple's -lbl (line-by-line) option — which
causes DejaGNU to reset the timeout after each match — and also by
breaking up regular expressions that try to match more than 2000
characters (the default Expect buffer size) in one go into multiple
matches.
Some tests use the same regular expression, so I created a procedure for
them. This is the case for "i" / "info", "info set" / "show", and "set
print" tests.
The tests for "show print" don't actually test their output, so this
patch improves them by checking some lines of the output.
Reviewed-By: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
|
|
In commit db4ab410dec3 I rewrote OP_STORE handling to support writing
near the end of a section. The rewrite had some bugs, fixed in commit
3e02c4891dcb. However I wasn't entirely happy with the code writing
the bitfield:
- it doesn't support 64-bit fields with a bit offset,
- the code is duplicated and inelegant,
- the stack ought to be popped whenever seeing one of these relocs,
even if the reloc can't be applied.
This patch fixes all of the above.
In addition, it is clear from the OP_STORE description in the ABI that
a 64-bit field is encoded as 0 in r_size, so I've decoded that in
alpha_ecoff_swap_reloc_in. The aborts there are not appropriate as
they can be triggered by user input (fuzzed object files). Also,
stack underflow wasn't checked in alpha_relocate_section.
* coff-alpha.c (alpha_ecoff_swap_reloc_in): Replace aborts
with asserts. Decode ALPHA_R_OP_STORE r_size of zero.
(write_bit_field): New function.
(alpha_ecoff_get_relocated_section_contents): Use it.
(alpha_relocate_section): Here too. Catch stack underflow.
|
|
The SFrame FRE start address (fre_start_addr) is defined as unsigned
32-bit integer, as it is an offset from SFrame FDE function start
address (sfde_func_start_address) and functions only grow upwards
(towards higher addresses).
The SFrame FRE start IP offset is a synonym to the SFrame FRE start
address. The SFrame FRE end IP offset is either the value of the
subsequent FDE start address minus one, if that exists, or the FDE
function size minus one otherwise. Both should therefore be handled
as unsigned 32-bit integer.
In libsframe the "lookup PC" (pc) and SFrame FDE function start address
(sfde_func_start_address) are both signed integers, as they are actually
offsets from the SFrame section. The unsigned FDE start/end IP offsets
may therefore only be safely compared against the offset of the lookup
PC from FDE function start address if the FDE function start address is
lower or equal to the lookup PC, as this guarantees the offset to be
always positive:
Given:
lookup_pc = pc - sframe_addr
sfde_func_start_address = func_start_addr - sframe_addr
If the FDE function start address is lower or equal than the lookup PC,
which both are signed offsets from SFrame section, then the function
start address is also lower or equal to the PC, which are both unsigned:
sfde_func_start_address <= lookup_pc
func_start_addr - sframe_addr <= pc - sframe_addr
func_start_addr <= pc
With that the offset of the lookup PC from FDE function start address
(lookup_pc - sfde_func_start_address) must always be positive, if
FDE function start address is lower or equal to the lookup PC:
lookup_pc - sfde_func_start_address
= pc - sframe_addr - (func_start_addr - sframe_addr)
= pc - func_start_addr
libsframe/
* sframe.c (sframe_find_fre): Define and handle start_ip_offset
and end_ip_offset as unsigned (same as FRE fre_start_addr).
(sframe_fre_check_range_p): Likewise. Define PC offset (from
function start address) as unsigned.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
|
|
The SFrame FREs for an SFrame FDE are sorted on their start address.
Therefore the linear search for a matching SFrame FRE can be stopped,
if its start address is greater than the searched for PC.
libsframe/
* sframe.c (sframe_find_fre): Stop search if FRE's start IP is
greater than PC.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
|
|
sframe_get_funcdesc_with_addr_internal erroneously returns the last FDE,
if its function start address is lower than the searched for address.
Simplify the binary search for a SFrame FDE for a given address. Only
return an FDE, if the searched for address is within the bounds of the
FDE function start address and function size.
libsframe/
* sframe.c (sframe_get_funcdesc_with_addr_internal): Correct
binary search for SFrame FDE.
libsframe/testsuite/
* libsframe.find/plt-findfre-1.c: Add test for out of range
PLT6.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
|
|
The testcase had usages of some magic numbers, making it difficult to
keep up when format changes come along.
libsframe/testsuite/
* libsframe.find/findfunc-1.c: Restructure a bit. Run test for two
ways of placement of .sframe and .text.
|
|
The testcase had usages of some magic numbers, making it difficult to
keep up when format changes come along.
libsframe/testsuite/
* libsframe.find/findfre-1.c: Restructure a bit. Run test for two
ways of placement of .sframe and .text.
|
|
SFrame FDEs of type SFRAME_FDE_TYPE_PCMASK are used for repetitive code
patterns, e.g., pltN entries. For SFrame FDEs of type
SFRAME_FDE_TYPE_PCMASK, sframe_fre_check_range_p erroneously tested the
given PC instead of the masked PC offset from function start address.
Therefore it only worked correctly by chance, e.g., if the function start
address was aligned on the repetition block size.
For regular SFrame FDEs the PC offset from function start address must
be within a SFrame FRE's start IP offset and end IP offset. For SFrame
FDEs of type SFRAME_FDE_TYPE_PCMASK, the masked PC offset must be within
that range.
SFrame FRE start/end IP offsets are relative to the SFrame FDE function
start address. For regular SFrame FDEs, the PC offset from function
start address must be within a SFrame FRE's start IP offset and end IP
offset. For SFRAME_FDE_TYPE_PCMASK type FDEs, the masked PC offset must
be within that range.
Exercise the testcase for a variety of placements; without the fix some
of these tests will fail. Also, make the testcase itself easier to
follow by adding appropriate vars where applicable.
libsframe/
* sframe.c (sframe_fre_check_range_p): Fix logic for
SFRAME_FDE_TYPE_PCMASK type FDE.
libsframe/testsuite/
* libsframe.find/plt-findfre-1.c: Adjust the test for a variety
of placements of .sframe and .plt.
Co-Authored-by: Jens Remus <jremus@linux.ibm.com>
|
|
Fix PR gas/32953 - sframe: incorrect handling of .cfi_same_value in gas
As per documentation, .cfi_same_value indicates that the current value
of register is the same like in the previous frame, i.e. no restoration
needed.
gas/
* gen-sframe.c (sframe_xlate_do_same_value): New definition.
(sframe_do_cfi_insn): Handle DW_CFA_same_value.
gas/testsuite/
* gas/cfi-sframe/cfi-sframe.exp: Add new tests.
* gas/cfi-sframe/cfi-sframe-common-11.d: New test.
* gas/cfi-sframe/cfi-sframe-common-11.s: New test.
|
|
Factor out compare_pointers, use it in compare_symbols and compare_msymbols,
and add comments about unstable sorting result.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
In compare_symbols in gdb/linespec.c:
...
uia = (uintptr_t) a.symbol->symtab ()->compunit ()->objfile ()->pspace ();
uib = (uintptr_t) b.symbol->symtab ()->compunit ()->objfile ()->pspace ();
if (uia < uib)
return true;
if (uia > uib)
return false;
...
we compare pointers to struct program_space, which gives unstable sorting
results.
The assumption is that this doesn't matter, but as PR32202 demonstrates,
sometimes it does.
While PR32202 is fixed elsewhere, it seems like a good idea to stabilize this
comparison, because it comes at a small cost and possibly prevents
hard-to-reproduce user-visible ordering issues.
Fix this by comparing the program space IDs instead of the pointers.
Likewise in compare_msymbols.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
With test-case gdb.multi/pending-bp-del-inferior.exp, occasionally I run into:
...
(gdb) info breakpoints^M
Num Type Disp Enb Address What^M
3 dprintf keep y <MULTIPLE> ^M
printf "in foo"^M
3.1 y 0x004004dc in foo at $c:21 inf 2^M
3.2 y 0x004004dc in foo at $c:21 inf 1^M
(gdb) FAIL: $exp: bp_pending=false: info breakpoints before inferior removal
...
The FAIL happens because the test-case expects:
- breakpoint location 3.1 to be in inferior 1, and
- breakpoint location 3.2 to be in inferior 2
but it's the other way around.
I managed to reproduce this with a trigger patch in
compare_symbols from gdb/linespec.c:
...
uia = (uintptr_t) a.symbol->symtab ()->compunit ()->objfile ()->pspace ();
uib = (uintptr_t) b.symbol->symtab ()->compunit ()->objfile ()->pspace ();
- if (uia < uib)
+ if (uia > uib)
return true;
- if (uia > uib)
+ if (uia < uib)
return false;
...
The order enforced by compare_symbols shows up in the "info breakpoints"
output because breakpoint::add_location doesn't enforce an ordering for equal
addresses:
...
auto ub = std::upper_bound (m_locations.begin (), m_locations.end (),
loc,
[] (const bp_location &left,
const bp_location &right)
{ return left.address < right.address; });
m_locations.insert (ub, loc);
...
Fix this by using new function bp_location_is_less_than
(forwarding to bp_location_ptr_is_less_than) in breakpoint::add_location.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
PR gdb/32202
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32202
|
|
bp_location_ptr_is_less_than
In breakpoint.c, we have:
...
/* A comparison function for bp_location AP and BP being interfaced to
std::sort. Sort elements primarily by their ADDRESS (no matter what
bl_address_is_meaningful says), secondarily by ordering first
permanent elements and tertiarily just ensuring the array is sorted
stable way despite std::sort being an unstable algorithm. */
static int
bp_location_is_less_than (const bp_location *a, const bp_location *b)
...
There are few problems here:
- the return type is int. While std::sort allows this, because int is
convertible to bool, it's clearer to use bool directly,
- it's not abundantly clear from either function name or comment that we can
use this to sort std::vector<bp_location *> but not
std::vector<bp_location>, and
- the comment mentions AP and BP, but there are no such parameters.
Fix this by:
- changing the return type to bool,
- renaming the function to bp_location_ptr_is_less_than and mentioning
std::vector<bp_location *> in the comment, and
- updating the comment to use the correct parameter names.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
ALPHA_R_OP_STORE copies one byte too many and also will cause out of
range error when it tries to copy from the end of section. Since
"endbyte" is already rounded to next full byte, there is enough bits
to copy and the additional "+ 1" is erroneous in bytes count. I also
believe size is incorrectly decreased.
|
|
Let's not introduce support for breakpoint types the target beneath does
not support, even though we could while replaying.
Otherwise, users may set breakpoints during replay that then couldn't be
inserted into the target when switching back to recording.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Relocation overflows can silently write incorrect value to
the file, so overflow checks are added to avoid this.
|
|
|
|
|
|
tls_maybe_erase_slot
Functions tls_maybe_fill_slot and tls_maybe_erase_slot blindly assume
that the passe solibs come from solib-svr4. This is not always the
case, because they are called even on the systems where the solib
implementation isn't solib-svr4. Add some checks to return early in
that case.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32990
Change-Id: I0a281e1f4826aa1914460c2213f0fae1bdc9af7c
Tested-By: Hannes Domani <ssbssa@yahoo.de>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
There is no need to allocate the addrmap_mutable on the heap.
Change-Id: Ia6ec17101a44ae5eaffbf3382c9639414ce5343e
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Replace the macro with a function. I don't see a need to use a macro
here, a function is easier to read.
Change-Id: I22370040cb546470498d64939b246b03700af398
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
On x86_64-linux, with gcc 7.5.0 I ran into a build breaker:
...
gdb/dwarf2/read.c: In function ‘dwo_unit* lookup_dwo_unit_in_dwp()’:
gdb/dwarf2/read.c:7403:22: error: unused variable ‘inserted’ \
[-Werror=unused-variable]
auto [it, inserted] = dwo_unit_set.emplace (std::move (dwo_unit));
^
...
Fix this by dropping the unused variable.
Tested on x86_64-linux, by completing a build.
|
|
Change-Id: I4335fbfdabe49778fe37b08689eec59be94c424b
|
|
|
|
Spotted a small white space mistake in the NEWS file. Fixed.
|
|
The buildbot pointed out this compilation failure on AlmaLinux, with g++
8.5.0, which is not seen on more recent systems:
CXX gdbtypes.o
In file included from ../../binutils-gdb/gdb/gdbtypes.c:39:
../../binutils-gdb/gdb/dwarf2/read.h:639:8: error: ‘mutex’ in namespace ‘std’ does not name a type
std::mutex dwo_files_lock;
^~~~~
../../binutils-gdb/gdb/dwarf2/read.h:639:3: note: ‘std::mutex’ is defined in header ‘<mutex>’; did you forget to ‘#include <mutex>’?
../../binutils-gdb/gdb/dwarf2/read.h:35:1:
+#include <mutex>
../../binutils-gdb/gdb/dwarf2/read.h:639:3:
std::mutex dwo_files_lock;
^~~
Fix it by including <mutex> in dwarf2/read.h.
Change-Id: Iba334a3dad217c86841a5e804d0f386876f5ff2f
|
|
In commit 2711e4754fc ("Ensure cooked_index_entry self-tests are run"), we
rewrite the function definition of _initialize_dwarf2_entry into a normal
form that allows the make-init-c script to detect it:
...
void _initialize_dwarf2_entry ();
-void _initialize_dwarf2_entry ()
+void
+_initialize_dwarf2_entry ()
...
Update make-init-c to also detect the "void _initialize_dwarf2_entry ()"
variant.
Tested on x86_64-linux, by reverting commit 2711e4754fc, rebuilding and
checking that build/gdb/init.c doesn't change.
|
|
Add a dwarf assembly test-case using a DW_FORM_strx in a .dwo file.
Tested on x86_64-linux.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The dwo_lock mutex is used to synchronize access to some dwo/dwp-related
data structures, such as dwarf2_per_bfd::dwo_files and
dwp_file::loaded_{cus,tus}. Right now the scope of the lock is kind of
coarse. It is taken in the top-level lookup_dwo_unit function, and held
while the thread:
- looks up an existing dwo_file in the per-bfd hash table for the given
id/signature
- if there's no existing dwo_file, attempt to find a .dwo file, open
it, build the list of units it contains
- if a new dwo_file was created, insert it in the per-bfd hash table
- look up the desired unit in the dwo_file
And something similar for the dwp code path. This means that two
indexing thread can't read in two dwo files simultaneously. This isn't
ideal in terms of parallelism.
This patch breaks this lock into 3 more fine grained locks:
- one lock to access dwarf2_per_bfd::dwo_files
- one lock to access dwp_file::loaded_{cus,tus}
- one lock in try_open_dwop_file, where we do two operations that
aren't thread safe (bfd_check_format and gdb_bfd_record_inclusion)
Unfortunately I don't see a clear speedup on my computer with 8 threads.
But the change shouldn't hurt, in theory, and hopefully this can be a
piece that helps in making GDB scale better on machines with many cores
(if we ever bump the max number of worker threads).
This patch uses "double-checked locking" to avoid holding the lock(s)
for the whole duration of reading in dwo files. The idea is, when
looking up a dwo with a given name:
- with the lock held, check for an existing dwo_file with that name in
dwarf2_per_bfd::dwo_files, if found return it
- if not found, drop the lock, load the dwo file and create a dwo_file
describing it
- with the lock held, attempt to insert the new dwo_file in
dwarf2_per_bfd::dwo_files. If an entry exists, it means another
thread simultaneously created an equivalent dwo_file, but won the
race. Drop the new dwo_file and use the existing one. The new
dwo_file is automatically deleted, because it is help by a unique_ptr
and the insertion into the unordered_set fails.
Note that it shouldn't normally happen for two threads to look up a dwo
file with the same name, since different units will point to different
dwo files. But it were to happen, we handle it. This way of doing
things allows two threads to read in two different dwo files
simulatenously, which in theory should help get better parallelism. The
same technique is used for dwp_file::loaded_{cus,tus}.
I have some local CI jobs that run the fission and fission-dwp boards,
and I haven't seen regressions. In addition to the regular testing, I
ran a few tests using those boards on a ThreadSanitizer build of GDB.
Change-Id: I625c98b0aa97b47d5ee59fe22a137ad0eafc8c25
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
|
|
For the same reason as explained in the previous patch (allocations on
obstacks aren't thread-safe), change the allocation of
dwarf2_section_info object for dwo files within dwp files to use "new".
The dwo_file::section object is not always owned by the dwo_file, so
introduce a new "dwo_file::section_holder" object that is only set when
the dwo_file owns the dwarf2_section_info.
Change-Id: I74c4608573c7a435bf3dadb83f96a805d21798a2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The following patch reduces the duration where the dwo_lock mutex is
taken. One operation that is not thread safe is the allocation on
dwo_units on the per_bfd obstack:
dwo_unit *dwo_unit = OBSTACK_ZALLOC (&per_bfd->obstack, struct dwo_unit);
We could take the lock around this allocation, but I think it's just
easier to avoid the problem by having the dwo_unit objects allocated
with "new".
Change-Id: Ida04f905cb7941a8826e6078ed25dbcf57674090
Approved-By: Tom Tromey <tom@tromey.com>
|
|
While debugging a new failure in my long-suffering "search-in-psyms"
series, I found that the C++ name canonicalizer did not handle a case
like "some_name::operator new []". This should remove the space,
resulting in "some_name::operator new[]" -- but does not.
This happens because the parser requires an operator to be followed by
argument types. That is, it's expected.
However, it seems to me that we do want to be able to canonicalize a
name like this. It will appear in the DWARF as a DW_AT_name, and
furthermore it could be entered by the user.
This patch fixes this problem by changing the grammar to supply the
"()" itself, then removing the trailing "()" when changing to string
form (in the functions that matter).
This isn't ideal -- it might miss a very obscure case involving the
gdb extension of providing fully-qualified names for function-local
statics -- but it improves the situation at least.
It's possible a better solution might be to rewrite the name
canonicalizer. I was wondering if this could perhaps be done without
reference to the grammar -- just by examining the tokens. However,
that's much more involved.
Let me know what you think.
Regression tested on x86-64 Fedora 40.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32939
Reviewed-By: Keith Seitz <keiths@redhat.com>
|
|
ctf_arc_import_parent, called by the cached-opening machinery used by
ctf_archive_next and archive-wide lookup functions like
ctf_arc_lookup_symbol, has an err-pointer parameter like all other opening
functions. Unfortunately it unconditionally initializes it whenever
provided, even if there was no error, which can lead to its being
initialized to an uninitialized value. This is not technically an
API-contract violation, since we don't define what happens to the error
value except when an error happens, but it is still unpleasant.
Initialize it only when there is an actual error, so we never initialize it
to an uninitialized value.
While we're at it, improve all the opening pathways: on success, set errp to
0, rather than leaving it what it was, reducing the likelihood of
uninitialized error param returns in callers too. (This is inconsistent
with the treatment of ctf_errno(), but the err value being a parameter
passed in from outside makes the divergence acceptable: in open functions,
you're never going to be overwriting some old error value someone might want
to keep around across multiple calls, some of which are successful and some
of which are not.)
Soup up existing tests to verify all this.
Thanks to Bruce McCulloch for the original patch, and Stephen Brennan for
the report.
libctf/
PR libctf/32903
* ctf-archive.c (ctf_arc_open_internal): Zero errp on success.
(ctf_dict_open_sections): Zero errp at the start.
(ctf_arc_import_parent): Intialize err.
* ctf-open.c (ctf_bufopen): Zero errp at the start.
* testsuite/libctf-lookup/add-to-opened.c: Make sure one-element
archive opens update errp.
* testsuite/libctf-writable/ctf-compressed.c: Make sure real archive
opens update errp.
|
|
This patch adds support for RISC-V RVA23 and RVB23 Profiles[1].
[1] https://github.com/riscv/riscv-profiles/releases/tag/rva23-rvb23-ratified
bfd/ChangeLog:
* elfxx-riscv.c: New profiles.
gas/ChangeLog:
* testsuite/gas/riscv/attribute-19.d: New test.
* testsuite/gas/riscv/attribute-20.d: New test.
|
