Sync with the list of flags from gdbsupport, and add a few more of
our own to catch recent issues. Comment out the C++-specific flags
as we don't build with C++.
The test gdb.base/watchpoint.exp has a proc named 'test_stepping'
which claims to "Test stepping and other mundane operations with
watchpoints enabled". It sets a watchpoint on ival2, performs an
inferior function call (which is not at all mundane), and uses 'next',
'until', and, finally, does a 'step'.
However, that final 'step' command steps to (but not over/through) the
line at which the assignment to ival2 takes place. At no time while
performing these operations is a watchpoint hit.
This commit adds a test to see what happens when stepping over/through
the assignment to ival2. The watchpoint on ival2 should be triggered
during this step. I've added another 'step' to make sure that the
correct statement is reached after performing the watchpoint-hitting
step.
After running the 'test_stepping' proc, gdb.base/watchpoint.exp does
a clean_restart before doing further tests, so nothing depends upon
'test_stepping' to stop at the particular statement at which it had
been stopping.
I've examined all tests which set watchpoints and step. I haven't
been able to identify a(nother) test case which tests what happens
when stepping over/through a statement which triggers a watchpoint.
Therefore, adding these new 'step' tests is testing something which
hasn't being tested elsewhere.
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
This was split over multiple lines and ended up missing a space.
Reported-by: David Abdurachmanov <davidlt@rivosinc.com>
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
I noticed it when I was trying to set a breakpoint at ExitProcess:
```
(gdb) b ExitProcess
Breakpoint 1 at 0x14001fdd0
(gdb) r
Starting program: C:\qiewer\heob\heob64.exe
Warning:
Cannot insert breakpoint 1.
Cannot access memory at address 0x3dbf4120
Cannot insert breakpoint 1.
Cannot access memory at address 0x77644120
```
The problem doesn't exist in gdb 13.2, and the difference can easily be
seen when printing ExitProcess.
gdb 14.1:
```
(gdb) p ExitProcess
$1 = {<text variable, no debug info>} 0x77644120 <UserHandleGrantAccess+36128>
```
gdb 13.2:
```
(gdb) p ExitProcess
$1 = {<text variable, no debug info>} 0x77734120 <ntdll!RtlExitUserProcess>
```
The new behavior started with 9675da2535,
where VMA was then calculated relative to FORWARD_DLL_NAME, while it was
relative to DLL_NAME before.
Fixed by calculating VMA relative to DLL_NAME again.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31112
Approved-By: Tom Tromey <tom@tromey.com>
The previous patch fixed the immediate performance problem with Ada
name matching, by having a subset of matches call
expand_symtabs_matching rather than expand_matching_symbols. However,
it seemed to me that expand_matching_symbols should not be needed at
all.
To achieve this, this patch changes ada_lookup_name_info::split_name
to use the decoded name, rather than the encoded name. In order to
make this work correctly, a new decoded form is used: one that does
not decode operators (this is already done) and also does not decode
wide characters. The latter change is done so that changes to the Ada
source charset don't affect the DWARF index.
With this in place, we can change ada-lang.c to always use
expand_symtabs_matching rather than expand_matching_symbols.
A user reported that certain operations -- like printing a large
structure -- could be slow. I tracked this down to
ada-lang.c:map_matching_symbols taking an inordinate amount of time.
Specifically, calls like the one to look for a parallel "__XVZ"
variable, in ada_to_fixed_type_1, could result in gdb walking over all
the entries in the cooked index over and over.
Looking into this reveals that
cooked_index_functions::expand_matching_symbols is not written
efficiently -- it ignores its "ordered_compare" parameter. While
fixing this would be good, it turns out that this entire method isn't
needed; so this series removes it.
However, the deletion is not done in this patch. This one, instead,
fixes the immediate cause of the slowdown, by using
objfile::expand_symtabs_matching when possible. This approach is
faster because it is more selective about which index entries to
examine.
Changes introduced by commit 9e8915c6ce
caused a regression that meant hardware watchpoint stops would not
trigger in reverse execution or replay mode. This was documented in
PR breakpoints/21969.
The problem is that record_check_stopped_by_breakpoint always overwrites
record_full_stop_reason, thus loosing the TARGET_STOPPED_BY_WATCHPOINT
value which would be checked afterwards.
This commit fixes that by not overwriting the stop-reason in
record_full_stop_reason if we're not stopped at a breakpoint.
And the test for hw watchpoints in gdb.reverse/watch-reverse.exp actually
tested sw watchpoints again, since "set can-use-hw-watchpoints 1"
doesn't convert enabled watchpoints to use hardware.
This is fixed by disabling said watchpoint while enabling hw watchpoints.
The same is not done for gdb.reverse/watch-precsave.exp, since it's not
possible to use hw watchpoints in restored recordings anyways.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=21969
Approved-by: Guinevere Larsen <blarsen@redhat.com>
After this commit:
commit 33ae45434d
Date: Mon Dec 4 14:23:17 2023 +0000
gdb: Enable early init of thread pool size
I am now seeing this assert from libstdc++:
/usr/include/c++/9/bits/stl_algo.h:3715: constexpr const _Tp& std::clamp(const _Tp&, const _Tp&, const _Tp&) [with _Tp = int]: Assertion '!(__hi < __lo)' failed.
This may only be visible because I compile with:
-D_GLIBCXX_DEBUG=1 -D_GLIBCXX_DEBUG_PEDANTIC=1
but I haven't checked. The issue the assert is highlighting is real,
and is caused by this block of code:
if (n_threads < 0)
{
const int hardware_threads = std:🧵:hardware_concurrency ();
/* Testing in #29959 indicates that parallel efficiency drops between
n_threads=5 to 8. Therefore, clamp the default value to 8 to avoid an
excessive number of threads in the pool on many-core systems. */
const int throttle = 8;
n_threads = std::clamp (hardware_threads, hardware_threads, throttle);
}
The arguments to std::clamp are VALUE, LOW, HIGH, but in the above, if
we have more than 8 hardware threads available the LOW will be greater
than the HIGH, which is triggering the assert I see above.
I believe std::clamp is the wrong tool to use here. Instead std::min
would be a better choice; we want the smaller value of
HARDWARE_THREADS or THROTTLE. If h/w threads is 2, then we want 2,
but if h/w threads is 16 we want 8, this is what std::min gives us.
After this commit, I no longer see the assert.
This reverts commit 1c04f72368 ("[gdb/symtab] Fix assert in set_length"), due
to a regression reported in PR29572, and implements a different fix for PR29453.
The fix is to not use the CU table in a .debug_names section to construct
all_units, but instead use create_all_units, and then verify the CU
table from .debug_names. This also fixes PR25969, so remove the KFAIL.
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29572
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25969
Prior to commit 0e3c1eebb2 nm output depended on the host unsigned
long when printing "negative" symbol values for 32-bit targets.
Commit 0e3c1eebb2 made the output match that seen with a 64-bit host
unsigned long. The fact that nm output changed depending on host is
of course a bug, but it is reasonable to expect 32-bit target output
is only 32 bits. So this patch makes 32-bit target output the same as
it was on 32-bit hosts prior to 0e3c1eebb2.
PR 31096
* nm.c (print_format_string): Make it a static buffer.
(get_print_format): Merge into..
(set_print_format): ..this, renamed from set_print_width. When
print_width is 32, set up print_format_string for an int32_t
value. Don't malloc print_format_string. Adjust calls.
(print_value): Correct printing of 32-bit values.
Tobias reported on IRC that the linker fails to build with GCC 4.8.5.
In configure I've tried to use everything actually used in the sha1.c
x86 hw implementation, but unfortunately I forgot about implicit function
declarations. GCC before 7 did have <cpuid.h> header and bit_SHA define
and __get_cpuid function defined inline, but it didn't define
__get_cpuid_count, which compiled fine (and the configure test is
intentionally compile time only) due to implicit function declaration,
but then failed to link when linking the linker, because
__get_cpuid_count wasn't defined anywhere.
The following patch fixes that by using what autoconf uses in AC_CHECK_DECL
to make sure the functions are declared.
2023-12-05 Jakub Jelinek <jakub@redhat.com>
* configure.ac (HAVE_X86_SHA1_HW_SUPPORT): Verify __get_cpuid and
__get_cpuid_count are not implicitly declared.
* configure: Regenerated.
On mingw targets it's possible that there are multiple trampoline
symbols for __cxa_throw, in each module where a throw is done, but
without a corresponding global symbol.
Since commit 77f2120b20 they cancel each
other out in search_minsyms_for_name, which makes it impossible to set
a breakpoint there:
(gdb) b __cxa_throw
Function "__cxa_throw" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 2 (__cxa_throw) pending.
(gdb) c
Continuing.
[Inferior 1 (process 10004) exited with code 03]
With catch throw it also doesn't work, and you don't even get an error
message:
(gdb) catch throw
Catchpoint 2 (throw)
(gdb) c
Continuing.
[Inferior 1 (process 5532) exited with code 03]
(gdb)
The fix is to simply ignore other trampoline symbols when looking for
corresponding global symbols, and it's working as expected:
(gdb) b __cxa_throw
Breakpoint 2 at 0x13f091590 (2 locations)
(gdb) c
Continuing.
Breakpoint 2.1, 0x000000013f091590 in __cxa_throw ()
(gdb)
And catch throw also works again:
(gdb) catch throw
Catchpoint 2 (throw)
(gdb) c
Continuing.
Catchpoint 2.1 (exception thrown), 0x000000013f181590 in __cxa_throw ()
(gdb)
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29548
Approved-By: Tom Tromey <tom@tromey.com>
Commit 33ae45434d updated the text reported by GDB when showing the
number of worker threads. However, it neglected to update the assertions
using this text, which caused index-file.exp to fail. This commit
corrects this omission.
Tested index-file.exp is fixed on my local machine.
Approved-By: Tom Tromey <tom@tromey.com>
PR 31106
* elfcode.h (elf_write_relocs): Do not convert a relocation against a zero-value absolute symbol into a relocation without a symbol if the symbol is being used for a complex relocation.
compute_delayed_physnames does this:
size_t len = strlen (physname);
...
if (physname[len] == ')') /* shortcut */
break;
However, physname[len] will always be \0.
This patch changes it to the correct len-1.
Fix some of the sim_fpu calls to use the right types. While I'm
not familiar with the MIPS ISA in these cases, these look like
simple oversights due to the name/type mismatches. This at least
fixes compiling with -Wenum-conversion.
No functional change here, but makes it a little easier to read the
generated code when editors aren't highlighting all the spurious
trailing whitespace on lines.
The sim_stop argument is an enum and should only be one of those
values, not a signal constant. Fix the logic to pass the right
sim_xxx & SIM_xxx values in the right arguments.
We only support UTF-8 nowadays, so stop using ISO-8859-1.
Maybe we should delete this logic entirely, but for now,
do the bare min conversion to keep it compiling.
This is aimed at fixing holes in two alpha-ecoff relocation functions
that access section contents without first bounds checking offsets.
I've also rewritten ALPHA_R_OP_STORE handling to support writing to
the bytes near the end of the section.
* coff-alpha.c (alpha_ecoff_get_relocated_section_contents): Don't
bother checking ALPHA_R_LITERAL insn. Range check before reading
contents for ALPHA_R_GPDISP, and simplify handling. Rewrite
ALPHA_R_OP_STORE handling. Correct error callback args.
(alpha_relocate_section): Similarly. Don't abort, report errors.
In commit 7ac6d0c38c I made more use of free_contents in
_bfd_elf_slurp_version_tables, a variable added to tag the case where
raw verneed and verdefs have been read locally by the function, and
thus should be freed before returning. In retrospect it may have been
better to do without the extra variable entirely. It's easy to infer
when "contents" should be freed, costing a little extra on an error
path but costing less elsewhere.
* elf.c (_bfd_elf_slurp_version_tables): Don't use free_contents.
This adds the efi target name handling for riscv64 to objcopy.
binutils:
* binutils/objcopy.c: add riscv64 handling to
convert_efi_target()
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Reuse the bfd/development.sh script like most other project to
determine whether the current source tree is a dev build (e.g.
git) or a release build, and disable the warnings for releases.
This code tries to use attach_type enums as hw_phb_decode, and while
they're setup to have compatible values, the compiler doesn't like it
when the cast is missing. So cast it explicitly and then use that.
sim/ppc/hw_phb.c:322:28: error:
implicit conversion from enumeration type 'attach_type'
(aka 'enum _attach_type') to different enumeration type
'hw_phb_decode' [-Werror,-Wenum-conversion]
Don't conflate HAVE_GETRUSAGE & HAVE_SYS_RESOURCE_H. Use the latter
to include the header and nothing else. Use the former to determine
whether to use the function and nothing else. If we find a system
that doesn't follow POSIX and provides only one of these, we can
figure out how to support it then. The manual local definition is
clashing with the system ones and leading to build failures with
newer C standards.
sim/ppc/emul_netbsd.c:51:5: error: a function declaration without a
prototype is deprecated in all versions of C and is treated as a
zero-parameter prototype in C2x, conflicting with a previous
declaration [-Werror,-Wdeprecated-non-prototype]
aarch64-elf fails the ld-aarch64/bfd-far-3.d test, due to the stubs
being emitted in a different order to that of aarch64-linux. They are
emitted in a different order due to stub names for local symbols
having the section id in the stub name. aarch64-linux-ld generates
one more section than aarch64-elf-ld. That section is .gnu.hash. So
the stub names differ and are hashed to different slots in
stub_hash_table.
Fix this by running the test with --hash-style=sysv, and adjust
expected output. I've also changed the branch over stubs emitted at
the start of a group of stubs to not care about the symbol, for all
groups not just the one that needed changing.
* ld-aarch64/bti-far-3.d: Add --hash-style=sysv. Adjust
expected output.
In the commit:
commit 4793f551a5
Date: Mon Nov 27 13:33:17 2023 +0000
gdb: allow use of ~ in 'save gdb-index' command
I added a test which has a directory name within the GDB command,
which then appears in the test name as I failed to give the test a
better name.
Fixed in this commit.
'@' is a special symbol meaning 'command' in GNU texinfo.
If the GDBINIT or GDBINIT_DIR path during configuration
included an '@' character, the makeinfo command would fail,
as it interpreted the '@' in the path as a start of a command
when expanding the path in the docs.
This patch simply escapes any '@' characters in the path,
by replacing them with '@@'. This was already done for the
bugurl variable.
This was detected because the 'Jenkins' tool sometimes puts
an '@' in the workspace path.
Approved-By: Tom Tromey <tom@tromey.com>
While working on gdb's .debug_names writer, I found a couple of small
bugs in binutils .debug_names dumping.
First, the DWARF spec (section 6.1.1.4.6 Name Table) says:
These two arrays are indexed starting at 1, [...]
I think it is clearer for binutils to follow this, particularly
because DW_IDX_parent refers to this number.
Second, I think the handling of an empty hash table is slightly wrong.
Currently the dumping code assumes there is always an array of hashes.
However, section 6.1.1.4.5 Hash Lookup Table says:
The optional hash lookup table immediately follows the list of
type signatures.
and then:
The hash lookup table is actually two separate arrays: an array of
buckets, followed immediately by an array of hashes.
My reading of this is that the hash table as a whole is optional, and
so the hashes will not exist in this case. (This also makes sense
because the hashes are not useful without the buckets anyway.)
This patch fixes both of these problems. FWIW I have some gdb patches
in progress that change gdb both to omit the hash table and to use
DW_IDX_parent.
2023-12-04 Tom Tromey <tom@tromey.com>
* dwarf.c (display_debug_names): Handle empty .debug_names hash
table. Name entries start at 1.
Add support for jump visualization for the s390 architecture in
disassembly:
objdump -d --visualize-jumps ...
Annotate the (conditional) jump and branch relative instructions with
information required for jump visualization:
- jump: Unconditional jump / branch relative.
- condjump: Conditional jump / branch relative.
- jumpsr: Jump / branch relative to subroutine.
Unconditional jump and branch relative instructions are annotated as
jump.
Conditional jump and branch relative instructions, jump / branch
relative on count/index, and compare and jump / branch relative
instructions are annotated as condjump.
Jump and save (jas, jasl) and branch relative and save (bras, brasl)
instructions are annotated as jumpsr (jump to subroutine).
Provide instruction information required for jump visualization during
disassembly.
The instruction type is provided after determining the opcode.
For non-code it is set to dis_noninsn. Otherwise it defaults to
dis_nonbranch. No annotation is done for data reference instructions
(i.e. instruction types dis_dref and dis_dref2). Note that the
instruction type needs to be provided before printing of the
instruction, as it is used in print_address_func() to translate the
argument value into an address if it is assumed to be a PC-relative
offset. Note that this is never the case on s390, as
print_address_func() is only called with addresses and never with
offsets.
The target of the (conditional) jump and branch relative instructions
is provided during print, when the PC relative operand is decoded.
include/
* opcode/s390.h: Define opcode flags to annotate instruction
class information for jump visualization:
S390_INSTR_FLAG_CLASS_BRANCH, S390_INSTR_FLAG_CLASS_RELATIVE,
S390_INSTR_FLAG_CLASS_CONDITIONAL, and
S390_INSTR_FLAG_CLASS_SUBROUTINE.
Define opcode flags mask S390_INSTR_FLAG_CLASS_MASK for above
instruction class information.
Define helpers for common instruction class flag combinations:
S390_INSTR_FLAGS_CLASS_JUMP, S390_INSTR_FLAGS_CLASS_CONDJUMP,
and S390_INSTR_FLAGS_CLASS_JUMPSR.
opcodes/
* s390-mkopc.c: Add opcode flags to annotate information
for jump visualization: jump, condjump, and jumpsr.
* s390-opc.txt: Annotate (conditional) jump and branch relative
instructions with information for jump visualization.
* s390-dis.c (print_insn_s390, s390_print_insn_with_opcode):
Provide instruction information for jump visualization.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
Reviewed-by: Andreas Krebbel <krebbel@linux.ibm.com>
I noticed that gdbserver/win32-low.cc has a fall-through comment that
should have been converted to use the annotation instead.
This patch makes the change.
This commit enables the early initialization commands (92e4e97a9f) to
modify the number of threads used by gdb's thread pool.
The motivation here is to prevent gdb from spawning a detrimental number
of threads on many-core systems under environments with restrictive
ulimits.
With gdb before this commit, the thread pool takes the following sizes:
1. Thread pool size is initialized to 0.
2. After the maintenance commands are defined, the thread pool size is
set to the number of system cores (if it has not already been set).
3. Using early initialization commands, the thread pool size can be
changed using "maint set worker-threads".
4. After the first prompt, the thread pool size can be changed as in the
previous step.
Therefore after step 2. gdb has potentially launched hundreds of threads
on a many-core system.
After this change, step 2 and 3 are reversed so there is an opportunity
to set the required number of threads without needing to default to the
number of system cores first.
There does exist a configure option (added in 261b07488b) to disable
multithreading, but this does not allow for an already deployed gdb to
be configured.
Additionally, the default number of worker threads is clamped at eight
to control the number of worker threads spawned on many-core systems.
This value was chosen as testing recorded on bugzilla issue 29959
indicates that parallel efficiency drops past this point.
GDB built with GCC 13.
No test suite regressions detected. Compilers: GCC, ACfL, Intel, Intel
LLVM, NVHPC; Platforms: x86_64, aarch64.
The scenario that interests me the most involves preventing GDB from
spawning any worker threads at all. This was tested by counting the
number of clones observed by strace:
strace -e clone,clone3 gdb/gdb -q \
--early-init-eval-command="maint set worker-threads 0" \
-ex q ./gdb/gdb |& grep --count clone
The new test relies on "gdb: install CLI uiout while processing early
init files" developed by Andrew Burgess. This patch will need pushing
prior to this change.
The clamping was tested on machines with both 16 cores and a single
core. "maint show worker-threads" correctly reported eight and one
respectively.
Approved-By: Tom Tromey <tom@tromey.com>
