| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
I spotted that the code_breakpoint::disabled_by_cond flag doesn't work
how I'd expect it too. The flag appears to be "sticky" in some
situations; once a code_breakpoint::disabled_by_cond flag is marked
true, then, in some cases the flag wont automatically become false
again, even when you'd think it should.
The problem is in update_breakpoint_locations. In this function,
which is called as a worker of code_breakpoint::re_set, GDB computes a
new set of locations for a breakpoint, the new locations are then
installed into the breakpoint.
However, before installing the new locations GDB attempts to copy the
bp_location::enabled and bp_location::disabled_by_cond flag from the
old locations into the new locations.
The reason for copying the ::enabled flag makes sense. This flag is
controlled by the user. When we create the new locations if GDB can
see that a new location is equivalent to one of the old locations, and
if the old location was disabled by the user, then the new location
should also be disabled.
However, I think the logic behind copying the ::disabled_by_cond flag
is wrong. The disabled_by_cond flag is controlled by GDB and should
toggle automatically. If the condition string can be parsed then the
flag should be false (b/p enabled), if the condition string can't be
parsed then the flag should be true (b/p disabled).
As we always parse the condition string in update_breakpoint_locations
before we try to copy the ::enabled flag value then the
::disabled_by_cond flag should already be correct, there's no need to
copy over the ::disabled_by_cond value from the old location.
As a concrete example, consider a b/p placed within the main
executable, but with a condition that depends on a variable within a
shared library.
When the b/p is initially created the b/p will be disabled as the
condition string will be invalid (the shared library variable isn't
available yet).
When the inferior starts the shared library is loaded and the
condition variable becomes available to GDB. When the shared library
is loaded breakpoint_re_set is called which (eventually) calls
update_breakpoint_locations.
A new location is computed for the breakpoint and the condition string
is parsed. As the shared library variable is now know the expression
parses correctly and ::disabled_by_cond is left false for the new
location.
But currently GDB spots that the new location is at the same address
as the old location and copies disabled_by_cond over from the old
location, which marks the b/p location as disabled. This is not what
I would expect.
The solution is simple, don't copy over disabled_by_cond.
While writing a test I found another problem though. The
disabled_by_cond flag doesn't get set true when it should! This is
the exact opposite of the above.
The problem here is in solib_add which is (despite the name) called
whenever the shared library set changes, including when a shared
library is unloaded.
Imagine an executable that uses dlopen/dlclose to load a shared
library. Given an example of a b/p in the main executable that has a
condition that uses a variable from our shared library, a library
which might be unloaded with dlclose.
My expectation is that, when the library is unloaded, GDB will
automatically mark the breakpoint as disabled_by_cond, however, this
was not happening.
The problem is that in solib_add we only call breakpoint_re_set when
shared libraries are added, not when shared libraries are removed.
The solution I think is to just call breakpoint_re_set in both cases,
now the disabled_by_cond flag is updated as I'd expect.
Unfortunately, making this change causes a regression when running:
make check-gdb \
TESTS="gdb.trace/change-loc.exp" \
RUNTESTFLAGS="--target_board=native-gdbserver"
This test unloads a shared library and expects breakpoints within the
shared library to enter the PENDING state (because the bp_location's
shlib_disabled flag will be set). However, the new call to
breakpoint_re_set means that this is no longer the case.
The breakpoint_re_set call means that update_breakpoint_locations is
called, which then checks if all locations for a breakpoint are
pending or not. In this test not all locations are pending, and so
GDB recalculates the locations of each breakpoint, this means that
pending locations are discarded.
There is a but report PR gdb/32404 which mentions the problems with
shlib_disabled pending breakpoints, and how they are prone to being
randomly deleted if the user can cause GDB to trigger a call to
breakpoint_re_set. This patch just adds another call to
breakpoint_re_set, which triggers this bug in this one test case.
For now I have marked this test as KFAIL. I do plan to try and
address the pending (shlib_disabled) breakpoint problem in the future,
but I'm not sure when that will be right now.
There are, of course, tests to cover all these cases.
During review I was pointed at bug PR gdb/32079 as something that this
commit might fix, or help in fixing.
And this commit is part of the fix for that bug, but is not the
complete solution. However, the remaining parts of the fix for that
bug are not really related to the content of this commit.
The problem in PR gdb/32079 is that the inferior maps multiple copies
of libc in different linker namespaces using dlmopen (actually libc is
loaded as a consequence of loading some other library into a different
namespace, but that's just a detail). The user then uses a 'next'
command to move the inferior forward.
GDB sets up internal breakpoints on the longjmp symbols, of which
there are multiple copies (there is a copy in every loaded libc).
However, the 'next' command is, in the problem case, stepping over a
dlclose call which unloads one of the loaded libc libraries.
In current HEAD GDB in solib_add we fail to call breakpoint_re_set()
when the library is unloaded; breakpoint_re_set() would delete and
then recreate the longjmp breakpoints. As breakpoint_re_set() is not
called GDB thinks that the the longjmp breakpoint in the now unloaded
libc still exists, and is still inserted.
When the inferior stops after the 'next' GDB tries to delete and
remove the longjmp breakpoint which fails as the libc in which the
breakpoint was inserted is no longer mapped in.
When the user tries to 'next' again GDB tries to re-insert the still
existing longjmp breakpoint which again fails as the memory in which
the b/p should be inserted is no longer part of the inferior memory
space.
This commit helps a little. Now when the libc library is unmapped GDB
does call breakpoint_re_set(). This deletes the longjmp breakpoints
including the one in the unmapped library, then, when we try to
recreate the longjmp breakpoints (at the end of breakpoint_re_set) we
don't create a b/p in the now unmapped copy of libc.
However GDB does still think that the deleted breakpoint is inserted.
The breakpoint location remains in GDB's data structures until the
next time the inferior stops, at which point GDB tries to remove the
breakpoint .... and fails.
However, as the b/p is now deleted, when the user tries to 'next' GDB
no longer tries to re-insert the b/p, and so one of the problems
reported in PR gdb/32079 is resolved.
I'll fix the remaining issues from PR gdb/32079 in a later commit in
this series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32079
Tested-By: Hannes Domani <ssbssa@yahoo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I noticed a spot in dwarf2/index-write.c that was mis-formatted. This
fixes it.
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
correcting indentation as required.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
converting spaces to tabs and fixing alignment as appropriate.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
capitalizing sentences and adding full stops and spaces after them.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
removing and adding new lines as appropriate.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
removing trailing space.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
gdb/testsuite/rocm.exp: Use system GPU(s) to detect features
Background
----------
This patch revisits the purpose of hcc_amdgpu_targets{} in
order to address the separation of concerns between:
- GPU targets passed to the compiler. This kind of target
is passed as an argument to flags like "--offload-arch=...",
"--targets=...", etc.
- GPU targets as in available GPU devices on the system. This
is crucial for finding which capabilities are available,
and therefore which tests should be executed or skipped.
Code change
-----------
- A new "find_amdgpu_devices{}" procedure is added. It is
responsible for listing the GPU devices that are available
on the system.
- "hcc_amdgpu_targets{}" is rewritten to use the newly added
"find_amdgpu_devices{}" when there's no environment variable
(HCC_AMDGPU_TARGET) set.
- The output of "hcc_amdgpu_targets{}" is now only used in
places that set the target for the building toolchains.
- The output of "find_amdgpu_devices{}" is used anywhere that
needs to evaluate the GPU features.
Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)
Change-Id: Ib11021dbe674aa40192737ede78284a1bc531513
|
|
The part "... this command by default creates it produces a single ..."
sounds wrong. Replace with "... this command by default produces a
single ...".
Change-Id: I39cc533fa5a2bf473ca9e361ee0e6426d7d37ac6
|
|
Change-Id: Ibd8d6c35c2cc02e309f83b11b5fd1172dfa05283
|
|
clang 19 fails to build gdb with this error:
/home/simark/src/binutils-gdb/gdb/compile/compile-object-load.c:302:3: error: cannot initialize a member subobject of type 'void (*)(const char *, ...) __attribute__((noreturn))' with an lvalue of type 'void (const char *, ...)'
302 | link_callbacks_einfo, /* einfo */
| ^~~~~~~~~~~~~~~~~~~~
This illustrates that the bfd_link_callbacks array is missing an entry
for the "fatal" callback, add it.
The fatal field was added very recently, in d26161914 ("PR 32603, more
ld -w misbehaviour"). We're lucky that the new callback was marked with
the noreturn attribute and that clang checks that, otherwise this would
have gone unnoticed.
Change-Id: I68b63d89f2707359e6254da23bdc0776b0e03ba2
|
|
While working on another series, I discovered that the existing code
in gdb.ada/complete.exp that conditionally accepts a completion does
not work correctly. The code assumes that wrapping a line in "(...)?"
will make the entire line optional, but really this will only match a
blank line.
Meanwhile, I needed this same patch for a second series I'm working
on, so I've pulled this out. As it only affects Ada, I am going to
check it in.
|
|
I noticed a non-bool-like use of target_get_tib_address in
windows-tdep.c. After fixing this I thought it would be good to
document the target method; and this also lead to some non-bool-like
commentary in remote.c. This patch fixes all of these nits.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Now that stabs is deprecated, we should probably warn our users of it
before removing support, so that they have time to react and either make
themselves heard, or fix things on their end so that they can still debug
their applications.
This commit adds a new function that emits a warning whenever GDB does
stabs reading. Since there are several places where stabs is
re-invented, this warning had to be added to many places, but I think I
managed to warn everywhere relevant without duplicating warnings.
Also, the test gdb.stabs/weird.exp explicitly checks for GDB warnings
when reading stabs, so it had to be updated to account for the
deprecation warning. It is done generically, since it will be removed in
the next release anyway.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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.
|
|
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.
|
|
This changes prepare_one_comp_unit to be a private method of
cutu_reader. This should make it somewhat simpler to reason about.
|
|
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.
|
|
This moves the producer_is_realview to producer.c.
|
|
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.
|
|
When using the MI interpreter, if someone was to attach to a ROCm
process which has active GPU waves, GDB would issue a segfault as
follows:
attach 1994813
&"attach 1994813\n"
~"Attaching to process 1994813\n"
=thread-group-started,id="i1",pid="1994813"
=thread-created,id="1",group-id="i1"
=thread-created,id="2",group-id="i1"
~"[New LWP 1994828]\n"
*running,thread-id="2"
=thread-created,id="3",group-id="i1"
~"[New LWP 1994825]\n"
*running,thread-id="3"
=thread-created,id="4",group-id="i1"
~"[New LWP 1994823]\n"
*running,thread-id="4"
^done
=library-loaded,...
[...]
~"[Thread debugging using libthread_db enabled]\n"
~"Using host libthread_db library \"/lib/x86_64-linux-gnu/libthread_db.so.1\".\n"
=thread-created,id="5",group-id="i1"
&"\n\n"
&"Fatal signal: "
&"Segmentation fault"
&"\n"
&"----- Backtrace -----\n"
&"Backtrace unavailable\n"
&"---------------------\n"
&"A fatal error internal to GDB has been detected, further\ndebugging is not possible. GDB will now terminate.\n\n"
&"This is a bug, please report it."
&" For instructions, see:\n"
&"<https://github.com/ROCm-Developer-Tools/ROCgdb/issues>"
&"."
&"\n\n"
Segmentation fault
The issue comes from using a non-initialized pointer in mi_on_resume_1:
if (!mi->running_result_record_printed && mi->mi_proceeded)
{
gdb_printf (mi->raw_stdout, "%s^running\n",
mi->current_token ? mi->current_token : "");
}
In this instance, "mi->current_token" has an uninitialized value. This is a
regression introduced by:
commit def2803789208a617c429b5dcf2026decb25ce0c
Date: Wed Sep 6 11:02:00 2023 -0400
gdb/mi: make current_token a field of mi_interp
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>
|
|
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>
|
|
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>
|
|
Same as the previous patch, but for the containing section.
Change-Id: I469147cce21525d61b3cf6edd9a9f4b12027c176
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Similar to the previous patch, but for the offset within the containing
section.
Change-Id: I1d76e1f88002bca924e0b12fd78c7ea49d36c0ec
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
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.
- add_type_unit
- lookup_dwo_file_slot
- create_dwo_unit_in_dwp_v1
- create_dwp_v2_or_v5_section
- create_dwo_unit_in_dwp_v2
- create_dwo_unit_in_dwp_v5
- lookup_dwo_unit_in_dwp
Change-Id: I200cd77850ce0ffa29fc1b9d924056fdce2559f8
Approved-By: Tom Tromey <tom@tromey.com>
|
|
No behavior changes expected.
Change-Id: I16ff6c67058362c65cc8edb05d1948e48be6b2e1
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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:
kernel/ptrace.c: ptrace_regset():
kiov->iov_len = min(kiov->iov_len,
(__kernel_size_t) (regset->n * regset->size));
if (req == PTRACE_GETREGSET)
return copy_regset_to_user(task, view, regset_no, 0,
kiov->iov_len, kiov->iov_base);
else
return copy_regset_from_user(task, view, regset_no, 0,
kiov->iov_len, kiov->iov_base);
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]
[1] https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#control-and-status-registers-related-to-watchpoints
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=531936dee53e
[3] https://github.com/llvm/llvm-project/commit/ff79d83caeee
Signed-off-by: Hui Li <lihui@loongson.cn>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
|
|
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>
|
|
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>
|
|
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
|
|
Fix typos:
...
overriden -> overridden
reate -> create
...
Tested on x86_64-linux.
I
|
|
Change-Id: I9a6bf27b72f7efb1cc4cea5345db14969e794bdb
|
|
Change-Id: I420280721cb734a2e061743309bf9b25d2179f8f
|
|
This is reported as unused by clangd.
Change-Id: Ida5a93b632cd4477fb91df1ab0edf66f12a28f64
|
|
clangd reports this include as unused.
Change-Id: I6a4224d8aa581fea2336da124c89ade09f573af3
|
|
The `get_mi_thread_list` procedure's body is incorrectly indented.
Fix it.
There is one line that was already long. Consider it an exception and
don't bother breaking it.
|
|
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
Approved-By: Tom de Vries <tdevries@suse.de>
|
|
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.
[1] https://lore.kernel.org/all/87jzb2tdb7.fsf_-_@email.froward.int.ebiederm.org
Signed-off-by: Ivan Kokshaysky <ink@unseen.parts>
Reviewed-By: Maciej W. Rozycki <macro@orcam.me.uk>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
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.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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':
┌─hello.c────────────────────────────────────────────────┐
│ 10 return 0; │
│ 11 } │
│ 12 │
│ 13 /* The main function. */ │
│ 14 │
│ 15 int │
│ 16 main () │
│ 17 { │
│ 18 printf ("Hello World\n"); │
│ 19 call_me ( 0, 1, 2, 3, 4, 5, 6, 7 ); │
│ 20 return 0; │
│ 21 } │
│ │
│ │
└────────────────────────────────────────────────────────┘
Compare this to GDB 16.2:
┌─hello.c────────────────────────────────────────────────┐
│ 17 { │
│ 18 printf ("Hello World\n"); │
│ 19 call_me ( 0, 1, 2, 3, 4, 5, 6, 7 ); │
│ 20 return 0; │
│ 21 } │
│ │
│ │
│ │
│ │
│ │
│ │
│ │
│ │
│ │
└────────────────────────────────────────────────────────┘
I think the new layout is not as good because it is missing the
context of the function name. The new behaviour started with the
commit:
commit 49e607f511c1fab82a0116990a72d1915c74bb4a
Author: Stephan Rohr <stephan.rohr@intel.com>
Date: Sat Aug 3 02:07:42 2024 -0700
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.
After this commit, the 'tui enable' state is now:
┌─hello.c─────────────────────────────────────────────────────┐
│ 11 } │
│ 12 │
│ 13 /* The main function. */ │
│ 14 │
│ 15 int │
│ 16 main () │
│ 17 { │
│ 18 printf ("Hello World\n"); │
│ 19 call_me ( 0, 1, 2, 3, 4, 5, 6, 7 ); │
│ 20 return 0; │
│ 21 } │
│ │
│ │
│ │
└─────────────────────────────────────────────────────────────┘
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.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
Throughout gdb/dwarf2, use `*_up` typedefs. Add a few missing typedefs,
and move some so they are, ideally, just after the corresponding class.
Change-Id: Iab5cd8fc2e9989d4bd8d4868586703c2312f254f
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Rename them to include "worker" in the name. Otherwise, it's easy to be
confused and think that they are sub-classes of "cooked_index".
Change-Id: I625ef076f9485f3873db530493f60a53d02c1991
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
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>
|
|
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
/tmp/dw2-skipped-line-entries-2.c:
dw2-skipped-line-entries-2.c 211 0x401128
/tmp/dw2-skipped-line-entries-3.c:
dw2-skipped-line-entries-3.c 311 0x401132
/tmp/dw2-skipped-line-entries-1.c:
dw2-skipped-line-entries-1.c 104 0x40113c
dw2-skipped-line-entries-1.c 105 0x401146 x
dw2-skipped-line-entries-1.c - 0x401150
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.
Approved-By: Tom Tromey <tom@tromey.com>
|
