forked from Imagelibrary/binutils-gdb
b9b1a8273b1641c585510b7584d3a0f63be6935b
104203 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Zoran Zaric
|
b9b1a8273b |
Add new register access interface to expr.c
DWARF expression evaluator is currently using get_frame_register_bytes and put_frame_register_bytes interface for register access. The problem with evaluator using this interface is that it allows a bleed out register access. This means that if the caller specifies a larger amount of data then the size of a specified register, the operation will continue accessing the neighboring registers until a full amount of data has been reached. DWARF specification does not define this behavior, so a new simplified register access interface is needed instead. * dwarf2/expr.c (read_from_register): New function. (write_to_register): New function. (rw_pieced_value): Now calls the read_from_register and write_to_register functions. Change-Id: I885b6a02353bc3fcf0dd5c600b5f1a73a7f9e340 |
||
|
Zoran Zaric
|
0dc32c82c3 |
Add as_lval argument to expression evaluator
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. Change-Id: I04000d8a506030c747a82cd14d1729a90339ebaf |
||
|
Zoran Zaric
|
e6bada58eb |
Simplify dwarf_expr_context class interface
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 - eval_exp 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 eval_exp 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. 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::eval_exp): New function. (dwarf_expr_context::execute_stack_op): Change to use property_addr_info structure. * dwarf2/expr.h (struct dwarf_expr_context): New eval_exp declaration. Change eval and fetch_result method to private. * dwarf2/frame.c (execute_stack_op): Change to call eval_exp method. * dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Change to call eval_exp method. (dwarf2_locexpr_baton_eval): Change to call eval_exp method. Change-Id: I8abd0dbf93e74a8a5adce143bfdea83c802233c3 |
||
|
Zoran Zaric
|
5e8cb1a0bb |
Make DWARF evaluator return a single struct value
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. Change-Id: I03829eb03de7b3dc38e06757d1fa02fc299e25a5 |
||
|
Zoran Zaric
|
17ac2b0a7e |
Move piece_closure and its support to expr.c
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. * dwarf2/loc.h (invalid_synthetic_pointer): Expose function. Change-Id: Ibe6b53db07dc0b6a80138870fa3187cdcce84597 |
||
|
Zoran Zaric
|
467ee74161 |
Merge evaluate_for_locexpr_baton evaluator
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. Change-Id: I7925e07fa93245ea542af81a9f9f0bfbdac7f309 |
||
|
Zoran Zaric
|
754848f25e |
Remove empty frame and full evaluators
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. Change-Id: Iadf2c52002f79a927f0a8327842163bfecfd2db6 |
||
|
Zoran Zaric
|
c5ce968925 |
Inline get_reg_value method of 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. Change-Id: I7bee8587d30983355d371f0769068f712cfbce1c |
||
|
Zoran Zaric
|
c729602de0 |
Move push_dwarf_reg_entry_value to expr.c
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.
Change-Id: I770c2eebaf27089384a26ea2f852f44ca2e2d8f9
|
||
|
Zoran Zaric
|
abcb8a73df |
Move read_mem to dwarf_expr_context
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. Change-Id: Ied28c2c0fc808fb5e41f9816df8ac731fa4ac2fa |
||
|
Zoran Zaric
|
de00629fea |
Move get_object_address 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. Change-Id: I67ee9e004af1f0d513d8832f10948c7cf22dd422 |
||
|
Zoran Zaric
|
0440f23844 |
Move dwarf_call 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. Change-Id: Ib0b6048e9b264e54cfdbd8dd76f91cd3f76af673 |
||
|
Zoran Zaric
|
3b3ac5a5fe |
Move compilation unit info to dwarf_expr_context
This patch moves the compilation unit context information and support from dwarf_expr_executor and dwarf_evaluate_loc_desc to dwarf_expr_context evaluator. The idea is to report an error when a given operation requires a compilation unit information to be resolved, which is not available. gdb/ChangeLog: * dwarf2/expr.c (ensure_have_per_cu): New function. (dwarf_expr_context::dwarf_expr_context): Add compilation unit context information. (dwarf_expr_context::get_base_type): Move from dwarf_evaluate_loc_desc. (dwarf_expr_context::get_addr_index): Remove method. (dwarf_expr_context::dwarf_variable_value): Remove method. (dwarf_expr_context::execute_stack_op): Call compilation unit context info check. Inline get_addr_index and dwarf_variable_value methods. * dwarf2/expr.h (struct dwarf_expr_context): Add compilation context info. Remove get_addr_index and dwarf_variable_value. * dwarf2/frame.c (dwarf_expr_executor::get_addr_index): Remove method. (dwarf_expr_executor::dwarf_variable_value): Remove method. * dwarf2/loc.c (sect_variable_value): Expose function. (dwarf_evaluate_loc_desc::get_addr_index): Remove method. (dwarf_evaluate_loc_desc::dwarf_variable_value): Remove method. (class dwarf_evaluate_loc_desc): Move compilation unit context information to dwarf_expr_context class. * dwarf2/loc.h (sect_variable_value): Expose function. Change-Id: Ib159fbbdffb25fa442cd6174d90e9522e8ccc9d1 |
||
|
Zoran Zaric
|
528991c044 |
Remove get_frame_cfa from dwarf_expr_context
Following the idea of merging the evaluators, the get_frame_cfa 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, it becomes apparent that the method is only called once and it can be inlined. It is also necessary to check if the frame context information was provided before the DW_OP_call_frame_cfa operation is executed. gdb/ChangeLog: * dwarf2/expr.c (dwarf_expr_context::get_frame_cfa): Remove method. (dwarf_expr_context::execute_stack_op): Call frame context info check for DW_OP_call_frame_cfa. Remove use of get_frame_cfa. * dwarf2/expr.h (dwarf_expr_context::get_frame_cfa): Remove method. * dwarf2/frame.c (dwarf_expr_context::get_frame_cfa): Remove method. * dwarf2/loc.c (dwarf_expr_context::get_frame_cfa): Remove method. Change-Id: I126a97a51bbb4c97f4ee669c96d054426bd79070 |
||
|
Zoran Zaric
|
da7e10b674 |
Move frame context info to dwarf_expr_context
Following 15 patches in this patch series is cleaning up the design of the DWARF expression evaluator (dwarf_expr_context) to make future extensions of that evaluator easier and cleaner to implement. There are three subclasses of the dwarf_expr_context class (dwarf_expr_executor, dwarf_evaluate_loc_desc and evaluate_for_locexpr_baton). Here is a short description of each class: - dwarf_expr_executor is evaluating a DWARF expression in a context of a Call Frame Information. The overridden methods of this subclass report an error if a specific DWARF operation, represented by that method, is not allowed in a CFI context. The source code of this subclass lacks the support for composite as well as implicit pointer location description. - dwarf_evaluate_loc_desc can evaluate any expression with no restrictions. All of the methods that this subclass overrides are actually doing what they are intended to do. This subclass contains a full support for all location description types. - evaluate_for_locexpr_baton subclass is a specialization of the dwarf_evaluate_loc_desc subclass and it's function is to add support for passed in buffers. This seems to be a way to go around the fact that DWARF standard lacks a bit offset support for memory location descriptions as well as using any location description for the push object address functionality. It all comes down to this question: what is a function of a DWARF expression evaluator? Is it to evaluate the expression in a given context or to check the correctness of that expression in that context? Currently, the only reason why there is a dwarf_expr_executor subclass is to report an invalid DWARF expression in a context of a CFI, but is that what the evaluator is supposed to do considering that the evaluator is not tied to a given DWARF version? There are more and more vendor and GNU extensions that are not part of the DWARF standard, so is it that impossible to expect that some of the extensions could actually lift the previously imposed restrictions of the CFI context? Not to mention that every new DWARF version is lifting some restrictions anyway. The thing that makes more sense for an evaluator to do, is to take the context of an evaluation and checks the requirements of every operation evaluated against that context. With this approach, the evaluator would report an error only if parts of the context, necessary for the evaluation, are missing. If this approach is taken, then the unification of the dwarf_evaluate_loc_desc, dwarf_expr_executor and dwarf_expr_context is the next logical step. This makes a design of the DWARF expression evaluator cleaner and allows more flexibility when supporting future vendor and GNU extensions. Additional benefit here is that now all evaluators have access to all location description types, which means that a vendor extended CFI rules could support composite location description as well. This also means that a new evaluator interface can be changed to return a single struct value (that describes the result of the evaluation) instead of a caller poking around the dwarf_expr_context internal data for answers (like it is done currently). This patch starts the merging proccess by moving the frame context information and support from dwarf_expr_executor and dwarf_evaluate_loc_desc to dwarf_expr_context evaluator. The idea is to report an error when a given operation requires a frame information to be resolved, if that information is not present. gdb/ChangeLog: * dwarf2/expr.c (ensure_have_frame): New function. (read_addr_from_reg): Add from frame.c. (dwarf_expr_context::dwarf_expr_context): Add frame info to dwarf_expr_context. (dwarf_expr_context::read_addr_from_reg): Remove. (dwarf_expr_context::get_reg_value): Move from dwarf_evaluate_loc_desc. (dwarf_expr_context::get_frame_base): Move from dwarf_evaluate_loc_desc. (dwarf_expr_context::execute_stack_op): Call frame context info check. Remove use of read_addr_from_reg method. * dwarf2/expr.h (struct dwarf_expr_context): Add frame info member, read_addr_from_reg, get_reg_value and get_frame_base declaration. (read_addr_from_reg): Move to expr.c. * dwarf2/frame.c (read_addr_from_reg): Move to dwarf_expr_context. (dwarf_expr_executor::read_addr_from_reg): Remove. (dwarf_expr_executor::get_frame_base): Remove. (dwarf_expr_executor::get_reg_value): Remove. (execute_stack_op): Use read_addr_from_reg function instead of read_addr_from_reg method. * dwarf2/loc.c (dwarf_evaluate_loc_desc::get_frame_base): Move to dwarf_expr_context. (dwarf_evaluate_loc_desc::get_reg_value): Move to dwarf_expr_context. (dwarf_evaluate_loc_desc::read_addr_from_reg): Remove. (dwarf2_locexpr_baton_eval):Use read_addr_from_reg function instead of read_addr_from_reg method. Change-Id: Ic0eb7266933e33330f3feff9140a1d0630c1a589 |
||
|
Zoran Zaric
|
a358e4f261 |
Replace the symbol needs evaluator with a parser
This patch addresses a design problem with the symbol_needs_eval_context
class. It exposes the problem by introducing two new testsuite test
cases.
To explain the issue, I first need to explain the dwarf_expr_context
class that the symbol_needs_eval_context class derives from.
The intention behind the dwarf_expr_context class is to commonize the
DWARF expression evaluation mechanism for different evaluation
contexts. Currently in gdb, the evaluation context can contain some or
all of the following information: architecture, object file, frame and
compilation unit.
Depending on the information needed to evaluate a given expression,
there are currently three distinct DWARF expression evaluators:
- Frame: designed to evaluate an expression in the context of a call
frame information (dwarf_expr_executor class). This evaluator doesn't
need a compilation unit information.
- Location description: designed to evaluate an expression in the
context of a source level information (dwarf_evaluate_loc_desc
class). This evaluator expects all information needed for the
evaluation of the given expression to be present.
- Symbol needs: designed to answer a question about the parts of the
context information required to evaluate a DWARF expression behind a
given symbol (symbol_needs_eval_context class). This evaluator
doesn't need a frame information.
The functional difference between the symbol needs evaluator and the
others is that this evaluator is not meant to interact with the actual
target. Instead, it is supposed to check which parts of the context
information are needed for the given DWARF expression to be evaluated by
the location description evaluator.
The idea is to take advantage of the existing dwarf_expr_context
evaluation mechanism and to fake all required interactions with the
actual target, by returning back dummy values. The evaluation result is
returned as one of three possible values, based on operations found in a
given expression:
- SYMBOL_NEEDS_NONE,
- SYMBOL_NEEDS_REGISTERS and
- SYMBOL_NEEDS_FRAME.
The problem here is that faking results of target interactions can yield
an incorrect evaluation result.
For example, if we have a conditional DWARF expression, where the
condition depends on a value read from an actual target, and the true
branch of the condition requires a frame information to be evaluated,
while the false branch doesn't, fake target reads could conclude that a
frame information is not needed, where in fact it is. This wrong
information would then cause the expression to be actually evaluated (by
the location description evaluator) with a missing frame information.
This would then crash the debugger.
The gdb.dwarf2/symbol_needs_eval_fail.exp test introduces this
scenario, with the following DWARF expression:
DW_OP_addr $some_variable
DW_OP_deref
DW_OP_bra 4 # conditional jump to DW_OP_bregx
DW_OP_lit0
DW_OP_skip 3 # jump to DW_OP_stack_value
DW_OP_bregx $dwarf_regnum 0
DW_OP_stack_value
This expression describes a case where some variable dictates the
location of another variable. Depending on a value of some_variable, the
variable whose location is described by this expression is either read
from a register or it is defined as a constant value 0. In both cases,
the value will be returned as an implicit location description on the
DWARF stack.
Currently, when the symbol needs evaluator fakes a memory read from the
address behind the some_variable variable, the constant value 0 is used
as the value of the variable A, and the check returns the
SYMBOL_NEEDS_NONE result.
This is clearly a wrong result and it causes the debugger to crash.
The scenario might sound strange to some people, but it comes from a
SIMD/SIMT architecture where $some_variable is an execution mask. In
any case, it is a valid DWARF expression, and GDB shouldn't crash while
evaluating it. Also, a similar example could be made based on a
condition of the frame base value, where if that value is concluded to
be 0, the variable location could be defaulted to a TLS based memory
address.
The gdb.dwarf2/symbol_needs_eval_timeout.exp test introduces a second
scenario. This scenario is a bit more abstract due to the DWARF
assembler lacking the CFI support, but it exposes a different
manifestation of the same problem. Like in the previous scenario, the
DWARF expression used in the test is valid:
DW_OP_lit1
DW_OP_addr $some_variable
DW_OP_deref
DW_OP_skip 4 # jump to DW_OP_fbreg
DW_OP_drop
DW_OP_fbreg 0
DW_OP_dup
DW_OP_lit0
DW_OP_eq
DW_OP_bra -9 # conditional jump to DW_OP_drop
DW_OP_stack_value
Similarly to the previous scenario, the location of a variable A is an
implicit location description with a constant value that depends on a
value held by a global variable. The difference from the previous case
is that DWARF expression contains a loop instead of just one branch. The
end condition of that loop depends on the expectation that a frame base
value is never zero. Currently, the act of faking the target reads will
cause the symbol needs evaluator to get stuck in an infinite loop.
Somebody could argue that we could change the fake reads to return
something else, but that would only hide the real problem.
The general impression seems to be that the desired design is to have
one class that deals with parsing of the DWARF expression, while there
are virtual methods that deal with specifics of some operations.
Using an evaluator mechanism here doesn't seem to be correct, because
the act of evaluation relies on accessing the data from the actual
target with the possibility of skipping the evaluation of some parts of
the expression.
To better explain the proposed solution for the issue, I first need to
explain a couple more details behind the current design:
There are multiple places in gdb that handle DWARF expression parsing
for different purposes. Some are in charge of converting the expression
to some other internal representation (decode_location_expression,
disassemble_dwarf_expression and dwarf2_compile_expr_to_ax), some are
analysing the expression for specific information
(compute_stack_depth_worker) and some are in charge of evaluating the
expression in a given context (dwarf_expr_context::execute_stack_op
and decode_locdesc).
The problem is that all those functions have a similar (large) switch
statement that handles each DWARF expression operation. The result of
this is a code duplication and harder maintenance.
As a step into the right direction to solve this problem (at least for
the purpose of a DWARF expression evaluation) the expression parsing was
commonized inside of an evaluator base class (dwarf_expr_context). This
makes sense for all derived classes, except for the symbol needs
evaluator (symbol_needs_eval_context) class.
As described previously the problem with this evaluator is that if the
evaluator is not allowed to access the actual target, it is not really
evaluating.
Instead, the desired function of a symbol needs evaluator seems to fall
more into expression analysis category. This means that a more natural
fit for this evaluator is to be a symbol needs analysis, similar to the
existing compute_stack_depth_worker analysis.
Another problem is that using a heavyweight mechanism of an evaluator
to do an expression analysis seems to be an unneeded overhead. It also
requires a more complicated design of the parent class to support fake
target reads.
The reality is that the whole symbol_needs_eval_context class can be
replaced with a lightweight recursive analysis function, that will give
more correct result without compromising the design of the
dwarf_expr_context class.
The downside of this approach is adding of one more similar switch
statement, but at least this way the new symbol needs analysis will be
a lightweight mechnism and it will provide a correct result for any
given DWARF expression.
A more desired long term design would be to have one class that deals
with parsing of the DWARF expression, while there would be a virtual
methods that deal with specifics of some DWARF operations. Then that
class would be used as a base for all DWARF expression parsing mentioned
at the beginning.
This however, requires a far bigger changes that are out of the scope
of this patch series.
To support the new implementation, I have also added two new self tests,
found in a file dwarf2/loc.c (named symbol_needs_cond_nonzero and
symbol_needs_cond_zero), which expose a similar problem to the one
described in the first testsuite test case. The difference is that the
global some_variable is replaced with a use of a
DW_OP_push_object_address operation. The idea is for the symbol needs
evaluator to return the SYMBOL_NEEDS_FRAME value, regardless of the
evaluation of that operation.
The new analysis requires the DWARF location description for the
argc argument of the niam function to change in the assembly file
gdb.python/amd64-py-framefilter-invalidarg.S. Originally, expression
ended with a 0 value byte, which was never reached by the symbol needs
evaluator, because it was detecting a stack underflow when evaluating
the operation before. The new approach does not simulate a DWARF
stack anymore, so the 0 value byte needs to be removed because it
makes the DWARF expression invalid.
gdb/ChangeLog:
* dwarf2/loc.c (class symbol_needs_eval_context): Remove.
(dwarf2_get_symbol_read_needs): New function.
(dwarf2_loc_desc_get_symbol_read_needs): Remove.
(locexpr_get_symbol_read_needs): Use
dwarf2_get_symbol_read_needs.
(symbol_needs_cond_zero): New function.
(symbol_needs_cond_nonzero): New function.
(_initialize_dwarf2loc): Register selftests.
gdb/testsuite/ChangeLog:
* gdb.python/amd64-py-framefilter-invalidarg.S : Update argc
DWARF location expression.
* lib/dwarf.exp (_location): Handle DW_OP_fbreg.
* gdb.dwarf2/symbol_needs_eval.c: New file.
* gdb.dwarf2/symbol_needs_eval_fail.exp: New file.
* gdb.dwarf2/symbol_needs_eval_timeout.exp: New file.
Change-Id: I6f18c7cd4c5a7733e0e4372002ec2d2ef44c5601
|
||
|
Alexander Fedotov
|
60a7223fdd |
gdbsupport: Use LOCALAPPDATA to determine cache dir
Use the LOCALAPPDATA environment variable to determine the cache dir when running on Windows with native command line, otherwise nasty warning "Couldn't determine a path for index cached directory" appears. Change-Id: I77903f9f0cb4743555866b8aea892cef55132589 |
||
|
Tom de Vries
|
b46551b20c |
[gdb/testsuite] Simplify gdb.arch/amd64-gs_base.exp
Redo fix committed in commit |
||
|
Tom de Vries
|
8439f446a1 |
[gdb/testsuite] Fix gdb.ada/mi_task_arg.exp for -m32
When running test-case gdb.ada/mi_task_arg.exp with target board unix/-m32, I
run into:
...
(gdb) ^M
Expecting: ^(-stack-list-arguments 1[^M
]+)?(\^done,stack-args=\[ \
frame={level="0",args=\[\]}, \
frame={level="1",args=\[{name="<_task>",value="0x[0-9A-Fa-f]+"}\]}, \
frame={level="2",args=\[({name="self_id",value="0x[0-9A-Fa-f]+"})?\]},.*[^M
]+[(]gdb[)] ^M
[ ]*)
-stack-list-arguments 1^M
^done,stack-args=[ \
frame={level="0",args=[]}, \
frame={level="1",args=[{name="<_task>",value="0x808abf0"}]}, \
frame={level="2",args=[{name="self_id",value="<optimized out>"}]}, \
frame={level="3",args=[]},frame={level="4",args=[]}]^M
(gdb) ^M
FAIL: gdb.ada/mi_task_arg.exp: -stack-list-arguments 1 (unexpected output)
...
The problem is that we're expecting a $hex for the value of self_id, but
instead get <optimized out>.
Looking at the debug info for self_id:
...
<1><12a1f>: Abbrev Number: 84 (DW_TAG_subprogram)
<12a20> DW_AT_name : system__tasking__stages__task_wrapper
...
<2><12a35>: Abbrev Number: 61 (DW_TAG_formal_parameter)
<12a36> DW_AT_name : self_id
<12a40> DW_AT_location : 0x459e (location list)
...
it refers to location information here:
...
0000459e 08053060 080531ac (DW_OP_fbreg: 0)
000045aa 0805327c 080532a5 (DW_OP_fbreg: 0)
000045b6 08053320 08053324 (DW_OP_fbreg: 0)
...
while the pc used to retrieve the location information is 0x080531c5:
...
$ gdb -batch outputs/gdb.ada/mi_task_arg/task_switch \
-ex "break 57" -ex run -ex bt
...
#0 task_switch.break_me () at task_switch.adb:57
#1 0x0804aaae in task_switch.caller (<_task>=0x808abf0) \
at task_switch.adb:51
#2 0x080531c5 in system.tasking.stages.task_wrapper \
(self_id=<optimized out>) at s-tassta.adb:1295
...
which indeed falls outside of the ranges listed in the location info.
Fix this by accepting <optimized out> as valid value of self_id.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-12-08 Tom de Vries <tdevries@suse.de>
* gdb.ada/mi_task_arg.exp: Accept <optimized out> as valid value of
self_id.
|
||
|
GDB Administrator
|
28e36bf890 | Automatic date update in version.in | ||
|
Pedro Alves
|
bc545da73f |
gdb.base/break-on-linker-gcd-function.exp: Remove unused variable
Commit:
commit
|
||
|
Nick Clifton
|
846141822b |
Remove references to the unofficial SHF_GNU_BUILD_NOTE section flag.
binutils * objcopy.c (is_mergeable_note_section): Remove reference to SHF_GNU_BUILD_NOTE. include * elf/common.h (SHF_GNU_BUILD_NOTE): Delete. |
||
|
Siddhesh Poyarekar
|
014cc7f849 |
binutils: Make smart_rename safe too
smart_rename is capable of handling symlinks by copying and it also tries to preserve ownership and permissions of files when they're overwritten during the rename. This is useful in objcopy where the file properties need to be preserved. However because smart_rename does this using file names, it leaves a race window between renames and permission fixes. This change removes this race window by using file descriptors from the original BFDs that were used to manipulate these files wherever possible. The file that is to be renamed is also passed as a file descriptor so that we use fchown/fchmod on the file descriptor, thus making sure that we only modify the file we have opened to write. Further, in case the file is to be overwritten (as is the case in ar or objcopy), the permissions that need to be restored are taken from the file descriptor that was opened for input so that integrity of the file status is maintained all the way through to the rename. binutils/ * rename.c * ar.c (write_archive) [!defined (_WIN32) || defined (__CYGWIN32__)]: Initialize TARGET_STAT and OFD to pass to SMART_RENAME. * arsup.c (ar_save) [defined (_WIN32) || defined (__CYGWIN32__)]: Likewise. * bucomm.h (smart_rename): Add new arguments to declaration. * objcopy.c (strip_main)[defined (_WIN32) || defined (__CYGWIN32__)]: Initialize COPYFD and pass to SMART_RENAME. (copy_main) [defined (_WIN32) || defined (__CYGWIN32__)]: Likewise. * rename.c (try_preserve_permissions): New function. (smart_rename): Use it and add new arguments. |
||
|
Siddhesh Poyarekar
|
1a1c3b4cc1 |
objcopy: Get input file stat after BFD open
Get file state from the descriptor opened by copy_file for the input BFD. This ensures continuity in the view of the input file through the descriptor. At the moment it is only to preserve timestamps recorded at the point that we opened the file for input but in the next patch this state will also be used to preserve ownership and permissions wherever applicable. binutils/ * objcopy.c (copy_file): New argument IN_STAT. Return stat of ibfd through it. (strip_main): Remove redundant stat calls. adjust copy_file calls. (copy_main): Likewise. |
||
|
Siddhesh Poyarekar
|
365f5fb6d0 |
binutils: Use file descriptors from make_tempname
The purpose of creating a temporary file securely using mkstemp is defeated if it is closed in make_tempname and reopened later for use; it is as good as using mktemp. Get the file descriptor instead and then use it to create the BFD object. bfd/ * opncls.c (bfd_fdopenw): New function. * bfd-in2.h: Regenerate. binutils/ * bucomm.c (make_tempname): Add argument to return file descriptor. * bucomm.h (make_tempname): Likewise. * ar.c: Include libbfd.h. (write_archive): Adjust for change in make_tempname. Call bfd_fdopenw instead of bfd_openw. * objcopy.c: Include libbfd.h. (copy_file): New argument OFD. Use bfd_fdopenw instead of bfd_openw. (strip_main): Adjust for change in make_tempname and copy_file. (copy_main): Likewise. |
||
|
Tom Tromey
|
a4915e8d6c |
Use expression completer for "maint print type"
I happened to notice that expression completion did not work correctly for "maint print type". This patch adds the appropriate completer there. gdb/ChangeLog 2020-12-07 Tom Tromey <tromey@adacore.com> * maint.c (_initialize_maint_cmds): Use expression command completer for "maint print type". |
||
|
Alan Modra
|
cd8d2039b0 |
[GOLD] gcc-11 stringop-overflow warning
I'm unsure why this is deserving of a warning. Not writing the most efficient code surely can't be a real problem, but that is what https://gcc.gnu.org/bugzilla//show_bug.cgi?id=88059#c1 seems to say. plugin.cc:528:10: error: 'char* strncpy(char*, const char*, size_t)' specified bound depends on the length of the source argument [-Werror=stringop-overflow=] 528 | strncpy(tempdir, dir_template, len); | ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~ plugin.cc:526:22: note: length computed here 526 | size_t len = strlen(dir_template) + 1; | ~~~~~~^~~~~~~~~~~~~~ * plugin.cc (Plugin_recorder::init): Replace strncpy with memcpy. |
||
|
Alan Modra
|
fde0214a91 |
elf32-csky.c:3932:19: error: comparison is always false
It looks like csky missed out on an edit for
|
||
|
Nick Clifton
|
a315d3902d | README-how-to-make-a-release (point releases): Add a note to update the milestone list on sourceware's bugzilla. | ||
|
Tankut Baris Aktemur
|
1f58f6c259 |
gdb/completer: improve tab completion to consider the '-force-condition' flag
The commit
commit
|
||
|
Tankut Baris Aktemur
|
5759831a2d |
gdb/linespec: relax the position of the '-force-condition' flag
The break command's "-force-condition" flag is currently required to be followed by the "if" keyword. This prevents flexibility when using other keywords, e.g. "thread": (gdb) break main -force-condition thread 1 if foo Function "main -force-condition" not defined. Make breakpoint pending on future shared library load? (y or [n]) n Remove the requirement that "-force-condition" is always followed by an "if", so that more flexibility is obtained when positioning keywords. gdb/ChangeLog: 2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> * linespec.c (linespec_lexer_lex_keyword): The "-force-condition" keyword may be followed by any keyword. * breakpoint.c (find_condition_and_thread): Advance 'tok' by 'toklen' in the case for "-force-condition". gdb/testsuite/ChangeLog: 2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> * gdb.linespec/keywords.exp: Add tests to check positional flexibility of "-force-condition". |
||
|
Tankut Baris Aktemur
|
21e051b3d6 |
gdb/main: execute breakpoint commands for '-iex' and '-ex' commands
Suppose we have the script file below:
break main
commands
print 123
end
run
If started with this script file, GDB executes the breakpoint command:
$ gdb -q -x myscript --args ./test
Reading symbols from ./test...
Breakpoint 1 at 0x114e: file test.c, line 2.
Breakpoint 1, main () at test.c:2
2 return 0;
$1 = 123
(gdb)
However, if we remove the "run" line from the script and pass it with
the '-ex' option instead, the command is not executed:
$ gdb -q -x myscript_no_run --args ./test
Reading symbols from ./test...
Breakpoint 1 at 0x114e: file test.c, line 2.
Starting program: /path/to/test
Breakpoint 1, main () at test.c:2
2 return 0;
(gdb)
If the user enters a command at this point, the breakpoint command
is executed, yielding weird output:
$ gdb -q -x myscript_no_run --args ./test
Reading symbols from ./test...
Breakpoint 1 at 0x114e: file test.c, line 2.
Starting program: /path/to/test
Breakpoint 1, main () at test.c:2
2 return 0;
(gdb) print "a"
$1 = "a"
$2 = 123
When consuming script files, GDB runs bp actions after executing a
command. See `command_handler` in event-top.c:
if (c[0] != '#')
{
execute_command (command, ui->instream == ui->stdin_stream);
/* Do any commands attached to breakpoint we stopped at. */
bpstat_do_actions ();
}
However, for '-ex' commands, `bpstat_do_actions` is not invoked.
Hence, the misaligned output explained above occurs. To fix the
problem, add a call to `bpstat_do_actions` after executing a command.
gdb/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* main.c (catch_command_errors): Add a flag parameter; invoke
`bpstat_do_actions` if the flag is set.
(execute_cmdargs): Update a call to `catch_command_errors`.
gdb/testsuite/ChangeLog:
2020-12-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.base/bp-cmds-run-with-ex.c: New file.
* gdb.base/bp-cmds-run-with-ex.exp: New file.
* gdb.base/bp-cmds-run-with-ex.gdb: New file.
* gdb.gdb/python-interrupts.exp: Update the call to
'catch_command_errors' with the new argument.
* gdb.gdb/python-selftest.exp: Ditto.
|
||
|
Tom de Vries
|
f51f9f1d03 |
[gdb/ada] Handle shrink resize in replace_operator_with_call
In replace_operator_with_call, we resize the elts array like this:
...
exp->nelts = exp->nelts + 7 - oplen;
exp->resize (exp->nelts);
...
Although all the current callers ensure that the new size is bigger, it could
also be smaller, in which case the following memmove possibly reads out of
bounds:
...
memmove (exp->elts + pc + 7, exp->elts + pc + oplen,
EXP_ELEM_TO_BYTES (save_nelts - pc - oplen));
...
Fix this by doing the resize after the memmove in case the new size is
smaller.
Tested on x86_64-linux.
gdb/ChangeLog:
2020-12-07 Tom de Vries <tdevries@suse.de>
* ada-lang.c (replace_operator_with_call): Handle shrink resize.
|
||
|
Tom Tromey
|
00158a68d1 |
Fix struct expression regression
The patch to change struct expression to use new introduced a regression -- there is a spot that reallocates expressions that I failed to update. This patch rewrites this code to follow the new approach. Now the rewriting is done in place. gdb/ChangeLog 2020-12-06 Tom Tromey <tom@tromey.com> PR ada/26999 * ada-lang.c (replace_operator_with_call): Rewrite. |
||
|
GDB Administrator
|
13f11b0b61 | Automatic date update in version.in | ||
|
Giancarlo Frix
|
296cfb8889 |
s390: Fix BC instruction breakpoint handling
This fixes a long-lived bug in the s390 port.
When trying to step over a breakpoint set on a BC (branch on condition)
instruction with displaced stepping on IBM Z, gdb would incorrectly
adjust the pc regardless of whether or not the branch was taken. Since
the branch target is an absolute address, this would cause the inferior
to jump around wildly whenever the branch was taken, either crashing it
or causing it to behave unpredictably.
It turns out that the logic to handle BC instructions correctly was in
the code, but that the enum value representing its opcode has always
been incorrect.
This patch corrects the enum value to the actual opcode, fixing the
stepping problem. The enum value is also used in the prologue analysis
code, so this also fixes a minor bug where more of the prologue would
be read than was necessary.
gdb/ChangeLog:
PR breakpoints/27009
* s390-tdep.h (op_bc): Correct BC opcode value.
|
||
|
Joel Brobecker
|
63c457b911 |
gmp-utils: protect gdb_mpz exports against out-of-range values
The gdb_mpz class currently provides a couple of methods which
essentially export an mpz_t value into either a buffer, or an integral
type. The export is based on using the mpz_export function which
we discovered can be a bit treacherous if used without caution.
In particular, the initial motivation for this patch was to catch
situations where the mpz_t value was so large that it would not fit
in the destination area. mpz_export does not know the size of
the buffer, and therefore can happily write past the end of our buffer.
While designing a solution to the above problem, I also discovered
that we also needed to be careful when exporting signed numbers.
In particular, numbers which are larger than the maximum value
for a given signed type size, but no so large as to fit in the
*unsigned* version with the same size, would end up being exported
incorrectly. This is related to the fact that mpz_export ignores
the sign of the value being exportd, and assumes an unsigned export.
Thus, for such large values, the appears as if mpz_export is able
to fit our value into our buffer, but in fact, it does not.
Also, I noticed that gdb_mpz::write wasn't taking its unsigned_p
parameter, which was a hole.
For all these reasons, a new low-level private method called
"safe_export" has been added to class gdb_mpz, whose goal is
to perform all necessary checks and manipulations for a safe
and correct export. As a bonus, this method allows us to factorize
the handling of negative value exports.
The gdb_mpz::as_integer and gdb_mpz::write methods are then simplified
to take advantage of this new safe_export method.
gdb/ChangeLog:
* gmp-utils.h (gdb_mpz::safe_export): New private method.
(gdb_mpz::as_integer): Reimplement using gdb_mpz::safe_export.
* gmp-utils.c (gdb_mpz::write): Rewrite using gdb_mpz::safe_export.
(gdb_mpz::safe_export): New method.
* unittests/gmp-utils-selftests .c (gdb_mpz_as_integer):
Update function description.
(check_as_integer_raises_out_of_range_error): New function.
(gdb_mpz_as_integer_out_of_range): New function.
(_initialize_gmp_utils_selftests): Register
gdb_mpz_as_integer_out_of_range as a selftest.
|
||
|
GDB Administrator
|
6b1dce3a3d | Automatic date update in version.in | ||
|
Maciej W. Rozycki
|
0fcf331bb1 |
VAX/BFD: Do not warn about GOT addend mismatches if no GOT entry is made
Match the condition used in `elf_vax_instantiate_got_entries' for the
creation of GOT entries in the processing of R_VAX_GOT32 relocations in
`elf_vax_check_relocs', removing incorrect warnings about a GOT addend
mismatch like:
./ld-new: tmpdir/got-local-ref-off-r.o: warning: GOT addend of 1 to `bar_hidden' does not match previous GOT addend of 0
./ld-new: tmpdir/got-local-ref-off-r.o: warning: GOT addend of 2 to `bar_hidden' does not match previous GOT addend of 0
and corresponding failures with the test cases newly added here:
FAIL: GOT test (executable hidden reference with offset)
FAIL: GOT test (executable visible reference with offset)
for symbols that are considered local for reasons other than having been
forced local with a version script, which is usually the ELF visibility.
Correct code is produced regardless, but the warning breaks `-Werror'
compilation and may upset people regardless.
Interestingly this shows with executable links only, because in shared
library links code from `elf_link_add_object_symbols' triggers:
/* If the symbol already has a dynamic index, but
visibility says it should not be visible, turn it into
a local symbol. */
switch (ELF_ST_VISIBILITY (h->other))
{
case STV_INTERNAL:
case STV_HIDDEN:
(*bed->elf_backend_hide_symbol) (info, h, TRUE);
dynsym = FALSE;
break;
}
that sets `h->forced_local' like with a version script.
Add suitable test cases including disassembly to verify correct code has
been produced where no warnings have been issued, and that warnings do
get issued where necessary. Do not verify (broken) code produced in the
latter case; we should probably make the warning an error, or preferably
actually start supporting GOT references with different addends as they
appear feasible with explicitly relocated GOT that we use.
bfd/
* elf32-vax.c (elf_vax_check_relocs) <R_VAX_GOT32>: Use
SYMBOL_REFERENCES_LOCAL rather than `h->forced_local' to check
whether the symbol referred is local or not.
ld/
* testsuite/ld-vax-elf/got-local-exe-off-hidden.dd: New test
dump.
* testsuite/ld-vax-elf/got-local-exe-off-visible.dd: New test
dump.
* testsuite/ld-vax-elf/got-local-lib-off-hidden.dd: New test
dump.
* testsuite/ld-vax-elf/got-local-lib-off-visible.ed: New test
dump.
* testsuite/ld-vax-elf/got-local-off-external.ed: New test dump.
* testsuite/ld-vax-elf/got-local-exe-off.xd: New test dump.
* testsuite/ld-vax-elf/got-local-lib-off.xd: New test dump.
* testsuite/ld-vax-elf/got-local.ld: New test linker script.
* testsuite/ld-vax-elf/got-local-aux-off.s: New test source.
* testsuite/ld-vax-elf/got-local-def-off.s: New test source.
* testsuite/ld-vax-elf/got-local-ref-off-external.s: New test
source.
* testsuite/ld-vax-elf/got-local-ref-off-hidden.s: New test
source.
* testsuite/ld-vax-elf/got-local-ref-off-visible.s: New test
source.
* testsuite/ld-vax-elf/vax-elf.exp: Run the new tests.
|
||
|
Joel Brobecker
|
3c7ba803ac |
Fix TARGET_CHAR_BIT/HOST_CHAR_BIT confusion in gmp-utils.c
In a couple of gdb_mpz methods, we are computing the number of
bits in a gdb::array_view of gdb_byte. Since gdb_byte is defined
using a host-side type (see common-types.h), the number of bits
in a gdb_byte should be HOST_CHAR_BIT, not TARGET_CHAR_BIT.
gdb/ChangeLog:
* gmp-utils.c (gdb_mpz::read): Use HOST_CHAR_BIT instead of
TARGET_CHAR_BIT.
(gdb_mpz::write): Likewise.
|
||
|
H.J. Lu
|
7e45e7a9ab |
x86-64: Convert load to mov only for GOTPCRELX relocations
Since converting load to mov needs to rewrite the REX byte and we don't know if there is a REX byte with GOTPCREL relocation, do it only for GOTPCRELX relocations. bfd/ PR ld/27016 * elf64-x86-64.c (elf_x86_64_convert_load_reloc): Convert load to mov only for GOTPCRELX relocations. ld/ PR ld/27016 * testsuite/ld-x86-64/x86-64.exp: Run pr27016a and pr27016b. * testsuite/ld-x86-64/pr27016a.d: New file. * testsuite/ld-x86-64/pr27016a.s: Likewise. * testsuite/ld-x86-64/pr27016b.d: Likewise. * testsuite/ld-x86-64/pr27016b.s: Likewise. |
||
|
GDB Administrator
|
4979ae6a9e | Automatic date update in version.in | ||
|
Simon Marchi
|
372ff58fda |
gdb: use two displaced step buffers on amd64/Linux
As observed on a binary compiled on AMD64 Ubuntu 20.04, against glibc
2.31 (I think it's the libc that provides this startup code, right?),
there are enough bytes at the executable's entry point to hold more than
one displaced step buffer. gdbarch_max_insn_length is 16, and the
code at _start looks like:
0000000000001040 <_start>:
1040: f3 0f 1e fa endbr64
1044: 31 ed xor %ebp,%ebp
1046: 49 89 d1 mov %rdx,%r9
1049: 5e pop %rsi
104a: 48 89 e2 mov %rsp,%rdx
104d: 48 83 e4 f0 and $0xfffffffffffffff0,%rsp
1051: 50 push %rax
1052: 54 push %rsp
1053: 4c 8d 05 56 01 00 00 lea 0x156(%rip),%r8 # 11b0 <__libc_csu_fini>
105a: 48 8d 0d df 00 00 00 lea 0xdf(%rip),%rcx # 1140 <__libc_csu_init>
1061: 48 8d 3d c1 00 00 00 lea 0xc1(%rip),%rdi # 1129 <main>
1068: ff 15 72 2f 00 00 callq *0x2f72(%rip) # 3fe0 <__libc_start_main@GLIBC_2.2.5>
106e: f4 hlt
106f: 90 nop
The two buffers would occupy [0x1040, 0x1060).
I checked on Alpine, which uses the musl C library, the startup code
looks like:
0000000000001048 <_start>:
1048: 48 31 ed xor %rbp,%rbp
104b: 48 89 e7 mov %rsp,%rdi
104e: 48 8d 35 e3 2d 00 00 lea 0x2de3(%rip),%rsi # 3e38 <_DYNAMIC>
1055: 48 83 e4 f0 and $0xfffffffffffffff0,%rsp
1059: e8 00 00 00 00 callq 105e <_start_c>
000000000000105e <_start_c>:
105e: 48 8b 37 mov (%rdi),%rsi
1061: 48 8d 57 08 lea 0x8(%rdi),%rdx
1065: 45 31 c9 xor %r9d,%r9d
1068: 4c 8d 05 47 01 00 00 lea 0x147(%rip),%r8 # 11b6 <_fini>
106f: 48 8d 0d 8a ff ff ff lea -0x76(%rip),%rcx # 1000 <_init>
1076: 48 8d 3d 0c 01 00 00 lea 0x10c(%rip),%rdi # 1189 <main>
107d: e9 9e ff ff ff jmpq 1020 <__libc_start_main@plt>
Even though there's a _start_c symbol, it all appears to be code that
runs once at the very beginning of the program, so it looks fine if the
two buffers occupy [0x1048, 0x1068).
One important thing I discovered while doing this is that when debugging
a dynamically-linked executable, breakpoints in the shared library
loader are hit before executing the _start code, and these breakpoints
may be displaced-stepped. So it's very important that the buffer bytes
are restored properly after doing the displaced steps, otherwise the
_start code will be corrupted once we try to execute it.
Another thing that made me think about is that library constructors (as
in `__attribute__((constructor))`) run before _start. And they are free
to spawn threads. What if one of these threads executes a displaced
step, therefore changing the bytes at _start, while the main thread
executes _start? That doesn't sound good and I don't know how we could
prevent it. But this is a problem that predates the current patch.
Even when stress-testing the implementation, by making many threads do
displaced steps over and over, I didn't see a significant performance (I
confirmed that the two buffers were used by checking the "set debug
displaced" logs though). However, this patch mostly helps make the
feature testable by anybody with an AMD64/Linux machine, so I think it's
useful.
gdb/ChangeLog:
* amd64-linux-tdep.c (amd64_linux_init_abi): Pass 2 as the
number of displaced step buffers.
Change-Id: Ia0c96ea0fcda893f4726df6fdac7be5214620112
|
||
|
Simon Marchi
|
480af54cf6 |
gdb: make displaced stepping implementation capable of managing multiple buffers
The displaced_step_buffer class, introduced in the previous patch, manages access to a single displaced step buffer. Change it into displaced_step_buffers (note the plural), which manages access to multiple displaced step buffers. When preparing a displaced step for a thread, it looks for an unused buffer. For now, all users still pass a single displaced step buffer, so no real behavior change is expected here. The following patch makes a user pass more than one buffer, so the functionality introduced by this patch is going to be useful in the next one. gdb/ChangeLog: * displaced-stepping.h (struct displaced_step_buffer): Rename to... (struct displaced_step_buffers): ... this. <m_addr, m_current_thread, m_copy_insn_closure>: Remove. <struct displaced_step_buffer>: New inner class. <m_buffers>: New. * displaced-stepping.c (displaced_step_buffer::prepare): Rename to... (displaced_step_buffers::prepare): ... this, adjust for multiple buffers. (displaced_step_buffer::finish): Rename to... (displaced_step_buffers::finish): ... this, adjust for multiple buffers. (displaced_step_buffer::copy_insn_closure_by_addr): Rename to... (displaced_step_buffers::copy_insn_closure_by_addr): ... this, adjust for multiple buffers. (displaced_step_buffer::restore_in_ptid): Rename to... (displaced_step_buffers::restore_in_ptid): ... this, adjust for multiple buffers. * linux-tdep.h (linux_init_abi): Change supports_displaced_step for num_disp_step_buffers. * linux-tdep.c (struct linux_gdbarch_data) <num_disp_step_buffers>: New field. (struct linux_info) <disp_step_buf>: Rename to... <disp_step_bufs>: ... this, change type to displaced_step_buffers. (linux_displaced_step_prepare): Use linux_gdbarch_data::num_disp_step_buffers to create that number of buffers. (linux_displaced_step_finish): Adjust. (linux_displaced_step_copy_insn_closure_by_addr): Adjust. (linux_displaced_step_restore_all_in_ptid): Adjust. (linux_init_abi): Change supports_displaced_step parameter for num_disp_step_buffers, save it in linux_gdbarch_data. * aarch64-linux-tdep.c (aarch64_linux_init_abi): Adjust. * alpha-linux-tdep.c (alpha_linux_init_abi): Adjust. * amd64-linux-tdep.c (amd64_linux_init_abi_common): Change supports_displaced_step parameter for num_disp_step_buffers. (amd64_linux_init_abi): Adjust. (amd64_x32_linux_init_abi): Adjust. * arc-linux-tdep.c (arc_linux_init_osabi): Adjust. * arm-linux-tdep.c (arm_linux_init_abi): Adjust. * bfin-linux-tdep.c (bfin_linux_init_abi): Adjust. * cris-linux-tdep.c (cris_linux_init_abi): Adjust. * csky-linux-tdep.c (csky_linux_init_abi): Adjust. * frv-linux-tdep.c (frv_linux_init_abi): Adjust. * hppa-linux-tdep.c (hppa_linux_init_abi): Adjust. * i386-linux-tdep.c (i386_linux_init_abi): Adjust. * ia64-linux-tdep.c (ia64_linux_init_abi): Adjust. * m32r-linux-tdep.c (m32r_linux_init_abi): Adjust. * m68k-linux-tdep.c (m68k_linux_init_abi): * microblaze-linux-tdep.c (microblaze_linux_init_abi): * mips-linux-tdep.c (mips_linux_init_abi): Adjust. * mn10300-linux-tdep.c (am33_linux_init_osabi): Adjust. * nios2-linux-tdep.c (nios2_linux_init_abi): Adjust. * or1k-linux-tdep.c (or1k_linux_init_abi): Adjust. * ppc-linux-tdep.c (ppc_linux_init_abi): Adjust. * riscv-linux-tdep.c (riscv_linux_init_abi): Adjust. * rs6000-tdep.c (struct ppc_inferior_data) <disp_step_buf>: Change type to displaced_step_buffers. * s390-linux-tdep.c (s390_linux_init_abi_any): Adjust. * sh-linux-tdep.c (sh_linux_init_abi): Adjust. * sparc-linux-tdep.c (sparc32_linux_init_abi): Adjust. * sparc64-linux-tdep.c (sparc64_linux_init_abi): Adjust. * tic6x-linux-tdep.c (tic6x_uclinux_init_abi): Adjust. * tilegx-linux-tdep.c (tilegx_linux_init_abi): Adjust. * xtensa-linux-tdep.c (xtensa_linux_init_abi): Adjust. Change-Id: Ia9c02f207da2c9e1d9188020139619122392bb70 |
||
|
Simon Marchi
|
d965505887 |
gdb: change linux gdbarch data from post to pre-init
The following patch will need to fill a field in linux_gdbarch_data while the gdbarch is being built. linux_gdbarch_data is currently allocated as a post-init gdbarch data, meaning it's not possible to fill it before the gdbarch is completely initialized. Change it to a pre-init gdbarch data to allow this. The init_linux_gdbarch_data function doesn't use the created gdbarch, it only allocates the linux_gdbarch_data structure on the gdbarch's obstack, so the change is trivial. gdb/ChangeLog: * linux-tdep.c (init_linux_gdbarch_data): Change parameter to obkstack. (_initialize_linux_tdep): Register pre-init gdb data instead of post-init. Change-Id: If35ce91b6bb5435680d43b9268d811d95661644f |
||
|
Simon Marchi
|
187b041e25 |
gdb: move displaced stepping logic to gdbarch, allow starting concurrent displaced steps
Today, GDB only allows a single displaced stepping operation to happen
per inferior at a time. There is a single displaced stepping buffer per
inferior, whose address is fixed (obtained with
gdbarch_displaced_step_location), managed by infrun.c.
In the case of the AMD ROCm target [1] (in the context of which this
work has been done), it is typical to have thousands of threads (or
waves, in SMT terminology) executing the same code, hitting the same
breakpoint (possibly conditional) and needing to to displaced step it at
the same time. The limitation of only one displaced step executing at a
any given time becomes a real bottleneck.
To fix this bottleneck, we want to make it possible for threads of a
same inferior to execute multiple displaced steps in parallel. This
patch builds the foundation for that.
In essence, this patch moves the task of preparing a displaced step and
cleaning up after to gdbarch functions. This allows using different
schemes for allocating and managing displaced stepping buffers for
different platforms. The gdbarch decides how to assign a buffer to a
thread that needs to execute a displaced step.
On the ROCm target, we are able to allocate one displaced stepping
buffer per thread, so a thread will never have to wait to execute a
displaced step.
On Linux, the entry point of the executable if used as the displaced
stepping buffer, since we assume that this code won't get used after
startup. From what I saw (I checked with a binary generated against
glibc and musl), on AMD64 we have enough space there to fit two
displaced stepping buffers. A subsequent patch makes AMD64/Linux use
two buffers.
In addition to having multiple displaced stepping buffers, there is also
the idea of sharing displaced stepping buffers between threads. Two
threads doing displaced steps for the same PC could use the same buffer
at the same time. Two threads stepping over the same instruction (same
opcode) at two different PCs may also be able to share a displaced
stepping buffer. This is an idea for future patches, but the
architecture built by this patch is made to allow this.
Now, the implementation details. The main part of this patch is moving
the responsibility of preparing and finishing a displaced step to the
gdbarch. Before this patch, preparing a displaced step is driven by the
displaced_step_prepare_throw function. It does some calls to the
gdbarch to do some low-level operations, but the high-level logic is
there. The steps are roughly:
- Ask the gdbarch for the displaced step buffer location
- Save the existing bytes in the displaced step buffer
- Ask the gdbarch to copy the instruction into the displaced step buffer
- Set the pc of the thread to the beginning of the displaced step buffer
Similarly, the "fixup" phase, executed after the instruction was
successfully single-stepped, is driven by the infrun code (function
displaced_step_finish). The steps are roughly:
- Restore the original bytes in the displaced stepping buffer
- Ask the gdbarch to fixup the instruction result (adjust the target's
registers or memory to do as if the instruction had been executed in
its original location)
The displaced_step_inferior_state::step_thread field indicates which
thread (if any) is currently using the displaced stepping buffer, so it
is used by displaced_step_prepare_throw to check if the displaced
stepping buffer is free to use or not.
This patch defers the whole task of preparing and cleaning up after a
displaced step to the gdbarch. Two new main gdbarch methods are added,
with the following semantics:
- gdbarch_displaced_step_prepare: Prepare for the given thread to
execute a displaced step of the instruction located at its current PC.
Upon return, everything should be ready for GDB to resume the thread
(with either a single step or continue, as indicated by
gdbarch_displaced_step_hw_singlestep) to make it displaced step the
instruction.
- gdbarch_displaced_step_finish: Called when the thread stopped after
having started a displaced step. Verify if the instruction was
executed, if so apply any fixup required to compensate for the fact
that the instruction was executed at a different place than its
original pc. Release any resources that were allocated for this
displaced step. Upon return, everything should be ready for GDB to
resume the thread in its "normal" code path.
The displaced_step_prepare_throw function now pretty much just offloads
to gdbarch_displaced_step_prepare and the displaced_step_finish function
offloads to gdbarch_displaced_step_finish.
The gdbarch_displaced_step_location method is now unnecessary, so is
removed. Indeed, the core of GDB doesn't know how many displaced step
buffers there are nor where they are.
To keep the existing behavior for existing architectures, the logic that
was previously implemented in infrun.c for preparing and finishing a
displaced step is moved to displaced-stepping.c, to the
displaced_step_buffer class. Architectures are modified to implement
the new gdbarch methods using this class. The behavior is not expected
to change.
The other important change (which arises from the above) is that the
core of GDB no longer prevents concurrent displaced steps. Before this
patch, start_step_over walks the global step over chain and tries to
initiate a step over (whether it is in-line or displaced). It follows
these rules:
- if an in-line step is in progress (in any inferior), don't start any
other step over
- if a displaced step is in progress for an inferior, don't start
another displaced step for that inferior
After starting a displaced step for a given inferior, it won't start
another displaced step for that inferior.
In the new code, start_step_over simply tries to initiate step overs for
all the threads in the list. But because threads may be added back to
the global list as it iterates the global list, trying to initiate step
overs, start_step_over now starts by stealing the global queue into a
local queue and iterates on the local queue. In the typical case, each
thread will either:
- have initiated a displaced step and be resumed
- have been added back by the global step over queue by
displaced_step_prepare_throw, because the gdbarch will have returned
that there aren't enough resources (i.e. buffers) to initiate a
displaced step for that thread
Lastly, if start_step_over initiates an in-line step, it stops
iterating, and moves back whatever remaining threads it had in its local
step over queue to the global step over queue.
Two other gdbarch methods are added, to handle some slightly annoying
corner cases. They feel awkwardly specific to these cases, but I don't
see any way around them:
- gdbarch_displaced_step_copy_insn_closure_by_addr: in
arm_pc_is_thumb, arm-tdep.c wants to get the closure for a given
buffer address.
- gdbarch_displaced_step_restore_all_in_ptid: when a process forks
(at least on Linux), the address space is copied. If some displaced
step buffers were in use at the time of the fork, we need to restore
the original bytes in the child's address space.
These two adjustments are also made in infrun.c:
- prepare_for_detach: there may be multiple threads doing displaced
steps when we detach, so wait until all of them are done
- handle_inferior_event: when we handle a fork event for a given
thread, it's possible that other threads are doing a displaced step at
the same time. Make sure to restore the displaced step buffer
contents in the child for them.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb
gdb/ChangeLog:
* displaced-stepping.h (struct
displaced_step_copy_insn_closure): Adjust comments.
(struct displaced_step_inferior_state) <step_thread,
step_gdbarch, step_closure, step_original, step_copy,
step_saved_copy>: Remove fields.
(struct displaced_step_thread_state): New.
(struct displaced_step_buffer): New.
* displaced-stepping.c (displaced_step_buffer::prepare): New.
(write_memory_ptid): Move from infrun.c.
(displaced_step_instruction_executed_successfully): New,
factored out of displaced_step_finish.
(displaced_step_buffer::finish): New.
(displaced_step_buffer::copy_insn_closure_by_addr): New.
(displaced_step_buffer::restore_in_ptid): New.
* gdbarch.sh (displaced_step_location): Remove.
(displaced_step_prepare, displaced_step_finish,
displaced_step_copy_insn_closure_by_addr,
displaced_step_restore_all_in_ptid): New.
* gdbarch.c: Re-generate.
* gdbarch.h: Re-generate.
* gdbthread.h (class thread_info) <displaced_step_state>: New
field.
(thread_step_over_chain_remove): New declaration.
(thread_step_over_chain_next): New declaration.
(thread_step_over_chain_length): New declaration.
* thread.c (thread_step_over_chain_remove): Make non-static.
(thread_step_over_chain_next): New.
(global_thread_step_over_chain_next): Use
thread_step_over_chain_next.
(thread_step_over_chain_length): New.
(global_thread_step_over_chain_enqueue): Add debug print.
(global_thread_step_over_chain_remove): Add debug print.
* infrun.h (get_displaced_step_copy_insn_closure_by_addr):
Remove.
* infrun.c (get_displaced_stepping_state): New.
(displaced_step_in_progress_any_inferior): Remove.
(displaced_step_in_progress_thread): Adjust.
(displaced_step_in_progress): Adjust.
(displaced_step_in_progress_any_thread): New.
(get_displaced_step_copy_insn_closure_by_addr): Remove.
(gdbarch_supports_displaced_stepping): Use
gdbarch_displaced_step_prepare_p.
(displaced_step_reset): Change parameter from inferior to
thread.
(displaced_step_prepare_throw): Implement using
gdbarch_displaced_step_prepare.
(write_memory_ptid): Move to displaced-step.c.
(displaced_step_restore): Remove.
(displaced_step_finish): Implement using
gdbarch_displaced_step_finish.
(start_step_over): Allow starting more than one displaced step.
(prepare_for_detach): Handle possibly multiple threads doing
displaced steps.
(handle_inferior_event): Handle possibility that fork event
happens while another thread displaced steps.
* linux-tdep.h (linux_displaced_step_prepare): New.
(linux_displaced_step_finish): New.
(linux_displaced_step_copy_insn_closure_by_addr): New.
(linux_displaced_step_restore_all_in_ptid): New.
(linux_init_abi): Add supports_displaced_step parameter.
* linux-tdep.c (struct linux_info) <disp_step_buf>: New field.
(linux_displaced_step_prepare): New.
(linux_displaced_step_finish): New.
(linux_displaced_step_copy_insn_closure_by_addr): New.
(linux_displaced_step_restore_all_in_ptid): New.
(linux_init_abi): Add supports_displaced_step parameter,
register displaced step methods if true.
(_initialize_linux_tdep): Register inferior_execd observer.
* amd64-linux-tdep.c (amd64_linux_init_abi_common): Add
supports_displaced_step parameter, adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
(amd64_linux_init_abi): Adjust call to
amd64_linux_init_abi_common.
(amd64_x32_linux_init_abi): Likewise.
* aarch64-linux-tdep.c (aarch64_linux_init_abi): Adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
* arm-linux-tdep.c (arm_linux_init_abi): Likewise.
* i386-linux-tdep.c (i386_linux_init_abi): Likewise.
* alpha-linux-tdep.c (alpha_linux_init_abi): Adjust call to
linux_init_abi.
* arc-linux-tdep.c (arc_linux_init_osabi): Likewise.
* bfin-linux-tdep.c (bfin_linux_init_abi): Likewise.
* cris-linux-tdep.c (cris_linux_init_abi): Likewise.
* csky-linux-tdep.c (csky_linux_init_abi): Likewise.
* frv-linux-tdep.c (frv_linux_init_abi): Likewise.
* hppa-linux-tdep.c (hppa_linux_init_abi): Likewise.
* ia64-linux-tdep.c (ia64_linux_init_abi): Likewise.
* m32r-linux-tdep.c (m32r_linux_init_abi): Likewise.
* m68k-linux-tdep.c (m68k_linux_init_abi): Likewise.
* microblaze-linux-tdep.c (microblaze_linux_init_abi): Likewise.
* mips-linux-tdep.c (mips_linux_init_abi): Likewise.
* mn10300-linux-tdep.c (am33_linux_init_osabi): Likewise.
* nios2-linux-tdep.c (nios2_linux_init_abi): Likewise.
* or1k-linux-tdep.c (or1k_linux_init_abi): Likewise.
* riscv-linux-tdep.c (riscv_linux_init_abi): Likewise.
* s390-linux-tdep.c (s390_linux_init_abi_any): Likewise.
* sh-linux-tdep.c (sh_linux_init_abi): Likewise.
* sparc-linux-tdep.c (sparc32_linux_init_abi): Likewise.
* sparc64-linux-tdep.c (sparc64_linux_init_abi): Likewise.
* tic6x-linux-tdep.c (tic6x_uclinux_init_abi): Likewise.
* tilegx-linux-tdep.c (tilegx_linux_init_abi): Likewise.
* xtensa-linux-tdep.c (xtensa_linux_init_abi): Likewise.
* ppc-linux-tdep.c (ppc_linux_init_abi): Adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
* arm-tdep.c (arm_pc_is_thumb): Call
gdbarch_displaced_step_copy_insn_closure_by_addr instead of
get_displaced_step_copy_insn_closure_by_addr.
* rs6000-aix-tdep.c (rs6000_aix_init_osabi): Adjust calls to
clear gdbarch methods.
* rs6000-tdep.c (struct ppc_inferior_data): New structure.
(get_ppc_per_inferior): New function.
(ppc_displaced_step_prepare): New function.
(ppc_displaced_step_finish): New function.
(ppc_displaced_step_restore_all_in_ptid): New function.
(rs6000_gdbarch_init): Register new gdbarch methods.
* s390-tdep.c (s390_gdbarch_init): Don't call
set_gdbarch_displaced_step_location, set new gdbarch methods.
gdb/testsuite/ChangeLog:
* gdb.arch/amd64-disp-step-avx.exp: Adjust pattern.
* gdb.threads/forking-threads-plus-breakpoint.exp: Likewise.
* gdb.threads/non-stop-fair-events.exp: Likewise.
Change-Id: I387cd235a442d0620ec43608fd3dc0097fcbf8c8
|
||
|
Simon Marchi
|
c7acb87bc6 |
gdb: move displaced stepping types to displaced-stepping.{h,c}
Move displaced-stepping related stuff unchanged to displaced-stepping.h and displaced-stepping.c. This helps make the following patch a bit smaller and easier to read. gdb/ChangeLog: * Makefile.in (COMMON_SFILES): Add displaced-stepping.c. * aarch64-tdep.h: Include displaced-stepping.h. * displaced-stepping.h (struct displaced_step_copy_insn_closure): Move here. (displaced_step_copy_insn_closure_up): Move here. (struct buf_displaced_step_copy_insn_closure): Move here. (struct displaced_step_inferior_state): Move here. (debug_displaced): Move here. (displaced_debug_printf_1): Move here. (displaced_debug_printf): Move here. * displaced-stepping.c: New file. * gdbarch.sh: Include displaced-stepping.h in gdbarch.h. * gdbarch.h: Re-generate. * inferior.h: Include displaced-stepping.h. * infrun.h (debug_displaced): Move to displaced-stepping.h. (displaced_debug_printf_1): Likewise. (displaced_debug_printf): Likewise. (struct displaced_step_copy_insn_closure): Likewise. (displaced_step_copy_insn_closure_up): Likewise. (struct buf_displaced_step_copy_insn_closure): Likewise. (struct displaced_step_inferior_state): Likewise. * infrun.c (show_debug_displaced): Move to displaced-stepping.c. (displaced_debug_printf_1): Likewise. (displaced_step_copy_insn_closure::~displaced_step_copy_insn_closure): Likewise. (_initialize_infrun): Don't register "set/show debug displaced". Change-Id: I29935f5959b80425370630a45148fc06cd4227ca |
||
|
Simon Marchi
|
94b24c74e8 |
gdb: pass inferior to get_linux_inferior_data
Pass to get_linux_inferior_data the inferior for which we want to obtain the linux-specific data, rather than assuming the current inferior. This helps slightly reduce the diff in the upcoming main patch. Update the sole caller to pass the current inferior. gdb/ChangeLog: * linux-tdep.c (get_linux_inferior_data): Add inferior parameter. (linux_vsyscall_range): Pass current inferior. Change-Id: Ie4b61190e4a2e89b5b55a140cfecd4de66d92393 |
||
|
Simon Marchi
|
bab37966cf |
gdb: introduce status enum for displaced step prepare/finish
This is a preparatory patch to reduce the size of the diff of the upcoming main patch. It introduces enum types for the return values of displaced step "prepare" and "finish" operations. I find that this expresses better the intention of the code, rather than returning arbitrary integer values (-1, 0 and 1) which are difficult to remember. That makes the code easier to read. I put the new enum types in a new displaced-stepping.h file, because I introduce that file in a later patch anyway. Putting it there avoids having to move it later. There is one change in behavior for displaced_step_finish: it currently returns 0 if the thread wasn't doing a displaced step and 1 if the thread was doing a displaced step which was executed successfully. It turns out that this distinction is not needed by any caller, so I've merged these two cases into "_OK", rather than adding an extra enumerator. gdb/ChangeLog: * infrun.c (displaced_step_prepare_throw): Change return type to displaced_step_prepare_status. (displaced_step_prepare): Likewise. (displaced_step_finish): Change return type to displaced_step_finish_status. (resume_1): Adjust. (stop_all_threads): Adjust. * displaced-stepping.h: New file. Change-Id: I5c8fe07212cd398d5b486b5936d9d0807acd3788 |
||
|
Simon Marchi
|
7def77a1cf |
gdb: rename displaced_step_fixup to displaced_step_finish
This is a preparatory patch to reduce a little bit the diff size of the main patch later in this series. It renames the displaced_step_fixup function in infrun.c to displaced_step_finish. The rationale is to better differentiate the low and high level operations. We first have the low level operation of writing an instruction to a displaced buffer, called "copy_insn". The mirror low level operation to fix up the state after having executed the instruction is "fixup". The high level operation of preparing a thread for a displaced step (which includes doing the "copy_insn" and some more bookkeeping) is called "prepare" (as in displaced_step_prepare). The mirror high level operation to cleaning up after a displaced step (which includes doing the "fixup" and some more bookkeeping) is currently also called "fixup" (as in displaced_step_fixup), just like the low level operation. I think that choosing a different name for the low and high level cleanup operation makes it clearer, hence "finish". gdb/ChangeLog: * infrun.c (displaced_step_fixup): Rename to... (displaced_step_finish): ... this, update all callers. Change-Id: Id32f48c1e2091d09854c77fcedcc14d2519957a2 |