Re: [PATCH] fix info address for thread local vars

Re: [PATCH] fix info address for thread local vars

[Michael Elizabeth Chastain]

Elena Z writes:

  info address me
  Symbol "me" is a variable with complex or multiple locations (DWARF2).
  (gdb) FAIL: gdb.threads/tls.exp: info address me

I think this is a KFAIL.  Assuming that the information coming out of the
compiler is correct, then it is not an XFAIL.

The PR would be easy: gdb can't handle variables with complex or
multiple locations.

On my next spin I'll report on the numerous red hat linux 8
configurations and I can submit KFAIL-ish patches for the new tests.

Michael C

Re: [PATCH] fix info address for thread local vars

[Michael Elizabeth Chastain]

Hi Elena,

> I am also really tempted to commit the tls tests (to the mailine
> only), otherwise, the tls feature will bitrot. I'll work through
> refining the tests once they are in.

Could you file a PR like "tls tests not ready for prime time yet"
and then shovel in a bunch of setup_kfail?

It's cheesy but it might be better than letting the tests bit rot.
What do people think?

I'll even volunteer to sprinkle in the setup_kfail's if people
think this is a useful idea.

Michael C

Re: [PATCH] fix info address for thread local vars

[Elena Zannoni]

Michael Elizabeth Chastain writes:
 > Hi Elena,
 > 
 > > I am also really tempted to commit the tls tests (to the mailine
 > > only), otherwise, the tls feature will bitrot. I'll work through
 > > refining the tests once they are in.
 > 
 > Could you file a PR like "tls tests not ready for prime time yet"
 > and then shovel in a bunch of setup_kfail?
 > 
 > It's cheesy but it might be better than letting the tests bit rot.
 > What do people think?
 > 
 > I'll even volunteer to sprinkle in the setup_kfail's if people
 > think this is a useful idea.
 > 
 > Michael C


OK, after the commits I get these errors on a RHL system with all the
bits installed:

Running /home/ezannoni/gdb-sources/src/gdb/testsuite/gdb.threads/tls.exp ...
FAIL: gdb.threads/tls.exp: info address me

		=== gdb Summary ===

# of expected passes		59
# of unexpected failures	1


Running /home/ezannoni/gdb-sources/src/gdb/testsuite/gdb.threads/tls-shared.exp ...

		=== gdb Summary ===

# of expected passes		9


That failure is due to some other lossage with info address.
I think we need a test for info address in the testsuite....


info address me
Symbol "me" is a variable with complex or multiple locations (DWARF2).
(gdb) FAIL: gdb.threads/tls.exp: info address me


OK, should I xfail/kfail it? I'll create a PR for this.

elena

Re: [PATCH] fix info address for thread local vars

[Daniel Jacobowitz]

On Tue, Jul 22, 2003 at 02:40:13PM -0400, Elena Zannoni wrote:
> Michael Elizabeth Chastain writes:
>  > Hi Elena,
>  > 
>  > > I am also really tempted to commit the tls tests (to the mailine
>  > > only), otherwise, the tls feature will bitrot. I'll work through
>  > > refining the tests once they are in.
>  > 
>  > Could you file a PR like "tls tests not ready for prime time yet"
>  > and then shovel in a bunch of setup_kfail?
>  > 
>  > It's cheesy but it might be better than letting the tests bit rot.
>  > What do people think?
>  > 
>  > I'll even volunteer to sprinkle in the setup_kfail's if people
>  > think this is a useful idea.
>  > 
>  > Michael C
> 
> 
> OK, after the commits I get these errors on a RHL system with all the
> bits installed:
> 
> Running /home/ezannoni/gdb-sources/src/gdb/testsuite/gdb.threads/tls.exp ...
> FAIL: gdb.threads/tls.exp: info address me
> 
> 		=== gdb Summary ===
> 
> # of expected passes		59
> # of unexpected failures	1
> 
> 
> Running /home/ezannoni/gdb-sources/src/gdb/testsuite/gdb.threads/tls-shared.exp ...
> 
> 		=== gdb Summary ===
> 
> # of expected passes		9
> 
> 
> That failure is due to some other lossage with info address.
> I think we need a test for info address in the testsuite....
> 
> 
> info address me
> Symbol "me" is a variable with complex or multiple locations (DWARF2).
> (gdb) FAIL: gdb.threads/tls.exp: info address me
> 
> 
> OK, should I xfail/kfail it? I'll create a PR for this.

Sounds like a good idea.  Kfail, I think.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

[PATCH] fix info address for thread local vars

[Elena Zannoni]

