| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The internal AdaCore test suite found a bug with the char-printing
changes: on Windows, it is possible to have a wide character (in our
case, 0xBEEF) that is "printable" (as determined by iswprint) but
which cannot be converted to the current host charset.
This in turn would result in strange output like:
$2 = 48879 '\357\276'
where what we would expect in Ada would be:
$2 = 48879 '["00beef"]'
A similar problem could occur for C on Windows. There, the character
boundaries appeared lost to the user, so rather than '\xbeef' the user
would see '\357\276'.
This patch fixes this problem by checking the convertibility of a wide
character before printing it.
New in v3: Correctly check result of wcrtomb
New in v2: Skip the new check if the host encoding is UTF-8.
|
|
assoc.exp does this:
print pck.value := (Left => 3, Center => 7, Pck.Right => 2)
However, the test case is constructed so that "Center" has multiple
meanings: it might name the enumerator here:
type Posn is (Left, Center, Right);
or it might refer to this variable:
Center : Pck.Posn := Pck.Right;
The correct answer in this case is to pick the enumerator, because
that is the array's index type.
Originally I didn't fix this problem and instead used a kfail.
However, this same problem turned up again (in a slightly different
form) when the libgnat debuginfo was installed.
This patch fixes the bug by introducing new code in
ada_name_association::assign to handle the enumeration case.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33896
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33898
Tested-By: Tom de Vries <tdevries@suse.de>
|
|
This adds a helper method to ada_name_association, factoring out part
of ada_name_association::assign. This will be useful in the next
patch.
Tested-By: Tom de Vries <tdevries@suse.de>
|
|
Tom de Vries reported a regression due to a recent change in
gdb.ada/mi_exc_cond.exp. This patch should fix it.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33895
Approved-By: Tom de Vries <tdevries@suse.de>
|
|
This applies Alan's x86 commit df865c395a62 ("libgot-1 testcases")
simplifications to the respective s390 linker tests. With Alan's words:
There is no need for multiple tests of readelf --got-contents,
nor should the matching be so strict that changes in section layout
force editing of the testsuite.
ld/testsuite/
* ld-s390/binutils.exp (libgot-1*): Reduce number of tests.
* ld-s390/libgot_64-1.rd: New test.
* ld-s390/libgot_64-1a.rd: Remove.
* ld-s390/libgot_64-1b.rd: Likewise.
* ld-s390/libgot_64-1c.rd: Likewise.
* ld-s390/libgot_64-1d.rd: Likewise.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
|
|
|
|
I realized today that gdb.ada/assoc.exp has a kfail without a
corresponding bug report. So I filed a bug and this patch updates the
test.
|
|
I forgot to add 2026 to a copyright date in a file I recently
committed (but had originally submitted in 2025). Thinking this might
have happened before, I re-ran gdb/copyright.py and it pointed out a
surprising number of such changes.
|
|
Add debug_type, similar to debug_val and debug_exp:
...
(gdb) call debug_type (type)
array (1 .. 4) of character(gdb)
...
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This patch adds a .clang-format file to the gdb repository.
The resulting reformatting is what I'd describe as "ok but not great".
There are a few variances from our normal style, some discussed in
comments in the file, and some in the bug.
I've somewhat come around to the idea that some ugliness is
acceptable, particularly because I regularly see code that's already
ugly anyway -- either in formatting or along some other dimension.
I don't know of a way to enforce a particular version. I have only
tried clang-format 18 with this particular file, though Kevin Buettner
reported trying 19-21 as well. I've documented this in the file.
I used "AllowShortFunctionsOnASingleLine: InlineOnly" as previously
discussed. I feel that the spirit of the GNU style is that vertical
space is free, and we should use "None" here. (This goes against
something we previously decided on the list, though.)
The file is in the root directory for ease of use.
For the time being you should not bulk reformat files. I think we
should have a flag day for this, but at some later point. See the
earlier discussion for details.
New in v4:
* Comment fixes
* Remove ForEachMacros - no longer correct
* Remove IncludeCategories - no longer correct
New in v5:
* More fixes to the comments
New in v6:
* Removed 'StatementMacros' setting, we're no longer using
PyObject_HEAD
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30098
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
This reverts commit 02646a4c561ec88491114b87950cbb827c7d614c. See:
https://inbox.sourceware.org/gdb-patches/20260203185528.946918-1-guinevere@redhat.com
This commit introduced a non backward compatible change to how GDB
handled skip files. Something like:
skip -gfile dir/*.c
no longer matches every file within 'dir/', but now matches every file
in 'dir/' and within every sub-directory of 'dir/', which might not be
what the user wanted.
The original intention behind the commit is solid, we just need to
find a better implementation.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33872
|
|
PR 33897
* dwarf.c (display_debug_frames): Don't calculate "look_for"
until we have checked that cie_off is valid.
|
|
Fix the gdb.opt/empty-inline-cxx.exp test for ppc64le targets. This
test has been failing since it was introduced in commit:
commit 196349e7e5939c2bef11fd93f098401d5b7111a5
Date: Fri Sep 20 13:56:27 2024 +0100
gdb: handle empty ranges for inline subroutines
The failure looks like this:
(gdb) bt
#0 0x00000001000008d4 in main () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.opt/empty-inline-cxx.cc:63
(gdb) FAIL: gdb.opt/empty-inline-cxx.exp: opt_level=Og: backtrace in main
This is almost correct, except the test is not expecting the
"0x00000001000008d4 in" part. This first backtrace is done
immediately after the 'runto_main' call so the fact that we're not
stopped exactly at the start of line 63 is a little strange, but isn't
a critical part of this test. As such I've updated the test pattern
to accept an optional address within the backtrace.
This failure was brought to my attention in this mailing list post:
https://inbox.sourceware.org/gdb-patches/7fd2cc58-33ab-420a-83ba-e98cae69b028@linux.ibm.com
The test should now pass on ppc64le, and continue to pass on other
targets.
|
|
Not a significant leak, but there is no need for this strdup.
* readelf (parse_args): Don't strdup ".sframe" passed to
request_dump_byname.
|
|
Like the patch to free sframe decoder data, this also needs to ensure
the function doing the free is passed the actual context address.
bfd/
* elf-sframe.c (_bfd_elf_write_section_sframe): Don't pass
address of local var to sframe_encoder_free, pass the actual
context address.
* elf64-s390.c (elf_s390_link_hash_table_free): New function.
(elf_s390_link_hash_table_create): Set hash_table_free. Tidy
zmalloc call.
(_bfd_s390_elf_write_sframe_plt): Don't pass address of local
var to sframe_encoder_free, pass the actual context address.
* elflink.c: Include sframe-api.h.
(_bfd_elf_link_hash_table_free): Free sframe encoder data.
* elfxx-x86.c (elf_x86_link_hash_table_free): Likewise.
(_bfd_x86_elf_write_sframe_plt): Don't pass address of local
var to sframe_encoder_free, pass the actual context address.
libsframe/
* sframe.c (sframe_encode): Free context on error return path.
|
|
Call sframe_decoder_free in _bfd_elf_free_cached_info and correct
other calls to sframe_decoder_free so that sfd_info->stf_ctx is
cleared. If sfd_info->stf_ctx isn't cleared we get double frees.
* elf-sframe.c (sframe_decode): Do not pass local var sfd_ctx
address to sframe_decoder_free, pass the actual context address.
(_bfd_elf_merge_section_sframe): Likewise.
* elf.c: Include sframe-api.h.
(_bfd_elf_free_cached_info): Free sframe info.
|
|
If an error occurs after assigning tempbuf to dctx->sfd_buf, then
tempbuf will be freed twice. Avoid that by moving tempbuf and its
free on errors into the block where it is used.
* sframe.c (sframe_decode): Localise tempbuf.
|
|
|
|
Test gdb.base/chng-syms.exp sets timeout to 10 seconds unconditionally.
Not only this is not necessary as the default timeout seems to be the
same, it may actually lower the timeout if set in site.exp (which is
the common technique to setup testsuite on slow machines).
Fix this by removing timeout-setting code.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
When running gdb.ada/mi_ex_cond.exp on Debian 13, I get:
-catch-exception -c "i = 2" -e constraint_error
^error,msg="Your Ada runtime appears to be missing some debugging information.\nCannot insert Ada exception catchpoint in this configuration."
(gdb)
FAIL: gdb.ada/mi_ex_cond.exp: catch C_E if i = 2 (unexpected output)
And this is despite having the check "gnat_runtime_has_debug_info" at
the top of the file.
The problem is that the "gnat_runtime_has_debug_info" is done using the
toolchain's default mode, regarding linking to a static or shared
libgnat. On Debian, the default is specificalyl changed to be shared
[1].
However, the test forces linking to a static libgnat. The result is
that gnat_runtime_has_debug_info passes, seeing that when linking
against libgnat.so, debug info is available (distributed as separate
debug info, in /usr/lib/debug). But then, the test explicitly builds
against a static libgnat, and then fails because debug info is not
found.
I propose to remove that "force static" part of the test. It was added
in the test in the original commit that introduced it [2] without
explanation. Not forcing it makes more sense to me:
gnat_runtime_has_debug_info checks if, when using the default linking
mode of the toolchain, debug info can be found. And then, not
specifying anything to gdb_compile_ada builds the test binary with the
default mode of the toolchain. So the two are coherent.
With this patch, I see the failures shown by Tom Tromey in this patch
[3], and with his patch applied, the test passes fine.
[1] https://salsa.debian.org/toolchain-team/gcc/-/blob/ddaf85e5648d12377294ffad2032b46983c947a5/debian/patches/ada-link-lib.diff#L44-66
[2] https://gitlab.com/gnutools/binutils-gdb/-/commit/2df4d1d5c4393fd06c2bffe75499e70a8d8ac8a8
[3] https://inbox.sourceware.org/gdb-patches/20260209060847.3643023-1-tom@tromey.com/T/#u
Change-Id: I58bf94556d68e2f476430fed4334b339dcf2819c
Approved-By: Tom Tromey <tom@tromey.com>
|
|
gdb.ada/char_enum_overload.exp fails because LLVM optimizes away the
necessary function parameters. Adding a couple of calls to Do_Nothing
makes this test work.
|
|
In the support for FEAT_TLBID feature patch, few <operation>
variants are added to the aarch64_sys_regs_tlbi table,
and that patch was missing the tests for following <operation>
variants, which are covered in this patch.
* VMALLWS2E1
* VMALLWS2E1IS
* VMALLWS2E1OS
* VMALLWS2E1NXS
* VMALLWS2E1ISNXS
* VMALLWS2E1OSNXS
|
|
releases there should be both binutils.tar and binutils-with-gold.tar files.
|
|
Add a missing error check, and make another error check a little more
stringent. If it were ever possible for oav2_parse_attr to return
zero, the loop would not terminate.
* elf-attrs.c (oav2_parse_subsection): Check read_ntbs return
for errors. Tidy loop reading attrs, and error on <= 0.
|
|
It is possble for elf_object_p to hit errors after or during the loop
processing sections with bfd_section_from_shdr. That means we might
have some object attributes malloc'd. They need cleaning up in
elf_object_p because bfd_check_format_matches only handles tidying of
bfd_alloc memory.
* elfcode.h (elf_object_p): Free object attributes on failure.
|
|
|
|
When libgnat debuginfo is installed, gdb.ada/mi_ex_cond.exp will fail.
I tracked this down to two problems:
1. The MI output has changed a little. This probably wasn't noticed
because this test only works when libgnat debuginfo is available,
which isn't the norm. This patch updates the expected output.
2. The variable "I" in the test triggers the Ada disambiguation menu,
because some variables named "I" are visible in libgnat. While at
its root this is a deeper problem in the Ada support, it's
complicated to fix this. Since this isn't the point of the test,
this patch just renames the variable in question.
There are some other Ada tests that fail in this setup, but I think
they are a bit more complicated to fix.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
We have many times the pattern:
current_source_location *loc
= current_source_key.get (pspace);
if (loc == nullptr)
loc = current_source_key.emplace (pspace);
return loc;
I thought it would be nice to have it directly part of registry::key.
Add a try_emplace method, which is like emplace, except that it returns
the existing data, if any. The try_emplace name comes from similar
methods in std::map or gdb::unordered_map
(ankerl::unordered_dense::map).
Replace as many callers as I could find to use it.
Change-Id: I21f922ca065a354f041caaf70484d6cfbcbb76ed
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Since we use C++ and not C, I think that we should gradually move to
using references for things that can never be nullptr. One example of
this is the return value of the emplace method.
Change it to return a reference, and (to keep the patch straightforward)
update all callers to take the address. More patches could follow to
propagate the use of references further.
Change-Id: I725539694cf496f8288918cc29d7aaae9aca2292
Approved-By: Tom Tromey <tom@tromey.com>
|
|
scm-arch.c uses "void" as a registry data type here:
/* Use a 'void *' here because it isn't guaranteed that SCM is a
pointer. */
static const registry<gdbarch>::key<void, gdb::noop_deleter<void>>
arch_object_data;
This conflicts with my subsequent patch that makes the emplace method
return a reference because it's not valid to have `void &`.
Circumvent the problem by defining a structure type to use in the
registry, instead of storing the SCM directly. I think that makes the
code more straightforward and less hacky anyway (at the cost of one
extra allocate per gdbarch that you would use in your Guile code...).
Change-Id: I8d92234a9b0384098fa066dc79a42195dee7ca04
Approved-By: Tom Tromey <tom@tromey.com>
|
|
|
|
Andrew pointed out that some code in cli-logging.c should use the
filename style. This patch fixes these spots.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This changes a couple of spots to use std::make_unique rather than
'new'. This is a bit more idiomatic. I've only touched code
involving ui_file here, there are plenty more changes like this that
could be made, but I considered those unrelated to this series.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This changes timestamped_file to own the stream it wraps. This is
done simply because it was appropriate for all the code using this
class.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
PR gdb/33531 points out that while some "set logging" commands will
warn if you attempt to change settings when logging is already active,
"set logging file" does not. This patch corrects this oversight.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33531
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
tee_file is no longer used and can be deleted.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This patch changes how gdb output redirection is done.
Currently, output is done via the UI. gdb_stdout, for example, is a
define the expands to an lvalue referencing a field in the current UI.
When redirecting, this field may temporarily be reset; and when
logging is enabled or disabled, this is also done.
This has lead to bugs where the combination of redirection and logging
results in use-after-free. Crashes are readily observable; see the
new test cases.
This patch upends this. Now, gdb_stdout is simply an rvalue, and
refers to the current interpreter. The interpreter provides ui_files
that do whatever rewriting is needed (mostly for MI); then output is
forward to the current UI via an indirection (see the new
ui::passthrough_file).
The ui provides paging, logging, timestamps, and the final stream that
writes to an actual file descriptor.
Redirection is handled at the ui layer. Rather than changing the
output pipeline, new ui_files are simply swapped in by rewriting
pointers, hopefully with a scoped_restore.
Redirecting at the ui layer means that interpreter rewriting is still
applied when capturing output. This fixes one of the reported bugs.
Not changing the pipeline means that the problems with the combination
of redirect and logging simply vanish. Logging just changes a flag
and doesn't involve object destruction.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17697
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28620
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28798
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28948
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This adds a new logging_file subclass of ui_file. This new subclass
handles the details of logging, by consulting the relevant globals.
I think a dependency on globals is warranted here, because the logging
settings themselves are global.
The idea of this approach is that rather than modifying the output
pipeline in response to logging commands, a logging_file will simply
always be in the pipeline, and will then react to the appropriate
settings. ("Appropriate" because there are tests that the logger
doesn't immediately react to changes, so it captures settings at the
moment logging starts.)
The new code isn't actually used yet -- nothing in this patch
constructs a logging_file. It's separate for easier review.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
While working on this series, I found a number of odd styling issues
recurred. For instance, the issue where the pager would lose track
and style subsequent output incorrectly reappeared.
It turned out that different ui_file objects in the output pipeline
would get confused about their current style. And, looking deeper at
this, I realized that mainly it is the pager that really needs to
track the current style at all. All the other file implementations
can be purely reactive (except the buffered stream code, as Andrew
pointed out).
This patch moves m_applied_style from ui_file and into pager_file.
This necessitated making ui_file::vprintf virtual, so that the base
class could pass in the "plain" style as the starting point, whereas
the pager could use the applied style. (I did not investigate whether
this was truly necessary, and I somewhat suspect it might not be.)
This straightforward approach caused some regressions, mostly
involving extra ANSI escapes being emitted. I fixed most of these by
arranging for ui_out::call_do_message to track styles a little more
thoroughly.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
IMO it looks a little nicer if cli-style.c:do_show uses a single
printf to display its output. This is also slightly more
i18-friendly.
I noticed this while debugging a case where multiple redundant ANSI
escapes were sent. In conjunction with a subsequent patch, this
approach happens to fix that problem. FWIW I don't consider such
redundancies to be bugs, but it was convenient to change this one.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
A while back, I removed the ui_file::can_page method. It wasn't
needed for a while, due to how output redirections were done.
With the new approach in this series, it's convenient to have this
method again, because the pipeline downstream from the pager won't
change when logging -- the logging filter will be enabled or disabled
without reconfiguring the pipeline. This method lets the pager adapt
correctly.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
I noticed that when creating a new output_unit, all the calls looked
like:
m_buffered_output.emplace_back ("", indent).m_stream = stream;
It seems better to simply pass the stream to the constructor.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
I noticed that fputs_highlighted writes a single character at a time.
It's more idiomatic to use ui_file::write.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
gdb_stdtargin is only used in a single place; and, strangely, does not
follow the current UI. This patch removes the global and changes the
one user to use gdb_stdin instead.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
gdb_stdin is never overridden, so it doesn't need to be an lvalue.
This patch changes how it is implemented.
Future patches will change the other streams here, but I thought since
this one is truly different from the others, it should be handled
separately.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
The buffered stream code is currently in ui-out.h and ui-out.c, which
seems weird because it seems more like a low-level ui-file idea (for
the most part, it does also affect some ui_out).
This patch moves the declarations to a new header file,
buffered-streams.h and the implementation to buffered-streams.c. This
seems cleaner to me.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This rewrites get_unbuffered to use iteration rather than recursion.
I also noticed a typo in the documentation comment, so that's fixed as
well.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This turns wrapped_file into a template, specialized by the type of
pointer it holds. This makes it cleaner to have it own the stream it
points to.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Currently, most of the output streams are handled by the UI -- with
the sole exception of gdb_stdtarg. This doesn't make sense to me, and
it is is useful in the longer term to make it per-UI so that the
logging approach can be uniform across all streams. So, this moves it
to the UI, following the implementation approach of all other output
streams.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
pager_file overrides write_async_safe but does so in the same way as
its superclass. So, remove the unnecessary override.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
