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.
`-march=r5900' already enables the R5900 short loop workaround.
However, the R5900 ISA and most other MIPS ISAs are mutually
exclusive since R5900-specific instructions are generated as well.
The `-mfix-r5900' option can be used in combination with e.g.
`-mips2' or `-mips3' to generate generic MIPS binaries that also
work with the R5900 target.
This change has been tested with `make RUNTESTFLAGS=mips.exp
check-gas' for the targets `mipsr5900el-unknown-linux-gnu',
`mipsr5900el-elf' and `mips3-unknown-linux-gnu'.
gas/
* config/tc-mips.c (mips_fix_r5900, mips_fix_r5900_explicit):
New variables.
(options): Add OPTION_FIX_R5900 and OPTION_NO_FIX_R5900
enumeration constants.
(md_longopts): Add "mfix-r5900" and "mno-fix-r5900" options.
(can_swap_branch_p, md_parse_option, mips_after_parse_args):
Handle the new options.
(md_show_usage): Document the `-mfix-r5900' option.
* doc/as.texi: Likewise.
* doc/c-mips.texi: Likewise.
* testsuite/gas/mips/mips.exp: Run R5900 dump tests.
* testsuite/gas/mips/r5900-fix.d: Test `-mfix-r5900' option.
* testsuite/gas/mips/r5900-fix.s: Likewise.
* testsuite/gas/mips/r5900-no-fix.d: Test `-mno-fix-r5900'.
* testsuite/gas/mips/r5900-no-fix.s: Likewise.
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.
PR 23932
* elf.c (IS_CONTAINED_BY_LMA): Add a check for a negative section
size.
(rewrite_elf_program_header): If no sections are mapped into a
segment return an error.
IFUNC resolvers must always be called via their global entry point.
They will be called from ld.so rather than from the local executable.
PR 23937
bfd/
* elf64-ppc.c (write_plt_relocs_for_local_syms): Don't add local
entry offset for ifuncs.
ld/
* testsuite/ld-powerpc/pr23937.d,
* testsuite/ld-powerpc/pr23937.s: New test.
* testsuite/ld-powerpc/powerpc.exp: Run it.
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.
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.
When discovering the statement lists via their header variable
statement_list, file_chain and input_file_chain it can be confusing to
figure out what they are for. They can point to the same initial
statement and the relation between the next field they use is not
obvious from the name.
This commit adds comment for each of those statement list header to
explain what they are for and what next field they use. It also rewrite
the comment for the next fields to simply redirect the reader to the
list header to avoid duplication of documentation.
2018-11-29 Thomas Preud'homme <thomas.preudhomme@linaro.org>
ld/
* ldlang.c (statement_list): Document purpose and what next field it
uses.
(file_chain): Likewise.
(input_file_chain): Likewise.
* ldlang.h (lang_statement_header_type): Document statement list header
the next pointer correspond to.
(lang_statement_header_type): Replace comment for next and
next_real_file field to refer the reader to their corresponding
statement list header.
.note.gnu.property section in IR inputs should be ignored. Don't
merge them.
PR ld/23929
* elf-properties.c (_bfd_elf_link_setup_gnu_properties): Don't
merge .note.gnu.property section in IR inputs.
PR 23931
* objdump.c (dump_relocs_in_section): When checking for an
unreasonable amount of relocs in a bfd, allow for the fact that
the internal representation of a reloc may be bigger than the
external representation.
Accept R_MIPS_HI16, R_MIPS_HIGHER and R_MIPS_HIGHEST relocations and
their compressed counterparts in PIC code where the symbol referred is
absolute. Such an operation is meaningful, because an absolute symbol
effectively is a constant the calculation of the value of which has been
deferred to the static link time, and which is not going to change any
further at the dynamic load time. Therefore there is no need ever to
refuse the use of these relocations with such symbols, as the resulting
run-time value observed by the program will be correct even in PIC code.
This is not the case with R_MIPS_26 and its compressed counterparts,
because the run-time value calculated by the instructions these
relocations are used with depends on the address of the instruction
itself, and that can change according to the base address used by the
dynamic loader. Therefore these relocations have to continue being
rejected in PIC code even with absolute symbols.
This allows successful linking of code that relies on previous linker
behavior up to commit 861fb55ab5 ("Defer allocation of R_MIPS_REL32
GOT slots"), <https://sourceware.org/ml/binutils/2008-08/msg00096.html>,
which introduced the problematic check missing this special exception
for absolute symbols.
bfd/
* elfxx-mips.c (_bfd_mips_elf_check_relocs) <R_MIPS16_HI16>
<R_MIPS_HI16, R_MIPS_HIGHER, R_MIPS_HIGHEST, R_MICROMIPS_HI16>
<R_MICROMIPS_HIGHER, R_MICROMIPS_HIGHEST>: Also accept an
absolute symbol in PIC code.
ld/
* testsuite/ld-mips-elf/pic-reloc-0.d: New test.
* testsuite/ld-mips-elf/pic-reloc-1.d: New test.
* testsuite/ld-mips-elf/pic-reloc-2.d: New test.
* testsuite/ld-mips-elf/pic-reloc-3.d: New test.
* testsuite/ld-mips-elf/pic-reloc-4.d: New test.
* testsuite/ld-mips-elf/pic-reloc-absolute-hi.ld: New test
linker script.
* testsuite/ld-mips-elf/pic-reloc-absolute-lo.ld: New test
linker script.
* testsuite/ld-mips-elf/pic-reloc-ordinary.ld: New test linker
script.
* testsuite/ld-mips-elf/pic-reloc-j.s: New test source.
* testsuite/ld-mips-elf/pic-reloc-lui.s: New test source.
* testsuite/ld-mips-elf/mips-elf.exp: Run the new tests.
Switch from `_bfd_error_handler' to `info->callbacks->einfo' with error
reporting concerning the use of position-dependent relocations such as
R_MIPS_HI16 or R_MIPS_26 in PIC code and continue processing so that any
subsequent link errors are also shown rather than the linker terminating
right away. This can reduce user frustration where correcting one error
only reveals another one; instead all are shown together making them all
possible to investigate at once. The use of the `%X' specifier causes
the linker to terminate unsuccessfully at the end of processing.
Also fix the message to say `cannot' rather than `can not'.
bfd/
* elfxx-mips.c (_bfd_mips_elf_check_relocs) <R_MIPS16_26>
<R_MIPS_26, R_MICROMIPS_26_S1>: Use `info->callbacks->einfo'
rather than `_bfd_error_handler' to report refused relocations
in PIC code and continue processing. Fix error message: `can
not' -> `cannot'.
Similarly to commit 174d0a74a2 ("PowerPC/BFD: Convert `%P: %H:' to
`%H:' in error messages") convert linker relocation error messages to
use `%H:' rather `%P: %H:', removing inconsistent message composition
like:
$ cat reloc-j.s
.text
.globl foo
.ent foo
foo:
j bar
j bar
.end foo
$ cat reloc-j.ld
SECTIONS
{
bar = 0x12345678;
.text : { *(.text) }
/DISCARD/ : { *(*) }
}
$ as -o reloc-j.o reloc-j.s
$ ld -T reloc-j.ld -o reloc-j reloc-j.o
ld: tmpdir/reloc-j.o: in function `foo':
(.text+0x0): relocation truncated to fit: R_MIPS_26 against `bar'
ld: (.text+0x8): relocation truncated to fit: R_MIPS_26 against `bar'
$
where subsequent lines referring to issues within a single function have
the name of the linker executable prepended, but the first one does not.
As noted with the commit referred this breaks a GNU Coding Standard's
requirement that error messages from compilers should look like this:
source-file-name:lineno: message
also quoted in `vfinfo' code handling these specifiers.
Remove the linker name prefix then, making the messages now look like:
$ ld -T reloc-j.ld -o reloc-j reloc-j.o
tmpdir/reloc-j.o: in function `foo':
(.text+0x0): relocation truncated to fit: R_MIPS_26 against `bar'
(.text+0x8): relocation truncated to fit: R_MIPS_26 against `bar'
$
instead.
ld/
* ldmain.c (reloc_overflow): Use `%H:' rather than `%P: %H:'
with `einfo'.
(reloc_dangerous): Likewise.
(unattached_reloc): Likewise.
Initialize *uncompressed_align_pow_p to 0 since *uncompressed_align_pow_p
is passed to bfd_is_section_compressed_with_header as uninitialized,
PR binutils/23919
* compress.c (bfd_is_section_compressed_with_header): Initialize
*uncompressed_align_pow_p to 0.
The fix for PR ld/22263 causes TLS relocations using ADRP to be relaxed
into MOVZ, however this causes issues for the erratum code.
The erratum code scans the input sections looking for ADRP instructions
and notes their location in the stream.
It then later tries to find them again in order to generate the linker
stubs. Due to the relaxation it instead finds a MOVZ and hard aborts.
Since this relaxation is a valid one, and in which case the erratum no
longer applies, it shouldn't abort but instead just continue.
This changes the TLS relaxation code such that when it finds an ADRP and
it relaxes it, it removes the erratum entry from the work list by changing
the stub type into none so the stub is ignored.
The entry is not actually removed as removal is a more expensive operation
and we have already allocated the memory anyway.
The clearing is done for IE->LE and GD->LE relaxations, and a testcase is
added for the IE case. The GD case I believe to be impossible to get together
with the erratum sequence due to the required BL which would break the sequence.
However to cover all basis I have added the guard there as well.
build on native hardware and regtested on
aarch64-none-elf, aarch64-none-elf (32 bit host),
aarch64-none-linux-gnu, aarch64-none-linux-gnu (32 bit host)
Cross-compiled and regtested on
aarch64-none-linux-gnu, aarch64_be-none-linux-gnu
Testcase in PR23940 tested and works as expected now and benchmarks ran on A53
showing no regressions and no issues.
bfd/ChangeLog:
PR ld/23904
* elfnn-aarch64.c (_bfd_aarch64_adrp_p): Use existing constants.
(_bfd_aarch64_erratum_843419_branch_to_stub): Use _bfd_aarch64_adrp_p.
(struct erratum_835769_branch_to_stub_clear_data): New.
(_bfd_aarch64_erratum_843419_clear_stub): New.
(clear_erratum_843419_entry): New.
(elfNN_aarch64_tls_relax): Use it.
(elfNN_aarch64_relocate_section): Pass input_section.
(aarch64_map_one_stub): Handle branch type none as valid.
ld/ChangeLog:
PR ld/23904
* testsuite/ld-aarch64/aarch64-elf.exp: Add erratum843419_tls_ie.
* testsuite/ld-aarch64/erratum843419_tls_ie.d: New test.
* testsuite/ld-aarch64/erratum843419_tls_ie.s: New test.
PR binutils/23919
binutils* readelf.c (dump_sections_as_strings): Remove bogus addralign check.
(dump_sections_as_bytes): Likewise.
(load_specific_debug_sections): Likewise.
* testsuite/binutils-all/dw2-3.rS: Adjust alignment.
* testsuite/binutils-all/dw2-3.rt: Likewise.
bfd * bfd.c (bfd_update_compression_header): Explicitly set alignment.
(bfd_check_compression_header): Add uncompressed_alignment_power
argument. Check ch_addralign is a power of 2.
* bfd-in2.h: Regenerated.
* compress.c (bfd_compress_section_contents): Get and set
orig_uncompressed_alignment_pow if section is decompressed.
(bfd_is_section_compressed_with_header): Add and get
uncompressed_align_pow_p argument.
(bfd_is_section_compressed): Add uncompressed_align_power argument
to bfd_is_section_compressed_with_header call.
(bfd_init_section_decompress_status): Get and set
uncompressed_alignment_power.
* elf.c (_bfd_elf_make_section_from_shdr): Add
uncompressed_align_power argument to
bfd_is_section_compressed_with_header call.
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 0803633106 ("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.
This test fails on powerpc64le due to the justsyms_lib being built
with exported_data at 0x2010000, apparently due to the powerpc target
code generating an empty relro .branch_lt section. Since the test
relies on the library having exported_data at 0x2000000, avoid the
problem by linking with -z norelro. Also, the test doesn't need to
avoid checking the function symbol on powerpc elfv2.
* testsuite/Makefile.am (justsyms_lib): Link with -z norelro.
* testsuite/Makefile.in: Regenerate.
* testsuite/justsyms_exec.c (main): Do check exported_func
on PowerPC64 ELFv2.
PowerPC64 ELFv2 uses the top 3 bits of st_other to encode a function's
local entry point offset from its global entry point. Allow st_other
bits except for visibility==default.
* testsuite/ver_test_14.sh: Accept objdump -T display of st_other
bits on powerpc64le.
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 0803633106 ("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.
Simon Marchi
John Baldwin
Fredrik Noring
Andrew Burgess
Sergio Durigan Junior
Pedro Alves
Nick Clifton
Alan Modra
GDB Administrator
Philippe Waroquiers
Rajendra SY
Jim Wilson
Tom Tromey
Thomas Preud'homme
H.J. Lu
Maciej W. Rozycki
Tamar Christina
Jozef Lawrynowicz
Mark Wielaard
Andre Vieira
Alan Hayward