Here is something to fix the output of info address on a TLS
variable. It makes it the same as the old output, with the same amount
of information. I want to commit this on both the branch and the
mainline. 

I am also really tempted to commit the tls tests (to the mailine
only), otherwise, the tls feature will bitrot. I'll work through
refining the tests once they are in.

elena

2003-07-21  Elena Zannoni  <ezannoni@redhat.com>

	* findvar.c (read_var_value): Remove case for thread local storage
	variables.  It is now entirely handled by the dwarf2 location
	expression code.
	* printcmd.c (address_info): Ditto.
	* symtab.h (address_class): Remove LOC_THREAD_LOCAL_STATIC
	enumeration value.
	(struct symbol): Remove objfile field, which was used by
	LOC_THREAD_LOCAL_STATIC only.
	* dwarf2read.c (decode_locdesc): Remove is_thread_local variable.
	* dwarf2loc.h (struct dwarf2_loclist_baton): Add comment about
	usage of objfile pointer.
	* dwarf2loc.c (locexpr_describe_location): Add case to handle
	thread local variables.
	Add include of objfiles.h.
	* dwarf2expr.c (execute_stack_op): Add comments about thread local
	storage variables.
	* Makefile.in (dwarf2loc.o): Update dependencies.

Index: Makefile.in
===================================================================
RCS file: /cvs/uberbaum/gdb/Makefile.in,v
retrieving revision 1.420
diff -u -p -r1.420 Makefile.in
--- Makefile.in	17 Jul 2003 12:49:52 -0000	1.420
+++ Makefile.in	21 Jul 2003 19:16:24 -0000
@@ -1681,7 +1681,8 @@ dwarf2expr.o: dwarf2expr.c $(defs_h) $(s
 	 $(gdbcore_h) $(dwarf2expr_h)
 dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_out_h) $(value_h) $(frame_h) \
 	$(gdbcore_h) $(target_h) $(inferior_h) $(dwarf2expr_h) \
-	$(dwarf2loc_h) $(ax_h) $(ax_gdb_h) $(regcache_h) $(gdb_string_h)
+	$(dwarf2loc_h) $(ax_h) $(ax_gdb_h) $(regcache_h) $(objfiles_h) \
+	$(gdb_string_h)
 dwarf2-frame.o: $(defs_h) $(dwarf2expr_h) $(elf_dwarf2_h) $(frame_h) \
 	$(frame_base_h) $(frame_unwind_h) $(gdbcore_h) $(gdbtypes_h) \
 	$(symtab_h) $(objfiles_h) $(regcache_h) $(gdb_assert_h) \
 
