GCC 3.1 Prerelease

GCC 3.1 Prerelease

[Mark Mitchell]

I'm now spinning the prerelease.  Assuming it works, the tarballs will
be on the FTP server by the type I wake up tomorrow morning...

From this point hence, please do not make any further non-documentation
check-ins to the GCC 3.1 branch without my explicit approval.

There are a handful of open bugs remaining that need fixes.  By far and
away the most worrisome are the IRIX regressions in PR 6212 and PR 6304.
These are significant problems (crashes in the generated code).  The
first allegedly stems from a patch by Kenner -- although one problematic
part of that patch has already been reverted.  The second problem allegedly
stems from a patch by Jan.  Gentlemen, your help in tracking down these
problems would be very much appreciated.

I unfortunately at temporarily without access to my IRIX development
platform; my access to ASCI bluemountain should be back soon, but I
don't have it now.  So, we're going to need someone else to look at
these if possible.

Joseph, if you have time to look at PR 6343 (C front end regression
involving "attribute((weak))"), please do so.  I can imagine this
being a significant problem.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: GCC 3.1 Prerelease

[Alan Modra]

On Tue, Apr 23, 2002 at 02:01:25AM -0700, Mark Mitchell wrote:
> 
> Joseph, if you have time to look at PR 6343 (C front end regression
> involving "attribute((weak))"), please do so.  I can imagine this
> being a significant problem.

I've been using this, which at least cures the problem with
__register_frame_info*.  Credit for the patch goes to Franz Sirl.

diff -urpN -xCVS -x'*~' -xTAGS gcc-31.orig/gcc/c-decl.c gcc-31/gcc/c-decl.c
--- gcc-31.orig/gcc/c-decl.c	2002-04-03 09:00:04.000000000 +0930
+++ gcc-31/gcc/c-decl.c	2002-04-23 18:06:31.000000000 +0930
@@ -1955,7 +1955,16 @@ duplicate_decls (newdecl, olddecl, diffe
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+  if (DECL_WEAK (newdecl) && DECL_RTL (newdecl)
+      && GET_CODE (DECL_RTL (newdecl)) == MEM
+      && XEXP (DECL_RTL (newdecl), 0)
+      && GET_CODE (XEXP (DECL_RTL (newdecl), 0)) == SYMBOL_REF)
+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (newdecl), 0)) = 1;
+
   /* For functions, static overrides non-static.  */
   if (TREE_CODE (newdecl) == FUNCTION_DECL)
     {
diff -urpN -xCVS -x'*~' -xTAGS gcc-31.orig/gcc/cp/decl.c gcc-31/gcc/cp/decl.c
--- gcc-31.orig/gcc/cp/decl.c	2002-04-17 18:56:57.000000000 +0930
+++ gcc-31/gcc/cp/decl.c	2002-04-23 18:06:31.000000000 +0930
@@ -3645,7 +3645,15 @@ duplicate_decls (newdecl, olddecl)
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+  if (DECL_WEAK (newdecl) && DECL_RTL (newdecl)
+      && GET_CODE (DECL_RTL (newdecl)) == MEM
+      && XEXP (DECL_RTL (newdecl), 0)
+      && GET_CODE (XEXP (DECL_RTL (newdecl), 0)) == SYMBOL_REF)
+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (newdecl), 0)) = 1;
   DECL_ONE_ONLY (newdecl) |= DECL_ONE_ONLY (olddecl);
   DECL_DEFER_OUTPUT (newdecl) |= DECL_DEFER_OUTPUT (olddecl);
   TREE_PUBLIC (newdecl) = TREE_PUBLIC (olddecl);


-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: GCC 3.1 Prerelease

[Franz Sirl]

At 12:28 23.04.2002, Alan Modra wrote:
>On Tue, Apr 23, 2002 at 02:01:25AM -0700, Mark Mitchell wrote:
> >
> > Joseph, if you have time to look at PR 6343 (C front end regression
> > involving "attribute((weak))"), please do so.  I can imagine this
> > being a significant problem.
>
>I've been using this, which at least cures the problem with
>__register_frame_info*.  Credit for the patch goes to Franz Sirl.

Hmm, have you tried recompiling glibc with this patch? I seem to recall 
glibc using constructs that would produce an error now.

Anyway, I'll look into that stuff again later today.

Franz.



>diff -urpN -xCVS -x'*~' -xTAGS gcc-31.orig/gcc/c-decl.c gcc-31/gcc/c-decl.c
>--- gcc-31.orig/gcc/c-decl.c    2002-04-03 09:00:04.000000000 +0930
>+++ gcc-31/gcc/c-decl.c 2002-04-23 18:06:31.000000000 +0930
>@@ -1955,7 +1955,16 @@ duplicate_decls (newdecl, olddecl, diffe
>      }
>
>    /* Merge the storage class information.  */
>-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
>+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
>+    declare_weak (olddecl);
>+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
>+    declare_weak (newdecl);
>+  if (DECL_WEAK (newdecl) && DECL_RTL (newdecl)
>+      && GET_CODE (DECL_RTL (newdecl)) == MEM
>+      && XEXP (DECL_RTL (newdecl), 0)
>+      && GET_CODE (XEXP (DECL_RTL (newdecl), 0)) == SYMBOL_REF)
>+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (newdecl), 0)) = 1;
>+
>    /* For functions, static overrides non-static.  */
>    if (TREE_CODE (newdecl) == FUNCTION_DECL)
>      {
>diff -urpN -xCVS -x'*~' -xTAGS gcc-31.orig/gcc/cp/decl.c gcc-31/gcc/cp/decl.c
>--- gcc-31.orig/gcc/cp/decl.c   2002-04-17 18:56:57.000000000 +0930
>+++ gcc-31/gcc/cp/decl.c        2002-04-23 18:06:31.000000000 +0930
>@@ -3645,7 +3645,15 @@ duplicate_decls (newdecl, olddecl)
>      }
>
>    /* Merge the storage class information.  */
>-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
>+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
>+    declare_weak (olddecl);
>+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
>+    declare_weak (newdecl);
>+  if (DECL_WEAK (newdecl) && DECL_RTL (newdecl)
>+      && GET_CODE (DECL_RTL (newdecl)) == MEM
>+      && XEXP (DECL_RTL (newdecl), 0)
>+      && GET_CODE (XEXP (DECL_RTL (newdecl), 0)) == SYMBOL_REF)
>+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (newdecl), 0)) = 1;
>    DECL_ONE_ONLY (newdecl) |= DECL_ONE_ONLY (olddecl);
>    DECL_DEFER_OUTPUT (newdecl) |= DECL_DEFER_OUTPUT (olddecl);
>    TREE_PUBLIC (newdecl) = TREE_PUBLIC (olddecl);
>
>
>--
>Alan Modra
>IBM OzLabs - Linux Technology Centre

Re: GCC 3.1 Prerelease

[Alan Modra]

On Tue, Apr 23, 2002 at 01:04:39PM +0200, Franz Sirl wrote:
> At 12:28 23.04.2002, Alan Modra wrote:
> >On Tue, Apr 23, 2002 at 02:01:25AM -0700, Mark Mitchell wrote:
> >>
> >> Joseph, if you have time to look at PR 6343 (C front end regression
> >> involving "attribute((weak))"), please do so.  I can imagine this
> >> being a significant problem.
> >
> >I've been using this, which at least cures the problem with
> >__register_frame_info*.  Credit for the patch goes to Franz Sirl.
> 
> Hmm, have you tried recompiling glibc with this patch? I seem to recall 
> glibc using constructs that would produce an error now.

I've been compiling a glibc snapshot with a last ChangeLog entry of
2002-01-18  Andreas Schwab  <schwab@suse.de>

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: GCC 3.1 Prerelease

[Franz Sirl]

[-- Attachment #1: Type: text/plain, Size: 1376 bytes --]

At 13:13 23.04.2002, Alan Modra wrote:
>On Tue, Apr 23, 2002 at 01:04:39PM +0200, Franz Sirl wrote:
> > At 12:28 23.04.2002, Alan Modra wrote:
> > >On Tue, Apr 23, 2002 at 02:01:25AM -0700, Mark Mitchell wrote:
> > >>
> > >> Joseph, if you have time to look at PR 6343 (C front end regression
> > >> involving "attribute((weak))"), please do so.  I can imagine this
> > >> being a significant problem.
> > >
> > >I've been using this, which at least cures the problem with
> > >__register_frame_info*.  Credit for the patch goes to Franz Sirl.
> >
> > Hmm, have you tried recompiling glibc with this patch? I seem to recall
> > glibc using constructs that would produce an error now.
>
>I've been compiling a glibc snapshot with a last ChangeLog entry of
>2002-01-18  Andreas Schwab  <schwab@suse.de>

OK, try compiling glibc with the attached patch, it should give you some 
warnings :-).

What do you all think about this patch? It covers all the cases I could 
think of, but maybe I missed something. Please take a close look at my 
changes to declare_weak, I'm not too sure my DECL checking is 100% 
correct/allowed. Can someone think of a better warning message? And what 
about a merge_weak() function (naming? in which file?) instead of 
duplicating the code?

The testcases are planned for the dg framework, but I haven't added all the 
dg funkiness yet :-).

Franz.


