On Mon, 01 Dec 2008 19:29:55 +0100, Ulrich Weigand wrote:

Followed your advices, updated the patch.


Thanks,
Jan

------------------------------------------------------------------------------

> Jan Kratochvil wrote:
> 
> > +	/* SYMBOL_VALUE_ADDRESS should return the translated address.  */
> 
> I'm not sure I understand this comment -- the translated address of
> a thread-local variable obviously depends on the thread in which it
> is evaluated, so how should a single location like SYMBOL_VALUE_ADDRESS
> be able to hold that value for all threads?

SYMBOL_VALUE_ADDRESS is used many times across GDB code and I assumed in most
of such cases it would be more appropriate to return the absolute address
valid for current inferior_ptid.

But I now briefly checked all these points and found out in fact all of them
work with SYMBOL_VALUE_ADDRESS referring to function symbols - PC values (just
except value_static_field() but that is also not in use with TLS symbols).
Sure PC values are never TLS-translated.


> > +	    /* SYMBOL_VALUE_ADDRESS should return the translated address.  */
> > +	    if (section
> > +		&& (section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0)
> > +	      {
> > +		printf_filtered (_("a thread-local variable at offset %s "
> > +				   "at final address "), paddr_nz (load_addr));
> > +		load_addr = target_translate_tls_address (section->objfile,
> > +							  load_addr);
> > +	      }
> 
> Again, I think "at final address" may be misleading; if we give an absolute
> address, it should explicitly mention it is relative to the current thread.

`final' was put there as it printed before:

Symbol "X" is a thread-local variable at offset 0x1234 at address 0xabcd0000.

and I found it ambiguous whether the bytes of the variable X are in fact
placed at (a) address 0xabcd0000 or (b) address 0xabcd1234.


> In the LOC_COMPUTED case, we'd output something like "at offset ...
> in the thread-local storage for ...".  For consistency reasons, I'd
> prefer to have the same output in the LOC_UNDEFINED case as well.

While I copied the message from the LOC_COMPUTED case I agree it should be
consistent and if I wish the absolute address there it should be printed even
in the LOC_COMPUTED case which would be more a scope of a different patch.


> On the other hand, I'm wondering if we should perform the resolution here
> at all; isn't "info address" also allowed when the target is not actually
> running, so we don't even have a current thread?

While I forgot about this case I find the current behavior appropriate:

(gdb) info address a_thread_local
Symbol "a_thread_local" is a thread-local variable at offset 0 at final address Cannot find thread-local variables on this target

But it is no longer printed due to the consistency advice above.


On Mon, 01 Dec 2008 19:29:31 +0100, Gary Funck wrote:
> --- gdb/printcmd.c      20 Nov 2008 16:13:11 -0000      1.138
> +++ gdb/printcmd.c      30 Nov 2008 21:10:06 -0000
> @@ -1234,6 +1234,7 @@ address_info (char *exp, int from_tty)
> +       struct obj_section *obj_section;
> Is obj_section referenced?

Removed.