Unlike the forms consuming/producing integer data, the floating point
ones so far required the 2nd argument to be present, contrary to
documentation. To avoid code duplication, split float_length() out of
hex_float() (taking the opportunity to adjust error message wording).
This is to be able to generate data acted upon by AVX512-BF16 and
AMX-BF16 insns. While not part of the IEEE standard, the format is
sufficiently standardized to warrant handling in config/atof-ieee.c.
Arm, where custom handling was implemented, may want to leverage this as
well. To be able to also use the hex forms supported for other floating
point formats, a small addition to the generic hex_float() is needed.
Extend existing x86 testcases.
This is to be able to generate data passed to {,V}CVTPH2PS and acted
upon by AVX512-FP16 insns. To be able to also use the hex forms
supported for other floating point formats, a small addition to the
generic hex_float() is needed.
Extend existing x86 testcases.
The ELF psABI-s are quite clear here: On 32-bit the data type is 12
bytes long (with 2 bytes of trailing padding), while on 64-bit it is 16
bytes long (with 6 bytes of padding). Make hex_float() capable of
handling such padding.
Note that this brings the emitted data size of .dc.x / .dcb.x in line
also for non-ELF targets; so far they were different depending on input
format (dec vs hex).
Extend the existing x86 testcases.
The ELF psABI-s are quite clear here: On 32-bit the underlying data type
is 12 bytes long (with 2 bytes of trailing padding), while on 64-bit it
is 16 bytes long (with 6 bytes of padding). Make s_space() capable of
handling 'x' (and 'p') type floating point being other than 12 bytes
wide (also adjusting documentation). This requires duplicating the
definition of X_PRECISION in the target speciifc header; the compiler
would complain if this was out of sync with config/atof-ieee.c.
Note that for now padding space doesn't get separated from actual
storage, which means that things will work correctly only for little-
endian cases, and which also means that by specifying large enough
numbers padding space can be set to non-zero. Since the logic is needed
for a single little-endian architecture only for now, I'm hoping that
this might be acceptable for the time being; otherwise the change will
become more intrusive.
Note also that this brings the emitted data size of .ds.x vs .tfloat in
line for non-ELF targets as well; the issue will be even more obvious
when further taking into account a subsequent patch fixing .dc.x/.dcb.x
(where output sizes currently differ depending on input format).
Extend existing x86 testcases.
The ELF psABI-s are quite clear here: On 32-bit the data type is 12
bytes long (with 2 bytes of trailing padding), while on 64-bit it is 16
bytes long (with 6 bytes of padding). Make ieee_md_atof() capable of
handling such padding, and specify the needed padding for x86 (leaving
non-ELF targets alone for now). Split the existing x86 testcase.
Relocation offset checks were completely missing in the rl78 backend,
allowing a relocation to write over memory anywhere. This was true
for rl78_special_reloc, a function primarily used when applying debug
relocations, and in rl78_elf_relocate_section used by the linker.
This patch fixes those problems by correcting inaccuracies in the
relocation howtos, then uses those howtos to sanity check relocation
offsets before applying relocations. In addition, the patch
implements overflow checking using the howto information rather than
the ad-hoc scheme implemented in relocate_section. I implemented the
overflow checking in rl78_special_reloc too.
* elf32-rl78.c (RL78REL, RL78_OP_REL): Add mask parameter.
(rl78_elf_howto_table): Set destination masks. Correct size and
bitsize of DIR32_REV. Correct complain_on_overflow for many relocs
as per tests in relocate_section. Add RH_SFR. Correct bitsize
for RH_SADDR. Set size to 3 and bitsize to 0 for all OP relocs.
(check_overflow): New function.
(rl78_special_reloc): Check that reloc address is within section.
Apply relocations using reloc howto. Check for overflow.
(RANGE): Delete.
(rl78_elf_relocate_section): Sanity check r_offset. Perform
overflow checking using reloc howto.
I noticed that the fission-reread.exp test case can cause a complaint
when run with --target_board=cc-with-debug-names:
warning: Section .debug_aranges in [...]/fission-reread has duplicate debug_info_offset 0x0, ignoring .debug_aranges.
The bug here is that this executable has both .debug_info and
.debug_types, and both have a CU at offset 0x0. This triggers the
duplicate warning.
Because .debug_types doesn't provide any address ranges, these CUs can
be ignored. That is, this bug turns out to be another regression from
the info/types merger patch.
This patch fixes the problem by having this loop igore type units.
fission-reread.exp is updated to test for the bug.
While debugging another patch series, I wanted to dump an addrmap. I
came up with this patch, which generalizes the addrmap-dumping code
from psymtab.c and moves it to addrmap.c. psymtab.c is changed to use
the new code.
I spotted what I think is a buglet in proceed_after_vfork_done. After a
vfork child exits or execs, we resume all the threads of the parent. To
do so, we iterate on all threads using iterate_over_threads with the
proceed_after_vfork_done callback. Each thread is resumed if the
following condition is true:
if (thread->ptid.pid () == pid
&& thread->state == THREAD_RUNNING
&& !thread->executing
&& !thread->stop_requested
&& thread->stop_signal () == GDB_SIGNAL_0)
where `pid` is the pid of the vfork parent. This is not multi-target
aware: since it only filters on pid, if there is an inferior with the
same pid in another target, we could end up resuming a thread of that
other inferior. The chances of the stars aligning for this to happen
are tiny, but still.
Fix that by iterating only on the vfork parent's threads, instead of on
all threads. This is more efficient, as we iterate on just the required
threads (inferiors have their own thread list), and we can drop the pid
check. The resulting code is also more straightforward in my opinion,
so it's a win-win.
Change-Id: I14647da72e2bf65592e82fbe6efb77a413a4be3a
Before Guile v2.1 [1], calls to `scm_make_smob_type' implicitly added
the created class to the exports list of (oop goops); v2.1+ does not
implicitly create bindings in any modules. This means that the GDB
manual subsection documenting exported types is not quite right when GDB
is linked against Guile <v2.1 (types are exported from (oop goops))
instead of (gdb)) and incorrect when linked against Guile v2.1+ (types
are not bound to any variables at all!).
There is a range of cases in which it's necessary or convenient to be
able to refer to a GDB smob type, for instance:
- Pattern matching based on the type of a value.
- Defining GOOPS methods handling values from GDB (GOOPS methods
typically use dynamic dispatch based on the types of the arguments).
- Type-checking assertions when applying some defensive programming on
an interface.
- Generally any other situation one might encounter in a dynamically
typed language that might need some introspection.
If you're more familiar with Python, it would be quite similar to being
unable to refer to the classes exported from the GDB module (which is to
say: not crippling for the most part, but makes certain tasks more
difficult than necessary).
This commit makes a small change to GDB's smob registration machinery
to make sure registered smobs get exported from the current
module. This will likely cause warnings to the user about conflicting
exports if they load both (gdb) and (oop goops) from a GDB linked
against Guile v2.0, but it shouldn't impact functionality (and seemed
preferable to trying to un-export bindings from (oop goops) if v2.0
was detected).
[1]: This changed with Guile commit
28d0871b553a3959a6c59e2e4caec1c1509f8595
gdb/ChangeLog:
2021-06-07 George Barrett <bob@bob131.so>
* guile/scm-gsmob.c (gdbscm_make_smob_type): Export registered
smob type from the current module.
gdb/testsuite/ChangeLog:
2021-06-07 George Barrett <bob@bob131.so>
* gdb.guile/scm-gsmob.exp (test exports): Add tests to make
sure the smob types currently listed in the GDB manual get
exported from the (gdb) module.
Change-Id: I7dcd791276b48dfc9edb64fc71170bbb42a6f6e7
* dwarf2dbg.c (get_directory_table_entry): Ensure that dir[0]
contains current working directory.
(out_dir_and_file_list): Likewise.
* testsuite/gas/elf/dwarf-5-dir0.s: New test source file.
* testsuite/gas/elf/dwarf-5-dir0.d: New test driver.
* testsuite/gas/elf/elf.exp: Run the new test.
* testsuite/gas/elf/dwarf-5-file0.d: Adjust expected output.
* testsuite/gas/i386/dwarf5-line-1.d: Likewise.
* testsuite/gas/i386/dwarf5-line-2.d: Likewise.
I found a few .c files that rely on objfiles.h, but that only include
it indirectly, via dwarf2/read.h -> psympriv.h. If that include is
removed (something my new DWARF indexer series does), then the build
will break.
It seemed harmless and correct to add these includes now, making the
eventual series a little smaller.
This is a bit more than just preventing the divide by zero. Most of
the patch is tidying up error reporting, so that for example, linking
an object file with a reloc stack underflow produces a linker error
rather than just displaying a message that might be ignored.
PR 28176
* elf32-rl78.c (RL78_STACK_PUSH, RL78_STACK_POP): Delete.
(rl78_stack_push, rl78_stack_pop): New inline functions.
(rl78_compute_complex_reloc): Add status and error message params.
Use new inline stack handling functions. Report stack overflow
or underflow, and divide by zero.
(rl78_special_reloc): Return status and error message from
rl78_compute_complex_reloc.
(rl78_elf_relocate_section): Similarly. Modernise reloc error
reporting. Delete unused bfd_reloc_other case. Don't assume
DIR24S_PCREL overflow is due to undefined function.
(rl78_offset_for_reloc): Adjust to suit rl78_compute_complex_reloc.
When reading a .gdb_index that contains a non-empty symbol table with only
empty entries, gdb doesn't recognize it as empty.
Fix this by recognizing that the constant pool is empty, and then setting the
symbol table to empty.
Tested on x86_64-linux.
gdb/ChangeLog:
2021-08-01 Tom de Vries <tdevries@suse.de>
PR symtab/28159
* dwarf2/read.c (read_gdb_index_from_buffer): Handle symbol table
filled with empty entries.
gdb/testsuite/ChangeLog:
2021-08-01 Tom de Vries <tdevries@suse.de>
PR symtab/28159
* gdb.dwarf2/dw2-zero-range.exp: Remove kfail.
The way that init.c is generated does not allow for an initialization
function to be conditionally defined -- doing so will result in a link
error.
This patch fixes a build problem that arises from such a conditional
definition. It can be reproduce with --disable-unit-tests.
In PR28004 the following warning / Internal error is reported:
...
$ gdb -q -batch \
-iex "set sysroot $(pwd -P)/repro" \
./repro/gdb \
./repro/core \
-ex bt
...
Program terminated with signal SIGABRT, Aborted.
#0 0x00007ff8fe8e5d22 in raise () from repro/usr/lib/libc.so.6
[Current thread is 1 (LWP 1762498)]
#1 0x00007ff8fe8cf862 in abort () from repro/usr/lib/libc.so.6
warning: (Internal error: pc 0x7ff8feb2c21d in read in psymtab, \
but not in symtab.)
warning: (Internal error: pc 0x7ff8feb2c218 in read in psymtab, \
but not in symtab.)
...
#2 0x00007ff8feb2c21e in __gnu_debug::_Error_formatter::_M_error() const \
[clone .cold] (warning: (Internal error: pc 0x7ff8feb2c21d in read in \
psymtab, but not in symtab.)
) from repro/usr/lib/libstdc++.so.6
...
The warning is about the following:
- in find_pc_sect_compunit_symtab we try to find the address
(0x7ff8feb2c218 / 0x7ff8feb2c21d) in the symtabs.
- that fails, so we try again in the partial symtabs.
- we find a matching partial symtab
- however, the partial symtab has a full symtab, so
we should have found a matching symtab in the first step.
The addresses are:
...
(gdb) info sym 0x7ff8feb2c218
__gnu_debug::_Error_formatter::_M_error() const [clone .cold] in \
section .text of repro/usr/lib/libstdc++.so.6
(gdb) info sym 0x7ff8feb2c21d
__gnu_debug::_Error_formatter::_M_error() const [clone .cold] + 5 in \
section .text of repro/usr/lib/libstdc++.so.6
...
which correspond to unrelocated addresses 0x9c218 and 0x9c21d:
...
$ nm -C repro/usr/lib/libstdc++.so.6.0.29 | grep 000000000009c218
000000000009c218 t __gnu_debug::_Error_formatter::_M_error() const \
[clone .cold]
...
which belong to function __gnu_debug::_Error_formatter::_M_error() in
/build/gcc/src/gcc/libstdc++-v3/src/c++11/debug.cc.
The partial symtab that is found for the addresses is instead the one for
/build/gcc/src/gcc/libstdc++-v3/src/c++98/bitmap_allocator.cc, which is
incorrect.
This happens as follows.
The bitmap_allocator.cc CU has DW_AT_ranges at .debug_rnglist offset 0x4b50:
...
00004b50 0000000000000000 0000000000000056
00004b5a 00000000000a4790 00000000000a479c
00004b64 00000000000a47a0 00000000000a47ac
...
When reading the first range 0x0..0x56, it doesn't trigger the "start address
of zero" complaint here:
...
/* A not-uncommon case of bad debug info.
Don't pollute the addrmap with bad data. */
if (range_beginning + baseaddr == 0
&& !per_objfile->per_bfd->has_section_at_zero)
{
complaint (_(".debug_rnglists entry has start address of zero"
" [in module %s]"), objfile_name (objfile));
continue;
}
...
because baseaddr != 0, which seems incorrect given that when loading the
shared library individually in gdb (and consequently baseaddr == 0), we do see
the complaint.
Consequently, we run into this case in dwarf2_get_pc_bounds:
...
if (low == 0 && !per_objfile->per_bfd->has_section_at_zero)
return PC_BOUNDS_INVALID;
...
which then results in this code in process_psymtab_comp_unit_reader being
called with cu_bounds_kind == PC_BOUNDS_INVALID, which sets the set_addrmap
argument to 1:
...
scan_partial_symbols (first_die, &lowpc, &highpc,
cu_bounds_kind <= PC_BOUNDS_INVALID, cu);
...
and consequently, the CU addrmap gets build using address info from the
functions.
During that process, addrmap_set_empty is called with a range that includes
0x9c218 and 0x9c21d:
...
(gdb) p /x start
$7 = 0x9989c
(gdb) p /x end_inclusive
$8 = 0xb200d
...
but it's called for a function at DIE 0x54153 with DW_AT_ranges at 0x40ae:
...
000040ae 00000000000b1ee0 00000000000b200e
000040b9 000000000009989c 00000000000998c4
000040c3 <End of list>
...
and neither range includes 0x9c218 and 0x9c21d.
This is caused by this code in partial_die_info::read:
...
if (dwarf2_ranges_read (ranges_offset, &lowpc, &highpc, cu,
nullptr, tag))
has_pc_info = 1;
...
which pretends that the function is located at addresses 0x9989c..0xb200d,
which is indeed not the case.
This patch fixes the first problem encountered: fix the "start address of
zero" complaint warning by removing the baseaddr part from the condition.
Same for dwarf2_ranges_process.
The effect is that:
- the complaint is triggered, and
- the warning / Internal error is no longer triggered.
This does not fix the observed problem in partial_die_info::read, which is
filed as PR28200.
Tested on x86_64-linux.
Co-Authored-By: Simon Marchi <simon.marchi@polymtl.ca>
gdb/ChangeLog:
2021-07-29 Simon Marchi <simon.marchi@polymtl.ca>
Tom de Vries <tdevries@suse.de>
PR symtab/28004
* gdb/dwarf2/read.c (dwarf2_rnglists_process, dwarf2_ranges_process):
Fix zero address complaint.
* gdb/testsuite/gdb.dwarf2/dw2-zero-range-shlib.c: New test.
* gdb/testsuite/gdb.dwarf2/dw2-zero-range.c: New test.
* gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp: New file.
It matters not at all if pc is incremented from its initial NULL
value, but avoid this silly runtime ubsan error.
* doc/chew.c (perform): Avoid incrementing NULL pc.
This patch is more about the style of bounds checking we ought to use,
rather than a real problem. An overflow of "octet + reloc_size" can
only happen with huge sections which would certainly cause out of
memory errors.
* reloc.c (bfd_reloc_offset_in_range): Avoid possible overflow.
The obj_convert table shouldn't be accessed without first checking the
index against the table size.
PR 28175
* coff-tic30.c (reloc_processing): Sanity check reloc symbol index.
* coff-z80.c (reloc_processing): Likewise.
* coff-z8k.c (reloc_processing): Likewise.
Indexing the howto table was seriously broken by a missing entry, and
use of assertions about user input rather than testing the input.
PR 28173
* elf32-nds32.c (nds32_elf_howto_table): Add missing empty howto.
(bfd_elf32_bfd_reloc_type_table_lookup): Replace assertions with
range checks. Return NULL if unsupported reloc type. Remove
dead code. Take an unsigned int param.
(nds32_info_to_howto_rel): Test for NULL howto or howto name
return from lookup. Remove assertion.
(nds32_info_to_howto): Remove unnecessary ATTRIBUTE_UNUSED.
Test for NULL howto or howto name return from lookup.
bfin pcrel24 relocs are weird, they apply to the reloc address minus
two. That means reloc addresses of 0 and 1 are invalid. Check that,
and fix other reloc range checking.
PR 28172
* elf32-bfin.c (bfin_pcrel24_reloc): Correct reloc range check.
(bfin_imm16_reloc, bfin_byte4_reloc, bfin_bfd_reloc): Likewise.
(bfin_final_link_relocate): Likewise.
[PATCH] GDB Testsuite, update compile-cplus.exp
Update the gdb.compile/compile-cplus.exp test to
handle errors generated when passing bad arguments
into the gdb-compile command.
This matches changes made to gdb.compile/compile.exp
in the past as part of
"Migrate rest of compile commands to new options framework"
e6ed716cd5
[gdb] Handle .TOC. sections during gdb-compile for rs6000 target.
When we encounter a .TOC. symbol in the object we are loading,
we need to associate this with the .toc section in order to
properly resolve other symbols in the object. IF a .toc section
is not found, iterate the sections until we find one with the
SEC_ALLOC flag. If that also fails, fall back to using
the *ABS* section, pointed to by bfd_abs_section_ptr.
In gdb.base/attach.exp, proc do_attach_failure_tests, we attach to a
process. When then try to attach to the same process in another
inferior, expecting it to fail. We then come back to the first inferior
and try to kill it, to clean up the test. When using the
native-extended-gdbserver board, this "kill" test passes, even though it
didn't actually work:
add-inferior
[New inferior 2]
Added inferior 2 on connection 1 (extended-remote localhost:2347)
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: add empty inferior 2
inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: switch to inferior 2
attach 817032
Attaching to process 817032
Attaching to process 817032 failed
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: fail to attach again
inferior 1
[Switching to inferior 1 [process 817032] (/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/attach/attach)]
[Switching to thread 1.1 (Thread 817032.817032)]
#0 main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/attach.c:19
19 while (! should_exit)
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: switch to inferior 1
kill
Kill the program being debugged? (y or n) y
Remote connection closed <==== That's unexpected
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: exit after attach failures
When the second attach fails, gdbserver seems to break the connection
(it hangs up on the existing remote target) and start listening again
for incoming connections. This is documented in PR 19558 [1].
Make the expected output regexp for the kill command tighter (it
currently accepts anything). Use "set confirm off" so we don't have to
deal with the confirmation. And to be really sure the extended-remote
target still works, try to run the inferior again after killing. The
now tests are kfail'ed when the target is gdbserver.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=19558
gdb/testsuite/ChangeLog:
* gdb.base/attach.exp (do_attach_failure_tests): Make kill
regexp tighter, run inferior after killing it. Kfail when
target is gdbserver.
Change-Id: I99c5cd3968ce2ec962ace35b016f842a243b7a0d
When running this test with the native-extended-gdbserver, we get:
main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/attach.c:19
19 while (! should_exit)
The program being debugged has been started already.
Start it from the beginning? (y or n) PASS: gdb.base/attach.exp: cmdline attach run: run to prompt
y
Don't know how to run. Try "help target".
(gdb) FAIL: gdb.base/attach.exp: cmdline attach run: run to main
This test tests using both "-p <pid>" and "-ex start" on the command line,
making sure that we first attach and then run.
Normally, after that "y", we should see the program running again.
However, a particuliarity of the native-extended-gdbserver is that it
uses "set auto-connect-native-target off" on the command line. The full
GDB command line is:
./gdb -nw -nx -data-directory /home/simark/build/binutils-gdb/gdb/testsuite/../data-directory \
-iex set height 0 -iex set width 0 -ex set auto-connect-native-target off \
-ex set sysroot -quiet -iex set height 0 -iex set width 0 --pid=536609 -ex start
The attach succeeds. I guess it is done before "set
auto-connect-native-target off", or it somehow bypasses it. When the
"start" is executed, the native target is unpushed, while killing the
existing process, but not re-pushed, due to "set
auto-connect-native-target off". So we get that "Don't know how to run"
message.
Really, I think it's a case of the test doing things incompatible with
the board, I think it should just be skipped. And as we can see with
the current code, there were some attempts at doing this, just using the
wrong checks:
- isnative: this is a dejagnu proc which checks if the target board has
the same triplet as the build machine. In the case of
native-extended-gdbserver, it does.
- is_remote target: this checks whether the target board is remote, as
in executing on a different machin. native-extended-gdbserver is not
remote.
Since the --pid option specifically attaches to a process using the
native target, change the test to use gdb_is_target_native instead.
gdb/testsuite/ChangeLog:
* gdb.base/attach.exp (test_command_line_attach_run): Use
gdb_is_target_native to check if test is supported.
Change-Id: I762e127f39623889999dc9ed2185540a0951bfb0
Print the extra information contained in target_waitstatus for these
events. For TARGET_WAITKIND_{FORKED,VFORKED}, the extra information is
contained in related_pid, and is the ptid of the new process. For
TARGET_WAITKIND_EXECD, it,s the exec'd path name in execd_pathname.
Print it using the same format used for TARGET_WAITKIND_STOPPED and
others.
Here are sample outputs for all three events:
[infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
[infrun] print_target_wait_results: 726890.726890.0 [process 726890],
[infrun] print_target_wait_results: status->kind = vforked, related_pid = 726894.726894.0
[infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
[infrun] print_target_wait_results: 727045.727045.0 [process 727045],
[infrun] print_target_wait_results: status->kind = forked, related_pid = 727049.727049.0
[infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
[infrun] print_target_wait_results: 727119.727119.0 [process 727119],
[infrun] print_target_wait_results: status->kind = execd, execd_pathname = /usr/bin/ls
Change-Id: I4416a74e3bf792a625a68bf26c51689e170f2184
The ptid_t::to_string method was introduced recently, to format a ptid_t
for debug purposes. It formats the ptid exactly as is done in
print_target_wait_results, so make print_target_wait_results use it.
Change-Id: I0a81c8040d3e1858fb304cb28366b34d94eefe4d
There are cases where the result of the expression evaluation is
expected to be in a form of a value and not location description.
One place that has this requirement is dwarf_entry_parameter_to_value
function, but more are expected in the future. Until now, this
requirement was fulfilled by extending the evaluated expression with
a DW_OP_stack_value operation at the end.
New implementation, introduces a new evaluation argument instead.
* dwarf2/expr.c (dwarf_expr_context::fetch_result): Add as_lval
argument.
(dwarf_expr_context::eval_exp): Add as_lval argument.
* dwarf2/expr.h (struct dwarf_expr_context): Add as_lval
argument to fetch_result and eval_exp methods.
* dwarf2/frame.c (execute_stack_op): Add as_lval argument.
* dwarf2/loc.c (dwarf_entry_parameter_to_value): Remove
DWARF expression extension.
(dwarf2_evaluate_loc_desc_full): Add as_lval argument support.
(dwarf2_evaluate_loc_desc): Add as_lval argument support.
(dwarf2_locexpr_baton_eval): Add as_lval argument support.
Idea of this patch is to get a clean and simple public interface for
the dwarf_expr_context class, looking like:
- constructor,
- destructor,
- push_address method and
- evaluate method.
Where constructor should only ever require a target architecture
information. This information is held in per object file
(dwarf2_per_objfile) structure, so it makes sense to keep that
structure as a constructor argument. It also makes sense to get the
address size from that structure, but unfortunately that interface
doesn't exist at the moment, so the dwarf_expr_context class user
needs to provide that information.
The push_address method is used to push a CORE_ADDR as a value on
top of the DWARF stack before the evaluation. This method can be
later changed to push any struct value object on the stack.
The evaluate method is the method that evaluates a DWARF expression
and provides the evaluation result, in a form of a single struct
value object that describes a location. To do this, the method requires
a context of the evaluation, as well as expected result type
information. If the type information is not provided, the DWARF generic
type will be used instead.
To avoid storing the gdbarch information in the evaluator object, that
information is now always acquired from the per_objfile object.
All data members are now private and only visible to the evaluator
class, so a m_ prefix was added to all of their names to reflect that.
To make this distinction clear, they are also accessed through objects
this pointer, wherever that was not the case before.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::dwarf_expr_context): Add
address size argument.
(dwarf_expr_context::read_mem): Change to use property_addr_info
structure.
(dwarf_expr_context::evaluate): New function.
(dwarf_expr_context::execute_stack_op): Change to use
property_addr_info structure.
* dwarf2/expr.h (struct dwarf_expr_context): New evaluate
declaration. Change eval and fetch_result method to private.
(dwarf_expr_context::gdbarch): Remove member.
(dwarf_expr_context::stack): Make private and add m_ prefix.
(dwarf_expr_context::addr_size): Make private and add
m_ prefix.
(dwarf_expr_context::recursion_depth): Make private and add
m_ prefix.
(dwarf_expr_context::max_recursion_depth): Make private and
add m_ prefix.
(dwarf_expr_context::len): Make private and add m_ prefix.
(dwarf_expr_context::data): Make private and add m_ prefix.
(dwarf_expr_context::initialized): Make private and add
m_ prefix.
(dwarf_expr_context::pieces): Make private and add m_ prefix.
(dwarf_expr_context::per_objfile): Make private and add
m_ prefix.
(dwarf_expr_context::frame): Make private and add m_ prefix.
(dwarf_expr_context::per_cu): Make private and add m_ prefix.
(dwarf_expr_context::addr_info): Make private and add
m_ prefix.
* dwarf2/frame.c (execute_stack_op): Change to call evaluate
method.
* dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Change to call
evaluate method.
(dwarf2_locexpr_baton_eval): Change to call evaluate method.
The patch is addressing the issue of class users writing and reading
the internal data of the dwarf_expr_context class.
At this point, all conditions are met for the DWARF evaluator to return
an evaluation result in a form of a single struct value object.
gdb/ChangeLog:
* dwarf2/expr.c (pieced_value_funcs): Chenge to static
function.
(allocate_piece_closure): Change to static function.
(dwarf_expr_context::fetch_result): New function.
* dwarf2/expr.h (struct piece_closure): Remove declaration.
(struct dwarf_expr_context): fetch_result new declaration.
fetch, fetch_address and fetch_in_stack_memory members move
to private.
(allocate_piece_closure): Remove.
* dwarf2/frame.c (execute_stack_op): Change to use
fetch_result.
* dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Change to use
fetch_result.
(dwarf2_locexpr_baton_eval): Change to use fetch_result.
* dwarf2/loc.h (invalid_synthetic_pointer): Expose function.
Fixing a bug where the value_copy function did not copy the stack data
and initialized members of the struct value. This is needed for the
next patch where the DWARF expression evaluator is changed to return a
single struct value object.
* value.c (value_copy): Change to also copy the stack data
and initialized members.
Following 5 patches series is trying to clean up the interface of the
DWARF expression evaluator class (dwarf_expr_context).
After merging all expression evaluators into one class, the next
logical step is to make a clean user interface for that class. To do
that, we first need to address the issue of class users writing and
reading the internal data of the class directly.
Fixing the case of writing is simple, it makes sense for an evaluator
instance to be per architecture basis. Currently, the best separation
seems to be per object file, so having that data (dwarf2_per_objfile)
as a constructor argument makes sense. It also makes sense to get the
address size from that object file, but unfortunately that interface
does not exist at the moment.
Luckily, address size information is already available to the users
through other means. As a result, the address size also needs to be a
class constructor argument, at least until a better interface for
acquiring that information from an object file is implemented.
The rest of the user written data comes down to a context of an
evaluated expression (compilation unit context, frame context and
passed in buffer context) and a source type information that a result
of evaluating expression is representing. So, it makes sense for all of
these to be arguments of an evaluation method.
To address the problem of reading the dwarf_expr_context class
internal data, we first need to understand why it is implemented that
way?
This is actualy a question of which existing class can be used to
represent both values and a location descriptions and why it is not
used currently?
The answer is in a struct value class/structure, but the problem is
that before the evaluators were merged, only one evaluator had an
infrastructure to resolve composite and implicit pointer location
descriptions.
After the merge, we are now able to use the struct value to represent
any result of the expression evaluation. It also makes sense to move
all infrastructure for those location descriptions to the expr.c file
considering that that is the only place using that infrastructure.
What we are left with in the end is a clean public interface of the
dwarf_expr_context class containing:
- constructor,
- destructor,
- push_address method and
- eval_exp method.
The idea with this particular patch is to move piece_closure structure
and the interface that handles it (lval_funcs) to expr.c file.
While implicit pointer location descriptions are still not useful in
the CFI context (of the AMD's DWARF standard extensions), the composite
location descriptions are certainly necessary to describe a results of
specific compiler optimizations.
Considering that a piece_closure structure is used to represent both,
there was no benefit in splitting them.
gdb/ChangeLog:
* dwarf2/expr.c (struct piece_closure): Add from loc.c.
(allocate_piece_closure): Add from loc.c.
(bits_to_bytes): Add from loc.c.
(rw_pieced_value): Add from loc.c.
(read_pieced_value): Add from loc.c.
(write_pieced_value): Add from loc.c.
(check_pieced_synthetic_pointer): Add from loc.c.
(indirect_pieced_value): Add from loc.c.
(coerce_pieced_ref): Add from loc.c.
(copy_pieced_value_closure): Add from loc.c.
(free_pieced_value_closure): Add from loc.c.
(sect_variable_value): Add from loc.c.
* dwarf2/loc.c (sect_variable_value): Move to expr.c.
(struct piece_closure): Move to expr.c.
(allocate_piece_closure): Move to expr.c.
(bits_to_bytes): Move to expr.c.
(rw_pieced_value): Move to expr.c.
(read_pieced_value): Move to expr.c.
(write_pieced_value): Move to expr.c.
(check_pieced_synthetic_pointer): Move to expr.c.
(indirect_pieced_value): Move to expr.c.
(coerce_pieced_ref): Move to expr.c.
(copy_pieced_value_closure): Move to expr.c.
(free_pieced_value_closure): Move to expr.c.
The evaluate_for_locexpr_baton is the last derived class from the
dwarf_expr_context class. It's purpose is to support the passed in
buffer functionality.
Although, it is not really necessary to merge this class with it's
base class, doing that simplifies new expression evaluator design.
Considering that this functionality is going around the DWARF standard,
it is also reasonable to expect that with a new evaluator design and
extending the push object address functionality to accept any location
description, there will be no need to support passed in buffers.
Alternatively, it would also makes sense to abstract the interaction
between the evaluator and a given resource in the near future. The
passed in buffer would then be a specialization of that abstraction.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::read_mem): Merge with
evaluate_for_locexpr_baton implementation.
* dwarf2/loc.c (class evaluate_for_locexpr_baton): Remove
class.
(evaluate_for_locexpr_baton::read_mem): Move to
dwarf_expr_context.
(dwarf2_locexpr_baton_eval): Instantiate dwarf_expr_context
instead of evaluate_for_locexpr_baton class.
There are no virtual methods that require different specialization in
dwarf_expr_context class. This means that derived classes
dwarf_expr_executor and dwarf_evaluate_loc_desc are not needed any
more.
As a result of this, the evaluate_for_locexpr_baton class base class
is now the dwarf_expr_context class.
There might be a need for a better class hierarchy when we know more
about the direction of the future DWARF versions and gdb extensions,
but that is out of the scope of this patch series.
gdb/ChangeLog:
* dwarf2/frame.c (class dwarf_expr_executor): Remove class.
(execute_stack_op): Instantiate dwarf_expr_context instead of
dwarf_evaluate_loc_desc class.
* dwarf2/loc.c (class dwarf_evaluate_loc_desc): Remove class.
(dwarf2_evaluate_loc_desc_full): Instantiate dwarf_expr_context
instead of dwarf_evaluate_loc_desc class.
(struct evaluate_for_locexpr_baton): Derive from
dwarf_expr_context.
The get_reg_value method is a small function that is only called once,
so it can be inlined to simplify the dwarf_expr_context class.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::get_reg_value): Remove
method.
(dwarf_expr_context::execute_stack_op): Inline get_reg_value
method.
* dwarf2/expr.h (dwarf_expr_context::get_reg_value): Remove
method.
Following the idea of merging the evaluators, the
push_dwarf_reg_entry_value method can be moved from
dwarf_expr_executor and dwarf_evaluate_loc_desc classes
to their base class dwarf_expr_context.
gdb/ChangeLog:
* dwarf2/expr.c
(dwarf_expr_context::push_dwarf_reg_entry_value): Move from
dwarf_evaluate_loc_desc.
* dwarf2/frame.c
(dwarf_expr_executor::push_dwarf_reg_entry_value): Remove
method.
* dwarf2/loc.c (dwarf_expr_reg_to_entry_parameter): Expose
function.
(dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value): Move to
dwarf_expr_context.
* dwarf2/loc.h (dwarf_expr_reg_to_entry_parameter): Expose
function.
Following the idea of merging the evaluators, the read_mem method can
be moved from dwarf_expr_executor and dwarf_evaluate_loc_desc classes
to their base class dwarf_expr_context.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::read_mem): Move from
dwarf_evaluate_loc_desc.
* dwarf2/frame.c (dwarf_expr_executor::read_mem): Remove
method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::read_mem): Move to
dwarf_expr_context.
Following the idea of merging the evaluators, the get_object_address
and can be moved from dwarf_expr_executor and dwarf_evaluate_loc_desc
classes to their base class dwarf_expr_context.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::get_object_address): Move
from dwarf_evaluate_loc_desc.
(class dwarf_expr_context): Add object address member to
dwarf_expr_context.
* dwarf2/expr.h (dwarf_expr_context::get_frame_pc): Remove
method.
* dwarf2/frame.c (dwarf_expr_executor::get_object_address):
Remove method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::get_object_address):
move to dwarf_expr_context.
(class dwarf_evaluate_loc_desc): Move object address member to
dwarf_expr_context.
Following the idea of merging the evaluators, the dwarf_call and
get_frame_pc method can be moved from dwarf_expr_executor and
dwarf_evaluate_loc_desc classes to their base class dwarf_expr_context.
Once this is done, the get_frame_pc can be replace with lambda
function.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::dwarf_call): Move from
dwarf_evaluate_loc_desc.
(dwarf_expr_context::get_frame_pc): Replace with lambda.
* dwarf2/expr.h (dwarf_expr_context::get_frame_pc): Remove
method.
* dwarf2/frame.c (dwarf_expr_executor::dwarf_call): Remove
method.
(dwarf_expr_executor::get_frame_pc): Remove method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::get_frame_pc): Remove
method.
(dwarf_evaluate_loc_desc::dwarf_call): Move to
dwarf_expr_context.
(per_cu_dwarf_call): Inline function.