[-- Attachment #2: gcc-weaksym-4.patch --]
[-- Type: application/octet-stream, Size: 3000 bytes --]

Index: gcc/c-decl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.300.2.5
diff -u -p -r1.300.2.5 c-decl.c
--- gcc/c-decl.c	28 Mar 2002 18:49:57 -0000	1.300.2.5
+++ gcc/c-decl.c	23 Apr 2002 16:51:11 -0000
@@ -1955,7 +1955,16 @@ duplicate_decls (newdecl, olddecl, diffe
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+  if (DECL_WEAK (newdecl) && DECL_RTL (newdecl)
+      && GET_CODE (DECL_RTL (newdecl)) == MEM
+      && XEXP (DECL_RTL (newdecl), 0)
+      && GET_CODE (XEXP (DECL_RTL (newdecl), 0)) == SYMBOL_REF)
+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (newdecl), 0)) = 1;
+
   /* For functions, static overrides non-static.  */
   if (TREE_CODE (newdecl) == FUNCTION_DECL)
     {
Index: gcc/varasm.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.250.2.7
diff -u -p -r1.250.2.7 varasm.c
--- gcc/varasm.c	25 Mar 2002 00:54:26 -0000	1.250.2.7
+++ gcc/varasm.c	23 Apr 2002 16:51:11 -0000
@@ -4998,12 +4998,14 @@ declare_weak (decl)
 {
   if (! TREE_PUBLIC (decl))
     error_with_decl (decl, "weak declaration of `%s' must be public");
-  else if (TREE_ASM_WRITTEN (decl))
+  else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_ASM_WRITTEN (decl))
     error_with_decl (decl, "weak declaration of `%s' must precede definition");
   else if (SUPPORTS_WEAK)
     {
       if (! DECL_WEAK (decl))
 	weak_decls = tree_cons (NULL, decl, weak_decls);
+      if (TREE_USED (decl))
+	warning_with_decl (decl, "weak declaration of `%s' after first use may result in unspecified behaviour");
     }
   else
     warning_with_decl (decl, "weak declaration of `%s' not supported");
Index: gcc/cp/decl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.866.2.23
diff -u -p -r1.866.2.23 decl.c
--- gcc/cp/decl.c	16 Apr 2002 03:15:54 -0000	1.866.2.23
+++ gcc/cp/decl.c	23 Apr 2002 16:51:12 -0000
@@ -3645,7 +3645,16 @@ duplicate_decls (newdecl, olddecl)
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+  if (DECL_WEAK (newdecl) && DECL_RTL (newdecl)
+      && GET_CODE (DECL_RTL (newdecl)) == MEM
+      && XEXP (DECL_RTL (newdecl), 0)
+      && GET_CODE (XEXP (DECL_RTL (newdecl), 0)) == SYMBOL_REF)
+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (newdecl), 0)) = 1;
+
   DECL_ONE_ONLY (newdecl) |= DECL_ONE_ONLY (olddecl);
   DECL_DEFER_OUTPUT (newdecl) |= DECL_DEFER_OUTPUT (olddecl);
   TREE_PUBLIC (newdecl) = TREE_PUBLIC (olddecl);

