Alan Modra [Thu, 20 Feb 2025 01:19:25 +0000 (11:49 +1030)]
PR 32721, internal error in tc-i386.c:parse_register
pr30117 showed one of the assertions added by 4d1bb7955a8b was too
strict. oss-fuzz also found the second assertion to be too strict,
with this testcase distilled from 7k of garbage source:
A=%eax%%!
Y=A
Z=A
or $6,Z
PR 32721
* config/tc-i386.c (parse_register): Move "know" into
condition. Simplify.
Tom Tromey [Sat, 8 Feb 2025 02:14:35 +0000 (19:14 -0700)]
Hoist language-finding in expand_symtabs_matching
Right now, cooked_index_functions::expand_symtabs_matching computes
the language for each component of a split name, using the language of
the corresponding entry.
Instead, I think that we want to do all the comparisons using the
final entry's language. I don't think there's a way to trigger bad
behavior here right now, but with another series I'm working on, we
end up with some entries whose language can't reliably be determined;
and in this case using the final entry's language avoids issues.
I suspect we could also dispense with the per-segment name-matcher
lookup as well.
Tom Tromey [Fri, 31 Jan 2025 18:25:45 +0000 (11:25 -0700)]
Move producer checks to dwarf2_cu
This changes the various producer-checking functions to be methods on
dwarf2_cu. It adds a few new caching members as well -- every one
that could reasonably be done this way has been converted, with the
only exception being a gdbarch hook.
Note the new asserts in the accessors. Without the earlier
prepare_one_comp_unit change, these could trigger in some modes.
Tom Tromey [Fri, 31 Jan 2025 19:25:05 +0000 (12:25 -0700)]
Clean up calls to prepare_one_comp_unit
Currently, prepare_one_comp_unit is called somewhat haphazardly: it is
mostly called when a CU is read, but some places manage to instantiate
a cutu_reader* without calling it, and some code (e.g.,
read_file_scope) calls it without really needing to.
Aside from contributing to the general confusion around CU reading,
this doesn't really cause problems in the current tree. However, it
is possible for the DWARF reader to check the CU's producer before it
is ever set -- which is certainly unintended.
Tom Tromey [Sat, 8 Feb 2025 19:43:21 +0000 (12:43 -0700)]
Clean up DW_TAG_namelist handling in new_symbol
In dwarf2/read.c:new_symbol, DW_TAG_namelist is listed in the same
part of the "switch" as other tags. However, it effectively shares no
code with these. This patch splits it into its own case.
Longer term I think new_symbol should be split up drastically.
Before this patch, current_token was a global implicitly 0-initialized. Since
it is now a class field, it is not 0-initialized by default anymore. This
patch changes this.
Change-Id: I3f00b080318a70405d881ff0abe02b2c5cb1f9d8 Approved-By: Simon Marchi <simon.marchi@efficios.com> Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 17 Feb 2025 20:54:09 +0000 (15:54 -0500)]
gdb/dwarf: add logging for CU expansion
I was trying to get an understanding of which CUs were expanded when,
and how much time it was taking. I wrote this patch to add some logging
related to that, and I think it would be useful to have upstream, to
better understand performance problems related to over-eager CU
expansion, for example.
- add DWARF_READ_SCOPED_DEBUG_START_END
- use it in process_queue, to wrap the related expansion messages
together
- add a message in maybe_queue_comp_unit when enqueuing a comp unit
- add timing information to messages in process_queue, indicating how
much time it took to expand a given symtab
- count the number of expansions done in a single call to process_queue
[dwarf-read] process_queue: start: Expanding one or more symtabs of objfile /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw-form-ref-addr-with-type-units/dw-form-ref-addr-with-type-units ...
[dwarf-read] process_queue: Expanding symtab of CU at offset 0xc
[dwarf-read] maybe_queue_comp_unit: Queuing CU for expansion: section offset = 0x38b, queue size = 2
[dwarf-read] process_queue: Done expanding CU at offset 0xc, took 0.001s
[dwarf-read] process_queue: Expanding symtab of CU at offset 0x38b
[dwarf-read] process_queue: Done expanding CU at offset 0x38b, took 0.000s
[dwarf-read] process_queue: Done expanding 2 symtabs.
[dwarf-read] process_queue: end: Expanding one or more symtabs of objfile /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw-form-ref-addr-with-type-units/dw-form-ref-addr-with-type-units ...
Change-Id: I5237d50e0c1d06be33ea83a9120b5fe1cf7ab8c2 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 17 Feb 2025 19:59:35 +0000 (14:59 -0500)]
gdb/dwarf: set is_debug_types in signatured_type constructor
This makes it more obvious that all created signatured_type objects have
this flag set.
Also, remove an unnecessary assignment in create_cus_hash_table: when
constructing the dwarf2_per_cu_data object, is_debug_types is already
initialized to 0/false.
Change-Id: I6d28b17ac77edc040172254f6970d05ebc4a47f4 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 17 Feb 2025 19:59:32 +0000 (14:59 -0500)]
gdb/dwarf: pass dwarf2_per_bfd to dwarf2_per_cu_data constructor
Pass a dwarf2_per_bfd to the constructor of dwarf2_per_cu_data and set
the per_bfd field there. All "real" instantiations of
dwarf2_per_cu_data must have a valid, non-nullptr dwarf2_per_bfd
backlink, this makes it a bit more obvious. The instantiations of
dwarf2_per_cu_data that receive a nullptr dwarf2_per_bfd are the ones
used to do hash map lookups and the ones used in selftests.
Remove an unnecessary assignment of per_bfd in
fill_in_sig_entry_from_dwo_entry: the per_bfd field is already set when
the signatured_type object is constructor (before that, it was set in
allocate_signatured_type).
Change-Id: Ifeebe55fdb1bc2de4de9c852033fafe8abdfde8a Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 17 Feb 2025 19:59:31 +0000 (14:59 -0500)]
gdb/dwarf: change some functions from "per objfile" to "per bfd"
I noticed that the following functions accept a "dwarf2_per_objfile",
but they can actually accept a less specific "dwarf2_per_bfd". This
makes it more obvious that the work they do is per BFD and not per
objfile.
Qwinci [Tue, 18 Feb 2025 18:47:51 +0000 (20:47 +0200)]
gdb/remote: don't error if qGetTIBAddr is unsupported
This change makes it possible to debug PE executables run in e.g. Qemu
without needing to set osabi to none, it breaks backtrace
and commands like finish if frame pointers are not present but SEH unwind info is.
Hui Li [Tue, 11 Feb 2025 12:18:28 +0000 (20:18 +0800)]
gdb: LoongArch: Extend the maximum number of hardware watchpoints
The maximum number of load/store watchpoints and fetch instruction
watchpoints is 14 each according to LoongArch Reference Manual [1],
so extend the maximum number of hardware watchpoints from 8 to 14.
A new struct user_watch_state_v2 was added into uapi in the related
kernel commit 531936dee53e ("LoongArch: Extend the maximum number of
watchpoints") [2], but there may be no struct user_watch_state_v2 in
the system header in time. Modify the struct loongarch_user_watch_state
in GDB which is same with the uapi struct user_watch_state_v2.
As far as I can tell, the only users for this struct in the userspace
are GDB and LLDB, there are no any problems of software compatibility
between the application and kernel according to the analysis.
The compatibility problem has been considered while developing and
testing. When the applications in the userspace get watchpoint state,
the length will be specified which is no bigger than the sizeof struct
user_watch_state or user_watch_state_v2, the actual length is assigned
as the minimal value of the application and kernel in the generic code
of ptrace:
For example, there are four kind of combinations, all of them work well.
(1) "older kernel + older app", the actual length is 8+(8+8+4+4)*8=200;
(2) "newer kernel + newer app", the actual length is 8+(8+8+4+4)*14=344;
(3) "older kernel + newer app", the actual length is 8+(8+8+4+4)*8=200;
(4) "newer kernel + older app", the actual length is 8+(8+8+4+4)*8=200.
BTW, LLDB also made this change in the related commit ff79d83caeee
("[LLDB][LoongArch] Extend the maximum number of watchpoints") [3]
Alan Modra [Wed, 19 Feb 2025 12:15:29 +0000 (22:45 +1030)]
binutils/dwarf.c debug_information leak
It is possible with fuzzed files to have num_debug_info_entries zero
after allocating space for debug_information, leading to multiple
allocations.
* dwarf.c (process_debug_info): Don't test num_debug_info_entries
to determine whether debug_information has been allocated,
test alloc_num_debug_info_entries.
gdbserver, remote: introduce "id_str" in the "qXfer:threads:read" XML
GDB prints the target id of a thread in various places such as the
output of the "info threads" command in the "Target Id" column or when
switching to a thread. A target can define what to print for a given
ptid by overriding the `pid_to_str` method.
The remote target is a gateway behind which one of many various
targets could be running. The remote target converts a given ptid to
a string in a uniform way, without consulting the low target at the
server-side.
In this patch we introduce a new attribute in the XML that is sent in
response to the "qXfer:threads:read" RSP packet, so that a low target
at the server side, if it wishes, can specify what to print as the
target id of a thread.
Note that the existing "name" attribute or the "extra" text provided
in the XML are not sufficient for the server-side low target to
achieve the goal. Those attributes, when present, are simply appended
to the target id by GDB.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org> Approved-By: Simon Marchi <simon.marchi@efficios.com>
testsuite, mi: prevent buffer overflow in get_mi_thread_list
If there is a large number of threads in the input program, the expect
buffer in `get_mi_thread_list` would become full. Prevent this by
consuming the buffer in small pieces.
Regression-tested using the gdb.mi tests.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom de Vries [Tue, 18 Feb 2025 07:47:33 +0000 (08:47 +0100)]
[gdb/testsuite] Don't start gdb in gdb.base/gstack.exp
In test-case gdb.base/gstack.exp we start a gdb implicitly using
prepare_for_testing.
The gdb is not really used, but its spawn_id (available in variable
gdb_spawn_id) is used in a gdb_test_multiple, which is used to interact with
the gstack process.
Usually, a running gdb is cleaned up at test-case exit in gdb_finish, which
calls gdb_exit, which by default calls gdb_default_exit, which does
'send_gdb "quit\n"'.
However, this sends a quit to the host process expect is currently talking to,
defined by board_info(host,fileid), and after spawning gstack that's gstack, not
gdb.
Fix this by:
- using build_executable instead of prepare_for_testing to not spawn an unused
gdb, and
- changing the gdb_test_multiple into a gdb_expect, eliminating the implicit use
of gdb_spawn_id.
Tested on x86_64-linux.
Reviewed-By: Keith Seitz <keiths@redhat.com>
PR testsuite/32709
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32709
Alan Modra [Sun, 16 Feb 2025 22:55:30 +0000 (09:25 +1030)]
bfd_set_section_alignment errors
I noticed when making the change from "einfo" to "fatal" that the
alignment error in _bfd_elf_link_create_gnu_property_sec lacked a %P,
and then decided that a bfd_set_section_alignment that can't happen
does not merit a separate error message. elfxx-x86.c had copied the
same code, so fix that too. In fact, every bfd_set_section_alignment
call in elfxx-x86.c will always return true absent some future
programming error. This patch makes those that accompany making a
section lose their "failed to align " error and share the "failed to
create" error. Those that are changing alignment of a section created
elsewhere now abort on bfd_set_section_alignment returning false.
Alan Modra [Sun, 16 Feb 2025 13:04:55 +0000 (23:34 +1030)]
PR 32603, more ld -w misbehaviour
Commit 8d97c1a53f3d claimed to replace all einfo calls using %F with
a call to fatal. It did so only for the ld/ directory. This patch
adds a "fatal" to linker callbacks, and replaces those calls in bfd/
too.
Andrew Oates [Sun, 16 Feb 2025 15:16:25 +0000 (16:16 +0100)]
gdb: fix color_option_def compile error (clang)
color_option_def was added in commit 6447969d0 ("Add an option with a
color type."), but not used.
The color_option_def constructor passes the wrong number of arguments
to the option_def constructor. Since color_option_def is a template and
never actually instantiated, GCC does not fail to compile this. clang
generates an error (see below).
This passes nullptr to the extra_literals_ option_def ctor argument,
which matches what filename_option_def above it does.
clang's generated error:
../../gdb/cli/cli-option.h:343:7: error: no matching constructor for initialization of 'option_def'
: option_def (long_option_, var_color,
^ ~~~~~~~~~~~~~~~~~~~~~~~~
../../gdb/cli/cli-option.h:50:13: note: candidate constructor not viable: requires 8 arguments, but 7 were provided
constexpr option_def (const char *name_,
^
../../gdb/cli/cli-option.h:37:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 7 were provided
struct option_def
^
../../gdb/cli/cli-option.h:37:8: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 7 were provided
Alan Modra [Sat, 15 Feb 2025 06:36:58 +0000 (17:06 +1030)]
PR32698, potential null pointer dereference in tekhex.c
PR 32698
* tekhex.c (find_chunk): Remove unnecessary casts.
(insert_byte): Check and return status from find_chunk.
(move_section_contents): Likewise.
(tekhex_get_section_contents, tekhex_set_arch_mach): Return
status from move_section_contents.
(first_phase): Check and return status from first_phase.
Alan Modra [Fri, 14 Feb 2025 12:40:29 +0000 (23:10 +1030)]
riscv disassembler leak
Commit 3f61a38b5e81 moved the disassembler subset_list from a static
variable to disassembler private_data. It is now malloc'd in
riscv_init_disasm_info so should be freed when disassemble_free_target
runs.
Anghelo Carvajal [Sat, 15 Feb 2025 01:30:58 +0000 (01:30 +0000)]
MIPS objdump: Add `eabi32` and `eabi64` ABI options
Extend gpr and fpr register names with names suitable for both EABIs.
Heavily inspired by the EABI documenation written by Eric Christopher,
which can be read at
https://sourceware.org/legacy-ml/binutils/2003-06/msg00436.html
MIPS/GAS/testsuite: Reuse n64 GPR disassembly for n32
The MIPS ABI register names are the same between n64 and n32, so remove
duplication and use n64 GPR disassembly output for the n32 test as well.
The tests were developed long before we gained output reuse support.
MIPS/GAS: Set default CPU to MIPS64r6 for 64-bit "img" configurations
Fix broken commit 070961b377b3 ("MIPS: Set r6 as default arch if vendor
is img") that sets up GAS in an inconsistent way where "img" vendor has
been used with a 64-bit configuration, such as `mips64-img-linux-gnu'.
In that case GAS is set up to use a 64-bit ABI by default combined with
the MIPS32r6 CPU, which is 32-bit.
Consequently GAS always fails to assemble even trivial input, producing
a message such as:
Assembler messages:
Error: -march=mips32r6 is not compatible with the selected ABI
.../gas/testsuite/gas/all/nop.s:2: Error: `gp=32' used with a 64-bit ABI
unless the defaults have been suitably overridden either for the ABI or
the CPU.
Set the default CPU to MIPS64r6 for 64-bit "img" vendor configurations
then and adjust the GAS testsuite accordingly, removing 1048 FAIL and 3
ERROR regression test results for the `mips64-img-linux-gnu' and
`mips64el-img-linux-gnu' targets each.
MIPS/GAS/testsuite: Support negated targets for default architecture
Add support for giving negated targets in the list of targets passed to
`mips_arch_create' for the purpose of setting the default architecture.
This is so that a subset of targets can be excluded from matching within
a broader set of targets.
Ivan Kokshaysky [Sat, 15 Feb 2025 01:30:58 +0000 (01:30 +0000)]
alpha, ld: remove -taso option
The -taso switch was quite useful 25 years ago for porting 32-bit
code with broken integer-pointer casting. Not anymore. The EF_ALPHA_32BIT
Linux support is going to be dropped in kernel v6.14 [1], NetBSD and OpenBSD
never had it, so there is no point in keeping the -taso option around.
Also remove alpha special case that uses -taso from gdb.base/dump.exp
in gdb testsuite.
Andrew Burgess [Thu, 13 Feb 2025 16:43:39 +0000 (16:43 +0000)]
gdb/testsuite: clean ups in gdb.python/py-source-styling.exp
The top comment in gdb.python/py-source-styling.exp was completely
wrong, clearly a cut&paste job from elsewhere. Write a comment that
actually reflects what the test does.
I've also moved the allow_python_tests check earlier in the file.
And I changed some 'return -1' into just 'return'. I'm not aware that
the '-1' adds any value.
I also folded a 'pass $gdb_test_name' into the preceding gdb_assert,
which I think is neater.
There is no change in what is actually being tested after this commit.
Andrew Burgess [Thu, 6 Feb 2025 15:02:37 +0000 (15:02 +0000)]
gdb/tui: use maybe_update for source centring in an extra case
I noticed that, with recent versions of GDB, when the TUI is enabled
before the inferior is started, the source code display is not as
helpful as it used to be. Here's a simple test program being
displayed using GDB 15.2, at this point the inferior has not started,
all I've done is 'tui enable':
gdb: adjust the default place of 'list' to main's prologue
I don't think the new behaviour is really a problem with that commit,
rather, when using 'tui enable' before the inferior has started GDB
ends up calling tui_source_window_base::rerender(), and then passes
through the code path which calls update_source_window_with_addr().
When using 'tui enable' after the inferior has started, GDB again
calls tui_source_window_base::rerender(), but this time has a frame,
and so takes the second code path, which centres the selected source
line, and then calls update_source_window.
The point is that the update_source_window_with_addr() path doesn't
include the logic to centre the source line.
Before the above commit this was fine as GDB's default location would
be prior to main, and so we got the "good" TUI output. After the
above commit the default location is now main's prologue, and without
the centring logic, the first line shown is main's prologue.
I propose fixing this by having update_source_window_with_addr() call
maybe_update(). This will first check if the requested line is
already visible, and if not, show the requested line with centring
applied.
It's not identical to the old behaviour, but that was never the
objective, we do however, see the context around main's prologue,
which will usually be enough to see the function name and return type,
which I think is useful.
Andrew Burgess [Thu, 6 Feb 2025 12:16:48 +0000 (12:16 +0000)]
gdb/tui: update maybe_update to take gdbarch
This is a refactor to setup for the next commit.
The maybe_update function currently takes a frame_info_ptr&, however,
it only uses this to get the frame's gdbarch.
In the next commit I want to call maybe_update when I have a gdbarch,
but no frame_info_ptr& (the inferior hasn't even started).
So, update maybe_update to take the gdbarch, and update the callers to
pass that through. Most callers already have the gdbarch to hand, but
in one place I do need to extract this from the frame_info_ptr&.
There should be no user visible changes after this commit.
Tom Tromey [Fri, 14 Feb 2025 00:45:23 +0000 (17:45 -0700)]
Handle DW_FORM_data4 in read-debug-names.c
The recent .debug_names patches caused the writer to emit
DW_FORM_data4. Unfortunately the reader did not handle this form.
This patch updates the reader to handle a few DW_FORM_data* forms.
The complaint that is there went unnoticed -- I only found this when
debugging a failure in another series. More evidence, IMO, that
complaints should be removed.
I think the reason the failure itself went unnoticed is that the
symbol table code in gdb often works by accident, and in particular in
small programs like the ones in the test suite, it's often the case
that a CU will be expanded for some other reason, causing the test to
pass without really touching the index code. The aforementioned
series is aimed at fixing this.
It would probably be good to unify the abbrev/form code to some
degree, but it's mildly a pain as some forms don't make sense here and
because we recently discovered other issues with gdb's DW_FORM_data*
handling.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Simon Marchi [Tue, 26 Nov 2024 05:32:21 +0000 (00:32 -0500)]
gdbserver: use `gdb::unordered_map`
Replace the few uses of `std::unordered_map` in gdbserver with
`gdb::unordered_map`.
The only one of these that is likely to ever see a lot of elements is
probably `process_info::m_ptid_thread_map`. It was added precisely to
improve performance when there are a lot of threads, so I guess using
`gdb::unordered_map` here won't hurt. I changed the others too, since
it's easy.
Change-Id: Ibc4ede5245551fdd7717cb349a012d05726f4363 Reviewed-By: Stephan Rohr <stephan.rohr@intel.com>
Simon Marchi [Wed, 12 Feb 2025 15:52:51 +0000 (10:52 -0500)]
gdb/dwarf: use term "shard" instead of "index"
A bit more changes as in 8e745eac7db3 ("gdb/dwarf: rename
cooked_index::m_vector to m_shards"). I think it's clearer if the term
"index" is reserved for the whole thing, while "shard" or "index shard"
are used for the parts.
Change-Id: I457bb0016a70f3f9918f4a3c3977262a7801705b Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Thu, 13 Feb 2025 20:08:29 +0000 (15:08 -0500)]
gdb/python/dap: prefix internal attributes with underscore
I'm currently reading the DAP code, and I think this would help. This
is pretty much standard Python style, we do it as some places but not
others. I think it helps readability, by saying that this attribute
isn't mean to be accessed outside the class.
A similar pass could be done for internal methods, I haven't done that.
Change-Id: I8e8789b39adafe62d14404d19f7fc75e2a364e01 Approved-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Sat, 25 Jan 2025 13:00:12 +0000 (13:00 +0000)]
gdb: only update m_last_subfile after writing a line table entry
While working on another patch which changes how we parse the line
DWARF line tables I noticed what I think is a minor bug in how we
process the line tables.
What I noticed is that my new line table parser was adding more END
markers into the parsed table than GDB's current approach. This
difference was observed when processing the debug information for
libstdc++.
Here is the line table from the new test, this is a reasonable
reproduction of the problem case that I observed in the actual debug
line table:
Contents of the .debug_line section:
dw2-skipped-line-entries-1.c:
File name Line number Starting address View Stmt
dw2-skipped-line-entries-1.c 101 0x40110a x
/tmp/dw2-skipped-line-entries-2.c:
dw2-skipped-line-entries-2.c 201 0x401114 x
/tmp/dw2-skipped-line-entries-3.c:
dw2-skipped-line-entries-3.c 301 0x40111e x
/tmp/dw2-skipped-line-entries-1.c:
dw2-skipped-line-entries-1.c 102 0x401128 x
dw2-skipped-line-entries-1.c 103 0x401128 x
dw2-skipped-line-entries-1.c 104 0x401128 x
The problem is caused by the entry for line 211. Notice that this
entry is at the same address as the previous entries. Further, the
entry for 211 is a non-statement entry, while the previous entries are
statement entries.
As the entry for line 211 is a non-statement entry, and the previous
entries at that address are statement entries in a different symtab,
it is thought that it is better to prefer the earlier entries (in
dw2-skipped-line-entries-1.c), and so the entry for line 211 will be
discarded.
As GDB parses the line table it switches between the 3 symtabs (based
on source filename) adding the relevant entries to each symtab.
Additionally, as GDB switches symtabs, it adds an END entry to the
previous symtab.
The problem then is that, for the line 211 entry, this is the only
entry in dw2-skipped-line-entries-2.c before we switch symtab again.
But the line 211 entry is discarded. This means that GDB switches
from dw2-skipped-line-entries-1.c to dw2-skipped-line-entries-2.c, and
then on to dw2-skipped-line-entries-3.c without ever adding an entry
to dw2-skipped-line-entries-2.c.
And here then is the bug. GDB updates its idea of the previous symtab
not when an entry is written into a symtab, but every time we change
symtab.
In this case, when we switch to dw2-skipped-line-entries-3.c we add
the END marker to dw2-skipped-line-entries-2.c, even though no entries
were written to dw2-skipped-line-entries-2.c. At the same time, no
END marker is ever written into dw2-skipped-line-entries-1.c as the
dw2-skipped-line-entries-2.c entry (for line 211) was discarded.
Here is the 'maint info line-table' for dw2-skipped-line-entries-1.c
before this patch:
INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN
0 101 0x000000000040110a 0x000000000040110a Y
1 END 0x0000000000401114 0x0000000000401114 Y
2 102 0x0000000000401128 0x0000000000401128 Y
3 103 0x0000000000401128 0x0000000000401128 Y
4 104 0x0000000000401128 0x0000000000401128 Y
5 104 0x000000000040113c 0x000000000040113c
6 105 0x0000000000401146 0x0000000000401146 Y
7 END 0x0000000000401150 0x0000000000401150 Y
And after this patch:
INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN
0 101 0x000000000040110a 0x000000000040110a Y
1 END 0x0000000000401114 0x0000000000401114 Y
2 102 0x0000000000401128 0x0000000000401128 Y
3 103 0x0000000000401128 0x0000000000401128 Y
4 104 0x0000000000401128 0x0000000000401128 Y
5 END 0x0000000000401132 0x0000000000401132 Y
6 104 0x000000000040113c 0x000000000040113c
7 105 0x0000000000401146 0x0000000000401146 Y
8 END 0x0000000000401150 0x0000000000401150 Y
Notice that we gained an extra entry, the END marker that was added at
position #5 in the table.
Now, does this matter? I cannot find any bugs that trigger because of
this behaviour.
So why fix it? First, the current behaviour is inconsistent, as we
switch symtabs, we usually get an END marker in the previous symtab.
But occasionally we don't. I don't like things that are inconsistent
for no good reason. And second, as I said, I want to change the line
table parsing. To do this I want to check that my new parser creates
an identical table to the current parser. But my new parser naturally
"fixes" this inconsistency, so I have two choices, do extra work to
make my new parser bug-compatible with the current one, or fix the
current one. I'd prefer to just fix the current line table parser.
There's a test that includes the above example and checks that the END
markers are put in the correct place. But as I said, I've not been
able to trigger any negative behaviour from the current solution, so
there's no test that exposes any broken behaviour.
Jan Beulich [Fri, 14 Feb 2025 09:47:50 +0000 (10:47 +0100)]
x86: drop redundant i.operands checks from output_disp()
The opcode space, major opcode, and - where applicable - opcode
extension checks fully qualify the insns we're after; operand matching
has been done far earlier, so wrong operand counts cannot occur here.
Jan Beulich [Fri, 14 Feb 2025 08:35:07 +0000 (09:35 +0100)]
x86: correct ISA-used version recording
Updating should be based solely on the current instruction. For example,
recording of VEX-encoded insns as v3 should be independent of there
being earlier AMX insns.
Further for BASELINE only a very limited set of the
GNU_PROPERTY_X86_FEATURE_2_* bits should actually be taken into account:
Most of the bits represent advanced (later) features (XSAVE, XSAVEOPT,
and XSAVEC for example being part of v3).
Jan Beulich [Fri, 14 Feb 2025 08:33:18 +0000 (09:33 +0100)]
gas: fix rs_fill_nop listing
In commit a0094f1a70e1 ("gas: make .nops output visible in listing") I
was wrongly assuming fr_fix would be zero for rs_fill_nop, when that's
only a side effect of listing_newline() inserting dummy frags, but only
when file/line did actually change from the previous invocation. This is
in particular not going to be true when the .nops directive isn't the
first statement on a line.
Jan Beulich [Fri, 14 Feb 2025 08:32:35 +0000 (09:32 +0100)]
x86/APX: make .insn extended-EVEX capable
So far tricks had to be played to use .insn to encode extended-EVEX
insns; the X4 bit couldn't be controlled at all. Extend the syntax just
enough to cover all features, taking care to reject invalid feature
combinations (albeit aiming at being as lax there as possible, to offer
users as much flexibility as we can - we don't, after all, know what
future will bring).
In a pre-existing testcase replace all but one .byte; the one that needs
to remain wants to have EVEX.U clear in a way that's neither
controllable via AVX10/256 embedded rounding (would otherwise also set
EVEX.ND), nor via the index register (EVEX.X4), as there's no memory
operand. For one of the converted instances ModR/M.mod needs correcting:
An 8-bit displacement requires that to be 1, not 2. Also adjust source
comments to better represent what the bad insns mimic.
Hau Hsu [Fri, 14 Feb 2025 02:40:53 +0000 (10:40 +0800)]
RISC-V: Make SSAMOSWAP.W available for rv64
Previously we limited SSAMOSWAP.W only available on RV32, but it should
be available on RV64 as well.
See
https://github.com/riscv/riscv-cfi/blob/main/src/cfi_backward.adoc
https://github.com/riscv/riscv-isa-manual/blob/702a3e6e843235a2a13b918ae6938b04f8974ffc/src/unpriv-cfi.adoc#L789
Alan Modra [Thu, 13 Feb 2025 01:46:26 +0000 (12:16 +1030)]
dlltool memory leaks
dlltool copies strings with strdup all over the place, seeming to take
the attitude that anything might be modified. That leads to lots of
memory leaks. Fixing the leaks by removing the strdup calls of course
means you need to take good care that strings *aren't* modified. This
isn't as easy as it sounds due to functions like xlate that have
const char* params but then manage to modify the strings. I've fixed
xlate, but if I've missed something somewhere then this patch likely
will break dlltool. Testsuite coverage of dlltool isn't good.
The leaks in defparse.y are small. It also is a little work to verify
that all the strings I'm freeing in defparse.y are in fact malloc'd,
which is no doubt why the leaks are there.
Using bfd_xalloc in make_one_lib_file and functions called from there
results in memory being freed automatically at the bfd_close in
make_one_lib_file, without any fuss.
The patch also makes use of xasprintf to replace xmalloc followed by
sprintf.
* defparse.y (opt_name2): Free incoming ID strings after
adding prefix/suffix.
* dlltool.c (struct ifunct): Constify char* fields.
(struct iheadt, struct dlist): Likewise.
(set_dll_name_from_def, def_heapsize, def_stacksize),
(def_section, assemble_file): Use xasprintf.
(def_name, def_library): Free dll_name and name.
(def_description, new_directove): Don't strdup incoming args.
(append_import): Likewise.
(def_import): Free module after appending dllext.
(run): Free temp_base.
(scan_filtered_symbols): Don't segfault on NULL strchr return.
Remove unnecessary strdup.
(scan_drectve_symbols): Likewise. Constify pointers.
Use bfd_malloc_and_get_section. Use xmemdup.
(add_excludes): Use xasprintf and xstrdup.
(gen_exp_file): Free xlate return. Constify pointer to suit
struct changes. Free copy.
(xlate): Always copy arg. Use xasprintf and xstrdup.
(make_imp_label): Add bfd arg. Use bfd_xalloc.
(gen_lib_file): Adjust to suit.
(make_one_lib_file): Likewise. Use bfd_xalloc for section data
and relocs. Simplify code calling xlate, and free xlate return.
(dll_name_list_free_contents): Flatten recursion.
(mangle_defs): Free d_export_vec.
(main): Formatting. Use xasprintf.
* resres.c (write_res_id): Free section data.
Alan Modra [Thu, 13 Feb 2025 00:29:51 +0000 (10:59 +1030)]
gas: replace bfd_alloc with notes_alloc
bfd_alloc can return NULL on out-of-memory so code needs to check the
return value and print an error. That check was missing in write.c.
notes_alloc won't return NULL, instead the underlying obstack_alloc
prints an OOM message and the process exits. This is more convenient,
and when the bfd_alloc memory is attached to the gas output bfd it is
released only slightly before the notes obstack.
* config/obj-macho.c (obj_mach_o_set_indirect_symbols): Use
notes_calloc rather than bfd_zalloc.
* write.c (set_symtab): Use notes_alloc.
Alan Modra [Wed, 12 Feb 2025 21:55:01 +0000 (08:25 +1030)]
gas obj-coff memory leaks
This patch addresses memory leaks in gas that show up when running the
testsuite on x86_64-w64-mingw32. The seh_ctx_cur, and weak sym naming
leaks can occur many times during assembly. The symbol hook and
section leaks are not so important since this memory needs to persist
until closing the output bfd.
* config/obj-coff-seh.c (do_seh_endproc): Free seh_ctx_cur and
its fields.
* config/obj-coff-seh.h (struct seh_context): Remove unused
"next" field.
* config/obj-coff.c (coff_obj_symbol_new_hook): Use notes_alloc
for aux entries.
(coff_obj_symbol_clone_hook): Likewise.
(obj_coff_def): Don't strdup name unless we need to do so
for tc_canonicalize_symbol_name. Free after making symbol.
(weak_name2altname, weak_altname2name): Return a char*.
(weak_uniquify): Use notes_concat.
(pecoff_obj_set_weak_hook, pecoff_obj_clear_weak_hook): Free name
returned by weak_name2altname.
(coff_frob_symbol): Similarly for weak_altname2name.
(obj_coff_section): Use notes_memdup0.
* symbols.h: Add include guard.
(notes_memdup0): New inline function.
Tom Tromey [Thu, 13 Feb 2025 03:43:38 +0000 (20:43 -0700)]
Remove assumption from py-symbol.exp
The current py-symbol.exp test makes an assumption about which symbol
will be returned first. I don't think gdb should really make promises
about the order in which the symbols are listed, though, and a series
I am working on changes this behavior. This patch changes the test to
merely ensure that both symbols are returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Kevin Buettner [Thu, 13 Feb 2025 17:53:17 +0000 (10:53 -0700)]
Update my maintenance areas in MAINTAINERS file
I've dropped maintenance of the mep target. Additionally, I'm removed
myself as an authorized committer for PowerPC, ia64, AIX, and
GNU/Linux PPC native.
gdb, testsuite: Rename set_sanitizer procedures to append_environment.
The procedures set_sanitizer_1, set_sanitizer and set_sanitizer_default
are used for the configuration of ASAN specific environment variables.
However, they are actually generic. Rename them to append_environment*
so that their purpose is more clear.
Tom Tromey [Wed, 12 Feb 2025 15:49:30 +0000 (08:49 -0700)]
Reorder gnatmake arguments in inline-section-gc.exp, again
Tom de Vries pointed out that commit 8cfa1fc4 ("Reorder gnatmake
arguments in inline-section-gc.exp") caused a regression with an older
version of dejagnu.
This patch works around that problem by further reordering the
arguments to gnatmake and also arranging to leave gnatmake in "-margs"
mode.
Rohr, Stephan [Thu, 1 Aug 2024 17:46:18 +0000 (19:46 +0200)]
gdb: remove check for minimal symbols in 'start_command'
GDB aborts the 'start' command if the minimal symbols cannot be
resolved. On Windows, GDB reads the minimal symbols from the COFF
header of the PE file. The symbol table is deprecated and the
number of symbols in the COFF header may be zero:
FILE HEADER VALUES
8664 machine (x64)
E number of sections 66E889EC time date stamp Mon Sep 16 21:41:32 2024
FB400 file pointer to symbol table
0 number of symbols
F0 size of optional header
22 characteristics
GDB is not able to read the minimal symbols; the `start' command fails
with an error:
(gdb) start
No symbol table loaded. Use the "file" command.
Manually inserting a breakpoint in main works fine:
(gdb) tbreak main
Temporary breakpoint 1 at 0x14000100c: file test.cpp, line 6.
(gdb) run
Starting program: C:\test-clang
Temporary breakpoint 1, main () at test.cpp:6
6 std::cout << "Hello World.\n";
Remove the check entirely; a 'NOT_FOUND_ERROR' is thrown if 'main'
cannot be resolved. The error is consumed in 'create_breakpoint ()'
and an error message is displayed to the user.
Approved-by: Kevin Buettner <kevinb@redhat.com> Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Simon Marchi [Mon, 10 Feb 2025 16:54:58 +0000 (11:54 -0500)]
gdb/dwarf: rename cooked_index::m_vector to m_shards
I think that is clearer and helps readability.
Rename a few iteration variables from "index" or "idx" to "shard". In
my mental model, the "index" is the whole thing, so it's confusing to
use that word when referring to shards.
Change-Id: I208cb839e873c514d1f8eae250d4a16f31016148 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 10 Feb 2025 16:54:57 +0000 (11:54 -0500)]
gdb/dwarf: remove cooked_index::vec_type
I find this typedef to be confusing. The name is a bit too generic, so
it's not clear what it represents. When using the typedef for a
cooked_index_shard unique pointer, I think that spelling out the vector
type is not overly long.
Change-Id: I99fdab5cd925c37c3835b466ce40ec9c1ec7209d Approved-By: Tom Tromey <tom@tromey.com>
Kito Cheng [Mon, 20 Jan 2025 11:24:36 +0000 (19:24 +0800)]
RISC-V: Add .bfloat16 directive
RISC-V already support bfloat16 instruciton like Zfbfmin, Zvfbfmin and
Zvfbfwma, so I think it's reasonable to add .bfloat16 directive to
support bfloat16 data type.
And the code logic mostly support by common code already.
Nelson Chu [Thu, 6 Feb 2025 13:47:37 +0000 (21:47 +0800)]
RISC-V: Move all global static target stuff into private data for disassembler.
I got a request said that the JDK multi-thread compiler may be broken
if two or more threads are trying to print/disassemble stuff, and filling
the disassemble_info, setting callbacks, and grabbing the function pointer
to disasm at the same time. Since such as the target global static stuff,
including subset of extensions and mapping symbol stuff, seems to only be
one globally. Ideally, for dis-assembler, all global static target stuff
should/can be better to be defined into the target private data, since they
are target-dependency.
opcodes/
* riscv-dis.c: Moved all global static target-dependency stuff into
riscv_private_data, including architecture and mapping symbol stuff.
(set_default_riscv_dis_options): Updated since global static target-
dependency stuff are moved into riscv_private_data.
(parse_riscv_dis_option_without_args): Likewise.
(parse_riscv_dis_option): Likewise.
(parse_riscv_dis_options): Likewise.
(maybe_print_address): Likewise.
(print_reg_list): Likewise.
(riscv_get_spimm): Likewise.
(print_insn_args): Likewise.
(riscv_disassemble_insn): Likewise.
(riscv_update_map_state): Likewise.
(riscv_search_mapping_symbol): Likewise.
(riscv_data_length): Likewise.
(print_insn_riscv): Likewise. Call the riscv_init_disasm_info before
parsing any disassembler options, since the related stuff are moved
into riscv_private_data.
(riscv_init_disasm_info): Likewise. Parse and set the architecture
string and privileged spec version since riscv_get_disassembler is
no longer needed.
(riscv_get_disassembler): Removed.
(disassemble_free_riscv): Only free the subset_list if
riscv_private_data exsits.
* disassemble.c (disassembler): Since riscv_get_disassembler is
removed, call to print_insn_riscv.
* disassemble.h: Removed extern riscv_get_disassembler.
Flavio Cruz [Mon, 3 Feb 2025 04:15:22 +0000 (23:15 -0500)]
Port GDB to Hurd x86_64.
This port extends the existing i686 port to support x86_64 by reusing
existing code whenever it makes sense.
* gdb/amd64-gnu-tdep.c: Adds logic for handling signal frames and
position of amd64 registers in the different Hurd structs.
The signal code is very similar to i686, except the trampoline code
is adapted.
* gdb/config/i386/nm-i386gnu.h: renamed to gdb/config/i386/nm-x86-gnu.h
and adapt it for x86_64.
* gdb/config/i386/i386gnu.mn: renamed to gdb/config/i386/nm-x86-gnu.mn
and reuse it for x86_64.
* gdb/configure.host: recognize gnu64 as a host.
* gdb/configure.nat: recognize gnu64 host and update existing i386gnu to
reuse the new shared files.
* gdb/configure.tgt: recognize x86_64-*-gnu* triplet and use
amd64-gnu-tdep.c.
* gdb/i386-gnu-tdep.c: added i386_gnu_thread_state_reg_offset that is
copied from i386-gnu-nat.c. This makes it similar to amd64.
* gdb/i386-gnu-nat.c: rename it to x86-gnu-nat.c since we reuse this for
i386 and amd64. Updated REG_ADDR to use one of the structures. Added
VALID_REGISTER to make sure it's a register we can provide at this time
(not all of them are available in amd64). FLAGS_REGISTER is either rfl
or efl depending on the arch. Renamed functions and class from i386 to x86
whenever they can be reused.
Clean up after commit 112cf77b1855 ("MIPS: use is_whitespace()") and
untangle the code flow in the handling of forced size suffixes, noting
that owing to the loop right above the only data `c' can hold at this
point is '\0', '.', or a white-space character. No functional change.
MIPS16/GAS: Reject instructions that end with a dot
Fix a regression from commit 3fb49709438e ("MIPS16/GAS: Fix forced size
suffixes with argumentless instructions") and reject MIPS16 instructions
that end with a dot and no forced size suffix following, e.g.:
Clean up after commit 29c108c96106 ("MIPS: Support `-gnuabi64' target
triplet suffix for 64-bit Linux targets") and discard individual MIPS
"want64=true" settings, the use of which has been superseded by commit 42429eacb42f ("Require a 64-bit bfd_vma for MIPS ELF") back in 2013[1].
I spotted some places where tui_win_info::refresh_window() was being
called when suppress_output was false. This means that there is no
tui_batch_rendering in place on the call stack, and so, after that
commit, we might be performing more wrefresh() calls than necessary.
Before the above commit we would have been calling wnoutrefresh() and,
due to the missing tui_batch_rendering, there might have been a delay
before doupdate() was called.
To (hopefully) make screen updates smoother, this commit adds
tui_batch_rendering in a few places where it is possible that there
might be multiple window updates performed, this will mean the final
write to screen is deferred until the tui_batch_rendering goes out of
scope.
Other than possibly smother screen updates, there should be no user
visible changes after this commit.
Andrew Burgess [Sat, 8 Feb 2025 21:32:05 +0000 (21:32 +0000)]
gdb/tui: remove unnecessary wmove call from tui_status_window
I've been looking recently at when the TUI calls wnoutrefresh vs
wrefresh, and the ordering of other screen update actions relative to
these calls.
I noticed in tui_status_window::rerender() a call to wmove() that is
placed after the refresh_window() call. This surely means that the
cursor is moved, but, this update is not sent to the screen.
But we call wmove() at the start of tui_status_window::rerender()
before anything is sent to the screen, so the final wmove() call is
pointless as far as I can tell.
I propose removing it. This is trivial, but removing pointless work
like this slowly makes the TUI code easier to understand.
There should be no user visible changes after this commit.
Guinevere Larsen [Mon, 13 Jan 2025 17:04:04 +0000 (14:04 -0300)]
gdb: Deprecate stabs debug info
GCC has deprecated stabs generation in GCC 12 and entirely removed it in
GCC 13, which was released in April 2023. At the time it was proposed
that GDB deprecate stabs as well, but the decision was to support it a
bit longer. With this patch, it'll be deprecated on GDB 17, and removed
on GDB 18, which following the current cadence, will be released early
2026, meaning we will have supported stabs for nearly 3 years longer
than GCC, which I think is reasonable.
As pointed out in the previous discussion on this topic[1], there are
several existing issues on the code, and none of the current maintainers
knows how to fix it. Unless someone steps up to fix this before the
removal on GDB 18, I don't see why we should keep this old code that
breaks all conventions of modern debuginfo readers and doesn't even
work, instead of being able to further advance adjacent code.
Finally, deprecating and removing stabs will make a.out/dbx inferiors be
essentially unsupported, as the only debuginfo GDB supports for those
formats is stabs, meaning users would only have assembly-level debugging
for that format. With that in mind, this commit deprecates the a.out/dbx
format as well.
Simon Marchi [Sun, 9 Feb 2025 05:51:04 +0000 (00:51 -0500)]
gdb/dwarf: create multiple cooked index shards when reading .debug_names
New in v2:
- install address map in a single shard
- update test gdb.mi/mi-sym-info.exp to cope with the fact that
different symbols could be returned when using --max-results
When playing with the .debug_names reader, I noticed it was
significantly slower than the DWARF scanner. Using a "performance"
build of GDB (with optimization, no runtime sanitizer enabled, etc), I
measure with the following command on a rather large debug info file
(~4 GB):
$ time ./gdb -q -nx --data-directory=data-directory <binary> -iex 'maint set dwarf sync on' -batch
This measures the time it takes for GDB to build the cooked index (plus
some startup and exit overhead). I have a version of the binary without
.debug_names and a version with .debug_names added using gdb-add-index.
The results are:
- without .debug_names: 7.5 seconds
- with .debug_names: 24 seconds
This is a bit embarrassing, given that the purpose of .debug_names is to
accelerate things :). The reason is that the .debug_names processing is
not parallelized at all, while the DWARF scanner is heavily
parallelized.
The process of creating the cooked index from .debug_names is roughly in
two steps:
1. scanning of .debug_names and creation of cooked index entries (see
mapped_debug_names_reader::scan_all_names)
2. finalization of the index, name canonicalization and sorting of the
entries (see cooked_index::set_contents).
This patch grabs a low hanging fruit by creating multiple cooked index
shards instead of a single one during step one. Just doing this allows
the second step of the processing to be automatically parallelized, as
each shard is sent to a separate thread to be finalized.
With this patch, I get:
- without .debug_names: 7.5 seconds
- with .debug_names: 9.7 seconds
Not as fast as we'd like, but it's an improvement.
The process of scanning .debug_names could also be parallelized to shave
off a few seconds. My profiling shows that out of those ~10 seconds of
excecution, about 6 are inside scan_all_names. Assuming perfect
parallelization with 8 threads, it means that at best we could shave
about 5 seconds from that time, which sounds interesting. I gave it a
shot, but it's a much more intrusive change, I'm not sure if I will
finish it.
This patch caused some regressions in gdb.mi/mi-sym-info.exp with the
cc-with-debug-names board, in the test about the `--max-results` switch.
It appears at this test is relying on the specific symbols returned when
using `--max-results`. As far as I know, we don't guarantee which
specific symbols are returned, so any of the matching symbols could be
returned.
The round robin method used in this patch to assign index entries to
shards ends up somewhat randomizing which CU gets expanded first during
the symbol search, and therefore which order they appear in the
objfile's CU list, and therefore which one gets searched first.
I meditated on whether keeping compunits sorted within objfiles would
help make things more stable and predictable. It would somewhat, but it
wouldn't remove all sources of randomness. It would still possible for
a call to `expand_symtabs_matching` to stop on the first hit. Which
compunit gets expanded then would still be dependent on the specific
`quick_symbol_functions` internal details / implementation.
Commit 5b99c5718f1c ("[gdb/testsuite] Fix various issues in
gdb.mi/mi-sym-info.exp") had already started to make the test a bit more
flexible in terms of which symbols it accepts, but with this patch, I
think it's possible to get wildly varying results. I therefore modified
the test to count the number of returned symbols, but not expect any
specific symbol.
Change-Id: Ifd39deb437781f72d224ec66daf6118830042941 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Sun, 9 Feb 2025 05:51:02 +0000 (00:51 -0500)]
gdb/dwarf: allow for cooked_index_shard::m_addrmap to be nullptr
The following patch makes the .debug_names reader create multiple cooked
index shards, only one of them having an address map. The others will
have a nullptr address map.
Change the code using cooked_index_shard::m_addrmap to account for the
fact that it can be nullptr.
Change-Id: Id05b974e661d901dd43bb5ecb3a8fcfc15abc7ed Approved-By: Tom Tromey <tom@tromey.com>