Re: RFC: initial TLS patch

[Andrew Cagney <ac131313 at ges dot redhat dot com>]

> +                   else if (is_thread_local)
> +                     {
> +                       SYMBOL_CLASS (sym) = LOC_THREAD_LOCAL_STATIC;
> +                       SYMBOL_OBJFILE (sym) = objfile;
> +                       SYMBOL_VALUE_ADDRESS (sym) = addr;
> +                     }
Can I suggest awarding this mechanism a new LOC name.  That way the past 
history of LOC_THREAD_LOCAL_STATIC won't flow into this new mechanism. 
As for thread local static, looks like a separate pass can purge GDB of it.



> !   TD_NOTALLOC	  /* TLS memory not yet allocated.  */
Suggest TLS -> Thread [Local] memory


> + extern struct so_list *get_solib_by_objfile (struct objfile *OBJFILE);
Tipo OBJFILE -> objfile.


> Index: gdb/target.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.c,v
> retrieving revision 1.36
> diff -c -r1.36 target.c
> *** gdb/target.c	15 Jun 2002 21:07:58 -0000	1.36
> --- gdb/target.c	29 Jun 2002 03:38:48 -0000
> ***************
> *** 610,615 ****
> --- 610,616 ----
>         INHERIT (to_async_mask_value, t);
>         INHERIT (to_find_memory_regions, t);
>         INHERIT (to_make_corefile_notes, t);
> +       INHERIT (to_get_thread_local_value, t);
>         INHERIT (to_magic, t);
Does this also need some debug stuff added?

>   #undef INHERIT
> Index: gdb/target.h
> + 
> +     /* Return the thread-local value of type TYPE at OFFSET in the
> +        thread-local storage for the thread PTID and the shared library
> +        or executable file given by OBJFILE.  If that block of
> +        thread-local storage hasn't been allocated yet, this function
> +        may return an error, or try to concoct an appropriate
> +        (non-lvalue) value based on the initialization image.  */
> +     struct value *(*to_get_thread_local_value) (ptid_t PTID,
> +                                                 struct objfile *OBJFILE,
> +                                                 CORE_ADDR OFFSET,
Per other [discussion] thread.  I understand that at this time all that 
this needs to compute / return are: thread storage base address; 
conversion status - not allocated, readonly, read-write.  Having it 
return something more complicated like a ``struct value'' can be left to 
the person that actually needs the mechanism - I figure they will be in 
a better position to determine exactly what mechanism is needed.

Perhaphs there should be a separate ``struct location'' object?


> ***************
> *** 1021,1026 ****
> --- 1038,1050 ----
>   
>   #define target_make_corefile_notes(BFD, SIZE_P) \
>        (current_target.to_make_corefile_notes) (BFD, SIZE_P)
> + 
> + 
> + /* Thread-local values.  */
> + #define target_get_thread_local_value \
> +     (current_target.to_get_thread_local_value)
> + #define target_has_get_thread_local_value() \
> +     (target_get_thread_local_value != 0)
``_has_'' in the target vector appears to be used when testing 
attributes such as ``the target has memory'' so 
target_get_thread_local_value_p() would be better.  Suggest ``!= NULL''.



> +       if (! so)
> +         error ((objfile_is_library
> +                 ? ("Cannot find shared library `%s' in dynamic linker's "
> +                    "module list")
> +                 : ("Cannot find executable file `%s' in dynamic linker's "
> +                    "module list")),
> +                objfile->name);
Can I suggest writing these as:
	if (objfile_is_library)
	  error (...)
	else
	  error (...)
it is going to make checking and for correct i18n much easier.  All 
errors will match:
	error[[:space:]]\(_\"[A-Z]


> + #ifdef THREAD_DB_DEFINES_TD_NOTALLOC
I suspect you mean THREAD_DB_HAS_TD_NOTALLOC, if it were a #define, 
#ifdef TD_NOTALLOC would work :-)

enjoy,
Andrew