[-- Attachment #3: weak-2.c --]
[-- Type: text/plain, Size: 2508 bytes --]


// test function addresses with __attribute__((weak))
extern void * ffoo1a (void) __attribute__((weak));
extern void * ffoo1a (void);
void * foo1a (void)
{
  return (void *)ffoo1a;
}

extern void * ffoo2a (void);
extern void * ffoo2a (void) __attribute__((weak));
void * foo2a (void)
{
  return (void *)ffoo2a;
}

extern void * ffoo3a (void);
void * foo3a (void)
{
  return (void *)ffoo3a;
}
extern void * ffoo3a (void) __attribute__((weak));



// test function addresses with #pragma weak
#pragma weak ffoo1b
extern void * ffoo1b (void);
void * foo1b (void)
{
  return (void *)ffoo1b;
}

extern void * ffoo2b (void);
#pragma weak ffoo2b
void * foo2b (void)
{
  return (void *)ffoo2b;
}

extern void * ffoo3b (void);
void * foo3b (void)
{
  return (void *)ffoo3b;
}
#pragma weak ffoo3b




// test variable addresses with __attribute__((weak))
extern int vfoo1c __attribute__((weak));
extern int vfoo1c;
void * foo1c (void)
{
  return (void *)&vfoo1c;
}

extern int vfoo2c;
extern int vfoo2c __attribute__((weak));
void * foo2c (void)
{
  return (void *)&vfoo2c;
}

extern int vfoo3c;
void * foo3c (void)
{
  return (void *)&vfoo3c;
}
extern int vfoo3c __attribute__((weak));

extern int vfoo4c __attribute__((weak));
int vfoo4c;
void * foo4c (void)
{
  return (void *)&vfoo4c;
}

int vfoo5c;
extern int vfoo5c __attribute__((weak));
void * foo5c (void)
{
  return (void *)&vfoo5c;
}

int vfoo6c;
void * foo6c (void)
{
  return (void *)&vfoo6c;
}
extern int vfoo6c __attribute__((weak));

extern int vfoo7c;
void * foo7c (void)
{
  return (void *)&vfoo7c;
}
int vfoo7c __attribute__((weak));

extern int vfoo8c __attribute__((weak));
void * foo8c (void)
{
  return (void *)&vfoo8c;
}
extern int vfoo8c __attribute__((weak));
int vfoo8c;


// test variable addresses with #pragma weak
#pragma weak vfoo1d
extern int vfoo1d;
void * foo1d (void)
{
  return (void *)&vfoo1d;
}

extern int vfoo2d;
#pragma weak vfoo2d
void * foo2d (void)
{
  return (void *)&vfoo2d;
}

extern int vfoo3d;
void * foo3d (void)
{
  return (void *)&vfoo3d;
}
#pragma weak vfoo3d

#pragma weak vfoo4d
int vfoo4d;
void * foo4d (void)
{
  return (void *)&vfoo4d;
}

int vfoo5d;
#pragma weak vfoo5d
void * foo5d (void)
{
  return (void *)&vfoo5d;
}

int vfoo6d;
void * foo6d (void)
{
  return (void *)&vfoo6d;
}
#pragma weak vfoo6d

extern int vfoo7d;
void * foo7d (void)
{
  return (void *)&vfoo7d;
}
#pragma weak vfoo7d
int vfoo7d;

extern int vfoo8d;
void * foo8d (void)
{
  return (void *)&vfoo8d;
}
int vfoo8d;
#pragma weak vfoo8d

[-- Attachment #4: weak-3.c --]
[-- Type: text/plain, Size: 120 bytes --]



extern void * foo (void);
void * foo (void)
{
  return (void *)foo;
}
extern void * foo (void) __attribute__((weak));

[-- Attachment #5: weak-4.c --]
[-- Type: text/plain, Size: 89 bytes --]



extern void * foo (void);
void * foo (void)
{
  return (void *)foo;
}
#pragma weak foo

Re: GCC 3.1 Prerelease

[Richard Henderson]

On Tue, Apr 23, 2002 at 07:28:22PM +0200, Franz Sirl wrote:
> What do you all think about this patch? It covers all the cases I could 
> think of, but maybe I missed something.

I don't see why functions should be singled out in having to have
the weakening before the definition.

Otherwise it looks ok.


r~

Re: GCC 3.1 Prerelease

[Franz Sirl]

[-- Attachment #1: Type: text/plain, Size: 1548 bytes --]

On Tuesday 23 April 2002 20:04, Richard Henderson wrote:
> On Tue, Apr 23, 2002 at 07:28:22PM +0200, Franz Sirl wrote:
> > What do you all think about this patch? It covers all the cases I could
> > think of, but maybe I missed something.
>
> I don't see why functions should be singled out in having to have
> the weakening before the definition.

Hmm, I didn't want to get too aggressive in here (declare_weak wasn't called 
for VAR_DECLs before) and judging from the comments in tree.h it looks like 
TREE_ASM_WRITTEN has a slightly different meaning for a VAR_DECL. But if you 
think it's OK, I'll happily change it.

Looking at the warnings in glibc it turned out I was already too aggressive, 
the patch would warn for:

extern double strtod (__const char *__restrict __nptr,
                      char **__restrict __endptr);

extern double __strtod_internal (__const char *__restrict __nptr,
                                 char **__restrict __endptr, int __group);

extern __inline double
  strtod (__const char *__restrict __nptr, char **__restrict __endptr)
{
  return __strtod_internal (__nptr, __endptr, 0);
}

extern __inline double
  atof (__const char *__nptr)
{
    return strtod (__nptr, (char **) ((void *)0));
}

double __attribute__ ((weak))
  strtod (nptr, endptr)
    const char *nptr;
    char **endptr;
{
  return __strtod_internal (nptr, endptr, 0 );
}

I don't think we want a warning here for strtod?

Appended an updated patch with Jason's suggestions integrated as well. I'll 
run a few more tests tomorrow.

Franz.


[-- Attachment #2: gcc-weaksym-5.patch --]
[-- Type: text/plain, Size: 2826 bytes --]

Index: gcc/c-decl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.300.2.5
diff -u -p -r1.300.2.5 c-decl.c
--- gcc/c-decl.c	28 Mar 2002 18:49:57 -0000	1.300.2.5
+++ gcc/c-decl.c	23 Apr 2002 21:41:28 -0000
@@ -1955,7 +1955,11 @@ duplicate_decls (newdecl, olddecl, diffe
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+
   /* For functions, static overrides non-static.  */
   if (TREE_CODE (newdecl) == FUNCTION_DECL)
     {
Index: gcc/varasm.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.250.2.7
diff -u -p -r1.250.2.7 varasm.c
--- gcc/varasm.c	25 Mar 2002 00:54:26 -0000	1.250.2.7
+++ gcc/varasm.c	23 Apr 2002 21:41:28 -0000
@@ -4998,17 +4998,25 @@ declare_weak (decl)
 {
   if (! TREE_PUBLIC (decl))
     error_with_decl (decl, "weak declaration of `%s' must be public");
-  else if (TREE_ASM_WRITTEN (decl))
+  else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_ASM_WRITTEN (decl))
     error_with_decl (decl, "weak declaration of `%s' must precede definition");
   else if (SUPPORTS_WEAK)
     {
       if (! DECL_WEAK (decl))
 	weak_decls = tree_cons (NULL, decl, weak_decls);
+      if (TREE_CODE (decl) != FUNCTION_DECL && TREE_USED (decl))
+	warning_with_decl (decl, "weak declaration of `%s' after first use may result in unspecified behaviour");
     }
   else
     warning_with_decl (decl, "weak declaration of `%s' not supported");
 
   DECL_WEAK (decl) = 1;
+
+  if (DECL_RTL_SET_P (decl)
+      && GET_CODE (DECL_RTL (decl)) == MEM
+      && XEXP (DECL_RTL (decl), 0)
+      && GET_CODE (XEXP (DECL_RTL (decl), 0)) == SYMBOL_REF)
+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (decl), 0)) = 1;
 }
 
 /* Emit any pending weak declarations.  */
Index: gcc/cp/decl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.866.2.23
diff -u -p -r1.866.2.23 decl.c
--- gcc/cp/decl.c	16 Apr 2002 03:15:54 -0000	1.866.2.23
+++ gcc/cp/decl.c	23 Apr 2002 21:41:28 -0000
@@ -3645,7 +3645,11 @@ duplicate_decls (newdecl, olddecl)
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+
   DECL_ONE_ONLY (newdecl) |= DECL_ONE_ONLY (olddecl);
   DECL_DEFER_OUTPUT (newdecl) |= DECL_DEFER_OUTPUT (olddecl);
   TREE_PUBLIC (newdecl) = TREE_PUBLIC (olddecl);

Re: GCC 3.1 Prerelease

[Richard Henderson]

On Wed, Apr 24, 2002 at 12:03:42AM +0200, Franz Sirl wrote:
> (declare_weak wasn't called for VAR_DECLs before)

Huh?  It must have been.

> ... and judging from the comments in tree.h it looks like 
> TREE_ASM_WRITTEN has a slightly different meaning for a VAR_DECL.

What makes you think that?

> I don't think we want a warning here for strtod?

I can make an argument either way.  I'm inclined to warn, since
older gcc would in fact render this into rtl right away, which 
has then already made decisions based on DECL_WEAK.


r~

Re: GCC 3.1 Prerelease

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

While I was looking at this bug earlier (before I saw that you were working
on it), I made this change.  Checking TREE_SYMBOL_REFERENCED makes more
sense than TREE_USED anyway, since it's the symbol we care about.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 652 bytes --]

*** varasm.c.~1~	Wed Apr 24 01:04:56 2002
--- varasm.c	Tue Apr 23 15:43:46 2002
*************** weak_finish ()
*** 5022,5028 ****
        tree decl = TREE_VALUE (t);
        const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
  
!       if (! TREE_USED (decl))
  	continue;
  
  #ifdef ASM_WEAKEN_DECL
--- 5022,5030 ----
        tree decl = TREE_VALUE (t);
        const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
  
!       /* We can't rely on TREE_USED here, as decl might be a discarded
! 	 duplicate.  */
!       if (! TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
  	continue;
  
  #ifdef ASM_WEAKEN_DECL

Re: GCC 3.1 Prerelease

[Franz Sirl]

On Wednesday 24 April 2002 19:31, Jason Merrill wrote:
> While I was looking at this bug earlier (before I saw that you were working
> on it), I made this change.  Checking TREE_SYMBOL_REFERENCED makes more
> sense than TREE_USED anyway, since it's the symbol we care about.

Hmm, it seems to make more sense for the warning check too, with TREE_USED 
changed to TREE_SYMBOL_REFERENCED the c++ regression 
g++.old-deja/g++.jason/template39.C went away along with a bunch of 
regressions in the libstdc++ testsuite, except one:

FAIL: 26_numerics/complex_inserters_extractors.cc (test for excess errors)
Excess errors:
/home/fsirl/TC/gcc/BUILD/obj-gcc31-ppc/ppc-linux/libstdc++-v3/include/bits/basic_string.h: 
In instantiation of `const size_t std::basic_string<char, gnu_cha
r_traits, std::allocator<char> >::npos':
/home/fsirl/TC/gcc/BUILD/gcc-3.1/libstdc++-v3/testsuite/26_numerics/complex_inserters_extractors.cc:109:   
instantiated from here
/home/fsirl/TC/gcc/BUILD/obj-gcc31-ppc/ppc-linux/libstdc++-v3/include/bits/basic_string.h:217: 
warning: weak declaration of `const size_t std::basic_string<
char, gnu_char_traits, std::allocator<char> >::npos' after first use may 
result in unspecified behaviour

Can a c++ expert tell me if this warning makes sense in this testcase or does 
the warning check need still more refinement?

Hmm, Jason, I'm just noticing the purpose of the test in weak_finish. This 
seems to be a behaviour change compared to earlier gcc releases. A simple 
file like that:

#pragma weak foo1
extern int foo2 __attribute__((weak));

will now result in this assembly file:

        .file   "weak-pragma.c"
        .ident  "GCC: (GNU) 3.1 20020423 (prerelease)"

But upto 3.0.x we got:

        .file   "weak-pragma.c"
        .weak   foo2
        .weak   foo1
        .ident  "GCC: (GNU) 3.0.3 20011213 (prerelease)"

Are we sure there is no code out there relying on these lonely .weak 
statements? Was this change intended?

Franz.

Re: GCC 3.1 Prerelease

[Richard Henderson]

On Wed, Apr 24, 2002 at 09:01:39PM +0200, Franz Sirl wrote:
> Are we sure there is no code out there relying on these lonely .weak 
> statements? Was this change intended?

Yes and yes.


r~

Re: GCC 3.1 Prerelease

[Jason Merrill]

>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:

> Hmm, it seems to make more sense for the warning check too, with TREE_USED 
> changed to TREE_SYMBOL_REFERENCED the c++ regression 
> g++.old-deja/g++.jason/template39.C went away along with a bunch of 
> regressions in the libstdc++ testsuite, except one:

> FAIL: 26_numerics/complex_inserters_extractors.cc (test for excess errors)
> Excess errors:
> /home/fsirl/TC/gcc/BUILD/obj-gcc31-ppc/ppc-linux/libstdc++-v3/include/bits/basic_string.h: 
> In instantiation of `const size_t std::basic_string<char, gnu_char_traits, std::allocator<char> >::npos':
> /home/fsirl/TC/gcc/BUILD/gcc-3.1/libstdc++-v3/testsuite/26_numerics/complex_inserters_extractors.cc:109:   
> instantiated from here
> /home/fsirl/TC/gcc/BUILD/obj-gcc31-ppc/ppc-linux/libstdc++-v3/include/bits/basic_string.h:217: 
> warning: weak declaration of `const size_t std::basic_string<char,
> gnu_char_traits, std::allocator<char> >::npos' after first use may result
> in unspecified behaviour

> Can a c++ expert tell me if this warning makes sense in this testcase or does 
> the warning check need still more refinement?

The latter.  We don't want to warn about the C++ frontend's internal
trickery with DECL_WEAK.

I'd prefer to omit the warning entirely on the branch.

> Hmm, Jason, I'm just noticing the purpose of the test in weak_finish.
...
> Are we sure there is no code out there relying on these lonely .weak 
> statements? Was this change intended?

I believe so.  But Richard was the one who made the change.

Jason

[PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Franz Sirl]

[-- Attachment #1: Type: text/plain, Size: 2228 bytes --]

On Wednesday 24 April 2002 22:01, Jason Merrill wrote:
> >>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:
> >
> > Hmm, it seems to make more sense for the warning check too, with
> > TREE_USED changed to TREE_SYMBOL_REFERENCED the c++ regression
> > g++.old-deja/g++.jason/template39.C went away along with a bunch of
> > regressions in the libstdc++ testsuite, except one:
> >
> > FAIL: 26_numerics/complex_inserters_extractors.cc (test for excess
> > errors) Excess errors:
> > /home/fsirl/TC/gcc/BUILD/obj-gcc31-ppc/ppc-linux/libstdc++-v3/include/bit
> >s/basic_string.h: In instantiation of `const size_t
> > std::basic_string<char, gnu_char_traits, std::allocator<char> >::npos':
> > /home/fsirl/TC/gcc/BUILD/gcc-3.1/libstdc++-v3/testsuite/26_numerics/compl
> >ex_inserters_extractors.cc:109: instantiated from here
> > /home/fsirl/TC/gcc/BUILD/obj-gcc31-ppc/ppc-linux/libstdc++-v3/include/bit
> >s/basic_string.h:217: warning: weak declaration of `const size_t
> > std::basic_string<char, gnu_char_traits, std::allocator<char> >::npos'
> > after first use may result in unspecified behaviour
> >
> > Can a c++ expert tell me if this warning makes sense in this testcase or
> > does the warning check need still more refinement?
>
> The latter.  We don't want to warn about the C++ frontend's internal
> trickery with DECL_WEAK.

What kind of trickery? Can I detect that in declare_weak?

> I'd prefer to omit the warning entirely on the branch.

Well, I prefer overzealous warnings over no warning at all in the same way I 
prefer ICEs over miscompiled code :-). Besides that extra warning, I'm really 
satisfied now with my patch otherwise and unless you think it's overly 
complicated to get rid of it, I would like to fix it even for the branch.

Attached the patch I successfully bootstrapped and regtested (minus the 
additional libstdc++ fail) on powerpc-linux-gnu and x86-linux-gnu.

Franz.

	PR c/6343
	* c-decl.c (duplicate_decls): Use declare_weak to merge weak status.
	* varasm.c (declare_weak): Make sure we don't give an error on VAR_DECLs.
	Warn about potential miscompilations.
	Mark RTL with SYMBOL_REF_WEAK.

cp:
	* decl.c (duplicate_decls): Use declare_weak to merge weak status.


[-- Attachment #2: gcc-weaksym-6.patch --]
[-- Type: text/plain, Size: 2826 bytes --]

Index: gcc/c-decl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.300.2.5
diff -u -p -r1.300.2.5 c-decl.c
--- gcc/c-decl.c	28 Mar 2002 18:49:57 -0000	1.300.2.5
+++ gcc/c-decl.c	23 Apr 2002 21:41:28 -0000
@@ -1955,7 +1955,11 @@ duplicate_decls (newdecl, olddecl, diffe
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+
   /* For functions, static overrides non-static.  */
   if (TREE_CODE (newdecl) == FUNCTION_DECL)
     {
Index: gcc/varasm.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.250.2.7
diff -u -p -r1.250.2.7 varasm.c
--- gcc/varasm.c	25 Mar 2002 00:54:26 -0000	1.250.2.7
+++ gcc/varasm.c	23 Apr 2002 21:41:28 -0000
@@ -4998,17 +4998,25 @@ declare_weak (decl)
 {
   if (! TREE_PUBLIC (decl))
     error_with_decl (decl, "weak declaration of `%s' must be public");
-  else if (TREE_ASM_WRITTEN (decl))
+  else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_ASM_WRITTEN (decl))
     error_with_decl (decl, "weak declaration of `%s' must precede definition");
   else if (SUPPORTS_WEAK)
     {
       if (! DECL_WEAK (decl))
 	weak_decls = tree_cons (NULL, decl, weak_decls);
+      if (TREE_CODE (decl) != FUNCTION_DECL && TREE_USED (decl))
+	warning_with_decl (decl, "weak declaration of `%s' after first use may result in unspecified behaviour");
     }
   else
     warning_with_decl (decl, "weak declaration of `%s' not supported");
 
   DECL_WEAK (decl) = 1;
+
+  if (DECL_RTL_SET_P (decl)
+      && GET_CODE (DECL_RTL (decl)) == MEM
+      && XEXP (DECL_RTL (decl), 0)
+      && GET_CODE (XEXP (DECL_RTL (decl), 0)) == SYMBOL_REF)
+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (decl), 0)) = 1;
 }
 
 /* Emit any pending weak declarations.  */
Index: gcc/cp/decl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.866.2.23
diff -u -p -r1.866.2.23 decl.c
--- gcc/cp/decl.c	16 Apr 2002 03:15:54 -0000	1.866.2.23
+++ gcc/cp/decl.c	23 Apr 2002 21:41:28 -0000
@@ -3645,7 +3645,11 @@ duplicate_decls (newdecl, olddecl)
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+
   DECL_ONE_ONLY (newdecl) |= DECL_ONE_ONLY (olddecl);
   DECL_DEFER_OUTPUT (newdecl) |= DECL_DEFER_OUTPUT (olddecl);
   TREE_PUBLIC (newdecl) = TREE_PUBLIC (olddecl);

Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Jason Merrill]

>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:

> On Wednesday 24 April 2002 22:01, Jason Merrill wrote:

>> The latter.  We don't want to warn about the C++ frontend's internal
>> trickery with DECL_WEAK.

> What kind of trickery? Can I detect that in declare_weak?

In the C++ frontend, in general an entity with vague linkage (such as a
template instantion) has DECL_EXTERNAL set until EOF, at which point we
decide what to do with it.  If we can't tell whether or not this is the One
True translation unit for the entity, we mark it weak/comdat and emit it as
needed.

>> I'd prefer to omit the warning entirely on the branch.

> Well, I prefer overzealous warnings over no warning at all in the same way I 
> prefer ICEs over miscompiled code :-). Besides that extra warning, I'm really 
> satisfied now with my patch otherwise and unless you think it's overly 
> complicated to get rid of it, I would like to fix it even for the branch.

The warning should only apply to weak externs; the address of a weak
definition is never 0.  That should fix the C++ case above.

Jason

Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Franz Sirl]

[-- Attachment #1: Type: text/plain, Size: 1958 bytes --]

On Thursday 25 April 2002 21:54, Jason Merrill wrote:
> >>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:
> >
> > On Wednesday 24 April 2002 22:01, Jason Merrill wrote:
> >> The latter.  We don't want to warn about the C++ frontend's internal
> >> trickery with DECL_WEAK.
> >
> > What kind of trickery? Can I detect that in declare_weak?
>
> In the C++ frontend, in general an entity with vague linkage (such as a
> template instantion) has DECL_EXTERNAL set until EOF, at which point we
> decide what to do with it.  If we can't tell whether or not this is the One
> True translation unit for the entity, we mark it weak/comdat and emit it as
> needed.

OK, I added TREE_STATIC to the warning check and this killed the regression.

> >> I'd prefer to omit the warning entirely on the branch.
> >
> > Well, I prefer overzealous warnings over no warning at all in the same
> > way I prefer ICEs over miscompiled code :-). Besides that extra warning,
> > I'm really satisfied now with my patch otherwise and unless you think
> > it's overly complicated to get rid of it, I would like to fix it even for
> > the branch.
>
> The warning should only apply to weak externs; the address of a weak
> definition is never 0.  That should fix the C++ case above.

Ah, there's the forest :-). This cuts down the cases to warn about. I moved 
the check to a separate merge_weak function so I can check both OLDDECL and 
NEWDECL.

Bootstrapped without regressions on powerpc-linux-gnu and x86-linux-gnu.

OK to commit to mainline and branch?

Franz.

	PR c/6343
	* c-decl.c (duplicate_decls): Call merge_weak.
	* c-pragma.c (apply_pragma_weak): Warn about misuse.
	* output.h (merge_weak): Prototype merge_weak.
	* varasm.c (merge_weak): New function.
	(declare_weak): Make sure we don't give an error on VAR_DECLs.
	Mark RTL with SYMBOL_REF_WEAK.

cp:
	* decl.c (duplicate_decls): Call merge_weak.

testsuite:
	* gcc.dg/weak-[2-7].c: New tests.





[-- Attachment #2: gcc-weaksym-7.patch --]
[-- Type: text/plain, Size: 4553 bytes --]

Index: gcc/c-decl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.300.2.6
diff -u -p -r1.300.2.6 c-decl.c
--- gcc/c-decl.c	26 Apr 2002 00:23:32 -0000	1.300.2.6
+++ gcc/c-decl.c	28 Apr 2002 09:20:07 -0000
@@ -1955,7 +1955,8 @@ duplicate_decls (newdecl, olddecl, diffe
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  merge_weak (newdecl, olddecl);
+
   /* For functions, static overrides non-static.  */
   if (TREE_CODE (newdecl) == FUNCTION_DECL)
     {
Index: gcc/c-pragma.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/c-pragma.c,v
retrieving revision 1.46.6.3
diff -u -p -r1.46.6.3 c-pragma.c
--- gcc/c-pragma.c	22 Mar 2002 22:49:38 -0000	1.46.6.3
+++ gcc/c-pragma.c	28 Apr 2002 09:20:07 -0000
@@ -287,6 +287,10 @@ apply_pragma_weak (decl, value)
     decl_attributes (&decl, build_tree_list (get_identifier ("alias"),
 				             build_tree_list (NULL, value)),
 		     0);
+  if (SUPPORTS_WEAK && DECL_EXTERNAL (decl) && TREE_USED (decl)
+      && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
+    warning_with_decl (decl, "applying #pragma weak `%s' after first use may result in unspecified behaviour");
+
   declare_weak (decl);
 }
 
Index: gcc/output.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/output.h,v
retrieving revision 1.94.2.1
diff -u -p -r1.94.2.1 output.h
--- gcc/output.h	23 Apr 2002 08:11:21 -0000	1.94.2.1
+++ gcc/output.h	28 Apr 2002 09:20:07 -0000
@@ -231,6 +231,8 @@ extern void mergeable_constant_section	P
 
 /* Declare DECL to be a weak symbol.  */
 extern void declare_weak		PARAMS ((tree));
+/* Merge weak status.  */
+extern void merge_weak			PARAMS ((tree, tree));
 #endif /* TREE_CODE */
 
 /* Emit any pending weak declarations.  */
Index: gcc/varasm.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.250.2.7
diff -u -p -r1.250.2.7 varasm.c
--- gcc/varasm.c	25 Mar 2002 00:54:26 -0000	1.250.2.7
+++ gcc/varasm.c	28 Apr 2002 09:20:07 -0000
@@ -4990,6 +4990,32 @@ output_constructor (exp, size, align)
    to be emitted.  */
 static tree weak_decls;
 
+/* Merge weak status between NEWDECL and OLDDECL.  */
+
+void
+merge_weak (newdecl, olddecl)
+     tree newdecl;
+     tree olddecl;
+{
+  tree decl;
+
+  if ((! DECL_WEAK (newdecl) && ! DECL_WEAK (olddecl))
+      || (DECL_WEAK (newdecl) && DECL_WEAK (olddecl)))
+    return;
+
+  decl = DECL_WEAK (olddecl) ? newdecl : olddecl;
+
+  if (SUPPORTS_WEAK
+      && DECL_EXTERNAL (newdecl) && DECL_EXTERNAL (olddecl)
+      && (TREE_CODE (decl) != VAR_DECL
+	  || ! TREE_STATIC (decl))
+      && TREE_USED (decl)
+      && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
+    warning_with_decl (decl, "weak declaration of `%s' after first use may result in unspecified behaviour");
+
+  declare_weak (decl);
+}
+
 /* Declare DECL to be a weak symbol.  */
 
 void
@@ -4998,7 +5024,7 @@ declare_weak (decl)
 {
   if (! TREE_PUBLIC (decl))
     error_with_decl (decl, "weak declaration of `%s' must be public");
-  else if (TREE_ASM_WRITTEN (decl))
+  else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_ASM_WRITTEN (decl))
     error_with_decl (decl, "weak declaration of `%s' must precede definition");
   else if (SUPPORTS_WEAK)
     {
@@ -5009,6 +5035,12 @@ declare_weak (decl)
     warning_with_decl (decl, "weak declaration of `%s' not supported");
 
   DECL_WEAK (decl) = 1;
+
+  if (DECL_RTL_SET_P (decl)
+      && GET_CODE (DECL_RTL (decl)) == MEM
+      && XEXP (DECL_RTL (decl), 0)
+      && GET_CODE (XEXP (DECL_RTL (decl), 0)) == SYMBOL_REF)
+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (decl), 0)) = 1;
 }
 
 /* Emit any pending weak declarations.  */
Index: gcc/cp/decl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.866.2.25
diff -u -p -r1.866.2.25 decl.c
--- gcc/cp/decl.c	23 Apr 2002 23:52:07 -0000	1.866.2.25
+++ gcc/cp/decl.c	28 Apr 2002 09:20:08 -0000
@@ -3645,7 +3645,8 @@ duplicate_decls (newdecl, olddecl)
     }
 
   /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  merge_weak (newdecl, olddecl);
+
   DECL_ONE_ONLY (newdecl) |= DECL_ONE_ONLY (olddecl);
   DECL_DEFER_OUTPUT (newdecl) |= DECL_DEFER_OUTPUT (olddecl);
   TREE_PUBLIC (newdecl) = TREE_PUBLIC (olddecl);

[-- Attachment #3: weak-2.c --]
[-- Type: text/x-csrc, Size: 1397 bytes --]

/* { dg-do compile } */
/* { dg-options "-fno-common" } */

/* COFF does not support weak, and dg doesn't support UNSUPPORTED.  */
/* { dg-do compile { xfail *-*-coff i?86-pc-cygwin h8300-*-hms } } */

/* { dg-final { global target_triplet } } */
/* { dg-final { if [string match h8300-*-hms $target_triplet ] {return} } } */
/* { dg-final { if [string match i?86-pc-cygwin $target_triplet ] {return} } } */
/* { dg-final { if [string match *-*-coff $target_triplet ] {return} } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1a" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1b" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1c" } } */
/* { dg-final { scan-assembler-not "weak\[^ \t\]*\[ \t\]ffoo1d" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1e" } } */

/* test function addresses with #pragma weak */

#pragma weak ffoo1a
extern void * ffoo1a (void);
void * foo1a (void)
{
  return (void *)ffoo1a;
}

extern void * ffoo1b (void);
#pragma weak ffoo1b
void * foo1b (void)
{
  return (void *)ffoo1b;
}

extern void * ffoo1c (void);  /* { dg-warning "applying #pragma weak" "applying #pragma weak" } */
void * foo1c (void)
{
  return (void *)ffoo1c;
}
#pragma weak ffoo1c


int ffoo1d (void);
#pragma weak ffoo1d


extern void * ffoo1e (void);
#pragma weak ffoo1e
void * foo1e (void)
{
  if (ffoo1e)
    ffoo1e ();
  return 0;
}


[-- Attachment #4: weak-3.c --]
[-- Type: text/x-csrc, Size: 2133 bytes --]

/* { dg-do compile } */
/* { dg-options "-fno-common" } */

/* COFF does not support weak, and dg doesn't support UNSUPPORTED.  */
/* { dg-do compile { xfail *-*-coff i?86-pc-cygwin h8300-*-hms } } */

/* { dg-final { global target_triplet } } */
/* { dg-final { if [string match h8300-*-hms $target_triplet ] {return} } } */
/* { dg-final { if [string match i?86-pc-cygwin $target_triplet ] {return} } } */
/* { dg-final { if [string match *-*-coff $target_triplet ] {return} } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1a" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1b" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1c" } } */
/* { dg-final { scan-assembler-not "weak\[^ \t\]*\[ \t\]ffoo1d" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1e" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1f" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]ffoo1g" } } */

/* test function addresses with __attribute__((weak)) */

extern void * ffoo1a (void) __attribute__((weak));
extern void * ffoo1a (void);
void * foo1a (void)
{
  return (void *)ffoo1a;
}


extern void * ffoo1b (void);
extern void * ffoo1b (void) __attribute__((weak));
void * foo1b (void)
{
  return (void *)ffoo1b;
}


extern void * ffoo1c (void);  /* { dg-warning "weak declaration" "weak declaration" } */
void * foo1c (void)
{
  return (void *)ffoo1c;
}
extern void * ffoo1c (void) __attribute__((weak));


int ffoo1d (void);
int ffoo1d (void) __attribute__((weak));


extern void * ffoo1e (void);
extern void * ffoo1e (void)  __attribute__((weak));
void * foo1e (void)
{
  if (ffoo1e)
    ffoo1e ();
  return 0;
}


extern void * ffoo1f (void);    /* { dg-warning "weak declaration" "weak declaration" } */
extern void * ffoox1f (void);
void * foo1f (void)
{
  if (ffoo1f)
    ffoo1f ();
  return 0;
}
extern void * ffoo1f (void)  __attribute__((weak, alias ("ffoox1f")));


extern void * ffoo1g (void);
extern void * ffoox1g (void);
extern void * ffoo1g (void)  __attribute__((weak, alias ("ffoox1g")));
void * foo1g (void)
{
  if (ffoo1g)
    ffoo1g ();
  return 0;
}

[-- Attachment #5: weak-5.c --]
[-- Type: text/x-csrc, Size: 2807 bytes --]

/* { dg-do compile } */
/* { dg-options "-fno-common" } */

/* COFF does not support weak, and dg doesn't support UNSUPPORTED.  */
/* { dg-do compile { xfail *-*-coff i?86-pc-cygwin h8300-*-hms } } */

/* { dg-final { global target_triplet } } */
/* { dg-final { if [string match h8300-*-hms $target_triplet ] {return} } } */
/* { dg-final { if [string match i?86-pc-cygwin $target_triplet ] {return} } } */
/* { dg-final { if [string match *-*-coff $target_triplet ] {return} } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1a" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1b" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1c" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1d" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1e" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1f" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1g" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1h" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1i" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1j" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1k" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1l" } } */

/* test variable addresses with __attribute__ ((weak)) */

extern int vfoo1a __attribute__((weak));
extern int vfoo1a;
void * foo1a (void)
{
  return (void *)&vfoo1a;
}


extern int vfoo1b;
extern int vfoo1b __attribute__((weak));
void * foo1b (void)
{
  return (void *)&vfoo1b;
}


extern int vfoo1c;  /* { dg-warning "weak declaration" "weak declaration" } */
void * foo1c (void)
{
  return (void *)&vfoo1c;
}
extern int vfoo1c __attribute__((weak));


extern int vfoo1d __attribute__((weak));
int vfoo1d;
void * foo1d (void)
{
  return (void *)&vfoo1d;
}


int vfoo1e;
extern int vfoo1e __attribute__((weak));
void * foo1e (void)
{
  return (void *)&vfoo1e;
}


int vfoo1f;
void * foo1f (void)
{
  return (void *)&vfoo1f;
}
extern int vfoo1f __attribute__((weak));


extern int vfoo1g;
void * foo1g (void)
{
  return (void *)&vfoo1g;
}
int vfoo1g __attribute__((weak));


extern int vfoo1h __attribute__((weak));
void * foo1h (void)
{
  return (void *)&vfoo1h;
}
extern int vfoo1h __attribute__((weak));
int vfoo1h;


extern int vfoo1i __attribute__((weak));
void * foo1i (void)
{
  return (void *)&vfoo1i;
}
extern int vfoo1i __attribute__((weak));
extern int vfoo1i;


extern int vfoo1j __attribute__((weak));
void * foo1j (void)
{
  return (void *)&vfoo1j;
}
extern int vfoo1j;
extern int vfoo1j __attribute__((weak));


extern int vfoo1k __attribute__((weak));
int vfoo1k = 1;


int vfoox1l = 1;
extern int vfoo1l __attribute__((alias ("vfoox1l")));
extern int vfoo1l __attribute__((weak, alias ("vfoox1l")));


[-- Attachment #6: weak-4.c --]
[-- Type: text/x-csrc, Size: 2385 bytes --]

/* { dg-do compile } */
/* { dg-options "-fno-common" } */

/* COFF does not support weak, and dg doesn't support UNSUPPORTED.  */
/* { dg-do compile { xfail *-*-coff i?86-pc-cygwin h8300-*-hms } } */

/* { dg-final { global target_triplet } } */
/* { dg-final { if [string match h8300-*-hms $target_triplet ] {return} } } */
/* { dg-final { if [string match i?86-pc-cygwin $target_triplet ] {return} } } */
/* { dg-final { if [string match *-*-coff $target_triplet ] {return} } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1a" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1b" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1c" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1d" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1e" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1f" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1g" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1h" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1i" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1j" } } */
/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]vfoo1k" } } */

/* test variable addresses with #pragma weak */

#pragma weak vfoo1a
extern int vfoo1a;
void * foo1a (void)
{
  return (void *)&vfoo1a;
}


extern int vfoo1b;
#pragma weak vfoo1b
void * foo1b (void)
{
  return (void *)&vfoo1b;
}


extern int vfoo1c;  /* { dg-warning "applying #pragma weak" "applying #pragma weak" } */
void * foo1c (void)
{
  return (void *)&vfoo1c;
}
#pragma weak vfoo1c


#pragma weak vfoo1d
int vfoo1d;
void * foo1d (void)
{
  return (void *)&vfoo1d;
}


int vfoo1e;
#pragma weak vfoo1e
void * foo1e (void)
{
  return (void *)&vfoo1e;
}


int vfoo1f;
void * foo1f (void)
{
  return (void *)&vfoo1f;
}
#pragma weak vfoo1f


extern int vfoo1g;  /* { dg-warning "applying #pragma weak" "applying #pragma weak" } */
void * foo1g (void)
{
  return (void *)&vfoo1g;
}
#pragma weak vfoo1g
int vfoo1g;


extern int vfoo1h;
void * foo1h (void)
{
  return (void *)&vfoo1h;
}
int vfoo1h;
#pragma weak vfoo1h


int vfoo1i;
extern int vfoo1i;
void * foo1i (void)
{
  return (void *)&vfoo1i;
}
#pragma weak vfoo1i


extern int vfoo1j;
int vfoo1j;
void * foo1j (void)
{
  return (void *)&vfoo1j;
}
#pragma weak vfoo1j


#pragma weak vfoo1k
int vfoo1k = 1;


[-- Attachment #7: weak-6.c --]
[-- Type: text/x-csrc, Size: 140 bytes --]

/* { dg-do compile } */

extern void * foo (void);
void * foo (void) { return (void *)foo; } /* { dg-error "precede" } */

#pragma weak foo

[-- Attachment #8: weak-7.c --]
[-- Type: text/x-csrc, Size: 171 bytes --]

/* { dg-do compile } */

extern void * foo (void);
void * foo (void) { return (void *)foo; } /* { dg-error "precede" } */

extern void * foo (void) __attribute__((weak));

Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Mark Mitchell]

+  if ((! DECL_WEAK (newdecl) && ! DECL_WEAK (olddecl))
+      || (DECL_WEAK (newdecl) && DECL_WEAK (olddecl)))
+    return;
+

Just:

  if (DECL_WEAK (newdecl) == DECL_WEAK (olddecl))
    return;

seems easier to read to me.

+    warning_with_decl (decl, "applying #pragma weak `%s' after first use 
may result in unspecified behaviour");
+

Just say "results in unspecified behavior" or even "is invalid".  There's
no reason to be so cautious; either the construct is legal or it isn't.

OK for mainline and branch with these changes.

Thanks for working so hard on this!

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Jason Merrill]

>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:

> +  if (SUPPORTS_WEAK
> +      && DECL_EXTERNAL (newdecl) && DECL_EXTERNAL (olddecl)
> +      && (TREE_CODE (decl) != VAR_DECL
> +	  || ! TREE_STATIC (decl))
> +      && TREE_USED (decl)
> +      && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
> +    warning_with_decl (decl, "weak declaration of `%s' after first use may result in unspecified behaviour");

I intended that TREE_SYMBOL_REFERENCED should be used rather than
TREE_USED, not in addition.  I'd probably check DECL_ASSEMBLER_NAME_SET_P
here, too; if the name hasn't been generated, it hasn't been referenced.
Perhaps we want a new macro, say

#define DECL_SYMBOL_REFERENCED(DECL) \
 (DECL_ASSEMBLER_NAME_SET_P (DECL) \
  && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (DECL)))

Thanks,
Jason

Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Mark Mitchell]

--On Sunday, April 28, 2002 10:57:55 PM +0100 Jason Merrill 
<jason@redhat.com> wrote:

>>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:
>
>> +  if (SUPPORTS_WEAK
>> +      && DECL_EXTERNAL (newdecl) && DECL_EXTERNAL (olddecl)
>> +      && (TREE_CODE (decl) != VAR_DECL
>> +	  || ! TREE_STATIC (decl))
>> +      && TREE_USED (decl)
>> +      && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
>> +    warning_with_decl (decl, "weak declaration of `%s' after first use
>> may result in unspecified behaviour");
>
> I intended that TREE_SYMBOL_REFERENCED should be used rather than
> TREE_USED, not in addition.  I'd probably check DECL_ASSEMBLER_NAME_SET_P
> here, too; if the name hasn't been generated, it hasn't been referenced.
> Perhaps we want a new macro, say
>
># define DECL_SYMBOL_REFERENCED(DECL) \
>  (DECL_ASSEMBLER_NAME_SET_P (DECL) \
>   && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (DECL)))

What Jason says makes sense to me.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Franz Sirl]

On Sunday 28 April 2002 23:57, Jason Merrill wrote:
> >>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:
> >
> > +  if (SUPPORTS_WEAK
> > +      && DECL_EXTERNAL (newdecl) && DECL_EXTERNAL (olddecl)
> > +      && (TREE_CODE (decl) != VAR_DECL
> > +	  || ! TREE_STATIC (decl))
> > +      && TREE_USED (decl)
> > +      && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
> > +    warning_with_decl (decl, "weak declaration of `%s' after first use
> > may result in unspecified behaviour");
>
> I intended that TREE_SYMBOL_REFERENCED should be used rather than
> TREE_USED, not in addition.  I'd probably check DECL_ASSEMBLER_NAME_SET_P
> here, too; if the name hasn't been generated, it hasn't been referenced.
> Perhaps we want a new macro, say
>
> #define DECL_SYMBOL_REFERENCED(DECL) \
>  (DECL_ASSEMBLER_NAME_SET_P (DECL) \
>   && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (DECL)))

But this would warn for these cases, which I think isn't correct:

// case 1
extern void * ffoo1g (void);
extern void * ffoox1g (void);
extern void * ffoo1g (void)  __attribute__((weak, alias ("ffoox1g")));
void * foo1g (void)
{
  if (ffoo1g)
    ffoo1g ();
  return 0;
}

// case 2
extern int vfoo1i __attribute__((weak));
void * foo1i (void)
{
  return (void *)&vfoo1i;
}
extern int vfoo1i __attribute__((weak));
extern int vfoo1i;

// case 3
extern int vfoo1j __attribute__((weak));
void * foo1j (void)
{
  return (void *)&vfoo1j;
}
extern int vfoo1j;
extern int vfoo1j __attribute__((weak));


Actually, looking at handle_alias_attribute(), TREE_USED is exactly the flag 
to test here. The addition of DECL_ASSEMBLER_NAME_SET_P seems reasonable 
though. What do you think?

Franz.

Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Jason Merrill]

>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:

> On Sunday 28 April 2002 23:57, Jason Merrill wrote:

>> I intended that TREE_SYMBOL_REFERENCED should be used rather than
>> TREE_USED, not in addition.  I'd probably check DECL_ASSEMBLER_NAME_SET_P
>> here, too; if the name hasn't been generated, it hasn't been referenced.
>> Perhaps we want a new macro, say
>> 
>> #define DECL_SYMBOL_REFERENCED(DECL) \
>> (DECL_ASSEMBLER_NAME_SET_P (DECL) \
>> && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (DECL)))

> But this would warn for these cases, which I think isn't correct:

> // case 1
> extern void * ffoo1g (void);
> extern void * ffoox1g (void);
> extern void * ffoo1g (void)  __attribute__((weak, alias ("ffoox1g")));
> void * foo1g (void)
> {
>   if (ffoo1g)
>     ffoo1g ();
>   return 0;
> }

I don't see why this would get a warning.

> // case 2
> extern int vfoo1i __attribute__((weak));
> void * foo1i (void)
> {
>   return (void *)&vfoo1i;
> }
> extern int vfoo1i __attribute__((weak));
> extern int vfoo1i;

I suppose this (and case 3) might get a warning from calling declare_weak
on the later decls.  It seems to me that we don't want to call declare_weak
on newdecl, since we're only going to throw it away.  Adding it to
weak_decls prevents gc from discarding it.

> Actually, looking at handle_alias_attribute(), TREE_USED is exactly the
> flag to test here.

I suppose that if there's a reference which hasn't been emitted yet, we
could get a false negative here.  So yes, TREE_USED is right for
declare_weak, and not TREE_SYMBOL_REFERENCED.

My patch actually used TREE_SYMBOL_REFERENCED in weak_finish, at which
point all references should have been emitted.  Of course, if you make
that change while all the duplicate decls go on weak_decls, we'd end up
weakening the symbol once for each decl.

> The addition of DECL_ASSEMBLER_NAME_SET_P seems reasonable though.

If we aren't looking at DECL_ASSEMBLER_NAME, there's no reason to look at
_SET_P.

So never mind.  If you want to tweak the patch to avoid duplication on
weak_decls, that might be good for the trunk, but your last patch should be
fine for the branch.

Thanks,
Jason

Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Mark Mitchell]

> So never mind.  If you want to tweak the patch to avoid duplication on
> weak_decls, that might be good for the trunk, but your last patch should
> be fine for the branch.

Thanks for the review.

Franz, check it in.

Thanks to both!

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: GCC 3.1 Prerelease

[Jason Merrill]

>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:

> +  if (DECL_WEAK (newdecl) && DECL_RTL (newdecl)
> +      && GET_CODE (DECL_RTL (newdecl)) == MEM
> +      && XEXP (DECL_RTL (newdecl), 0)
> +      && GET_CODE (XEXP (DECL_RTL (newdecl), 0)) == SYMBOL_REF)
> +    SYMBOL_REF_WEAK (XEXP (DECL_RTL (newdecl), 0)) = 1;

This should use DECL_RTL_SET_P.  Also, wouldn't it make sense to handle
this in declare_weak?

Jason

Re: GCC 3.1 Prerelease

[Per Bothner]

I just created pr java/6425.
I'm biased, but I think this needs to be fixed before the release.
Nic Ferrier discovered it, I verified it yesterday, and sent a
message to the java list.  No answers yet.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: GCC 3.1 Prerelease

[Mark Mitchell]

--On Tuesday, April 23, 2002 08:54:45 AM -0700 Per Bothner 
<per@bothner.com> wrote:

> I just created pr java/6425.
> I'm biased, but I think this needs to be fixed before the release.

Don't be biased; just tell me if it's a regression. :-)

If it is, we should fix it.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: GCC 3.1 Prerelease

[Per Bothner]

Mark Mitchell wrote:
> 
> 
> --On Tuesday, April 23, 2002 08:54:45 AM -0700 Per Bothner 
> <per@bothner.com> wrote:
> 
>> I just created pr java/6425.
>> I'm biased, but I think this needs to be fixed before the release.
> 
> 
> Don't be biased; just tell me if it's a regression. :-)
> 
> If it is, we should fix it.

It's a regression.  Last time I tried to build Kawa using
gcj, it worked.  Now it doesn't.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: GCC 3.1 Prerelease

[Mark Mitchell]

--On Tuesday, April 23, 2002 09:49:34 AM -0700 Per Bothner 
<per@bothner.com> wrote:

> Mark Mitchell wrote:
>>
>>
>> --On Tuesday, April 23, 2002 08:54:45 AM -0700 Per Bothner
>> <per@bothner.com> wrote:
>>
>>> I just created pr java/6425.
>>> I'm biased, but I think this needs to be fixed before the release.
>>
>>
>> Don't be biased; just tell me if it's a regression. :-)
>>
>> If it is, we should fix it.
>
> It's a regression.  Last time I tried to build Kawa using
> gcj, it worked.  Now it doesn't.

Please mark that sucker high priority then.  And see if you can fix it! :-)

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: GCC 3.1 Prerelease

