[Bug lto/61886] [4.8/4.9/4.10 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[Bug lto/61886] [4.8/4.9/4.10 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Btw, it seems that at the point we merge in lto-symtab.c the cgraph nodes are
not yet populated.  Neither of the two candidates are marked as alias.

So maybe the wrong thing already happens earlier, during compile-time
and we shouldn't do any symtab merging?  at least from symbols originating
from the same TU?  That is, tree merging already should catch most
true equivalencies and cgraph merging shouldn't be symtab-driven?

That said, I wonder how to fix things up properly.

The following fixes the testcase, not merging the actual symtab intra-TU
and retaining decls that have a symtab entry recorded.

Index: gcc/lto/lto-symtab.c
===================================================================
--- gcc/lto/lto-symtab.c        (revision 212927)
+++ gcc/lto/lto-symtab.c        (working copy)
@@ -575,6 +575,9 @@ lto_symtab_merge_symbols_1 (symtab_node

       if (!lto_symtab_symbol_p (e))
        continue;
+      /* Do not merge symtab nodes originating from the same TU.  */
+      if (e->lto_file_data == prevailing->lto_file_data)
+       continue;
       cgraph_node *ce = dyn_cast <cgraph_node *> (e);
       if (ce && !DECL_BUILT_IN (e->decl))
        lto_cgraph_replace_node (ce, cgraph (prevailing));
@@ -680,6 +683,12 @@ lto_symtab_prevailing_decl (tree decl)
   if (TREE_CODE (decl) == FUNCTION_DECL && DECL_BUILT_IN (decl))
     return decl;

+  /* If the decl retained its symtab node then it prevails.  This
+     preserves semantically different decls for the same underlying
+     symbol, like aliases that have been resolved at compile-time.  */
+  if (symtab_get_node (decl))
+    return decl;
+
   /* Ensure DECL_ASSEMBLER_NAME will not set assembler name.  */
   gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));

[Bug lto/61886] [4.8/4.9/4.10 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.7.4
      Known to fail|                            |4.10.0, 4.8.3, 4.9.1

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I'm testing the patch, testcase:

/* { dg-lto-do link } */
/* { dg-lto-options { { -flto -O2 -Werror } } } */

typedef __SIZE_TYPE__ size_t;
typedef struct _IO_FILE FILE;

extern size_t __fread_chk_warn (void *__restrict __ptr, size_t __ptrlen, size_t
__size, size_t __n, FILE *__restrict __stream) __asm__ ("" "__fread_chk")     
__attribute__ ((__warn_unused_result__)) __attribute__((__warning__ ("fread
called with bigger size * nmemb than length " "of destination buffer")));

extern __inline __attribute__ ((__always_inline__)) __attribute__
((__gnu_inline__)) __attribute__ ((__artificial__)) __attribute__
((__warn_unused_result__))
size_t
fread (void *__restrict __ptr, size_t __size, size_t __n,
       FILE *__restrict __stream)
{
  if (__builtin_object_size (__ptr, 0) != (size_t) -1)
    {
      if (!__builtin_constant_p (__size)
          || !__builtin_constant_p (__n)
          || (__size | __n) >= (((size_t) 1) << (8 * sizeof (size_t) / 2)))
        return __fread_chk (__ptr, __builtin_object_size (__ptr, 0), __size,
__n, __stream);
      if (__size * __n > __builtin_object_size (__ptr, 0))
        return __fread_chk_warn (__ptr, __builtin_object_size (__ptr, 0),
__size, __n, __stream);
    }
}

volatile size_t nmemb;
FILE *fp;
int main ()
{
  char file_contents[4096];
  /* We shouldn't get this resolved to a call to __fread_chk_warn.  */
  return fread (file_contents, 1, nmemb, fp);
}

[Bug lto/61886] [4.8/4.9/4.10 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fully preprocessed glibc fread stuff (reduced testcase drops main path
and a prototype for __fread_chk):

extern size_t __fread_chk (void *__restrict __ptr, size_t __ptrlen,
      size_t __size, size_t __n,
      FILE *__restrict __stream) __attribute__ ((__warn_unused_result__));
extern size_t __fread_alias (void *__restrict __ptr, size_t __size, size_t __n,
FILE *__restrict __stream) __asm__ ("" "fread")


            __attribute__ ((__warn_unused_result__));
extern size_t __fread_chk_warn (void *__restrict __ptr, size_t __ptrlen, size_t
__size, size_t __n, FILE *__restrict __stream) __asm__ ("" "__fread_chk")




     __attribute__ ((__warn_unused_result__)) __attribute__((__warning__
("fread called with bigger size * nmemb than length " "of destination
buffer")))
                                 ;

extern __inline __attribute__ ((__always_inline__)) __attribute__
((__gnu_inline__)) __attribute__ ((__artificial__)) __attribute__
((__warn_unused_result__)) size_t
fread (void *__restrict __ptr, size_t __size, size_t __n,
       FILE *__restrict __stream)
{
  if (__builtin_object_size (__ptr, 0) != (size_t) -1)
    {
      if (!__builtin_constant_p (__size)
   || !__builtin_constant_p (__n)
   || (__size | __n) >= (((size_t) 1) << (8 * sizeof (size_t) / 2)))
 return __fread_chk (__ptr, __builtin_object_size (__ptr, 0), __size, __n,
__stream);

      if (__size * __n > __builtin_object_size (__ptr, 0))
 return __fread_chk_warn (__ptr, __builtin_object_size (__ptr, 0), __size, __n,
__stream);
    }
  return __fread_alias (__ptr, __size, __n, __stream);
}

[Bug lto/61886] [4.8/4.9/4.10 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2014-07-23
           Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #5 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
This is a difficult problem.  In early cgraph days GCC code was converted to
assume that there are never duplicated declarations of a given symbol. This is
precisely what glibc does to keep these duplicates flow through the
compilation.
It is a long standing bug that we have such duplicate declarations and that we
do not reject these as an error or fix internaly.

A clear fix would be to make alias from glibc SO for fread_chk and
fread_chk_warn that can be used for those two different calls.  But without
changling glibc's API we need to work out how to introduce the alias
internally.  We support similar bookeeping for wearkrefs that are "syntactic"
aliases resulting in the same assembler name as their target.  I suppose we can
do the same here, but it is ugly - I would much preffer those hacks to not
exist at all.

I will try to prepare patch and see how contained it is.

Honza

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|4.10.0                      |5.0

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Honza, did you get anything working?

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #7 from Jan Hubicka <hubicka at ucw dot cz> ---
> Honza, did you get anything working?
Sorry, I am at Mountain trip till 6th of September, so I do not have much
chance to hack.  Will take it as priority afterwards.  I think cleanest
approach would be to lower these calls into sequence of warning/error builtin
and call.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #8 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 27 Aug 2014, hubicka at ucw dot cz wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886
> 
> --- Comment #7 from Jan Hubicka <hubicka at ucw dot cz> ---
> > Honza, did you get anything working?
> Sorry, I am at Mountain trip till 6th of September, so I do not have much
> chance to hack.  Will take it as priority afterwards.  I think cleanest
> approach would be to lower these calls into sequence of warning/error builtin
> and call.

The issue is that we resolve aliases in a bogus way, not warning/error
stuff.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #9 from Jan Hubicka <hubicka at ucw dot cz> ---
> The issue is that we resolve aliases in a bogus way, not warning/error
> stuff.

Those are not aliases, but duplicated declarations that are supposed to not
exist since one declaration rule was introduced in early 4.x series.
It is bug that frotnend allowed creating them and made it part of the fortify
interface. I still think it is better to resolve those early and start
rejecting
any other uses than the fortify case...

Honza

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 8 Sep 2014, hubicka at ucw dot cz wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886
> 
> --- Comment #9 from Jan Hubicka <hubicka at ucw dot cz> ---
> > The issue is that we resolve aliases in a bogus way, not warning/error
> > stuff.
> 
> Those are not aliases, but duplicated declarations that are supposed to not
> exist since one declaration rule was introduced in early 4.x series.
> It is bug that frotnend allowed creating them and made it part of the fortify
> interface. I still think it is better to resolve those early and start
> rejecting
> any other uses than the fortify case...

Well - whatever makes the fortify case work as expected works for me.
Note that it would be nice to fix this on the 4.9 branch as well.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #11 from Jan Hubicka <hubicka at ucw dot cz> ---
Hi,
this patch implements the lowring.  Each call with warn attribute triggers code
in cgraphunit that inserts call to bulitin_warning/error that is output at
expansion time.

Do we have way to define bulitin that is not user accessible?

Also we do not have way to define LOOPING_CONST bulitin, so I am simply forcing
the flag in cgraphunit.c that is somewhat ugly.

One of consequences of this approach is that indirect calls to functions with
warn attributes will not produce warning.  I think it is sort of acceptable
because we also do not warn when the indirect call is produced late by RTL
backend.

Honza

Index: expr.c
===================================================================
--- expr.c    (revision 215901)
+++ expr.c    (working copy)
@@ -10346,21 +10346,7 @@ expand_expr_real_1 (tree exp, rtx target
       if (CALL_EXPR_VA_ARG_PACK (exp))
     error ("%Kinvalid use of %<__builtin_va_arg_pack ()%>", exp);
       {
-    tree fndecl = get_callee_fndecl (exp), attr;
-
-    if (fndecl
-        && (attr = lookup_attribute ("error",
-                     DECL_ATTRIBUTES (fndecl))) != NULL)
-      error ("%Kcall to %qs declared with attribute error: %s",
-         exp, identifier_to_locale (lang_hooks.decl_printable_name (fndecl,
1)),
-         TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
-    if (fndecl
-        && (attr = lookup_attribute ("warning",
-                     DECL_ATTRIBUTES (fndecl))) != NULL)
-      warning_at (tree_nonartificial_location (exp),
-              0, "%Kcall to %qs declared with attribute warning: %s",
-              exp, identifier_to_locale (lang_hooks.decl_printable_name
(fndecl, 1)),
-              TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
+    tree fndecl = get_callee_fndecl (exp);

     /* Check for a built-in function.  */
     if (fndecl && DECL_BUILT_IN (fndecl))
Index: builtin-types.def
===================================================================
--- builtin-types.def    (revision 215901)
+++ builtin-types.def    (working copy)
@@ -581,3 +581,5 @@ DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_LDO
              BT_VOLATILE_PTR, BT_LONGDOUBLE)
 DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_SIZE, BT_VOID,
              BT_VOLATILE_PTR, BT_SIZE)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_CONST_STRING_CONST_STRING,
+             BT_VOID, BT_CONST_STRING, BT_CONST_STRING)
Index: builtins.c
===================================================================
--- builtins.c    (revision 215901)
+++ builtins.c    (working copy)
@@ -59,6 +59,8 @@ along with GCC; see the file COPYING3.
 #include "builtins.h"
 #include "ubsan.h"
 #include "cilk.h"
+#include "pretty-print.h"
+#include "print-tree.h"


 static tree do_mpc_arg1 (tree, tree, int (*)(mpc_ptr, mpc_srcptr, mpc_rnd_t));
@@ -6816,6 +6818,31 @@ expand_builtin (tree exp, rtx target, rt
       expand_builtin_cilk_pop_frame (exp);
       return const0_rtx;

+    case BUILT_IN_WARNING:
+      const char * arg0;
+      const char * arg1;
+      if (!validate_arglist (exp, POINTER_TYPE, POINTER_TYPE, VOID_TYPE)
+      || (arg0 = c_getstr (CALL_EXPR_ARG (exp, 0))) == NULL
+      || (arg1 = c_getstr (CALL_EXPR_ARG (exp, 1))) == NULL)
+    warning_at (tree_nonartificial_location (exp), 0,
+            "%K__builtin_warning used without both arguments being string
constants",
+            exp);
+      else
+    warning_at (tree_nonartificial_location (exp),
+            0, "%Kcall to %qs declared with attribute warning: %s",
+            exp, arg0, identifier_to_locale (arg1));
+      return const0_rtx;
+    case BUILT_IN_ERROR:
+      if (!validate_arglist (exp, POINTER_TYPE, POINTER_TYPE, VOID_TYPE)
+      || (arg0 = c_getstr (CALL_EXPR_ARG (exp, 0))) == NULL
+      || (arg1 = c_getstr (CALL_EXPR_ARG (exp, 1))) == NULL)
+    error ("%K__builtin_error used without both arguments being string
constants",
+           exp);
+      else
+    error ("%Kcall to %qs declared with attribute warning: %s",
+           exp, arg0, identifier_to_locale (arg1));
+      return const0_rtx;
+
     default:    /* just do library call, if unknown builtin */
       break;
     }
Index: cgraphunit.c
===================================================================
--- cgraphunit.c    (revision 215901)
+++ cgraphunit.c    (working copy)
@@ -211,6 +211,7 @@ along with GCC; see the file COPYING3.
 #include "tree-nested.h"
 #include "gimplify.h"
 #include "dbgcnt.h"
+#include "expr.h"

 /* Queue of cgraph nodes scheduled to be added into cgraph.  This is a
    secondary queue used during optimization to accommodate passes that
@@ -976,8 +977,30 @@ analyze_functions (void)
         cnode->analyze ();

           for (edge = cnode->callees; edge; edge = edge->next_callee)
-        if (edge->callee->definition)
-           enqueue_node (edge->callee);
+        {
+          tree attr, err_attr = NULL;
+          if (edge->callee->definition)
+             enqueue_node (edge->callee);
+          if ((attr = lookup_attribute ("warning",
+                            DECL_ATTRIBUTES (edge->callee->decl))) != NULL
+              || (err_attr = lookup_attribute ("warning",
+                            DECL_ATTRIBUTES (edge->callee->decl))))
+            {
+              gimple_stmt_iterator gsi = gsi_for_stmt (edge->call_stmt);
+              const char *arg0 = lang_hooks.decl_printable_name
(edge->callee->decl, 1);
+              const char *arg1= TREE_STRING_POINTER
+                     (TREE_VALUE (TREE_VALUE (attr ? attr : err_attr)));
+              tree dest = builtin_decl_implicit (attr ? BUILT_IN_WARNING :
BUILT_IN_ERROR);
+
+              cgraph_node::get_create (dest)->set_const_flag (true, true);
+              gimple call = gimple_build_call (dest, 2,
+                               build_string_literal (strlen (arg0), arg0),
+                               build_string_literal (strlen (arg1), arg1));
+              gsi_insert_before (&gsi, call, GSI_SAME_STMT);
+              cnode->create_edge (cgraph_node::get_create (dest), call,
+                      edge->count, edge->frequency);
+            }
+        }
           if (optimize && flag_devirtualize)
         {
           cgraph_edge *next;
Index: builtins.def
===================================================================
--- builtins.def    (revision 215901)
+++ builtins.def    (working copy)
@@ -820,6 +820,9 @@ DEF_EXT_LIB_BUILTIN    (BUILT_IN_PRINTF_
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_VFPRINTF_CHK, "__vfprintf_chk",
BT_FN_INT_FILEPTR_INT_CONST_STRING_VALIST_ARG, ATTR_FORMAT_PRINTF_3_0)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_VPRINTF_CHK, "__vprintf_chk",
BT_FN_INT_INT_CONST_STRING_VALIST_ARG, ATTR_FORMAT_PRINTF_2_0)

+DEF_GCC_BUILTIN        (BUILT_IN_ERROR, "error",
BT_FN_VOID_CONST_STRING_CONST_STRING, ATTR_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN        (BUILT_IN_WARNING, "warning",
BT_FN_VOID_CONST_STRING_CONST_STRING, ATTR_NOTHROW_LEAF_LIST)
+
 /* Profiling hooks.  */
 DEF_BUILTIN (BUILT_IN_PROFILE_FUNC_ENTER, "__cyg_profile_func_enter",
BUILT_IN_NORMAL, BT_FN_VOID_PTR_PTR, BT_LAST,
          false, false, false, ATTR_NULL, true, true)

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #12 from Jakub Jelinek <jakub at redhat dot com> ---
On Mon, Oct 06, 2014 at 10:22:21PM +0200, Jan Hubicka wrote:
> this patch implements the lowring.  Each call with warn attribute triggers code
> in cgraphunit that inserts call to bulitin_warning/error that is output at
> expansion time.
> 
> Do we have way to define bulitin that is not user accessible?

internal-fn* builtins are not user accessible.

    Jakub

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #14 from Jakub Jelinek <jakub at redhat dot com> ---
On Mon, Oct 06, 2014 at 11:55:23PM +0200, Jan Hubicka wrote:
> Hi,
> I am testing this variant of the patch.
> For gcc-4.9 branch it may make sense to enable the new patch for LTO only.

Not printing the inlining backtrace would be IMHO a significant regression.

    Jakub

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #15 from Jan Hubicka <hubicka at ucw dot cz> ---
> On Mon, Oct 06, 2014 at 11:55:23PM +0200, Jan Hubicka wrote:
> > Hi,
> > I am testing this variant of the patch.
> > For gcc-4.9 branch it may make sense to enable the new patch for LTO only.
> 
> Not printing the inlining backtrace would be IMHO a significant regression.

OK, how do I print it?  I keep the BLOCK of the original expresison, so it is
there.

Honza
> 
> 	Jakub
>From gcc-bugs-return-463410-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Oct 06 22:27:38 2014
Return-Path: <gcc-bugs-return-463410-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 5182 invoked by alias); 6 Oct 2014 22:27:38 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 5157 invoked by uid 48); 6 Oct 2014 22:27:34 -0000
From: "davidgkinniburgh at yahoo dot co.uk" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug fortran/63469] New: Automatic reallocation of allocatable scalar length even when substring implicitly specified
Date: Mon, 06 Oct 2014 22:27:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: new
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: fortran
X-Bugzilla-Version: 4.9.1
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: davidgkinniburgh at yahoo dot co.uk
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_id short_desc product version bug_status bug_severity priority component assigned_to reporter
Message-ID: <bug-63469-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-10/txt/msg00431.txt.bz2
Content-length: 1323

https://gcc.gnu.org/bugzilla/show_bug.cgi?idc469

            Bug ID: 63469
           Summary: Automatic reallocation of allocatable scalar length
                    even when substring implicitly specified
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: davidgkinniburgh at yahoo dot co.uk

CHARACTER(:), ALLOCATABLE :: s

ALLOCATE (character(32) :: s)

s(1:32) = 'string'
print *, 'Length of ', s, ' with substring = ', LEN(s)

s(:) = 'string'
print *, 'Length of ', s, ' with substring = ', LEN(s)

s = 'string'
print *, 'Length of ', s, ' without substring = ', LEN(s)


gfortran (4.9.1 and earlier) gives:

 Length of string                           with substring =           32
 Length of string with substring =            6
 Length of string without substring =            6


IVF (15.0) gives:

 Length of string                           with substring =           32
 Length of string                           with substring =           32
 Length of string without substring =            6

which I think is correct.

It is the implicit definition of both the beginning and the ending of 's' that
seems to do the damage.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #16 from Jakub Jelinek <jakub at redhat dot com> ---
On Tue, Oct 07, 2014 at 12:18:24AM +0200, Jan Hubicka wrote:
> > On Mon, Oct 06, 2014 at 11:55:23PM +0200, Jan Hubicka wrote:
> > > Hi,
> > > I am testing this variant of the patch.
> > > For gcc-4.9 branch it may make sense to enable the new patch for LTO only.
> > 
> > Not printing the inlining backtrace would be IMHO a significant regression.
> 
> OK, how do I print it?  I keep the BLOCK of the original expresison, so it is there.

%K in the format string, assuming the call has locus with the right block,
should do that.  At least with -g, without -g or with LTO it will be less
accurate.

    Jakub

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #17 from Jan Hubicka <hubicka at ucw dot cz> ---
> 
> %K in the format string, assuming the call has locus with the right block,
> should do that.  At least with -g, without -g or with LTO it will be less
> accurate.

Yep, for that I need a tree to pass in. Do I need to build something or is
there
way to pass in gimple statmeent?

Honza

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #18 from Jan Hubicka <hubicka at ucw dot cz> ---
Hi,
actually I can just add the location to the first argument to avoid the need
to build extra tree...

Somewhat ugly, but seems to work.

Index: internal-fn.c
===================================================================
--- internal-fn.c    (revision 215958)
+++ internal-fn.c    (working copy)
@@ -37,6 +37,8 @@ along with GCC; see the file COPYING3.
 #include "stringpool.h"
 #include "tree-ssanames.h"
 #include "diagnostic-core.h"
+#include "builtins.h"
+#include "pretty-print.h"

 /* The names of each internal function, indexed by function number.  */
 const char *const internal_fn_name_array[] = {
@@ -915,6 +917,28 @@ expand_BUILTIN_EXPECT (gimple stmt)
     emit_move_insn (target, val);
 }

+static void
+expand_OUTPUT_ERROR (gimple stmt)
+{
+  const char * arg0 = c_getstr (gimple_call_arg (stmt, 0));
+  const char * arg1 = c_getstr (gimple_call_arg (stmt, 1));
+  error_at (gimple_location (stmt),
+        "%KCall to %qs declared with attribute error: %s",
+        gimple_call_arg (stmt, 0),
+        arg0, identifier_to_locale (arg1));
+}
+
+static void
+expand_OUTPUT_WARNING (gimple stmt)
+{
+  const char * arg0 = c_getstr (gimple_call_arg (stmt, 0));
+  const char * arg1 = c_getstr (gimple_call_arg (stmt, 1));
+  warning_at (gimple_location (stmt),
+          0, "%Kcall to %qs declared with attribute warning: %s",
+          gimple_call_arg (stmt, 0),
+          arg0, identifier_to_locale (arg1));
+}
+
 /* Routines to expand each internal function, indexed by function number.
    Each routine has the prototype:

Index: cgraphunit.c
===================================================================
--- cgraphunit.c    (revision 215958)
+++ cgraphunit.c    (working copy)
@@ -211,6 +211,8 @@ along with GCC; see the file COPYING3.
 #include "tree-nested.h"
 #include "gimplify.h"
 #include "dbgcnt.h"
+#include "expr.h"
+#include "internal-fn.h"

 /* Queue of cgraph nodes scheduled to be added into cgraph.  This is a
    secondary queue used during optimization to accommodate passes that
@@ -976,8 +978,33 @@ analyze_functions (void)
         cnode->analyze ();

           for (edge = cnode->callees; edge; edge = edge->next_callee)
-        if (edge->callee->definition)
-           enqueue_node (edge->callee);
+        {
+          tree attr, err_attr = NULL;
+          if (edge->callee->definition)
+             enqueue_node (edge->callee);
+          if ((attr = lookup_attribute ("warning",
+                            DECL_ATTRIBUTES (edge->callee->decl))) != NULL
+              || (err_attr = lookup_attribute ("warning",
+                            DECL_ATTRIBUTES (edge->callee->decl))))
+            {
+              gimple_stmt_iterator gsi = gsi_for_stmt (edge->call_stmt);
+              const char *arg0 = lang_hooks.decl_printable_name
(edge->callee->decl, 1);
+              const char *arg1= TREE_STRING_POINTER
+                     (TREE_VALUE (TREE_VALUE (attr ? attr : err_attr)));
+              tree arg0_expr = build_string_literal (strlen (arg0), arg0);
+
+              gimple call = gimple_build_call_internal
+                      (attr ? IFN_OUTPUT_WARNING : IFN_OUTPUT_ERROR, 2,
+                       arg0_expr,
+                       build_string_literal (strlen (arg1), arg1));
+              gsi_insert_before (&gsi, call, GSI_SAME_STMT);
+              gimple_set_location (call, gimple_location (edge->call_stmt));
+              gimple_set_block (call, gimple_block (edge->call_stmt));
+              /* Disgnostic code needs tree to pick inline stack from. */
+              SET_EXPR_LOCATION (arg0_expr, gimple_location
(edge->call_stmt));
+              TREE_SET_BLOCK (arg0_expr, gimple_block (edge->call_stmt));
+            }
+        }
           if (optimize && flag_devirtualize)
         {
           cgraph_edge *next;
Index: builtin-types.def
===================================================================
--- builtin-types.def    (revision 215958)
+++ builtin-types.def    (working copy)
@@ -581,3 +581,5 @@ DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_LDO
              BT_VOLATILE_PTR, BT_LONGDOUBLE)
 DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_SIZE, BT_VOID,
              BT_VOLATILE_PTR, BT_SIZE)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_CONST_STRING_CONST_STRING,
+             BT_VOID, BT_CONST_STRING, BT_CONST_STRING)
Index: expr.c
===================================================================
--- expr.c    (revision 215958)
+++ expr.c    (working copy)
@@ -10346,21 +10346,7 @@ expand_expr_real_1 (tree exp, rtx target
       if (CALL_EXPR_VA_ARG_PACK (exp))
     error ("%Kinvalid use of %<__builtin_va_arg_pack ()%>", exp);
       {
-    tree fndecl = get_callee_fndecl (exp), attr;
-
-    if (fndecl
-        && (attr = lookup_attribute ("error",
-                     DECL_ATTRIBUTES (fndecl))) != NULL)
-      error ("%Kcall to %qs declared with attribute error: %s",
-         exp, identifier_to_locale (lang_hooks.decl_printable_name (fndecl,
1)),
-         TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
-    if (fndecl
-        && (attr = lookup_attribute ("warning",
-                     DECL_ATTRIBUTES (fndecl))) != NULL)
-      warning_at (tree_nonartificial_location (exp),
-              0, "%Kcall to %qs declared with attribute warning: %s",
-              exp, identifier_to_locale (lang_hooks.decl_printable_name
(fndecl, 1)),
-              TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
+    tree fndecl = get_callee_fndecl (exp);

     /* Check for a built-in function.  */
     if (fndecl && DECL_BUILT_IN (fndecl))

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The builtin-types.def part is unnecessary, I don't see internal-fn.def part.
Also, we might need to tune optimizations across the two internal calls (from
aliasing POV at least), we certainly want them to act as they have unspecified
other effects (i.e. we don't want DCE to delete them, or hoist them before
condition guarding them, on the other side, it would be nice if alias would
know they don't clobber nor read any memory, are leaf etc.).

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #21 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The only duplicate decls are the C extern inline __attribute__((gnu_inline))
(or -std=c89/gnu89 extern inline) or C++ inline __attribute__((gnu_inline)).
We do have a middle-end representation of those, don't we?
Do you have a problem that they have the same asm names, or DECL_NAME,
something else?
If you wanted different asm names (e.g. normal asm name plus space at the end),
we'd need some code to change calls to the functions with space after it back
into ones without it if inlining failed.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #22 from Jan Hubicka <hubicka at ucw dot cz> ---
Concerning Richard's comment, it is true that we will produce more warings then
before in case function is optimized out.  But we always did produce extra
warnings
when the function call is optimized out (as dead or pure/const unused) during
RTL optimization. Of course this was more common before we got tree-ssa, but
the
feature AFAIK predates that.

This is ugly area, because it exposes too much of internals.

> The only duplicate decls are the C extern inline __attribute__((gnu_inline))
> (or -std=c89/gnu89 extern inline) or C++ inline __attribute__((gnu_inline)).
> We do have a middle-end representation of those, don't we?

We don't.  We really replace inline version by offline and mark it noinline.
With Winline you get warning "redefined extern inline functions are not
considered for inlining"
Here I think a way around would be to make C/C++ FEs to produce a static inline
function
and record in its cgraph node that it should be used for inlining of some other
symbol.
Unreachable code removal would need to be extended to deal with this (i.e. not
remove it until
references to the real symbol disappears) and inliner can handle it easily
redirecting
to the inline version prior inlining.

I never got past implementing the frontend part though.

> Do you have a problem that they have the same asm names, or DECL_NAME,
> something else?

Asm name, since Zack's one declaration changes, we are supposed to have only
one
decl for an ASM name.  It is not always true - the warning attribute is one
offender
and in some cases FEs still break the rule.

> If you wanted different asm names (e.g. normal asm name plus space at the end),
> we'd need some code to change calls to the functions with space after it back
> into ones without it if inlining failed.

This still does break one declaration rule because we would end up with two
declarations and two symbols for one real symbol.  This cause problems, because
we need to consider this i.e. in testing symbols for equality etc.

We do have a precedent here - the weakrefs are the same evil.  I could
generalize
weakref code to also handle warnings(), pattern match this sepcific use (where
we have two symbols for one asm name that differs by warnings), keep the
non-warning
global, turn the warning decl static "weakref" translating into the global
decl.
Making it static is needed to support different warning messsages across
multiple
LTO units - we must not merge the warning annotated symbols.

Now of course this will need a lot of extra special casing of "weakref" because
currently we belive the visibility properties of the duplicated decl that does
not match the visibility properties of the real node.
For weakref I just redirect all refernces to the prevailing declaration if it
exists
that solves the problem.  This would disable the warnings.

So while I can implement this, it is not bacportable to 4.8/4.9 and it will
likely
trigger interesting side cases for a while, like weakref did.

We can also put warning attribute into gimple call.

Honza

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #23 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 7 Oct 2014, hubicka at ucw dot cz wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886
> 
> --- Comment #22 from Jan Hubicka <hubicka at ucw dot cz> ---
> We can also put warning attribute into gimple call.

I like that better.  We can easily add a GF_CALL_WARN / GF_CALL_ERROR
flag, but the question is where to put the warning string...  the
issue with this bugreport is that it might not be on the decl
(because the decl is now replaced with one with/without the attribute).
Maybe if we always "merge" the warning attributes then the call flag
tells us whether to ignore it or not ...

Thus for this particular bug (bogus warning) just adding the flag
as a requirement to emit the warning/error would be enough.  We
may still lose warnings/errors if the wrong decl is picked for
a GF_CALL_WARN call though.

Yeah, now the idea of adding a generic annotation pointer to
gimple stmts will pop up again ... of course I don't like that
very much.

OTOH we had the idea of avoiding warnings from the middle-end
for dead code by queuing warnings to emit on stmts and only
emit them if the stmt is still live at a certain point during
the compilation.

All that said - what about going with the simple GF_CALL_WARN_OR_ERROR
flag to avoid the false "positives"?

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
But is warning/error attribute the only thing on aliases that can hold extra
semantics info (now or in the future)?  I'd say LTO symtab merging should merge
what is mergeable, and should leave leave as separate decls with the same
asm-name what holds non-mergeable semantics on it.
Say, if you declare some function (or different, just with same asm name) with
warning attribute in one TU, with error attribute in another TU and without it
on another TU, IMHO those three decls shouldn't be merged together, you should
note in cgraph that you have aliases that have the same asm name but different
semantics and just ensure that you use the right cgraph nodes and decls in the
corresponding callers.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #25 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 8 Oct 2014, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886
> 
> --- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> But is warning/error attribute the only thing on aliases that can hold extra
> semantics info (now or in the future)?  I'd say LTO symtab merging should merge
> what is mergeable, and should leave leave as separate decls with the same
> asm-name what holds non-mergeable semantics on it.
> Say, if you declare some function (or different, just with same asm name) with
> warning attribute in one TU, with error attribute in another TU and without it
> on another TU, IMHO those three decls shouldn't be merged together, you should
> note in cgraph that you have aliases that have the same asm name but different
> semantics and just ensure that you use the right cgraph nodes and decls in the
> corresponding callers.

Yes, I tried to fix things in this direction but failed (maybe didn't try 
hard enough).  Basically I'd never merge decls in lto-symtab - tree
merging already merges exactly equivalent function decls - but only
fixup the cgraph for the tree merging that was done.

Richard.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #26 from Jan Hubicka <hubicka at ucw dot cz> ---
> But is warning/error attribute the only thing on aliases that can hold extra
> semantics info (now or in the future)?  I'd say LTO symtab merging should merge
> what is mergeable, and should leave leave as separate decls with the same
> asm-name what holds non-mergeable semantics on it.
> Say, if you declare some function (or different, just with same asm name) with
> warning attribute in one TU, with error attribute in another TU and without it
> on another TU, IMHO those three decls shouldn't be merged together, you should
> note in cgraph that you have aliases that have the same asm name but different
> semantics and just ensure that you use the right cgraph nodes and decls in the
> corresponding callers.

A I tried to explain, it is currently design decision to have  one declaration
and one symtam node for one symbol.  The one decl rule was introduced by
Codesourcery (Zack) in 2003-4. He updated frontends to avoid copying and
dropped code that dealt with duplicated declarations.  Due to lack of sanity
checking some cases remained, like this one (because at that time we did not
really have proper asm name hash).  There are couple open PRs since that time
that was never corrected.

So either we need to fix remaining cases or revisit the decision and audit
backend/middleend for duplicated decls.  There are cases I know that would need
updating
 - symbol equality folding
 - alias analysis
 - Symbol table allowing many to one mapping for symtab entries.
   I think it would be better to avoid duplicated entries in symbol table,
   so we will need way to associate all declarations with a given symbol.
   Probably ont that big deal except for updating code that deals with 
   changing declaration associated with the node and we need to decide what
   declaration control symbol's visibility. 

   Obviously if user provide two declarations, one is static and ohter public,
   we either want to error or do someting sane.
 - we need to produce errors when user defines two different symbols of same
name
   (currently we produce invalid asm)
 - anchors
 - Zack dropped some code from dwarf2out
 - Visibility in varasm - those need to follow the main declaration.  I had
great
   fun fixing effects of this on AIX

I definitly found the one decl scheme somewhat restrictive, but it also makes
things easier and avoids weird bugs. We could revert this decision, but that is
a project.

Concerning Richards plan to annotate statements with warning strings, I think
we could follow same scheme as EH regions and histograms does - i.e. have on
side hash table annotating statements.

For IPA I am trying to convince Martin Liska to introduce symtab annotation
template
for me that makes it easy to add data to a symbol that is removed/duplicated
along with
the symbol.  Cgraph has the hook API for it, but convenient C++ wrap would be
great.

Here I think we want something similar and convert the existing EH/histograms
to it.

Honza

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #27 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #28 from Richard Biener <rguenth at gcc dot gnu.org> ---
Still broken :(

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zackw at panix dot com

--- Comment #29 from Richard Biener <rguenth at gcc dot gnu.org> ---
*** Bug 62249 has been marked as a duplicate of this bug. ***

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #30 from Richard Biener <rguenth at gcc dot gnu.org> ---
So, do we really want to go without this fixed again, for GCC 5?  Honza?  After
all this is an underlying wrong-code issue!  (wrong function is picked as
prevailing)

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #31 from Jan Hubicka <hubicka at ucw dot cz> ---
> So, do we really want to go without this fixed again, for GCC 5?  Honza?  After
> all this is an underlying wrong-code issue!  (wrong function is picked as
> prevailing)

Well, I have only two hands and do not see reasonably simple solution. Will
look into it
again.  How this cause wrong code?

Honza

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #32 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 11 Feb 2015, hubicka at ucw dot cz wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886
> 
> --- Comment #31 from Jan Hubicka <hubicka at ucw dot cz> ---
> > So, do we really want to go without this fixed again, for GCC 5?  Honza?  After
> > all this is an underlying wrong-code issue!  (wrong function is picked as
> > prevailing)
> 
> Well, I have only two hands and do not see reasonably simple solution. Will
> look into it
> again.  How this cause wrong code?

Hmm, maybe it can't (the "aliases" map to the same symbol).  But at least
if I produce another decl with say, attribute(regparm), and that gets
picked even though I didn't call it then it would be wrong-code
(of course that decl is technically invalid as the symbol it refers to
has different calling conventions).

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[zackw at panix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #34 from Zack Weinberg <zackw at panix dot com> ---
> As I tried to explain, it is currently design decision to have one declaration
> and one symtam node for one symbol.  The one decl rule was introduced by
> Codesourcery (Zack) in 2003-4. He updated frontends to avoid copying and
> dropped code that dealt with duplicated declarations.  Due to lack of sanity
> checking some cases remained, like this one (because at that time we did not
> really have proper asm name hash).  There are couple open PRs since that time
> that was never corrected.

It's been long enough that I don't recall precisely what my goals were, but I
don't think it was my intention to enforce a "one decl per asm name" rule at
the time.  I was trying to deal with C front-end issues: according to
https://gcc.gnu.org/ml/gcc-patches/2004-01/msg00808.html, "at least bug 12267,
bug 12336, bug 12373, bug 12391, bug 12560, bug 12934, bug 13129".  My
recollection is that the fundamental problem was getting more than one
TREE_DECL object per *declared name* in -funit-at-a-time mode, which was shiny
and new back then!

... Digging through the mailing list archives, maybe I'm remembering wrong. 
https://gcc.gnu.org/ml/gcc-patches/2004-03/msg01440.html has

> The old code took the copying
> approach; this was bad because it violated the basic assumption made
> elsewhere in the compiler that there is exactly one DECL node for each
> assembly-level symbol.  Hence all the bugs.

I do not remember anymore what parts of the compiler were making those
assumptions, but I'm guessing it was probably the debug information generators,
or mostly those.  Anyhow, I don't think y'all should be taking a decision I
made for the C front end under time and peer pressure (people were *really mad*
at me for leaving some of those bugs unfixed for several releases in a row) in
2004 as a permanent compiler-wide design constraint ten years later :)

(Note also bug 13801, and the discussion of its fix, here:
https://gcc.gnu.org/ml/gcc-patches/2004-08/msg00085.html -- we wound up having
to back down from the "not only is there only one DECL per assembly level
symbol, it only ever accumulates information" rule that I originally wanted,
due to C90 conformance problems.)

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #35 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Zack,
happy to hear from you again! Indeed the problem back was quite sloppy and we
kind of mixed up symbols, assembler names and declarations in not well defined
way.

I think the safest way to go is to build on the alias machinery.  For weakref
we already have sense of "syntactic alias" (those that ends up translated to
their target symbols) and I did some auditing recently (motivated by ICF bugs).
Currently we have  node->weakref saying if symbol is weakref and we do have
good part of code aware of this.  I guess we can have node->syntactic_alias
(better name welcome, perhaps transparent?) that express the fact that alias
should get translated to the final symbol during RTL output the same way as we
do weakref on targets where they are not supported.
Then it is a question where we want to translate the duplicated declarations
into these aliases.  I guess I can do it within the visibility itself or the
FEs can be responsible for it.

We can also get more fancy and try to solve the GNU extern inline issues - have
a syntactic alias with also has boddy associated with it.

I will try to start pushing things this direction.

[Bug lto/61886] [4.8/4.9/5 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #36 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
This is probably too risky to fix for 5.1
(https://gcc.gnu.org/ml/gcc/2015-03/msg00241.html) though I am having WIP patch
now - we need to introduce transparent alias support and make lto-symtab to
turn incompatible decls into transparent aliases.

I hope to have patch ready in a week for consideration. Chromium is using
fortify, so it would be nice to fix this.

[Bug lto/61886] [4.8/4.9/5/6 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.5                       |4.9.3

--- Comment #37 from Richard Biener <rguenth at gcc dot gnu.org> ---
The gcc-4_8-branch is being closed, re-targeting regressions to 4.9.3.

[Bug lto/61886] [4.9/5/6 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

--- Comment #38 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.9.3 has been released.

[Bug lto/61886] [4.9/5/6 Regression] LTO breaks fread with _FORTIFY_SOURCE=2

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61886

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.9.3                       |4.9.4