Message ID | 20221116024619.1465996-1-ppalka@redhat.com |
---|---|
State | Committed |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id ADB283952526 for <patchwork@sourceware.org>; Wed, 16 Nov 2022 02:47:28 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org ADB283952526 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1668566848; bh=PGxk0ozAGOFe9JWxb1XcR4O0s4ogU/CfNuINspHazXM=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=KP2Bqhj7TiYAf0KmQOP73ULP/M0sZrNbu3MsJSQj7PHb9Oa5OZi3JLgioy5dLY7kr 6Vkl4Tb9qkKIMi7vRx7m3+IG8AdtC5l0FBsA5AJaF8Pd5WPzG7iGzadpoBcznhjljk UD2cp7WPyFhL3MzOSLuej5iYvL4YmcIY4BcFZRQw= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id CEA1138A8158 for <gcc-patches@gcc.gnu.org>; Wed, 16 Nov 2022 02:46:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CEA1138A8158 Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-256-Ou5gCi0DMzCQqHtBlzxQKQ-1; Tue, 15 Nov 2022 21:46:23 -0500 X-MC-Unique: Ou5gCi0DMzCQqHtBlzxQKQ-1 Received: by mail-qk1-f199.google.com with SMTP id x2-20020a05620a448200b006fa7dad5c1cso15833255qkp.10 for <gcc-patches@gcc.gnu.org>; Tue, 15 Nov 2022 18:46:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=PGxk0ozAGOFe9JWxb1XcR4O0s4ogU/CfNuINspHazXM=; b=VAMMPbfkB1r7W8/itRRvTK7zUbNIIPlH6S2qodBxr/ntRwyZJtJvgelqCxAXPoloDL 9mtUgtldso3+PheJo1ORWwI32AeEJjKqyotOclugCS08G7RvM+M3t49XPix3vc/CcbYS ToFfxxtfrl80X3ytCDWWVYXeLprtc0oh0CPSmecw2OSUjDqVkc2qkrCMrWHl/eXel7M1 06LoxRuz1mnXT3t30+VhkrSMAnhbGQY5BjXT6CDiBwisLDbUC2vVEgKTYtFeUTD5MJP+ jt+sdojkjOrmfYhWnhpCi2GAWG5KEzoyc7OMYyaDjBCje5D/0UxsxTekjwGXS+oJVf+d R5tw== X-Gm-Message-State: ANoB5pl0u5EpsTUKDIH7XzCgY6P440bIW/5IaaQ98iMoXsrVNkKxDhw8 25GxfGOM9BCsf+Siomm+iAVv1rNz9Hr3SgB8qKBwHemPI0ojqA/ka6NQLJDYqsb5D3liU4D5H+C x0qnwqeB+lprRInsaCiIz0UdQE5JjlqoKm2qTpcKyFH+iGiOfOoNEL9+iAlR3D/re2zk= X-Received: by 2002:a05:620a:199d:b0:6fa:e099:bc12 with SMTP id bm29-20020a05620a199d00b006fae099bc12mr18228139qkb.308.1668566782285; Tue, 15 Nov 2022 18:46:22 -0800 (PST) X-Google-Smtp-Source: AA0mqf4UVahcSOFvrAXuwWxrfj3EjhyDw6bmH19imtoJJJiO1/JmqpreXoJf2vd2f9op0BuRtO/G7g== X-Received: by 2002:a05:620a:199d:b0:6fa:e099:bc12 with SMTP id bm29-20020a05620a199d00b006fae099bc12mr18228121qkb.308.1668566781987; Tue, 15 Nov 2022 18:46:21 -0800 (PST) Received: from localhost.localdomain (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id v14-20020ac873ce000000b003a5689134afsm8091561qtp.36.2022.11.15.18.46.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Nov 2022 18:46:21 -0800 (PST) To: gcc-patches@gcc.gnu.org Cc: libstdc++@gcc.gnu.org, Patrick Palka <ppalka@redhat.com> Subject: [PATCH] libstdc++: Fix stream initialization with static library [PR107701] Date: Tue, 15 Nov 2022 21:46:19 -0500 Message-Id: <20221116024619.1465996-1-ppalka@redhat.com> X-Mailer: git-send-email 2.38.1.436.geea7033409 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-14.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Patrick Palka via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Patrick Palka <ppalka@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
libstdc++: Fix stream initialization with static library [PR107701]
|
|
Commit Message
Patrick Palka
Nov. 16, 2022, 2:46 a.m. UTC
When linking with a static library, the linker seems to exclude a constituent .o object (including its global initializers) if nothing from it is referenced by the program (unless e.g. --whole-archive is used). This behavior breaks iostream when linking with static libstdc++.a (on systems that support init priorities) because we're defining the global initializer for the standard stream objects in a separate TU (ios_init.cc) from the stream objects definitions (globals_io.cc). Thus in order to ensure that the global initializer for the standard stream objects doesn't get wrongly dropped when statically linking, we need to perform the initialization from the same TU that actually defines the stream objects. Tested on x86_64-pc-linux-gnu, does this look OK for trunk? PR libstdc++/107701 libstdc++-v3/ChangeLog: * src/c++98/globals_io.cc: Include "io_base_init.h" here instead of ... * src/c++98/ios_init.cc: ... here. * testsuite/17_intro/static.cc: Run the test as well. --- libstdc++-v3/src/c++98/globals_io.cc | 2 ++ libstdc++-v3/src/c++98/ios_init.cc | 2 -- libstdc++-v3/testsuite/17_intro/static.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
Comments
On Tue, 15 Nov 2022, Patrick Palka wrote: > When linking with a static library, the linker seems to exclude a > constituent .o object (including its global initializers) if nothing > from it is referenced by the program (unless e.g. --whole-archive is > used). This behavior breaks iostream when linking with static libstdc++.a > (on systems that support init priorities) because we're defining the > global initializer for the standard stream objects in a separate TU > (ios_init.cc) from the stream objects definitions (globals_io.cc). > > Thus in order to ensure that the global initializer for the standard > stream objects doesn't get wrongly dropped when statically linking, > we need to perform the initialization from the same TU that actually > defines the stream objects. > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? v2 with more comment fixes/additions: -- >8 -- Subject: [PATCH] libstdc++: Fix stream initialization with static library [PR107701] When linking with a static library, the linker seems to discard a constituent .o object (including its global initializers) if nothing from it is referenced by the program (unless e.g. --whole-archive is used). This behavior breaks iostream when linking with static libstdc++.a (on systems that support init priorities) because we define the global initializer for the standard stream objects in a separate TU (ios_init.cc) from the stream objects definitions (globals_io.cc). Thus in order to ensure that the global initializer for the standard stream objects doesn't get wrongly dropped when statically linking, we need to perform the initialization from the same TU that actually defines the stream objects. Tested on x86_64-pc-linux-gnu, does this look OK for trunk? PR libstdc++/107701 libstdc++-v3/ChangeLog: * include/std/iostream (__ioinit): Adjust comment. * src/c++98/globals_io.cc: Include "io_base_init.h" here instead of ... * src/c++98/ios_init.cc: ... here. * src/c++98/ios_base_init.h (__ioinit): Extend comment. * testsuite/17_intro/static.cc: Run the test as well. --- libstdc++-v3/include/std/iostream | 2 +- libstdc++-v3/src/c++98/globals_io.cc | 2 ++ libstdc++-v3/src/c++98/ios_base_init.h | 1 + libstdc++-v3/src/c++98/ios_init.cc | 2 -- libstdc++-v3/testsuite/17_intro/static.cc | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/libstdc++-v3/include/std/iostream b/libstdc++-v3/include/std/iostream index ff78e1cfb87..0c62e9aeb7f 100644 --- a/libstdc++-v3/include/std/iostream +++ b/libstdc++-v3/include/std/iostream @@ -74,7 +74,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // For construction of filebuffers for cout, cin, cerr, clog et. al. // When the init_priority attribute is usable, we do this initialization - // in the compiled library instead (src/c++98/ios_init.cc). + // in the compiled library instead (src/c++98/globals_io.cc). #if !__has_attribute(__init_priority__) static ios_base::Init __ioinit; #endif diff --git a/libstdc++-v3/src/c++98/globals_io.cc b/libstdc++-v3/src/c++98/globals_io.cc index 04fecb22aeb..bfd808b5bbd 100644 --- a/libstdc++-v3/src/c++98/globals_io.cc +++ b/libstdc++-v3/src/c++98/globals_io.cc @@ -69,6 +69,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION fake_wostream wclog; #endif +#include "ios_base_init.h" + _GLIBCXX_END_NAMESPACE_VERSION } // namespace diff --git a/libstdc++-v3/src/c++98/ios_base_init.h b/libstdc++-v3/src/c++98/ios_base_init.h index 1c71038f4a1..b600ec3298e 100644 --- a/libstdc++-v3/src/c++98/ios_base_init.h +++ b/libstdc++-v3/src/c++98/ios_base_init.h @@ -7,6 +7,7 @@ // sufficiently early (so that it happens before any other global // constructor when statically linking with libstdc++.a), instead of // doing so in (each TU that includes) <iostream>. +// This needs to be done in the same TU that defines the stream objects. #if __has_attribute(init_priority) static ios_base::Init __ioinit __attribute__((init_priority(90))); #endif diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc index 4016fcab785..1b5132f1c2d 100644 --- a/libstdc++-v3/src/c++98/ios_init.cc +++ b/libstdc++-v3/src/c++98/ios_init.cc @@ -75,8 +75,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION extern wostream wclog; #endif -#include "ios_base_init.h" - ios_base::Init::Init() { if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0) diff --git a/libstdc++-v3/testsuite/17_intro/static.cc b/libstdc++-v3/testsuite/17_intro/static.cc index ffa7ecb7077..a0d6ed081f8 100644 --- a/libstdc++-v3/testsuite/17_intro/static.cc +++ b/libstdc++-v3/testsuite/17_intro/static.cc @@ -1,4 +1,4 @@ -// { dg-do link { target c++11 } } +// { dg-do run { target c++11 } } // { dg-require-static-libstdcxx } // { dg-options "-static-libstdc++" }
On Wed, 16 Nov 2022 at 02:46, Patrick Palka via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > When linking with a static library, the linker seems to exclude a > constituent .o object (including its global initializers) if nothing > from it is referenced by the program (unless e.g. --whole-archive is > used). This behavior breaks iostream when linking with static libstdc++.a > (on systems that support init priorities) because we're defining the > global initializer for the standard stream objects in a separate TU > (ios_init.cc) from the stream objects definitions (globals_io.cc). > > Thus in order to ensure that the global initializer for the standard > stream objects doesn't get wrongly dropped when statically linking, > we need to perform the initialization from the same TU that actually > defines the stream objects. > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? OK, thanks. I'm a little nervous that future improvements to --gc-sections or LTO could cause the same problem, but I think we're OK for now. > > PR libstdc++/107701 > > libstdc++-v3/ChangeLog: > > * src/c++98/globals_io.cc: Include "io_base_init.h" here > instead of ... > * src/c++98/ios_init.cc: ... here. > * testsuite/17_intro/static.cc: Run the test as well. > --- > libstdc++-v3/src/c++98/globals_io.cc | 2 ++ > libstdc++-v3/src/c++98/ios_init.cc | 2 -- > libstdc++-v3/testsuite/17_intro/static.cc | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libstdc++-v3/src/c++98/globals_io.cc b/libstdc++-v3/src/c++98/globals_io.cc > index 04fecb22aeb..bfd808b5bbd 100644 > --- a/libstdc++-v3/src/c++98/globals_io.cc > +++ b/libstdc++-v3/src/c++98/globals_io.cc > @@ -69,6 +69,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > fake_wostream wclog; > #endif > > +#include "ios_base_init.h" > + > _GLIBCXX_END_NAMESPACE_VERSION > } // namespace > > diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc > index 4016fcab785..1b5132f1c2d 100644 > --- a/libstdc++-v3/src/c++98/ios_init.cc > +++ b/libstdc++-v3/src/c++98/ios_init.cc > @@ -75,8 +75,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > extern wostream wclog; > #endif > > -#include "ios_base_init.h" > - > ios_base::Init::Init() > { > if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0) > diff --git a/libstdc++-v3/testsuite/17_intro/static.cc b/libstdc++-v3/testsuite/17_intro/static.cc > index ffa7ecb7077..a0d6ed081f8 100644 > --- a/libstdc++-v3/testsuite/17_intro/static.cc > +++ b/libstdc++-v3/testsuite/17_intro/static.cc > @@ -1,4 +1,4 @@ > -// { dg-do link { target c++11 } } > +// { dg-do run { target c++11 } } > // { dg-require-static-libstdcxx } > // { dg-options "-static-libstdc++" } > > -- > 2.38.1.436.geea7033409 >
On Wed, 16 Nov 2022 at 10:55, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Wed, 16 Nov 2022 at 02:46, Patrick Palka via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: > > > > When linking with a static library, the linker seems to exclude a > > constituent .o object (including its global initializers) if nothing > > from it is referenced by the program (unless e.g. --whole-archive is > > used). This behavior breaks iostream when linking with static libstdc++.a > > (on systems that support init priorities) because we're defining the > > global initializer for the standard stream objects in a separate TU > > (ios_init.cc) from the stream objects definitions (globals_io.cc). > > > > Thus in order to ensure that the global initializer for the standard > > stream objects doesn't get wrongly dropped when statically linking, > > we need to perform the initialization from the same TU that actually > > defines the stream objects. > > > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > > OK, thanks. Oops, I missed the v2. That's definitely better with the improve comments. OK for v2. > > I'm a little nervous that future improvements to --gc-sections or LTO > could cause the same problem, but I think we're OK for now. > > > > > > PR libstdc++/107701 > > > > libstdc++-v3/ChangeLog: > > > > * src/c++98/globals_io.cc: Include "io_base_init.h" here > > instead of ... > > * src/c++98/ios_init.cc: ... here. > > * testsuite/17_intro/static.cc: Run the test as well. > > --- > > libstdc++-v3/src/c++98/globals_io.cc | 2 ++ > > libstdc++-v3/src/c++98/ios_init.cc | 2 -- > > libstdc++-v3/testsuite/17_intro/static.cc | 2 +- > > 3 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/libstdc++-v3/src/c++98/globals_io.cc b/libstdc++-v3/src/c++98/globals_io.cc > > index 04fecb22aeb..bfd808b5bbd 100644 > > --- a/libstdc++-v3/src/c++98/globals_io.cc > > +++ b/libstdc++-v3/src/c++98/globals_io.cc > > @@ -69,6 +69,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > fake_wostream wclog; > > #endif > > > > +#include "ios_base_init.h" > > + > > _GLIBCXX_END_NAMESPACE_VERSION > > } // namespace > > > > diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc > > index 4016fcab785..1b5132f1c2d 100644 > > --- a/libstdc++-v3/src/c++98/ios_init.cc > > +++ b/libstdc++-v3/src/c++98/ios_init.cc > > @@ -75,8 +75,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > extern wostream wclog; > > #endif > > > > -#include "ios_base_init.h" > > - > > ios_base::Init::Init() > > { > > if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0) > > diff --git a/libstdc++-v3/testsuite/17_intro/static.cc b/libstdc++-v3/testsuite/17_intro/static.cc > > index ffa7ecb7077..a0d6ed081f8 100644 > > --- a/libstdc++-v3/testsuite/17_intro/static.cc > > +++ b/libstdc++-v3/testsuite/17_intro/static.cc > > @@ -1,4 +1,4 @@ > > -// { dg-do link { target c++11 } } > > +// { dg-do run { target c++11 } } > > // { dg-require-static-libstdcxx } > > // { dg-options "-static-libstdc++" } > > > > -- > > 2.38.1.436.geea7033409 > >
diff --git a/libstdc++-v3/src/c++98/globals_io.cc b/libstdc++-v3/src/c++98/globals_io.cc index 04fecb22aeb..bfd808b5bbd 100644 --- a/libstdc++-v3/src/c++98/globals_io.cc +++ b/libstdc++-v3/src/c++98/globals_io.cc @@ -69,6 +69,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION fake_wostream wclog; #endif +#include "ios_base_init.h" + _GLIBCXX_END_NAMESPACE_VERSION } // namespace diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc index 4016fcab785..1b5132f1c2d 100644 --- a/libstdc++-v3/src/c++98/ios_init.cc +++ b/libstdc++-v3/src/c++98/ios_init.cc @@ -75,8 +75,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION extern wostream wclog; #endif -#include "ios_base_init.h" - ios_base::Init::Init() { if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0) diff --git a/libstdc++-v3/testsuite/17_intro/static.cc b/libstdc++-v3/testsuite/17_intro/static.cc index ffa7ecb7077..a0d6ed081f8 100644 --- a/libstdc++-v3/testsuite/17_intro/static.cc +++ b/libstdc++-v3/testsuite/17_intro/static.cc @@ -1,4 +1,4 @@ -// { dg-do link { target c++11 } } +// { dg-do run { target c++11 } } // { dg-require-static-libstdcxx } // { dg-options "-static-libstdc++" }