[Tom Tromey]

>>>>> "Per" == Per Bothner <per@bothner.com> writes:

Per> I just created pr java/6425.  I'm biased, but I think this needs
Per> to be fixed before the release.

I looked at this.

First, the problem occurs on this line:

    sbuf.append(RealNum.toScaledInt(number, 10).toString());

Breaking this into two lines, like so, makes the code compile:

    IntNum x = RealNum.toScaledInt(number, 10);
    sbuf.append(x.toString());

That's interesting information.  It helped me track down the
difference between what happens when things go ok and when they don't.
However I don't fully understand what it means.


The class-loading code seems pretty fragile.  If it is going to
compare file names it seems like it ought to compare the results of
realpath().  Otherwise there might be ways to fool it.  However, while
this code isn't ideal (as far as I can see anyway), I think the
problem lies elsewhere.


The appended patch fixes the problem for me.  What happened was we got
to this code with an EXPR_WITH_FILE_LOCATION like this:

            arg 0 <expr_with_file_location 0x4012a420
                arg 0 <identifier_node 0x4012a480 RealNum.toScaledInt tree_2> arg 1 <identifier_node 0x40126340 FixedRealFormat.java>
                arg 2 <tree_list 0x4012917c
                    purpose <expr_with_file_location 0x4012a4a0
                        arg 0 <identifier_node 0x4012a400 RealNum> arg 1 <identifier_node 0x40126340 FixedRealFormat.java>
                        FixedRealFormat.java:9:16>
                    chain <tree_list 0x40129190
                        purpose <expr_with_file_location 0x4012a4c0 arg 0 <identifier_node 0x4012a440 toScaledInt> arg 1 <identifier_node 0x40126340 FixedRealFormat.java>
                            FixedRealFormat.java:9:23>>>
                FixedRealFormat.java:9:16>

