[Bug c++/50594] New: Option -fwhole-program discards replaced new operator for std::string

[Bug c++/50594] New: Option -fwhole-program discards replaced new operator for std::string

[z0sh at sogetthis dot com]

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

             Bug #: 50594
           Summary: Option -fwhole-program discards replaced new operator
                    for std::string
    Classification: Unclassified
           Product: gcc
           Version: 4.6.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: z0sh@sogetthis.com


The following program fails to output the messages in the replaced allocation
operators, but *only* during the string allocation *and* when the option
`-fwhole-program` is present. (Tested on 4.6.1 and 4.4.3.) That is, the program
behaves differently when compiles with the following two commands:

  g++ -std=c++0x -o prog prog.cpp
  g++ -std=c++0x -o prog prog.cpp -fwhole-program

Note that the allocation used by the "map" is always done correctly.

Program code:

#include <new>
#include <string>
#include <iostream>
#include <cstdlib>
#include <map>

void * operator new(std::size_t n) throw(std::bad_alloc)
{
  void * const p = std::malloc(n);

  if (p == NULL) throw std::bad_alloc();

  std::cerr << "new() requests " << n << " bytes, allocated at " << p << ".\n";

  return p;
}

void operator delete(void * p) throw()
{
  std::cerr << "delete() at " << p << ".\n";
  std::free(p);
}

int main()
{
  std::string s = "Hello World.";

  std::map<int, int> m { { 0, 1 } };

  return s.length();
}

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-10-02 23:03:17 UTC ---
I think you need to mark operator new and delete as being exported.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

--- Comment #2 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-02 23:08:12 UTC ---
Note that basic_string<char>, at variance with any std::map instantiation, is
exported by the *.so, in particular the functions managing memory.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[z0sh at sogetthis dot com]

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

--- Comment #3 from Kerrek SB <z0sh at sogetthis dot com> 2011-10-02 23:32:47 UTC ---
Thank you for the replies. Is this behaviour standard-conforming? Also, could
you tell me how I "mark the operators as exported", or just anything else
that'll make the program behave as expected?

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

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

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-02 23:41:39 UTC ---
I don't see anything non-standard conforming in the library, for sure.

Better asking Richard, I think, but I'm afraid you can't really do what you
would like to do as soon as you start linking in code from *any* library (not
just the standard C++ library and its basic_string instantiations) managing
memory.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[daniel.kruegler at googlemail dot com]

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

Daniel Krügler <daniel.kruegler at googlemail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |daniel.kruegler at
                   |                            |googlemail dot com

--- Comment #5 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2011-10-03 13:17:11 UTC ---
I would have expected that the shown program works as expected. I'm quoting
ISO/IEC 14882:2003(E) (but N3290 seems to say the same):

1) [lib.replacement.functions] p2+3:

"2 - A C++ program may provide the definition for any of eight dynamic memory
allocation function signatures declared in header <new> (3.7.3, clause
18):[..]"

"3 - The program’s definitions are used instead of the default versions
supplied by the implementation (18.4). Such replacement occurs prior to program
startup (3.2, 3.6)."

[C++11 adds here: "The program’s definitions shall not be specified as inline."
But we have no inline problem here]

2) [lib.new.delete.single] p2 or p11:

"Replaceable: a C++ program may define a function with this function signature
that displaces the default version defined by the C++ Standard library."

Especially note that (2) says "this function signature", which does IMO not
allow to require the addition of export directives or anything else.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

