| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This removes the need for the caller to explicitly manage the memory
for the returned system call list. The sole caller only needed the
system call numbers rather than the full syscall structures.
get_syscalls_by_group now uses a boolean return value to indicate if
the requested group exists.
gdb/ChangeLog:
* break-catch-syscall.c (catch_syscall_split_args): Pass 'result'
to get_syscalls_by_group.
* xml-syscall.c [!HAVE_LIBEXPAT] (get_syscalls_by_group): Return
false.
[HAVE_LIBEXPAT] (xml_list_syscalls_by_group): Append syscall
numbers to an existing vector of integers and return a bool.
(get_syscalls_by_group): Accept pointer to vector of integers
and change return type to bool.
* xml-syscall.h (get_syscalls_by_group): Likewise.
|
|
* riscv-tdep.c (riscv_print_one_register_info): For MSTATUS, add
comment for SD field, and correct xlen calculation. For MISA, add
comment for MXL field, add call to register_size, and correct base
calculation.
|
|
gdb/ChangeLog:
* NEWS(New targets): Add or1k*-*-linux*.
|
|
ARI warning.
2018-12-12 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* nat/linux-osdata.c (linux_xfer_osdata_info_os_types): Forward
declare on one line to fix ARI warning.
|
|
When running the test gdb.base/annota1.exp with:
make check-gdb RUNTESTFLAGS="--target_board=native-extended-gdbserver gdb.base/annota1.exp"
I would see a failure due to some unexpected lines in GDB's output.
The extra lines (when compared with a native run) were about file
transfer from the remote back to GDB.
This commit extends the regexp for this test to allow for these extra
lines, and also splits the rather long regexp up into a list of parts.
With this change in place I see no failures for gdb.base/annota1.exp
when using the native-extended-gdbserver target board, nor with a
native run on X86-64/Linux.
gdb/testsuite/ChangeLog:
* gdb.base/annota1.exp: Update a test regexp.
|
|
I ran into a situation where attempting to make an inferior function
call would trigger an assertion, like this:
(gdb) call some_inferior_function ()
../../src/gdb/regcache.c:310: internal-error: void regcache::restore(readonly_detached_regcache*): Assertion `src != NULL' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
The problem that triggers the assertion is that in the function
save_infcall_suspend_state, we basically did this:
1. Create empty infcall_suspend_state object.
2. Fill fields of infcall_suspend_state object.
The problem is causes is that if filling any of the fields triggered
an exception then the infcall_suspend_state object would be deleted
while in a partially filled in state.
In the specific case I encountered, I had a remote RISC-V target that
claimed in its target description to support floating point registers.
However, this was not true, and when GDB tried to read a floating
point register the remote sent back an error. This error would cause
an exception to be thrown while creating the
readonly_detached_regcache, which in turn caused GDB to try and delete
an infcall_suspend_state which didn't have any register state, and
this triggered the assertion.
To prevent this problem we have two possibilities, either, rewrite the
restore code the handle partially initialised infcall_suspend_state
objects, or, prevent partially initialised infcall_suspend_state
objects from existing. The second of these seems like a better
solution.
So, in this patch, I move the filling in of the different
infcall_suspend_state fields within a new constructor for
infcall_suspend_state. Now, if generating one of those fields fails
the destructor for infcall_suspend_state will not be executed and GDB
will not try to restore the partially saved state.
With this patch in place GDB now behaves like this:
(gdb) call some_inferior_function ()
Could not fetch register "ft0"; remote failure reply 'E99'
(gdb)
The inferior function call is aborted due to the error.
This has been tested against x86-64/Linux native, native-gdbserver,
and native-extended-gdbserver with no regressions. I've manually
tested this against my baddly behaving target and confirmed the
inferior function call is aborted as described above.
gdb/ChangeLog:
* infrun.c (infcall_suspend_state::infcall_suspend_state): New.
(infcall_suspend_state::registers): New.
(infcall_suspend_state::restore): New.
(infcall_suspend_state::thread_suspend): Rename to...
(infcall_suspend_state::m_thread_suspend): ...this.
(infcall_suspend_state::registers): Rename to...
(infcall_suspend_state::m_registers): ...this.
(infcall_suspend_state::siginfo_gdbarch): Rename to...
(infcall_suspend_state::m_siginfo_gdbarch): ...this.
(infcall_suspend_state::siginfo_data): Rename to...
(infcall_suspend_state::m_siginfo_data): ...this.
(save_infcall_suspend_state): Rewrite to use infcall_suspend_state
constructor.
(restore_infcall_suspend_state): Rewrite to use
infcall_suspend_state::restore method.
(get_infcall_suspend_state_regcache): Use
infcall_suspend_state::registers method.
|
|
This commit fixes some test failures in gdb.base/varargs.exp when
running on targets with floating point hardware. Floating point
unnamed (variadic) arguments should be passed in integer registers
according to the abi.
After this commit I see no failures in gdb.base/varargs.exp on 32 or
64 bit targets with floating point hardware.
gdb/ChangeLog:
* riscv-tdep.c (riscv_call_arg_scalar_float): Unnamed (variadic)
arguments are passed in integer registers.
(riscv_call_arg_complex_float): Likewise.
|
|
Valgrind reports leaks in all linux osdata annex transfers of linux-osdata.c.
A typical leak (this one is of gdb.base/info-os) is:
==10592== VALGRIND_GDB_ERROR_BEGIN
==10592== 65,536 bytes in 1 blocks are definitely lost in loss record 3,175 of 3,208
==10592== at 0x4C2E273: realloc (vg_replace_malloc.c:826)
==10592== by 0x409B0C: xrealloc (common-utils.c:62)
==10592== by 0x408BC3: buffer_grow(buffer*, char const*, unsigned long) [clone .part.1] (buffer.c:40)
==10592== by 0x5263DF: linux_xfer_osdata_processes(unsigned char*, unsigned long, unsigned long) (linux-osdata.c:370)
==10592== by 0x520875: linux_nat_xfer_osdata (linux-nat.c:4214)
...
The leaks are created because the linux_xfer_osdata_* functions
transfer the ownership of their 'static struct buffer' memory
to their 'static char *buf' local var, but then call buffer_free
instead of xfree-ing buf.
I see no reason why the ownership of the memory has to be transferred
from a local var to another local var, so the fix consists in dropping
the 'static char *buf' and accessing the struct buffer memory where needed.
Also, because this bug was replicated in all functions, and there was
a non neglectible amount of duplicated code, the setup and usage
of the 'static struct buffer' is factorized in a new function
common_getter. The buffer for a specific annex is now a member
of the struct osdata_type instead of being a static var of each
linux_xfer_osdata_* function.
Thanks to this, all the linux_xfer_osdata_* do not have
anymore any logic related to the partial transfer of data: they now
only build the xml data in a struct buffer.
This all removes about 300 SLOC.
Note: git diff/git format-patch shows a lot of differences only due
to space changes/indentation changes.
So, git diff -w helps to look only at the relevant differences.
gdb/ChangeLog
2018-12-11 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* nat/linux-osdata.c (common_getter): New function.
(struct osdata_type): Change getter to take_snapshot.
Add LONGEST len_avail and struct buffer buffer.
Change all elements in the initializer.
Add an element for the list of types.
(linux_xfer_osdata_info_os_types): New function.
(linux_common_xfer_osdata): Use common_getter for the list of types.
Replace getter call by common_getter.
(linux_xfer_osdata_cpus): Remove args READBUF, OFFSET, LEN.
Add arg BUFFER. Only keep the code that adds data in BUFFER.
(linux_xfer_osdata_fds): Likewise.
(linux_xfer_osdata_modules): Likewise.
(linux_xfer_osdata_msg): Likewise.
(linux_xfer_osdata_processes): Likewise.
(linux_xfer_osdata_processgroups): Likewise.
(linux_xfer_osdata_sem): Likewise.
(linux_xfer_osdata_shm): Likewise.
(linux_xfer_osdata_isockets): Likewise.
(linux_xfer_osdata_threads): Likewise.
|
|
|
|
|
|
The FPU is optional on RISC-V. The gdb.base/float.exp test currently
assumes that an fpu is always available on RISC-V. Update the test so
that this is not the case.
gdb/testsuite/ChangeLog:
* gdb.base/float.exp: Handle RISC-V targets without an FPU.
|
|
This fixes an ARI warning in riscv-tdep.c that whitespace before a
gdb/ChangeLog:
* riscv-tdep.c (riscv_register_name): Fix ARI warning by removing
leading whitespace before #include line.
|
|
A failure that seems to cause a long/infinite time is the following:
For a not clear reason, tid-reuse.c spawner thread sometimes gets an error:
tid-reuse: /bd/home/philippe/gdb/git/build_moreaa/gdb/testsuite/../../../moreaa/gdb/testsuite/gdb.threads/tid-reuse.c:58: spawner_thread_func: Assertion `rc == 0' failed.
which causes a SIGABRT to be trapped by gdb, and tid-reuse does not reach the
after_count breakpoint:
Thread 2 "tid-reuse" received signal SIGABRT, Aborted.
[Switching to Thread 0x7ffff7518700 (LWP 10368)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) FAIL: gdb.threads/tid-reuse.exp: continue to breakpoint: after_count
After that, tid-reuse.exp gets the value of reuse_time, but this one kept its
initial value of -1 (as unsigned) :
print reuse_time
$1 = 4294967295
(gdb) PASS: gdb.threads/tid-reuse.exp: get reuse_time
tid-reuse then dies, and the .exp script continues (with some FAIL)
till it executes:
set timeout [expr $reuse_time * 2]
leading to the error:
(gdb) ERROR: integer value too large to represent as non-long integer
while executing
"expect {
-i exp8 -timeout 8589934590
-re ".*A problem internal to GDB has been detected" {
fail "$message (GDB internal error)"
gdb_intern..."
("uplevel" body line 1)
invoked from within
"uplevel $body" ARITH IOVERFLOW {integer value too large to represent as non-long integer} integer value too large to represent as non-long integer
ERROR: GDB process no longer exists
and then everything blocks.
This last 'GDB process no longer exists' is strange, as I still see the gdb
when this all blocks, e.g.
philippe 16058 31085 0 20:30 pts/15 00:00:00 /bin/bash -c rootme=`pwd`; export rootme; srcdir=../../../binutils-gdb/gdb/testsuite ; export srcdir ; EXPECT=`if [
philippe 16386 16058 0 20:30 pts/15 00:00:00 expect -- /usr/share/dejagnu/runtest.exp --status GDB_PARALLEL=yes --outdir=outputs/gdb.threads/tid-reuse gdb.thre
philippe 24848 16386 0 20:30 pts/20 00:00:00 /bd/home/philippe/gdb/git/build_binutils-gdb/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory /bd/home/philip
This patch gives a default value of 60, so that if ever something wrong happens
in tid-reuse, then the value retrieved by the .exp script stays in a reasonable
range.
Simon verified the patch by:
"I replaced the pthread_create call with the value 1 to simulate a
failure, and the test succeeds to fail quickly with your patch applied.
Without your patch, I get the infinite hang that you describe."
Compared to V1:
As suggested by Pedro, this version checks the pthread calls return
code (in particular of pthread_create) and reports the failure reason,
instead of just aborting.
gdb/testsuite/ChangeLog
2018-12-09 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* gdb.threads/tid-reuse.c (REUSE_TIME_CAP): Declare as 60.
(reuse_time): Initialize to REUSE_TIME_CAP.
(check_rc): New function.
(main): Use REUSE_TIME_CAP instead of hardcoded 60.
Check pthread_create rc.
(spawner_thread_func): Check pthread_create and pthread_join rc.
|
|
On some systems where ncurses is only available in the "wide" version
(compiled with --with-widec), there might be no libtinfo.so, only a
libtinfow.so. Look for libtinfow in addition to libtinfo.
gdb/ChangeLog:
YYYY-MM-DD Simon Marchi <simon.marchi@ericsson.com>
Дилян Палаузов <dilyan.palauzov@aegee.org>
PR gdb/23950
* configure.ac: Search for tgetent in libtinfow.
* configure: Re-generate.
|
|
internal process handle.
Valgrind reports the below leak:
==25327== VALGRIND_GDB_ERROR_BEGIN
==25327== 672 bytes in 1 blocks are definitely lost in loss record 2,759 of 3,251
==25327== at 0x4C2E07C: calloc (vg_replace_malloc.c:752)
==25327== by 0x7FDCB3E: ???
==25327== by 0x532A7A: try_thread_db_load_1 (linux-thread-db.c:828)
==25327== by 0x532A7A: try_thread_db_load(char const*, int) (linux-thread-db.c:997)
==25327== by 0x53354D: try_thread_db_load_from_sdir (linux-thread-db.c:1074)
==25327== by 0x53354D: thread_db_load_search (linux-thread-db.c:1129)
==25327== by 0x53354D: thread_db_load() (linux-thread-db.c:1187)
==25327== by 0x611AF1: operator() (functional:2127)
==25327== by 0x611AF1: notify (observable.h:106)
==25327== by 0x611AF1: symbol_file_add_with_addrs(bfd*, char const*, enum_flags<symfile_add_flag>, std::vector<other_sections, std::allocator<other_sections> >*, enum_flags<objfile_flag>, objfile*) (symfile.c:1158)
==25327== by 0x5F5C4A: solib_read_symbols(so_list*, enum_flags<symfile_add_flag>) (solib.c:691)
==25327== by 0x5F6A8B: solib_add(char const*, int, int) (solib.c:1003)
==25327== by 0x5F6BF7: handle_solib_event() (solib.c:1281)
==25327== by 0x3D0A94: bpstat_stop_status(address_space const*, unsigned long, thread_info*, target_waitstatus const*, bpstats*) (breakpoint.c:5417)
==25327== by 0x4FF133: handle_signal_stop(execution_control_state*) (infrun.c:5874)
==25327== by 0x502C29: handle_inferior_event_1 (infrun.c:5300)
==25327== by 0x502C29: handle_inferior_event(execution_control_state*) (infrun.c:5335)
==25327== by 0x5041DB: fetch_inferior_event(void*) (infrun.c:3868)
==25327== by 0x4A1E7C: gdb_wait_for_event(int) (event-loop.c:859)
...
This leak is created because a call to td_ta_new allocates some resources
that must be freed with td_ta_delete, and that was missing.
With this patch, the nr of GDB executions leaking during regression tests
decreases further from 566 to 380.
Note that the gdbserver equivalent code is properly calling
td_ta_delete: see thread_db_mourn in thread-db.c.
Tests run natively on debian/amd64, and run under valgrind.
gdb/ChangeLog
2018-12-08 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* linux-thread-db.c (struct thread_db_info): Add td_ta_delete_p.
(thread_db_err_str): Forward declare.
(delete_thread_db_info): Call td_ta_delete_p if available.
(try_thread_db_load_1): Acquire td_ta_delete address.
* nat/gdb_thread_db.h (td_ta_delete_ftype): Declare.
|
|
Back in:
commit 85ae1317add94adef4817927e89cff80b92813dd
Author: Stan Shebs <shebs@codesourcery.com>
AuthorDate: Thu Dec 8 02:27:47 1994 +0000
* source.c: Various cosmetic changes.
(forward_search_command): Handle very long source lines correctly.
a buffer with a hard limit was converted to a heap buffer:
@@ -1228,15 +1284,26 @@ forward_search_command (regex, from_tty)
stream = fdopen (desc, FOPEN_RT);
clearerr (stream);
while (1) {
-/* FIXME!!! We walk right off the end of buf if we get a long line!!! */
- char buf[4096]; /* Should be reasonable??? */
- register char *p = buf;
+ static char *buf = NULL;
+ register char *p;
+ int cursize, newsize;
+
+ cursize = 256;
+ buf = xmalloc (cursize);
+ p = buf;
However, reverse_search_command has the exact same problem, and that
wasn't fixed. We still have that "we walk right off" comment...
Recently, the xmalloc above was replaced with a xrealloc, because as
can be seen above, that 'buf' variable above was a static local,
otherwise we'd be leaking. This commit replaces that and the
associated manual buffer growing with a gdb::def_vector<char>. I
don't think there's much point in reusing the buffer across command
invocations.
While doing this, I realized that reverse_search_command is almost
identical to forward_search_command. So this commit factors out a
common helper function instead of duplicating a lot of code.
There are some tests for "forward-search" in gdb.base/list.exp, but
since they use the "search" alias, they were a bit harder to find than
expected. That's now fixed, both by testing both variants, and by
adding some commentary. Also, there are no tests for the
"reverse-search" command, so this commit adds some for that too.
gdb/ChangeLog:
2018-12-08 Pedro Alves <palves@redhat.com>
* source.c (forward_search_command): Rename to ...
(search_command_helper): ... this. Add 'forward' parameter.
Tweak to use a gdb::def_vector<char> instead of a xrealloc'ed
buffer. Handle backward searches too.
(forward_search_command, reverse_search_command): Reimplement by
calling search_command_helper.
gdb/testsuite/ChangeLog:
2018-12-08 Pedro Alves <palves@redhat.com>
* gdb.base/list.exp (test_forward_search): Rename to ...
(test_forward_reverse_search): ... this. Also test reverse-search
and the forward-search alias.
|
|
The current .dir-locals file for GDB causes files that would usually
open in c-mode (for example, files ending in .c) to open in c++-mode.
However, all of the other settings applied for c-mode appear to get
reset when the file is switched over to c++-mode.
For example, we currently say:
(c-mode . ((c-file-style . "GNU")
(mode . c++)
(indent-tabs-mode . t)
(tab-width . 8)
(c-basic-offset . 2)
(eval . (c-set-offset 'innamespace 0))
))
(c++-mode . ((eval . (when (fboundp 'c-toggle-comment-style)
(c-toggle-comment-style 1)))))
So, when we enter c++-mode `indent-tabs-mode` is reset to its global
value, as are all of the other settings listed for c-mode.
This commit copies all of the settings (except the `mode` setting)
from the c-mode list to the c++-mode list.
The emacs documentation doesn't mention that `mode` causes this
resetting behaviour, so, in case this is an emacs bug, I'm using emacs
version 26.1. Having the settings duplicated shouldn't cause any
problems except for a slight maintenance overhead.
gdb/ChangeLog:
* .dir-locals.el: Copy most of the settings from c-mode over to
c++-mode.
|
|
Up until now OpenRISC GDB only has supported bare metal debugging. This
patch adds linux userspace debugging and core dump analysis support.
The changes are loosely based on nios2 and riscv implementations.
This was tested with linux 4.20 core dumps for executables linked
against musl libc.
bfd/ChangeLog:
* elf32-or1k.c (or1k_grok_prstatus): New function.
(or1k_grok_psinfo): Likewise.
gdb/ChangeLog:
* Makefile.in (ALL_TARGET_OBS): Add or1k-linux-tdep.o.
* configure.tgt: Add or1k*-*-linux*.
* or1k-linux-tdep.c: New file.
* or1k-tdep.c (or1k_gdbarch_init): Call gdbarch_init_osabi.
|
|
Building for x86_64/-m32 with --enable-64-bit-bfd, compilation fails
with:
src/gdb/dwarf2read.c: In instantiation of ‘gdb::array_view<const unsigned char> get_gdb_index_contents_from_section(objfile*, T*) [with T = dwarf2_per_objfile]’:
src/gdb/dwarf2read.c:6266:54: required from here
src/gdb/dwarf2read.c:6192:37: error: narrowing conversion of ‘section->dwarf2_section_info::size’ from ‘bfd_size_type {aka long long unsigned int}’ to ‘size_t {aka unsigned int}’ inside { } [-Werror=narrowing]
return {section->buffer, section->size};
~~~~~~~~~^~~~
This fixes it.
gdb/ChangeLog:
2018-12-07 Pedro Alves <palves@redhat.com>
* dwarf2read.c (get_gdb_index_contents_from_section): Use
gdb::make_array_view.
|
|
Valgrind detects the following leak:
==28395== VALGRIND_GDB_ERROR_BEGIN
==28395== 5 bytes in 1 blocks are definitely lost in loss record 20 of 2,770
==28395== at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==28395== by 0x41D9E7: xmalloc (common-utils.c:44)
==28395== by 0x78BF39: xstrdup (xstrdup.c:34)
==28395== by 0x51F1AC: _initialize_language() (language.c:1175)
==28395== by 0x6B3356: initialize_all_files() (init.c:308)
==28395== by 0x66D194: gdb_init(char*) (top.c:2159)
==28395== by 0x554C11: captured_main_1 (main.c:863)
==28395== by 0x554C11: captured_main (main.c:1167)
==28395== by 0x554C11: gdb_main(captured_main_args*) (main.c:1193)
==28395== by 0x29D837: main (gdb.c:32)
==28395==
==28395== VALGRIND_GDB_ERROR_END
This is a very small leak (1 block/5 bytes), happening only once
per GDB startup as far as I can see. But this fix make the nr of leaking
GDB in the testsuite decreasing from 628 to 566.
It is unclear why a xstrdup-ed value is assigned to 'language'
at initialization time, while a static "auto" string is assigned
as part of the set_language_command.
So, that shows that it is ok to initialize 'language' directly
with "auto".
Also, I cannot find any place where 'language' is xfree-d.
No leak was detected for 'range' and 'case_sensitive', but
similarly, no indication why a static string cannot be assigned.
Regression-tested on debian/x86_64.
Also, full testsuite run under valgrind, less tests leaking,
and no dangling pointer problem detected.
gdb/ChangeLog
2018-12-05 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* language.c (_initialize_language): Fix leak by assigning
a static string to language. Same for range and case_sensitive,
even if no leak is detected for these variables.
|
|
Not all sed implementations support alternation via \| in the default
regular expressions. Instead, resort to separate sed expressions via
-e for $debugdir and $datadir. This fixes the default setting of the
auto-load directories on FreeBSD. Previously on FreeBSD the sed
invocation was a no-op causing the debugdir and datadir values to be
expanded yielding an autoload path of ':${prefix}/share/gdb'.
gdb/ChangeLog:
* configure: Re-generate.
* configure.ac: Use separate sed expressions to escape variables
in auto-load directories.
|
|
Currently, if the target announces that it has floating point
registers in its target description then GDB assumes that the hardware
float ABI should be used. However, there's nothing stopping a user
compiling a program for the soft-float abi, and then trying to run
this on a target with hardware floating point registers.
This commit adjusts the logic that decides if GDB should use the
hardware float abi. The primary decision now is based on what the ELF
currently being executed says in its headers. If the file was
compiled for h/w float abi, then GDB uses h/w float abi, otherwise s/w
float is used.
If the current BFD is not an ELF then we don't currently have a
mechanism for figuring out if the file was compiled for float or not.
In this case we disable the h/w float abi. This shouldn't be a
problem as, right now, the RISC-V linker can only produce ELFs.
If there is NO current BFD (can this happen?) then we will enable h/w
float abi if the target has floating point hardware, otherwise, s/w
float abi is used.
This commit also adds some sanity checking that the features requested
in the BFD (xlen and flen) match the target description.
For testing I ran the testsuite on a target that returns a target
description containing both integer and floating point registers, but
used a compiler that didn't have floating point support. Before this
commit I would see failures on may tests that made inferior calls
using floating point arguments, after this commit, all of these issues
are resolved. One example from the testsuite is
gdb.base/infcall-nested-structs.exp.
gdb/ChangeLog:
* riscv-tdep.c (riscv_features_from_gdbarch_info): New function.
(riscv_find_default_target_description): Use new function to
extract feature from gdbarch_info.
(riscv_gdbarch_init): Add error checks for xlen and flen between
target description and bfd headers. Be smarter about when we
think the hardware floating point abi should be used.
|
|
In the config/sim.exp file two functions are defined. Both of these
functions define local timeout variables and then call gdb_expect,
which (through a call to get_largest_timeout) will find the local
definition of timeout.
However, both of these functions set the local timeout to some
arbitrary value and print a log message for this "new" timeout just
before returning.
As in both cases, the timeout is a local variable, this final setting
of the timeout has no effect and can be removed.
As having log messages about the timeout being adjusted could cause
confusion I've removed all logging related to timeouts in this
function, timeouts are adjusted throughout the testsuite without any
logging, there doesn't seem to be any good reason why these functions
should get their own logging.
With the logging gone there seems to be little need to a local timeout
variable at all, and so I've folded the local timeout directly into
the call to gdb_expect.
gdb/testsuite/ChangeLog:
* config/sim.exp (gdb_target_sim): Remove redundant adjustment of
local timeout variable before return, and remove all local timeout
variable entirely.
(gdb_load): Likewise.
|
|
On some heavily loaded AArch64 boxes, GDB will sometimes hang forever when
the inferior creates a thread. This hang happens inside the kernel during
the ptrace call to set hardware watchpoints or hardware breakpoints.
Currently, GDB will always set hw wp/bp at the start of each thread even if
there are none set in the process.
This patch works around the issue by avoiding setting hw wp/bp if there
are none set for the process.
On an effected machine, this fix drastically reduces the racy nature of the
gdb.threads test set. I ran the entire gdb test suite across all processors
for 100 iterations, then ran the results through the racy tests script.
Without the patch, 58 .exp files in gdb.threads were marked as racy. After
the patch this reduced to the same ~14 tests as the non effected boxes.
Clearly GDB will still be subject to hangs on an effect box if hw wp/bp's are
used prior to creating inferior threads on a heavily loaded system.
To enable this in gdbserver, the sequence in gdbserver add_lwp() is switched
to the same as gdb order as gdb, to ensure the thread is registered before
calling new_thread(). This allows aarch64_linux_new_thread() to read the
ptid.
gdb/ChangeLog:
* nat/aarch64-linux-hw-point.c
(aarch64_linux_any_set_debug_regs_state): New function.
* nat/aarch64-linux-hw-point.h
(aarch64_linux_any_set_debug_regs_state): New declaration.
* nat/aarch64-linux.c (aarch64_linux_new_thread): Check if any
BPs or WPs are set.
gdb/gdbserver/ChangeLog:
* linux-low.c (add_lwp): Switch ordering.
|
|
When caching a proc using gdb_caching_proc, it will become less likely to
be executed, and consequently it's going to be harder to detect that the
proc is racy. OTOH, in general the proc is easy to rerun. So, add a
test-case to run all uncached gdb_caching_procs a number of times and detect
inconsistent results.
The purpose of caching is to reduce runtime, so rerunning is somewhat
counter-productive in that aspect, but it's better than uncached, because the
number of reruns is constant-bounded, and the increase in runtime is bound to
this test-case, and can be disabled on slow targets.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2018-12-01 Tom de Vries <tdevries@suse.de>
* gdb.base/gdb-caching-proc.exp: New file.
|
|
kinfo_getfile() requires a couple of system calls to fetch the list of
open file descriptors. This can be much cheaper than invoking fstat
on all of the values from 0 to the open file resource limit maximum.
gdb/ChangeLog:
* common/filestuff.c [HAVE_KINFO_GETFILE]: Include headers.
(fdwalk) [HAVE_KINFO_GETFILE]: Use kinfo_getfile.
|
|
Valgrind reports this leak:
==798== VALGRIND_GDB_ERROR_BEGIN
==798== 32 (24 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 447 of 3,143
==798== at 0x4C2C48C: operator new(unsigned long) (vg_replace_malloc.c:334)
==798== by 0x51D401: linespec_parser_new(ls_parser*, int, language_defn const*, program_space*, symtab*, int, linespec_result*) (linespec.c:2756)
==798== by 0x524BF7: decode_line_full(event_location const*, int, program_space*, symtab*, int, linespec_result*, char const*, char const*) (linespec.c:3271)
==798== by 0x3E8893: parse_breakpoint_sals(event_location const*, linespec_result*) (breakpoint.c:9067)
==798== by 0x3E4E7F: create_breakpoint(gdbarch*, event_location const*, char const*, int, char const*, int, int, bptype, int, auto_boolean, breakpoint_ops const*, int, int, int, unsigned int) (breakpoint.c:9248)
==798== by 0x3E55F5: break_command_1(char const*, int, int) (breakpoint.c:9434)
==798== by 0x40BA68: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1888)
==798== by 0x665300: execute_command(char const*, int) (top.c:630)
...
linespec_parser_new allocates a std::vector<symtab *> at line 2756, and stores
the pointer to this vector in PARSER_RESULT (parser)->file_symtabs. At 3
different places in linespec.c, another std::vector is assigned to a
linespec->file_symtabs, without first deleting the current value.
The leak is fixed by assigning the vector itself instead of the pointer.
Everything should be moved, so there is no significant data copy
involved.
Tested on debian/amd64, + a bunch of tests re-run under valgrind
(including the test that throws an error).
gdb/ChangeLog:
* linespec.c (symtab_vector_up): Remove.
(symtabs_from_filename): Change return type to std::vector.
(collect_symtabs_from_filename): Likewise.
(create_sals_line_offset): Assign return value of
collect_symtabs_from_filename to *ls->file_symtabs.
(convert_explicit_location_to_linespec): Remove call to release.
(parse_linespec): Likewise.
(symtab_collector) <symtab_collector>: Remove initialization of
m_symtabs.
<release_symtabs>: Change return type to std::vector<symtab *>.
<operator ()>: Adjust.
|
|
Not all of the architecture-specific FreeBSD target files were
including the right headers to enable conditionals in fbsd-nat.h after
the C++ target conversion. As a result, certain operations like 'info
auxv' and 'p $_siginfo' were not working for some native targets
(noticed on RISC-V). Fix this in a couple of ways:
1) Declare fbsd_nat_target::xfer_partial unconditionally and only use
conditionals in the function body for individual target objects.
Originally this function was only used to read the ELF auxiliary
vector, so the entire function was conditional on a macro required
for that object (KERN_AUXV_PROC). However, xfer_partial has since
grown support for additional objects. Making the function
unconditional avoids needing to add the right header to fbsd-nat.h
and allows each target object to use independent requirements.
This did require using a more explicit conditional test for the
$_siginfo support. Removing the "outer" KERN_PROC_AUXV test
enabled $_siginfo for all kernels with PT_LWPINFO, but some older
kernels (FreeBSD 6.0) exposed PT_LWPINFO with a different siginfo
format. Instead use an explicit test for when the current siginfo
format was adopted (shipped in FreeBSD 7.0). This actually enables
$_siginfo on a wider range of kernels as KERN_PROC_AUXV wasn't
introduced until FreeBSD 9.1/10.0.
2) Include <sys/proc.h> in fbsd-nat.h for the definition of
TDP_RFPPWAIT that governs support for fork following.
gdb/ChangeLog:
* fbsd-nat.c [__FreeBSD_version >= 700009] (USE_SIGINFO): Macro
defined.
(union sigval32, struct siginfo32, fbsd_siginfo_size)
(fbsd_convert_siginfo): Make conditional on USE_SIGINFO instead
of KERN_PROC_AUXV and PT_LWPINFO.
(fbsd_nat_target::xfer_partial): Define method unconditionally.
Make TARGET_OBJECT_SIGNAL_INFO conditional on USE_SIGINFO.
Make TARGET_OBJECT_AUXV conditional on KERN_PROC_AUXV.
Make TARGET_OBJECT_FREEBSD_VMMAP and
TARGET_OBJECT_FREEBSD_PS_STRINGS conditional on KERN_PROC_VMMAP
and KERN_PROC_PS_STRINGS.
* fbsd-nat.h: Include <sys/proc.h>.
(fbsd_nat_target::xfer_partial): Declare method unconditionally.
|
|
Adds riscv_linux_nat_target::read_description method to find a
suitable target description for the native linux target we are running
on.
Currently this will supply a suitably sized set of x-registers, and
will probe the kernel to see if the f-registers are readable. If they
are readable then we currently assume that the f-registers are the
same size as the x-registers as I don't know of a good way to probe
the f-register length. This will obviously need fixing in future.
As of Linux 4.19 there is no ptrace support for reading the
f-registers, this should appear in 4.20, so right now we only return
target descriptions without f-registers.
gdb/ChangeLog:
* riscv-linux-nat.c: Add 'inferior.h' and 'target-descriptions.h'
header files.
(riscv_linux_nat_target::read_description): New method.
|
|
GDB relies on the fact that if two target descriptions have the same
contents, then they will be the same object instance (having the same
address). One place where this is a requirement is in
GDBARCH_LIST_LOOKUP_BY_INFO which is used to find previously created
gdbarch objects.
In GDBARCH_LIST_LOOKUP_BY_INFO a pointer comparison is made on the
gdbarch's target description, if the pointers are different then it is
assumed the gdbarches have different, non-compatible target
descriptions.
Previously we would create duplicate target descriptions in the belief
that RISCV_GDBARCH_INIT would spot this duplication and discard the
second instance. However, this was incorrect, and instead we ended up
creating duplicate gdbarch objects.
With this commit every unique feature set will create one and only one
target description, the feature set and resulting target description
is then cached so that the same target description object can be
returned later.
Many other target avoid this problem by creating a small number of
named target descriptions, and returning one of these. However, we
currently have 8 possible target descriptions (32 vs 64 bit for x-reg
and f-reg, and h/w or s/w float abi) and creating each of these just
to avoid a dynamic cache seems pointless.
gdb/ChangeLog:
* arch/riscv.h (riscv_gdbarch_features::hash): New method.
* arch/riscv.c (struct riscv_gdbarch_features_hasher): New.
(riscv_tdesc_cache): New global.
(riscv_create_target_description): Look in the cache before
creating a new target description.
|
|
Add '==' and '!=' operators for the struct riscv_gdb_features,
allowing a small simplification.
gdb/ChangeLog:
* arch/riscv.h (riscv_gdb_features::operator==): New.
(riscv_gdb_features::operator!=): New.
* riscv-tdep.c (riscv_gdbarch_init): Make use of the inequality
operator.
|
|
Makes more of the interface related to fetching target descriptions
constant.
gdb/ChangeLog:
* arch/riscv.h (riscv_create_target_description): Make return type
const.
* arch/riscv.c (riscv_create_target_description): Likewise.
* riscv-tdep.c (riscv_find_default_target_description): Likewise.
|
|
First of all, I would like to express my gratitude to Keith Seitz, Jan
Kratochvil and Tom Tromey, who were really kind and helped a lot with
this bug. The patch itself was authored by Jan.
This all began with:
https://bugzilla.redhat.com/show_bug.cgi?id=1639242
py-bt is broken, results in exception
In summary, the error reported by the bug above is:
$ gdb -args python3
GNU gdb (GDB) Fedora 8.1.1-3.fc28
(...)
Reading symbols from python3...Reading symbols from /usr/lib/debug/usr/bin/python3.6-3.6.6-1.fc28.x86_64.debug...done.
done.
Dwarf Error: could not find partial DIE containing offset 0x316 [in module /usr/lib/debug/usr/bin/python3.6-3.6.6-1.fc28.x86_64.debug]
After a long investigation, and after thinking that the problem might
actually be on DWZ's side, we were able to determine that there's
something wrong going on when
dwarf2read.c:dwarf2_find_containing_comp_unit performs a binary search
over all of the CUs belonging to an objfile in order to find the CU
which contains a DIE at an specific offset. The current algorithm is:
static struct dwarf2_per_cu_data *
dwarf2_find_containing_comp_unit (sect_offset sect_off,
unsigned int offset_in_dwz,
struct dwarf2_per_objfile *dwarf2_per_objfile)
{
struct dwarf2_per_cu_data *this_cu;
int low, high;
const sect_offset *cu_off;
low = 0;
high = dwarf2_per_objfile->all_comp_units.size () - 1;
while (high > low)
{
struct dwarf2_per_cu_data *mid_cu;
int mid = low + (high - low) / 2;
mid_cu = dwarf2_per_objfile->all_comp_units[mid];
cu_off = &mid_cu->sect_off;
if (mid_cu->is_dwz > offset_in_dwz
|| (mid_cu->is_dwz == offset_in_dwz && *cu_off >= sect_off))
high = mid;
else
low = mid + 1;
}
For the sake of this example, let's consider that "sect_off =
0x7d".
There are a few important things going on here. First,
"dwarf2_per_objfile->all_comp_units ()" will be sorted first by
whether the CU is a DWZ CU, and then by cu->sect_off. In this
specific bug, "offset_in_dwz" is false, which means that, for the most
part of the loop, we're going to do "high = mid" (i.e, we'll work with
the lower part of the vector).
In our particular case, when we reach the part where "mid_cu->is_dwz
== offset_in_dwz" (i.e, both are false), we end up with "high = 2" and
"mid = 1". I.e., there are only 2 elements in the vector who are not
DWZ. The vector looks like this:
#0: cu->sect_off = 0; length = 114; is_dwz = false <-- low
#1: cu->sect_off = 114; length = 7796; is_dwz = false <-- mid
#2: cu->sect_off = 0; length = 28; is_dwz = true <-- high
...
The CU we want is #1, which is exactly where "mid" is. Also, #1 is
not DWZ, which is also exactly what we want. So we perform the second
comparison:
(mid_cu->is_dwz == offset_in_dwz && *cu_off >= sect_off)
^^^^^^^^^^^^^^^^^^^
Because "*cu_off = 114" and "sect_off = 0x7d", this evaluates to
false, so we end up with "low = mid + 1 = 2", which actually gives us
the wrong CU (i.e., a CU that is DWZ). Next in the code, GDB does:
gdb_assert (low == high);
this_cu = dwarf2_per_objfile->all_comp_units[low];
cu_off = &this_cu->sect_off;
if (this_cu->is_dwz != offset_in_dwz || *cu_off > sect_off)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
{
if (low == 0 || this_cu->is_dwz != offset_in_dwz)
error (_("Dwarf Error: could not find partial DIE containing "
"offset %s [in module %s]"),
sect_offset_str (sect_off),
bfd_get_filename (dwarf2_per_objfile->objfile->obfd));
...
Triggering the error we saw in the original bug report.
It's important to notice that we see the error message because the
selected CU is a DWZ one, but we're looking for a non-DWZ CU here.
However, even when the selected CU is *not* a DWZ (and we don't see
any error message), we still end up with the wrong CU. For example,
suppose that the vector had:
#0: cu->sect_off = 0; length = 114; is_dwz = false
#1: cu->sect_off = 114; length = 7796; is_dwz = false
#2: cu->sect_off = 7910; length = 28; is_dwz = false
...
I.e., #2's "is_dwz" is false instead of true. In this case, we still
want #1, because that's where the DIE is located. After the loop ends
up in #2, we have "is_dwz" as false, which is what we wanted, so we
compare offsets. In this case, "7910 >= 0x7d", so we set "mid = high
= 2". Next iteration, we have "mid = 0 + (2 - 0) / 2 = 1", and thus
we examining #1. "is_dwz" is still false, but "114 >= 0x7d" also
evaluates to false, so "low = mid + 1 = 2", which makes the loop stop.
Therefore, we end up choosing #2 as our CU, even though #1 is the
right one.
The problem here is happening because we're comparing "sect_off"
directly against "*cu_off", while we should actually be comparing
against "*cu_off + mid_cu->length" (i.e., the end offset):
...
|| (mid_cu->is_dwz == offset_in_dwz
&& *cu_off + mid_cu->length >= sect_off))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
And this is what the patch does. The idea is that if GDB is searching
for an offset that falls above the *end* of the CU being
analyzed (i.e., "mid"), then the next iteration should try a
higher-offset CU next. The previous algorithm was using
the *beginning* of the CU.
Unfortunately, I could not devise a testcase for this problem, so I am
proposing a fix with this huge explanation attached to it in the hope
that it is sufficient. After talking a bit to Keith (our testcase
guru), it seems that one would have to create an objfile with both DWZ
and non-DWZ sections, which may prove very hard to do, I think.
I ran this patch on our BuildBot, and no regressions were detected.
gdb/ChangeLog:
2018-11-30 Jan Kratochvil <jan.kratochvil@redhat.com>
Keith Seitz <keiths@redhat.com>
Tom Tromey <tom@tromey.com>
Sergio Durigan Junior <sergiodj@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1613614
* dwarf2read.c (dwarf2_find_containing_comp_unit): Add
'mid_cu->length' to '*cu_off' when checking if 'sect_off' is
inside the CU.
|
|
Given that a target's stratum is a property of the type, and not of an
instance of the type, get rid of to_stratum data field and replace it
with a virtual method.
I.e., when we have e.g., 10 target remote instances active, there's no
need for each of the instances to have their own to_stratum copy.
gdb/ChangeLog:
2018-11-30 Pedro Alves <palves@redhat.com>
* aix-thread.c (aix_thread_target) <aix_thread_target>: Delete.
<stratum>: New override.
* bfd-target.c (aix_thread_target) <aix_thread_target>: Delete.
<stratum>: New override.
* bsd-uthread.c (bsd_uthread_target) <bsd_uthread_target>: Delete.
<stratum>: New override.
* exec.c (exec_target) <exec_target>: Delete.
<stratum>: New override.
* gdbarch-selftests.c (register_to_value_test): Adjust to use the
stratum method instead of the to_stratum field.
* linux-thread-db.c (thread_db_target) <thread_db_target>: Delete.
<stratum>: New override.
(thread_db_target::thread_db_target): Delete.
* make-target-delegates (print_class): Don't print a ctor
declaration. Print a stratum method override declaration.
* process-stratum-target.h (process_stratum_target)
<process_stratum_target>: Delete.
<stratum>: New override.
* ravenscar-thread.c (ravenscar_thread_target)
<ravenscar_thread_target>: Delete.
<stratum>: New override.
* record-btrace.c (record_btrace_target)
<record_btrace_target>: Delete.
<stratum>: New override.
* record-full.c (record_full_base_target)
<record_full_base_target>: Delete.
<stratum>: New override.
* record.c (record_disconnect, record_detach)
(record_mourn_inferior, record_kill): Adjust to use the stratum
method instead of the to_stratum field.
* regcache.c (cooked_read_test, cooked_write_test): Likewise.
* sol-thread.c (sol_thread_target)
<sol_thread_target>: Delete.
<stratum>: New override.
* spu-multiarch.c (spu_multiarch_target)
<spu_multiarch_target>: Delete.
<stratum>: New override.
* target-delegates.c: Regenerate.
* target.c (target_stack::push, target_stack::unpush)
(pop_all_targets_above, pop_all_targets_at_and_above)
(info_target_command, target_require_runnable)
(target_stack::find_beneath): Adjust to use the stratum method
instead of the to_stratum field.
(dummy_target::dummy_target): Delete.
(dummy_target::stratum): New.
(debug_target::debug_target): Delete.
(debug_target::stratum): New.
(maintenance_print_target_stack): Adjust to use the stratum method
instead of the to_stratum field.
* target.h (struct target_ops) <stratum>: New method.
<to_stratum>: Delete.
<is_pushed>: Adjust to use the stratum method
instead of the to_stratum field.
|
|
This patch converts the default_child_has_foo functions to
process_stratum_target methods. This simplifies "regular"
non-inf_child process_stratum targets, since they no longer have to
override the target_ops::has_foo methods to call the default_child_foo
functions. A couple targets need to override the new defaults
(corelow and tracefiles), but it still seems like a good tradeoff,
since those are expected to be little different (target doesn't run).
gdb/ChangeLog:
2018-11-30 Pedro Alves <palves@redhat.com>
* corelow.c (core_target) <has_all_memory, has_execution>: New
overrides.
* inf-child.c (inf_child_target::has_all_memory)
(inf_child_target::has_memory, inf_child_target::has_stack)
(inf_child_target::has_registers)
(inf_child_target::has_execution): Delete.
* inf-child.h (inf_child_target) <has_all_memory, has_memory,
has_stack, has_registers, has_execution>: Delete.
* process-stratum-target.c
(process_stratum_target::has_all_memory)
(process_stratum_target::has_memory)
(process_stratum_target::has_stack)
(process_stratum_target::has_registers)
(process_stratum_target::has_execution): New.
* process-stratum-target.h (process_stratum_target)
<has_all_memory, has_memory, has_stack, has_registers,
has_execution>: New method overrides.
* ravenscar-thread.c (ravenscar_thread_target) <has_all_memory,
has_memory, has_stack, has_registers, has_execution>: Delete.
* remote-sim.c (gdbsim_target) <has_stack, has_registers,
has_execution>: Delete.
* remote.c (remote_target) <has_all_memory, has_memory, has_stack,
has_registers, has_execution>: Delete.
* target.c (default_child_has_all_memory)
(default_child_has_memory, default_child_has_stack)
(default_child_has_registers, default_child_has_execution):
Delete.
* target.h (default_child_has_all_memory)
(default_child_has_memory, default_child_has_stack)
(default_child_has_registers, default_child_has_execution):
Delete.
* tracefile.h (tracefile_target) <has_execution>: New override.
|
|
This adds a base class that all process_stratum targets inherit from.
default_thread_address_space/default_thread_architecture only make
sense for process_stratum targets, so they are transformed to
process_stratum_target methods/overrides.
gdb/ChangeLog:
2018-11-30 Pedro Alves <palves@redhat.com>
* Makefile.in (COMMON_SFILES): Add process-stratum-target.c.
* bsd-kvm.c: Include "process-stratum-target.h".
(bsd_kvm_target): Now inherits from process_stratum_target.
(bsd_kvm_target::bsd_kvm_target): Default it.
* corelow.c: Include "process-stratum-target.h".
(core_target): Now inherits from process_stratum_target.
(core_target::core_target): Don't set to_stratum here.
* inf-child.c (inf_child_target::inf_child_target): Delete.
* inf-child.h: Include "process-stratum-target.h".
(inf_child_target): Inherit from process_stratum_target.
(inf_child_target) <inf_child_target>: Default it.
<can_async_p, supports_non_stop, supports_disable_randomization>:
Delete overrides.
* process-stratum-target.c: New file.
* process-stratum-target.h: New file.
* remote-sim.c: Include "process-stratum-target.h".
(gdbsim_target): Inherit from process_stratum_target.
<gdbsim_target>: Default it.
* remote.c: Include "process-stratum-target.h".
(remote_target): Inherit from process_stratum_target.
<remote_target>: Default it.
* target.c (default_thread_address_space)
(default_thread_architecture): Delete.
* target.h (target_ops) <thread_architecture>: Now returns NULL by
default.
<thread_address_space>: Ditto.
* test-target.h: Include "process-stratum-target.h" instead of
"target.h".
(test_target_ops): Inherit from process_stratum_target.
<test_target_ops>: Default it.
* tracefile.c (tracefile_target::tracefile_target): Delete.
* tracefile.h: Include "process-stratum-target.h".
(tracefile_target): Inherit from process_stratum_target.
<tracefile_target>: Default it.
* target-delegates.c: Regenerate.
|
|
There's no need to have all target.h users seeing this type.
Also helps with a follow up patch.
gdb/ChangeLog:
2018-11-30 Pedro Alves <palves@redhat.com>
* Makefile.in (COMMON_SFILES): Add test-target.c.
* gdbarch-selftests.c: Include "test-target.h".
* regcache.c: Include "test-target.h".
* target.c (test_target_info, test_target_ops::info): Move to ...
* test-target.c: ... this new file.
* target.h (test_target_ops): Move to ...
* test-target.h: ... this new file.
|
|
Valgrind reports the below leak.
Fix the leak by using xrealloc, even for the first allocation,
as buf is static.
==29158== 5,888 bytes in 23 blocks are definitely lost in loss record 3,028 of 3,149
==29158== at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==29158== by 0x41B557: xmalloc (common-utils.c:44)
==29158== by 0x60B7D9: forward_search_command(char const*, int) (source.c:1563)
==29158== by 0x40BA68: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1888)
==29158== by 0x665300: execute_command(char const*, int) (top.c:630)
...
gdb/ChangeLog
2018-11-29 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* source.c (forward_search_command): Fix leak by using
xrealloc even for the first allocation in the loop, as buf
is static.
|
|
This fixes failures in the gdb.base/exitsignal.exp test.
gdb/ChangeLog:
PR gdb/23093
* gdb/fbsd-tdep.c (fbsd_gdb_signal_from_target)
(fbsd_gdb_signal_to_target): New.
(fbsd_init_abi): Install gdbarch "signal_from_target" and
"signal_to_target" methods.
|
|
Commit 6b1747cd1 ("invoke_xmethod & array_view") contains this change:
- argvec = (struct value **) alloca (sizeof (struct value *) * 4);
+ value *argvec_storage[3];
+ gdb::array_view<value *> argvec = argvec_storage;
However, value_x_unop still does:
argvec[2] = value_from_longest (builtin_type (gdbarch)->builtin_int, 0);
argvec[3] = 0;
This triggers an error with -fsanitize=address from userdef.exp:
ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdcf185068 at pc 0x000000e4f912 bp 0x7ffdcf184d80 sp 0x7ffdcf184d70
WRITE of size 8 at 0x7ffdcf185068 thread T0
#0 0xe4f911 in value_x_unop(value*, exp_opcode, noside) ../../binutils-gdb/gdb/valarith.c:557
[...]
I think the two assignments to argvec[3] should just be removed, and
that this was intended in the earlier patch but just missed.
This passes userdef.exp with -fsanitize=address.
gdb/ChangeLog
2018-11-29 Tom Tromey <tom@tromey.com>
* valarith.c (value_x_unop): Don't set argvec[3].
|
|
-fsanitize=address pointed out a use-after-free in gdbserver. In
particular, handle_detach could reference "process" after it was
deleted by detach_inferior. Avoiding this also necessitated changing
target_ops::join to take a pid rather than a process_info*.
Tested by the buildbot using a few of the gdbserver builders.
gdb/gdbserver/ChangeLog
2018-11-29 Tom Tromey <tom@tromey.com>
* win32-low.c (win32_join): Take pid, not process.
* target.h (struct target_ops) <join>: Change argument type.
(join_inferior): Change argument name.
* spu-low.c (spu_join): Take pid, not process.
* server.c (handle_detach): Preserve pid before destroying
process.
* lynx-low.c (lynx_join): Take pid, not process.
* linux-low.c (linux_join): Take pid, not process.
|
|
Remove a semicolon that should not be there, as reported in PR 23917:
CXX sparc-linux-nat.o
/home/emaisin/src/binutils-gdb/gdb/sparc-linux-nat.c:39:3: error: expected unqualified-id before ‘{’ token
{ sparc_store_inferior_registers (regcache, regnum); }
^
Tested by rebuilding the file manually (make sparc-linux-nat.o) in a
sparc64-linux-gnu build.
gdb/ChangeLog:
PR gdb/23917
* sparc-linux-nat.c (sparc_linux_nat_target): Remove extraneous
semicolon.
|
|
The recent commit 080363310650 ("Per-inferior thread list, thread
ranges/iterators, down with ALL_THREADS, etc.") removed the
definitions of is_running/is_stopped/is_exited but missed updating a
couple uses of is_exited in Solaris-specific code.
Tested by Rainer Orth on amd64-pc-solaris2.11.
gdb/ChangeLog:
2018-11-26 Pedro Alves <palves@redhat.com>
* procfs.c (procfs_notice_thread): Replace uses of
in_thread_list/is_exited with find_thread_ptid/THREAD_EXITED.
* sol-thread.c (sol_thread_target::wait)
(sol_update_thread_list_callback): Likewise.
|
|
It is unfortunately not uncommon to have tests hanging on some of the
BuildBot workers. For example, the ppc64be/ppc64le+gdbserver builders
are especially in a bad state when it comes to testing GDB/gdbserver,
and we can have builds that take an absurd amount of time to
finish (almost 1 week for one single build, for example).
It may be hard to diagnose these failures, because sometimes we don't
have access to the faulty systems, and other times we're just too busy
to wait and check which test is actually hanging. During one of our
conversations about the topic, someone proposed that it would be a
good idea to have a timestamp put together with stdout output, so that
we can come back later and examine which tests are taking too long to
complete.
Here's my proposal to do this. The very first thing I tried to do was
to use "ts(1)" to achieve this feature, and it obviously worked, but
the problem is that I'm afraid "ts(1)" may not be widely available on
every system we support. Therefore, I decided to implement a *very*
simple version of "ts(1)", in Python 3, which basically does the same
thing: iterate over the stdin lines, and prepend a timestamp onto
them.
As for testsuite/Makefile.in, the user can now specify two new
variables to enable timestamp'ed output: TS (which enables the
output), and TS_FORMAT (optional, used to specify another timestamp
format according to "strftime").
Here's an example of how the output looks like:
...
[Nov 22 17:07:19] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/call-strs.exp ...
[Nov 22 17:07:19] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/step-over-no-symbols.exp ...
[Nov 22 17:07:20] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/all-architectures-6.exp ...
[Nov 22 17:07:20] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/hashline3.exp ...
[Nov 22 17:07:20] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/max-value-size.exp ...
[Nov 22 17:07:20] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/quit-live.exp ...
[Nov 22 17:07:46] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/paginate-bg-execution.exp ...
[Nov 22 17:07:56] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/gcore-buffer-overflow.exp ...
[Nov 22 17:07:56] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/gcore-relro.exp ...
[Nov 22 17:07:56] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/watchpoint-delete.exp ...
[Nov 22 17:07:56] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp ...
[Nov 22 17:07:56] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/vla-sideeffect.exp ...
[Nov 22 17:07:57] [1234] Running binutils-gdb/gdb/testsuite/gdb.base/unload.exp ...
...
(What, gdb.base/quit-live.exp is taking 26 seconds to complete?!)
Output to stderr is not timestamp'ed, but I don't think that will be a
problem for us. If it is, we can revisit the solution and extend it.
gdb/testsuite/ChangeLog:
2018-11-25 Sergio Durigan Junior <sergiodj@redhat.com>
* Makefile.in (TIMESTAMP): New variable.
(check-single): Add $(TIMESTAMP) to the end of $(DO_RUNTEST)
command.
(check-single-racy): Likewise.
(check/%.exp): Likewise.
(check-racy/%.exp): Likewise.
(workers/%.worker): Likewise.
(build-perf): Likewise.
(check-perf): Likewise.
* README: Describe new "TS" and "TS_FORMAT" variables.
* print-ts.py: New file.
|
|
This removes some comments that I believe were made obsolete by the
recent change to cli_ui_out::do_field_fmt. The comment in mi_ui_out
probably was just copy/paste, because I think aligning never made
sense in an MI context.
gdb/ChangeLog
2018-11-25 Tom Tromey <tom@tromey.com>
* ui-out.c (ui_out::field_fmt): Remove comment.
* tui/tui-out.c (tui_ui_out::do_field_fmt): Remove comment.
* mi/mi-out.c (mi_ui_out::do_field_fmt): Remove comment.
|
|
Leak fixed in '8e6a5953e1d Fix 4K leak in open_source_file' has been partially
undone by '2179fbc36d23 Return scoped_fd from open_source_file'. Re-add the
transfer of current s->fullname to the unique_xmalloc_ptr fullname given to
find_and_open_source.
|
|
The cannot store/fetch register functions are only used for checking
if a register can be accessed using PEEKUSER/POKEUSER.
The AArch64 port doesn't support this method of access, so remove the
unused functions.
gdb/gdbserver:
* linux-aarch64-low.c (aarch64_cannot_store_register): Remove.
(aarch64_cannot_fetch_register): Likewise.
(struct linux_target_ops): Update references.
|
|
The recent commit 080363310650 ("Per-inferior thread list, thread
ranges/iterators, down with ALL_THREADS, etc.") removed the
definitions of is_running/is_stopped/is_exited but missed removing the
declarations.
gdb/ChangeLog:
2018-11-23 Pedro Alves <palves@redhat.com>
* gdbthread.h (enum thread_state): Move comments here.
(is_running, is_stopped, is_exited): Remove declarations.
|
|
As preparation for multi-target, this patch makes each inferior have
its own thread list.
This isn't absolutely necessary for multi-target, but simplifies
things. It originally stemmed from the desire to eliminate the
init_thread_list calls sprinkled around, plus it makes it more
efficient to iterate over threads of a given inferior (no need to
always iterate over threads of all inferiors).
We still need to iterate over threads of all inferiors in a number of
places, which means we'd need adjust the ALL_THREADS /
ALL_NON_EXITED_THREADS macros. However, naively tweaking those macros
to have an extra for loop, like:
#define ALL_THREADS (thr, inf) \
for (inf = inferior_list; inf; inf = inf->next) \
for (thr = inf->thread_list; thr; thr = thr->next)
causes problems with code that does "break" or "continue" within the
ALL_THREADS loop body. Plus, we need to declare the extra "inf" local
variable in order to pass it as temporary variable to ALL_THREADS
(etc.)
It gets even trickier when we consider extending the macros to filter
out threads matching a ptid_t and a target. The macros become tricker
to read/write. Been there.
An alternative (which was my next attempt), is to replace the
ALL_THREADS etc. iteration style with for_each_all_threads,
for_each_non_exited_threads, etc. functions which would take a
callback as parameter, which would usually be passed a lambda.
However, I did not find that satisfactory at all, because the
resulting code ends up a little less natural / more noisy to read,
write and debug/step-through (due to use of lambdas), and in many
places where we use "continue;" to skip to the next thread now need to
use "return;". (I ran into hard to debug bugs caused by a
continue/return confusion.)
I.e., before:
ALL_NON_EXITED_THREADS (tp)
{
if (tp->not_what_I_want)
continue;
// do something
}
would turn into:
for_each_non_exited_thread ([&] (thread_info *tp)
{
if (tp->not_what_I_want)
return;
// do something
});
Lastly, the solution I settled with was to replace the ALL_THREADS /
ALL_NON_EXITED_THREADS / ALL_INFERIORS macros with (C++20-like) ranges
and iterators, such that you can instead naturaly iterate over
threads/inferiors using range-for, like e.g,.:
// all threads, including THREAD_EXITED threads.
for (thread_info *tp : all_threads ())
{ .... }
// all non-exited threads.
for (thread_info *tp : all_non_exited_threads ())
{ .... }
// all non-exited threads of INF inferior.
for (thread_info *tp : inf->non_exited_threads ())
{ .... }
The all_non_exited_threads() function takes an optional filter ptid_t as
parameter, which is quite convenient when we need to iterate over
threads matching that filter. See e.g., how the
set_executing/set_stop_requested/finish_thread_state etc. functions in
thread.c end up being simplified.
Most of the patch thus is about adding the infrustructure for allowing
the above. Later on when we get to actual multi-target, these
functions/ranges/iterators will gain a "target_ops *" parameter so
that e.g., we can iterate over all threads of a given target that
match a given filter ptid_t.
The only entry points users needs to be aware of are the
all_threads/all_non_exited_threads etc. functions seen above. Thus,
those functions are declared in gdbthread.h/inferior.h. The actual
iterators/ranges are mainly "internals" and thus are put out of view
in the new thread-iter.h/thread-iter.c/inferior-iter.h files. That
keeps the gdbthread.h/inferior.h headers quite a bit more readable.
A common/safe-iterator.h header is added which adds a template that
can be used to build "safe" iterators, which are forward iterators
that can be used to replace the ALL_THREADS_SAFE macro and other
instances of the same idiom in future.
There's a little bit of shuffling of code between
gdbthread.h/thread.c/inferior.h in the patch. That is necessary in
order to avoid circular dependencies between the
gdbthread.h/inferior.h headers.
As for the init_thread_list calls sprinkled around, they're all
eliminated by this patch, and a new, central call is added to
inferior_appeared. Note how also related to that, there's a call to
init_wait_for_inferior in remote.c that is eliminated.
init_wait_for_inferior is currently responsible for discarding skipped
inline frames, which had to be moved elsewhere. Given that nowadays
we always have a thread even for single-threaded processes, the
natural place is to delete a frame's inline frame info when we delete
the thread. I.e., from clear_thread_inferior_resources.
gdb/ChangeLog:
2018-11-22 Pedro Alves <palves@redhat.com>
* Makefile.in (COMMON_SFILES): Add thread-iter.c.
* breakpoint.c (breakpoints_should_be_inserted_now): Replace
ALL_NON_EXITED_THREADS with all_non_exited_threads.
(print_one_breakpoint_location): Replace ALL_INFERIORS with
all_inferiors.
* bsd-kvm.c: Include inferior.h.
* btrace.c (btrace_free_objfile): Replace ALL_NON_EXITED_THREADS
with all_non_exited_threads.
* common/filtered-iterator.h: New.
* common/safe-iterator.h: New.
* corelow.c (core_target_open): Don't call init_thread_list here.
* darwin-nat.c (thread_info_from_private_thread_info): Replace
ALL_THREADS with all_threads.
* fbsd-nat.c (fbsd_nat_target::resume): Replace
ALL_NON_EXITED_THREADS with inf->non_exited_threads.
* fbsd-tdep.c (fbsd_make_corefile_notes): Replace
ALL_NON_EXITED_THREADS with inf->non_exited_threads.
* fork-child.c (postfork_hook): Don't call init_thread_list here.
* gdbarch-selftests.c (register_to_value_test): Adjust.
* gdbthread.h: Don't include "inferior.h" here.
(struct inferior): Forward declare.
(enum step_over_calls_kind): Moved here from inferior.h.
(thread_info::deletable): Definition moved to thread.c.
(find_thread_ptid (inferior *, ptid_t)): Declare.
(ALL_THREADS, ALL_THREADS_BY_INFERIOR, ALL_THREADS_SAFE): Delete.
Include "thread-iter.h".
(all_threads, all_non_exited_threads, all_threads_safe): New.
(any_thread_p): Declare.
(thread_list): Delete.
* infcmd.c (signal_command): Replace ALL_NON_EXITED_THREADS with
all_non_exited_threads.
(proceed_after_attach_callback): Delete.
(proceed_after_attach): Take an inferior pointer instead of an
integer PID. Adjust to use range-for.
(attach_post_wait): Pass down inferior pointer instead of pid.
Use range-for instead of ALL_NON_EXITED_THREADS.
(detach_command): Remove init_thread_list call.
* inferior-iter.h: New.
* inferior.c (struct delete_thread_of_inferior_arg): Delete.
(delete_thread_of_inferior): Delete.
(delete_inferior, exit_inferior_1): Use range-for with
inf->threads_safe() instead of iterate_over_threads.
(inferior_appeared): Call init_thread_list here.
(discard_all_inferiors): Use all_non_exited_inferiors.
(find_inferior_id, find_inferior_pid): Use all_inferiors.
(iterate_over_inferiors): Use all_inferiors_safe.
(have_inferiors, number_of_live_inferiors): Use
all_non_exited_inferiors.
(number_of_inferiors): Use all_inferiors and std::distance.
(print_inferior): Use all_inferiors.
* inferior.h: Include gdbthread.h.
(enum step_over_calls_kind): Moved to gdbthread.h.
(struct inferior) <thread_list>: New field.
<threads, non_exited_threads, threads_safe>: New methods.
(ALL_INFERIORS): Delete.
Include "inferior-iter.h".
(ALL_NON_EXITED_INFERIORS): Delete.
(all_inferiors_safe, all_inferiors, all_non_exited_inferiors): New
functions.
* inflow.c (child_interrupt, child_pass_ctrlc): Replace
ALL_NON_EXITED_THREADS with all_non_exited_threads.
* infrun.c (follow_exec): Use all_threads_safe.
(clear_proceed_status, proceed): Use all_non_exited_threads.
(init_wait_for_inferior): Don't clear inline frame state here.
(infrun_thread_stop_requested, for_each_just_stopped_thread): Use
all_threads instead of ALL_NON_EXITED_THREADS.
(random_pending_event_thread): Use all_non_exited_threads instead
of ALL_NON_EXITED_THREADS. Use a lambda for repeated code.
(clean_up_just_stopped_threads_fsms): Use all_non_exited_threads
instead of ALL_NON_EXITED_THREADS.
(handle_no_resumed): Use all_non_exited_threads instead of
ALL_NON_EXITED_THREADS. Use all_inferiors instead of
ALL_INFERIORS.
(restart_threads, switch_back_to_stepped_thread): Use
all_non_exited_threads instead of ALL_NON_EXITED_THREADS.
* linux-nat.c (check_zombie_leaders): Replace ALL_INFERIORS with
all_inferiors.
(kill_unfollowed_fork_children): Use inf->non_exited_threads
instead of ALL_NON_EXITED_THREADS.
* linux-tdep.c (linux_make_corefile_notes): Use
inf->non_exited_threads instead of ALL_NON_EXITED_THREADS.
* linux-thread-db.c (thread_db_target::update_thread_list):
Replace ALL_INFERIORS with all_inferiors.
(thread_db_target::thread_handle_to_thread_info): Use
inf->non_exited_threads instead of ALL_NON_EXITED_THREADS.
* mi/mi-interp.c (multiple_inferiors_p): New.
(mi_on_resume_1): Simplify using all_non_exited_threads and
multiple_inferiors_p.
* mi/mi-main.c (mi_cmd_thread_list_ids): Use all_non_exited_threads
instead of ALL_NON_EXITED_THREADS.
* nto-procfs.c (nto_procfs_target::open): Don't call
init_thread_list here.
* record-btrace.c (record_btrace_target_open)
(record_btrace_target::stop_recording)
(record_btrace_target::close)
(record_btrace_target::record_is_replaying)
(record_btrace_target::resume, record_btrace_target::wait)
(record_btrace_target::record_stop_replaying): Use
all_non_exited_threads instead of ALL_NON_EXITED_THREADS.
* record-full.c (record_full_wait_1): Use all_non_exited_threads
instead of ALL_NON_EXITED_THREADS.
* regcache.c (cooked_read_test): Remove reference to global
thread_list.
* remote-sim.c (gdbsim_target::create_inferior): Don't call
init_thread_list here.
* remote.c (remote_target::update_thread_list): Use
all_threads_safe instead of ALL_NON_EXITED_THREADS.
(remote_target::process_initial_stop_replies): Replace
ALL_INFERIORS with all_non_exited_inferiors and use
all_non_exited_threads instead of ALL_NON_EXITED_THREADS.
(remote_target::open_1): Don't call init_thread_list here.
(remote_target::append_pending_thread_resumptions)
(remote_target::remote_resume_with_hc): Use all_non_exited_threads
instead of ALL_NON_EXITED_THREADS.
(remote_target::commit_resume)
(remote_target::remove_new_fork_children): Replace ALL_INFERIORS
with all_non_exited_inferiors and use all_non_exited_threads
instead of ALL_NON_EXITED_THREADS.
(remote_target::kill_new_fork_children): Use
all_non_exited_threads instead of ALL_NON_EXITED_THREADS. Remove
init_thread_list and init_wait_for_inferior calls.
(remote_target::remote_btrace_maybe_reopen)
(remote_target::thread_handle_to_thread_info): Use
all_non_exited_threads instead of ALL_NON_EXITED_THREADS.
* target.c (target_terminal::restore_inferior)
(target_terminal_is_ours_kind): Replace ALL_INFERIORS with
all_non_exited_inferiors.
* thread-iter.c: New file.
* thread-iter.h: New file.
* thread.c: Include "inline-frame.h".
(thread_list): Delete.
(clear_thread_inferior_resources): Call clear_inline_frame_state.
(init_thread_list): Use all_threads_safe instead of
ALL_THREADS_SAFE. Adjust to per-inferior thread lists.
(new_thread): Adjust to per-inferior thread lists.
(add_thread_silent): Pass inferior to find_thread_ptid.
(thread_info::deletable): New, moved from the header.
(delete_thread_1): Adjust to per-inferior thread lists.
(find_thread_global_id): Use inf->threads().
(find_thread_ptid): Use find_inferior_ptid and pass inferior to
find_thread_ptid.
(find_thread_ptid(inferior*, ptid_t)): New overload.
(iterate_over_threads): Use all_threads_safe.
(any_thread_p): New.
(thread_count): Use all_threads and std::distance.
(live_threads_count): Use all_non_exited_threads and
std::distance.
(valid_global_thread_id): Use all_threads.
(in_thread_list): Use find_thread_ptid.
(first_thread_of_inferior): Adjust to per-inferior thread lists.
(any_thread_of_inferior, any_live_thread_of_inferior): Use
inf->non_exited_threads().
(prune_threads, delete_exited_threads): Use all_threads_safe.
(thread_change_ptid): Pass inferior pointer to find_thread_ptid.
(set_resumed, set_running): Use all_non_exited_threads.
(is_thread_state, is_stopped, is_exited, is_running)
(is_executing): Delete.
(set_executing, set_stop_requested, finish_thread_state): Use
all_non_exited_threads.
(print_thread_info_1): Use all_inferiors and all_threads.
(thread_apply_all_command): Use all_non_exited_threads.
(thread_find_command): Use all_threads.
(update_threads_executing): Use all_non_exited_threads.
* tid-parse.c (parse_thread_id): Use inf->threads.
* x86-bsd-nat.c (x86bsd_dr_set): Use inf->non_exited_threads ().
|
|
A following commit to make each inferior have its own thread list
exposes a problem with bf93d7ba99 ("Add thread after updating gdbarch
when exec'ing"), which is that we can't defer adding the thread
because that breaks try_open_exec_file which deep inside ends up
calling inferior_thread():
#5 0x0000000000637c78 in internal_error(char const*, int, char const*, ...) (file=0xc151f8 "src/gdb/thread.c", line=165, fmt=0xc15180 "%s: Assertion `%s' failed.") at src/gdb/common/errors.c:55
#6 0x00000000008a3d80 in inferior_thread() () at src/gdb/thread.c:165
#7 0x0000000000456f91 in try_thread_db_load_1(thread_db_info*) (info=0x277eb00) at src/gdb/linux-thread-db.c:830
#8 0x0000000000457554 in try_thread_db_load(char const*, int) (library=0xb01a4f "libthread_db.so.1", check_auto_load_safe=0)
at src/gdb/linux-thread-db.c:1002
#9 0x0000000000457861 in try_thread_db_load_from_sdir() () at src/gdb/linux-thread-db.c:1079
#10 0x0000000000457b72 in thread_db_load_search() () at src/gdb/linux-thread-db.c:1134
#11 0x0000000000457d29 in thread_db_load() () at src/gdb/linux-thread-db.c:1192
#12 0x0000000000457e51 in check_for_thread_db() () at src/gdb/linux-thread-db.c:1244
#13 0x0000000000457ed2 in thread_db_new_objfile(objfile*) (objfile=0x270ff60) at src/gdb/linux-thread-db.c:1273
#14 0x000000000045a92e in std::_Function_handler<void (objfile*), void (*)(objfile*)>::_M_invoke(std::_Any_data const&, objfile*&&) (__functor=..., __args#0=@0x7ffef3efe140: 0x270ff60) at /usr/include/c++/7/bits/std_function.h:316
#15 0x00000000007bbebf in std::function<void (objfile*)>::operator()(objfile*) const (this=0x24e1d18, __args#0=0x270ff60)
at /usr/include/c++/7/bits/std_function.h:706
#16 0x00000000007bba86 in gdb::observers::observable<objfile*>::notify(objfile*) const (this=0x117ce80 <gdb::observers::new_objfile>, args#0=0x270ff60) at src/gdb/common/observable.h:106
#17 0x0000000000856000 in symbol_file_add_with_addrs(bfd*, char const*, symfile_add_flags, section_addr_info*, objfile_flags, objfile*) (abfd=0x1d7dae0, name=0x254bfc0 "/ho
The problem is latent currently because inferior_thread() at that
point manages to return a thread, even though it's the wrong one (of
the old inferior).
The problem originally fixed by bf93d7ba99 was:
(...) we should avoid doing register reads
after a process does an exec and before we've updated that inferior's
gdbarch. Otherwise, we may interpret the registers using the wrong
architecture.
(...) The call to "add_thread" done just after adding the inferior is
problematic, because it ends up reading the registers (because the ptid
is re-used, we end up doing a switch_to_thread to it, which tries to
update stop_pc). (...)
The register-reading issue is no longer a problem nowadays, ever since
switch_to_thread stopped reading the stop_pc in git commit
f2ffa92bbce9 ("gdb: Eliminate the 'stop_pc' global").
So this commit basically reverts bf93d7ba99.
gdb/ChangeLog:
2018-11-22 Pedro Alves <palves@redhat.com>
* infrun.c (follow_exec) <set follow-exec new>: Add thread and
switch to it before calling into try_open_exec_file.
|
