Bug 54314 - [4.8 Regression] undefined references to 'construction vtable for std::ostream-in-std::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >'
Summary: [4.8 Regression] undefined references to 'construction vtable for std::ostrea...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.8.0
: P3 major
Target Milestone: 4.8.0
Assignee: Jason Merrill
URL:
Keywords: link-failure
: 53518 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-08-18 10:32 UTC by Ruben Van Boxem
Modified: 2021-12-24 08:36 UTC (History)
10 users (show)

See Also:
Host:
Target: *-*-mingw*
Build:
Known to work: 4.7.2
Known to fail: 4.8.0
Last reconfirmed: 2012-09-10 00:00:00


Attachments
proposed patch (195 bytes, application/octet-stream)
2012-08-21 20:10 UTC, gee
Details
Draft, sanity checked x86_64-linux (466 bytes, patch)
2012-09-26 09:00 UTC, Paolo Carlini
Details | Diff
proposed patch (1.13 KB, patch)
2013-01-26 05:38 UTC, Jason Merrill
Details | Diff
gcc48-pr54314.patch (752 bytes, patch)
2013-02-04 10:44 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ruben Van Boxem 2012-08-18 10:32:30 UTC
I'm trying to build GCC 4.8 for x86_64-w64-mingw32. I have a cross-compiler but I cannot build my C++ code with it.

The error is exactly the same as the one detailed here:
http://gcc.gnu.org/ml/gcc-patches/2012-05/msg01142.html

I seem to be unable to detect when exactly this problem shows up, but I run into it when building PPL-0.12.1 with the Linux->Windows cross-compiler:

