From 0b31a4bcec2efec9bc8ca40deb61123c52690a2e Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 3 Jun 2016 14:11:08 -0600 Subject: [PATCH] PR python/20190 - compute TLS symbol without a frame PR python/20190 arose from an exception I noticed when trying to use the Python unwinder for Spider Monkey in Firefox. The problem is that the unwinder wants to examine the value of a thread-local variable. However, sympy_value rejects this because symbol_read_needs_frame returns true for a TLS variable. This problem arose once before, though in a different context: https://sourceware.org/bugzilla/show_bug.cgi?id=11803 At the time Pedro and Daniel pointed out a simpler way to fix that bug (see links in 20190 if you are interested); but for this new bug I couldn't think of a similar fix and ended up implementing Daniel's other suggestion: https://sourceware.org/ml/gdb-patches/2010-07/msg00393.html That is, this patch makes it possible to detect whether a symbol needs a specific frame, or whether it just needs the inferior to have registers. Built and regtested on x86-64 Fedora 24. 2016-07-26 Tom Tromey * symtab.c (register_symbol_computed_impl): Update. PR python/20190: * value.h (symbol_read_needs): Declare. (symbol_read_needs_frame): Add comment. * symtab.h (struct symbol_computed_ops) : Update comment. : Rename. Change return type. * findvar.c (symbol_read_needs): New function. (symbol_read_needs_frame): Rewrite. (default_read_var_value): Use symbol_read_needs. * dwarf2loc.c (struct symbol_needs_baton): Rename. : Renamed from needs_frame. Changed type. (needs_frame_read_addr_from_reg, symbol_needs_get_reg_value) (symbol_needs_read_mem, symbol_needs_frame_base) (symbol_needs_frame_cfa, symbol_needs_tls_address) (symbol_needs_dwarf_call): Rename. (needs_dwarf_reg_entry_value): Update. (symbol_needs_ctx_funcs, dwarf2_loc_desc_get_symbol_read_needs): Rename and update. (locexpr_get_symbol_read_needs, loclist_symbol_needs): Likewise. (dwarf2_locexpr_funcs, dwarf2_loclist_funcs): Update. * defs.h (enum symbol_needs_kind): New. 2016-07-26 Tom Tromey PR python/20190: * gdb.threads/tls.exp (check_thread_local): Add python symbol test. --- gdb/ChangeLog | 25 ++++++ gdb/defs.h | 19 +++++ gdb/dwarf2loc.c | 121 ++++++++++++++++-------------- gdb/findvar.c | 31 +++++--- gdb/symtab.c | 2 +- gdb/symtab.h | 10 ++- gdb/testsuite/ChangeLog | 6 ++ gdb/testsuite/gdb.threads/tls.exp | 17 +++++ gdb/value.h | 7 ++ 9 files changed, 169 insertions(+), 69 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2f3710d9e4..a4d6258bb0 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,28 @@ +2016-07-26 Tom Tromey + + * symtab.c (register_symbol_computed_impl): Update. + PR python/20190: + * value.h (symbol_read_needs): Declare. + (symbol_read_needs_frame): Add comment. + * symtab.h (struct symbol_computed_ops) : Update + comment. + : Rename. Change return type. + * findvar.c (symbol_read_needs): New function. + (symbol_read_needs_frame): Rewrite. + (default_read_var_value): Use symbol_read_needs. + * dwarf2loc.c (struct symbol_needs_baton): Rename. + : Renamed from needs_frame. Changed type. + (needs_frame_read_addr_from_reg, symbol_needs_get_reg_value) + (symbol_needs_read_mem, symbol_needs_frame_base) + (symbol_needs_frame_cfa, symbol_needs_tls_address) + (symbol_needs_dwarf_call): Rename. + (needs_dwarf_reg_entry_value): Update. + (symbol_needs_ctx_funcs, dwarf2_loc_desc_get_symbol_read_needs): + Rename and update. + (locexpr_get_symbol_read_needs, loclist_symbol_needs): Likewise. + (dwarf2_locexpr_funcs, dwarf2_loclist_funcs): Update. + * defs.h (enum symbol_needs_kind): New. + 2016-07-26 Pedro Alves * nat/linux-ptrace.c: Include "gregset.h". diff --git a/gdb/defs.h b/gdb/defs.h index 5088390e2c..fee5f411eb 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -629,6 +629,25 @@ enum gdb_osabi extern double atof (const char *); /* X3.159-1989 4.10.1.1 */ #endif +/* Enumerate the requirements a symbol has in order to be evaluated. + These are listed in order of "strength" -- a later entry subsumes + earlier ones. This fine-grained distinction is important because + it allows for the evaluation of a TLS symbol during unwinding -- + when unwinding one has access to registers, but not the frame + itself, because that is being constructed. */ + +enum symbol_needs_kind +{ + /* No special requirements -- just memory. */ + SYMBOL_NEEDS_NONE, + + /* The symbol needs registers. */ + SYMBOL_NEEDS_REGISTERS, + + /* The symbol needs a frame. */ + SYMBOL_NEEDS_FRAME +}; + /* Dynamic target-system-dependent parameters for GDB. */ #include "gdbarch.h" diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 548e4683a6..e60475fecc 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -2726,21 +2726,21 @@ dwarf2_compile_property_to_c (struct ui_file *stream, } -/* Helper functions and baton for dwarf2_loc_desc_needs_frame. */ +/* Helper functions and baton for dwarf2_loc_desc_get_symbol_read_needs. */ -struct needs_frame_baton +struct symbol_needs_baton { - int needs_frame; + enum symbol_needs_kind needs; struct dwarf2_per_cu_data *per_cu; }; /* Reads from registers do require a frame. */ static CORE_ADDR -needs_frame_read_addr_from_reg (void *baton, int regnum) +symbol_needs_read_addr_from_reg (void *baton, int regnum) { - struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton; + struct symbol_needs_baton *nf_baton = (struct symbol_needs_baton *) baton; - nf_baton->needs_frame = 1; + nf_baton->needs = SYMBOL_NEEDS_FRAME; return 1; } @@ -2748,61 +2748,64 @@ needs_frame_read_addr_from_reg (void *baton, int regnum) Reads from registers do require a frame. */ static struct value * -needs_frame_get_reg_value (void *baton, struct type *type, int regnum) +symbol_needs_get_reg_value (void *baton, struct type *type, int regnum) { - struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton; + struct symbol_needs_baton *nf_baton = (struct symbol_needs_baton *) baton; - nf_baton->needs_frame = 1; + nf_baton->needs = SYMBOL_NEEDS_FRAME; return value_zero (type, not_lval); } /* Reads from memory do not require a frame. */ static void -needs_frame_read_mem (void *baton, gdb_byte *buf, CORE_ADDR addr, size_t len) +symbol_needs_read_mem (void *baton, gdb_byte *buf, CORE_ADDR addr, size_t len) { memset (buf, 0, len); } /* Frame-relative accesses do require a frame. */ static void -needs_frame_frame_base (void *baton, const gdb_byte **start, size_t * length) +symbol_needs_frame_base (void *baton, const gdb_byte **start, size_t * length) { static gdb_byte lit0 = DW_OP_lit0; - struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton; + struct symbol_needs_baton *nf_baton = (struct symbol_needs_baton *) baton; *start = &lit0; *length = 1; - nf_baton->needs_frame = 1; + nf_baton->needs = SYMBOL_NEEDS_FRAME; } /* CFA accesses require a frame. */ static CORE_ADDR -needs_frame_frame_cfa (void *baton) +symbol_needs_frame_cfa (void *baton) { - struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton; + struct symbol_needs_baton *nf_baton = (struct symbol_needs_baton *) baton; - nf_baton->needs_frame = 1; + nf_baton->needs = SYMBOL_NEEDS_FRAME; return 1; } -/* Thread-local accesses do require a frame. */ +/* Thread-local accesses require registers, but not a frame. */ static CORE_ADDR -needs_frame_tls_address (void *baton, CORE_ADDR offset) +symbol_needs_tls_address (void *baton, CORE_ADDR offset) { - struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton; + struct symbol_needs_baton *nf_baton = (struct symbol_needs_baton *) baton; - nf_baton->needs_frame = 1; + if (nf_baton->needs <= SYMBOL_NEEDS_REGISTERS) + nf_baton->needs = SYMBOL_NEEDS_REGISTERS; return 1; } -/* Helper interface of per_cu_dwarf_call for dwarf2_loc_desc_needs_frame. */ +/* Helper interface of per_cu_dwarf_call for + dwarf2_loc_desc_get_symbol_read_needs. */ static void -needs_frame_dwarf_call (struct dwarf_expr_context *ctx, cu_offset die_offset) +symbol_needs_dwarf_call (struct dwarf_expr_context *ctx, cu_offset die_offset) { - struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) ctx->baton; + struct symbol_needs_baton *nf_baton = + (struct symbol_needs_baton *) ctx->baton; per_cu_dwarf_call (ctx, die_offset, nf_baton->per_cu, ctx->funcs->get_frame_pc, ctx->baton); @@ -2815,9 +2818,10 @@ needs_dwarf_reg_entry_value (struct dwarf_expr_context *ctx, enum call_site_parameter_kind kind, union call_site_parameter_u kind_u, int deref_size) { - struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) ctx->baton; + struct symbol_needs_baton *nf_baton = + (struct symbol_needs_baton *) ctx->baton; - nf_baton->needs_frame = 1; + nf_baton->needs = SYMBOL_NEEDS_FRAME; /* The expression may require some stub values on DWARF stack. */ dwarf_expr_push_address (ctx, 0, 0); @@ -2841,38 +2845,39 @@ needs_get_obj_addr (void *baton) return 1; } -/* Virtual method table for dwarf2_loc_desc_needs_frame below. */ +/* Virtual method table for dwarf2_loc_desc_get_symbol_read_needs + below. */ -static const struct dwarf_expr_context_funcs needs_frame_ctx_funcs = +static const struct dwarf_expr_context_funcs symbol_needs_ctx_funcs = { - needs_frame_read_addr_from_reg, - needs_frame_get_reg_value, - needs_frame_read_mem, - needs_frame_frame_base, - needs_frame_frame_cfa, - needs_frame_frame_cfa, /* get_frame_pc */ - needs_frame_tls_address, - needs_frame_dwarf_call, + symbol_needs_read_addr_from_reg, + symbol_needs_get_reg_value, + symbol_needs_read_mem, + symbol_needs_frame_base, + symbol_needs_frame_cfa, + symbol_needs_frame_cfa, /* get_frame_pc */ + symbol_needs_tls_address, + symbol_needs_dwarf_call, NULL, /* get_base_type */ needs_dwarf_reg_entry_value, needs_get_addr_index, needs_get_obj_addr }; -/* Return non-zero iff the location expression at DATA (length SIZE) - requires a frame to evaluate. */ +/* Compute the correct symbol_needs_kind value for the location + expression at DATA (length SIZE). */ -static int -dwarf2_loc_desc_needs_frame (const gdb_byte *data, size_t size, - struct dwarf2_per_cu_data *per_cu) +static enum symbol_needs_kind +dwarf2_loc_desc_get_symbol_read_needs (const gdb_byte *data, size_t size, + struct dwarf2_per_cu_data *per_cu) { - struct needs_frame_baton baton; + struct symbol_needs_baton baton; struct dwarf_expr_context *ctx; int in_reg; struct cleanup *old_chain; struct objfile *objfile = dwarf2_per_cu_objfile (per_cu); - baton.needs_frame = 0; + baton.needs = SYMBOL_NEEDS_NONE; baton.per_cu = per_cu; ctx = new_dwarf_expr_context (); @@ -2884,7 +2889,7 @@ dwarf2_loc_desc_needs_frame (const gdb_byte *data, size_t size, ctx->ref_addr_size = dwarf2_per_cu_ref_addr_size (per_cu); ctx->offset = dwarf2_per_cu_text_offset (per_cu); ctx->baton = &baton; - ctx->funcs = &needs_frame_ctx_funcs; + ctx->funcs = &symbol_needs_ctx_funcs; dwarf_expr_eval (ctx, data, size); @@ -2903,7 +2908,9 @@ dwarf2_loc_desc_needs_frame (const gdb_byte *data, size_t size, do_cleanups (old_chain); - return baton.needs_frame || in_reg; + if (in_reg) + baton.needs = SYMBOL_NEEDS_FRAME; + return baton.needs; } /* A helper function that throws an unimplemented error mentioning a @@ -3736,15 +3743,17 @@ locexpr_read_variable_at_entry (struct symbol *symbol, struct frame_info *frame) dlbaton->size); } -/* Return non-zero iff we need a frame to evaluate SYMBOL. */ -static int -locexpr_read_needs_frame (struct symbol *symbol) +/* Implementation of get_symbol_read_needs from + symbol_computed_ops. */ + +static enum symbol_needs_kind +locexpr_get_symbol_read_needs (struct symbol *symbol) { struct dwarf2_locexpr_baton *dlbaton = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol); - return dwarf2_loc_desc_needs_frame (dlbaton->data, dlbaton->size, - dlbaton->per_cu); + return dwarf2_loc_desc_get_symbol_read_needs (dlbaton->data, dlbaton->size, + dlbaton->per_cu); } /* Return true if DATA points to the end of a piece. END is one past @@ -4473,7 +4482,7 @@ locexpr_generate_c_location (struct symbol *sym, struct ui_file *stream, const struct symbol_computed_ops dwarf2_locexpr_funcs = { locexpr_read_variable, locexpr_read_variable_at_entry, - locexpr_read_needs_frame, + locexpr_get_symbol_read_needs, locexpr_describe_location, 0, /* location_has_loclist */ locexpr_tracepoint_var_ref, @@ -4530,9 +4539,11 @@ loclist_read_variable_at_entry (struct symbol *symbol, struct frame_info *frame) return value_of_dwarf_block_entry (SYMBOL_TYPE (symbol), frame, data, size); } -/* Return non-zero iff we need a frame to evaluate SYMBOL. */ -static int -loclist_read_needs_frame (struct symbol *symbol) +/* Implementation of get_symbol_read_needs from + symbol_computed_ops. */ + +static enum symbol_needs_kind +loclist_symbol_needs (struct symbol *symbol) { /* If there's a location list, then assume we need to have a frame to choose the appropriate location expression. With tracking of @@ -4540,7 +4551,7 @@ loclist_read_needs_frame (struct symbol *symbol) is disabled in GCC at the moment until we figure out how to represent it. */ - return 1; + return SYMBOL_NEEDS_FRAME; } /* Print a natural-language description of SYMBOL to STREAM. This @@ -4684,7 +4695,7 @@ loclist_generate_c_location (struct symbol *sym, struct ui_file *stream, const struct symbol_computed_ops dwarf2_loclist_funcs = { loclist_read_variable, loclist_read_variable_at_entry, - loclist_read_needs_frame, + loclist_symbol_needs, loclist_describe_location, 1, /* location_has_loclist */ loclist_tracepoint_var_ref, diff --git a/gdb/findvar.c b/gdb/findvar.c index c64ec06b55..6e28a296de 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -337,14 +337,13 @@ address_to_signed_pointer (struct gdbarch *gdbarch, struct type *type, store_signed_integer (buf, TYPE_LENGTH (type), byte_order, addr); } -/* Will calling read_var_value or locate_var_value on SYM end - up caring what frame it is being evaluated relative to? SYM must - be non-NULL. */ -int -symbol_read_needs_frame (struct symbol *sym) +/* See value.h. */ + +enum symbol_needs_kind +symbol_read_needs (struct symbol *sym) { if (SYMBOL_COMPUTED_OPS (sym) != NULL) - return SYMBOL_COMPUTED_OPS (sym)->read_needs_frame (sym); + return SYMBOL_COMPUTED_OPS (sym)->get_symbol_read_needs (sym); switch (SYMBOL_CLASS (sym)) { @@ -358,7 +357,7 @@ symbol_read_needs_frame (struct symbol *sym) case LOC_REF_ARG: case LOC_REGPARM_ADDR: case LOC_LOCAL: - return 1; + return SYMBOL_NEEDS_FRAME; case LOC_UNDEF: case LOC_CONST: @@ -374,9 +373,17 @@ symbol_read_needs_frame (struct symbol *sym) case LOC_CONST_BYTES: case LOC_UNRESOLVED: case LOC_OPTIMIZED_OUT: - return 0; + return SYMBOL_NEEDS_NONE; } - return 1; + return SYMBOL_NEEDS_FRAME; +} + +/* See value.h. */ + +int +symbol_read_needs_frame (struct symbol *sym) +{ + return symbol_read_needs (sym) == SYMBOL_NEEDS_FRAME; } /* Private data to be used with minsym_lookup_iterator_cb. */ @@ -575,6 +582,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block, struct value *v; struct type *type = SYMBOL_TYPE (var); CORE_ADDR addr; + enum symbol_needs_kind sym_need; /* Call check_typedef on our type to make sure that, if TYPE is a TYPE_CODE_TYPEDEF, its length is set to the length of the target type @@ -583,8 +591,11 @@ default_read_var_value (struct symbol *var, const struct block *var_block, set the returned value type description correctly. */ check_typedef (type); - if (symbol_read_needs_frame (var)) + sym_need = symbol_read_needs (var); + if (sym_need == SYMBOL_NEEDS_FRAME) gdb_assert (frame != NULL); + else if (sym_need == SYMBOL_NEEDS_REGISTERS && !target_has_registers) + error (_("Cannot read `%s' without registers"), SYMBOL_PRINT_NAME (var)); if (frame != NULL) frame = get_hosting_frame (var, var_block, frame); diff --git a/gdb/symtab.c b/gdb/symtab.c index 5c4305d581..e776a0c996 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -6092,7 +6092,7 @@ register_symbol_computed_impl (enum address_class aclass, gdb_assert (ops != NULL); gdb_assert (ops->tracepoint_var_ref != NULL); gdb_assert (ops->describe_location != NULL); - gdb_assert (ops->read_needs_frame != NULL); + gdb_assert (ops->get_symbol_read_needs != NULL); gdb_assert (ops->read_variable != NULL); return result; diff --git a/gdb/symtab.h b/gdb/symtab.h index 6f00baf463..9df4efbf2c 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -629,7 +629,8 @@ struct symbol_computed_ops frame FRAME. If the variable has been optimized out, return zero. - Iff `read_needs_frame (SYMBOL)' is zero, then FRAME may be zero. */ + Iff `read_needs_frame (SYMBOL)' is not SYMBOL_NEEDS_FRAME, then + FRAME may be zero. */ struct value *(*read_variable) (struct symbol * symbol, struct frame_info * frame); @@ -640,8 +641,11 @@ struct symbol_computed_ops struct value *(*read_variable_at_entry) (struct symbol *symbol, struct frame_info *frame); - /* Return non-zero if we need a frame to find the value of the SYMBOL. */ - int (*read_needs_frame) (struct symbol * symbol); + /* Find the "symbol_needs_kind" value for the given symbol. This + value determines whether reading the symbol needs memory (e.g., a + global variable), just registers (a thread-local), or a frame (a + local variable). */ + enum symbol_needs_kind (*get_symbol_read_needs) (struct symbol * symbol); /* Write to STREAM a natural-language description of the location of SYMBOL, in the context of ADDR. */ diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index aa7aa28f62..1ba1d91a9f 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2016-07-26 Tom Tromey + + PR python/20190: + * gdb.threads/tls.exp (check_thread_local): Add python symbol + test. + 2016-07-26 Markus Metzger * gdb.btrace/record_goto.exp: se is_amd64_regs_target for diff --git a/gdb/testsuite/gdb.threads/tls.exp b/gdb/testsuite/gdb.threads/tls.exp index 29384e5449..5c07844f14 100644 --- a/gdb/testsuite/gdb.threads/tls.exp +++ b/gdb/testsuite/gdb.threads/tls.exp @@ -14,6 +14,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . */ +load_lib gdb-python.exp + standard_testfile tls.c tls2.c if [istarget "*-*-linux"] then { @@ -70,6 +72,17 @@ proc check_thread_local {number} { "= $expected_value" \ "${number} thread local storage" + if {![skip_python_tests]} { + gdb_test_no_output \ + "python sym = gdb.lookup_symbol('a_thread_local')\[0\]" \ + "${number} look up a_thread_local symbol" + # We intentionally do not pass a frame to "value" here. If a + # TLS variable requires a frame, this will fail. However, if + # it does not require a frame, then it will succeed. + gdb_test "python print(sym.value())" "$expected_value" \ + "${number} get symbol value without frame" + } + gdb_test "p K::another_thread_local" \ "= $me_variable" \ "${number} another thread local storage" @@ -139,6 +152,10 @@ proc check_thread_stack {number spin_threads spin_threads_level} { } clean_restart ${binfile} + +gdb_test "print a_thread_local" \ + "Cannot read .a_thread_local. without registers" + if ![runto_main] then { fail "Can't run to main" return 0 diff --git a/gdb/value.h b/gdb/value.h index 0b417b4ea5..3299e87a19 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -672,6 +672,13 @@ extern struct value *value_of_register (int regnum, struct frame_info *frame); struct value *value_of_register_lazy (struct frame_info *frame, int regnum); +/* Return the symbol's reading requirement. */ + +extern enum symbol_needs_kind symbol_read_needs (struct symbol *); + +/* Return true if the symbol needs a frame. This is a wrapper for + symbol_read_needs that simply checks for SYMBOL_NEEDS_FRAME. */ + extern int symbol_read_needs_frame (struct symbol *); extern struct value *read_var_value (struct symbol *var,