Index: dwarf2expr.c
===================================================================
RCS file: /cvs/uberbaum/gdb/dwarf2expr.c,v
retrieving revision 1.8
diff -u -p -r1.8 dwarf2expr.c
--- dwarf2expr.c	22 May 2003 18:37:05 -0000	1.8
+++ dwarf2expr.c	21 Jul 2003 19:16:25 -0000
@@ -641,6 +641,14 @@ execute_stack_op (struct dwarf_expr_cont
 	  break;
 
 	case DW_OP_GNU_push_tls_address:
+	  /* Variable is at a constant offset in the thread-local
+	  storage block into the objfile for the current thread and
+	  the dynamic linker module containing this expression. Here
+	  we return returns the offset from that base.  The top of the
+	  stack has the offset from the beginning of the thread
+	  control block at which the variable is located.  Nothing
+	  should follow this operator, so the top of stack would be
+	  returned.  */
 	  result = dwarf_expr_fetch (ctx, 0);
 	  dwarf_expr_pop (ctx);
 	  result = (ctx->get_tls_address) (ctx->baton, result);
Index: dwarf2loc.c
===================================================================
RCS file: /cvs/uberbaum/gdb/dwarf2loc.c,v
retrieving revision 1.9
diff -u -p -r1.9 dwarf2loc.c
--- dwarf2loc.c	22 May 2003 18:37:05 -0000	1.9
+++ dwarf2loc.c	21 Jul 2003 19:16:26 -0000
@@ -29,6 +29,7 @@
 #include "ax.h"
 #include "ax-gdb.h"
 #include "regcache.h"
+#include "objfiles.h"
 
 #include "elf/dwarf2.h"
 #include "dwarf2expr.h"
@@ -185,6 +186,8 @@ dwarf_expr_tls_address (void *baton, COR
     addr = target_get_thread_local_address (inferior_ptid,
 					    debaton->objfile,
 					    offset);
+  /* It wouldn't be wrong here to try a gdbarch method, too; finding
+     TLS is an ABI-specific thing.  But we don't do that yet.  */
   else
     error ("Cannot find thread-local variables on this target");
 
@@ -405,6 +408,34 @@ locexpr_describe_location (struct symbol
 			"a variable in register %s", REGISTER_NAME (regno));
       return 1;
     }
+
+  /* The location expression for a TLS variable looks like this (on a
+     64-bit LE machine):
+
+     DW_AT_location    : 10 byte block: 3 4 0 0 0 0 0 0 0 e0
+                        (DW_OP_addr: 4; DW_OP_GNU_push_tls_address)
+     
+     0x3 is the encoding for DW_OP_addr, which has an operand as long
+     as the size of an address on the target machine (here is 8
+     bytes).  0xe0 is the encoding for DW_OP_GNU_push_tls_address.
+     The operand represents the offset at which the variable is within
+     the thread local storage.  */
+
+  if (dlbaton->size > 1 
+      && dlbaton->data[dlbaton->size - 1] == DW_OP_GNU_push_tls_address)
+    if (dlbaton->data[0] == DW_OP_addr)
+      {
+	int bytes_read;
+	CORE_ADDR offset = dwarf2_read_address (&dlbaton->data[1],
+						&dlbaton->data[dlbaton->size - 2],
+						&bytes_read);
+	fprintf_filtered (stream, 
+			  "a thread-local variable at offset %s in the"
+			  "thread-local storage for `%s'",
+			  paddr_nz (offset), dlbaton->objfile->name);
+	return 1;
+      }
+  
 
   fprintf_filtered (stream,
 		    "a variable with complex or multiple locations (DWARF2)");
Index: dwarf2loc.h
===================================================================
RCS file: /cvs/uberbaum/gdb/dwarf2loc.h,v
retrieving revision 1.2
diff -u -p -r1.2 dwarf2loc.h
--- dwarf2loc.h	13 Apr 2003 15:43:35 -0000	1.2
+++ dwarf2loc.h	21 Jul 2003 19:16:26 -0000
@@ -55,6 +55,10 @@ struct dwarf2_loclist_baton
   unsigned short size;
 
   /* The objfile containing the symbol whose location we're computing.  */
+  /* Used (only???) by thread local variables.  The objfile in which
+     this symbol is defined.  To find a thread-local variable (e.g., a
+     variable declared with the `__thread' storage class), we may need
+     to know which object file it's in.  */
   struct objfile *objfile;
 };
 
Index: dwarf2read.c
===================================================================
RCS file: /cvs/uberbaum/gdb/dwarf2read.c,v
retrieving revision 1.97
diff -u -p -r1.97 dwarf2read.c
--- dwarf2read.c	26 Jun 2003 21:20:39 -0000	1.97
+++ dwarf2read.c	21 Jul 2003 19:16:32 -0000
@@ -438,12 +438,6 @@ static int islocal;		/* Variable is at t
 				   this function, so we can't say
 				   which register it's relative to;
 				   use LOC_LOCAL.  */
-static int is_thread_local;     /* Variable is at a constant offset in the
-                                   thread-local storage block for the
-                                   current thread and the dynamic linker
-                                   module containing this expression.
-                                   decode_locdesc returns the offset from
-                                   that base.  */
 
 /* DW_AT_frame_base values for the current function.
    frame_base_reg is -1 if DW_AT_frame_base is missing, otherwise it
@@ -6788,7 +6782,6 @@ decode_locdesc (struct dwarf_block *blk,
   offreg = 0;
   isderef = 0;
   islocal = 0;
-  is_thread_local = 0;
   optimized_out = 1;
 
   while (i < size)
@@ -7014,7 +7007,6 @@ decode_locdesc (struct dwarf_block *blk,
 	  break;
 
         case DW_OP_GNU_push_tls_address:
-          is_thread_local = 1;
 	  /* The top of the stack has the offset from the beginning
 	     of the thread control block at which the variable is located.  */
 	  /* Nothing should follow this operator, so the top of stack would
Index: findvar.c
===================================================================
RCS file: /cvs/uberbaum/gdb/findvar.c,v
retrieving revision 1.61
diff -u -p -r1.61 findvar.c
--- findvar.c	7 Jul 2003 14:36:57 -0000	1.61
+++ findvar.c	21 Jul 2003 19:16:33 -0000
@@ -512,20 +512,6 @@ addresses have not been bound by the dyn
 	break;
       }
 
-    case LOC_THREAD_LOCAL_STATIC:
-      {
-        if (target_get_thread_local_address_p ())
-          addr = target_get_thread_local_address (inferior_ptid,
-                                                  SYMBOL_OBJFILE (var),
-                                                  SYMBOL_VALUE_ADDRESS (var));
-        /* It wouldn't be wrong here to try a gdbarch method, too;
-           finding TLS is an ABI-specific thing.  But we don't do that
-           yet.  */
-        else
-          error ("Cannot find thread-local variables on this target");
-        break;
-      }
-
     case LOC_TYPEDEF:
       error ("Cannot look up value of a typedef");
       break;
