Previously, TARGET_WNOHANG was cleared if a target supported async
mode even if async mode wasn't currently enabled. This change only
permits TARGET_WNOHANG if async mode is enabled.
Now that target_resume always enables async mode after target::resume
returns, these calls are redundant.
The other place that target resume methods are invoked outside of
target_resume are as the beneath target in record_full_wait_1. In
this case, async mode should already be enabled when supported by the
target before the resume method is invoked due to the following:
In general, targets which support async mode run as async until
::wait returns TARGET_WAITKIND_NO_RESUMED to indicate that there are
no unwaited for children (either they have exited or are stopped).
When that occurs, the loop in wait_one disables async mode. Later
if a stopped child is resumed, async mode is re-enabled in
do_target_resume before waiting for the next event.
In the case of record_full_wait_1, this function is invoked from the
::wait target method when fetching an event. If the underlying
target supports async mode, then an earlier call to do_target_resume
to resume the child reporting an event in the loop in
record_full_wait_1 would have already enabled async mode before
::wait was invoked. In addition, nothing in the code executed in
the loop in record_full_wait_1 disables async mode. Async mode is
only disabled higher in the call stack in wait_one after ::wait
returns.
It is also true that async mode can be disabled by an
INF_EXEC_COMPLETE event passed to inferior_event_handle, but all of
the places that invoke that are in the gdb core which is "above" a
target ::wait method.
Note that there is an earlier call to enable async mode in
linux_nat_target::resume. That call also marks the async event pipe
to report an existing event after enabling async mode, so it needs to
stay.
Enabling async mode above the target layer removes duplicate code in
::resume methods of async-capable targets. Commit 5b6d1e4fa4f
("Multi-target support") enabled async mode in do_target_resume after
target_resume returns which is a step in this direction. However,
other callers of target_resume such as target_continue do not enable
async mode. Rather than enabling async mode in each of the callers
after target_resume returns, enable async mode at the end of
target_resume.
This pulls out the implementation of an event pipe used to implement
target async support in both linux-low.cc (gdbserver) and linux-nat.c
(gdb).
This will be used to replace the existing event pipe in linux-low.cc
and linux-nat.c in future commits.
Co-Authored-By: Lancelot SIX <lsix@lancelotsix.com>
Currently there are two problems with the detection of
source-highlight via pkg-config in GDB's configure script:
1. The LDFLAGS variable is used to pass the 'pkg-config --libs' output
to AC_LINK_IFELSE, which results in the "-L/some/path
-lsource-highlight" preceding the conftest.cpp, which can result in a
failure to find symbols referenced in conftest.cpp, if the linker is
using --as-needed by default.
2. The CFLAGS variable is used to pass the 'pkg-config --cflags'
output to AC_LINK_IFELSE. However, as the current language is C++,
AC_LINK_IFELSE will actuall use CXXFLAGS, not CFLAGS, so any flags
returned from pkg-config will not be seen.
This patch fixes both of these mistakes, allowing GDB to correctly
configure and build using source-highlight installed into a custom
prefix, e.g. ~/opt/gdb-git (because the system version of
source-highlight is too old).
The INTERNAL_GDBFLAGS runtest variable was updated in 55c3ad88013
([gdb/testsuite] Prevent pagination in GDB_INTERNALFLAGS, 2020-10-26) to
disable pagination, and in aae1c79a03a (PR python/12227..., 2010-12-07)
to point to the data directory, but its default value mentioned in the
testsuite's README was not kept up to date.
To avoid it getting out of sync even more, point the reader to the
definition of the variable in lib/gdb.exp, and move the explanation of
the different flags there. Also adjust the example in the README
so it follows the flags added in 55c3ad88013.
Change-Id: I3533608a7d6ae5198af09c7dc7743bde24c19ed7
Using dummy entry in riscv_supported_std_ext cause confusing and wrongly
support `b` and `k` extensions.
bfd/
* elfxx-riscv.c (riscv_supported_std_ext): Drop unsupported
extensions.
(riscv_ext_canonical_order): New.
(riscv_init_ext_order): Use riscv_ext_canonical_order rather
than riscv_supported_std_ext to compute canonical order.
V2 Changes:
- Use `*ext` rather than `*ext != NULL` for checking is reach end of
string.
"DO NOT EDIT!" says the comment at the top of bfd-in2.h. Move the new
type field where it belongs.
PR ld/28841
* section.c (struct bfd_section): Add type. Formatting.
(BFD_FAKE_SECTION): Formatting.
* bfd-in2.h: Regenerate.
This was left in subdirs because of the dynamic cgen usage. However,
we can move this breakpoint call to runtime and let gdb detect whether
the symbol exists.
I saw some failures in the test gdb.mi/mi-multi-commands.exp that I
added recently. This test was added in commit:
commit d08cbc5d3203118da5583296e49273cf82378042
Date: Wed Dec 22 12:57:44 2021 +0000
gdb: unbuffer all input streams when not using readline
The failures I see only occurred when my machine was very heavily
loaded.
In this test I send multiple commands from dejagnu to gdb with a
single send_gdb call. In a well behaving world what I want to happen
is that the gdb console sees both commands arrive and echos the text
of those commands. Then gdb starts processing the first command,
prints the result, and then processes the second command, and prints
the result.
However, what I saw in my loaded environment was that only after
sending the two commands, only the first command was echoed to gdb's
terminal. Then gdb started processing the first command, and started
to write the output. Now, mixed in with the first command output, the
second command was echoed to gdb's terminal. Finally, gdb would
finish printing the first command output, and would read and handle
the second command.
This mixing of command echoing with the first command output was
causing the test matching patterns to fail.
In this commit I change the command I use in the test from a CLI
command to an MI command, this reduces the number of lines of output
that come from the test, CLI commands sent through the MI interpreter
are echoed back like this:
(gdb)
set $a = "FIRST COMMAND"
&"set $a = \"FIRST COMMAND\"\n"
^done
(gdb)
While this is not the case for true MI command:
(gdb)
-data-evaluate-expression $a
^done,value="\"FIRST COMMAND\""
(gdb)
Less output makes for simpler patterns to match against.
Next, when sending two command to gdb I was previously trying to spot
the output of the first command followed by the prompt with nothing
between. This is not really needed, for the first command I can look
for just the ^done,value="\"FIRST COMMAND\"" string, then I can start
looking for the output of the second command.
So long as the second pattern matches up to the gdb prompt, then I can
be sure than nothing is left over in the expect buffer to muck up
later matches.
As to see the second command output gdb must have read in the second
command, the second command output never suffers from the corruption
that the first command output does.
Since making this change, I've not seen a failure in this test.
This fixes a GDB crash reported in bug pr/28900, related to reading in
some stabs debug information.
In this commit my goal is to stop GDB crashing. I am not trying to
ensure that GDB makes the best possible use of the available stabs
debug information. At this point I consider stabs a legacy debug
format, with only limited support in GDB.
So, the problem appears to be that, when reading in the stabs data, we
need to find a N_SO entry, this is the entry that defines the start of
a compilation unit (or at least the location of a corresponding source
file).
It is while handling an N_SO that GDB creates a psymtab to hold the
incoming debug information (symbols, etc).
The problem we hit in the bug is that we encounter some symbol
information (an N_PC entry) outside of an N_SO entry - that is we find
some symbol information that is not associated with any source file.
We already have some protection for this case, look (in
read_dbx_symtab) at the handling of N_PC entries of type 'F' and 'f',
if we have no psymtab (the pst variable is nullptr) then we issue a
complaint. However, for whatever reason, in both 'f' and 'F'
handling, there is one place where we assume that the pst
variable (the psymtab) is not nullptr. This is a mistake.
In this commit, I guard these two locations (in 'f' and 'F' handling)
so we no longer assume pst is not nullptr.
While I was at it, I audited all the other uses of pst in
read_dbx_symtab, and in every potentially dangerous case I added a
nullptr check, and issue a suitable complaint if pst is found to be
nullptr.
It might well be true that we could/should do something smarter if we
see a debug symbol outside of an N_SO entry, and if anyone wanted to
do that work, they're welcome too. But this commit is just about
preventing the nullptr access, and the subsequent GDB crash.
I don't have any tests for this change, I have no idea how to generate
weird stabs data for testing. The original binary from the bug report
now loads just fine without GDB crashing.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28900
While taking a look through dbxread.c I spotted a couple of places
where making use of std::string would remove the need for manual
memory allocation and memcpy.
During review Simon pointed out that the same code exists in
xcoffread.c, so I've applied the same fix there too.
There should be no user visible changes after this commit.
A have had situation where a unfiltered output (done using
fputs_unfiltered) ended up triggering pagination. The backtrace for this was:
...
#24 0x000055839377ee4e in check_async_event_handlers () at ../../gdb/async-event.c:335
#25 0x0000558394b67b57 in gdb_do_one_event () at ../../gdbsupport/event-loop.cc:216
#26 0x0000558394587454 in gdb_readline_wrapper (prompt=0x7ffd907712d0 "--Type <RET> for more, q to quit, c to continue without paging--") at ../../gdb/top.c:1148
#27 0x0000558394707270 in prompt_for_continue () at ../../gdb/utils.c:1438
#28 0x00005583947088b3 in fputs_maybe_filtered (linebuffer=0x60c0000f4000 " [...quite big message...]", stream=0x60300028e9d0, filter=0) at ../../gdb/utils.c:1752
#29 0x0000558394708e57 in fputs_unfiltered (linebuffer=0x60c0000f4000 " [...quite big message...]", stream=0x60300028e9d0) at ../../gdb/utils.c:1811
...
This comes from what appears to be a oversight in fputs_maybe_filtered. This
function has a FILTER parameter which if true makes the function pause after
every screenful (i.e. triggers pagination).
The filter parameter is correctly used to guard the first place where
prompt_for_continue. There is a second place in the function which can call
prompt_for_continue, but is currently unguarded. I believe that this is an
oversight, this patch fixes that.
Tested on Linux-x86_64, no regression observed.
Change-Id: Iad8ffd50a87cf20077500878e2564b5a7dc81ece
As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 this
code will typically wait4() a second time on the same process that was
already wait4()'d a few lines above. While this used to be
harmless/idempotent (when we assumed that the process already exited),
this now causes a deadlock in the WIFSTOPPED case.
The early (~2019) history of bug #24069 cautiously suggests to use
WNOHANG instead of outright deleting the call. However, tests on the
current version of Darwin (Big Sur) demonstrate that gdb runs just fine
without a redundant call to wait4(), as would be expected.
Notwithstanding the debatable value of conserving bug compatibility with
an OS release that is more than a decade old, there is scant evidence of
what that double-wait4() was supposed to achieve in the first place - A
cursory investigation with `git blame` pinpoints commits bb00b29d7802
and a80b95ba67e2 from the 2008-2009 era, but fails to answer the
"why" question conclusively.
Co-Authored-By: Philippe Blain <levraiphilippeblain@gmail.com>
Change-Id: Id4e4415d66d6ff6b3552b60d761693f17015e4a0
This adds a constructor to bound_minimal_symbol, to avoid a build
failure with clang that Simon pointed out.
I also took the opportunity to remove some redundant initializations,
and to change one use of push_back to emplace_back, as suggested by
Simon.
Much of the gas source and older BFD source use "long" for function
parameters and variables, when other types would be more appropriate.
This patch fixes one of those cases. Dollar labels and numeric local
labels do not need large numbers. Small positive itegers are usually
all that is required. Due to allowing longs, it was possible for
fb_label_name and dollar_label_name to overflow their buffers.
* symbols.c: Delete unnecessary forward declarations.
(dollar_labels, dollar_label_instances): Use unsigned int.
(dollar_label_defined, dollar_label_instance): Likewise.
(define_dollar_label): Likewise.
(fb_low_counter, fb_labels, fb_label_instances): Likewise.
(fb_label_instance_inc, fb_label_instance): Likewise.
(fb_label_count, fb_label_max): Make them size_t.
(dollar_label_name, fb_label_name): Rewrite using sprintf.
* symbols.h (dollar_label_defined): Update prototype.
(define_dollar_label, dollar_label_name): Likewise.
(fb_label_instance_inc, fb_label_name): Likewise.
* config/bfin-lex.l (yylex): Remove unnecessary casts.
* expr.c (integer_constant): Likewise.
* read.c (read_a_source_file): Limit numeric label range to int.
There are quite a few ubsan warnings in gas. This one disappears with
a code tidy.
* read.c (s_app_line): Rename 'l' to 'linenum'. Avoid ubsan
warning.
BFD generally doesn't handle anything but a power of two section
alignment, and ELF sh_addralign is required to be an integral power of
two (or zero) by the ELF spec. Of course this is ignored by fuzzers,
and because bfd_log2 rounds up, we can end up with alignment_power
being 32 on a 32-bit object or 64 on a 64-bit object. That then
triggers ubsan warnings in places like bfd_update_compression_header
where we want to convert from alignment_power back to an alignment.
I suppose we could reject object files that have non-compliant
sh_addralign, but I think it's also reasonable to use the greatest
power of two divisor of sh_addralign, ie. the rightmost 1 bit.
* elf.c (_bfd_elf_make_section_from_shdr): Use greatest power
of two divisor of sh_addralign.
(_bfd_elf_assign_file_position_for_section): Likewise.
(assign_file_positions_for_non_load_sections): Likewise.
There was an omission on 3e6dc39ed7a8 "sim/testsuite: Set
global_cc_os also when no compiler is found"; global_cc_os
wasn't set for other than the primary target, which means
that the "unguarded" use of global_cc_os in
testsuite/cris/c/c.exp caused the dreaded "ERROR: can't read
"global_cc_os": no such variable" when e.g. configuring for
pru-elf and doing "make check-sim". Better initializing
both variables at the top to default values, rather than
adding another single 'set global_cc_os ""', to reduce the
risk of not setting them properly if or when that
if-statement-chain is made longer.
sim/testsuite:
* lib/sim-defs.exp (sim_init_toolchain): Default
global_cc_os and global_cc_works properly, before if-chain.
Add has_sib to struct instr_info and use SIB info only if ins->has_sib
is true.
PR binutils/28892
* i386-dis.c (instr_info): Add has_sib.
(get_sib): Set has_sib.
(OP_E_memory): Replace havesib with ins->has_sib.
(OP_VEX): Use ins->sib.index only if ins->has_sib is true.
It is possible for a compiler to optimize a function in a such ways that
the function does not follow the calling convention of the target. In
such situation, the compiler can use the DW_AT_calling_convention
attribute with the value DW_CC_nocall to tell the debugger that it is
unsafe to call the function. The DWARF5 standard states, in 3.3.1.1:
> If the value of the calling convention attribute is the constant
> DW_CC_nocall, the subroutine does not obey standard calling
> conventions, and it may not be safe for the debugger to call this
> subroutine.
Non standard calling convention can affect GDB's assumptions in multiple
ways, including how arguments are passed to the function, how values are
returned, and so on. For this reason, it is unsafe for GDB to try to do
the following operations on a function with marked with DW_CC_nocall:
- call / print an expression requiring the function to be evaluated,
- inspect the value a function returns using the 'finish' command,
- force the value returned by a function using the 'return' command.
This patch ensures that if a command which relies on GDB's knowledge of
the target's calling convention is used on a function marked nocall, GDB
prints an appropriate message to the user and does not proceed with the
operation which is unreliable.
Note that it is still possible for someone to use a vendor specific
value for the DW_AT_calling_convention attribute for example to indicate
the use of an alternative calling convention. This commit does not
prevent this, and target dependent code can be adjusted if one wanted to
support multiple calling conventions.
Tested on x86_64-Linux, with no regression observed.
Change-Id: I72970dae68234cb83edbc0cf71aa3d6002a4a540
Add an argument to the get_return_value function to indicate the symbol
of the function the debuggee is returning from. This will be used by
the following patch.
Since the function return type can be deduced from the symbol remove the
value_type argument which becomes redundant.
No user visible change after this patch.
Tested on x86_64-linux.
Change-Id: Idf1279f1f7199f5022738a6679e0fa63fbd22edc
Co-authored-by: Simon Marchi <simon.marchi@polymtl.ca>
...when we know we have a working compiler. This will reduce
the risk of faulty edits by exposing them rather than hiding
them as "unresolved". It also harmonizes behavior with that of
run_sim_test.
* c/c.exp: Mark C tests failing compilation test errors.
Calls to basename were added here as part of commit
e1e1ae6e9b5e "sim: testsuite: fix objdir handling", but that
commit missed adding "#include <libgen.h>" or the equivalent
GNU extension, see basename(3). Fixing that shows a logical
error in the change to openpf1.c; the non-/-prefixed
code-path was changed instead of the "/"-prefixed code-path,
which is the one executed after that commit.
For "newlib" these tests failed linking after that commit.
Recent newlib has the (asm-renamed) GNU-extension-variant of
basename, but we're better off not using it at all.
Unfortunately, compilation failures for C tests run by the
machinery in c.exp are currently just marked "unresolved",
in contrast to C and assembler tests run by calling
run_sim_test.
The interaction of calling with the full program-path vs.
use of --sysroot exposes a consistency problem: when
--sysroot is used, argv[0] isn't the path by which the
program can find itself. It's undecided whether argv[0] for
the program running in the simulator should be edited
(related to the naked argument to the simulator before
passing on to the simulated program) to remove a leading
--sysroot. Either way, such a change would be out of scope
for this commit.
* c/stat3.c (mybasename): New macro. Use it instead of basename.
* c/openpf1.c: Correct basename-related change and update related
comment.
