| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This changes instantiate_pretty_printer to use gdbpy_ref, removing a
call to Py_DECREF.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (instantiate_pretty_printer): Use gdbpy_ref.
|
|
One call to varobj_clear_saved_item is from the varobj destructor.
This is no longer needed, so this patch removes the call; then inlines
the function into the sole remaining caller.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (varobj_clear_saved_item): Remove.
(update_dynamic_varobj_children): Update.
(varobj::~varobj): Don't call varobj_clear_saved_item.
|
|
This changes varobj_item::value to be a value_ref_ptr, removing some
manual management.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (install_dynamic_child, varobj_clear_saved_item)
(update_dynamic_varobj_children, create_child)
(create_child_with_value): Update.
* varobj-iter.h (struct varobj_item) <value>: Now a
value_ref_ptr.
* python/py-varobj.c (py_varobj_iter::next): Call release_value.
|
|
This changes varobj_dynamic::child_iter to be a unique_ptr, removing
some manual management.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (struct varobj_dynamic) <child_iter>: Now unique_ptr.
(varobj_get_iterator): Return unique_ptr.
(update_dynamic_varobj_children, install_visualizer)
(varobj::~varobj): Update.
* python/python-internal.h (py_varobj_get_iterator): Return
unique_ptr.
* python/py-varobj.c (py_varobj_get_iterator): Return unique_ptr.
|
|
This changes varobj_dynamic::saved_item to be a unique_ptr.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (struct varobj_dynamic) <saved_item>: Now unique_ptr.
(varobj_clear_saved_item, update_dynamic_varobj_children):
Update.
|
|
This changes varobj_iter::next to return a unique_ptr. This fits in
with the ongoing theme of trying to express these ownership transfers
via the type system.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (update_dynamic_varobj_children): Update.
* varobj-iter.h (struct varobj_iter) <next>: Change return type.
* python/py-varobj.c (struct py_varobj_iter) <next>: Change return
type.
(py_varobj_iter::next): Likewise.
|
|
This changes the varobj iteration code to use a C++ class rather than
a C struct with a separate "ops" structure. The only implementation
is updated to use inheritance. This simplifies the code quite nicely.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (update_dynamic_varobj_children, install_visualizer)
(varobj::~varobj): Update.
* varobj-iter.h (struct varobj_iter): Change to interface class.
(struct varobj_iter_ops): Remove.
(varobj_iter_next, varobj_iter_delete): Remove.
* python/py-varobj.c (struct py_varobj_iter): Derive from
varobj_iter. Add constructor, destructor. Rename members.
(py_varobj_iter::~py_varobj_iter): Rename from
py_varobj_iter_dtor.
(py_varobj_iter::next): Rename from py_varobj_iter_next.
(py_varobj_iter_ops): Remove.
(py_varobj_iter): Rename from py_varobj_iter_ctor.
(py_varobj_iter_new): Remove.
(py_varobj_get_iterator): Update.
|
|
This changes all_root_varobjs to take a function_view. This
simplifies some of the callers, in particular we can remove a data
type that only existed to be passed through.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.h (all_root_varobjs): Take a function_view.
* varobj.c (all_root_varobjs): Take a function_view.
(varobj_invalidate_iter): Remove unused parameter.
(varobj_invalidate): Update.
* mi/mi-cmd-var.c (struct mi_cmd_var_update): Remove.
(mi_cmd_var_update_iter): Change parameters.
|
|
This changes varobj.c:rootlist to be a std::list. This lets us remove
some code. std::list is chosen because its iterator invalidation
approach suits the all_root_varobjs API.
I considered replacing all_root_varobjs with "external iteration", but
haven't gotten around to doing so.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (struct varobj_root) <next>: Remove.
(struct vlist): Remove.
(rootlist): Now a std::list.
(install_variable, uninstall_variable, all_root_varobjs): Update.
|
|
varobj.c currently has its own hash table implementation. This patch
replaces it with htab_t, simplifying the code.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* varobj.c (VAROBJ_TABLE_SIZE): Remove.
(varobj_table): Now htab_t.
(varobj_get_handle, install_variable, uninstall_variable):
Update.
(hash_varobj, eq_varobj_and_string): New functions.
(hash_varobj): Update.
|
|
This changes bp_location to derive from refcounted_object, introduces
a ref_ptr specialization for this type, and then changes
bpstats::bp_location_at to use that specialization. This removes some
manual reference counting and simplifies the code.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* inline-frame.c (stopped_by_user_bp_inline_frame): Update.
* ada-lang.c (check_status_exception): Update.
* breakpoint.c (free_bp_location): Remove.
(decref_bp_location): Use bp_location_ref_policy.
(bpstats::bpstats): Don't call incref_bp_location.
(bpstats::~bpstats): Remove.
(bpstats::bpstats): Update.
(bpstat_check_watchpoint, bpstat_check_breakpoint_conditions)
(bp_location::bp_location): Update.
(incref_bp_location): Remove.
(bkpt_print_it): Update.
* breakpoint.h (class bp_location): Derive from
refcounted_object.
(struct bpstats): Remove destructor.
<bp_location_at>: Now a bp_location_ref_ptr.
<refc>: Remove.
(bp_location_ref_ptr): New typedef.
(struct bp_location_ref_policy): New.
|
|
We can remove scoped_inc_dec_ref by changing the sole user to instead
keep a vector of thread_info_ref objects. This removes some manual
reference counting and simplifies the code a bit.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* thread.c (class scoped_inc_dec_ref): Remove.
(tp_array_compar_ascending, tp_array_compar_descending): Change
parameter types.
(thread_apply_all_command): Use thread_info_ref.
|
|
This changes stop_context to use a thread_info_ref, removing some
manual reference counting.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* infrun.c (struct stop_context) <thread>: Now a thread_info_ref.
(stop_context::stop_context): Update.
(stop_context::~stop_context): Remove.
|
|
This changes current_inferior_ to be an inferior_ref, removing some
manual reference counting.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* inferior.c (current_inferior_): Change type.
(current_inferior, set_current_inferior, initialize_inferiors):
Update.
|
|
This changes enable_thread_stack_temporaries to use a thread_info_ref,
removing some manual reference counting.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* gdbthread.h (class enable_thread_stack_temporaries) <m_thr>:
Change type.
|
|
The Ravenscar support assumes that the thread ID is the same as the
CPU ID that appears in the Ada task structure. However, on some
systems, gdbserver will report thread IDs that are off by some
constant. This can happen, e.g., with qemu in a scenario where there
is an additional (unreported) CPU in the emulation.
The Ada Ravenscar runtimes have been modified to store this offset in
a global variable. This patch changes gdb to read this variable, when
it exists, and apply the offset to the base CPU ID. This fixes some
crashes that otherwise occur.
2020-12-11 Tom Tromey <tromey@adacore.com>
* ada-tasks.c (struct ada_tasks_pspace_data) <cpu_id_offset>: New
field.
(ada_get_tcb_types_info): Look for __gnat_gdb_cpu_first_id.
(read_atcb): Use cpu_id_offset.
|
|
When configuring gdb using --with-mpfr=no and running test-case
gdb.base/float128.exp, we run into:
...
FAIL: gdb.base/float128.exp: print large128 (GDB may be missing MPFR support!)
...
Fix this by detecting that gdb was build without mpfr using the show
configuration command, and changing the FAIL into UNSUPPORTED.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-12-11 Tom de Vries <tdevries@suse.de>
PR testsuite/26954
* gdb.base/float128.exp: Detect and handle no mpfr support.
|
|
That test fails intermittently for me. The problem is a race condition
between the exec syscall and GDB resuming threads.
The initial situation is that we have two threads, let's call them
"leader" and "other". Leader is the one who is going to do the exec.
We stop at the breakpoint on the all_started function, so both threads
are stopped. When resuming, GDB resumes leader first and other second.
However, between resuming the two threads, leader has time to run and do
its exec, making other disappear. When GDB tries to resume other, it is
ino longer there. We get some "Couldn't get registers: No such
process." messages, and the state is a bit messed up.
The issue can be triggered consistently by adding a small delay after
the resume syscall:
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index d5a062163c7..9540339a9da 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -308,6 +308,8 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal));
if (errno != 0)
perror_with_name (("ptrace"));
+ for (int i = 0 ; i < 100; i++)
+ usleep (10000);
}
/* Wait for the child specified by PTID to do something. Return the
This patch is about fixing the test to avoid this, since the test is not
about testing this particular corner case. Handling of multi-threaded
program doing execs should be improved too, but that's not the goal of
this patch.
Fix it by adding a synchronization point in the test to make sure both
threads were resumed by GDB before doing the exec. I added two
pthread_barrier_wait calls in each thread (for a total of three). I
think adding one call in each thread would not be enough, because this
could happen:
- both threads reach the first barrier
- the "other" thread is scheduled so has time to run and hit the second
barrier
- the "leader" thread hits the all_started function breakpoint, causing
both threads to be stopped by GDB
- GDB resumes the "leader" thread
- Since the "other" thread has already reached the second barrier, the
"leader" thread is free to run past its second barrier and do the
exec, while GDB still hasn't resumed the second one
By adding two barrier calls in each thread, I think we are good. The test
passes consistently for me, even with the artificial delay added.
gdb/testsuite/ChangeLog:
PR gdb/24694
* gdb.multi/multi-arch-exec.c (thread_start, main): Add barrier
calls.
Change-Id: I25c8ea9724010b6bf20b42691c716235537d0e27
|
|
In commit 4d91ddd342 "[gdb/testsuite] Fix unbalanced braces in
gdb.tui/new-layout.exp", I tried to fix a problem with test-case
gdb.tui/new-layout.exp when running with tcl 8.5.
However, at that point I only had access to the log containing the failure,
and unfortunately my patch turned out not to be effective.
So, finally fix this problem by guarding the problematic code with:
...
if { [tcl_version_at_least 8 6] } { ... }
...
Tested on x86_64-linux, specifically SLE-11 where I ran into the failure.
gdb/testsuite/ChangeLog:
2020-12-10 Tom de Vries <tdevries@suse.de>
PR testsuite/26947
* gdb.tui/new-layout.exp: Don't execute tests with unbalanced curly
braces for tcl 8.5 and earlier.
|
|
I'm seeing a libstdc++ assertion failure when running GDB's "maint selftest"
command when GDB is configured with the following CFLAGS and CXXFLAGS as
part of the configure line:
CFLAGS='-D_GLIBCXX_DEBUG -g3 -O0' CXXFLAGS='-D_GLIBCXX_DEBUG -g3 -O0'
This is what I see when running the self tests:
(gdb) maint selftest
Running selftest aarch64-analyze-prologue.
Running selftest aarch64-process-record.
Running selftest arm-record.
Running selftest arm_analyze_prologue.
Running selftest array_view.
Running selftest child_path.
Running selftest cli_utils.
Running selftest command_structure_invariants.
Running selftest copy_bitwise.
Running selftest copy_integer_to_size.
Running selftest cp_remove_params.
Running selftest cp_symbol_name_matches.
Running selftest dw2_expand_symtabs_matching.
/usr/include/c++/11/string_view:211: constexpr const value_type& std::basic_string_view<_CharT, _Traits>::operator[](std::basic_string_view<_CharT, _Traits>::size_type) const [with _CharT = char; _Traits = std::char_traits<char>; std::basic_string_view<_CharT, _Traits>::const_reference = const char&; std::basic_string_view<_CharT, _Traits>::size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed.
Aborted (core dumped)
Here's a partial stack trace:
#0 0x00007ffff6ef6262 in raise () from /lib64/libc.so.6
#1 0x00007ffff6edf8a4 in abort () from /lib64/libc.so.6
#2 0x00000000004249bf in std::__replacement_assert (
__file=0xef7480 "/usr/include/c++/11/string_view", __line=211,
__function=0xef7328 "constexpr const value_type& std::basic_string_view<_CharT, _Traits>::operator[](std::basic_string_view<_CharT, _Traits>::size_type) const [with _CharT = char; _Traits = std::char_traits<char>; std::ba"...,
__condition=0xef7311 "__pos < this->_M_len")
at /usr/include/c++/11/x86_64-redhat-linux/bits/c++config.h:2624
#3 0x0000000000451737 in std::basic_string_view<char, std::char_traits<char> >::operator[] (this=0x7fffffffc200, __pos=8)
at /usr/include/c++/11/string_view:211
#4 0x00000000004329f5 in ada_fold_name (name="function")
at /ironwood1/sourceware-git/rawhide-master/bld/../../worktree-master/gdb/ada-lang.c:988
And, looking at frame #4...
(top-gdb) up 4
at /ironwood1/sourceware-git/rawhide-master/bld/../../worktree-master/gdb/ada-lang.c:988
988 fold_buffer[i] = tolower (name[i]);
(top-gdb) p i
$1 = 8
(top-gdb) p name.size()
$2 = 8
My patch adjusts the comparison to only copy name.size() characters
from the string. I've added a separate statement for NUL character
termination of fold_buffer[].
gdb/ChangeLog:
* ada-lang.c (ada_fold_name): Fix off-by-one error.
|
|
I noticed a spurious newline on infrun debugging output. The following patch
fixes that. I'll push as obvious.
gdb/ChangeLog:
2020-12-10 Luis Machado <luis.machado@linaro.org>
* breakpoint.c (should_be_inserted): Don't output newline.
|
|
The FPSIMD dump in signal frames and ptrace FPSIMD dump in the SVE context
structure follows the target endianness, whereas the SVE dumps are
endianness-independent (LE).
Therefore, when the system is in BE mode, we need to reverse the bytes
for the FPSIMD data.
Given the V registers are larger than 64-bit, I've added a way for value
bytes to be set, as opposed to passing a 64-bit fixed quantity. This fits
nicely with the unwinding *_got_bytes function and makes the trad-frame
more flexible and capable of saving larger registers.
The memory for the bytes is allocated via the frame obstack, so it gets freed
after we're done inspecting the frame.
gdb/ChangeLog:
2020-12-10 Luis Machado <luis.machado@linaro.org>
* aarch64-linux-tdep.c (aarch64_linux_restore_vreg) New function.
(aarch64_linux_sigframe_init): Call aarch64_linux_restore_vreg.
* aarch64-tdep.h (V_REGISTER_SIZE): Move to ...
* arch/aarch64.h: ... here.
* nat/aarch64-sve-linux-ptrace.c: Include endian.h.
(aarch64_maybe_swab128): New function.
(aarch64_sve_regs_copy_to_reg_buf)
(aarch64_sve_regs_copy_from_reg_buf): Adjust FPSIMD entries.
* trad-frame.c (trad_frame_reset_saved_regs): Initialize
the data field.
(TF_REG_VALUE_BYTES): New enum value.
(trad_frame_value_bytes_p): New function.
(trad_frame_set_value_bytes): New function.
(trad_frame_set_reg_value_bytes): New function.
(trad_frame_get_prev_register): Handle register values saved as bytes.
* trad-frame.h (trad_frame_set_reg_value_bytes): New prototype.
(struct trad_frame_saved_reg) <data>: New field.
(trad_frame_set_value_bytes): New prototype.
(trad_frame_value_bytes_p): New prototype.
|
|
This function allows to create a BFD handle using an accessible memory
range in a target memory. It is currently contained in a JIT module but
this functionality may be of wider usefullness - for example, reading
ELF binaries contained within a core dump.
gdb/ChangeLog:
2020-12-07 Mihails Strasuns <mihails.strasuns@intel.com>
* jit.c (mem_bfd*, bfd_open_from_target_memory): Removed.
* gdb_bfd.h (gdb_bfd_open_from_target_memory): New function.
* gdb_bfd.c (mem_bfd*, gdb_bfd_open_from_target_memory): New functions.
|
|
Joel recently pointed out add_angle_brackets to me. This patch
changes one spot in ada-lang.c to use this function rather than doing
it on its own.
gdb/ChangeLog
2020-12-09 Tom Tromey <tromey@adacore.com>
* ada-lang.c (ada_lookup_encoded_symbol): Use add_angle_brackets.
|
|
In some cases, GNAT can emit 128-bit constants for fixed-point types.
This patch changes gdb to handle this scenario, by changing the
low-level rational-reading functions in dwarf2/read.c to work directly
with gdb_mpz values. (I'm not sure offhand if these 128-bit patches
have gone into upstream GCC yet -- but they will eventually, and
meanwhile I think it should be clear that this patch is otherwise
harmless.)
gdb/ChangeLog
2020-12-09 Tom Tromey <tromey@adacore.com>
* dwarf2/read.c (get_dwarf2_rational_constant): Change "numerator"
and "denominator" to gdb_mpz. Handle block forms.
(get_dwarf2_unsigned_rational_constant): Change "numerator" and
"denominator" to gdb_mpz.
(finish_fixed_point_type): Update.
(has_zero_over_zero_small_attribute): Update.
|
|
This removes ada-operator.def and fortran-operator.def, merging their
contents into std-operator.def.
Note that the comment for OP_EXTENDED0 is a bit wrong. IMO this
constant could be removed, as it is only used for a single assert that
does not provide much value. However, I haven't done so here.
gdb/ChangeLog
2020-12-09 Tom Tromey <tromey@adacore.com>
* expprint.c (op_name): Update.
* expression.h (enum exp_opcode): Update.
* std-operator.def: Add more opcodes.
* ada-operator.def, fortran-operator.def: Remove, moving contents
into std-operator.def.
|
|
I forgot to include fixes for review comments I got before pushing the
previous commits (or I pushed the wrong commits). This one fixes it.
- Return {} instead of false in get_discrete_low_bound and
get_discrete_high_bound.
- Compute high bound after confirming low bound is valid in
get_discrete_bounds.
gdb/ChangeLog:
* gdbtypes.c (get_discrete_low_bound, get_discrete_high_bound):
Return {} instead of false.
(get_discrete_bounds): Compute high bound only if low bound is
valid.
Change-Id: I5f9a66b3672adfac9441068c899ab113ab2c331a
|
|
Since commit 7c6f27129631 ("gdb: make get_discrete_bounds check for
non-constant range bounds"), subscripting flexible array member fails:
struct no_size
{
int n;
int items[];
};
(gdb) p *ns
$1 = {n = 3, items = 0x5555555592a4}
(gdb) p ns->items[0]
Cannot access memory at address 0xfffe555b733a0164
(gdb) p *((int *) 0x5555555592a4)
$2 = 101 <--- we would expect that
(gdb) p &ns->items[0]
$3 = (int *) 0xfffe5559ee829a24 <--- wrong address
Since the flexible array member (items) has an unspecified size, the array type
created for it in the DWARF doesn't have dimensions (this is with gcc 9.3.0,
Ubuntu 20.04):
0x000000a4: DW_TAG_array_type
DW_AT_type [DW_FORM_ref4] (0x00000038 "int")
DW_AT_sibling [DW_FORM_ref4] (0x000000b3)
0x000000ad: DW_TAG_subrange_type
DW_AT_type [DW_FORM_ref4] (0x00000031 "long unsigned int")
This causes GDB to create a range type (TYPE_CODE_RANGE) with a defined
constant low bound (dynamic_prop with kind PROP_CONST) and an undefined
high bound (dynamic_prop with kind PROP_UNDEFINED).
value_subscript gets both bounds of that range using
get_discrete_bounds. Before commit 7c6f27129631, get_discrete_bounds
didn't check the kind of the dynamic_props and would just blindly read
them as if they were PROP_CONST. It would return 0 for the high bound,
because we zero-initialize the range_bounds structure. And it didn't
really matter in this case, because the returned high bound wasn't used
in the end.
Commit 7c6f27129631 changed get_discrete_bounds to return a failure if
either the low or high bound is not a constant, to make sure we don't
read a dynamic prop that isn't a PROP_CONST as a PROP_CONST. This
change made get_discrete_bounds start to return a failure for that
range, and as a result would not set *lowp and *highp. And since
value_subscript doesn't check get_discrete_bounds' return value, it just
carries on an uses an uninitialized value for the low bound. If
value_subscript did check the return value of get_discrete_bounds, we
would get an error message instead of a bogus value. But it would still
be a bug, as we wouldn't be able to print the flexible array member's
elements.
Looking at value_subscript, we see that the low bound is always needed,
but the high bound is only needed if !c_style. So, change
value_subscript to use get_discrete_low_bound and
get_discrete_high_bound separately. This fixes the case described
above, where the low bound is known but the high bound isn't (and is not
needed). This restores the original behavior without accessing a
dynamic_prop in a wrong way.
A test is added. In addition to the case described above, a case with
an array member of size 0 is added, which is a GNU C extension that
existed before flexible array members were introduced. That case
currently fails when compiled with gcc <= 8. gcc <= 8 produces DWARF
similar to the one shown above, while gcc 9 adds a DW_AT_count of 0 in
there, which makes the high bound known. A case where an array member
of size 0 is the only member of the struct is also added, as that was
how PR 28675 was originally reported, and it's an interesting corner
case that I think could trigger other funny bugs.
Question about the implementation: in value_subscript, I made it such
that if the low or high bound is unknown, we fall back to zero. That
effectively makes it the same as it was before 7c6f27129631. But should
we instead error() out?
gdb/ChangeLog:
PR 26875, PR 26901
* gdbtypes.c (get_discrete_low_bound): Make non-static.
(get_discrete_high_bound): Make non-static.
* gdbtypes.h (get_discrete_low_bound): New declaration.
(get_discrete_high_bound): New declaration.
* valarith.c (value_subscript): Only fetch high bound if
necessary.
gdb/testsuite/ChangeLog:
PR 26875, PR 26901
* gdb.base/flexible-array-member.c: New test.
* gdb.base/flexible-array-member.exp: New test.
Change-Id: I832056f80e6c56f621f398b4780d55a3a1e299d7
|
|
get_discrete_bounds is not flexible for ranges (TYPE_CODE_RANGE), in the
sense that it returns true (success) only if both bounds are present and
constant values.
This is a problem for code that only needs to know the low bound and
fails unnecessarily if the high bound is unknown.
Split the function in two, get_discrete_low_bound and
get_discrete_high_bound, that both return an optional. Provide a new
implementation of get_discrete_bounds based on the two others, so the
callers don't have to be changed.
gdb/ChangeLog:
* gdbtypes.c (get_discrete_bounds): Implement with
get_discrete_low_bound and get_discrete_high_bound.
(get_discrete_low_bound): New.
(get_discrete_high_bound): New.
Change-Id: I986b5e9c0dd969800e3fb9546af9c827d52e80d0
|
|
get_discrete_bounds currently has three possible return values (see its
current doc for details). It appears that for all callers, it would be
sufficient to have a boolean "worked" / "didn't work" return value.
Change the return type of get_discrete_bounds to bool and adjust all
callers. Doing so simplifies the following patch.
gdb/ChangeLog:
* gdbtypes.h (get_discrete_bounds): Return bool, adjust all
callers.
* gdbtypes.c (get_discrete_bounds): Return bool.
Change-Id: Ie51feee23c75f0cd7939742604282d745db59172
|
|
Instead of returning a boolean status and returning the value through a
pointer, return an optional that does both jobs. This helps in the
following patches, and I think it is an improvement in general.
gdb/ChangeLog:
* ada-lang.c (ada_value_slice_from_ptr): Adjust.
(ada_value_slice): Adjust.
(pos_atr): Adjust.
* gdbtypes.c (get_discrete_bounds): Adjust.
(discrete_position): Return optional.
* gdbtypes.h (discrete_position): Return optional.
Change-Id: I758dbd8858b296ee472ed39ec35db1dbd624a5ae
|
|
Redo fix committed in commit 67748e0f66 "[gdb/testsuite] Make
gdb.arch/amd64-gs_base.exp unsupported for i386" using is_amd64_regs_target.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-12-08 Tom de Vries <tdevries@suse.de>
* gdb.arch/amd64-gs_base.exp: Undo commit 67748e0f66, reimplement
using is_amd64_regs_target.
|
|
When running test-case gdb.ada/mi_task_arg.exp with target board unix/-m32, I
run into:
...
(gdb) ^M
Expecting: ^(-stack-list-arguments 1[^M
]+)?(\^done,stack-args=\[ \
frame={level="0",args=\[\]}, \
frame={level="1",args=\[{name="<_task>",value="0x[0-9A-Fa-f]+"}\]}, \
frame={level="2",args=\[({name="self_id",value="0x[0-9A-Fa-f]+"})?\]},.*[^M
]+[(]gdb[)] ^M
[ ]*)
-stack-list-arguments 1^M
^done,stack-args=[ \
frame={level="0",args=[]}, \
frame={level="1",args=[{name="<_task>",value="0x808abf0"}]}, \
frame={level="2",args=[{name="self_id",value="<optimized out>"}]}, \
frame={level="3",args=[]},frame={level="4",args=[]}]^M
(gdb) ^M
FAIL: gdb.ada/mi_task_arg.exp: -stack-list-arguments 1 (unexpected output)
...
The problem is that we're expecting a $hex for the value of self_id, but
instead get <optimized out>.
Looking at the debug info for self_id:
...
<1><12a1f>: Abbrev Number: 84 (DW_TAG_subprogram)
<12a20> DW_AT_name : system__tasking__stages__task_wrapper
...
<2><12a35>: Abbrev Number: 61 (DW_TAG_formal_parameter)
<12a36> DW_AT_name : self_id
<12a40> DW_AT_location : 0x459e (location list)
...
it refers to location information here:
...
0000459e 08053060 080531ac (DW_OP_fbreg: 0)
000045aa 0805327c 080532a5 (DW_OP_fbreg: 0)
000045b6 08053320 08053324 (DW_OP_fbreg: 0)
...
while the pc used to retrieve the location information is 0x080531c5:
...
$ gdb -batch outputs/gdb.ada/mi_task_arg/task_switch \
-ex "break 57" -ex run -ex bt
...
#0 task_switch.break_me () at task_switch.adb:57
#1 0x0804aaae in task_switch.caller (<_task>=0x808abf0) \
at task_switch.adb:51
#2 0x080531c5 in system.tasking.stages.task_wrapper \
(self_id=<optimized out>) at s-tassta.adb:1295
...
which indeed falls outside of the ranges listed in the location info.
Fix this by accepting <optimized out> as valid value of self_id.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-12-08 Tom de Vries <tdevries@suse.de>
* gdb.ada/mi_task_arg.exp: Accept <optimized out> as valid value of
self_id.
|
|
Commit:
commit 4d142eaa28c64565b58fcdb5a83377ec9b778cb1
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
AuthorDate: Tue Jul 2 20:06:12 2013 +0000
gdb/testsuite/
* gdb.base/break-on-linker-gcd-function.exp: Replace
prepare_for_testing by build_executable_from_specs and clean_restart.
...
did:
set additional_flags {-ffunction-sections -Wl,--gc-sections}
-if {[prepare_for_testing $testfile.exp $testfile $srcfile \
- [list debug c++ additional_flags=$additional_flags]]} {
+if {[build_executable_from_specs $testfile.exp $testfile \
+ {c++ additional_flags=-Wl,--gc-sections} \
+ $srcfile {debug c++ additional_flags=-ffunction-sections}]} {
and that left the additional_flags variable behind. Remove it.
gdb/testsuite/ChangeLog:
* gdb.base/break-on-linker-gcd-function.exp: Remove unused
'additional_flags' variable.
|
|
I happened to notice that expression completion did not work correctly
for "maint print type". This patch adds the appropriate completer
there.
gdb/ChangeLog
2020-12-07 Tom Tromey <tromey@adacore.com>
* maint.c (_initialize_maint_cmds): Use expression command
completer for "maint print type".
|
|
The commit
commit 733d554a4625db4ffb89b7a20e1cf27ab071ef4d
Author: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Date: Tue Oct 27 10:56:03 2020 +0100
gdb/breakpoint: add flags to 'condition' and 'break' commands to force condition
introduced the '-force-condition' flag to the 'break' command. This
flag was defined as a keyword like 'thread', 'task', and 'if'.
However, it starts with '-'. This difference caused an uncovered case
when tab-completing a seemingly complete linespec.
Below, we see "-force-condition" in the completion list, where both
the options and the keywords are listed:
(gdb) break -function main <TAB>
-force-condition -function -label -line -qualified
-source if task thread
But tab-completing '-' lists only options:
(gdb) break -function main -<TAB>
-function -label -line -qualified -source
This patch fixes the problem by adding keywords to the completion
list, so that we see:
(gdb) break -function main -<TAB>
-force-condition -function -label -line -qualified -source
gdb/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* completer.c (complete_explicit_location): Also add keywords
that start with '-' to the completion list.
gdb/testsuite/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.linespec/explicit.exp: Extend with a test to check completing
'-' after seemingly complete options.
|
|
The break command's "-force-condition" flag is currently required to
be followed by the "if" keyword. This prevents flexibility when using
other keywords, e.g. "thread":
(gdb) break main -force-condition thread 1 if foo
Function "main -force-condition" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
Remove the requirement that "-force-condition" is always followed by
an "if", so that more flexibility is obtained when positioning
keywords.
gdb/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* linespec.c (linespec_lexer_lex_keyword): The "-force-condition"
keyword may be followed by any keyword.
* breakpoint.c (find_condition_and_thread): Advance 'tok' by
'toklen' in the case for "-force-condition".
gdb/testsuite/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.linespec/keywords.exp: Add tests to check positional
flexibility of "-force-condition".
|
|
Suppose we have the script file below:
break main
commands
print 123
end
run
If started with this script file, GDB executes the breakpoint command:
$ gdb -q -x myscript --args ./test
Reading symbols from ./test...
Breakpoint 1 at 0x114e: file test.c, line 2.
Breakpoint 1, main () at test.c:2
2 return 0;
$1 = 123
(gdb)
However, if we remove the "run" line from the script and pass it with
the '-ex' option instead, the command is not executed:
$ gdb -q -x myscript_no_run --args ./test
Reading symbols from ./test...
Breakpoint 1 at 0x114e: file test.c, line 2.
Starting program: /path/to/test
Breakpoint 1, main () at test.c:2
2 return 0;
(gdb)
If the user enters a command at this point, the breakpoint command
is executed, yielding weird output:
$ gdb -q -x myscript_no_run --args ./test
Reading symbols from ./test...
Breakpoint 1 at 0x114e: file test.c, line 2.
Starting program: /path/to/test
Breakpoint 1, main () at test.c:2
2 return 0;
(gdb) print "a"
$1 = "a"
$2 = 123
When consuming script files, GDB runs bp actions after executing a
command. See `command_handler` in event-top.c:
if (c[0] != '#')
{
execute_command (command, ui->instream == ui->stdin_stream);
/* Do any commands attached to breakpoint we stopped at. */
bpstat_do_actions ();
}
However, for '-ex' commands, `bpstat_do_actions` is not invoked.
Hence, the misaligned output explained above occurs. To fix the
problem, add a call to `bpstat_do_actions` after executing a command.
gdb/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* main.c (catch_command_errors): Add a flag parameter; invoke
`bpstat_do_actions` if the flag is set.
(execute_cmdargs): Update a call to `catch_command_errors`.
gdb/testsuite/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.base/bp-cmds-run-with-ex.c: New file.
* gdb.base/bp-cmds-run-with-ex.exp: New file.
* gdb.base/bp-cmds-run-with-ex.gdb: New file.
* gdb.gdb/python-interrupts.exp: Update the call to
'catch_command_errors' with the new argument.
* gdb.gdb/python-selftest.exp: Ditto.
|
|
In replace_operator_with_call, we resize the elts array like this:
...
exp->nelts = exp->nelts + 7 - oplen;
exp->resize (exp->nelts);
...
Although all the current callers ensure that the new size is bigger, it could
also be smaller, in which case the following memmove possibly reads out of
bounds:
...
memmove (exp->elts + pc + 7, exp->elts + pc + oplen,
EXP_ELEM_TO_BYTES (save_nelts - pc - oplen));
...
Fix this by doing the resize after the memmove in case the new size is
smaller.
Tested on x86_64-linux.
gdb/ChangeLog:
2020-12-07 Tom de Vries <tdevries@suse.de>
* ada-lang.c (replace_operator_with_call): Handle shrink resize.
|
|
The patch to change struct expression to use new introduced a
regression -- there is a spot that reallocates expressions that I
failed to update.
This patch rewrites this code to follow the new approach. Now the
rewriting is done in place.
gdb/ChangeLog
2020-12-06 Tom Tromey <tom@tromey.com>
PR ada/26999
* ada-lang.c (replace_operator_with_call): Rewrite.
|
|
This fixes a long-lived bug in the s390 port.
When trying to step over a breakpoint set on a BC (branch on condition)
instruction with displaced stepping on IBM Z, gdb would incorrectly
adjust the pc regardless of whether or not the branch was taken. Since
the branch target is an absolute address, this would cause the inferior
to jump around wildly whenever the branch was taken, either crashing it
or causing it to behave unpredictably.
It turns out that the logic to handle BC instructions correctly was in
the code, but that the enum value representing its opcode has always
been incorrect.
This patch corrects the enum value to the actual opcode, fixing the
stepping problem. The enum value is also used in the prologue analysis
code, so this also fixes a minor bug where more of the prologue would
be read than was necessary.
gdb/ChangeLog:
PR breakpoints/27009
* s390-tdep.h (op_bc): Correct BC opcode value.
|
|
The gdb_mpz class currently provides a couple of methods which
essentially export an mpz_t value into either a buffer, or an integral
type. The export is based on using the mpz_export function which
we discovered can be a bit treacherous if used without caution.
In particular, the initial motivation for this patch was to catch
situations where the mpz_t value was so large that it would not fit
in the destination area. mpz_export does not know the size of
the buffer, and therefore can happily write past the end of our buffer.
While designing a solution to the above problem, I also discovered
that we also needed to be careful when exporting signed numbers.
In particular, numbers which are larger than the maximum value
for a given signed type size, but no so large as to fit in the
*unsigned* version with the same size, would end up being exported
incorrectly. This is related to the fact that mpz_export ignores
the sign of the value being exportd, and assumes an unsigned export.
Thus, for such large values, the appears as if mpz_export is able
to fit our value into our buffer, but in fact, it does not.
Also, I noticed that gdb_mpz::write wasn't taking its unsigned_p
parameter, which was a hole.
For all these reasons, a new low-level private method called
"safe_export" has been added to class gdb_mpz, whose goal is
to perform all necessary checks and manipulations for a safe
and correct export. As a bonus, this method allows us to factorize
the handling of negative value exports.
The gdb_mpz::as_integer and gdb_mpz::write methods are then simplified
to take advantage of this new safe_export method.
gdb/ChangeLog:
* gmp-utils.h (gdb_mpz::safe_export): New private method.
(gdb_mpz::as_integer): Reimplement using gdb_mpz::safe_export.
* gmp-utils.c (gdb_mpz::write): Rewrite using gdb_mpz::safe_export.
(gdb_mpz::safe_export): New method.
* unittests/gmp-utils-selftests .c (gdb_mpz_as_integer):
Update function description.
(check_as_integer_raises_out_of_range_error): New function.
(gdb_mpz_as_integer_out_of_range): New function.
(_initialize_gmp_utils_selftests): Register
gdb_mpz_as_integer_out_of_range as a selftest.
|
|
In a couple of gdb_mpz methods, we are computing the number of
bits in a gdb::array_view of gdb_byte. Since gdb_byte is defined
using a host-side type (see common-types.h), the number of bits
in a gdb_byte should be HOST_CHAR_BIT, not TARGET_CHAR_BIT.
gdb/ChangeLog:
* gmp-utils.c (gdb_mpz::read): Use HOST_CHAR_BIT instead of
TARGET_CHAR_BIT.
(gdb_mpz::write): Likewise.
|
|
As observed on a binary compiled on AMD64 Ubuntu 20.04, against glibc
2.31 (I think it's the libc that provides this startup code, right?),
there are enough bytes at the executable's entry point to hold more than
one displaced step buffer. gdbarch_max_insn_length is 16, and the
code at _start looks like:
0000000000001040 <_start>:
1040: f3 0f 1e fa endbr64
1044: 31 ed xor %ebp,%ebp
1046: 49 89 d1 mov %rdx,%r9
1049: 5e pop %rsi
104a: 48 89 e2 mov %rsp,%rdx
104d: 48 83 e4 f0 and $0xfffffffffffffff0,%rsp
1051: 50 push %rax
1052: 54 push %rsp
1053: 4c 8d 05 56 01 00 00 lea 0x156(%rip),%r8 # 11b0 <__libc_csu_fini>
105a: 48 8d 0d df 00 00 00 lea 0xdf(%rip),%rcx # 1140 <__libc_csu_init>
1061: 48 8d 3d c1 00 00 00 lea 0xc1(%rip),%rdi # 1129 <main>
1068: ff 15 72 2f 00 00 callq *0x2f72(%rip) # 3fe0 <__libc_start_main@GLIBC_2.2.5>
106e: f4 hlt
106f: 90 nop
The two buffers would occupy [0x1040, 0x1060).
I checked on Alpine, which uses the musl C library, the startup code
looks like:
0000000000001048 <_start>:
1048: 48 31 ed xor %rbp,%rbp
104b: 48 89 e7 mov %rsp,%rdi
104e: 48 8d 35 e3 2d 00 00 lea 0x2de3(%rip),%rsi # 3e38 <_DYNAMIC>
1055: 48 83 e4 f0 and $0xfffffffffffffff0,%rsp
1059: e8 00 00 00 00 callq 105e <_start_c>
000000000000105e <_start_c>:
105e: 48 8b 37 mov (%rdi),%rsi
1061: 48 8d 57 08 lea 0x8(%rdi),%rdx
1065: 45 31 c9 xor %r9d,%r9d
1068: 4c 8d 05 47 01 00 00 lea 0x147(%rip),%r8 # 11b6 <_fini>
106f: 48 8d 0d 8a ff ff ff lea -0x76(%rip),%rcx # 1000 <_init>
1076: 48 8d 3d 0c 01 00 00 lea 0x10c(%rip),%rdi # 1189 <main>
107d: e9 9e ff ff ff jmpq 1020 <__libc_start_main@plt>
Even though there's a _start_c symbol, it all appears to be code that
runs once at the very beginning of the program, so it looks fine if the
two buffers occupy [0x1048, 0x1068).
One important thing I discovered while doing this is that when debugging
a dynamically-linked executable, breakpoints in the shared library
loader are hit before executing the _start code, and these breakpoints
may be displaced-stepped. So it's very important that the buffer bytes
are restored properly after doing the displaced steps, otherwise the
_start code will be corrupted once we try to execute it.
Another thing that made me think about is that library constructors (as
in `__attribute__((constructor))`) run before _start. And they are free
to spawn threads. What if one of these threads executes a displaced
step, therefore changing the bytes at _start, while the main thread
executes _start? That doesn't sound good and I don't know how we could
prevent it. But this is a problem that predates the current patch.
Even when stress-testing the implementation, by making many threads do
displaced steps over and over, I didn't see a significant performance (I
confirmed that the two buffers were used by checking the "set debug
displaced" logs though). However, this patch mostly helps make the
feature testable by anybody with an AMD64/Linux machine, so I think it's
useful.
gdb/ChangeLog:
* amd64-linux-tdep.c (amd64_linux_init_abi): Pass 2 as the
number of displaced step buffers.
Change-Id: Ia0c96ea0fcda893f4726df6fdac7be5214620112
|
|
The displaced_step_buffer class, introduced in the previous patch,
manages access to a single displaced step buffer. Change it into
displaced_step_buffers (note the plural), which manages access to
multiple displaced step buffers.
When preparing a displaced step for a thread, it looks for an unused
buffer.
For now, all users still pass a single displaced step buffer, so no real
behavior change is expected here. The following patch makes a user pass
more than one buffer, so the functionality introduced by this patch is
going to be useful in the next one.
gdb/ChangeLog:
* displaced-stepping.h (struct displaced_step_buffer): Rename
to...
(struct displaced_step_buffers): ... this.
<m_addr, m_current_thread, m_copy_insn_closure>: Remove.
<struct displaced_step_buffer>: New inner class.
<m_buffers>: New.
* displaced-stepping.c (displaced_step_buffer::prepare): Rename
to...
(displaced_step_buffers::prepare): ... this, adjust for multiple
buffers.
(displaced_step_buffer::finish): Rename to...
(displaced_step_buffers::finish): ... this, adjust for multiple
buffers.
(displaced_step_buffer::copy_insn_closure_by_addr): Rename to...
(displaced_step_buffers::copy_insn_closure_by_addr): ... this,
adjust for multiple buffers.
(displaced_step_buffer::restore_in_ptid): Rename to...
(displaced_step_buffers::restore_in_ptid): ... this, adjust for
multiple buffers.
* linux-tdep.h (linux_init_abi): Change supports_displaced_step
for num_disp_step_buffers.
* linux-tdep.c (struct linux_gdbarch_data)
<num_disp_step_buffers>: New field.
(struct linux_info) <disp_step_buf>: Rename to...
<disp_step_bufs>: ... this, change type to
displaced_step_buffers.
(linux_displaced_step_prepare): Use
linux_gdbarch_data::num_disp_step_buffers to create that number
of buffers.
(linux_displaced_step_finish): Adjust.
(linux_displaced_step_copy_insn_closure_by_addr): Adjust.
(linux_displaced_step_restore_all_in_ptid): Adjust.
(linux_init_abi): Change supports_displaced_step parameter for
num_disp_step_buffers, save it in linux_gdbarch_data.
* aarch64-linux-tdep.c (aarch64_linux_init_abi): Adjust.
* alpha-linux-tdep.c (alpha_linux_init_abi): Adjust.
* amd64-linux-tdep.c (amd64_linux_init_abi_common): Change
supports_displaced_step parameter for num_disp_step_buffers.
(amd64_linux_init_abi): Adjust.
(amd64_x32_linux_init_abi): Adjust.
* arc-linux-tdep.c (arc_linux_init_osabi): Adjust.
* arm-linux-tdep.c (arm_linux_init_abi): Adjust.
* bfin-linux-tdep.c (bfin_linux_init_abi): Adjust.
* cris-linux-tdep.c (cris_linux_init_abi): Adjust.
* csky-linux-tdep.c (csky_linux_init_abi): Adjust.
* frv-linux-tdep.c (frv_linux_init_abi): Adjust.
* hppa-linux-tdep.c (hppa_linux_init_abi): Adjust.
* i386-linux-tdep.c (i386_linux_init_abi): Adjust.
* ia64-linux-tdep.c (ia64_linux_init_abi): Adjust.
* m32r-linux-tdep.c (m32r_linux_init_abi): Adjust.
* m68k-linux-tdep.c (m68k_linux_init_abi):
* microblaze-linux-tdep.c (microblaze_linux_init_abi):
* mips-linux-tdep.c (mips_linux_init_abi): Adjust.
* mn10300-linux-tdep.c (am33_linux_init_osabi): Adjust.
* nios2-linux-tdep.c (nios2_linux_init_abi): Adjust.
* or1k-linux-tdep.c (or1k_linux_init_abi): Adjust.
* ppc-linux-tdep.c (ppc_linux_init_abi): Adjust.
* riscv-linux-tdep.c (riscv_linux_init_abi): Adjust.
* rs6000-tdep.c (struct ppc_inferior_data) <disp_step_buf>:
Change type to displaced_step_buffers.
* s390-linux-tdep.c (s390_linux_init_abi_any): Adjust.
* sh-linux-tdep.c (sh_linux_init_abi): Adjust.
* sparc-linux-tdep.c (sparc32_linux_init_abi): Adjust.
* sparc64-linux-tdep.c (sparc64_linux_init_abi): Adjust.
* tic6x-linux-tdep.c (tic6x_uclinux_init_abi): Adjust.
* tilegx-linux-tdep.c (tilegx_linux_init_abi): Adjust.
* xtensa-linux-tdep.c (xtensa_linux_init_abi): Adjust.
Change-Id: Ia9c02f207da2c9e1d9188020139619122392bb70
|
|
The following patch will need to fill a field in linux_gdbarch_data
while the gdbarch is being built. linux_gdbarch_data is currently
allocated as a post-init gdbarch data, meaning it's not possible to fill
it before the gdbarch is completely initialized. Change it to a
pre-init gdbarch data to allow this.
The init_linux_gdbarch_data function doesn't use the created gdbarch,
it only allocates the linux_gdbarch_data structure on the gdbarch's
obstack, so the change is trivial.
gdb/ChangeLog:
* linux-tdep.c (init_linux_gdbarch_data): Change parameter to
obkstack.
(_initialize_linux_tdep): Register pre-init gdb data instead of
post-init.
Change-Id: If35ce91b6bb5435680d43b9268d811d95661644f
|
|
displaced steps
Today, GDB only allows a single displaced stepping operation to happen
per inferior at a time. There is a single displaced stepping buffer per
inferior, whose address is fixed (obtained with
gdbarch_displaced_step_location), managed by infrun.c.
In the case of the AMD ROCm target [1] (in the context of which this
work has been done), it is typical to have thousands of threads (or
waves, in SMT terminology) executing the same code, hitting the same
breakpoint (possibly conditional) and needing to to displaced step it at
the same time. The limitation of only one displaced step executing at a
any given time becomes a real bottleneck.
To fix this bottleneck, we want to make it possible for threads of a
same inferior to execute multiple displaced steps in parallel. This
patch builds the foundation for that.
In essence, this patch moves the task of preparing a displaced step and
cleaning up after to gdbarch functions. This allows using different
schemes for allocating and managing displaced stepping buffers for
different platforms. The gdbarch decides how to assign a buffer to a
thread that needs to execute a displaced step.
On the ROCm target, we are able to allocate one displaced stepping
buffer per thread, so a thread will never have to wait to execute a
displaced step.
On Linux, the entry point of the executable if used as the displaced
stepping buffer, since we assume that this code won't get used after
startup. From what I saw (I checked with a binary generated against
glibc and musl), on AMD64 we have enough space there to fit two
displaced stepping buffers. A subsequent patch makes AMD64/Linux use
two buffers.
In addition to having multiple displaced stepping buffers, there is also
the idea of sharing displaced stepping buffers between threads. Two
threads doing displaced steps for the same PC could use the same buffer
at the same time. Two threads stepping over the same instruction (same
opcode) at two different PCs may also be able to share a displaced
stepping buffer. This is an idea for future patches, but the
architecture built by this patch is made to allow this.
Now, the implementation details. The main part of this patch is moving
the responsibility of preparing and finishing a displaced step to the
gdbarch. Before this patch, preparing a displaced step is driven by the
displaced_step_prepare_throw function. It does some calls to the
gdbarch to do some low-level operations, but the high-level logic is
there. The steps are roughly:
- Ask the gdbarch for the displaced step buffer location
- Save the existing bytes in the displaced step buffer
- Ask the gdbarch to copy the instruction into the displaced step buffer
- Set the pc of the thread to the beginning of the displaced step buffer
Similarly, the "fixup" phase, executed after the instruction was
successfully single-stepped, is driven by the infrun code (function
displaced_step_finish). The steps are roughly:
- Restore the original bytes in the displaced stepping buffer
- Ask the gdbarch to fixup the instruction result (adjust the target's
registers or memory to do as if the instruction had been executed in
its original location)
The displaced_step_inferior_state::step_thread field indicates which
thread (if any) is currently using the displaced stepping buffer, so it
is used by displaced_step_prepare_throw to check if the displaced
stepping buffer is free to use or not.
This patch defers the whole task of preparing and cleaning up after a
displaced step to the gdbarch. Two new main gdbarch methods are added,
with the following semantics:
- gdbarch_displaced_step_prepare: Prepare for the given thread to
execute a displaced step of the instruction located at its current PC.
Upon return, everything should be ready for GDB to resume the thread
(with either a single step or continue, as indicated by
gdbarch_displaced_step_hw_singlestep) to make it displaced step the
instruction.
- gdbarch_displaced_step_finish: Called when the thread stopped after
having started a displaced step. Verify if the instruction was
executed, if so apply any fixup required to compensate for the fact
that the instruction was executed at a different place than its
original pc. Release any resources that were allocated for this
displaced step. Upon return, everything should be ready for GDB to
resume the thread in its "normal" code path.
The displaced_step_prepare_throw function now pretty much just offloads
to gdbarch_displaced_step_prepare and the displaced_step_finish function
offloads to gdbarch_displaced_step_finish.
The gdbarch_displaced_step_location method is now unnecessary, so is
removed. Indeed, the core of GDB doesn't know how many displaced step
buffers there are nor where they are.
To keep the existing behavior for existing architectures, the logic that
was previously implemented in infrun.c for preparing and finishing a
displaced step is moved to displaced-stepping.c, to the
displaced_step_buffer class. Architectures are modified to implement
the new gdbarch methods using this class. The behavior is not expected
to change.
The other important change (which arises from the above) is that the
core of GDB no longer prevents concurrent displaced steps. Before this
patch, start_step_over walks the global step over chain and tries to
initiate a step over (whether it is in-line or displaced). It follows
these rules:
- if an in-line step is in progress (in any inferior), don't start any
other step over
- if a displaced step is in progress for an inferior, don't start
another displaced step for that inferior
After starting a displaced step for a given inferior, it won't start
another displaced step for that inferior.
In the new code, start_step_over simply tries to initiate step overs for
all the threads in the list. But because threads may be added back to
the global list as it iterates the global list, trying to initiate step
overs, start_step_over now starts by stealing the global queue into a
local queue and iterates on the local queue. In the typical case, each
thread will either:
- have initiated a displaced step and be resumed
- have been added back by the global step over queue by
displaced_step_prepare_throw, because the gdbarch will have returned
that there aren't enough resources (i.e. buffers) to initiate a
displaced step for that thread
Lastly, if start_step_over initiates an in-line step, it stops
iterating, and moves back whatever remaining threads it had in its local
step over queue to the global step over queue.
Two other gdbarch methods are added, to handle some slightly annoying
corner cases. They feel awkwardly specific to these cases, but I don't
see any way around them:
- gdbarch_displaced_step_copy_insn_closure_by_addr: in
arm_pc_is_thumb, arm-tdep.c wants to get the closure for a given
buffer address.
- gdbarch_displaced_step_restore_all_in_ptid: when a process forks
(at least on Linux), the address space is copied. If some displaced
step buffers were in use at the time of the fork, we need to restore
the original bytes in the child's address space.
These two adjustments are also made in infrun.c:
- prepare_for_detach: there may be multiple threads doing displaced
steps when we detach, so wait until all of them are done
- handle_inferior_event: when we handle a fork event for a given
thread, it's possible that other threads are doing a displaced step at
the same time. Make sure to restore the displaced step buffer
contents in the child for them.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb
gdb/ChangeLog:
* displaced-stepping.h (struct
displaced_step_copy_insn_closure): Adjust comments.
(struct displaced_step_inferior_state) <step_thread,
step_gdbarch, step_closure, step_original, step_copy,
step_saved_copy>: Remove fields.
(struct displaced_step_thread_state): New.
(struct displaced_step_buffer): New.
* displaced-stepping.c (displaced_step_buffer::prepare): New.
(write_memory_ptid): Move from infrun.c.
(displaced_step_instruction_executed_successfully): New,
factored out of displaced_step_finish.
(displaced_step_buffer::finish): New.
(displaced_step_buffer::copy_insn_closure_by_addr): New.
(displaced_step_buffer::restore_in_ptid): New.
* gdbarch.sh (displaced_step_location): Remove.
(displaced_step_prepare, displaced_step_finish,
displaced_step_copy_insn_closure_by_addr,
displaced_step_restore_all_in_ptid): New.
* gdbarch.c: Re-generate.
* gdbarch.h: Re-generate.
* gdbthread.h (class thread_info) <displaced_step_state>: New
field.
(thread_step_over_chain_remove): New declaration.
(thread_step_over_chain_next): New declaration.
(thread_step_over_chain_length): New declaration.
* thread.c (thread_step_over_chain_remove): Make non-static.
(thread_step_over_chain_next): New.
(global_thread_step_over_chain_next): Use
thread_step_over_chain_next.
(thread_step_over_chain_length): New.
(global_thread_step_over_chain_enqueue): Add debug print.
(global_thread_step_over_chain_remove): Add debug print.
* infrun.h (get_displaced_step_copy_insn_closure_by_addr):
Remove.
* infrun.c (get_displaced_stepping_state): New.
(displaced_step_in_progress_any_inferior): Remove.
(displaced_step_in_progress_thread): Adjust.
(displaced_step_in_progress): Adjust.
(displaced_step_in_progress_any_thread): New.
(get_displaced_step_copy_insn_closure_by_addr): Remove.
(gdbarch_supports_displaced_stepping): Use
gdbarch_displaced_step_prepare_p.
(displaced_step_reset): Change parameter from inferior to
thread.
(displaced_step_prepare_throw): Implement using
gdbarch_displaced_step_prepare.
(write_memory_ptid): Move to displaced-step.c.
(displaced_step_restore): Remove.
(displaced_step_finish): Implement using
gdbarch_displaced_step_finish.
(start_step_over): Allow starting more than one displaced step.
(prepare_for_detach): Handle possibly multiple threads doing
displaced steps.
(handle_inferior_event): Handle possibility that fork event
happens while another thread displaced steps.
* linux-tdep.h (linux_displaced_step_prepare): New.
(linux_displaced_step_finish): New.
(linux_displaced_step_copy_insn_closure_by_addr): New.
(linux_displaced_step_restore_all_in_ptid): New.
(linux_init_abi): Add supports_displaced_step parameter.
* linux-tdep.c (struct linux_info) <disp_step_buf>: New field.
(linux_displaced_step_prepare): New.
(linux_displaced_step_finish): New.
(linux_displaced_step_copy_insn_closure_by_addr): New.
(linux_displaced_step_restore_all_in_ptid): New.
(linux_init_abi): Add supports_displaced_step parameter,
register displaced step methods if true.
(_initialize_linux_tdep): Register inferior_execd observer.
* amd64-linux-tdep.c (amd64_linux_init_abi_common): Add
supports_displaced_step parameter, adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
(amd64_linux_init_abi): Adjust call to
amd64_linux_init_abi_common.
(amd64_x32_linux_init_abi): Likewise.
* aarch64-linux-tdep.c (aarch64_linux_init_abi): Adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
* arm-linux-tdep.c (arm_linux_init_abi): Likewise.
* i386-linux-tdep.c (i386_linux_init_abi): Likewise.
* alpha-linux-tdep.c (alpha_linux_init_abi): Adjust call to
linux_init_abi.
* arc-linux-tdep.c (arc_linux_init_osabi): Likewise.
* bfin-linux-tdep.c (bfin_linux_init_abi): Likewise.
* cris-linux-tdep.c (cris_linux_init_abi): Likewise.
* csky-linux-tdep.c (csky_linux_init_abi): Likewise.
* frv-linux-tdep.c (frv_linux_init_abi): Likewise.
* hppa-linux-tdep.c (hppa_linux_init_abi): Likewise.
* ia64-linux-tdep.c (ia64_linux_init_abi): Likewise.
* m32r-linux-tdep.c (m32r_linux_init_abi): Likewise.
* m68k-linux-tdep.c (m68k_linux_init_abi): Likewise.
* microblaze-linux-tdep.c (microblaze_linux_init_abi): Likewise.
* mips-linux-tdep.c (mips_linux_init_abi): Likewise.
* mn10300-linux-tdep.c (am33_linux_init_osabi): Likewise.
* nios2-linux-tdep.c (nios2_linux_init_abi): Likewise.
* or1k-linux-tdep.c (or1k_linux_init_abi): Likewise.
* riscv-linux-tdep.c (riscv_linux_init_abi): Likewise.
* s390-linux-tdep.c (s390_linux_init_abi_any): Likewise.
* sh-linux-tdep.c (sh_linux_init_abi): Likewise.
* sparc-linux-tdep.c (sparc32_linux_init_abi): Likewise.
* sparc64-linux-tdep.c (sparc64_linux_init_abi): Likewise.
* tic6x-linux-tdep.c (tic6x_uclinux_init_abi): Likewise.
* tilegx-linux-tdep.c (tilegx_linux_init_abi): Likewise.
* xtensa-linux-tdep.c (xtensa_linux_init_abi): Likewise.
* ppc-linux-tdep.c (ppc_linux_init_abi): Adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
* arm-tdep.c (arm_pc_is_thumb): Call
gdbarch_displaced_step_copy_insn_closure_by_addr instead of
get_displaced_step_copy_insn_closure_by_addr.
* rs6000-aix-tdep.c (rs6000_aix_init_osabi): Adjust calls to
clear gdbarch methods.
* rs6000-tdep.c (struct ppc_inferior_data): New structure.
(get_ppc_per_inferior): New function.
(ppc_displaced_step_prepare): New function.
(ppc_displaced_step_finish): New function.
(ppc_displaced_step_restore_all_in_ptid): New function.
(rs6000_gdbarch_init): Register new gdbarch methods.
* s390-tdep.c (s390_gdbarch_init): Don't call
set_gdbarch_displaced_step_location, set new gdbarch methods.
gdb/testsuite/ChangeLog:
* gdb.arch/amd64-disp-step-avx.exp: Adjust pattern.
* gdb.threads/forking-threads-plus-breakpoint.exp: Likewise.
* gdb.threads/non-stop-fair-events.exp: Likewise.
Change-Id: I387cd235a442d0620ec43608fd3dc0097fcbf8c8
|
|
Move displaced-stepping related stuff unchanged to displaced-stepping.h
and displaced-stepping.c. This helps make the following patch a bit
smaller and easier to read.
gdb/ChangeLog:
* Makefile.in (COMMON_SFILES): Add displaced-stepping.c.
* aarch64-tdep.h: Include displaced-stepping.h.
* displaced-stepping.h (struct displaced_step_copy_insn_closure):
Move here.
(displaced_step_copy_insn_closure_up): Move here.
(struct buf_displaced_step_copy_insn_closure): Move here.
(struct displaced_step_inferior_state): Move here.
(debug_displaced): Move here.
(displaced_debug_printf_1): Move here.
(displaced_debug_printf): Move here.
* displaced-stepping.c: New file.
* gdbarch.sh: Include displaced-stepping.h in gdbarch.h.
* gdbarch.h: Re-generate.
* inferior.h: Include displaced-stepping.h.
* infrun.h (debug_displaced): Move to displaced-stepping.h.
(displaced_debug_printf_1): Likewise.
(displaced_debug_printf): Likewise.
(struct displaced_step_copy_insn_closure): Likewise.
(displaced_step_copy_insn_closure_up): Likewise.
(struct buf_displaced_step_copy_insn_closure): Likewise.
(struct displaced_step_inferior_state): Likewise.
* infrun.c (show_debug_displaced): Move to displaced-stepping.c.
(displaced_debug_printf_1): Likewise.
(displaced_step_copy_insn_closure::~displaced_step_copy_insn_closure):
Likewise.
(_initialize_infrun): Don't register "set/show debug displaced".
Change-Id: I29935f5959b80425370630a45148fc06cd4227ca
|
|
Pass to get_linux_inferior_data the inferior for which we want to obtain
the linux-specific data, rather than assuming the current inferior.
This helps slightly reduce the diff in the upcoming main patch.
Update the sole caller to pass the current inferior.
gdb/ChangeLog:
* linux-tdep.c (get_linux_inferior_data): Add inferior
parameter.
(linux_vsyscall_range): Pass current inferior.
Change-Id: Ie4b61190e4a2e89b5b55a140cfecd4de66d92393
|
|
This is a preparatory patch to reduce the size of the diff of the
upcoming main patch. It introduces enum types for the return values of
displaced step "prepare" and "finish" operations. I find that this
expresses better the intention of the code, rather than returning
arbitrary integer values (-1, 0 and 1) which are difficult to remember.
That makes the code easier to read.
I put the new enum types in a new displaced-stepping.h file, because I
introduce that file in a later patch anyway. Putting it there avoids
having to move it later.
There is one change in behavior for displaced_step_finish: it currently
returns 0 if the thread wasn't doing a displaced step and 1 if the
thread was doing a displaced step which was executed successfully. It
turns out that this distinction is not needed by any caller, so I've
merged these two cases into "_OK", rather than adding an extra
enumerator.
gdb/ChangeLog:
* infrun.c (displaced_step_prepare_throw): Change return type to
displaced_step_prepare_status.
(displaced_step_prepare): Likewise.
(displaced_step_finish): Change return type to
displaced_step_finish_status.
(resume_1): Adjust.
(stop_all_threads): Adjust.
* displaced-stepping.h: New file.
Change-Id: I5c8fe07212cd398d5b486b5936d9d0807acd3788
|