--- Comment #6 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-03 13:22:23 UTC ---
I have no idea how this can be made to work with -fwhole-program, but other
people know better.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-03 13:41:50 UTC ---
(In reply to comment #3)
> Thank you for the replies. Is this behaviour standard-conforming?

The documentation for -fwhole-program says that all functions become static,
which I suspect makes your new and delete operators invalid for replacement
functions.  Andrew's suggestion works, just add these declarations:

void * operator new(std::size_t n) throw(std::bad_alloc)
__attribute__((externally_visible));
void operator delete(void * p) throw() __attribute__((externally_visible));

That prevents -fwhole-program from making them static.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-03 13:43:21 UTC ---
[basic.stc.dynamic.allocation] p1

"a program is ill-formed if an allocation function is declared in a namespace
scope other than global scope or declared static in global scope."

So using -fwhole-program and not making replacement functions
externally-visible produces an ill-formed program (which G++ should probably
reject)

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

--- Comment #9 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-03 13:45:03 UTC ---
Oh, thanks Jon for testing that. Indeed, as far as I'm concerned, the issue is
resolved.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[daniel.kruegler at googlemail dot com]

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

--- Comment #10 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2011-10-03 13:49:01 UTC ---
(In reply to comment #8)

I agree that given the "make static" contract of -fwhole-program (which I was
not aware about) the compiler behaves accordingly. I wonder whether it would be
OK to consider the magic functions as special even for -fwhole-program, because
they do have a "magic-kind-of" specification in the standard.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[redi at gcc dot gnu.org]

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

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-03 15:02:37 UTC ---
we could add __attribute__((externally_visible)) on the declarations in <new>

I don't know what other side effects that would have

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

--- Comment #12 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-03 15:12:53 UTC ---
Yes, yesterday, a bit sleepy, I also wondered that, but I'm not knowledgeable
enough of these mechanisms to say whether it would be otherwise safe.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[z0sh at sogetthis dot com]

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

--- Comment #13 from Kerrek SB <z0sh at sogetthis dot com> 2011-10-03 16:12:28 UTC ---
Very interesting. I understand that making the function static makes the
program ill-formed, but it's still somewhat surprising that a compiler flag
should turn a perfectly valid program into an invalid one.

Perhaps adding those visibility specifiers to the <new> header would be a good
idea, since it'd be a pure-library solution.

Thanks in any case for clarifying this!

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-10-04
     Ever Confirmed|0                           |1

--- Comment #14 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-04 09:26:57 UTC ---
I think that if the user may override certain builtin functions the frontend
should be responsible to inherit the proper visibility of those replacements
so it works even with -fwhole-program (or -flto).

Confirmed as a C++ frontend issue.  To the middle-end those functions are
not in any way special.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |paolo.carlini at oracle dot
                   |                            |com

--- Comment #15 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-04 09:36:45 UTC ---
Interesting, let's add Jason in CC.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[redi at gcc dot gnu.org]

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

--- Comment #16 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-04 09:54:57 UTC ---
I don't think they're special to the front end either, it transforms "new T"
into a call to the relevant library allocation function and then invokes a
constructor.  The allocation functions are just ordinary functions implemented
in libsupc++ but as weak symbols so they can be overridden.  If we just declare
them externally_visible in libsupc++'s <new> won't that work?

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[rguenther at suse dot de]

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

--- Comment #17 from rguenther at suse dot de <rguenther at suse dot de> 2011-10-04 10:03:36 UTC ---
On Tue, 4 Oct 2011, redi at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50594
> 
> --- Comment #16 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-04 09:54:57 UTC ---
> I don't think they're special to the front end either, it transforms "new T"
> into a call to the relevant library allocation function and then invokes a
> constructor.  The allocation functions are just ordinary functions implemented
> in libsupc++ but as weak symbols so they can be overridden.  If we just declare
> them externally_visible in libsupc++'s <new> won't that work?

If it's required to include <new> to override them yes, but I doubt that
from looking at our testsuite ;)

Richard.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[redi at gcc dot gnu.org]

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

--- Comment #18 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-04 10:12:25 UTC ---
Ah yes, my mistake, <new> is not required for all of them, [basic.stc.dynamic]
says

The following allocation and deallocation functions (18.6) are implicitly
declared in global scope in each translation unit of a program.
  void* operator new(std::size_t);
  void* operator new[](std::size_t);
  void operator delete(void*);
  void operator delete[](void*);

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[z0sh at sogetthis dot com]

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

--- Comment #19 from Kerrek SB <z0sh at sogetthis dot com> 2011-10-04 11:59:39 UTC ---
> The following allocation and deallocation functions (18.6) are implicitly
declared in global scope in each translation unit of a program.

If those functions are declared implicitly, could you just amend that implicit
declaration to include the visibility attribute?

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[jason at gcc dot gnu.org]

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

--- Comment #20 from Jason Merrill <jason at gcc dot gnu.org> 2011-10-04 18:37:09 UTC ---
(In reply to comment #19)
> could you just amend that implicit declaration to include the visibility
> attribute?

Certainly.  They're declared in cxx_init_decl_processing:

      push_cp_library_fn (NEW_EXPR, newtype);
      push_cp_library_fn (VEC_NEW_EXPR, newtype);

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely.gcc at gmail dot
                   |                            |com
         AssignedTo|unassigned at gcc dot       |paolo.carlini at oracle dot
                   |gnu.org                     |com

--- Comment #21 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-11 23:30:07 UTC ---
I'm working on this. Indeed, the library bits at this point are more or less
trivial (minor nit: __externally_visible__, not externally_visible).

However, I guess we need a simple testcase for the 4 implicitly defined without
<new> included: can anybody figure out one?

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

--- Comment #22 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-11 23:39:47 UTC ---
... because otherwise I'm not confident I'm changing cxx_init_decl_processing
in the right way: I have a patchlet which fiddles with newattrs and newtype, I
*think* adding the attribute, it doesn't ICE ;), but I don't know if the
attribute is really alive.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo at gcc dot gnu.org]

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

--- Comment #23 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-10-12 18:41:03 UTC ---
Author: paolo
Date: Wed Oct 12 18:40:58 2011
New Revision: 179863

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=179863
Log:
2011-10-12  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/50594
    * decl.c (cxx_init_decl_processing): Add
    __attribute__((externally_visible)) to operator new and
    operator delete library fn.

2011-10-12  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/50594
    * libsupc++/new (operator new, operator delete): Decorate with
    __attribute__((__externally_visible__)).
    * include/bits/c++config: Add _GLIBCXX_THROW.
    * libsupc++/del_op.cc: Adjust.
    * libsupc++/del_opv.cc: Likewise.
    * libsupc++/del_opnt.cc: Likewise.
    * libsupc++/del_opvnt.cc: Likewise.
    * libsupc++/new_op.cc: Likewise.
    * libsupc++/new_opv.cc: Likewise.
    * libsupc++/new_opnt.cc: Likewise.
    * libsupc++/new_opvnt.cc: Likewise.
    * testsuite/18_support/50594.cc: New.
    * testsuite/ext/profile/mutex_extensions_neg.cc: Adjust dg-error
    line number.

Added:
    trunk/libstdc++-v3/testsuite/18_support/50594.cc
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/c++config
    trunk/libstdc++-v3/libsupc++/del_op.cc
    trunk/libstdc++-v3/libsupc++/del_opnt.cc
    trunk/libstdc++-v3/libsupc++/del_opv.cc
    trunk/libstdc++-v3/libsupc++/del_opvnt.cc
    trunk/libstdc++-v3/libsupc++/new
    trunk/libstdc++-v3/libsupc++/new_op.cc
    trunk/libstdc++-v3/libsupc++/new_opnt.cc
    trunk/libstdc++-v3/libsupc++/new_opv.cc
    trunk/libstdc++-v3/libsupc++/new_opvnt.cc
    trunk/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.7.0

--- Comment #24 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-12 18:42:23 UTC ---
Fixed for 4.7.0.

[Bug c++/50594] Option -fwhole-program discards replaced new operator for std::string

[fang at csl dot cornell.edu]

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

David Fang <fang at csl dot cornell.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fang at csl dot cornell.edu

--- Comment #25 from David Fang <fang at csl dot cornell.edu> 2012-03-12 16:31:03 UTC ---
Seeing this failing on powerpc-darwin8.
http://gcc.gnu.org/ml/gcc-testresults/2012-03/msg01296.html

Log shows failed assertion:
Executing on host:
/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/./gcc/g++
-shared-libgcc
-B/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/./gcc
-nostdinc++
-L/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/libstdc++-v3/src
-L/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/libstdc++-v3/src/.libs
-B/sw/lib/gcc4.7/powerpc-apple-darwin8.11.0/bin/
-B/sw/lib/gcc4.7/powerpc-apple-darwin8.11.0/lib/ -isystem
/sw/lib/gcc4.7/powerpc-apple-darwin8.11.0/include -isystem
/sw/lib/gcc4.7/powerpc-apple-darwin8.11.0/sys-include
-B/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/./libstdc++-v3/src/.libs
-g -O2 -D_GLIBCXX_ASSERT -fmessage-length=0 -ffunction-sections -fdata-sections
-g -O2 -g -O2 -DLOCALEDIR="." -nostdinc++
-I/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/libstdc++-v3/include/powerpc-apple-darwin8.11.0
-I/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/libstdc++-v3/include
-I/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/gcc-4.7.0-RC-20120302/libstdc++-v3/libsupc++
-I/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/gcc-4.7.0-RC-20120302/libstdc++-v3/include/backward
-I/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/gcc-4.7.0-RC-20120302/libstdc++-v3/testsuite/util
/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/gcc-4.7.0-RC-20120302/libstdc++-v3/testsuite/18_support/50594.cc
  -fwhole-program ./libtestc++.a -L/sw/lib -liconv  -lm   -o ./50594.exe   
(timeout = 600)
PASS: 18_support/50594.cc (test for excess errors)
Setting LD_LIBRARY_PATH to
:/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/gcc:/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/./libstdc++-v3/../libgomp/.libs:/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/./libstdc++-v3/src/.libs::/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/gcc:/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/./libstdc++-v3/../libgomp/.libs:/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/darwin_objdir/powerpc-apple-darwin8.11.0/./libstdc++-v3/src/.libs
/Volumes/Isolde/fink.build/gcc47-4.7.0-0.rc1.1/gcc-4.7.0-RC-20120302/libstdc++-v3/testsuite/18_support/50594.cc:64:
failed assertion `user_new_called'
FAIL: 18_support/50594.cc execution test