[ping] Ignore thread local symbols when generating dbx debug info

[Jim Wilson]

On Thu, 2008-02-07 at 19:43 -0500, John David Anglin wrote:
> Second ping.
> > http://gcc.gnu.org/ml/gcc-patches/2007-12/msg01166.html

Belatedly replying to this, now that I'm doing FSF gcc work again... I
had to spend a bit of time looking at the code.  This assumes some
knowledge of emutls, fortran, and hpux, all of which I am lacking.  I do
see the problem though.  A emutls variable access requires a call to
__emutls_get_address, and then we find the variable at a fixed offset
from the return value.  There is no way to express this in the stabs
debug info.

It is a bit annoying that the emutls.c file has no comments explaining
what it does.  It doesn't even have comments for each function as
required by the coding conventions.  It would be easier to understand if
this stuff was documented some place.

I thought it curious that this was failing for Fortran, but not
apparently for C.  Otherwise presumably you would have given a C
testcase.  I also wasn't able to reproduce this with a trivial C
example.  I tried debugging this a little bit.  I see that the normal
variable handling in expand_expr_real_1, case VAR_DECL, has an out for
emutls variables.  We create the necessary code to access it, and then
return, before we use DECL_RTL, which calls make_decl_rtl which sets the
DECL_RTL field.  Since the emutls variable has no DECL_RTL, we don't try
to emit debug info for it.  However, with the Fortran testcase, the
DECL_RTL got created in nonoverlapping_memrefs_p during the cse pass.
This seems questionable to me.  With emutls, a variable may or may not
have a DECL_RTL set, depending on how it was optimized.  This could lead
to confusion later.  It may also be causing subtle bugs.  For instance,
mudflap has hooks in make_decl_rtl, which may or may not be called with
emutls, depending on how it was optimized, which means mudflap may or
may not be tracking the variable.  Perhaps we should be creating the
DECL_RTL always in expand_expr_real_1.  Perhaps make_decl_rtl should
have support for emutls variables.

Anyways, the dbxout.c change looks basically OK to me.  It would be nice
if there was a comment explaining what the patch is doing.  You are
right that canonical form for RTL will always put the SYMBOL_REF before
the CONST_INT in a PLUS.  It looks like we only have to handle the
output of dbxout_expand_expr, so we don't have to handle any more
complicated expressions.  However, this points to another possible
solution, which looks a little simpler to me.  We can put this test in
dbxout_expand_expr.  I attached a patch that does this.

Trying my patch on the testcase, I see the result is a little bit
different than yours.  With my patch, debug info is disabled for foo,
thrc, bar1, and bar2.  With your patch, debug info is disable for foo,
thrc, and bar2.  Which means you still have debug info for bar1.  The
debug info seems confused for bar1 though.  Internally, gcc thinks it is
addressed relative to thrc_, and then doesn't emit the address because
it is a global, so it slips through your checks.

Can you test my patch and see if it works?  If not, then your patch is
fine, preferably with a comment added like the one in my patch.

Jim
-------------- next part --------------
2008-03-04  James E. Wilson  <wilson@tuliptree.org>

	* dbxout.c (dbxout_expand_expr, case VAR_DECL): Return NULL for
	emulated thread local variables.

Index: dbxout.c
===================================================================
--- dbxout.c	(revision 132829)
+++ dbxout.c	(working copy)
@@ -2332,6 +2332,15 @@
   switch (TREE_CODE (expr))
     {
     case VAR_DECL:
+      /* We can't handle emulated tls variables, because the address is an
+	 offset to the return value of __emutls_get_address, and there is no
+	 way to express that in stabs.  Also, there are name mangling issues
+	 here.  We end up with references to undefined symbols if we don't
+	 disable debug info for these variables.  */
+      if (!targetm.have_tls && DECL_THREAD_LOCAL_P (expr))
+	return NULL;
+      /* FALLTHRU */
+
     case PARM_DECL:
       if (DECL_HAS_VALUE_EXPR_P (expr))
 	return dbxout_expand_expr (DECL_VALUE_EXPR (expr));