[Bug middle-end/53093] New: [4.8 Regression]: tls/alias-1.c ICE, emutls

[Bug middle-end/53093] New: [4.8 Regression]: tls/alias-1.c ICE, emutls

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

             Bug #: 53093
           Summary: [4.8 Regression]: tls/alias-1.c ICE, emutls
    Classification: Unclassified
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hp@gcc.gnu.org
                CC: hubicka@gcc.gnu.org
            Target: cris-axis-elf


With revision 186677 this test passed.
>From revision 186688 and on, this test has failed as follows:

Running /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.dg/tls/tls.exp ...
FAIL: gcc.dg/tls/alias-1.c (internal compiler error)
FAIL: gcc.dg/tls/alias-1.c (test for excess errors)

With the message in gcc.log being:

Executing on host: /tmp/hpautotest-gcc1/cris-elf/gccobj/gcc/xgcc
-B/tmp/hpautotest-gcc1/cris-elf/gccobj/gcc/
/tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.dg/tls/alias-1.c 
-fno-diagnostics-show-caret    -ansi -pedantic-errors   -isystem
/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/./newlib/targ-include -isystem
/tmp/hpautotest-gcc1/gcc/newlib/libc/include
-B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/./libgloss/cris/
-L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/./libgloss/cris
-L/tmp/hpautotest-gcc1/gcc/libgloss/cris 
-B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/./newlib/
-L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/./newlib -sim3  -lm   -o
alias-1.exe    (timeout = 300)
/tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.dg/tls/alias-1.c:24:3: internal
compiler error: vector VEC(varpool_node_ptr,base) push domain error, in
create_emultls_var at tree-emutls.c:709
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
compiler exited with status 1

Author of suspect patch in revision range CC:ed.

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hubicka at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-04-24
     Ever Confirmed|0                           |1

--- Comment #1 from Jan Hubicka <hubicka at gcc dot gnu.org> 2012-04-24 13:09:20 UTC ---
mine. Will try to reproduce it on a cross.

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hubicka at ucw dot cz]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

--- Comment #2 from Jan Hubicka <hubicka at ucw dot cz> 2012-04-25 17:06:41 UTC ---
Hi,
the problem seems to be quite easy.  We have variable and alias.
The code first counts number of variables and allocated vectors, then it
inserts
aliases, too, and the length of vector is not enough anymore.

Can you, please, test the following patch?  I will try to work out why this
did not ICE before.

Honza

