[RFA] PR python/20190 - compute TLS symbol without a frame

[Tom Tromey <tom at tromey dot com>]

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 23.

2016-06-03  Tom Tromey  <tom@tromey.com>

	PR python/20190:
	* value.h (symbol_read_needs): Declare.
	(symbol_read_needs_frame): Add comment.
	* symtab.h (struct symbol_computed_ops) <read_variable>: Update
	comment.
	<read_needs_frame>: 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 needs_frame_baton) <needs>: Renamed from
	needs_frame.  Changed type.
	(needs_frame_read_addr_from_reg, needs_frame_get_reg_value)
	(needs_frame_frame_base, needs_frame_frame_cfa)
	(needs_frame_tls_address, needs_dwarf_reg_entry_value): Update.
	(dwarf2_loc_desc_needs_frame, locexpr_read_needs_frame)
	(loclist_read_needs_frame): Change return type.
	* defs.h (enum symbol_needs_kind): New.

2016-06-03  Tom Tromey  <tom@tromey.com>

	PR python/20190:
	* gdb.threads/tls.exp (check_thread_local): Add python symbol
	test.
---
 gdb/ChangeLog                     | 20 ++++++++++++++++++++
 gdb/defs.h                        | 16 ++++++++++++++++
 gdb/dwarf2loc.c                   | 29 ++++++++++++++++-------------
 gdb/findvar.c                     | 29 ++++++++++++++++++++---------
 gdb/symtab.h                      |  8 +++++---
 gdb/testsuite/ChangeLog           |  6 ++++++
 gdb/testsuite/gdb.threads/tls.exp | 10 ++++++++++
 gdb/value.h                       |  7 +++++++
 8 files changed, 100 insertions(+), 25 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index af4ddcc..ab9c3eb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,23 @@
+2016-06-03  Tom Tromey  <tom@tromey.com>
+
+	PR python/20190:
+	* value.h (symbol_read_needs): Declare.
+	(symbol_read_needs_frame): Add comment.
+	* symtab.h (struct symbol_computed_ops) <read_variable>: Update
+	comment.
+	<read_needs_frame>: 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 needs_frame_baton) <needs>: Renamed from
+	needs_frame.  Changed type.
+	(needs_frame_read_addr_from_reg, needs_frame_get_reg_value)
+	(needs_frame_frame_base, needs_frame_frame_cfa)
+	(needs_frame_tls_address, needs_dwarf_reg_entry_value): Update.
+	(dwarf2_loc_desc_needs_frame, locexpr_read_needs_frame)
+	(loclist_read_needs_frame): Change return type.
+	* defs.h (enum symbol_needs_kind): New.
+
 2016-06-02  Jon Turney  <jon.turney@dronecode.org.uk>
 
 	* windows-nat.c (handle_output_debug_string): Return type of
diff --git a/gdb/defs.h b/gdb/defs.h
index ed51396..ec90d30 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -617,6 +617,22 @@ 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.  */
+
+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 adb0ac2..5020f82 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2729,7 +2729,7 @@ dwarf2_compile_property_to_c (struct ui_file *stream,
 
 struct needs_frame_baton
 {
-  int needs_frame;
+  enum symbol_needs_kind needs;
   struct dwarf2_per_cu_data *per_cu;
 };
 
@@ -2739,7 +2739,7 @@ needs_frame_read_addr_from_reg (void *baton, int regnum)
 {
   struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton;
 
-  nf_baton->needs_frame = 1;
+  nf_baton->needs = SYMBOL_NEEDS_FRAME;
   return 1;
 }
 
@@ -2751,7 +2751,7 @@ needs_frame_get_reg_value (void *baton, struct type *type, int regnum)
 {
   struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton;
 
-  nf_baton->needs_frame = 1;
+  nf_baton->needs = SYMBOL_NEEDS_FRAME;
   return value_zero (type, not_lval);
 }
 
@@ -2772,7 +2772,7 @@ needs_frame_frame_base (void *baton, const gdb_byte **start, size_t * length)
   *start = &lit0;
   *length = 1;
 
-  nf_baton->needs_frame = 1;
+  nf_baton->needs = SYMBOL_NEEDS_FRAME;
 }
 
 /* CFA accesses require a frame.  */
