In normal deduplicating links, we insert every type (identified by its
unique hash) precisely once. But conflicting types appear in multiple
dicts, so for those, we loop, inserting them into every target dict
in turn (each corresponding to an input dict that type appears in).
But in cu-mapped links, some of those dicts may have been merged into
one: now that we are hiding duplicate conflicting types more
aggressively in such links, we are getting duplicate identical hidden
types turning up in large numbers.
Fix this by eliminating them in cu-mapping phase 1 (the phase in which this
merging takes place), by checking to see if a type with this hash has
already been inserted in this dict and skipping it if so. This is
redundant and a waste of time in other cu-mapping phases and in normal
links, but in cu-mapped links it saves a few tens to hundreds of kilobytes
in kernel-sized links.
libctf/
PR libctf/33047
* ctf-dedup.c (ctf_dedup_emit_type): Check for already-emitted
types in cu-mapping phase 1.
The previous commits dropped preservation of the non-root flag in ctf_link
and arranged to use it somewhat differently to track conflicting types in
cu-mapped CUs when doing cu-mapped links. This was necessary to prevent
entirely spuriously hidden types from appearing on the output of such links.
Bring it (and the test for it) back. The problem with the previous design
was that it implicitly assumed that the non-root flag it saw on the input
was always meant to be preserved (when in the final phase of cu-mapped links
it merely means that conflicting types were found in intermediate links),
and also that it could figure out what the non-root flag on the input was by
sucking in the non-root flag of the input type corresponding to an output in
the output mapping.
This method of getting properties of the input type *does* work *if* that
property was one of those hashed by the ctf_dedup_hash_type process. In
that case, every type with a given hash will have the same value for all
such properties, so it doesn't matter which one is consulted (the output
mapping points at an arbitrary one of those input types). But the non-root
flag is explicitly *not* hashed in: as a comment in ctf_dedup_rhash_type
notes, being non-root is not a property of a type, and two types (one
non-root, one not) can perfectly well be the same time even though one is
visible and one isn't.
So we cannot use that technique. We have to use something else, which means
we have to decide what to do if two identical types with different nonroot
flag values pop up. The most sensible thing to do is probably to say that
if all instances of a type are non-root-visible, the linked output should
also be non-root-visible: any root-visible types in that set, and the output
type is root-visible again.
We implement this with a new cd_nonroot_consistency dynhash, which maps type
hashes to the value 0 ("all instances root-visible"), 1 ("all instances
non-root-visible") or 2 ("inconsistent"). After hashing is over, we save a
bit of memory by deleting everything from this hashtab that doesn't have a
value of 1 ("non-root-visible"), then use this to decide whether to emit any
given type as non-root-visible or not.
However... that's not quite enough. In cu-mapped links, we want to
disregard this whole thing because we just hide everything -- but in phase
2, when we take the smushed-together CUs resulting from phase 1 and
deduplicate them against each other, we want to do what the previous commits
implemented and ignore the non-root flag entirely, instead falling back to
preventing clashes by hiding anything that would be considered conflicting.
We extend the existing cu_mapped parameter to various bits of ctf_dedup so
that it is now tristate: 0 means a normal link, 1 means the smush-it-
together phase of cu-mapped links, and 2 means the final phase of cu-mapped
links. We do the hide-conflicting stuff only in phase 2, meaning that
normal links by GNU ld can always respect the value of the nonroot flag put
on types in the input.
(One extra thing added as part of this: you can now efficiently delete the
last value returned by ctf_dynhash_next() by calling
ctf_dynhash_next_remove.)
We bring back the ctf-nonroot-linking test with one tweak: linking now works
on mingw as long as you're using the ucrt libc, so re-enable it for better
test coverage on that platform.
libctf/
PR libctf/33047
* ctf-hash.c (ctf_dynhash_next_remove): New.
* ctf-impl.h (struct ctf_dedup) [cd_nonroot_consistency]: New.
* ctf-link.c (ctf_link_deduplicating): Differentiate between
cu-mapped and non-cu-mapped links, even in the final phase.
* ctf-dedup.c (ctf_dedup_hash_type): Callback prototype addition.
Get the non-root flag and pass it down.
(ctf_dedup_rhash_type): Callback prototype addition. Document
restrictions on use of the nonroot flag.
(ctf_dedup_populate_mappings): Populate cd_nonroot_consistency.
(ctf_dedup_hash_type_fini): New function: delete now-unnecessary
values from cd_nonroot_consistency.
(ctf_dedup_init): Initialize it.
(ctf_dedup_fini): Destroy it.
(ctf_dedup): cu_mapping is now cu_mapping_phase. Call
ctf_dedup_hash_type_fini.
(ctf_dedup_emit_type): Use cu_mapping_phase and
cd_nonroot_consistency to propagate the non-root flag into outputs
for normal links, and to do name-based conflict checking only for
phase 2 of cu-mapped links.
(ctf_dedup_emit): cu_mapping is now cu_mapping_phase. Adjust
assertion accordingly.
* testsuite/libctf-writable/ctf-nonroot-linking.c: Bring back.
* testsuite/libctf-writable/ctf-nonroot-linking.lk: Likewise.
If types are conflicting, they are usually moved into separate child dicts
-- but not always. If they are added to the same dict by the cu-mapping
mechanism (as used e.g. for multi-TU kernel modules), we can easily end
up adding multiple conflicting types with the same name to the same dict.
The mechanism used for turning on the non-root-visible flag in order to do
this had a kludge attached which always hid types with the same name,
whether or not they were conflicting. This is unnecessary and can hide
types that should not be hidden, as well as hiding bugs. Remove it, and
replace it with two different approaches:
- for everything but cu-mapped links (the in-memory first phase of a link
with ctf_link_add_cu_mapping in force), check for duplicate names if
types are conflicting, and mark them as hidden if the names are found.
This will never happen in normal links (in an upcoming commit we will
suppress doing even this much in such cases).
- for cu-mapped links, the only case that merges multiple distinct target
dicts into one, we apply a big hammer and simply hide everything! The
non-root flag will be ignored in the next link phase anyway (which dedups
the cu-mapped pieces against each other), and this way we can be sure
that merging multiple types cannot incur name clashes at this stage.
The result seems to work: the only annoyance is that when enums with
conflicting enumerators are found in a single cu-mapped child (so, really
multiple merged children), you may end up with every instance of that enum
being hidden for reasons of conflictingness. I don't see a real way to
avoid that.
libctf/
PR libctf/33047
* ctf-dedup.c (ctf_dedup_emit_type): Only consider non
conflicting types. Improve type hiding in the presence of clashing
enumerators. Hide everything when doing a cu-mapped link: they will
be unhidden by the next link pass if nonconflicting.
This reverts commit 87b2f67310.
It is based on a misconception, that hidden types in the deduplicator
input should always be hidden in the output. For cu-mapped links,
and final links following cu-mapped links, this is not true: we want
to hide inputs if they were conflicting on the output and no more.
We will reintroduce the testcase once a better fix is found.
libctf/
PR libctf/33047
* ctf-dedup.c (ctf_dedup_emit_type): Don't respect the nonroot flag.
* testsuite/libctf-writable/ctf-nonroot-linking.c: Removed.
* testsuite/libctf-writable/ctf-nonroot-linking.lk: Removed.
I believe we previously agreed that the minimum supported Python
version should be 3.4. This patch makes this change, harmonizing the
documentation (which was inconsistent about the minimum version) and
the code.
New in v2: rebased, and removed a pre-3.4 workaround from __init__.py.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-by: Kevin Buettner <kevinb@redhat.com>
Acked-By: Tom de Vries <tdevries@suse.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31870
This changes elf_obj_sy.size from being malloc'd to being on the notes
obstack. That means no code needs to free these expressions, which in
turn means that the size expression can be shared when cloning
symbols. Nothing modifies the size expressions except when resolving.
In all cases I could see, if the size changes the entire expression is
replaced.
The patch also extracts code from elf_copy_symbol_attributes into a
separate function for use by riscv and aarch64.
* config/obj-elf.c (elf_obj_symbol_clone_hook): Delete.
(elf_copy_symbol_size): New function, extracted and modified from..
(elf_copy_symbol_attributes): ..here.
(obj_elf_size): Don't free size and use notes_alloc.
(elf_frob_symbol): Don't free size.
(elf_format_ops): Zero symbol_clone_hook.
* config/obj-elf.h (elf_obj_symbol_clone_hook): Delete.
(obj_symbol_clone_hook): Don't define.
(elf_copy_symbol_size): Declare.
* config/tc-aarch64.c (aarch64_elf_copy_symbol_attributes): Delete.
* config/tc-aarch64.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Define as
elf_copy_symbol_size.
* config/tc-alpha.c (s_alpha_end): notes_alloc symbol size exp.
* config/tc-ia64.c (dot_endp): Likewise.
* config/tc-kvx.c (kvx_endp): Likewise.
* config/tc-mips.c (s_mips_end): Likewise.
* config/tc-riscv.c (riscv_elf_copy_symbol_attributes): Delete.
* config/tc-riscv.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Define as
elf_copy_symbol_size.
If a symbol is at the start of the symbol chain then symbol_rootP
points at the symbol and symbol->x->previous is NULL. Testing either
condition is sufficient, there is no need to test both. Similarly for
the symbol at the end of the symbol chain.
I'm testing the previous/next pointer because it's most likely that
the symbol is not at the start/finish of the chain and thus we need to
use that pointer.
* symbols.c (symbol_remove): Tidy list handling.
(symbol_append, symbol_clone, symbol_insert): Likewise.
On x86, MAX_MEM_FOR_RS_ALIGN_CODE is 35, when the most common
alignment is 2**3 or 2**4, where the max memory required for the
alignment nops is 7 and 15 bytes respectively. So there is some
memory wasted since commit 83d94ae428. It's not a large amount,
especially considering that frag overhead on x86_46 is 144 bytes,
but even so I'd rather not be blamed for increasing gas memory usage.
So to reduce the memory we'd like to take the alignment into
consideration when initialising an rs_align_code frag. The only
difficulty here is start_bundle making an rs_align_code frag with an
alignment of zero initially, then later increasing the alignment. We
change that to use the bundle alignment when setting up the frag. I
think that is sufficient as bundle_align_p2 can't change in the middle
of a start_bundle/finish_bundle sequence.
I haven't modified any targets other than x86 in this patch. Most
won't benefit much due to using fairly small MAX_MEM_FOR_RS_ALIGN_CODE.
* read.c (start_bundle): Create rs_align_code frag with
bundle_align_p2 alignment, then set to zero alignment.
(finish_bundle): Adjust comment.
* frags.c (MAX_MEM_FOR_RS_ALIGN_CODE): Pass p2align and max
to macro.
* config/tc-i386.h (HANDLE_ALIGN): Assert that max_bytes is
sufficient for nop padding.
(max_mem_for_rs_align_code): New inline function.
(MAX_MEM_FOR_RS_ALIGN_CODE): Use it.
* config/tc-aarch64.h: Adjust MAX_MEM_FOR_RS_ALIGN_CODE.
* config/tc-alpha.h: Likewise.
* config/tc-arc.h: Likewise.
* config/tc-arm.h: Likewise.
* config/tc-epiphany.h: Likewise.
* config/tc-frv.h: Likewise.
* config/tc-ia64.h: Likewise.
* config/tc-kvx.h: Likewise.
* config/tc-loongarch.h: Likewise.
* config/tc-m32r.h: Likewise.
* config/tc-metag.h: Likewise.
* config/tc-mips.h: Likewise.
* config/tc-nds32.h: Likewise.
* config/tc-ppc.h: Likewise.
* config/tc-riscv.h: Likewise.
* config/tc-rl78.h: Likewise.
* config/tc-rx.h: Likewise.
* config/tc-score.h: Likewise.
* config/tc-sh.h: Likewise.
* config/tc-sparc.h: Likewise.
* config/tc-spu.h: Likewise.
* config/tc-tilegx.h: Likewise.
* config/tc-tilepro.h: Likewise.
* config/tc-visium.h: Likewise.
* config/tc-xtensa.h: Likewise.
The script syscalls/riscv-canonicalize-syscall-gen.py has been recently
introduced to help support record-full in riscv systems. However, it
was developed before commit 432eca4113,
which made the GDB enum more consistent, which forced the python script
to have a corner case for the "gdb_old_mmap" case.
Since the aforementioned commit has already been merged, we need to
update the special case for the mmap syscall. A special case is still
needed because the script would expect that the glibc sources call the
syscall "old_mmap", or that gdb call it "gdb_sys_mmap", neither of which
happens unfortunately.
This commit doesn't change the .c file because it was already fixed by a
different commit, 65ab41b7d5, which was
pushed as obvious to fix the build issues.
Approved-By: Tom Tromey <tom@tromey.com>
When DAP completion requests receives empty string to complete,
the script crashes due trying to access element -1 from list
being a result of `text.splitlines()` (which for `text == ""`
evaluates into empty list).
This patch adds simple check if `text` is populated, and when it
is not, skips transformations and assigns correct result directly.
Approved-By: Tom Tromey <tom@tromey.com>
I forgot to fix a `;;` typo when pushing a previous patch. Fix it, and
fix all the other instances I could find in the code base.
Change-Id: I298f9ffb1a5157925076ef67b439579b1aeeaa2b
Building current GDB on Cygwin, fails like so:
/home/pedro/gdb/src/gdbsupport/run-time-clock.cc: In function ‘void get_run_time(user_cpu_time_clock::time_point&, system_cpu_time_clock::time_point&, run_time_scope ’:
/home/pedro/gdb/src/gdbsupport/run-time-clock.cc:52:13: error: ‘RUSAGE_THREAD’ was not declared in this scope; did you mean ‘SIGEV_THREAD’?
52 | who = RUSAGE_THREAD;
| ^~~~~~~~~~~~~
| SIGEV_THREAD
Cygwin does not implement RUSAGE_THREAD. Googling around, I see
Cygwin is not alone, other platforms don't support it either. For
example, here is someone suggesting an alternative for darwin/macos:
https://stackoverflow.com/questions/5652463/equivalent-to-rusage-thread-darwin
Fix this by falling back to process scope if thread scope can't be
supported. I chose this instead of returning zero usage or some other
constant, because if gdb is built without threading support, then
process-scope run time usage is the right info to return.
But instead of falling back silently, print a warning (just once),
like so:
(gdb) maint set per-command time on
⚠️ warning: per-thread run time information not available on this platform
... so that developers on other platforms at least have a hint
upfront.
This new warning also shows on platforms that don't have getrusage in
the first place, but does not show if the build doesn't support
threading at all.
New tests are added to gdb.base/maint.exp, to expect the warning, and
also to ensure other "mt per-command" sub commands don't trigger the
new warning.
Change-Id: Ie01b916b62f87006f855e31594a5ac7cf09e4c02
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Tom Tromey <tom@tromey.com>
The solib-target implementation of solib_create_inferior_hook is empty.
Make that method / function pointer optional.
Change-Id: Ie27b8d2c4fc6df74069ac8f88fbe69cf88a6662d
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Two solib ops implementations have dummy implementations for the
in_dynsym_resolve_code callback. Make it optional, to avoid this.
Change-Id: I786776fb82ce1b96335a97713fbfe8074c84c00c
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
The only solib implementation that implements open_symbol_file_object is
SVR4. All others just return 0. Make it optional, to avoid having
these empty functions.
Change-Id: I835197a73323d39231d071f9a9eaac2553f10726
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
I don't think that the file solist.h is useful. It would make sense to
have `struct solib` in solib.h. And then, all that would remain is
`struct solib_ops` and some solib-related function declarations. So,
move it all to solib.h.
Change-Id: I20ecf19787c378066f2c7a6a8a737c1db7c55d9a
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Rename the field to m_solib_list, make it private. Update users to go
through the accessor.
Change-Id: Id720c3a0b1781f4acf31e0dc626f1bd23ed8b39a
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Adjust some comments and function names that refer to the ancient
so_list type.
Update some other stale comments in solib-svr4.c at the same time.
Change-Id: Ia42efa6554d0cc6abb4183b6bffc96c6358c5735
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
The `so_` prefix is unnecessary here, it's already clear by the fact
that they are field of the solib type (and previously so_list).
Change-Id: I2c6773afc121d7631901e602913ea8a068840d0b
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
With a hello world a.out, and using the compiler flags from target board
dwarf5-fission-debug-types:
...
$ gcc -gdwarf-5 -fdebug-types-section -gsplit-dwarf ~/data/hello.c
...
I run into:
...
$ gdb -q -batch a.out
terminate called after throwing an instance of 'gdb_exception_error'
...
What happens is that an error is thrown due to invalid dwarf, but the error is
not caught, causing gdb to terminate.
In a way, this is a duplicate of PR32861, in the sense that we no longer run
into this after:
- applying the proposed patch (work around compiler bug), or
- using gcc 9 or newer (compiler bug fixed).
But in this case, the failure mode is worse than in PR32861.
Fix this by catching the error in
cooked_index_worker_debug_info::process_skeletonless_type_units.
With the patch, we get instead:
...
$ gdb -q -batch a.out
Offset from DW_FORM_GNU_str_index or DW_FORM_strx pointing outside of \
.debug_str.dwo section in CU at offset 0x0 [in module a.out]
...
While we're at it, absorb the common use of
cooked_index_worker_result::note_error:
...
try
{
...
}
catch (gdb_exception &exc)
{
(...).note_error (std::move (exc));
}
...
into the method and rename it to catch_error, resulting in more compact code
for the fix:
...
(...).catch_error ([&] ()
{
...
});
...
While we're at it, also use it in
cooked_index_worker_debug_info::process_units which looks like it needs the
same fix.
Tested on x86_64-linux.
PR symtab/32979
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32979
When I run GDB under Valgrind, GDB seems to segfault
displaying:
Fatal signal: Segmentation fault
----- Backtrace -----
0x2803f7 ???
0x3c9696 ???
0x3c9899 ???
0x55f8fcf ???
0x486c000 ???
---------------------
A fatal error internal to GDB has been detected, further
debugging is not possible. GDB will now terminate.
This is a bug, please report it. For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.
warning: linux_ptrace_test_ret_to_nx: PC 0x5821c09d is neither near return address 0x486c000 nor is the return instruction 0x4f8f4a!
but then, acts like nothing happened and excutes normally. This is
because it's the child from linux_ptrace_test_ret_to_nx that
segfaults and parent GDB carries on normally. Restore the original
signal states to not to print confusing backtrace. After restoring,
only such warning is displayed:
warning: linux_ptrace_test_ret_to_nx: WSTOPSIG 19 is neither SIGTRAP nor SIGSEGV!
Specifying --gdwarf-5 with a source lacking a ".file 0" directive
results in this segfault.
* dwarf2dbg.c (out_debug_str): Use files[1] if files[0] is
empty regardless of dwarf level.
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 db4ab410de I rewrote OP_STORE handling to support writing
near the end of a section. The rewrite had some bugs, fixed in commit
3e02c4891d. 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
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.