/bin/bash ../../libtool  --tag=CXX   --mode=link x86_64-w64-mingw32-g++  -g -O2 -frounding-math  -W -Wall   -o ppl_pips.exe ppl_pips.o ../../src/libppl.la ../../utils/libppl_utils.a -L/home/ruben/mingw-w64/prereq/x86_64-w64-mingw32/install/lib -lgmpxx -lgmp 
libtool: link: x86_64-w64-mingw32-g++ -g -O2 -frounding-math -W -Wall -o ppl_pips.exe ppl_pips.o  ../../src/.libs/libppl.a -L/home/ruben/mingw-w64/prereq/x86_64-w64-mingw32/install/lib ../../utils/libppl_utils.a /home/ruben/mingw-w64/prereq/x86_64-w64-mingw32/install/lib/libgmpxx.a /home/ruben/mingw-w64/prereq/x86_64-w64-mingw32/install/lib/libgmp.a
ppl_pips.o: In function `basic_istream':
/home/ruben/mingw-w64/linux64mingw64/mingw64/x86_64-w64-mingw32/include/c++/4.8.0/istream:609: undefined reference to `construction vtable for std::istream-in-std::basic_istringstream<char, std::char_traits<char>, std::allocator<char> >'
/home/ruben/mingw-w64/linux64mingw64/mingw64/x86_64-w64-mingw32/include/c++/4.8.0/istream:609: undefined reference to `construction vtable for std::istream-in-std::basic_istringstream<char, std::char_traits<char>, std::allocator<char> >'
ppl_pips.o: In function `~basic_istream':
/home/ruben/mingw-w64/linux64mingw64/mingw64/x86_64-w64-mingw32/include/c++/4.8.0/istream:106: undefined reference to `construction vtable for std::istream-in-std::basic_istringstream<char, std::char_traits<char>, std::allocator<char> >'
ppl_pips.o: In function `basic_istream':
/home/ruben/mingw-w64/linux64mingw64/mingw64/x86_64-w64-mingw32/include/c++/4.8.0/istream:609: undefined reference to `construction vtable for std::istream-in-std::basic_istringstream<char, std::char_traits<char>, std::allocator<char> >'
/home/ruben/mingw-w64/linux64mingw64/mingw64/x86_64-w64-mingw32/include/c++/4.8.0/istream:609: undefined reference to `construction vtable for std::istream-in-std::basic_istringstream<char, std::char_traits<char>, std::allocator<char> >'
ppl_pips.o:/home/ruben/mingw-w64/linux64mingw64/mingw64/x86_64-w64-mingw32/include/c++/4.8.0/istream:106: more undefined references to `construction vtable for std::istream-in-std::basic_istringstream<char, std::char_traits<char>, std::allocator<char> >' follow
collect2: error: ld returned 1 exit status

This effectively makes g++ useless. I seem to remember only non-debug builds are affected.
Comment 1 gee 2012-08-18 14:53:53 UTC
bug 53518 also has this problem.
reference type in method argument causes symbol reference of construction
vtable for ***.
so, construction vtable for * symbol must not be marked as weak in PE target.
since PE target cannot handle weak symbol.
or, just reverting the problematic commit would be good.
Comment 2 Richard Biener 2012-08-20 07:00:27 UTC
Can you identify the commit that caused this?
Comment 3 gee 2012-08-20 15:21:53 UTC
--export-all-symbols didn't work at all.
pick-reverting just one commit failed with conflict.


[966](sec 86)(fl 0x00)(ty   0)(scl   2) (nx 0) 0x00000000 
Contents of section .rdata$_ZNSt14numeric_limitsIdE14min_exponent10E:
 0000 cdfeffff                          ....

following is one of the symbol that linker failed to include in shared library.

__ZNSt14numeric_limitsIdE14min_exponent10E
	.globl	__ZNSt14numeric_limitsIdE14min_exponent10E
	.section	.rdata$_ZNSt14numeric_limitsIdE14min_exponent10E,"dr"
	.align 4
__ZNSt14numeric_limitsIdE14min_exponent10E:
	.long	-307
Comment 4 gee 2012-08-20 16:15:41 UTC
(In reply to comment #3)
> --export-all-symbols didn't work at all.
> pick-reverting just one commit failed with conflict.
> 
> 
> [966](sec 86)(fl 0x00)(ty   0)(scl   2) (nx 0) 0x00000000 
> Contents of section .rdata$_ZNSt14numeric_limitsIdE14min_exponent10E:
>  0000 cdfeffff                          ....
> 
> following is one of the symbol that linker failed to include in shared library.
> 
> __ZNSt14numeric_limitsIdE14min_exponent10E
>     .globl    __ZNSt14numeric_limitsIdE14min_exponent10E
>     .section    .rdata$_ZNSt14numeric_limitsIdE14min_exponent10E,"dr"
>     .align 4
> __ZNSt14numeric_limitsIdE14min_exponent10E:
>     .long    -307

objdump -p src/.libs/cygstdc++-6.dll |grep "_ZNSt14numeric_limitsIdE14min_exponent10E"
        [1747] _ZNSt14numeric_limitsIdE14min_exponent10E
 85 .rdata$_ZNSt14numeric_limitsIdE14min_exponent10E 00000004  00000000  00000000  00004644  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
above symbol was defined in dll, sorry for the noise.


$ objdump -p src/.libs/cygstdc++-6.dll |grep "_ZTCSt13basic_fstreamIwSt11char_traitsIwEE0_St14basic_iostreamIwS1_E"


209 .rdata$_ZTCSt13basic_fstreamIwSt11char_traitsIwEE0_St14basic_iostreamIwS1_E 00000040  00000000  00000000  0000bd14  2**5
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA, LINK_ONCE_SAME_SIZE (COMDAT __ZTCSt13basic_fstreamIwSt11char_traitsIwEE0_St14basic_iostreamIwS1_E 966)

	.globl	__ZTCSt13basic_fstreamIwSt11char_traitsIwEE0_St14basic_iostreamIwS1_E
	.section	.rdata$_ZTCSt13basic_fstreamIwSt11char_traitsIwEE0_St14basic_iostreamIwS1_E,"dr"
	.linkonce same_size
	.align 32
__ZTCSt13basic_fstreamIwSt11char_traitsIwEE0_St14basic_iostreamIwS1_E:
	.long	136

only difference for both section is RELOC, LINK_ONCE_SAME_SIZE. and some symbol in section with reloc failed to export symbols within.
Comment 5 gee 2012-08-21 13:38:57 UTC
I think symbol _ZTCSt* need to be included in libstdc++/config/abi/pre/gnu.ver so that shared-library can export these symbols unless user   did append --disable-symvers.
nothing need to be done such as reverting the commit or so.
Comment 6 gee 2012-08-21 20:10:01 UTC
Created attachment 28065 [details]
proposed patch

just added one line.
_ZTC* is then exported.
Comment 7 Jason Merrill 2012-09-04 19:28:31 UTC
Changing component to libstdc++.
Comment 8 Paolo Carlini 2012-09-05 10:25:16 UTC
I think we should identify when this changed and why. Then, we can certainly add the export (please send a regular patch to the library mailing list) but at a new minor version, thus CXXABI_1.3.7.
Comment 9 Kai Tietz 2012-09-10 11:45:36 UTC
(In reply to comment #8)
> I think we should identify when this changed and why. Then, we can certainly
> add the export (please send a regular patch to the library mailing list) but at
> a new minor version, thus CXXABI_1.3.7.

The issue was exposed by trunc's new feature for PECOFF to place readonly-data with relocation into the .rdata section.  Interesting that this wasn't noticed before, but to add _[_]ZTC* to gnu.ver fixes the issue.
Comment 10 Ruben Van Boxem 2012-09-25 08:11:23 UTC
I can confirm adding the exports gets rid of the undefined references.
Comment 11 Paolo Carlini 2012-09-26 09:00:49 UTC
Created attachment 28279 [details]
Draft, sanity checked x86_64-linux
Comment 12 Paolo Carlini 2012-09-26 09:01:58 UTC
Kai, I attached a complete draft which passes a sanity check on Linux. Please let me know if it works for you and we can resolve this.
Comment 13 Benjamin Kosnik 2012-09-26 19:06:12 UTC
Hey P, I think you mean:

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/g
index 5265b21..396feec 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1322,6 +1322,7 @@ GLIBCXX_3.4.17 {
 } GLIBCXX_3.4.16;
 
 GLIBCXX_3.4.18 {
+
   global:
 
     # Names inside the 'extern' block are demangled names.
@@ -1330,6 +1331,9 @@ GLIBCXX_3.4.18 {
       std::random_device::*;
     };
 
+    # construction vtable
+    _ZTCSt*;
+
 } GLIBCXX_3.4.17;
 
 # Symbols in the support library (libsupc++) have their own tag.


ie, not in CXXABI for std:: non-support things.

This is an interesting thread thanks for the info Kai, very informative. The analysis looks good and patch looks correct, modulo above.

Anyway, i have to add this export to gnu-versioned as well, so if it's ok with you I'll check in this modified patch along with the gnu-versioned-namespace one.

best,
Benjamin
Comment 14 Kai Tietz 2012-09-26 20:09:16 UTC
(In reply to comment #13)
> Hey P, I think you mean:
> 
> diff --git a/libstdc++-v3/config/abi/pre/gnu.ver
> b/libstdc++-v3/config/abi/pre/g
> index 5265b21..396feec 100644
> --- a/libstdc++-v3/config/abi/pre/gnu.ver
> +++ b/libstdc++-v3/config/abi/pre/gnu.ver
> @@ -1322,6 +1322,7 @@ GLIBCXX_3.4.17 {
>  } GLIBCXX_3.4.16;
> 
>  GLIBCXX_3.4.18 {
> +
>    global:
> 
>      # Names inside the 'extern' block are demangled names.
> @@ -1330,6 +1331,9 @@ GLIBCXX_3.4.18 {
>        std::random_device::*;
>      };
> 
> +    # construction vtable
> +    _ZTCSt*;
> +
>  } GLIBCXX_3.4.17;
> 
>  # Symbols in the support library (libsupc++) have their own tag.
> 
> 
> ie, not in CXXABI for std:: non-support things.
> 
> This is an interesting thread thanks for the info Kai, very informative. The
> analysis looks good and patch looks correct, modulo above.
> 
> Anyway, i have to add this export to gnu-versioned as well, so if it's ok with
> you I'll check in this modified patch along with the gnu-versioned-namespace
> one.

Sure, I am fine by your modified patch.  Thanks to you Benjamin, and Paolo for the patch.
Comment 15 Paolo Carlini 2012-09-26 21:06:28 UTC
Ah, Ok. Benjamin, please handle this, because I don't want to make other mistakes here. Thanks!
Comment 16 Benjamin Kosnik 2012-09-27 00:05:07 UTC
Author: bkoz
Date: Thu Sep 27 00:05:03 2012
New Revision: 191788

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191788
Log:
2012-09-26  Benjamin Kosnik  <bkoz@redhat.com>

       PR libstdc++/54314
       * config/abi/pre/gnu.ver: Add vtable exports.
       * config/abi/pre/gnu-versioned-namespace.ver: Same.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
    trunk/libstdc++-v3/config/abi/pre/gnu.ver
Comment 17 Benjamin Kosnik 2012-09-27 17:05:35 UTC
Fixed.
Comment 18 Jason Merrill 2013-01-25 20:35:52 UTC
*** Bug 53518 has been marked as a duplicate of this bug. ***
Comment 19 Jason Merrill 2013-01-25 20:37:07 UTC
This change turns out to be wrong; we don't want to export the construction vtables, as they should only be referenced by the VTTs which are emitted along with them.  If other code is referring to those symbols, that's the bug that should be fixed.
Comment 20 Jason Merrill 2013-01-26 05:38:54 UTC
Created attachment 29277 [details]
proposed patch

This patch fixes the bug by setting DECL_VISIBILITY_SPECIFIED on the constructor vtable.  Interestingly, can_refer_decl_in_current_unit_p only checks for that in deciding whether or not it's safe to refer to an external decl, not what the specified visibility is; as a result, this issue wasn't showing up on linux-gnu because the standard library has an explicit visibility("default").  On targets that don't use attribute visibility, this was having trouble because the symbol had unspecified visibility at compile time but then was made hidden by the linker script.  Is there something better we can do about symbols that may or may not end up hidden at link time?

I think this change to make the constructor vtables hidden is safe because the optimizer change that caused us to refer to them inappropriately is new in 4.8, so we don't have to worry about ABI issues with earlier releases.

Any thoughts?
Comment 21 Jakub Jelinek 2013-01-26 10:05:58 UTC
I must say I'm surprised by the gimple-fold.c test, I'd really expect additional && DECL_VISIBILITY (decl) != VISIBILITY_DEFAULT .
Another alternative to make those always hidden (which could be an exported ABI change for some shared libraries, though unlikely to be actually a real problem) would be to add some new bit, either in the tree itself, or better in symtab
node, which would tell gimple-fold.c not to optimize it.  Honza?
Comment 22 Jakub Jelinek 2013-01-26 10:38:23 UTC
BTW, I agree with Jason that we shouldn't optimize these vtable reads.  When this hit libstdc++, it could hit very well any other C++ shared library which uses version scripts or some other ways how to enumerate what exactly is exported.
Comment 23 Jan Hubicka 2013-01-26 18:32:15 UTC
> I must say I'm surprised by the gimple-fold.c test, I'd really expect
> additional && DECL_VISIBILITY (decl) != VISIBILITY_DEFAULT .
> Another alternative to make those always hidden (which could be an exported ABI
> change for some shared libraries, though unlikely to be actually a real
> problem) would be to add some new bit, either in the tree itself, or better in
> symtab
> node, which would tell gimple-fold.c not to optimize it.  Honza?

Yep, I was just wondering about the same.  It seems C++ frontend has to mark those
declarations as unusable for referencing.  Is it easy to do on C++ side?
I think we want tree visibility flag here, because we do not really represent
stuff that may get into the game with TBAA.
Comment 24 Jason Merrill 2013-01-27 01:44:49 UTC
(In reply to comment #21)
> I must say I'm surprised by the gimple-fold.c test, I'd really expect
> additional && DECL_VISIBILITY (decl) != VISIBILITY_DEFAULT .

I'd actually expect DECL_VISIBILITY (decl) < VISIBILITY_PROTECTED, and not check DECL_VISIBILITY_SPECIFIED at all.  I don't think it matters whether the visibility that the decl ends up with is specified or inferred.

> Another alternative to make those always hidden (which could be an exported
> ABI change for some shared libraries, though unlikely to be actually a real
> problem)

At worst, it removes some unnecessarily exported symbols.  Which might cause ABI checkers to complain, but nothing else should refer to them; if something else had, we would have seen this bug before.  I think marking them as hidden is desirable, as it avoids unnecessary exports for user libraries that don't use linker scripts or explicit visibility as well as fixing this bug.

(In reply to comment #23)
> It seems C++ frontend has to mark those
> declarations as unusable for referencing.  Is it easy to do on C++ side?
> I think we want tree visibility flag here, because we do not really represent
> stuff that may get into the game with TBAA.

I don't understand.  My patch sets DECL_VISIBILITY to hidden; are you talking about a different tree visibility flag?
Comment 25 Jakub Jelinek 2013-01-28 11:27:51 UTC
I meant the ABI checkers only.  Anyway, on the other side given comments like:
   This mangling isn't part of the ABI specification; in the ABI
   specification, the vtable group is dumped in the same COMDAT as the
   main vtable, and is referenced only from that vtable, so it doesn't
   need an external name.  For binary formats without COMDAT sections,
   though, we need external names for the vtable groups.
and
Construction virtual tables are not exported because
they cannot be referred to from other object files;
their name is not standardized by the ABI.

perhaps making them hidden whenever possible is really desirable.
Comment 26 Jan Hubicka 2013-01-28 14:56:12 UTC
> perhaps making them hidden whenever possible is really desirable.

Yes, this seems fine to me. Just to be sure I understand the problem fully.
I believe there is still problem with folding. When folding references through
external vtable I think we can still invent direct references to construction
vtable.

While doing so we go through logic in can_refer_decl_in_current_unit_p.
This is trying to validate the reference to the symbol referred only outside
of current unit.

I sort of hacked it together trying to work out what rules are.
This is all wrong for construction vtables where, if I understand it right,
we can not reffer them in any case because other file may be from other compiler.
(with excetion of ltrans and LTO, also resolution data may tell us that the
symbol is defined?)

  /* We are folding reference from external vtable.  The vtable may reffer
     to a symbol keyed to other compilation unit.  The other compilation
     unit may be in separate DSO and the symbol may be hidden.  */
  if (DECL_VISIBILITY_SPECIFIED (decl)
      && DECL_EXTERNAL (decl)
      && (!(snode = symtab_get_node (decl)) || !snode->symbol.in_other_partition))
    return false;
  /* When function is public, we always can introduce new reference.
     Exception are the COMDAT functions where introducing a direct
     reference imply need to include function body in the curren tunit.  */
  if (TREE_PUBLIC (decl) && !DECL_COMDAT (decl))
    return true;
  /* We are not at ltrans stage; so don't worry about WHOPR.
     Also when still gimplifying all referred comdat functions will be
     produced.

     As observed in PR20991 for already optimized out comdat virtual functions
     it may be tempting to not necessarily give up because the copy will be
     output elsewhere when corresponding vtable is output.
     This is however not possible - ABI specify that COMDATs are output in
     units where they are used and when the other unit was compiled with LTO
     it is possible that vtable was kept public while the function itself
     was privatized. */
  if (!flag_ltrans && (!DECL_COMDAT (decl) || !cgraph_function_flags_ready))
    return true;

  /* OK we are seeing either COMDAT or static variable.  In this case we must
     check that the definition is still around so we can refer it.  */
  if (TREE_CODE (decl) == FUNCTION_DECL)
    ....
  return true;

I guess we need to add check for construction vtables and disable references
There are options.

1) Just add the check.  We will then miss all devirtualization oppurtunities
   through the construction vtable.  I guess it is what we should do for 4.8
   and earlier.  Marking them hidden will prevent folders from using them, but 
   what happens on targets not supporting visibility?

2) We may teach all optimizers, like ccp/gvn and fold to only "jump across them"
   and fold only when the reference to construction vtable is used by reference
   to method itself, but it is not how the optimizers are organized - they
   always do one folding at a time and expect that they can reffer to them.

3) Perhaps we can use DECL_VALUE_EXPR or somethin similar to keep track of
   the canonical way to reffer into tthe construction vtable? (i.e. by reference
   to vtable) We can then lower all direct references to the indirect
   references late, i.e. in expansion?

I suppose 3) makes most sense in longer run.  Implementing 2) without missing
optimization oppurtunities would be hard.
Honza
Comment 27 Jason Merrill 2013-01-28 15:42:57 UTC
(In reply to comment #26)
> 1) Just add the check.  We will then miss all devirtualization oppurtunities
>    through the construction vtable.

The front end does devirtualization itself for calls within constructors, so the impact of this isn't likely to be too bad.

>    what happens on targets not supporting visibility?

I think that setting DECL_VISIBILITY should do the trick even if it isn't reflected in the assembly output.  I added the DECL_ARTIFICIAL check to default_assemble_visibility so that this wouldn't cause warnings.

> 3) Perhaps we can use DECL_VALUE_EXPR or somethin similar to keep track of
>    the canonical way to reffer into tthe construction vtable? (i.e. by
> reference
>    to vtable) We can then lower all direct references to the indirect
>    references late, i.e. in expansion?

Hmm, that makes sense.
Comment 28 Jan Hubicka 2013-01-28 19:05:40 UTC
> > 1) Just add the check.  We will then miss all devirtualization oppurtunities
> >    through the construction vtable.
> 
> The front end does devirtualization itself for calls within constructors, so
> the impact of this isn't likely to be too bad.

Does it cover all the cases we can potentially handle by middle-end? Or does it stop
i.e. at function boundary?
On the other hand I would say that calls close to consturction are those that are
likely to be devirtualized, so we should not ignore thoe.

> >    what happens on targets not supporting visibility?
> 
> I think that setting DECL_VISIBILITY should do the trick even if it isn't
> reflected in the assembly output.  I added the DECL_ARTIFICIAL check to
> default_assemble_visibility so that this wouldn't cause warnings.

Yes, that seem like way to go.
> 
> > 3) Perhaps we can use DECL_VALUE_EXPR or somethin similar to keep track of
> >    the canonical way to reffer into tthe construction vtable? (i.e. by
> > reference
> >    to vtable) We can then lower all direct references to the indirect
> >    references late, i.e. in expansion?
> 
> Hmm, that makes sense.

OK, if you help me with C++ bits, I can give this a try.

Honza
Comment 29 Jason Merrill 2013-01-29 17:24:58 UTC
Author: jason
Date: Tue Jan 29 17:24:51 2013
New Revision: 195550

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195550
Log:
	PR libstdc++/54314
gcc/
	* varasm.c (default_assemble_visibility): Don't warn about
	visibility on artificial decls.
gcc/cp/
	* class.c (build_ctor_vtbl_group): Give construction vtables
	hidden visibility.
libstdc++-v3/
	* config/abi/pre/gnu.ver: Don't export construction vtables.
	* config/abi/pre/gnu-versioned-namespace.ver: Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c
    trunk/gcc/varasm.c
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
    trunk/libstdc++-v3/config/abi/pre/gnu.ver
Comment 30 Jason Merrill 2013-01-29 17:48:40 UTC
Fixed.

(In reply to comment #28)
> > The front end does devirtualization itself for calls within constructors, so
> > the impact of this isn't likely to be too bad.
> 
> Does it cover all the cases we can potentially handle by middle-end? Or does > it stop i.e. at function boundary?

It stops at the function boundary, so it isn't a complete solution, but it helps.
Comment 31 Rainer Orth 2013-02-04 10:16:18 UTC
The patch seems to have broken many testcases on platforms like Solaris 9 with
Sun as that lack visibility support:

FAIL: g++.dg/abi/covariant2.C -std=c++98 (test for excess errors)
Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/abi/covariant2.C:31:20: warning
: visibility attribute not supported in this configuration; ignored [-Wattribute
s]
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/abi/covariant2.C:31:20: warning
: visibility attribute not supported in this configuration; ignored [-Wattribute
s]

  Rainer
Comment 32 Jakub Jelinek 2013-02-04 10:22:36 UTC
(In reply to comment #31)
> The patch seems to have broken many testcases on platforms like Solaris 9 with
> Sun as that lack visibility support:
> 
> FAIL: g++.dg/abi/covariant2.C -std=c++98 (test for excess errors)
> Excess errors:
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/abi/covariant2.C:31:20:
> warning
> : visibility attribute not supported in this configuration; ignored
> [-Wattribute
> s]
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/abi/covariant2.C:31:20:
> warning
> : visibility attribute not supported in this configuration; ignored
> [-Wattribute
> s]

That's very weird, because build_vtable marks the vtable decl as DECL_ARTIFICIAL
and thus it shouldn't be warned about.  Or is it some other decl that copies the visibility from the construction vtable decl?
Comment 33 Jakub Jelinek 2013-02-04 10:44:19 UTC
Created attachment 29347 [details]
gcc48-pr54314.patch

Ah, I see, solaris and mingw/cygwin have their own assemble_visibility target hooks that warn on their own, so need the same adjustment as the default hook.
Comment 34 Jakub Jelinek 2013-02-04 17:20:05 UTC
Author: jakub
Date: Mon Feb  4 17:19:56 2013
New Revision: 195723

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195723
Log:
	PR libstdc++/54314
	* config/i386/winnt.c (i386_pe_assemble_visibility): Don't warn
	about visibility on artificial decls.
	* config/sol2.c (solaris_assemble_visibility): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/winnt.c
    trunk/gcc/config/sol2.c
Comment 35 Jan Hubicka 2013-03-12 12:42:34 UTC
Jason,
I am still bit confused how keyed construction vtables are supposed to work.  If I understand it right, and there is no C++ ABI mangling for those, I think it makes no sense to export them from the unit they are keyed to, since the other units can not referr them anyway.

So perhaps the construction vtables should be always comdat hidden?
Comment 36 Jason Merrill 2013-03-12 13:05:52 UTC
(In reply to comment #35)
> I am still bit confused how keyed construction vtables are supposed to work. 
> If I understand it right, and there is no C++ ABI mangling for those, I think
> it makes no sense to export them from the unit they are keyed to, since the
> other units can not refer them anyway.

Right.

> So perhaps the construction vtables should be always comdat hidden?

Hmm?  My earlier patch made them hidden, and they were already comdat.  Do you mean something else?
Comment 37 Jan Hubicka 2013-03-12 13:32:15 UTC
> > So perhaps the construction vtables should be always comdat hidden?
> 
> Hmm?  My earlier patch made them hidden, and they were already comdat.  Do you
> mean something else?
What I see in the testcase from PR56557 is
_ZTCSt13basic_fstreamIcSt11char_traitsIcEE0_Sd/443 (int (* std::basic_fstream<char>::_ZTCSt13basic_fstreamIcSt11char_traitsIcEE0_Sd [15])(...)) @0x7fa754fda3a8
  Type: variable
  Visibility: external public visibility_specified visibility:hidden virtual artificial
  Aux: @0x7fa754fda410  References: _ZTISd/446 (addr)_ZNSdD1Ev/233 (addr)_ZNSdD0Ev/234 (addr)_ZTISd/446 (addr)_ZThn16_NSdD1Ev/447 (addr)_ZThn16_NSdD0Ev/448 (addr)_ZTISd/446 (addr)_ZTv0_n24_NSdD1Ev/449 (addr)_ZTv0_n24_NSdD0Ev/450 (addr)
  Referring: _ZTTSt13basic_fstreamIcSt11char_traitsIcEE/403 (addr)_ZTTSt13basic_fstreamIcSt11char_traitsIcEE/403 (addr)_ZTTSt13basic_fstreamIcSt11char_traitsIcEE/403 (addr)
  Availability: not-ready
  Varpool flags: initialized analyzed finalized

I.e. it is not COMDAT, but I see it can not be because the class is explicitely instantiated the
the vtable may refer to stuff that is hidden in libstdc++.

The matching definition in fstream-inst.o is indeed COMDAT:

_ZTCSt13basic_fstreamIcSt11char_traitsIcEE0_Sd/862 (int (* std::basic_fstream<char>::_ZTCSt13basic_fstreamIcSt11char_traitsIcEE0_Sd [15])(...)) @0x7fd19064ca28
  Type: variable
  Visibility: force_output public weak comdat comdat_group:_ZTVSt13basic_fstreamIcSt11char_traitsIcEE one_only visibility_specified visibility:hidden virtual artificial
  Same comdat group as: _ZTTSt13basic_fstreamIcSt11char_traitsIcEE/861
  Aux: @0x7fd19064ca90  References: _ZTISd/1138 (addr)_ZNSdD1Ev/722 (addr)_ZNSdD0Ev/723 (addr)_ZTISd/1138 (addr)_ZThn16_NSdD1Ev/1139 (addr)_ZThn16_NSdD0Ev/1140 (addr)_ZTISd/1138 (addr)_ZTv0_n24_NSdD1Ev/1141 (addr)_ZTv0_n24_NSdD0Ev/1142 (addr)
  Referring: _ZTTSt13basic_fstreamIcSt11char_traitsIcEE/861 (addr)_ZTTSt13basic_fstreamIcSt11char_traitsIcEE/861 (addr)_ZTTSt13basic_fstreamIcSt11char_traitsIcEE/861 (addr)
  Availability: not-ready
  Varpool flags: initialized analyzed finalized

I assumed it to be public...  So I suppose there is no way to write constructor of fstream that would not be linked
into libstdc++.so but that is how it is intended to work...

Honza
Comment 38 Jason Merrill 2013-03-12 14:03:05 UTC
(In reply to comment #37)
> I assumed it to be public...  So I suppose there is no way to write constructor
> of fstream that would not be linked into libstdc++.so but that is how it is
> intended to work...

Other places could emit constructors (just not the VTT).  But they won't, because of the extern explicit instantiation.
Comment 39 Pawel Sikora 2013-09-10 09:46:49 UTC
Hi,

i see mentioned linker error on the current gcc-4.8.2 for i686-gnu-linux target
and somehow it works for x86_64-gnu-linux target.


../../../lib/libUnitTest++.a(Test.cpp.o):Test.cpp:function construction vtable for std::__7::basic_ostream<char, std::__7::char_traits<char> >-in-UnitTest::MemoryOutStream: error: undefined reference to 'virtual thunk to std::__7::basic_ostream<char, std::__7::char_traits<char> >::~basic_ostream()'
../../../lib/libUnitTest++.a(Test.cpp.o):Test.cpp:function construction vtable for std::__7::basic_ostream<char, std::__7::char_traits<char> >-in-UnitTest::MemoryOutStream: error: undefined reference to 'virtual thunk to std::__7::basic_ostream<char, std::__7::char_traits<char> >::~basic_ostream()'
../../../lib/libUnitTest++.a(Test.cpp.o):Test.cpp:function construction vtable for std::__7::basic_ostringstream<char, std::__7::char_traits<char>, std::__7::allocator<char> >-in-UnitTest::MemoryOutStream: error: undefined reference to 'virtual thunk to std::__7::basic_ostringstream<char, std::__7::char_traits<char>, std::__7::allocator<char> >::~basic_ostringstream()'
../../../lib/libUnitTest++.a(Test.cpp.o):Test.cpp:function construction vtable for std::__7::basic_ostringstream<char, std::__7::char_traits<char>, std::__7::allocator<char> >-in-UnitTest::MemoryOutStream: error: undefined reference to 'virtual thunk to std::__7::basic_ostringstream<char, std::__7::char_traits<char>, std::__7::allocator<char> >::~basic_ostringstream()'


MemoryOutStream is declared in the UnitTest++ as:

#include <sstream>

namespace UnitTest
{

class MemoryOutStream : public std::ostringstream
{
public:
    MemoryOutStream() {}
    char const* GetText() const;

private:
    MemoryOutStream(MemoryOutStream const&);
    void operator =(MemoryOutStream const&);

    mutable std::string m_text;
};

}
Comment 40 Pawel Sikora 2013-09-10 15:30:46 UTC
$ grep ZTv0 *
gnu.ver:    _ZTv0_n12_NS*;
gnu.ver:    _ZTv0_n24_NS*;
gnu-versioned-namespace.ver:    _ZTv0_n24_NS*;

versioned namespace doesn't provide *n12* for i686.
Comment 41 Kai Tietz 2013-09-10 16:19:48 UTC
Author: ktietz
Date: Tue Sep 10 16:19:45 2013
New Revision: 202474

URL: http://gcc.gnu.org/viewcvs?rev=202474&root=gcc&view=rev
Log:
        Backport from trunk.
        PR libstdc++/54314
        * config/abi/pre/gnu-versioned-namespace.ver: Add thunk _ZTv0_n12_NS*
        like in gnu.ver.

Modified:
    branches/gcc-4_8-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_8-branch/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver