[Bug middle-end/65074] New: [5 Regression] r220674 broke C++ PIEs

[Bug middle-end/65074] New: [5 Regression] r220674 broke C++ PIEs

[jakub at gcc dot gnu.org]

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

            Bug ID: 65074
           Summary: [5 Regression] r220674 broke C++ PIEs
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
                CC: hjl at gcc dot gnu.org, rth at gcc dot gnu.org

namespace std
{
  template <typename _CharT> struct char_traits;
  template <typename _CharT, typename = char_traits <_CharT> > class
basic_filebuf;
  template <typename _CharT, typename = char_traits <_CharT> > class
basic_ifstream;
  typedef basic_ifstream <char> ifstream;
  enum _Ios_Openmode { _S_in };
  class basic_streambuf
  {
  };
  struct basic_ios
  {
    typedef char char_type;
    void init (void *);
    typedef _Ios_Openmode openmode;
  };
  template <typename _CharT> class __basic_file;
  template <> struct __basic_file <char>
  {
    __basic_file ();
    ~__basic_file ();
  };
  template <typename, typename _Traits> struct basic_filebuf : public
basic_streambuf
  {
    __basic_file <char> _M_file;
    ~basic_filebuf () { this->close (); }
    void close ();
  };
  template <typename, typename _Traits> struct basic_ifstream : basic_ios
  {
    typedef _Traits traits_type;
    basic_filebuf <char_type, traits_type> _M_filebuf;
    basic_ifstream ();
    basic_ifstream (const char *, basic_ios::openmode = _S_in)
    {
      this->init (0);
    }
  };
  extern template class basic_filebuf <char>;
  ifstream f (0);
}

with -O2 -fpie starting with r220674 uses
call _ZNSt13basic_filebufIcSt11char_traitsIcEED1Ev
instead of
call _ZNSt13basic_filebufIcSt11char_traitsIcEED1Ev@PLT
that was used previously, despite that symbol being defined in libstdc++.so.6.

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |5.0

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This causes pdns-recursor link failure that without that would link fine.  The
reduced testcase is reduced too much, so with added int main () {} it doesn't
link even before r220674.

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[hjl.tools at gmail dot com]

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

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 34770
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34770&action=edit
A patch

Please try this.

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 34772
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34772&action=edit
gcc5-pr65074-test.patch

Testcase for the testsuite.

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-02-16
     Ever confirmed|0                           |1

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[rth at gcc dot gnu.org]

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

Richard Henderson <rth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34770|0                           |1
        is obsolete|                            |
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rth at gcc dot gnu.org

--- Comment #4 from Richard Henderson <rth at gcc dot gnu.org> ---
Created attachment 34785
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34785&action=edit
second patch

I prefer to remove the test against node->definition entirely.
It appears to mean only that we have a definition that we could
emit, not that we're going to actually emit the symbol.

In particular, for functions, we'd have to also check cgraph_node::process.
At which point I suspect that DECL_EXTERNAL is sufficient.  Which means
that we can at least set defined_locally based on DECL_EXTERNAL even if
there is no symtab_node.  I don't know that we'd ever not have one, but
that's certainly not obvious from here.

Starting regression testing on several platforms...

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[hubicka at gcc dot gnu.org]

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

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

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

--- Comment #5 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Hi,
as Jakub noticed I just fixed the same bug independently by
https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01004.html

The fix is bit different. It adds DECL_EXTERNAL check in addition to
node->definition.

Concerning discussion here, node->definition really just means that the
definition is there at the time you check the flag.  It does not imply it will
be output - it can be removed later by IPA optimization or it may be a
definition of external variable or alias.

Using node->process flag is not a good idea, because these flags are set only
while generating final assembler output, while we use binds_local_p during IPA
optimization, too.

I see nothing wrong with dropping the use of ->definition flag as suggested by
Richard. It is probably good cleanup compared to my change. The lto-cgraph.c
and symtab.c parts of my change are still useful to fix some of nonsenses WRT
definitions and ltrans partitions.

Honza

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[rth at gcc dot gnu.org]

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

--- Comment #6 from Richard Henderson <rth at gcc dot gnu.org> ---
Author: rth
Date: Thu Feb 19 15:14:24 2015
New Revision: 220816

URL: https://gcc.gnu.org/viewcvs?rev=220816&root=gcc&view=rev
Log:
PR middle-end/65074

 * varasm.c (default_binds_local_p_2): Don't test node->definition;
 test DECL_EXTERNAL independent of symtab_node.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/varasm.c

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[rth at gcc dot gnu.org]

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

--- Comment #7 from Richard Henderson <rth at gcc dot gnu.org> ---
Author: rth
Date: Thu Feb 19 15:48:50 2015
New Revision: 220817

URL: https://gcc.gnu.org/viewcvs?rev=220817&root=gcc&view=rev
Log:
PR middle-end/65074

 * g++.dg/opt/pr65074.C: New file.

Added:
    trunk/gcc/testsuite/g++.dg/opt/pr65074.C
Modified:
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/65074] [5 Regression] r220674 broke C++ PIEs

[rth at gcc dot gnu.org]

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

Richard Henderson <rth at gcc dot gnu.org> changed:

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

--- Comment #8 from Richard Henderson <rth at gcc dot gnu.org> ---
Fixed.