| Summary: | Option -fwhole-program discards replaced new operator for std::string | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Kerrek SB <z0sh> |
| Component: | c++ | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | daniel.kruegler, fang, jason, jwakely.gcc, rguenth |
| Priority: | P3 | ||
| Version: | 4.6.1 | ||
| Target Milestone: | 4.7.0 | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2011-10-04 00:00:00 | |
I think you need to mark operator new and delete as being exported. Note that basic_string<char>, at variance with any std::map instantiation, is exported by the *.so, in particular the functions managing memory. 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? 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. 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. I have no idea how this can be made to work with -fwhole-program, but other people know better. (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. [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) Oh, thanks Jon for testing that. Indeed, as far as I'm concerned, the issue is resolved. (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. we could add __attribute__((externally_visible)) on the declarations in <new> I don't know what other side effects that would have 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. 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! 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. Interesting, let's add Jason in CC. 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? 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.
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*); > 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?
(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); 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? ... 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. 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 Fixed for 4.7.0. 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 |
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(); }