@@ -2782,7 +2782,7 @@ needs_frame_frame_cfa (void *baton)
 {
   struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton;
 
-  nf_baton->needs_frame = 1;
+  nf_baton->needs = SYMBOL_NEEDS_FRAME;
   return 1;
 }
 
@@ -2792,7 +2792,8 @@ needs_frame_tls_address (void *baton, CORE_ADDR offset)
 {
   struct needs_frame_baton *nf_baton = (struct needs_frame_baton *) baton;
 
-  nf_baton->needs_frame = 1;
+  if (nf_baton->needs != SYMBOL_NEEDS_FRAME)
+    nf_baton->needs = SYMBOL_NEEDS_REGISTERS;
   return 1;
 }
 
@@ -2816,7 +2817,7 @@ needs_dwarf_reg_entry_value (struct dwarf_expr_context *ctx,
 {
   struct needs_frame_baton *nf_baton = (struct needs_frame_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);
@@ -2861,7 +2862,7 @@ static const struct dwarf_expr_context_funcs needs_frame_ctx_funcs =
 /* Return non-zero iff the location expression at DATA (length SIZE)
    requires a frame to evaluate.  */
 
-static int
+static enum symbol_needs_kind
 dwarf2_loc_desc_needs_frame (const gdb_byte *data, size_t size,
 			     struct dwarf2_per_cu_data *per_cu)
 {
@@ -2871,7 +2872,7 @@ dwarf2_loc_desc_needs_frame (const gdb_byte *data, size_t size,
   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 ();
@@ -2902,7 +2903,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,7 +3739,7 @@ locexpr_read_variable_at_entry (struct symbol *symbol, struct frame_info *frame)
 }
 
 /* Return non-zero iff we need a frame to evaluate SYMBOL.  */
-static int
+static enum symbol_needs_kind
 locexpr_read_needs_frame (struct symbol *symbol)
 {
   struct dwarf2_locexpr_baton *dlbaton
@@ -4530,7 +4533,7 @@ loclist_read_variable_at_entry (struct symbol *symbol, struct frame_info *frame)
 }
 
 /* Return non-zero iff we need a frame to evaluate SYMBOL.  */
-static int
+static enum symbol_needs_kind
 loclist_read_needs_frame (struct symbol *symbol)
 {
   /* If there's a location list, then assume we need to have a frame
@@ -4539,7 +4542,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
diff --git a/gdb/findvar.c b/gdb/findvar.c
index a39d897..71bc48d 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -337,11 +337,10 @@ 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);
@@ -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.  */
@@ -574,6 +581,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
@@ -582,8 +590,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 (_("Cannnot read `%s' without registers"), SYMBOL_PRINT_NAME (var));
 
   if (frame != NULL)
     frame = get_hosting_frame (var, var_block, frame);
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 6f00baf..4e452d6 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,9 @@ 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);
+  /* Return the requirements we need a frame to find the value of the
+     SYMBOL.  */
+  enum symbol_needs_kind (*read_needs_frame) (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 3b305a6..6ace0f4 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2016-06-03  Tom Tromey  <tom@tromey.com>
+
+	PR python/20190:
+	* gdb.threads/tls.exp (check_thread_local): Add python symbol
+	test.
+
 2016-06-02  Tom Tromey  <tom@tromey.com>
 
 	PR python/18984:
diff --git a/gdb/testsuite/gdb.threads/tls.exp b/gdb/testsuite/gdb.threads/tls.exp
index 29384e5..f9c0c27 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 <http://www.gnu.org/licenses/>.  */
 
+load_lib gdb-python.exp
+
 standard_testfile tls.c tls2.c
 
 if [istarget "*-*-linux"] then {
@@ -70,6 +72,14 @@ 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"
+	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"
diff --git a/gdb/value.h b/gdb/value.h
index f8ec854..1790258 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -671,6 +671,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,
-- 
2.5.5