With the growing number of selftests, I think it would be useful to be
able to run only a subset of the tests. This patch associates a name to
each registered selftest. It then allows doing something like:
(gdb) maintenance selftest aarch64
Running self-tests.
Running selftest aarch64-analyze-prologue.
Running selftest aarch64-process-record.
Ran 2 unit tests, 0 failed
or with gdbserver:
./gdbserver --selftest=aarch64
In both cases, only the tests that contain "aarch64" in their name are
ran. To help validate that the tests you want to run were actually ran,
it also prints a message with the test name before running each test.
Right now, all the arch-dependent tests are registered as a single test
of the selftests. To be able to filter those too, I made them
"first-class citizen" selftests. The selftest type is an interface,
with different implementations for "simple selftests" and "arch
selftests". The run_tests function simply iterates on that an invokes
operator() on each test.
I changed the tests data structure from a vector to a map, because
- it allows iterating in a stable (alphabetical) order
- it allows to easily verify if a test with a given name has been
registered, to avoid duplicates
There's also a new command "maintenance info selftests" that lists the
registered selftests.
gdb/ChangeLog:
* common/selftest.h (selftest): New struct/interface.
(register_test): Add name parameter, add new overload.
(run_tests): Add filter parameter.
(for_each_selftest_ftype): New typedef.
(for_each_selftest): New declaration.
* common/selftest.c (tests): Change type to
map<string, unique_ptr<selftest>>.
(simple_selftest): New struct.
(register_test): New function.
(register_test): Add name parameter and use it.
(run_tests): Add filter parameter and use it. Add prints.
Adjust to vector -> map change.
* aarch64-tdep.c (_initialize_aarch64_tdep): Add names when
registering selftests.
* arm-tdep.c (_initialize_arm_tdep): Likewise.
* disasm-selftests.c (_initialize_disasm_selftests): Likewise.
* dwarf2-frame.c (_initialize_dwarf2_frame): Likewise.
* dwarf2loc.c (_initialize_dwarf2loc): Likewise.
* findvar.c (_initialize_findvar): Likewise.
* gdbarch-selftests.c (_initialize_gdbarch_selftests): Likewise.
* maint.c (maintenance_selftest): Update call to run_tests.
(maintenance_info_selftests): New function.
(_initialize_maint_cmds): Register "maintenance info selftests"
command. Update "maintenance selftest" doc.
* regcache.c (_initialize_regcache): Add names when registering
selftests.
* rust-exp.y (_initialize_rust_exp): Likewise.
* selftest-arch.c (gdbarch_selftest): New struct.
(gdbarch_tests): Remove.
(register_test_foreach_arch): Add name parameter. Call
register_test.
(tests_with_arch): Remove, move most content to
gdbarch_selftest::operator().
(_initialize_selftests_foreach_arch): Remove.
* selftest-arch.h (register_test_foreach_arch): Add name
parameter.
(run_tests_with_arch): New declaration.
* utils-selftests.c (_initialize_utils_selftests): Add names
when registering selftests.
* utils.c (_initialize_utils): Likewise.
* unittests/array-view-selftests.c
(_initialize_array_view_selftests): Likewise.
* unittests/environ-selftests.c (_initialize_environ_selftests):
Likewise.
* unittests/function-view-selftests.c
(_initialize_function_view_selftests): Likewise.
* unittests/offset-type-selftests.c
(_initialize_offset_type_selftests): Likewise.
* unittests/optional-selftests.c
(_initialize_optional_selftests): Likewise.
* unittests/scoped_restore-selftests.c
(_initialize_scoped_restore_selftests): Likewise.
* NEWS: Document "maintenance selftest" and "maint info
selftests".
gdb/gdbserver/ChangeLog:
* server.c (captured_main): Accept argument for --selftest.
Update run_tests call.
* linux-x86-tdesc-selftest.c (initialize_low_tdesc): Add names
when registering selftests.
gdb/doc/ChangeLog:
* gdb.texinfo (Maintenance Commands): Document filter parameter
of "maint selftest". Document "maint info selftests" command.
Replace int with bool, because that's what it is.
gdb/ChangeLog:
* dwarf2expr.h (dwarf_expr_piece) <v.mem.in_stack_memory>:
Change type to bool.
(dwarf_stack_value) <in_stack_memory>: Likewise.
(dwarf_expr_context) <push_address>: Change parameter type to
bool.
<fetch_in_stack_memory>: Change return type to bool.
<push>: Change parameter type to bool.
* dwarf2expr.c (dwarf_expr_context::push): Change parameter type
to bool.
(dwarf_expr_context::push_address): Likewise.
(dwarf_expr_context::fetch_in_stack_memory): Change return type
to bool.
(dwarf_expr_context::execute_stack_op): Adjust.
* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Adjust.
Change the manually managed array dwarf_expr_piece::piece with an
std::vector. After passing the pieces array to allocate_piece_closure,
dwarf2_evaluate_loc_desc_full doesn't need that data anymore. We can
therefore move the content of the vector to avoid copying it.
Reg-tested on the buildbot.
gdb/ChangeLog:
* dwarf2expr.h (struct dwarf_expr_piece): Move up.
(struct dwarf_expr_context) <n_pieces>: Remove.
<pieces>: Change type to std::vector.
* dwarf2expr.c (dwarf_expr_context::dwarf_expr_context): Adjust.
(dwarf_expr_context::~dwarf_expr_context): Don't manually free
pieces.
(dwarf_expr_context::add_piece): Adjust.
* dwarf2loc.c (struct piece_closure): Initialize fields.
<n_pieces>: Remove.
<pieces>: Change type to std::vector.
(allocate_piece_closure): Adjust, change parameter to
std::vector rvalue and std::move it to piece_closure.
(rw_pieced_value): Adjust.
(check_pieced_synthetic_pointer): Adjust.
(indirect_synthetic_pointer): Adjust.
(coerce_pieced_ref): Adjust.
(free_pieced_value_closure): Adjust. Use delete to free
piece_closure.
(dwarf2_evaluate_loc_desc_full): Adjust. std::move ctx.pieces
to allocate_piece_closure.
(dwarf2_loc_desc_get_symbol_read_needs): Adjust.
In multiple places, we pass the gdbarch as an argument to some
functions, even though it's available in the agent_expr structure also
passed to the same functions. Remove these arguments and replace their
usage with accesses to agent_expr::gdbarch.
gdb/ChangeLog:
* dwarf2loc.h (dwarf2_compile_expr_to_ax): Remove gdbarch
parameter.
* symtab.h (struct symbol_computed_ops::tracepoint_var_ref):
Likewise.
* dwarf2loc.c (dwarf2_compile_expr_to_ax): Remove gdbarch
parameter, use agent_expr::gdbarch instead, update function
calls.
(locexpr_tracepoint_var_ref): Likewise.
(loclist_tracepoint_var_ref): Likewise.
* ax-gdb.c (gen_trace_static_fields): Likewise.
(gen_traced_pop): Likewise.
(gen_frame_args_address): Likewise.
(gen_frame_locals_address): Likewise.
(gen_var_ref): Likewise.
(gen_struct_ref_recursive): Likewise.
(gen_static_field): Likewise.
(gen_maybe_namespace_elt): Likewise.
(gen_expr): Likewise.
(gen_trace_for_var): Likewise.
(gen_trace_for_expr): Likewise.
(gen_trace_for_return_address): Likewise.
This Rust bug report:
https://github.com/rust-lang/rust/issues/41970
noted an error from gdb. What is happening here (for me, the original
report had a different error) is that a pieced DWARF expression is not
writing to every byte in the resulting value. GDB errors in this
case. However, it seems to me that it is always valid to write fewer
bytes; the issue comes from writing too many -- that is, the test is
reversed. The test was also checking the sub-object, but this also
seems incorrect, as it's expected for the expression to write the
entirety of the enclosing object. So, this patch reverses the test
and applies it to the outer type, not the subobject type.
Regtested on the buildbot.
gdb/ChangeLog
2017-07-09 Tom Tromey <tom@tromey.com>
* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Reverse size
check and apply to outer type.
gdb/testsuite/ChangeLog
2017-07-09 Tom Tromey <tom@tromey.com>
* gdb.dwarf2/shortpiece.exp: New file.
This commit eliminates make_cleanup_obstack_free, replacing it with a
new auto_obstack type that inherits obstack to add cdtors.
These changes in the parsers may not be obvious:
- obstack_init (&name_obstack);
- make_cleanup_obstack_free (&name_obstack);
+ name_obstack.clear ();
Here, the 'name_obstack' variable is a global. The change means that
the obstack's contents from a previous parse will stay around until
the next parsing starts. I.e., memory won't be reclaimed until then.
I don't think that's a problem, these objects don't really grow much
at all.
The other option I tried was to add a separate type that is like
auto_obstack but manages an external obstack, just for those cases. I
like the current approach better as that other approach adds more
boilerplate and yet another type to learn.
gdb/ChangeLog:
2017-06-27 Pedro Alves <palves@redhat.com>
* c-exp.y (name_obstack): Now an auto_obstack.
(yylex): Use auto_obstack::clear.
(c_parse): Use auto_obstack::clear instead of reinitializing and
freeing the obstack.
* c-lang.c (evaluate_subexp_c): Use auto_obstack.
* d-exp.y (name_obstack): Now an auto_obstack.
(yylex): Use auto_obstack::clear.
(d_parse): Use auto_obstack::clear instead of reinitializing and
freeing the obstack.
* dwarf2loc.c (fetch_const_value_from_synthetic_pointer): Use
auto_obstack.
* dwarf2read.c (create_addrmap_from_index)
(dwarf2_build_psymtabs_hard)
(update_enumeration_type_from_children): Likewise.
* gdb_obstack.h (auto_obstack): New type.
* go-exp.y (name_obstack): Now an auto_obstack.
(build_packaged_name): Use auto_obstack::clear.
(go_parse): Use auto_obstack::clear instead of reinitializing and
freeing the obstack.
* linux-tdep.c (linux_make_mappings_corefile_notes): Use
auto_obstack.
* printcmd.c (printf_wide_c_string, ui_printf): Use auto_obstack.
* rust-exp.y (work_obstack): Now an auto_obstack.
(rust_parse, rust_lex_tests): Use auto_obstack::clear instead of
reinitializing and freeing the obstack.
* utils.c (do_obstack_free, make_cleanup_obstack_free): Delete.
(host_char_to_target): Use auto_obstack.
* utils.h (make_cleanup_obstack_free): Delete declaration.
* valprint.c (generic_emit_char, generic_printstr): Use
auto_obstack.
In some cases we've been replacing heap-allocated gdb_byte buffers
managed with xmalloc/make_cleanup(xfree) with gdb::vector<gdb_byte>.
That usually pessimizes the code a little bit because std::vector
value-initializes elements (which for gdb_byte means
zero-initialization), while if you're creating a temporary buffer,
you're most certaintly going to fill it in with some data. An
alternative is to use
unique_ptr<gdb_byte[]> buf (new gdb_byte[size]);
but it looks like that's not very popular.
Recently, a use of obstacks in dwarf2read.c was replaced with
std::vector<gdb_byte> and that as well introduced a pessimization for
always memsetting the buffer when it's garanteed that the zeros will
be overwritten immediately. (see dwarf2read.c change in this patch to
find it.)
So here's a different take at addressing this issue "by design":
#1 - Introduce default_init_allocator<T>
I.e., a custom allocator that does default construction using default
initialization, meaning, no more zero initialization. That's the
default_init_allocation<T> class added in this patch.
See "Notes" at
<http://en.cppreference.com/w/cpp/container/vector/resize>.
#2 - Introduce def_vector<T>
I.e., a convenience typedef, because typing the allocator is annoying:
using def_vector<T> = std::vector<T, gdb::default_init_allocator<T>>;
#3 - Introduce byte_vector
Because gdb_byte vectors will be the common thing, add a convenience
"byte_vector" typedef:
using byte_vector = def_vector<gdb_byte>;
which is really the same as:
std::vector<gdb_byte, gdb::default_init_allocator<gdb_byte>>;
The intent then is to make "gdb::byte_vector" be the go-to for dynamic
byte buffers. So the less friction, the better.
#4 - Adjust current code to use it.
To set the example going forward. Replace std::vector uses and also
unique_ptr<byte[]> uses.
One nice thing is that with this allocator, for changes like these:
-std::unique_ptr<byte[]> buf (new gdb_byte[some_size]);
+gdb::byte_vector buf (some_size);
fill_with_data (buf.data (), buf.size ());
the generated code is the same as before. I.e., the compiler
de-structures the vector and gets rid of the unused "reserved vs size"
related fields.
The other nice thing is that it's easier to write
gdb::byte_vector buf (size);
than
std::unique_ptr<gdb_byte[]> buf (new gdb_byte[size]);
or even (C++14):
auto buf = std::make_unique<gdb_byte[]> (size); // zero-initializes...
#5 - Suggest s/std::vector<gdb_byte>/gdb::byte_vector/ going forward.
Note that this commit actually fixes a couple of bugs where the current
code is incorrectly using "std::vector::reserve(new_size)" and then
accessing the vector's internal buffer beyond the vector's size: see
dwarf2loc.c and charset.c. That's undefined behavior and may trigger
debug mode assertion failures. With default_init_allocator,
"resize()" behaves like "reserve()" performance wise, in that it
leaves new elements with unspecified values, but, it does that safely
without triggering undefined behavior when you access those values.
gdb/ChangeLog:
2017-06-14 Pedro Alves <palves@redhat.com>
* ada-lang.c: Include "common/byte-vector.h".
(ada_value_primitive_packed_val): Use gdb::byte_vector.
* charset.c (wchar_iterator::iterate): Resize the vector instead
of reserving it.
* common/byte-vector.h: Include "common/def-vector.h".
(wchar_iterator::m_out): Now a gdb::def_vector<gdb_wchar_t>.
* cli/cli-dump.c: Include "common/byte-vector.h".
(dump_memory_to_file, restore_binary_file): Use gdb::byte_vector.
* common/byte-vector.h: New file.
* common/def-vector.h: New file.
* common/default-init-alloc.h: New file.
* dwarf2loc.c: Include "common/byte-vector.h".
(rw_pieced_value): Use gdb::byte_vector, and resize the vector
instead of reserving it.
* dwarf2read.c: Include "common/byte-vector.h".
(data_buf::m_vec): Now a gdb::byte_vector.
* gdb_regex.c: Include "common/def-vector.h".
(compiled_regex::compiled_regex): Use gdb::def_vector<char>.
* mi/mi-main.c: Include "common/byte-vector.h".
(mi_cmd_data_read_memory): Use gdb::byte_vector.
* printcmd.c: Include "common/byte-vector.h".
(print_scalar_formatted): Use gdb::byte_vector.
* valprint.c: Include "common/byte-vector.h".
(maybe_negate_by_bytes, print_decimal_chars): Use
gdb::byte_vector.
Since read_pieced_value and write_pieced_value share significant logic,
this patch merges them into a single function rw_pieced_value.
gdb/ChangeLog:
* dwarf2loc.c (rw_pieced_value): New. Merge logic from...
(read_pieced_value, write_pieced_value): ...here. Reduce to
wrappers that just call rw_pieced_value.
So far write_pieced_value uses write_memory when writing memory pieces to
the target. However, this is a case where GDB potentially overwrites a
watchpoint value. In such a case write_memory_with_notification should be
used instead, so that memory_changed observers get notified.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): When writing the data for a
memory piece, use write_memory_with_notification instead of
write_memory.
In read_pieced_value's main loop, the variables `dest_offset_bits' and
`source_offset_bits' are basically just copies of `offset' and
`bits_to_skip', respectively. In write_pieced_value the copies are
reversed. This is not very helpful when trying to keep the logic between
these functions in sync. Since the copies are unnecessary, this patch
just removes them.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Remove unnecessary variables
dest_offset_bits and source_offset_bits.
(write_pieced_value): Likewise.
So far GDB ignores the piece offset of all kinds of DWARF bit
pieces (DW_OP_bit_piece) and treats such pieces as if the offset was zero.
This is fixed, and an appropriate test is added.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Respect the piece offset, as
given by DW_OP_bit_piece.
(write_pieced_value): Likewise.
Andreas Arnez <arnez@linux.vnet.ibm.com>
* gdb.dwarf2/var-access.exp: Add test for composite location with
nonzero piece offsets.
So far the main loop in read_pieced_value and write_pieced_value is
structured like this:
(1) Prepare a buffer and some variables we may need;
(2) depending on the DWARF piece type to be handled, use the buffer and
the prepared variables, ignore them, or even recalculate them.
This approach reduces readability and may also lead to unnecessary copying
of data. This patch moves the preparations to the places where sufficient
information is available and removes some of the variables involved.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Move the buffer allocation and
some other preparations to the places where sufficient information
is available.
(write_pieced_value): Likewise.
For big-endian targets the logic in read/write_pieced_value tries to take
a register piece from the LSB end. This requires offsets and sizes to be
adjusted accordingly, and that's where the current implementation has some
issues:
* The formulas for recalculating the bit- and byte-offsets into the
register are wrong. They just happen to yield correct results if
everything is byte-aligned and the piece's last byte belongs to the
given value.
* After recalculating the bit offset into the register, the number of
bytes to be copied from the register is not recalculated. Of course
this does not matter if everything (particularly the piece size) is
byte-aligned.
These issues are fixed. The size calculation is performed with a new
helper function bits_to_bytes().
gdb/ChangeLog:
* dwarf2loc.c (bits_to_bytes): New function.
(read_pieced_value): Fix offset calculations for register pieces
on big-endian targets.
(write_pieced_value): Likewise.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/var-access.exp: Add test for non-byte-aligned
register pieces.
When the variable 'buffer_size' in read_pieced_value and
write_pieced_value was introduced, it was needed for tracking the buffer's
allocated size. Now that the buffer's data type has been changed to a
std::vector, the variable is no longer necessary; so remove it.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Remove buffer_size variable.
(write_pieced_value): Likewise.
On big-endian targets, when targeting a bit-field, write_pieced_value
currently transfers the source value's *most* significant bits to the
target value, instead of its least significant bits. This is fixed.
In particular the fix adjusts the initial value of 'offset', which can now
potentially be nonzero. Thus the variable 'type_len' is renamed to
'max_offset', to avoid confusion. And for consistency, the affected logic
that was mirrored in read_pieced_value is changed there in the same way.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): When writing to a bit-field,
transfer the source value's least significant bits, instead of its
lowest-addressed ones. Rename type_len to max_offset.
(read_pieced_value): Mirror above changes to write_pieced_value as
applicable.
In write_pieced_value, when transferring the data to target memory via a
buffer, the bit offset within the target value is not reduced to its
sub-byte fraction before using it as a bit offset into the buffer. This
is fixed.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): In DWARF_VALUE_MEMORY,
truncate full bytes from dest_offset_bits before using it as an
offset into the buffer.
In write_pieced_value, when checking whether the data can be transferred
byte-wise, the current logic verifies the source- and destination offsets
to be byte-aligned, but not the transfer size. This is fixed.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): Include transfer size in
byte-wise check.
In write_pieced_value, the number of bytes containing a portion of the
bit-field in a given piece is calculated with the wrong starting offset;
thus the result may be off by one. This bug was probably introduced when
copying this logic from read_pieced_value. Fix it.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): Fix copy/paste error in the
calculation of this_size.
In the case of targeting a bit-field, read_pieced_value and
write_pieced_value calculate the number of bits preceding the bit-field
without considering the relative offset of the value's parent. This is
relevant for a structure variable like this:
struct s {
uint64_t foo;
struct {
uint32_t bar;
uint32_t bf : 10; /* <-- target bit-field */
} baz;
} s;
In this scenario, if 'val' is a GDB value representing s.baz.bf,
val->parent represents the whole s.baz structure, and the following holds:
- value_offset (val) == sizeof s.baz.bar == 4
- value_offset (val->parent) == sizeof s.foo == 8
The current logic would only use value_offset(val), resulting in the wrong
offset into the target value. This is fixed.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Respect parent value's offset
when targeting a bit-field.
(write_pieced_value): Likewise.
The addr_size field in the piece_closure data structure is a relic from
before introducing the typed DWARF stack. It is obsolete now. This patch
removes it.
gdb/ChangeLog:
* dwarf2loc.c (struct piece_closure) <addr_size>: Remove field.
(allocate_piece_closure): Drop addr_size parameter.
(dwarf2_evaluate_loc_desc_full): Adjust call to
allocate_piece_closure.
When taking a DW_OP_piece or DW_OP_bit_piece from a DW_OP_stack_value, the
existing logic always takes the piece from the lowest-addressed end, which
is wrong on big-endian targets. The DWARF standard states that the
"DW_OP_bit_piece operation describes a sequence of bits using the least
significant bits of that value", and this also matches the current logic
in GCC. For instance, the GCC guality test case pr54970.c fails on s390x
because of this.
This fix adjusts the piece accordingly on big-endian targets. It is
assumed that:
* DW_OP_piece shall take the piece from the LSB end as well;
* pieces reaching outside the stack value bits are considered undefined,
and a zero value can be used instead.
gdb/ChangeLog:
PR gdb/21226
* dwarf2loc.c (read_pieced_value): Anchor stack value pieces at
the LSB end, independent of endianness.
gdb/testsuite/ChangeLog:
PR gdb/21226
* gdb.dwarf2/nonvar-access.exp: Add checks for verifying that
stack value pieces are taken from the LSB end.
A field f in a structure composed of DWARF pieces may be located in
multiple pieces, where the first and last of those may contain bits from
other fields as well. So when writing to f, the beginning of the first
and the end of the last of those pieces may have to be skipped. But the
logic in write_pieced_value for handling one of those pieces is flawed
when the first and last piece are the same, i.e., f is contained in a
single piece:
< - - - - - - - - - piece_size - - - - - - - - - ->
+-------------------------------------------------+
| skipped_bits | f_bits | / / / / / / / / / / |
+-------------------------------------------------+
The current logic determines the size of the sub-piece to operate on by
limiting the piece size to the bit size of f and then subtracting the
skipped bits:
min (piece_size, f_bits) - skipped_bits
Instead of:
min (piece_size - skipped_bits, f_bits)
So the resulting sub-piece size is corrupted, leading to wrong handling of
this piece in write_pieced_value.
Note that the same bug was already found in read_pieced_value and fixed
there (but not in write_pieced_value), see PR 15391.
This patch swaps the calculations, bringing them into the same (correct)
order as in read_pieced_value.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): Fix order of calculations for
size capping.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/var-pieces.exp: Add test case for modifying a
variable at nonzero offset.
A while ago, back when GDB was a C program, the sect_offset and
cu_offset types were made structs in order to prevent incorrect mixing
of those offsets. Now that we require C++11, we can make them
integers again, while keeping the safety, by exploiting "enum class".
We can add a bit more safety, even, by defining operators that the
types _should_ support, helping making the suspicious uses stand out
more.
Getting at the underlying type is done with the new to_underlying
function added by the previous patch, which also helps better spot
where do we need to step out of the safety net. Mostly, that's around
parsing the DWARF, and when we print the offset for complaint/debug
purposes. But there are other occasional uses.
Since we have to define the sect_offset/cu_offset types in a header
anyway, I went ahead and generalized/library-fied the idea of "offset"
types, making it trivial to add more such types if we find a use. See
common/offset-type.h and the DEFINE_OFFSET_TYPE macro.
I needed a couple generaly-useful preprocessor bits (e.g., yet another
CONCAT implementation), so I started a new common/preprocessor.h file.
I included units tests covering the "offset" types API. These are
mostly compile-time tests, using SFINAE to check that expressions that
shouldn't compile (e.g., comparing unrelated offset types) really are
invalid and would fail to compile. This same idea appeared in my
pending enum-flags revamp from a few months ago (though this version
is a bit further modernized compared to what I had posted), and I plan
on reusing the "check valid expression" bits added here in that
series, so I went ahead and defined the CHECK_VALID_EXPR macro in its
own header -- common/valid-expr.h. I think that's nicer regardless.
I was borderline between calling the new types "offset" types, or
"index" types, BTW. I stuck with "offset" simply because that's what
we're already calling them, mostly.
gdb/ChangeLog:
2017-04-04 Pedro Alves <palves@redhat.com>
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/offset-type-selftests.c.
(SUBDIR_UNITTESTS_OBS): Add offset-type-selftests.o.
* common/offset-type.h: New file.
* common/preprocessor.h: New file.
* common/traits.h: New file.
* common/valid-expr.h: New file.
* dwarf2expr.c: Include "common/underlying.h". Adjust to use
sect_offset and cu_offset strong typedefs throughout.
* dwarf2expr.h: Adjust to use sect_offset and cu_offset strong
typedefs throughout.
* dwarf2loc.c: Include "common/underlying.h". Adjust to use
sect_offset and cu_offset strong typedefs throughout.
* dwarf2read.c: Adjust to use sect_offset and cu_offset strong
typedefs throughout.
* gdbtypes.h: Include "common/offset-type.h".
(cu_offset): Now an offset type (strong typedef) instead of a
struct.
(sect_offset): Likewise.
(union call_site_parameter_u): Rename "param_offset" field to
"param_cu_off".
* unittests/offset-type-selftests.c: New file.
Recently I fixed a bug that caused a DW_OP_implicit_pointer with non-zero
offset into a DW_OP_implicit_value to be handled incorrectly on big-endian
targets. GDB ignored the offset and copied the wrong bytes:
https://sourceware.org/ml/gdb-patches/2017-01/msg00251.html
But there is still a similar issue when a DW_OP_implicit_pointer points
into a DW_OP_stack_value instead; and again, the offset is ignored. There
is an important difference, though: While implicit values are treated like
blocks of data and anchored at the lowest-addressed byte, stack values
traditionally contain integer numbers and are anchored at the *least
significant* byte. Also, stack values do not come in varying sizes, but
are cut down appropriately when used. Thus, on big-endian targets the
scenario looks like this (higher addresses shown right):
|<- - - - - Stack value - - - - - - ->|
| |
|<- original object ->|
|
| offset ->|####|
^^^^
de-referenced
implicit pointer
(Note how the original object's size influences the position of the
de-referenced implicit pointer within the stack value. This is not the
case for little-endian targets, where the original object starts at offset
zero within the stack value.)
This patch implements the logic indicated in the above diagram and adds an
appropriate test case. A new function dwarf2_fetch_die_type_sect_off is
added; it is used for retrieving the original object's type, so its size
can be determined. That type is passed to dwarf2_evaluate_loc_desc_full
via a new parameter.
gdb/ChangeLog:
* dwarf2loc.c (indirect_synthetic_pointer): Get data type of
pointed-to DIE and pass it to dwarf2_evaluate_loc_desc_full.
(dwarf2_evaluate_loc_desc_full): New parameter subobj_type; rename
byte_offset to subobj_byte_offset. Fix the handling of
DWARF_VALUE_STACK on big-endian targets when coming via an
implicit pointer.
(dwarf2_evaluate_loc_desc): Adjust call to
dwarf2_evaluate_loc_desc_full.
* dwarf2loc.h (dwarf2_fetch_die_type_sect_off): New declaration.
* dwarf2read.c (dwarf2_fetch_die_type_sect_off): New function.
gdb/testsuite/ChangeLog:
* lib/dwarf.exp: Add support for DW_OP_implicit_pointer.
* gdb.dwarf2/nonvar-access.exp: Add test for stack value location
and implicit pointer into such a location.
This patch starts from the desire to eliminate
make_cleanup_ui_file_delete, but then goes beyond. It makes ui_file &
friends a real C++ class hierarchy, and switches temporary
ui_file-like objects to stack-based allocation.
- mem_fileopen -> string_file
mem_fileopen is replaced with a new string_file class that is treated
as a value class created on the stack. This alone eliminates most
make_cleanup_ui_file_delete calls, and, simplifies code a whole lot
(diffstat shows around 1k loc dropped.)
string_file's internal buffer is a std::string, thus the "string" in
the name. This simplifies the implementation much, compared to
mem_fileopen, which managed growing its internal buffer manually.
- ui_file_as_string, ui_file_strdup, ui_file_obsavestring all gone
The new string_file class has a string() method that provides direct
writable access to the internal std::string buffer. This replaced
ui_file_as_string, which forced a copy of the same data the stream had
inside. With direct access via a writable reference, we can instead
move the string out of the string_stream, avoiding deep string
copying.
Related, ui_file_xstrdup calls are replaced with xstrdup'ping the
stream's string, and ui_file_obsavestring is replaced by
obstack_copy0.
With all those out of the way, getting rid of the weird ui_file_put
mechanism was possible.
- New ui_file::printf, ui_file::puts, etc. methods
These simplify / clarify client code. I considered splitting
client-code changes, like these, e.g.:
- stb = mem_fileopen ();
- fprintf_unfiltered (stb, "%s%s%s",
- _("The valid values are:\n"),
- regdesc,
- _("The default is \"std\"."));
+ string_file stb;
+ stb.printf ("%s%s%s",
+ _("The valid values are:\n"),
+ regdesc,
+ _("The default is \"std\"."));
In two steps, with the first step leaving fprintf_unfiltered (etc.)
calls in place, and only afterwards do a pass to change all those to
call stb.printf etc.. I didn't do that split, because (when I tried),
it turned out to be pointless make-work: the first pass would have to
touch the fprintf_unfiltered line anyway, to replace "stb" with
"&stb".
- gdb_fopen replaced with stack-based objects
This avoids the need for cleanups or unique_ptr's. I.e., this:
struct ui_file *file = gdb_fopen (filename, "w");
if (filename == NULL)
perror_with_name (filename);
cleanups = make_cleanup_ui_file_delete (file);
// use file.
do_cleanups (cleanups);
is replaced with this:
stdio_file file;
if (!file.open (filename, "w"))
perror_with_name (filename);
// use file.
- odd contorsions in null_file_write / null_file_fputs around when to
call to_fputs / to_write eliminated.
- Global null_stream object
A few places that were allocating a ui_file in order to print to
"nowhere" are adjusted to instead refer to a new 'null_stream' global
stream.
- TUI's tui_sfileopen eliminated. TUI's ui_file much simplified
The TUI's ui_file was serving a dual purpose. It supported being used
as string buffer, and supported being backed by a stdio FILE. The
string buffer part is gone, replaced by using of string_file. The
'FILE *' support is now much simplified, by making the TUI's ui_file
inherit from stdio_file.
gdb/ChangeLog:
2017-02-02 Pedro Alves <palves@redhat.com>
* ada-lang.c (type_as_string): Use string_file.
* ada-valprint.c (ada_print_floating): Use string_file.
* ada-varobj.c (ada_varobj_scalar_image)
(ada_varobj_get_value_image): Use string_file.
* aix-thread.c (aix_thread_extra_thread_info): Use string_file.
* arm-tdep.c (_initialize_arm_tdep): Use string_printf.
* breakpoint.c (update_inserted_breakpoint_locations)
(insert_breakpoint_locations, reattach_breakpoints)
(print_breakpoint_location, print_one_detail_ranged_breakpoint)
(print_it_watchpoint): Use string_file.
(save_breakpoints): Use stdio_file.
* c-exp.y (oper): Use string_file.
* cli/cli-logging.c (set_logging_redirect): Use ui_file_up and
tee_file.
(pop_output_files): Use delete.
(handle_redirections): Use stdio_file and tee_file.
* cli/cli-setshow.c (do_show_command): Use string_file.
* compile/compile-c-support.c (c_compute_program): Use
string_file.
* compile/compile-c-symbols.c (generate_vla_size): Take a
'string_file &' instead of a 'ui_file *'.
(generate_c_for_for_one_variable): Take a 'string_file &' instead
of a 'ui_file *'. Use string_file.
(generate_c_for_variable_locations): Take a 'string_file &'
instead of a 'ui_file *'.
* compile/compile-internal.h (generate_c_for_for_one_variable):
Take a 'string_file &' instead of a 'ui_file *'.
* compile/compile-loc2c.c (push, pushf, unary, binary)
(print_label, pushf_register_address, pushf_register)
(do_compile_dwarf_expr_to_c): Take a 'string_file &' instead of a
'ui_file *'. Adjust.
* compile/compile.c (compile_to_object): Use string_file.
* compile/compile.h (compile_dwarf_expr_to_c)
(compile_dwarf_bounds_to_c): Take a 'string_file &' instead of a
'ui_file *'.
* cp-support.c (inspect_type): Use string_file and obstack_copy0.
(replace_typedefs_qualified_name): Use string_file and
obstack_copy0.
* disasm.c (gdb_pretty_print_insn): Use string_file.
(gdb_disassembly): Adjust reference the null_stream global.
(do_ui_file_delete): Delete.
(gdb_insn_length): Use null_stream.
* dummy-frame.c (maintenance_print_dummy_frames): Use stdio_file.
* dwarf2loc.c (dwarf2_compile_property_to_c)
(locexpr_generate_c_location, loclist_generate_c_location): Take a
'string_file &' instead of a 'ui_file *'.
* dwarf2loc.h (dwarf2_compile_property_to_c): Likewise.
* dwarf2read.c (do_ui_file_peek_last): Delete.
(dwarf2_compute_name): Use string_file.
* event-top.c (gdb_setup_readline): Use stdio_file.
* gdbarch.sh (verify_gdbarch): Use string_file.
* gdbtypes.c (safe_parse_type): Use null_stream.
* guile/scm-breakpoint.c (gdbscm_breakpoint_commands): Use
string_file.
* guile/scm-disasm.c (gdbscm_print_insn_from_port): Take a
'string_file *' instead of a 'ui_file *'.
(gdbscm_arch_disassemble): Use string_file.
* guile/scm-frame.c (frscm_print_frame_smob): Use string_file.
* guile/scm-ports.c (class ioscm_file_port): Now a class that
inherits from ui_file.
(ioscm_file_port_delete, ioscm_file_port_rewind)
(ioscm_file_port_put): Delete.
(ioscm_file_port_write): Rename to ...
(ioscm_file_port::write): ... this. Remove file_port_magic
checks.
(ioscm_file_port_new): Delete.
(ioscm_with_output_to_port_worker): Use ioscm_file_port and
ui_file_up.
* guile/scm-type.c (tyscm_type_name): Use string_file.
* guile/scm-value.c (vlscm_print_value_smob, gdbscm_value_print):
Use string_file.
* infcmd.c (print_return_value_1): Use string_file.
* infrun.c (print_target_wait_results): Use string_file.
* language.c (add_language): Use string_file.
* location.c (explicit_to_string_internal): Use string_file.
* main.c (captured_main_1): Use null_file.
* maint.c (maintenance_print_architecture): Use stdio_file.
* mi/mi-cmd-stack.c (list_arg_or_local): Use string_file.
* mi/mi-common.h (struct mi_interp) <out, err, log, targ,
event_channel>: Change type to mi_console_file pointer.
* mi/mi-console.c (mi_console_file_fputs, mi_console_file_flush)
(mi_console_file_delete): Delete.
(struct mi_console_file): Delete.
(mi_console_file_magic): Delete.
(mi_console_file_new): Delete.
(mi_console_file::mi_console_file): New.
(mi_console_file_delete): Delete.
(mi_console_file_fputs): Delete.
(mi_console_file::write): New.
(mi_console_raw_packet): Delete.
(mi_console_file::flush): New.
(mi_console_file_flush): Delete.
(mi_console_set_raw): Rename to ...
(mi_console_file::set_raw): ... this.
* mi/mi-console.h (class mi_console_file): New class.
(mi_console_file_new, mi_console_set_raw): Delete.
* mi/mi-interp.c (mi_interpreter_init): Use mi_console_file.
(mi_set_logging): Use delete and tee_file. Adjust.
* mi/mi-main.c (output_register): Use string_file.
(mi_cmd_data_evaluate_expression): Use string_file.
(mi_cmd_data_read_memory): Use string_file.
(mi_cmd_execute, print_variable_or_computed): Use string_file.
* mi/mi-out.c (mi_ui_out::main_stream): New.
(mi_ui_out::rewind): Use main_stream and
string_file.
(mi_ui_out::put): Use main_stream and string_file.
(mi_ui_out::mi_ui_out): Remove 'stream' parameter.
Allocate a 'string_file' instead.
(mi_out_new): Don't allocate a mem_fileopen stream here.
* mi/mi-out.h (mi_ui_out::mi_ui_out): Remove 'stream' parameter.
(mi_ui_out::main_stream): Declare method.
* printcmd.c (eval_command): Use string_file.
* psymtab.c (maintenance_print_psymbols): Use stdio_file.
* python/py-arch.c (archpy_disassemble): Use string_file.
* python/py-breakpoint.c (bppy_get_commands): Use string_file.
* python/py-frame.c (frapy_str): Use string_file.
* python/py-framefilter.c (py_print_type, py_print_single_arg):
Use string_file.
* python/py-type.c (typy_str): Use string_file.
* python/py-unwind.c (unwind_infopy_str): Use string_file.
* python/py-value.c (valpy_str): Use string_file.
* record-btrace.c (btrace_insn_history): Use string_file.
* regcache.c (regcache_print): Use stdio_file.
* reggroups.c (maintenance_print_reggroups): Use stdio_file.
* remote.c (escape_buffer): Use string_file.
* rust-lang.c (rust_get_disr_info): Use string_file.
* serial.c (serial_open_ops_1): Use stdio_file.
(do_serial_close): Use delete.
* stack.c (print_frame_arg): Use string_file.
(print_frame_args): Remove local mem_fileopen stream, not used.
(print_frame): Use string_file.
* symmisc.c (maintenance_print_symbols): Use stdio_file.
* symtab.h (struct symbol_computed_ops) <generate_c_location>:
Take a 'string_file *' instead of a 'ui_file *'.
* top.c (new_ui): Use stdio_file and stderr_file.
(free_ui): Use delete.
(execute_command_to_string): Use string_file.
(quit_confirm): Use string_file.
* tracepoint.c (collection_list::append_exp): Use string_file.
* tui/tui-disasm.c (tui_disassemble): Use string_file.
* tui/tui-file.c: Don't include "ui-file.h".
(enum streamtype, struct tui_stream): Delete.
(tui_file_new, tui_file_delete, tui_fileopen, tui_sfileopen)
(tui_file_isatty, tui_file_rewind, tui_file_put): Delete.
(tui_file::tui_file): New method.
(tui_file_fputs): Delete.
(tui_file_get_strbuf): Delete.
(tui_file::puts): New method.
(tui_file_adjust_strbuf): Delete.
(tui_file_flush): Delete.
(tui_file::flush): New method.
* tui/tui-file.h: Tweak intro comment.
Include ui-file.h.
(tui_fileopen, tui_sfileopen, tui_file_get_strbuf)
(tui_file_adjust_strbuf): Delete declarations.
(class tui_file): New class.
* tui/tui-io.c (tui_initialize_io): Use tui_file.
* tui/tui-regs.c (tui_restore_gdbout): Use delete.
(tui_register_format): Use string_stream.
* tui/tui-stack.c (tui_make_status_line): Use string_file.
(tui_get_function_from_frame): Use string_file.
* typeprint.c (type_to_string): Use string_file.
* ui-file.c (struct ui_file, ui_file_magic, ui_file_new): Delete.
(null_stream): New global.
(ui_file_delete): Delete.
(ui_file::ui_file): New.
(null_file_isatty): Delete.
(ui_file::~ui_file): New.
(null_file_rewind): Delete.
(ui_file::printf): New.
(null_file_put): Delete.
(null_file_flush): Delete.
(ui_file::putstr): New.
(null_file_write): Delete.
(ui_file::putstrn): New.
(null_file_read): Delete.
(ui_file::putc): New.
(null_file_fputs): Delete.
(null_file_write_async_safe): Delete.
(ui_file::vprintf): New.
(null_file_delete): Delete.
(null_file::write): New.
(null_file_fseek): Delete.
(null_file::puts): New.
(ui_file_data): Delete.
(null_file::write_async_safe): New.
(gdb_flush, ui_file_isatty): Adjust.
(ui_file_put, ui_file_rewind): Delete.
(ui_file_write): Adjust.
(ui_file_write_for_put): Delete.
(ui_file_write_async_safe, ui_file_read): Adjust.
(ui_file_fseek): Delete.
(fputs_unfiltered): Adjust.
(set_ui_file_flush, set_ui_file_isatty, set_ui_file_rewind)
(set_ui_file_put, set_ui_file_write, set_ui_file_write_async_safe)
(set_ui_file_read, set_ui_file_fputs, set_ui_file_fseek)
(set_ui_file_data): Delete.
(string_file::~string_file, string_file::write)
(struct accumulated_ui_file, do_ui_file_xstrdup, ui_file_xstrdup)
(do_ui_file_as_string, ui_file_as_string): Delete.
(do_ui_file_obsavestring, ui_file_obsavestring): Delete.
(struct mem_file): Delete.
(mem_file_new): Delete.
(stdio_file::stdio_file): New.
(mem_file_delete): Delete.
(stdio_file::stdio_file): New.
(mem_fileopen): Delete.
(stdio_file::~stdio_file): New.
(mem_file_rewind): Delete.
(stdio_file::set_stream): New.
(mem_file_put): Delete.
(stdio_file::open): New.
(mem_file_write): Delete.
(stdio_file_magic, struct stdio_file): Delete.
(stdio_file_new, stdio_file_delete, stdio_file_flush): Delete.
(stdio_file::flush): New.
(stdio_file_read): Rename to ...
(stdio_file::read): ... this. Adjust.
(stdio_file_write): Rename to ...
(stdio_file::write): ... this. Adjust.
(stdio_file_write_async_safe): Rename to ...
(stdio_file::write_async_safe) ... this. Adjust.
(stdio_file_fputs): Rename to ...
(stdio_file::puts) ... this. Adjust.
(stdio_file_isatty): Delete.
(stdio_file_fseek): Delete.
(stdio_file::isatty): New.
(stderr_file_write): Rename to ...
(stderr_file::write) ... this. Adjust.
(stderr_file_fputs): Rename to ...
(stderr_file::puts) ... this. Adjust.
(stderr_fileopen, stdio_fileopen, gdb_fopen): Delete.
(stderr_file::stderr_file): New.
(tee_file_magic): Delete.
(struct tee_file): Delete.
(tee_file::tee_file): New.
(tee_file_new): Delete.
(tee_file::~tee_file): New.
(tee_file_delete): Delete.
(tee_file_flush): Rename to ...
(tee_file::flush): ... this. Adjust.
(tee_file_write): Rename to ...
(tee_file::write): ... this. Adjust.
(tee_file::write_async_safe): New.
(tee_file_fputs): Rename to ...
(tee_file::puts): ... this. Adjust.
(tee_file_isatty): Rename to ...
(tee_file::isatty): ... this. Adjust.
* ui-file.h (struct obstack, struct ui_file): Don't
forward-declare.
(ui_file_new, ui_file_flush_ftype, set_ui_file_flush)
(ui_file_write_ftype)
(set_ui_file_write, ui_file_fputs_ftype, set_ui_file_fputs)
(ui_file_write_async_safe_ftype, set_ui_file_write_async_safe)
(ui_file_read_ftype, set_ui_file_read, ui_file_isatty_ftype)
(set_ui_file_isatty, ui_file_rewind_ftype, set_ui_file_rewind)
(ui_file_put_method_ftype, ui_file_put_ftype, set_ui_file_put)
(ui_file_delete_ftype, set_ui_file_data, ui_file_fseek_ftype)
(set_ui_file_fseek): Delete.
(ui_file_data, ui_file_delete, ui_file_rewind)
(struct ui_file): New.
(ui_file_up): New.
(class null_file): New.
(null_stream): Declare.
(ui_file_write_for_put, ui_file_put): Delete.
(ui_file_xstrdup, ui_file_as_string, ui_file_obsavestring):
Delete.
(ui_file_fseek, mem_fileopen, stdio_fileopen, stderr_fileopen)
(gdb_fopen, tee_file_new): Delete.
(struct string_file): New.
(struct stdio_file): New.
(stdio_file_up): New.
(struct stderr_file): New.
(class tee_file): New.
* ui-out.c (ui_out::field_stream): Take a 'string_file &' instead
of a 'ui_file *'. Adjust.
* ui-out.h (class ui_out) <field_stream>: Likewise.
* utils.c (do_ui_file_delete, make_cleanup_ui_file_delete)
(null_stream): Delete.
(error_stream): Take a 'string_file &' instead of a 'ui_file *'.
Adjust.
* utils.h (struct ui_file): Delete forward declaration..
(make_cleanup_ui_file_delete, null_stream): Delete declarations.
(error_stream): Take a 'string_file &' instead of a
'ui_file *'.
* varobj.c (varobj_value_get_print_value): Use string_file.
* xtensa-tdep.c (xtensa_verify_config): Use string_file.
* gdbarch.c: Regenerate.
When a variable's location is expressed as DW_OP_implicit_value, but the
given value is longer than needed, which bytes should be used? GDB's
current logic was introduced with a patch from 2011 and uses the "least
significant" bytes:
https://sourceware.org/ml/gdb-patches/2011-08/msg00123.html
Now consider a sub-value from such a location at a given offset, accessed
through DW_OP_implicit_pointer. Which bytes should be used for that? The
patch above *always* uses the last bytes on big-endian targets, ignoring
the offset.
E.g., given the code snippet
const char foo[] = "Hello, world!";
const char *a = &foo[0];
const char *b = &foo[7];
assume that `foo' is described as DW_OP_implicit_value and `a' and `b'
each as DW_OP_implicit_pointer into that value. Then with current GDB
`*a' and `*b' yield the same result -- the string's zero terminator.
This patch basically reverts the portion of the patch above that deals
with DW_OP_implicit_value. This fixes the offset handling and also goes
back to dropping the last instead of the first bytes on big-endian targets
if the implicit value is longer than needed. The latter aspect of the
change probably doesn't matter for actual programs, but simplifies the
logic.
The patch also cleans up the original code a bit and adds appropriate test
cases.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/dw2-op-stack-value.exp: Adjust expected result of
taking a 2-byte value out of a 4-byte DWARF implicit value on
big-endian targets.
* gdb.dwarf2/nonvar-access.exp: Add more comments to existing
logic. Add test cases for DW_OP_implicit.
gdb/ChangeLog:
* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): For
DWARF_VALUE_LITERAL, no longer ignore the offset on big-endian
targets. And if the implicit value is longer than needed, extract
the first bytes instead of the "least significant" ones.
This changes dwarf2_evaluate_loc_desc_full to use scoped_value_mark.
Note that this function previously called do_cleanup using the same
cleanup multiple times. I had thought this was buggy, but re-reading
make_my_cleanup2 indicates that it is not. Nevertheless it is
surprising, and at least one of the calls (the one that is completely
removed in this patch) seems to have been done under the assumption
that it would still have some effect.
2017-01-10 Tom Tromey <tom@tromey.com>
* value.h (scoped_value_mark::~scoped_value_mark): Call
free_to_mark.
(scoped_value_mark::free_to_mark): New method.
* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Use
scoped_value_mark.
This adds a scoped_value_mark class, that records the value mark in
the constructor and then calls value_free_to_mark in the destructor.
It then updates various spots in gdb to use this class, rather than a
cleanup.
It would be better overall to replace "struct value *" with a
shared_ptr, maybe eliminating the need for this class (watchpoints
would perhaps need some new mechanism as well). However, that's
difficult to do.
2017-01-10 Tom Tromey <tom@tromey.com>
* python/py-value.c (valpy_dereference, valpy_referenced_value)
(valpy_reference_value, valpy_const_value, valpy_get_address)
(valpy_get_dynamic_type, valpy_lazy_string, valpy_do_cast)
(valpy_getitem, valpy_call, valpy_binop_throw, valpy_negative)
(valpy_absolute, valpy_richcompare_throw): Use scoped_value_mark.
* dwarf2loc.c (dwarf2_loc_desc_get_symbol_read_needs): Use
scoped_value_mark.
* dwarf2-frame.c (execute_stack_op): Use scoped_value_mark.
* value.h (scoped_value_mark): New class.
This removes make_cleanup_htab_delete in favor of destructors,
building on an earlier patch that added the htab_up typedef.
Testing revealed that more cleanup-removal work was needed in
dwarf2loc.c, so this version of the patch changes code there to use
unordered_set and vector, removing some more cleanups.
2017-01-10 Tom Tromey <tom@tromey.com>
* utils.h (make_cleanup_htab_delete): Don't declare.
* utils.c (do_htab_delete_cleanup, make_cleanup_htab_delete):
Remove.
* linespec.c (decode_compound_collector): Add constructor,
destructor.
(lookup_prefix_sym): Remove cleanup.
(symtab_collector): Add constructor, destructor.
(collect_symtabs_from_filename): Remove cleanup.
* disasm.c (do_mixed_source_and_assembly): Use htab_up.
* compile/compile-c-symbols.c (generate_c_for_variable_locations):
Use htab_up.
* gnu-v3-abi.c (gnuv3_print_vtable): Use htab_up.
* dwarf2read.c (dw2_expand_symtabs_matching)
(dw2_map_symbol_filenames, dwarf_decode_macros)
(write_psymtabs_to_index): Use htab_up.
* dwarf2loc.c (func_verify_no_selftailcall)
(call_site_find_chain_1, func_verify_no_selftailcall)
(chain_candidate, call_site_find_chain_1): Use std::unordered_set,
std::vector, gdb::unique_xmalloc_ptr.
(call_sitep): Remove typedef.
(dwarf2_locexpr_baton_eval): Remove unused variable.
In ee40d8d (Move computed value's frame id to piece_closure), I only
updated read_pieced_value to use frame_id from piece_closure, but
forgot to update write_pieced_value, so it causes the following
internal error on arm-linux,
set variable l = 4^M
gdb/git/gdb/value.c:1579: internal-error: frame_id* deprecated_value_next_frame_id_hack(value*): Assertion `value->lval == lval_register' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
Quit this debugging session? (y or n) FAIL: gdb.base/store.exp: var longest l; setting l to 4 (GDB internal error)
This patch fixes the internal error.
gdb:
2017-01-04 Yao Qi <yao.qi@linaro.org>
* dwarf2loc.c (write_pieced_value): Don't use VALUE_FRAME_ID (to),
use c->frame_id when the piece location is DWARF_VALUE_REGISTER.
This applies the second part of GDB's End of Year Procedure, which
updates the copyright year range in all of GDB's files.
gdb/ChangeLog:
Update copyright year range in all GDB files.
Nowadays, we set computed value's frame id, which is a misuse to me.
The computed value itself doesn't care about frame id, but function
value_computed_funcs (val)->read (or read_pieced_value) cares about
which frame the register is relative to, so 'struct piece_closure' is
a better place to fit frame id.
This patch adds a frame id in 'struct piece_closure', and use it
instead of using computed value's frame id.
gdb:
2016-11-28 Yao Qi <yao.qi@linaro.org>
* dwarf2loc.c (struct piece_closure) <frame_id>: New field.
(allocate_piece_closure): Add new parameter 'frame' and set
closure's frame_id field accordingly.
(read_pieced_value): Get frame from closure instead of value.
(dwarf2_evaluate_loc_desc_full): Remove code getting frame id.
Don't set value's frame id.
The function copy_bitwise used for copying DWARF pieces can potentially
be invoked for large chunks of data. For instance, consider a large
struct one of whose members is currently located in a register. In this
case copy_bitwise would still copy the data bitwise in a loop, which is
much slower than necessary.
This change uses memcpy for the large part instead, if possible.
gdb/ChangeLog:
* dwarf2loc.c (copy_bitwise): Use memcpy for the middle part, if
it is byte-aligned.
This adds a unit test for the copy_bitwise function in dwarf2loc.c.
With the old (broken) version of copy_bitwise this test would generate
the following failure message:
(gdb) maintenance selftest
Self test failed: copy_bitwise 11000000 != 10000000 (7+2 -> 0)
gdb/ChangeLog:
2016-11-24 Andreas Arnez <arnez@linux.vnet.ibm.com>
Pedro Alves <palves@redhat.com>
* dwarf2loc.c (bits_to_str, check_copy_bitwise)
(copy_bitwise_tests): New functions.
(_initialize_dwarf2loc): Register the new function
copy_bitwise_tests as a unit test.
* selftest.c (run_self_tests): Improve the failure message's
wording and formatting.
When the user writes or reads a variable whose location is described
with DWARF pieces (DW_OP_piece or DW_OP_bit_piece), GDB's helper
function copy_bitwise is invoked for each piece. The implementation of
this function has a bug that may result in a corrupted copy, depending
on alignment and bit size. (Full-byte copies are not affected.)
This rewrites copy_bitwise, replacing its algorithm by a fixed version,
and adding an appropriate test case. Without the fix the new test case
fails, e.g.:
print def_t
$2 = {a = 0, b = 4177919}
(gdb) FAIL: gdb.dwarf2/nonvar-access.exp: print def_t
Written in binary, the wrong result above looks like this:
01111111011111111111111
Which means that two zero bits have sneaked into the copy of the
original all-one bit pattern. The test uses this simple all-one value
in order to avoid another GDB bug that causes the DWARF piece of a
DW_OP_stack_value to be taken from the wrong end on big-endian
architectures.
gdb/ChangeLog:
* dwarf2loc.c (extract_bits_primitive): Remove.
(extract_bits): Remove.
(copy_bitwise): Rewrite. Fixes a possible corruption that may
occur for non-byte-aligned copies.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/nonvar-access.exp: Add a test for accessing
non-byte-aligned bit fields.
The VALUE_FRAME_ID macro provides access to a member in struct value
that's used to hold the frame id that's used when determining a
register's value or when assigning to a register. The underlying
member has a long and obscure name. I won't refer to it here, but
will simply refer to VALUE_FRAME_ID as if it's the struct value member
instead of being a convenient macro.
At the moment, without this patch in place, VALUE_FRAME_ID is set in
value_of_register_lazy() and several other locations to hold the frame
id of the frame passed to those functions.
VALUE_FRAME_ID is used in the lval_register case of
value_fetch_lazy(). To fetch the register's value, it calls
get_frame_register_value() which, in turn, calls
frame_unwind_register_value() with frame->next.
A python based unwinder may wish to determine the value of a register
or evaluate an expression containing a register. When it does this,
value_fetch_lazy() will be called under some circumstances. It will
attempt to determine the frame id associated with the frame passed to
it. In so doing, it will end up back in the frame sniffer of the very
same python unwinder that's attempting to learn the value of a
register as part of the sniffing operation. This recursion is not
desirable.
As noted above, when value_fetch_lazy() wants to fetch a register's
value, it does so (indirectly) by unwinding from frame->next.
With this in mind, a solution suggests itself: Change VALUE_FRAME_ID
to hold the frame id associated with the next frame. Then, when it
comes time to obtain the value associated with the register, we can
simply unwind from the frame corresponding to the frame id stored in
VALUE_FRAME_ID. This neatly avoids the python unwinder recursion
problem by changing when the "next" operation occurs. Instead of the
"next" operation occuring when the register value is fetched, it
occurs earlier on when assigning a frame id to VALUE_FRAME_ID.
(Thanks to Pedro for this suggestion.)
This patch implements this idea.
It builds on the patch "Distinguish sentinel frame from null frame".
Without that work in place, it's necessary to check for null_id at
several places and then obtain the sentinel frame.
It also renames most occurences of VALUE_FRAME_ID to
VALUE_NEXT_FRAME_ID to reflect the new meaning of this field.
There are several uses of VALUE_FRAME_ID which were not changed. In
each case, the original meaning of VALUE_FRAME_ID is required to get
correct results. In all but one of these uses, either
put_frame_register_bytes() or get_frame_register_bytes() is being
called with the frame value obtained from VALUE_FRAME_ID. Both of
these functions perform some unwinding by performing a "->next"
operation on the frame passed to it. If we were to use the new
VALUE_NEXT_FRAME_ID macro, this would effectively do two "->next"
operations, which is not what we want.
The VALUE_FRAME_ID macro has been redefined in terms of
VALUE_NEXT_FRAME_ID. It simply fetches the previous frame's id,
providing this id as the value of the macro.
gdb/ChangeLog:
* value.h (VALUE_FRAME_ID): Rename to VALUE_NEXT_FRAME_ID. Update
comment. Create new VALUE_FRAME_ID which is defined in terms of
VALUE_NEXT_FRAME_ID.
(deprecated_value_frame_id_hack): Rename to
deprecated_value_next_frame_id_hack.
* dwarf2loc.c, findvar.c, frame-unwind.c, sentinel-frame.c,
valarith.c, valops.c, value.c: Adjust nearly all occurences of
VALUE_FRAME_ID to VALUE_NEXT_FRAME_ID. Add comments for those
which did not change.
* value.c (struct value): Rename frame_id field to next_frame_id.
Update comment.
(deprecated_value_frame_id_hack): Rename to
deprecated_value_next_frame_id_hack.
(value_fetch_lazy): Call frame_unwind_register_value()
instead of get_frame_register_value().
* frame.c (get_prev_frame_id_by_id): New function.
* frame.h (get_prev_frame_id_by_id): Declare.
* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Make
VALUE_NEXT_FRAME_ID refer to the next frame.
* findvar.c (value_of_register_lazy): Likewise.
(default_value_from_register): Likewise.
(value_from_register): Likewise.
* frame_unwind.c (frame_unwind_got_optimized): Likewise.
* sentinel-frame.c (sentinel_frame_prev_register): Likewise.
* value.h (VALUE_FRAME_ID): Update comment describing this macro.
This fixes some regressions found in the patch to convert
dwarf_expr_context to use methods. Specifically:
* get_base_type could erroneously throw; this was rewritten to move
the size checks into the only spot needing them.
* Previously the "symbol needs frame" implementation reused th
"cfa" function for the get_frame_pc slot; this reimplements
it under the correct name.
* Not enough members were saved and restored in one implementation
of push_dwarf_reg_entry_value; this patch fixes this oversight
and also takes the opportunity to remove an extraneous structure
definition.
2016-11-02 Tom Tromey <tom@tromey.com>
* dwarf2loc.c (dwarf_evaluate_loc_desc::get_base_type): Rename
from impl_get_base_type. Rewrite.
(struct dwarf_expr_baton): Remove.
(dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value): Save and
restore more fields.
(symbol_needs_eval_context::get_frame_pc): New method.
* dwarf2expr.h (dwarf_expr_context::get_base_type): Now public,
virtual.
(dwarf_expr_context::impl_get_base_type): Remove.
* dwarf2expr.c (dwarf_expr_context::get_base_type): Remove.
This patch converts the function pointers in dwarf_expr_context_funcs
into methods on dwarf_expr_context, and then updates the various
implementations and callers to follow.
NB this patch uses "override" (which caught a couple of renaming bugs
during development) -- but this is C++11, so this patch at least has
to wait for Pedro's patch that adds the OVERRIDE macro.
After this patch it would be possible to do one more, that makes
various members of dwarf_expr_context "protected"; but I haven't done
this.
2016-10-21 Tom Tromey <tom@tromey.com>
* dwarf2loc.c (struct dwarf_expr_context_funcs): Don't declare.
(dwarf_expr_read_addr_from_reg, dwarf_expr_get_reg_value)
(dwarf_expr_read_mem, dwarf_expr_frame_base): Rename; turn into
methods.
(get_frame_pc_for_per_cu_dwarf_call): New function.
(dwarf_expr_frame_cfa, dwarf_expr_frame_pc)
(dwarf_expr_tls_address): Rename; turn into methods.
(per_cu_dwarf_call): Remove arguments. Use
get_frame_pc_for_per_cu_dwarf_call.
(dwarf_evaluate_loc_desc): New class.
(dwarf_expr_dwarf_call, dwarf_expr_context)
(dwarf_expr_push_dwarf_reg_entry_value)
(dwarf_expr_get_addr_index, dwarf_expr_get_obj_addr): Rename; turn
into methods.
(dwarf_expr_ctx_funcs): Remove.
(dwarf2_evaluate_loc_desc_full): Update.
(dwarf2_locexpr_baton_eval): Update.
(symbol_needs_eval_context): New class.
(symbol_needs_read_addr_from_reg, symbol_needs_get_reg_value)
(symbol_needs_read_mem, symbol_needs_frame_base)
(symbol_needs_frame_cfa, symbol_needs_tls_address)
(symbol_needs_dwarf_call, needs_dwarf_reg_entry_value): Rename;
turn into methods.
(needs_get_addr_index, needs_get_obj_addr): Remove; turn into
methods.
(symbol_needs_ctx_funcs): Remove.
(dwarf2_loc_desc_get_symbol_read_needs): Update.
* dwarf2expr.h (struct dwarf_expr_context_funcs): Remove; turn
contents into methods.
(struct dwarf_expr_context) <baton, funcs>: Remove.
<read_addr_from_reg, get_reg_value, read_mem, get_frame_base,
get_frame_cfa, get_frame_pc, get_tls_address, dwarf_call,
impl_get_base_type, push_dwarf_block_entry_value, get_addr_index,
get_object_address>: Declare new methods.
(ctx_no_get_frame_base, ctx_no_get_frame_cfa)
(ctx_no_get_frame_pc, ctx_no_get_tls_address, ctx_no_dwarf_call)
(ctx_no_get_base_type, ctx_no_push_dwarf_reg_entry_value)
(ctx_no_get_addr_index): Don't declare.
* dwarf2expr.c (get_base_type): Use impl_get_base_type.
(execute_stack_op): Update.
(ctx_no_get_frame_base, ctx_no_get_frame_cfa)
(ctx_no_get_frame_pc, ctx_no_get_tls_address, ctx_no_dwarf_call)
(ctx_no_get_base_type, ctx_no_push_dwarf_reg_entry_value)
(ctx_no_get_addr_index): Remove; now methods on
dwarf_expr_context.
* dwarf2-frame.c (read_addr_from_reg): Take a frame_info, not a
baton.
(class dwarf_expr_executor): New class.
(get_reg_value, read_mem): Rename, turn into methods.
(execute_stack_op): Use dwarf_expr_executor.
This converts various DWARF expr functions to be members on
dwarf_expr_context, then fixes up the various users. This results in
somewhat less wordy code and sets the stage for the next patch.
2016-10-21 Tom Tromey <tom@tromey.com>
* dwarf2loc.c (per_cu_dwarf_call)
(dwarf_expr_push_dwarf_reg_entry_value)
(dwarf2_evaluate_loc_desc_full, dwarf2_locexpr_baton_eval)
(needs_dwarf_reg_entry_value)
(dwarf2_loc_desc_get_symbol_read_needs): Update.
* dwarf2expr.h (dwarf_expr_context) <push_address, eval, fetch,
fetch_address, fetch_in_stack_memory, address_type, grow_stack,
push, stack_empty_p, add_piece, get_base_type, execute_stack_op,
pop>: New method declarations.
(dwarf_expr_push_address, dwarf_expr_eval, dwarf_expr_fetch)
(dwarf_expr_fetch_address, dwarf_expr_fetch_in_stack_memory):
Don't declare.
* dwarf2expr.c (address_type, grow_stack, push, push_address)
(pop, fetch, fetch_address, fetch_in_stack_memory)
(stack_empty_p, add_piece, eval, get_base_type)
(execute_stack_op): Rename. Turn into methods.
* dwarf2-frame.c (execute_stack_op): Update.
This is the first step in the conversion of dwarf_expr_ctx to a C++
class. This conversion is done in steps to make the patches, and the
reviews, a bit simpler. This patch changes dwarf_expr_ctx to be
stack-allocated and removes the associated cleanup.
2016-10-21 Tom Tromey <tom@tromey.com>
* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Stack-allocate
dwarf_expr_context. Remove cleanups.
(dwarf2_locexpr_baton_eval)
(dwarf2_loc_desc_get_symbol_read_needs): Likewise.
* dwarf2expr.h (dwarf_expr_context, ~dwarf_expr_context): Add
constructors and destructors.
(new_dwarf_expr_context, free_dwarf_expr_context)
(make_cleanup_free_dwarf_expr_context): Don't declare.
* dwarf2-frame.c (execute_stack_op): Stack-allocate
dwarf_expr_context. Remove cleanups.
(dwarf_expr_context): Rename from new_dwarf_expr_context. Turn
into constructor.
(free_dwarf_expr_context, free_dwarf_expr_context_cleanup):
Remove.
(~dwarf_expr_context): Rename from
make_cleanup_free_dwarf_expr_context. Turn into destructor.
This removes some cleanups and manual allocation handling in
dwarf2loc.c with std::vector. Note that this patch has a case where
the vector would normally fall into the "use gdb::unique_ptr"
guidelines -- but here because the vector is immediately initialized,
I moved the initialization into the constructor call, for further
code savings.
2016-10-21 Tom Tromey <tom@tromey.com>
* dwarf2loc.c: Include <vector>.
(read_pieced_value, write_pieced_value)
(dwarf2_compile_expr_to_ax): Use std::vector.
Currently gdb supports DW_OP_GNU_push_tls_address, but not
DW_OP_form_tls_address. I think it would be better if the toolchain
as a whole moved to using the standard opcode, and the prerequisite to
this is getting gdb to recognize it.
GCC can sometimes emit DW_OP_form_tls_address for emultls targets. As
far as I know, nobody has ever tried this with gdb (since it wouldn't
work at all).
I don't think there's a major drawback to using a single opcode for
all targets, because computing the location of a thread-local is
already target specific.
This is PR gdb/11616.
I don't know how to write a test case for this; though it's worth
noting that there aren't explicit tests for DW_OP_GNU_push_tls_address
either -- and if I change GCC, these paths will be tested to the same
extent they are now.
2016-09-02 Tom Tromey <tom@tromey.com>
PR gdb/11616:
* dwarf2read.c (decode_locdesc): Handle DW_OP_form_tls_address.
* dwarf2loc.c (dwarf2_compile_expr_to_ax): Handle
DW_OP_form_tls_address.
(locexpr_describe_location_piece): Likewise.
* dwarf2expr.h (struct dwarf_expr_context_funcs): Update comment.
* dwarf2expr.c (execute_stack_op): Handle DW_OP_form_tls_address.
(ctx_no_get_tls_address): Mention DW_OP_form_tls_address.
* compile/compile-loc2c.c (struct insn_info): Update comment.
(compute_stack_depth_worker): Handle DW_OP_form_tls_address.
PR python/20190 arose from an exception I noticed when trying to use
the Python unwinder for Spider Monkey in Firefox.
The problem is that the unwinder wants to examine the value of a
thread-local variable. However, sympy_value rejects this because
symbol_read_needs_frame returns true for a TLS variable.
This problem arose once before, though in a different context:
https://sourceware.org/bugzilla/show_bug.cgi?id=11803
At the time Pedro and Daniel pointed out a simpler way to fix that bug
(see links in 20190 if you are interested); but for this new bug I
couldn't think of a similar fix and ended up implementing Daniel's
other suggestion:
https://sourceware.org/ml/gdb-patches/2010-07/msg00393.html
That is, this patch makes it possible to detect whether a symbol needs
a specific frame, or whether it just needs the inferior to have
registers.
Built and regtested on x86-64 Fedora 24.
2016-07-26 Tom Tromey <tom@tromey.com>
* symtab.c (register_symbol_computed_impl): Update.
PR python/20190:
* value.h (symbol_read_needs): Declare.
(symbol_read_needs_frame): Add comment.
* symtab.h (struct symbol_computed_ops) <read_variable>: Update
comment.
<get_symbol_read_needs>: Rename. Change return type.
* findvar.c (symbol_read_needs): New function.
(symbol_read_needs_frame): Rewrite.
(default_read_var_value): Use symbol_read_needs.
* dwarf2loc.c (struct symbol_needs_baton): Rename.
<needs>: Renamed from needs_frame. Changed type.
(needs_frame_read_addr_from_reg, symbol_needs_get_reg_value)
(symbol_needs_read_mem, symbol_needs_frame_base)
(symbol_needs_frame_cfa, symbol_needs_tls_address)
(symbol_needs_dwarf_call): Rename.
(needs_dwarf_reg_entry_value): Update.
(symbol_needs_ctx_funcs, dwarf2_loc_desc_get_symbol_read_needs):
Rename and update.
(locexpr_get_symbol_read_needs, loclist_symbol_needs): Likewise.
(dwarf2_locexpr_funcs, dwarf2_loclist_funcs): Update.
* defs.h (enum symbol_needs_kind): New.
2016-07-26 Tom Tromey <tom@tromey.com>
PR python/20190:
* gdb.threads/tls.exp (check_thread_local): Add python symbol
test.
GDB computes structure byte offsets using a 32 bit integer. And,
first it computes the offset in bits and then converts to bytes. The
result is that any offset that if 512K bytes or larger overflows.
This patch changes GDB to use LONGEST for such calculations.
PR gdb/17520 Structure offset wrong when 1/4 GB or greater.
* c-lang.h: Change all parameters, variables, and struct or union
members used as struct or union fie3ld offsets from int to
LONGEST.
* c-valprint.c: Likewise.
* cp-abi.c: Likewise.
* cp-abi.h: Likewise.
* cp-valprint.c: Likewise.
* d-valprint.c: Likewise.
* dwarf2loc.c: Likewise.
* eval.c: Likewise.
* extension-priv.h: Likewise.
* extension.c: Likewise.
* extension.h: Likewise.
* findvar.c: Likewise.
* gdbtypes.h: Likewise.
* gnu-v2-abi.c: Likewise.
* gnu-v3-abi.c: Likewise.
* go-valprint.c: Likewise.
* guile/guile-internal.h: Likewise.
* guile/scm-pretty-print.c: Likewise.
* jv-valprint.c Likewise.
* opencl-lang.c: Likewise.
* p-lang.h: Likewise.
* python/py-prettyprint.c: Likewise.
* python/python-internal.h: Likewise.
* spu-tdep.c: Likewise.
* typeprint.c: Likewise.
* valarith.c: Likewise.
* valops.c: Likewise.
* valprint.c: Likewise.
* valprint.h: Likewise.
* value.c: Likewise.
* value.h: Likewise.
* p-valprint.c: Likewise.
* c-typeprint.c (c_type_print_base): When printing offset, use
plongest, not %d.
* gdbtypes.c (recursive_dump_type): Ditto.
Simon Marchi