Index: printcmd.c
===================================================================
RCS file: /cvs/uberbaum/gdb/printcmd.c,v
retrieving revision 1.63
diff -u -p -r1.63 printcmd.c
--- printcmd.c	9 Jun 2003 15:20:21 -0000	1.63
+++ printcmd.c	21 Jul 2003 19:16:36 -0000
@@ -1262,12 +1262,6 @@ address_info (char *exp, int from_tty)
 			val, REGISTER_NAME (basereg));
       break;
 
-    case LOC_THREAD_LOCAL_STATIC:
-      printf_filtered ("a thread-local variable at offset %ld in the "
-                       "thread-local storage for `%s'",
-                       val, SYMBOL_OBJFILE (sym)->name);
-      break;
-
     case LOC_OPTIMIZED_OUT:
       printf_filtered ("optimized out");
       break;
Index: symtab.h
===================================================================
RCS file: /cvs/uberbaum/gdb/symtab.h,v
retrieving revision 1.76
diff -u -p -r1.76 symtab.h
--- symtab.h	26 Jun 2003 17:18:42 -0000	1.76
+++ symtab.h	21 Jul 2003 19:16:42 -0000
@@ -482,14 +482,6 @@ enum address_class
 
   LOC_HP_THREAD_LOCAL_STATIC,
 
-  /* Value is at a thread-specific location calculated by a
-     target-specific method.  SYMBOL_OBJFILE gives the object file
-     in which the symbol is defined; the symbol's value is the
-     offset into that objfile's thread-local storage for the current
-     thread.  */
-      
-  LOC_THREAD_LOCAL_STATIC,
-
   /* The variable does not actually exist in the program.
      The value is ignored.  */
 
@@ -605,12 +597,6 @@ struct symbol
   {
     /* Used by LOC_BASEREG and LOC_BASEREG_ARG.  */
     short basereg;
-
-    /* Used by LOC_THREAD_LOCAL_STATIC.  The objfile in which this
-       symbol is defined.  To find a thread-local variable (e.g., a
-       variable declared with the `__thread' storage class), we may
-       need to know which object file it's in.  */
-    struct objfile *objfile;
 
     /* For a LOC_COMPUTED or LOC_COMPUTED_ARG symbol, this is the
        baton and location_funcs structure to find its location.  For a

Re: [PATCH] fix info address for thread local vars

[Daniel Jacobowitz]

On Mon, Jul 21, 2003 at 03:49:27PM -0400, Elena Zannoni wrote:
> 
> Here is something to fix the output of info address on a TLS
> variable. It makes it the same as the old output, with the same amount
> of information. I want to commit this on both the branch and the
> mainline. 

Looks perfect to me.  Thanks for cleaning up!

> I am also really tempted to commit the tls tests (to the mailine
> only), otherwise, the tls feature will bitrot. I'll work through
> refining the tests once they are in.

Sure, sounds good to me too.  If they fail here I'll be more motivated
to work out any testsuite bugs in order to make them XFAIL when
necessary.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] fix info address for thread local vars

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Mon, Jul 21, 2003 at 03:49:27PM -0400, Elena Zannoni wrote:
 > > 
 > > Here is something to fix the output of info address on a TLS
 > > variable. It makes it the same as the old output, with the same amount
 > > of information. I want to commit this on both the branch and the
 > > mainline. 
 > 
 > Looks perfect to me.  Thanks for cleaning up!
 > 
 > > I am also really tempted to commit the tls tests (to the mailine
 > > only), otherwise, the tls feature will bitrot. I'll work through
 > > refining the tests once they are in.
 > 
 > Sure, sounds good to me too.  If they fail here I'll be more motivated
 > to work out any testsuite bugs in order to make them XFAIL when
 > necessary.
 > 
 > 

OK, I checked in the changes to info addr to both mainline and gdb-6.
I checked in the tests only to mainline. I'll do a first round of
cleanup for the testsuite now.

elena


 > -- 
 > Daniel Jacobowitz
 > MontaVista Software                         Debian GNU/Linux Developer