If we strip the outer EXPR_WITH_FILE_LOCATION, as the patch does, we
end up with qual_wfl as:

    <identifier_node 0x4012a400 RealNum>


Anyway, I don't really understand what is going on here.  I assume
there is some reason for the existing `if', but I don't know what it
is.  I'm going to try rebuilding libgcj with this patch and see how
that goes.

Alex, can you look at this quickly?  As far as I know you're the only
one who really understands this code.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>

	* parse.y (qualify_ambiguous_name) [case CALL_EXPR]: Always choose
	EXPR_WFL_QUALIFICATION of qual_wfl.

Index: parse.y
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/parse.y,v
retrieving revision 1.353.2.17
diff -u -r1.353.2.17 parse.y
--- parse.y 15 Apr 2002 09:28:53 -0000 1.353.2.17
+++ parse.y 23 Apr 2002 21:01:01 -0000
@@ -11219,11 +11219,8 @@
       {
       case CALL_EXPR:
 	qual_wfl = TREE_OPERAND (qual_wfl, 0);
-	if (TREE_CODE (qual_wfl) != EXPR_WITH_FILE_LOCATION)
-	  {
-	    qual = EXPR_WFL_QUALIFICATION (qual_wfl);
-	    qual_wfl = QUAL_WFL (qual);
-	  }
+	qual = EXPR_WFL_QUALIFICATION (qual_wfl);
+	qual_wfl = QUAL_WFL (qual);
 	break;
       case NEW_ARRAY_EXPR:
       case NEW_ANONYMOUS_ARRAY_EXPR:

Re: GCC 3.1 Prerelease

[Per Bothner]

Tom Tromey wrote:
> Index: ChangeLog
> from  Tom Tromey  <tromey@redhat.com>
> 
> 	* parse.y (qualify_ambiguous_name) [case CALL_EXPR]: Always choose
> 	EXPR_WFL_QUALIFICATION of qual_wfl.
> 
> Index: parse.y
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/java/parse.y,v
> retrieving revision 1.353.2.17
> diff -u -r1.353.2.17 parse.y
> --- parse.y 15 Apr 2002 09:28:53 -0000 1.353.2.17
> +++ parse.y 23 Apr 2002 21:01:01 -0000
> @@ -11219,11 +11219,8 @@
>        {
>        case CALL_EXPR:
>  	qual_wfl = TREE_OPERAND (qual_wfl, 0);
> -	if (TREE_CODE (qual_wfl) != EXPR_WITH_FILE_LOCATION)
> -	  {
> -	    qual = EXPR_WFL_QUALIFICATION (qual_wfl);
> -	    qual_wfl = QUAL_WFL (qual);
> -	  }
> +	qual = EXPR_WFL_QUALIFICATION (qual_wfl);
> +	qual_wfl = QUAL_WFL (qual);
>  	break;
>        case NEW_ARRAY_EXPR:
>        case NEW_ANONYMOUS_ARRAY_EXPR:

This patch causes building Kawa to break even earlier (while
generating class files).
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: GCC 3.1 Prerelease

[Tom Tromey]

>>>>> "Per" == Per Bothner <per@bothner.com> writes:

Per> This patch causes building Kawa to break even earlier (while
Per> generating class files).

Yeah, I saw that once I started rebuilding libgcj with it.  Try the
appended.  We really need Alex to look at this though.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>

	* parse.y (qualify_ambiguous_name) [case CALL_EXPR]: Always choose
	EXPR_WFL_QUALIFICATION of qual_wfl.

Index: parse.y
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/parse.y,v
retrieving revision 1.353.2.17
diff -u -r1.353.2.17 parse.y
--- parse.y 15 Apr 2002 09:28:53 -0000 1.353.2.17
+++ parse.y 23 Apr 2002 23:04:15 -0000
@@ -11219,7 +11219,9 @@
       {
       case CALL_EXPR:
 	qual_wfl = TREE_OPERAND (qual_wfl, 0);
-	if (TREE_CODE (qual_wfl) != EXPR_WITH_FILE_LOCATION)
+	if (TREE_CODE (qual_wfl) != EXPR_WITH_FILE_LOCATION
+	    || (EXPR_WFL_QUALIFICATION (qual_wfl)
+		&& TREE_CODE (EXPR_WFL_QUALIFICATION (qual_wfl)) == TREE_LIST))
 	  {
 	    qual = EXPR_WFL_QUALIFICATION (qual_wfl);
 	    qual_wfl = QUAL_WFL (qual);

Re: GCC 3.1 Prerelease

[Alexandre Petit-Bianco]

Tom Tromey writes:

> Yeah, I saw that once I started rebuilding libgcj with it.  Try the
> appended.  We really need Alex to look at this though.

From the debug session information that you sent, this is making
sense. I'm not seing any regression in all my build tests (3500 files)
and I believe it improves the situation with my (old) copy of Freenet.

I'll be able to do some real debugging tonight at the earliest. In the
meantime, if we're short on time, this should probably go in.

Thank you for looking into this,

./A

Re: GCC 3.1 Prerelease

[Tom Tromey]

>>>>> "Alex" == Alexandre Petit-Bianco <apbianco@cygnus.com> writes:

Alex> From the debug session information that you sent, this is making
Alex> sense. I'm not seing any regression in all my build tests (3500
Alex> files) and I believe it improves the situation with my (old)
Alex> copy of Freenet.

Alex> I'll be able to do some real debugging tonight at the
Alex> earliest. In the meantime, if we're short on time, this should
Alex> probably go in.

Thanks.

It rebuilt libgcj fine.  I'll run the test suite (plus Mauve) shortly.
I also plan to rebuild rhug with this patch.

Mark, assuming the tests go well, is this ok for the branch?  It fixes
the last high priority gcj PR.

Alex,

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>
	For PR java/6425:
	* parse.y (qualify_ambiguous_name) [case CALL_EXPR]: Always choose
	EXPR_WFL_QUALIFICATION of qual_wfl.

Index: parse.y
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/parse.y,v
retrieving revision 1.353.2.17
diff -u -r1.353.2.17 parse.y
--- parse.y 15 Apr 2002 09:28:53 -0000 1.353.2.17
+++ parse.y 24 Apr 2002 17:24:30 -0000
@@ -11219,7 +11219,9 @@
       {
       case CALL_EXPR:
 	qual_wfl = TREE_OPERAND (qual_wfl, 0);
-	if (TREE_CODE (qual_wfl) != EXPR_WITH_FILE_LOCATION)
+	if (TREE_CODE (qual_wfl) != EXPR_WITH_FILE_LOCATION
+	    || (EXPR_WFL_QUALIFICATION (qual_wfl)
+		&& TREE_CODE (EXPR_WFL_QUALIFICATION (qual_wfl)) == TREE_LIST))
 	  {
 	    qual = EXPR_WFL_QUALIFICATION (qual_wfl);
 	    qual_wfl = QUAL_WFL (qual);

Re: GCC 3.1 Prerelease

[Mark Mitchell]

> Mark, assuming the tests go well, is this ok for the branch?  It fixes
> the last high priority gcj PR.

Yes, thanks!

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: GCC 3.1 Prerelease

[Richard Henderson]

On Tue, Apr 23, 2002 at 02:01:25AM -0700, Mark Mitchell wrote:
> There are a handful of open bugs remaining that need fixes.  By far and
> away the most worrisome are the IRIX regressions in PR 6212 and PR 6304.

I've been looking at them off and on for the last week or so, but have
made practically no headway.  I've been thwarted by the inability to
attach a debugger to an N64 binary.  (SGI's dbx crashes and GDB complains
about incorrect dwarf2, i.e. gdb doesn't understand the random changes
SGI made to dwarf2.)

I've been considering ripping out all of the MIPS_DEBUGGING_INFO bits
in dwarf2out.c and seeing if that would allow progress.  :-/


r~

Re: GCC 3.1 Prerelease

[Mark Mitchell]

--On Tuesday, April 23, 2002 12:22:54 PM -0700 Richard Henderson 
<rth@redhat.com> wrote:

> On Tue, Apr 23, 2002 at 02:01:25AM -0700, Mark Mitchell wrote:
>> There are a handful of open bugs remaining that need fixes.  By far and
>> away the most worrisome are the IRIX regressions in PR 6212 and PR 6304.
>
> I've been looking at them off and on for the last week or so, but have
> made practically no headway.  I've been thwarted by the inability to
> attach a debugger to an N64 binary.

Bummer.

> I've been considering ripping out all of the MIPS_DEBUGGING_INFO bits
> in dwarf2out.c and seeing if that would allow progress.  :-/

Or use printf?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: GCC 3.1 Prerelease

[Richard Henderson]

On Tue, Apr 23, 2002 at 01:22:18PM -0700, Mark Mitchell wrote:
> Or use printf?

In all likelyhood, it's a problem restoring registers.  In 
which case, making any function calls at all is a non-starter.


r~

Re: GCC 3.1 Prerelease

[Mark Mitchell]

--On Tuesday, April 23, 2002 01:28:40 PM -0700 Richard Henderson 
<rth@redhat.com> wrote:

> On Tue, Apr 23, 2002 at 01:22:18PM -0700, Mark Mitchell wrote:
>> Or use printf?
>
> In all likelyhood, it's a problem restoring registers.  In
> which case, making any function calls at all is a non-starter.

I meant in the unwinder to try to see what it thought it was doing.

Are we already hosed at that point?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: GCC 3.1 Prerelease

[Richard Henderson]

On Tue, Apr 23, 2002 at 01:35:05PM -0700, Mark Mitchell wrote:
> I meant in the unwinder to try to see what it thought it was doing.

Oh, I see.  Yes, one could verify that we did in fact find
the frame we intended that way.

> Are we already hosed at that point?

I doubt it.


r~

mips n64 eh failures

[Richard Henderson]

On Tue, Apr 23, 2002 at 01:35:05PM -0700, Mark Mitchell wrote:
> I meant in the unwinder to try to see what it thought it was doing.

Hum.  Your advice to try printf was sound.  It wasn't getting
as far as I thought.  In fact, the entire problem appears to 
be in the fde sorting routines.

If I disable the "erratic" array, so that we do a simple heap
sort on the *entire* set of fdes (as opposed to attempting to
locate sequences of already sorted fdes), then the EH tests
start passing again.

More poking...


r~

Re: mips n64 eh failures

[Mark Mitchell]

--On Tuesday, April 23, 2002 04:15:43 PM -0700 Richard Henderson 
<rth@redhat.com> wrote:

> On Tue, Apr 23, 2002 at 01:35:05PM -0700, Mark Mitchell wrote:
>> I meant in the unwinder to try to see what it thought it was doing.
>
> Hum.  Your advice to try printf was sound.  It wasn't getting
> as far as I thought.  In fact, the entire problem appears to
> be in the fde sorting routines.

Ooooohhh.  That gave me some horrible flashback from GCC 2.95/GCC 3.0
where I remember trying to debug something that sounds very much like
this on IRIX.  The weird thing was that it seemed semi-transient; I
think it went away before I ever figured out what it was.  I remember
thinking that there was a miscompilation of one of the fde-sorting
routines, but I don't remember much more than that.  I remember
thinking it could be some kind of alignment issue; that depending on
where things happen to get put in memory something ends up 64-bit
aligned (good!) or not (bad!).

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: mips n64 eh failures

[Richard Henderson]

On Tue, Apr 23, 2002 at 04:27:07PM -0700, Mark Mitchell wrote:
> I remember thinking that there was a miscompilation of one of the
> fde-sorting routines, but I don't remember much more than that.

Good catch.  Adding

  for (i = 0; i < linear->count; ++i)
    printf("\t%p %d\n", linear->array[i], i+1 < linear->count ?
	   fde_compare (ob, linear->array[i], linear->array[i+1]) : 0);

i.e. compare each entry to the one following.  We should thus
get a sorted sequence with all comparisons == -1.  But instead,

        10025df0 -1
        10025e50 1
        10025e80 1
        10025eb0 -1
        10025ee0 -1
        10025f10 -1
        10025f40 -1
        10025f70 -1
        10025fa0 -1
        10025fd0 1
        10026000 1
        10026030 -1

The numbers are in fact sorted, but the comparisons are wrong.


r~

Re: mips n64 eh failures

[Richard Henderson]

On Tue, Apr 23, 2002 at 04:53:25PM -0700, Richard Henderson wrote:
> The numbers are in fact sorted, but the comparisons are wrong.

I lied.  I printed out the address of the fde, not
the pc it gets sorted on.


r~

Re: mips n64 eh failures

[Richard Henderson]

I've found the miscompilation.

          erratic->array[probe - linear->array] = NULL;

is being rendered as 

(insn 122 119 124 (set (reg:DI 108)
        (minus:DI (reg/v/f:DI 91)
            (reg/v/f:DI 83))) -1 (nil)
    (nil))

(insn 124 122 126 (set (reg/f:DI 109)
        (plus:DI (reg/v/f:DI 84)
            (reg:DI 108))) -1 (nil)
    (nil))

(insn 126 124 128 (set (reg:DI 110)
        (const_int 0 [0x0])) -1 (nil)
    (expr_list:REG_EQUAL (const_int 0 [0x0])
        (nil)))

(insn 128 126 130 (set (mem/s:BLK (reg/f:DI 109) [0 S1 A64])
        (unspec:BLK[ 
                (reg:DI 110)
            ]  1)) -1 (nil)
    (nil))

rather than a normal DImode store, as you'd expect for a pointer.
I have no idea why as yet, though the "Size 1" instead of "Size 8"
is extremely suspicious.


r~

Re: mips n64 eh failures

[Richard Henderson]

The bug is here:

revision 1.361
date: 2001/10/18 21:34:08;  author: kenner;  state: Exp;  lines: +85 -41
        * emit-rtl.c (gen_reg_rtx): Also reallocate reg_decl array.
        (offset_address): New function.
        (free_emit_status): Free regno_decl.
        (init_emit): Pass proper number of elements to xcalloc.
        Allocate regno_decl.
        (mark_emit_status): Mark regno_decl values.
        * expr.c (highest_pow2_factor): New function.
        (expand_assigment): Use it and offset_address.
        Properly handle ptr_mode vs. Pmode in offset calculation.
        (store_constructor, expand_expr_unaligned): Likewise.
        (expand_expr, case COMPONENT_EXPR): Likewise.

expr.c:3709:
        to_rtx = offset_address (to_rtx, offset_rtx,
                                 highest_pow2_factor (offset));

offset = 
 <plus_expr 0x401c2860
    type <integer_type 0x401bf708 long unsigned int ...>
    arg 0 <nop_expr 0x401dac6c type <integer_type 0x401bf708 long unsigned int>
       
        arg 0 <minus_expr 0x401c25c0 type <integer_type 0x4001d4b0 long int>
            arg 0 <parm_decl 0x401db4b0 probe>
	    arg 1 <parm_decl 0x401db5a0 linear>>>
    arg 1 <integer_cst 0x4001bbe0 ... constant 0>>

in that the two parm_decls have highest_pow2_factor == 1, which
forces the entire block of memory to have alignment 8.  Which
causes the mips backend to attempt an unaligned store, which 
fails for some reason.

So there is in fact a secondary bug in the mips backend.  Namely,
the movsi_usw and movdi_usd patterns are identical if the argument
is the constant zero.  Patch for that pending.

But the *primary* bug is that an unaligned store isn't necessary.

The following test case shows the bug on both mips and alpha.


r~



typedef long unsigned int size_t;
typedef int sword __attribute__ ((mode (SI)));
typedef unsigned int uword __attribute__ ((mode (SI)));
typedef unsigned int uaddr __attribute__ ((mode (pointer)));
typedef int saddr __attribute__ ((mode (pointer)));

struct fde_vector
{
  void *orig_data;
  size_t count;
  struct dwarf_fde *array[];
};

struct dwarf_fde
{
  uword length;
  sword CIE_delta;
  unsigned char pc_begin[];
} __attribute__ ((packed, aligned (__alignof__ (void *))));

typedef struct dwarf_fde fde;
struct object;

void
fde_split (fde **probe, struct fde_vector *linear, struct fde_vector *erratic)
{
  erratic->array[probe - linear->array] = 0;
}

Re: mips n64 eh failures

[Mark Mitchell]

--On Tuesday, April 23, 2002 07:24:24 PM -0700 Richard Henderson 
<rth@redhat.com> wrote:

> The bug is here:

It's huge of you to have tracked this down.  Incredibly huge.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com