Index: tree-emutls.c
===================================================================
--- tree-emutls.c       (revision 186831)
+++ tree-emutls.c       (working copy)
@@ -706,7 +706,7 @@ create_emultls_var (struct varpool_node 
   cdecl = new_emutls_decl (var->symbol.decl, var->alias_of);

   cvar = varpool_get_node (cdecl);
-  VEC_quick_push (varpool_node_ptr, control_vars, cvar);
+  VEC_safe_push (varpool_node_ptr, heap, control_vars, cvar);

   if (!var->alias)
     {

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

--- Comment #3 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-04-26 14:20:44 UTC ---
(In reply to comment #2)
> Can you, please, test the following patch?  I will try to work out why this
> did not ICE before.

Works for me; fixes the regression, x to cris-elf at r186817.

(For future reference please *attach* patches: I managed to mis-type it.)
thanks.

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hubicka at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

--- Comment #4 from Jan Hubicka <hubicka at gcc dot gnu.org> 2012-04-26 21:13:35 UTC ---
Created attachment 27250
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27250
proposed fix.

Thank you.
I actually found why the numbers does not match. I was wrong with my previous
analysis - the aliases are counted in first loop.

The issue is that when handling aliases the code is creating new aliases as it
walks the tree.  It depends on luck whether the walk will pick the new alias or
not. Can you, please try once again the attached patch? It should restore the
original behavior.

I am not sure that the code makes sense: I would expect TLS var of alias to be
alias of the TLS var of the target. The code however creates alias of the
target instead. I can't think of explanation how that would make sense.

Does the aliases behave sanely with emutls?

Honza

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

--- Comment #5 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-04-26 23:43:39 UTC ---
(In reply to comment #4)
> Can you, please try once again the attached patch? It should restore the
> original behavior.

In progress.  Considering the amount of gcc work you do, it wouldn't be
unexpected that you could do it too, it's the same setup for all
simulator-enabled targets and you have all needed code at your fingertips and
likely already on your computer.  Same old simtest-howto.html I've been nagging
about for years.

> Does the aliases behave sanely with emutls?

How do I tell?

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hubicka at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

--- Comment #6 from Jan Hubicka <hubicka at gcc dot gnu.org> 2012-04-27 12:03:00 UTC ---
> > Does the aliases behave sanely with emutls?
> 
> How do I tell?

Honestly I hoped you will know. Emutls is not my area.  I will try to play with
it on i386 target. Should not be too hard to figure out if the behaviour is
sane.
Basically I would expect that creating two module testcase and accessing same
TLS var with the var and alias should break with current implementation.

Thanks!
Honza

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

--- Comment #7 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-04-27 13:15:51 UTC ---
(In reply to comment #6)
> > > Does the aliases behave sanely with emutls?
> > How do I tell?
> Honestly I hoped you will know. Emutls is not my area.

I should have mentioned that the other tests covering emutls work, so perhaps I
could answer the first question with a weak "yes". :)

> Basically I would expect that creating two module testcase and accessing same
> TLS var with the var and alias should break with current implementation.

That might be so, but I don't know how to map "module" to a target which
doesn't have DSO's.  If you just mean different object files, then please do
add such a test, if there is none!  BTW, *-darwin* might be better for thorough
emutls testing.

Testing the patch in comment#4 finished: success.
thanks.

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hubicka at ucw dot cz]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

--- Comment #8 from Jan Hubicka <hubicka at ucw dot cz> 2012-05-02 14:34:02 UTC ---
> > > > Does the aliases behave sanely with emutls?
> > > How do I tell?
> > Honestly I hoped you will know. Emutls is not my area.
> 
> I should have mentioned that the other tests covering emutls work, so perhaps I
> could answer the first question with a weak "yes". :)

Hi,
so I managed to get a testcase that fails:
/* { dg-do link } */
/* { dg-require-alias "" } */
/* { dg-require-visibility "" } */
/* { dg-require-effective-target tls_runtime } */
/* Test that encode_section_info handles the change from externally
   defined to locally defined (via hidden).   Extracted from glibc.  */

struct __res_state {
        char x[123];
};

extern __thread struct __res_state bar
  __attribute__ ((tls_model ("initial-exec")));

__thread struct __res_state foo;
extern __thread struct __res_state bar
  __attribute__ ((alias ("foo")))
  __attribute__ ((visibility ("hidden")));
int main()
{ 
  bar.x[0] = 15;
  if (foo.x[0]!= 15)
    abort ();
  return 0;
}

I think the bug exists at least in GCC 4.7, but it may be interesting to try
older compilers, too.

I am testing the following fix on x86_64 with emutls enabled.  Basically the
bug is as I described: we want emutls variable of alias to be alias of emutls
variable of target, not the alias of target.  DECL_VALUE_EXPR is used to
translate vars to emutls vars.

Honza

Index: tree-emutls.c
===================================================================
--- tree-emutls.c    (revision 187011)
+++ tree-emutls.c    (working copy)
@@ -338,7 +338,7 @@ new_emutls_decl (tree decl, tree alias_o
   else 
     varpool_create_variable_alias (to,
                    varpool_node_for_asm
-                    (DECL_ASSEMBLER_NAME (alias_of))->symbol.decl);
+                    (DECL_ASSEMBLER_NAME (DECL_VALUE_EXPR
(alias_of)))->symbol.decl);
   return to;
 }

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hubicka at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

--- Comment #9 from Jan Hubicka <hubicka at gcc dot gnu.org> 2012-05-03 19:22:07 UTC ---
Author: hubicka
Date: Thu May  3 19:22:03 2012
New Revision: 187115

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187115
Log:

    PR middle-end/53093
    * tree-emutls.c (new_emutls_decl): Fix handling of aliases.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-emutls.c

[Bug middle-end/53093] [4.8 Regression]: tls/alias-1.c ICE, emutls

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53093

Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #10 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-05-31 09:59:17 UTC ---
fixed