aboutsummaryrefslogtreecommitdiff
path: root/gdb
AgeCommit message (Collapse)AuthorFilesLines
2021-08-05Simplify dwarf_expr_context class interfaceZoran Zaric4-232/+237
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.
2021-08-05Make DWARF evaluator return a single struct valueZoran Zaric5-198/+203
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.
2021-08-05Make value_copy also copy the stack data memberZoran Zaric1-0/+2
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.
2021-08-05Move piece_closure and its support to expr.cZoran Zaric4-599/+595
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.
2021-08-05Merge evaluate_for_locexpr_baton evaluatorZoran Zaric3-55/+28
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.
2021-08-05Remove empty frame and full evaluatorsZoran Zaric2-29/+5
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.
2021-08-05Inline get_reg_value method of dwarf_expr_contextZoran Zaric2-23/+7
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.
2021-08-05Move push_dwarf_reg_entry_value to expr.cZoran Zaric5-83/+72
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.
2021-08-05Move read_mem to dwarf_expr_contextZoran Zaric4-13/+10
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.
2021-08-05Move get_object_address to dwarf_expr_contextZoran Zaric3-18/+9
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.
2021-08-05Move dwarf_call to dwarf_expr_contextZoran Zaric4-70/+33
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.
2021-08-05Move compilation unit info to dwarf_expr_contextZoran Zaric5-82/+75
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. With this change, it also makes sense to always acquire ref_addr_size information from the compilation unit context, considering that all DWARF operations that refer to that information require a compilation unit context to be present during their evaluation. 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. (dwarf_expr_context::get_addr_index): Remove method. (dwarf_expr_context::dwarf_variable_value): Remove method. (dwarf_expr_context::ref_addr_size): Remove member. * 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.
2021-08-05Remove get_frame_cfa from dwarf_expr_contextZoran Zaric4-17/+4
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.
2021-08-05Move frame context info to dwarf_expr_contextZoran Zaric4-105/+91
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 process 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.
2021-08-05Cleanup of the dwarf_expr_context constructorZoran Zaric2-18/+9
Move the initial values for dwarf_expr_context class data members to the class declaration in expr.h. gdb/ChangeLog: * dwarf2/expr.c (dwarf_expr_context::dwarf_expr_context): Remove initial data members values. * dwarf2/expr.h (dwarf_expr_context): Add initial values to the class data members.
2021-08-05Replace the symbol needs evaluator with a parserZoran Zaric6-116/+678
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                    # conditional jump to DW_OP_bregx                    DW_OP_bra 4                    DW_OP_lit0                    # jump to DW_OP_stack_value                    DW_OP_skip 3                    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                        # jump to DW_OP_fbreg                        DW_OP_skip 4                        DW_OP_drop                        DW_OP_fbreg 0                        DW_OP_dup                        DW_OP_lit0                        DW_OP_eq                        # conditional jump to DW_OP_drop                        DW_OP_bra -9                        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 analysis treats the expression byte stream as a DWARF operation graph, where each graph node can be visited only once and each operation can decide if the frame context is needed for their evaluation. 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. The new analysis requires the DWARF location description for the argc argument of the main 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. 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.
2021-08-05gdb/testsuite: update test gdb.base/step-over-syscall.expAndrew Burgess2-11/+97
I was looking at PR gdb/19675 and the related test gdb.base/step-over-syscall.exp. This test includes a call to kfail when we are testing a displaced step over a clone syscall. While looking at the test I removed the call to kfail and ran the test, and was surprised that the test passed. I ran the test a few times and it does sometimes fail, but mostly it passed fine. PR gdb/19675 describes how, when we displaced step over a clone, the new thread is created with a $pc in the displaced step buffer. GDB then fails to "fix" this $pc (for the new thread), and the thread will be set running with its current $pc value. This means that the new thread will just start executing from whatever happens to be after the displaced stepping buffer. In the original PR gdb/19675 bug report Yao Qi was seeing the new thread cause a segfault, the problem is, what actually happens is totally undefined. On my machine, I'm seeing the new thread reenter main, it then starts trying to run the test again (in the new thread). This just happens to be safe enough (in this simple test) that most of the time the inferior doesn't crash. In this commit I try to make the test slightly more likely to fail by doing a couple of things. First, I added a static variable to main, this is set true when the first thread enters main, if a second thread ever enters main then I force an abort. Second, when the test is finishing I want to ensure that the new threads have had a chance to do "something bad" if they are going to. So I added a global counter, as each thread starts successfully it decrements the counter. The main thread does not proceed to the final marker function (where GDB has placed a breakpoint) until all threads have started successfully. This means that if the newly created thread doesn't successfully enter clone_fn then the counter will never reach zero and the test will timeout. With these two changes my hope is that the test should fail more reliably, and so, I have also changed the test to call setup_kfail before the specific steps that we expect to misbehave instead of just calling kfail and skipping parts of the test completely. The benefit of this is that if/when we fix GDB this test will start to KPASS and we'll know to update this test to remove the setup_kfail call.
2021-08-04gdb: Use unwinder name in frame_info::to_stringLancelot SIX1-2/+2
While working on a stack unwinding issue using 'set debug frame on', I noticed the frame_info::to_string method could be slightly improved. Unwinders have been given a name in a154d838a70e96d888620c072e2d6ea8bdf044ca. Before this patch, frame_info debug output prints the host address of the used unwinder, which is not easy to interpret. This patch proposes to use the unwinder name instead since we now have it. Before the patch: {level=1,type=NORMAL_FRAME,unwind=0x2ac1763ec0,pc=0x3ff7fc3460,id={stack=0x3ff7ea79b0,code=0x0000003ff7fc33ac,!special},func=0x3ff7fc33ac} With the patch: {level=1,type=NORMAL_FRAME,unwinder="riscv prologue",pc=0x3ff7fc3460,id={stack=0x3ff7ea79b0,code=0x0000003ff7fc33ac,!special},func=0x3ff7fc33ac} Tested on riscv64-linux-gnu.
2021-08-04gdb/testsuite: fix gdb.base/info-macros.exp with clangSimon Marchi1-4/+4
The test gdb.base/info-macros.exp says that it doesn't pass the "debug" option to prepare_for_testing because that would cause -g to appear after -g3 on the command line, and that would cause some gcc versions to not include macro info. I don't know what gcc versions this refers to. I tested with gcc 4.8, and that works fine with -g after -g3. The current state is problematic when testing with CC_FOR_TARGET=clang, because then only -fdebug-macro is included. No -g switch if included, meaning we get a binary without any debug info, and the test fails. One way to fix it would be to add "debug" to the options when the compiler is clang. However, the solution I chose was to specify "debug" in any case, even for gcc. Other macro tests such as gdb.base/macscp.exp do perfectly fine with it. Also, this lets the test use the debug flag specified by the board file. For example, we can test with GCC and DWARF 5, with: $ make check RUNTESTFLAGS="--target_board unix/gdb:debug_flags=-gdwarf-5" TESTS="gdb.base/info-macros.exp" With the hard-coded -g3, this wouldn't actually test with DWARF 5. Change-Id: I33fa92ee545007d3ae9c52c4bb2d5be6b5b698f1
2021-08-04gdb: avoid dereferencing empty str_offsets_base optional in dwarf_decode_macrosSimon Marchi3-7/+20
Since 4d7188abfdf2 ("gdbsupport: add debug assertions in gdb::optional::get"), some macro-related tests fail on Ubuntu 20.04 with the system gcc 9.3.0 compiler when building with _GLIBCXX_DEBUG. For example, gdb.base/info-macros.exp results in: (gdb) break -qualified main /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/gdb_optional.h:206: internal-error: T& gdb::optional<T>::get() [with T = long unsigned int]: Assertion `this->has_value ()' failed. The binary contains DWARF 4 debug info and includes a pre-standard (pre-DWARF 5) .debug_macro section. The CU doesn't have a DW_AT_str_offsets_base attribute (which doesn't exist in DWARF 4). The field dwarf2_cu::str_offsets_base is therefore empty. At dwarf2/read.c:24138, we unconditionally read the value in the optional, which triggers the assertion shown above. The same thing happens when building the test program with DWARF 5 with the same gcc compiler, as that version of gcc doesn't use indirect string forms, even with DWARF 5. So it still doesn't add a DW_AT_str_offsets_base attribute on the CU. Fix that by propagating down a gdb::optional<ULONGEST> for the str offsets base instead of ULONGEST. That value is only used in dwarf_decode_macro_bytes, when encountering an "strx" macro operation (DW_MACRO_define_strx or DW_MACRO_undef_strx). Add a check there that we indeed have a value in the optional before reading it. This is unlikely to happen, but could happen in theory with an erroneous file that uses DW_MACRO_define_strx but does not provide a DW_AT_str_offsets_base (in practice, some things would probably have failed before and stopped processing of debug info). I tested the complaint by inverting the condition and using a clang-compiled binary, which uses the strx operators. This is the result: During symbol reading: use of DW_MACRO_define_strx with unknown string offsets base [in module /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/info-macros/info-macros] The test now passes cleanly with the setup mentioned above, and the testsuite looks on par with how it was before 4d7188abfdf2. Change-Id: I7ebd2724beb7b9b4178872374c2a177aea696e77
2021-08-04gdb: fix typo in complaint in dwarf2/macro.cSimon Marchi1-1/+1
I saw this complaint when my code had some bug, and spotted the typo. Fix it, and while at it mention DW_MACRO as well (it would be confusing to only see DW_MACINFO with a file that uses a DWARF 5 .debug_macro section). I contemplated the idea of passing the knowledge of whether we are dealing with a .debug_macro section or .debug_macinfo section, to print only the right one. But in the end, I don't think that trouble is necessary for a complaint nobody is going to see. Change-Id: I276ce8da65c3eac5304f64a1e246358ed29cdbbc
2021-08-04gdb: fix warnings in bsd-kvm.cSimon Marchi1-3/+3
Building on OpenBSD, I get warnings like: CXX bsd-kvm.o /home/simark/src/binutils-gdb/gdb/bsd-kvm.c:241:18: error: ISO C++11 does not allow conversion from string literal to 'char *' [-Werror,-Wwritable-strings] nl[0].n_name = "_dumppcb"; ^ Silence those by adding casts. Change-Id: I2bef4eebcc306762a4e3e5b5c52f67ecf2820503
2021-08-04[gdb/symtab] Use lambda function instead of addrmap_foreach_checkTom de Vries1-30/+21
Use a lambda function instead of addrmap_foreach_check, which removes the need for static variables. Also remove unnecessary static on local var temp_obstack in test_addrmap. gdb/ChangeLog: 2021-08-04 Tom de Vries <tdevries@suse.de> * addrmap.c (addrmap_foreach_check): Remove. (array, val1, val2): Move ... (test_addrmap): ... here. Remove static on temp_obstack. Use lambda function instead of addrmap_foreach_check.
2021-08-04[gdb/symtab] Implement addrmap_mutable_findTom de Vries1-4/+126
Currently addrmap_mutable_find is not implemented: ... static void * addrmap_mutable_find (struct addrmap *self, CORE_ADDR addr) { /* Not needed yet. */ internal_error (__FILE__, __LINE__, _("addrmap_find is not implemented yet " "for mutable addrmaps")); } ... I implemented this because I needed it during debugging, to be able to do: ... (gdb) p ((dwarf2_psymtab *)addrmap_find (map, addr))->filename ... before and after a call to addrmap_set_empty. Since this is not used otherwise, added addrmap unit test. Build on x86_64-linux, tested by doing: ... $ gdb -q -batch -ex "maint selftest addrmap" Running selftest addrmap. Ran 1 unit tests, 0 failed ... gdb/ChangeLog: 2021-08-03 Tom de Vries <tdevries@suse.de> * gdb/addrmap.c (addrmap_mutable_find): Implement [GDB_SELF_TESTS] (CHECK_ADDRMAP_FIND): New macro. [GDB_SELF_TESTS] (core_addr, addrmap_foreach_check, test_addrmap) (_initialize_addrmap): New function.
2021-08-03gdb: follow-fork: push target and add thread in target_follow_forkSimon Marchi15-142/+172
In the context of ROCm-gdb [1], the ROCm target sits on top of the linux-nat target. when a process forks, it needs to carry over some data from the forking inferior to the fork child inferior. Ideally, the ROCm target would implement the follow_fork target_ops method, but there are some small problems. This patch fixes these, which helps the ROCm target, but also makes things more consistent and a bit nicer in general, I believe. The main problem is: when follow-fork-mode is "parent", target_follow_fork is called with the parent as the current inferior. When it's "child", target_follow_fork is called with the child as the current inferior. This means that target_follow_fork is sometimes called on the parent's target stack and sometimes on the child's target stack. The parent's target stack may contain targets above the process target, such as the ROCm target. So if follow-fork-child is "parent", the ROCm target would get notified of the fork and do whatever is needed. But the child's target stack, at that moment, only contains the exec and process target copied over from the parent. The child's target stack is set up by follow_fork_inferior, before calling target_follow_fork. In that case, the ROCm target wouldn't get notified of the fork. For consistency, I think it would be good to always call target_follow_fork on the parent inferior's target stack. I think it makes sense as a way to indicate "this inferior has called fork, do whatever is needed". The desired outcome of the fork (whether an inferior is created for the child, do we need to detach from the child) can be indicated by passed parameter. I therefore propose these changes: - make follow_fork_inferior always call target_follow_fork with the parent as the current inferior. That lets all targets present on the parent's target stack do some fork-related handling and push themselves on the fork child's target stack if needed. For this purpose, pass the child inferior down to target_follow_fork and follow_fork implementations. This is nullptr if no inferior is created for the child, because we want to detach from it. - as a result, in follow_fork_inferior, detach from the parent inferior (if needed) only after the target_follow_fork call. This is needed because we want to call target_follow_fork before the parent's target stack is torn down. - hand over to the targets in the parent's target stack (including the process target) the responsibility to push themselves, if needed, to the child's target stack. Also hand over the responsibility to the process target, at the same time, to create the child's initial thread (just like we do for follow_exec). - pass the child inferior to exec_on_vfork, so we don't need to swap the current inferior between parent and child. Nothing in exec_on_vfork depends on the current inferior, after this change. Although this could perhaps be replaced with just having the exec target implement follow_fork and push itself in the child's target stack, like the process target does... We would just need to make sure the process target calls beneath()->follow_fork(...). I'm not sure about this one. gdb/ChangeLog: * target.h (struct target_ops) <follow_fork>: Add inferior* parameter. (target_follow_fork): Likewise. * target.c (default_follow_fork): Likewise. (target_follow_fork): Likewise. * fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Likewise. (fbsd_nat_target::follow_fork): Likewise, and call inf_ptrace_target::follow_fork. * linux-nat.h (class linux_nat_target) <follow_fork>: Likewise. * linux-nat.c (linux_nat_target::follow_fork): Likewise, and call inf_ptrace_target::follow_fork. * obsd-nat.h (obsd_nat_target) <follow_fork>: Likewise. * obsd-nat.c (obsd_nat_target::follow_fork): Likewise, and call inf_ptrace_target::follow_fork. * remote.c (class remote_target) <follow_fork>: Likewise. (remote_target::follow_fork): Likewise, and call process_stratum_target::follow_fork. * process-stratum-target.h (class process_stratum_target) <follow_fork>: New. * process-stratum-target.c (process_stratum_target::follow_fork): New. * target-delegates.c: Re-generate. [1] https://github.com/ROCm-Developer-Tools/ROCgdb Change-Id: I460bd0af850f0485e8aed4b24c6d8262a4c69929
2021-08-03Fixes for mi-fortran-modules.exp fixesCarl Love1-2/+2
Output has additional information for a given filename. gdb/testsuite/ChangeLog * gdb.mi/mi-fortran-modules.exp (system_modules_pattern, system_module_symbols_pattern): Add check for additional symbols on the line
2021-08-03[gdb/testsuite] templates.exp to accept clang++ outputAlok Kumar Sharma1-0/+3
Please consider below testcase with intended error. `````````` constexpr const char cstring[] = "Eta"; template <const char*, typename T> class Column {}; using quick = Column<cstring,double>; // cstring without '&' void lookup() { quick c1; c1.ls(); } `````````` It produces below error. `````````` no member named 'ls' in 'Column<&cstring, double>'. `````````` Please note that error message contains '&' for cstring, which is absent in actual program. Clang++ does not generate & in such cases and this should also be accepted as correct output. gdb/testsuite/ChangeLog: * gdb.cp/templates.exp: Accept different but correct output from the Clang++ compiled binary also.
2021-08-02Handle compiler-generated suffixes in Ada namesTom Tromey3-1/+120
The compiler may add a suffix to a mangled name. A typical example would be splitting a function and creating a ".cold" variant. This patch changes Ada decoding (aka demangling) to handle these suffixes. It also changes the encoding process to handle them as well. A symbol like "function.cold" will now be displayed to the user as "function[cold]". The "." is not simply preserved because that is already used in Ada.
2021-08-02Remove uses of fprintf_symbol_filteredTom Tromey3-17/+7
I believe that many calls to fprintf_symbol_filtered are incorrect. In particular, there are some that pass a symbol's print name, like: fprintf_symbol_filtered (gdb_stdout, sym->print_name (), current_language->la_language, DMGL_ANSI); fprintf_symbol_filtered uses the "demangle" global to decide whether or not to demangle -- but print_name does this as well. This can lead to double-demangling. Normally this could be innocuous, except I also plan to change Ada demangling in a way that causes this to fail.
2021-08-02Handle type qualifier for enumeration nameTom Tromey4-2/+112
Pierre-Marie noticed that the Ada expression "TYPE'(NAME)" resolved incorrectly when "TYPE" was an enumeration type. Here, "NAME" should be unambiguous. This patch fixes this problem. Note that the patch is not perfect -- it does not give an error if TYPE is an enumeration type but NAME is not an enumerator but does have some other meaning in scope. Fixing this proved difficult, and so I've left it out.
2021-08-02Remove the type_qualifier globalTom Tromey1-13/+1
The type_qualifier global is no longer needed in the Ada expression parser, so this removes it.
2021-08-02Defer Ada character literal resolutionTom Tromey7-44/+204
In Ada, an enumeration type can use a character literal as one of the enumerators. The Ada expression parser handles the appropriate conversion. It turns out, though, that this conversion was handled incorrectly. For an expression like TYPE'(EXP), the conversion would be done for any such literal appearing in EXP -- but only the outermost such expression should really be affected. This patch defers the conversion until the resolution phase, fixing the bug.
2021-08-02Refactor Ada resolutionTom Tromey3-10/+42
In a subsequent patch, it will be convenient if an Ada expression operation can supply its own replacement object. This patch refactors Ada expression resolution to make this possible.
2021-08-02Remove add_symbols_from_enclosing_procsTom Tromey1-28/+2
I noticed that add_symbols_from_enclosing_procs is empty, and can be removed. The one caller, ada_add_local_symbols, can also be simplified, removing some code that, I think, was an incorrect attempt to handle nested functions.
2021-08-02Avoid crash in varobj deletionTom Tromey2-2/+16
PR varobj/28131 points out a crash in the varobj deletion code. It took a while to reproduce this, but essentially what happens is that a top-level varobj deletes its root object, then deletes the "dynamic" object. However, deletion of the dynamic object may cause ~py_varobj_iter to run, which in turn uses gdbpy_enter_varobj: gdbpy_enter_varobj::gdbpy_enter_varobj (const struct varobj *var) : gdbpy_enter (var->root->exp->gdbarch, var->root->exp->language_defn) { } However, because var->root has already been destroyed, this is invalid. I've added a new test case. This doesn't reliably crash, but the problem can easily be seen under valgrind (and, I presume, with ASAN, though I did not try this). Tested on x86-64 Fedora 32. I also propose putting this on the GDB 11 branch, with a suitable ChangeLog entry of course. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28131
2021-08-02[gdb/testsuite] Fix gdb.dwarf2/dw2-using-debug-str.exp with cc-with-dwz-mTom de Vries1-0/+13
When running with target board cc-with-dwz-m, we run into: ... (gdb) file dw2-using-debug-str-no-debug-str^M Reading symbols from dw2-using-debug-str-no-debug-str...^M (gdb) FAIL: gdb.dwarf2/dw2-using-debug-str.exp: file dw2-using-debug-str ... With native, the .debug_str section is present in the dw2-using-debug-str executable, and removed from the dw2-using-debug-str-no-debug-str executable. When loading the latter, a dwarf error is triggered. With cc-with-dwz-m, the .debug_str section is not present in the dw2-using-debug-str executable, because it's already moved to .tmp/dw2-using-debug-str.dwz. Consequently, the removal has no effect, and no dwarf error is triggered, which causes the FAIL. The same problem arises with target board cc-with-gnu-debuglink. Fix this by detecting whether the .debug_str section is missing, and skipping the remainder of the test-case. Tested on x86_64-linux. gdb/testsuite/ChangeLog: 2021-08-02 Tom de Vries <tdevries@suse.de> * gdb.dwarf2/dw2-using-debug-str.exp: Handle missing .debug_str section in dw2-using-debug-str.
2021-08-02[gdb/testsuite] Fix gdb.dwarf2/dw2-using-debug-str.exp with cc-with-gdb-indexTom de Vries1-12/+16
When running with target board cc-with-gdb-index, we run into: ... (gdb) file dw2-using-debug-str-no-debug-str^M Reading symbols from dw2-using-debug-str-no-debug-str...^M Dwarf Error: DW_FORM_strp used without required section^M (gdb) FAIL: gdb.dwarf2/dw2-using-debug-str.exp: file dw2-using-debug-str ... The test expects the dwarf error, but has no matching pattern for the entire output. Fix this by updating the regexp. Tested on x86_64-linux. gdb/testsuite/ChangeLog: 2021-08-02 Tom de Vries <tdevries@suse.de> * gdb.dwarf2/dw2-using-debug-str.exp: Update regexp to match cc-with-gdb-index output.
2021-08-02[gdb/testsuite] Fix gdb.dwarf2/per-bfd-sharing.exp with cc-with-gdb-indexTom de Vries1-1/+5
When running with target board cc-with-gdb-index, we run into: ... rm: cannot remove '/tmp/tmp.JmYTeiuFjj/*.gdb-index': \ No such file or directory^M FAIL: gdb.dwarf2/per-bfd-sharing.exp: \ couldn't remove files in temporary cache dir ... Fix this, as in gdb.base/index-cache.exp, by only FAILing when $expecting_index_cache_use. Tested on x86_64-linux. gdb/testsuite/ChangeLog: 2021-08-02 Tom de Vries <tdevries@suse.de> * gdb.dwarf2/per-bfd-sharing.exp: Only expect index-cache files when $expecting_index_cache_use.
2021-08-02[gdb/testsuite] Fix gdb.dwarf2/gdb-index-nodebug.exp with cc-with-gdb-indexTom de Vries1-2/+21
When running with target board cc-with-gdb-index, we run into: ... (gdb) save gdb-index .^M Error while writing index for `gdb-index-nodebug': \ Cannot use an index to create the index^M (gdb) FAIL: gdb.dwarf2/gdb-index-nodebug.exp: try to save gdb index ... Fix this by detecting an already present index, and marking the test unsupported. Tested on x86_64-linux. gdb/testsuite/ChangeLog: 2021-08-02 Tom de Vries <tdevries@suse.de> * gdb.dwarf2/gdb-index-nodebug.exp: Mark unsupported when index already present.
2021-08-02[gdb/testsuite] Fix gdb.dwarf2/fission-relative-dwo.exp with cc-with-gdb-indexTom de Vries1-0/+14
When running with target board cc-with-gdb-index, we run into: ... gdb compile failed, warning: Could not find DWO CU \ fission-relative-dwo.dwo(0x1234) referenced by CU at offset 0xc7 \ [in module outputs/gdb.dwarf2/fission-relative-dwo/.tmp/fission-relative-dwo] UNTESTED: gdb.dwarf2/fission-relative-dwo.exp: fission-relative-dwo.exp ERROR: failed to compile fission-relative-dwo ... The problem is that: - the .dwo file is found relative to the executable, and - cc-with-tweaks.sh moves the executable to a temp dir, but not the .dwo file. Fix this by copying the .dwo file alongside the executable in the temp dir. Verified changes using shellcheck. Tested on x86_64-linux. gdb/ChangeLog: 2021-08-02 Tom de Vries <tdevries@suse.de> * contrib/cc-with-tweaks.sh: Copy .dwo files alongside executable.
2021-08-02gdb: Make the builtin "boolean" type an unsigned typeShahab Vahedi2-1/+69
When printing the fields of a register that is of a custom struct type, the "unpack_bits_as_long ()" function is used: do_val_print (...) cp_print_value_fields (...) value_field_bitfield (...) unpack_value_bitfield (...) unpack_bits_as_long (...) This function may sign-extend the extracted field while returning it: val >>= lsbcount; if (...) { valmask = (((ULONGEST) 1) << bitsize) - 1; val &= valmask; if (!field_type->is_unsigned ()) if (val & (valmask ^ (valmask >> 1))) val |= ~valmask; } return val; lsbcount: Number of lower bits to get rid of. bitsize: The bit length of the field to be extracted. val: The register value. field_type: The type of field that is being handled. While the logic here is correct, there is a problem when it is handling "field_type"s of "boolean". Those types are NOT marked as "unsigned" and therefore they end up being sign extended. Although this is not a problem for "false" (0), it definitely causes trouble for "true". This patch constructs the builtin boolean type as such that it is marked as an "unsigned" entity. The issue tackled here was first encountered for arc-elf32 target running on an x86_64 machine. The unit-test introduced in this change has passed for all the targets (--enable-targets=all) running on the same x86_64 host. Fixes: https://sourceware.org/PR28104
2021-08-01[gdb/testsuite] Fix gdb.base/maint.exp with cc-with-gdb-indexTom de Vries1-13/+21
With target board cc-with-gdb-index we run into: ... FAIL: gdb.base/maint.exp: maint print statistics ... The output that is checked is: ... Statistics for 'maint':^M Number of "minimal" symbols read: 53^M Number of "full" symbols read: 40^M Number of "types" defined: 60^M Number of symbol tables: 7^M Number of symbol tables with line tables: 2^M Number of symbol tables with blockvectors: 2^M Number of read CUs: 2^M Number of unread CUs: 5^M Total memory used for objfile obstack: 20320^M Total memory used for BFD obstack: 4064^M Total memory used for string cache: 4064^M ... and the regexp doesn't match because it expects the "Number of read/unread CUs" lines in a different place. Fix this by updating the regexp. Tested on x86_64-linux. gdb/testsuite/ChangeLog: 2021-08-01 Tom de Vries <tdevries@suse.de> * gdb.base/maint.exp: Update "maint print statistics" to match output with target board cc-with-gdb-index.
2021-08-01[gdb/testsuite] Fix gdb.base/index-cache.exp with cc-with-gdb-indexTom de Vries1-1/+1
With target board cc-with-gdb-index we run into: ... FAIL: gdb.base/index-cache.exp: couldn't remove files in temporary cache dir ... The problem is that there are no files to remove, because the index cache isn't used, as indicated by $expecting_index_cache_use. Fix this by only FAILing when $expecting_index_cache_use. Tested on x86_64-linux. gdb/testsuite/ChangeLog: 2021-08-01 Tom de Vries <tdevries@suse.de> * gdb.base/index-cache.exp:
2021-07-30Use iterator_range in more placesTom Tromey1-50/+9
This changes a couple of spots to replace custom iterator range classes with a specialization of iterator_range. Regression tested on x86-64 Fedora 34.
2021-07-30Replace exception_print_same with operator!=Tom Tromey3-18/+1
I noticed that exception_print_same is only used in a single spot, and it seemed to be better as an operator!= method attached to gdb_exception. Regression tested on x86-64 Fedora 34.
2021-07-29gdb: fix nr_bits gdb_assert in append_flags_type_fieldSimon Marchi1-1/+1
The assertion gdb_assert (nr_bits >= 1 && nr_bits <= type_bitsize); is not correct. Well, it's correct in that we do want the number of bits to be in the range [1, type_bitsize]. But we don't check anywhere that the end of the specified flag is within the containing type. The following code should generate a failed assertion, as the flag goes past the 32 bits of the underlying type, but it's currently not caught: static void test_print_flag (gdbarch *arch) { type *flags_type = arch_flags_type (arch, "test_type", 32); type *field_type = builtin_type (arch)->builtin_uint32; append_flags_type_field (flags_type, 31, 2, field_type, "invalid"); } (You can test this by registering it as a selftest using selftests::register_test_foreach_arc and running.) Change the assertion to verify that the end bit is within the range of the underlying type. This implicitly verifies that nr_bits is not too big as well, so we don't need a separate assertion for that. Change-Id: I9be79e5fd7a5917bf25b03b598727e6274c892e8 Co-Authored-By: Tony Tye <Tony.Tye@amd.com>
2021-07-29obsd-nat: Report both thread and PID in ::pid_to_str.John Baldwin1-1/+1
This improves the output of info threads when debugging multiple inferiors (e.g. after a fork with detach_on_fork disabled).
2021-07-29obsd-nat: Various fixes for fork following.John Baldwin2-30/+25
- Don't use #ifdef's on ptrace ops. obsd-nat.h didn't include <sys/ptrace.h>, so the virtual methods weren't always overridden causing the fork following to not work. In addition, the thread and fork code is intertwined in ::wait and and the lack of #ifdef's there already assumed both were present. Finally, both of these ptrace ops have been present in OpenBSD for at least 10 years. - Move duplicated code to enable PTRACE_FORK event reporting to a single function and invoke it on new child processes reported via PTRACE_FORK. - Don't return early from PTRACE_FORK handling, but instead reset wptid to the correct ptid if the child reports its event before the parent. This allows the ptid fixup code to add thread IDs if the first event for a process is a PTRACE_FORK event. This also properly returns ptid's with thread IDs when reporting PTRACE_FORK events. - Handle detach_fork by skipping the PT_DETACH.
2021-07-29obsd-nat: Various fixes to obsd_nat_target::wait.John Baldwin1-49/+12
- Call inf_ptrace_target::wait instead of duplicating the code. Replace a check for WIFSTOPPED on the returned status from waitpid by checking for TARGET_WAITKIND_STOPPED in the parsed status as is done in fbsd_nat_target::wait. - Don't use inferior_ptid when deciding if a new process is a child vs parent of the fork. Instead, use find_inferior_pid and assume that if an inferior already exists, the pid in question is the parent; otherwise, the pid is the child. - Don't use inferior_ptid when deciding if the ptid of the process needs to be updated with an LWP ID, or if this is a new thread. Instead, use the approach from fbsd-nat which is to check if a ptid without an LWP exists and if so update the ptid of that thread instead of adding a new thread.
2021-07-29x86-bsd-nat: Only define gdb_ptrace when using debug registers.John Baldwin1-8/+8
This fixes an unused function warning on OpenBSD which does not support PT_GETDBREGS.