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

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3940188..82611b7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,28 @@ 
+2016-07-25  Tom Tromey  <tom@tromey.com>
+
+	* 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) <read_variable>: Update
+	comment.
+	<get_symbol_read_needs>: 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.
+	<needs>: 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-25  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* top.h (make_delete_ui_cleanup): New declaration.
diff --git a/gdb/defs.h b/gdb/defs.h
index 5088390..fee5f41 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 548e468..e60475f 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 c64ec06..6e28a29 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 5c4305d..e776a0c 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 6f00baf..9df4efb 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 609d262..065225c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@ 
+2016-07-25  Tom Tromey  <tom@tromey.com>
+
+	PR python/20190:
+	* gdb.threads/tls.exp (check_thread_local): Add python symbol
+	test.
+
 2016-07-25  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb.base/new-ui.exp (do_test_invalid_args): New
diff --git a/gdb/testsuite/gdb.threads/tls.exp b/gdb/testsuite/gdb.threads/tls.exp
index 29384e5..5c07844 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,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 0b417b4..3299e87 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,