This particular sort almost certainly does not need to be stable for
the ELF linker to work correctly. However it is conceivable that an
unstable sort could affect linker output, and thus different output be
seen with differing qsort implementations. The argument goes like
this: Given more than one strong alias symbol of equal section, value,
and size, the aliases will compare equal by elf_sort_symbol and thus
which one is chosen as the "real" symbol to be made dynamic depends on
qsort. Why would anyone define two symbols at the same address?
Well, sometimes the fact that there are more than one strong alias
symbol is due to linker script symbols like __bss_start being made
dynamic. This will match the first symbol defined in .bss if it
doesn't have correct size, and forgetting to properly set size and
type of symbols isn't as rare as it should be.
This patch adds some more heuristics to elf_sort_symbol.
* elflink.c (elf_sort_symbol): Sort on type and name as well.
(elf_link_add_object_symbols): Style fix.
elf_sort_sections tried to ensure a stable qsort by using target_index
as the final comparison, but target_index hasn't been set by anything
at the time elf_sort_sections was run. This patch arrange to have
target_index set.
* elf.c (_bfd_elf_map_sections_to_segments): Init target_index
for sections about to be sorted.
(assign_file_positions_for_load_sections): Likewise.
(elf_sort_sections): Don't bother optimising both TOEND case.
* elflink.c (bfd_elf_final_link): Reset target_index.
The linker SHF_LINK_ORDER section sorting had a number of defects.
1) The ordering was by VMA, which won't work with overlays. LMA is
better.
2) Zero size sections can result in two sections at the same LMA/VMA.
When only one of the two sections at the same LMA is zero size,
that one must be first.
3) Warnings given by elf_get_linked_section_vma won't ever be emitted
since elf_object_p warns and excludes objects with zero sh_link on
a SHF_LINK_ORDER section.
4) Section offset was adjusted down rather than up by section
alignment, possibly creating overlapping sections.
5) Finding the linked section did so the hard way, rather than simply
using elf_linked_to_section.
* elflink.c (elf_get_linked_section_vma): Delete.
(compare_link_order): Use elf_linked_to_section and sort by lma,
size, and id.
(elf_fixup_link_order): Use size_t variables where appropriate.
Make use of elf_linked_to_section. Formatting. Properly align
sections.
qsort isn't guaranteed to be a stable sort, that is, elements
comparing equal according to the comparison function may be reordered
relative to their original ordering. Of course sometimes you may not
care, but even in those cases it is good to force some ordering
(ie. not have the comparison function return 0) so that linker output
is reproducible over different libc qsort implementations.
One way to make qsort stable (which the glibc manual incorrectly says
is the only way) is to augment the elements being sorted with a
monotonic counter of some kind, and use that counter as the final
arbiter of ordering in the comparison function.
Another way is to set up an array of pointers into the array of
elements, first pointer to first element, second pointer to second
element and so so, and sort the pointer array rather than the element
array. Final arbiter in the comparison function then is the pointer
difference. This works well with, for example, the symbol pointers
returned by _bfd_elf_canonicalize_symtab which point into a symbol
array.
This patch fixes a few places where sorting by symbol pointers is
appropriate, and adds comments where qsort stability is a non-issue.
* elf-strtab.c (strrevcmp): Comment.
* merge.c (strrevcmp): Likewise.
* elf64-ppc.c (compare_symbols): Correct final pointer comparison.
Comment on why comparing pointers ensures a stable sort.
* elflink.c (struct elf_symbol): Add void* to union.
(elf_sort_elf_symbol): Ensure a stable sort with pointer comparison.
(elf_sym_name_compare): Likewise.
(bfd_elf_match_symbols_in_sections): Style fix.
(elf_link_sort_cmp1): Comment.
We get this warning when building with clang:
CXX ui-out.o
/home/smarchi/src/binutils-gdb/gdb/ui-out.c:590:22: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
do_message (style, format, args);
^~~~~~
This can be considered a legitimate warning, as call_do_message's format
parameter is not marked as a format string. Therefore, we should
normally mark the call_do_message method with the `format` attribute.
However, doing so just moves (and multiplies) the problem, as all the
uses of call_do_message in the vmessage method now warn. If we wanted
to continue on that path, we should silence the warning for each of
them, as a way of telling the compiler "it's ok, we know what we are
doing".
But since call_do_message is really just vmessage's little helper, it's
simpler to just silence the warning at that single point.
gdb/ChangeLog:
* ui-out.c (ui_out::call_do_message): Silence
-Wformat-nonliteral warning.
Change-Id: I58ad41793448f38835c5d6ba7b9e5c4dd8df260f
binutils* README-how-to-make-a-release: Add a note to reset the
development flag back to true after making a point release.
ld * NEWS: Delete superflous "Changes in 2.33" comment.
In an attempt to reduce the number of files re-build when some headers
are touched, I ran include-what-you-use with breakpoint.c as a guinea
pig. It revealed a few files that were unnecessary to include, which
this patch removes.
breakpoint.c uses tilde_expand from readline, hence the necessity to
include tilde.h. AFAIK, it's fine to include just that, and not the
whole readline headers.
include-what-you-use also reported many header files that should be
included but aren't, I suppose that breakpoint.c currently includes them
indirectly. For now I'll pretend I didn't see that :).
gdb/ChangeLog:
* breakpoint.c: Remove some includes: continuations.h, skip.h,
mi/mi-main.h, readline/readline.h, readline/history.h. Add
include: readline/tilde.h.
-#include "skip.h"
#include "ax-gdb.h"
#include "dummy-frame.h"
#include "interps.h"
@@ -69,11 +67,9 @@
#include "thread-fsm.h"
#include "tid-parse.h"
#include "cli/cli-style.h"
-#include "mi/mi-main.h"
/* readline include files */
-#include "readline/readline.h"
-#include "readline/history.h"
+#include "readline/tilde.h"
/* readline defines this. */
#undef savestring
Change-Id: I88bfe9071f2f973fd84caaf04b95c33a4dfb33de
Normally the gdb.reverse/*.exp test-cases pass on my system (apart from the
record/23188 KFAIL for gdb.reverse/step-precsave.exp). But when specifying
GLIBC_TUNABLES=glibc.tune.hwcaps=-XSAVEC_Usable to force glibc to use
_dl_runtime_resolve_xsave instead of _dl_runtime_resolve_xsavec, we run into
1054 FAILs like this:
...
(gdb) PASS: gdb.reverse/sigall-reverse.exp: b gen_HUP
continue^M
Continuing.^M
Process record does not support instruction 0xfae64 at address \
0x7ffff7ded958.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
0x00007ffff7ded958 in _dl_runtime_resolve_xsave () from \
/lib64/ld-linux-x86-64.so.2^M
(gdb) FAIL: gdb.reverse/sigall-reverse.exp: get signal ABRT
...
The problem is that the xsave instruction is not supported in
reverse-debugging (PR record/25038).
Add KFAILs for this PR.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2019-10-13 Tom de Vries <tdevries@suse.de>
PR record/25038
* gdb.reverse/sigall-precsave.exp: Add PR record/25038 KFAIL.
* gdb.reverse/sigall-reverse.exp: Same.
* gdb.reverse/solib-precsave.exp: Same.
* gdb.reverse/solib-reverse.exp: Same.
* gdb.reverse/step-precsave.exp: Same.
* gdb.reverse/until-precsave.exp: Same.
* gdb.reverse/until-reverse.exp: Same.
* lib/gdb.exp (gdb_continue_to_breakpoint): Same.
This variable is declared in tracepoint.h, which is already included
by remote.c.
gdb/ChangeLog:
2019-10-12 Christian Biesinger <cbiesinger@google.com>
* remote.c (remote_target::get_trace_status): Remove declaration of
trace_regblock_size.
Also removes an unnecessary declaration of cmdlist in cli-cmds.c.
I don't understand why it is there, the definition of cmdlist is
at the top of the same file.
gdb/ChangeLog:
2019-10-12 Christian Biesinger <cbiesinger@google.com>
* cli/cli-cmds.c (max_user_call_depth): Move comment to header.
(show_user): Remove declaration of cmdlist.
* cli/cli-cmds.h (max_user_call_depth): Declare.
* cli/cli-script.c (execute_user_command): Remove declaration
of max_user_call_depth.
Since I had to look at these function comments to fix the RISC-V ARI warnings,
I noticed that they make no sense. The pulongest and plongest comments are
swapped. phex is missing a comment. And phex_nz doesn't mention how it is
different from phex.
* gdbsupport/print-utils.h (pulongest): Fix comment.
(plongest): Likewise.
(phex): Add missing comment, mention leading zeros.
(phex_nz): Add mention of no leading zeros to comment.
> gdb/riscv-tdep.c:1657: code: %ll: Do not use printf(%ll), instead use printf(%s,phex()) to dump a 'long long' value
gdb/riscv-tdep.c:1657: "Writing %lld-byte nop instruction to %s: %s\n",
> gdb/riscv-tdep.c:1658: code: long long: Do not use 'long long', instead use LONGEST
gdb/riscv-tdep.c:1658: ((unsigned long long) sizeof (nop_insn)),
fprintf_unfiltered doesn't support z (or j for that matter), and fixing that
is a larger patch than I'd like to write, so this does basically what the
ARI warnings recommends. We don't need the cast as there is a prototype for
plongest.
* riscv-tdep.c (riscv_push_dummy_code): Change %lld to %s and use
plongest instead of unsigned long long cast.
stash_maybe_enable_info_hash_tables sets
stash->info_hash_status = STASH_INFO_HASH_ON;
regardless of the result of stash_maybe_update_info_hash_tables call. In
case it fails this results in repeated invocation of comp_unit_hash_info
for the same comp unit and assertion failure in this function.
Only set stash->info_hash_status = STASH_INFO_HASH_ON; when
stash_maybe_update_info_hash_tables is successful.
bfd/
2019-10-11 Max Filippov <jcmvbkbc@gmail.com>
* dwarf2.c (stash_maybe_enable_info_hash_tables): Only set
stash->info_hash_status = STASH_INFO_HASH_ON when
stash_maybe_update_info_hash_tables succeeds.
Once https://sourceware.org/ml/insight/2019-q4/msg00000.html lands,
we can just include gdbtk.h to get the declarations for
external_editor_command and gdbtk_test, instead of having to
declare them here in main.c.
gdb/ChangeLog:
2019-10-07 Christian Biesinger <cbiesinger@google.com>
* main.c (captured_main_1): Include gdbtk.h and remove declarations
for external_editor_command and gdbtk_test.
Some of the comparison functions in infcall-nested-structs.c contain
redundant comparisons like a.<some_field> == a.<some_field> instead of
a.<some_field> == b.<some_field>. They were introduced with this commit:
36eb4c5f9bbe6 - "infcall-nested-structs: Test up to five fields"
Fix the redundant comparisons.
gdb/testsuite/ChangeLog:
* gdb.base/infcall-nested-structs.c (cmp_struct_02_01)
(cmp_struct_02_02, cmp_struct_04_01, cmp_struct_04_02)
(cmp_struct_05_01, cmp_struct_static_02_01)
(cmp_struct_static_04_01, cmp_struct_static_06_01): Fix redundant
comparisons.
When running the gdb testsuite with target board unix/-fPIE/-pie, the
resulting ada executables are not PIE executables, because gnatmake doesn't
recognize -pie, and consequently doesn't pass it to gnatlink.
Fix this by replacing "-pie" with "-largs -pie -margs" in
target_compile_ada_from_dir, and doing the same for -no-pie.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2019-10-10 Tom de Vries <tdevries@suse.de>
PR testsuite/24888
* lib/ada.exp (target_compile_ada_from_dir): Route -pie/-no-pie to
gnatlink.
tui_data_window::show_registers currently calls erase_data_content.
However, I think it's better to have fewer calls to this (ideally just
one would suffice). This refactors that function to remove this call.
gdb/ChangeLog
2019-10-09 Tom Tromey <tom@tromey.com>
* tui/tui-regs.c (tui_data_window::show_registers): Don't call
erase_data_content.
This changes tui_gen_win_info::handle to be a specialization of
unique_ptr. This is perhaps mildly uglier in some spots, due to the
proliferation of "get"; but on the other hand it cleans up some manual
management and it allows for the removal of tui_delete_win.
gdb/ChangeLog
2019-10-09 Tom Tromey <tom@tromey.com>
* tui/tui-wingeneral.h (tui_delete_win): Don't declare.
* tui/tui-stack.c (tui_locator_window::rerender): Update.
* tui/tui-command.c (tui_cmd_window::resize)
(tui_refresh_cmd_win): Update.
* tui/tui-win.c (tui_resize_all, tui_set_focus_command): Update.
* tui/tui.c (tui_rl_other_window, tui_enable): Update.
* tui/tui-data.c (~tui_gen_win_info): Remove.
* tui/tui-layout.c (tui_gen_win_info::resize): Update.
* tui/tui-io.c (update_cmdwin_start_line, tui_putc, tui_puts)
(tui_redisplay_readline, tui_mld_flush)
(tui_mld_erase_entire_line, tui_mld_getc, tui_getc): Update.
* tui/tui-regs.c (tui_data_window::delete_data_content_windows)
(tui_data_window::erase_data_content)
(tui_data_item_window::rerender)
(tui_data_item_window::refresh_window): Update.
* tui/tui-wingeneral.c (tui_gen_win_info::refresh_window)
(box_win, tui_gen_win_info::make_window)
(tui_gen_win_info::make_visible): Update.
(tui_delete_win): Remove.
* tui/tui-winsource.c
(tui_source_window_base::do_erase_source_content): Update.
(tui_show_source_line, tui_source_window_base::update_tab_width)
(tui_source_window_base::update_exec_info): Update.
* tui/tui-data.h (struct curses_deleter): New.
(struct tui_gen_win_info) <handle>: Now a unique_ptr.
(struct tui_gen_win_info) <~tui_gen_win_info>: Define.
tui-wingeneral.h has an unused forward declaration. This removes it.
gdb/ChangeLog
2019-10-09 Tom Tromey <tom@tromey.com>
* tui/tui-wingeneral.h (struct tui_gen_win_info): Don't declare.
tui_win_is_auxiliary is not used, so remove it.
gdb/ChangeLog
2019-10-09 Tom Tromey <tom@tromey.com>
* tui/tui-data.c (tui_win_is_auxiliary): Remove.
* tui/tui-data.h (tui_win_is_auxiliary): Don't declare.
tui_default_win_viewport_height was only called from a single spot,
for a single type of window. This patch removes the function and
moves the logic into the sole caller.
gdb/ChangeLog
2019-10-09 Tom Tromey <tom@tromey.com>
* tui/tui-disasm.c (tui_get_low_disassembly_address): Compute
window height directly.
* tui/tui-layout.h (tui_default_win_viewport_height): Don't
declare.
* tui/tui-layout.c (tui_default_win_height): Remove.
(tui_default_win_viewport_height): Remove.
On openSUSE Leap 15.1 using rustc version 1.36.0 (using llvm 7), I get:
...
(gdb) PASS: gdb.rust/simple.exp: print e2.0
print k^M
$54 = simple::SpaceSaver::Thebox(40, 0x0)^M
(gdb) FAIL: gdb.rust/simple.exp: print k
...
while we're expecting:
...
gdb_test "print k" " = simple::SpaceSaver::Nothing"
...
When using a relatively recent version of Rust with a somewhat older version
of LLVM, the Rust compiler will emit a legacy encoding of enums (see also
quirk_rust_enum in dwarf2read.c).
So, the variable k:
...
<17><3d58>: Abbrev Number: 15 (DW_TAG_variable)
<3d59> DW_AT_location : 3 byte block: 91 b8 4 (DW_OP_fbreg: 568)
<3d5d> DW_AT_name : (indirect string, offset: 0xf9a): k
<3d61> DW_AT_alignment : 1
<3d62> DW_AT_decl_file : 1
<3d63> DW_AT_decl_line : 129
<3d64> DW_AT_type : <0x4232>
...
has type:
...
<2><4232>: Abbrev Number: 11 (DW_TAG_union_type)
<4233> DW_AT_name : (indirect string, offset: 0x3037): SpaceSaver
<4237> DW_AT_byte_size : 16
<4238> DW_AT_alignment : 8
<3><4239>: Abbrev Number: 9 (DW_TAG_member)
<423a> DW_AT_name : (indirect string, offset: 0x29f5): RUST$ENCODED$ENUM$0$Nothing
<423e> DW_AT_type : <0x4245>
<4242> DW_AT_alignment : 8
<4243> DW_AT_data_member_location: 0
...
The "RUST$ENCODED$ENUM$0$Nothing" means that field 0 is both a pointer and a
discriminant, and if the value is 0, then the enum is just a data-less variant
named "Nothing".
However, the corresponding type has two fields, where not field 0 but field 1
is a pointer, and field 0 is a byte:
...
<2><4245>: Abbrev Number: 8 (DW_TAG_structure_type)
<4246> DW_AT_name : (indirect string, offset: 0x2a11): Thebox
<424a> DW_AT_byte_size : 16
<424b> DW_AT_alignment : 8
<3><424c>: Abbrev Number: 9 (DW_TAG_member)
<424d> DW_AT_name : (indirect string, offset: 0x670): __0
<4251> DW_AT_type : <0x436b>
<4255> DW_AT_alignment : 1
<4256> DW_AT_data_member_location: 8
<3><4257>: Abbrev Number: 9 (DW_TAG_member)
<4258> DW_AT_name : (indirect string, offset: 0x1662): __1
<425c> DW_AT_type : <0x45da>
<4260> DW_AT_alignment : 8
<4261> DW_AT_data_member_location: 0
...
Mark this as xfail.
gdb/testsuite/ChangeLog:
2019-10-09 Tom de Vries <tdevries@suse.de>
PR testsuite/25048
* gdb.rust/simple.exp: Add xfails for incorrect DWARF.
I'm seeing this failure:
...
(gdb) print /x $bnd0 = {0x10, 0x20}^M
$23 = {lbound = 0x10, ubound = 0x20}^M
(gdb) FAIL: gdb.arch/i386-mpx.exp: verify size for bnd0
...
The test expects a pretty printer to be actived printing 'size 17':
...
set test_string ".*\\\: size 17.*"
gdb_test "print /x \$bnd0 = {0x10, 0x20}" "$test_string" "verify size for bnd0"
...
but that doesn't happen.
The pretty printer is for the type of the $bnd0 register, which is created
here in i386_bnd_type:
...
t = arch_composite_type (gdbarch,
"__gdb_builtin_type_bound128", TYPE_CODE_STRUCT);
append_composite_type_field (t, "lbound", bt->builtin_data_ptr);
append_composite_type_field (t, "ubound", bt->builtin_data_ptr);
TYPE_NAME (t) = "builtin_type_bound128";
...
And the pretty-printer is registered here in
gdb/python/lib/gdb/printer/bound_registers.py:
...
gdb.printing.add_builtin_pretty_printer ('mpx_bound128',
'^__gdb_builtin_type_bound128',
MpxBound128Printer)
...
Fix the pretty printer by changing the regexp argument of
add_builtin_pretty_printer to match "builtin_type_bound128", the TYPE_NAME.
Tested on x86_64-linux.
gdb/ChangeLog:
2019-10-09 Tom de Vries <tdevries@suse.de>
* python/lib/gdb/printer/bound_registers.py: Use
'^builtin_type_bound128' as regexp argument for
add_builtin_pretty_printer.
This makes it clearer that the structs are only used in this file. It
required moving the definition of extension_language_guile further
down in the file, because static structs can't be forward-declared.
gdb/ChangeLog:
2019-10-09 Christian Biesinger <cbiesinger@google.com>
* guile/guile.c (guile_extension_script_ops): Remove forward
declaration and mark as static.
(guile_script_ops): Likewise.
(extension_language_guile): Move further down in the file so
it can reference the definitions for guile_{extension_,}script_ops.
PR 25041
opcodes * avr-dis.c (avr_operand): Fix construction of address for lds/sts
instructions.
gas * testsuite/gas/avr/pr25041.s: New test.
* testsuite/gas/avr/pr25041.d: New test driver.
The testcase in the PR has two empty output sections, .sec1 with an
ALIGN and symbol assignment, and .sec2 just with an empty input
section. The symbol assignment results in .sec1 being kept, but
because it is empty this section doesn't take space from the memory
region as you might expect from the ALIGN. Instead the next section
.sec2, has vma/lma as if .sec1 wasn't present. However, .sec2 is
discarded and os->ignored set, which unfortunately meant that dot
wasn't set from .sec2 vma. That in turn results in .sec2 lma being
set incorrectly. That vma/lma difference is then propagated to
.sec3 where it is seen as an overlap.
PR 25081
* ldlang.c (lang_size_sections_1): Set lma from section vma
rather than dot.
Enable recording most of the new "arch13" instructions on z/Architecture
targets, except for the specialized-function-assist instructions:
SORTL - sort lists
DFLTCC - deflate conversion call
KDSA - compute digital signature authentication
gdb/ChangeLog:
* s390-tdep.c (390_process_record): Handle new arch13 instructions
except SORTL, DFLTCC, and KDSA.
Evil testcase with two debug info sections, with sizes of 2aaaabac4ec1
and ffffd5555453b140 result in a total size of 1. Reading the first
section of course overflows the buffer and tramples on other memory.
PR 25070
* dwarf2.c (_bfd_dwarf2_slurp_debug_info): Catch overflow of
total_size calculation.
windows_thread_info_struct::sf is unused, as is
struct safe_symbol_file_add_args in windows-nat.c.
This patch removes them both. Tested by grep and
rebuilding.
gdb/ChangeLog
2019-10-08 Tom Tromey <tromey@adacore.com>
* windows-nat.c (struct windows_thread_info_struct) <sf>: Remove.
(struct safe_symbol_file_add_args): Remove.
I noticed that windows-nat.c includes buildsym-legacy.h -- but there's
no reason to do so, as windows-nat.c doesn't create any symbols.
gdb/ChangeLog
2019-10-08 Tom Tromey <tromey@adacore.com>
* windows-nat.c: Don't include buildsym-legacy.h.
As pointed out by Simon, this changes ARI to allow the gdb-specific %p
printf extensions.
gdb/ChangeLog
2019-10-08 Tom Tromey <tromey@adacore.com>
* contrib/ari/gdb_ari.sh (%p): Allow gdb-specific %p extensions.
Also makes it localizable.
gdb/ChangeLog:
2019-10-08 Christian Biesinger <cbiesinger@google.com>
* language.c (show_language_command): Pass lang_frame_mismatch_warn
through _().
(lang_frame_mismatch_warn): Make const, mark with N_(), and
move comment...
* language.h (lang_frame_mismatch_warn): ... here. Also add
declaration.
* top.c (lang_frame_mismatch_warn): Remove declaration.
(check_frame_language_change): Pass lang_frame_mismatch_warn
through _().
's' is both a command and a modifier. If given as a command then we
aren't lacking an operation. I think the same goes when mri mode is
selected: any following command line used to be ignored.
PR 25079
* ar.c (decode_options): Don't try for command options if
write_armap or mri_mode is selected.
