[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
phosek closed this revision. phosek added inline comments. Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128 if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); mcgrathr wrote: > mcgrathr wrote: > > MaskRay wrote: > > > If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) > > > in that `-static` switches the whole link to its special static mode. (as > > > usually while you link libstdc++/libc++ statically, you can still link > > > other libraries normally) > > > > > > In ld.bfd/lld, `-Bstatic` is synonym with `-static`. > > I think this part of the change was unintentional and should be undone. > > Using `--pop-state` obviates the need for `-Bdynamic` to undo `-Bstatic`, > > but `-Bstatic` is still the right switch here. > Actually, it's wrong two ways: the `--pop-state` should come before `-lm`. > Neither `-static` nor `-Bstatic` should apply to `-lm` (or to `-lc` that > comes later, which `-static` might). `-static` vs `-Bstatic` is only a > latent bug given lld, but the `-lm` issue breaks the Zircon build today. > Addressed in D54082. Repository: rC Clang https://reviews.llvm.org/D53854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
mcgrathr reopened this revision. mcgrathr added inline comments. This revision is now accepted and ready to land. Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128 if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); mcgrathr wrote: > MaskRay wrote: > > If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in > > that `-static` switches the whole link to its special static mode. (as > > usually while you link libstdc++/libc++ statically, you can still link > > other libraries normally) > > > > In ld.bfd/lld, `-Bstatic` is synonym with `-static`. > I think this part of the change was unintentional and should be undone. > Using `--pop-state` obviates the need for `-Bdynamic` to undo `-Bstatic`, but > `-Bstatic` is still the right switch here. Actually, it's wrong two ways: the `--pop-state` should come before `-lm`. Neither `-static` nor `-Bstatic` should apply to `-lm` (or to `-lc` that comes later, which `-static` might). `-static` vs `-Bstatic` is only a latent bug given lld, but the `-lm` issue breaks the Zircon build today. Repository: rC Clang https://reviews.llvm.org/D53854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
mcgrathr added inline comments. Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128 if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); MaskRay wrote: > If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in > that `-static` switches the whole link to its special static mode. (as > usually while you link libstdc++/libc++ statically, you can still link other > libraries normally) > > In ld.bfd/lld, `-Bstatic` is synonym with `-static`. I think this part of the change was unintentional and should be undone. Using `--pop-state` obviates the need for `-Bdynamic` to undo `-Bstatic`, but `-Bstatic` is still the right switch here. Repository: rC Clang https://reviews.llvm.org/D53854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
MaskRay added inline comments. Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128 if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in that `-static` switches the whole link to its special static mode. (as usually while you link libstdc++/libc++ statically, you can still link other libraries normally) In ld.bfd/lld, `-Bstatic` is synonym with `-static`. Repository: rC Clang https://reviews.llvm.org/D53854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
This revision was automatically updated to reflect the committed changes. Closed by commit rC346064: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia (authored by phosek, committed by ). Changed prior to commit: https://reviews.llvm.org/D53854?vs=171948&id=172478#toc Repository: rC Clang https://reviews.llvm.org/D53854 Files: lib/Driver/ToolChains/Fuchsia.cpp test/Driver/fuchsia.cpp Index: lib/Driver/ToolChains/Fuchsia.cpp === --- lib/Driver/ToolChains/Fuchsia.cpp +++ lib/Driver/ToolChains/Fuchsia.cpp @@ -122,13 +122,14 @@ if (ToolChain.ShouldLinkCXXStdlib(Args)) { bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) && !Args.hasArg(options::OPT_static); +CmdArgs.push_back("--push-state"); +CmdArgs.push_back("--as-needed"); if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); -if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bdynamic"); +CmdArgs.push_back("-lm"); +CmdArgs.push_back("--pop-state"); } - CmdArgs.push_back("-lm"); } AddRunTimeLibs(ToolChain, D, CmdArgs, Args); Index: test/Driver/fuchsia.cpp === --- test/Driver/fuchsia.cpp +++ test/Driver/fuchsia.cpp @@ -15,7 +15,11 @@ // CHECK-NOT: crti.o // CHECK-NOT: crtbegin.o // CHECK: "-L[[SYSROOT]]{{/|}}lib" -// CHECK: "-lc++" "-lm" +// CHECK: "--push-state" +// CHECK: "--as-needed" +// CHECK: "-lc++" +// CHECK: "-lm" +// CHECK: "--pop-state" // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a" // CHECK: "-lc" // CHECK-NOT: crtend.o @@ -29,8 +33,10 @@ // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \ // RUN: -fuse-ld=lld 2>&1 \ // RUN: | FileCheck %s -check-prefix=CHECK-STATIC -// CHECK-STATIC: "-Bstatic" +// CHECK-STATIC: "--push-state" +// CHECK-STATIC: "--as-needed" +// CHECK-STATIC: "-static" // CHECK-STATIC: "-lc++" -// CHECK-STATIC: "-Bdynamic" // CHECK-STATIC: "-lm" +// CHECK-STATIC: "--pop-state" // CHECK-STATIC: "-lc" Index: lib/Driver/ToolChains/Fuchsia.cpp === --- lib/Driver/ToolChains/Fuchsia.cpp +++ lib/Driver/ToolChains/Fuchsia.cpp @@ -122,13 +122,14 @@ if (ToolChain.ShouldLinkCXXStdlib(Args)) { bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) && !Args.hasArg(options::OPT_static); +CmdArgs.push_back("--push-state"); +CmdArgs.push_back("--as-needed"); if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); -if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bdynamic"); +CmdArgs.push_back("-lm"); +CmdArgs.push_back("--pop-state"); } - CmdArgs.push_back("-lm"); } AddRunTimeLibs(ToolChain, D, CmdArgs, Args); Index: test/Driver/fuchsia.cpp === --- test/Driver/fuchsia.cpp +++ test/Driver/fuchsia.cpp @@ -15,7 +15,11 @@ // CHECK-NOT: crti.o // CHECK-NOT: crtbegin.o // CHECK: "-L[[SYSROOT]]{{/|}}lib" -// CHECK: "-lc++" "-lm" +// CHECK: "--push-state" +// CHECK: "--as-needed" +// CHECK: "-lc++" +// CHECK: "-lm" +// CHECK: "--pop-state" // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a" // CHECK: "-lc" // CHECK-NOT: crtend.o @@ -29,8 +33,10 @@ // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \ // RUN: -fuse-ld=lld 2>&1 \ // RUN: | FileCheck %s -check-prefix=CHECK-STATIC -// CHECK-STATIC: "-Bstatic" +// CHECK-STATIC: "--push-state" +// CHECK-STATIC: "--as-needed" +// CHECK-STATIC: "-static" // CHECK-STATIC: "-lc++" -// CHECK-STATIC: "-Bdynamic" // CHECK-STATIC: "-lm" +// CHECK-STATIC: "--pop-state" // CHECK-STATIC: "-lc" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
phosek updated this revision to Diff 171948. phosek marked 2 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D53854 Files: clang/lib/Driver/ToolChains/Fuchsia.cpp clang/test/Driver/fuchsia.cpp Index: clang/test/Driver/fuchsia.cpp === --- clang/test/Driver/fuchsia.cpp +++ clang/test/Driver/fuchsia.cpp @@ -15,7 +15,11 @@ // CHECK-NOT: crti.o // CHECK-NOT: crtbegin.o // CHECK: "-L[[SYSROOT]]{{/|}}lib" -// CHECK: "-lc++" "-lm" +// CHECK: "--push-state" +// CHECK: "--as-needed" +// CHECK: "-lc++" +// CHECK: "-lm" +// CHECK: "--pop-state" // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a" // CHECK: "-lc" // CHECK-NOT: crtend.o @@ -29,8 +33,10 @@ // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \ // RUN: -fuse-ld=lld 2>&1 \ // RUN: | FileCheck %s -check-prefix=CHECK-STATIC -// CHECK-STATIC: "-Bstatic" +// CHECK-STATIC: "--push-state" +// CHECK-STATIC: "--as-needed" +// CHECK-STATIC: "-static" // CHECK-STATIC: "-lc++" -// CHECK-STATIC: "-Bdynamic" // CHECK-STATIC: "-lm" +// CHECK-STATIC: "--pop-state" // CHECK-STATIC: "-lc" Index: clang/lib/Driver/ToolChains/Fuchsia.cpp === --- clang/lib/Driver/ToolChains/Fuchsia.cpp +++ clang/lib/Driver/ToolChains/Fuchsia.cpp @@ -122,13 +122,14 @@ if (ToolChain.ShouldLinkCXXStdlib(Args)) { bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) && !Args.hasArg(options::OPT_static); +CmdArgs.push_back("--push-state"); +CmdArgs.push_back("--as-needed"); if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); -if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bdynamic"); +CmdArgs.push_back("-lm"); +CmdArgs.push_back("--pop-state"); } - CmdArgs.push_back("-lm"); } AddRunTimeLibs(ToolChain, D, CmdArgs, Args); Index: clang/test/Driver/fuchsia.cpp === --- clang/test/Driver/fuchsia.cpp +++ clang/test/Driver/fuchsia.cpp @@ -15,7 +15,11 @@ // CHECK-NOT: crti.o // CHECK-NOT: crtbegin.o // CHECK: "-L[[SYSROOT]]{{/|}}lib" -// CHECK: "-lc++" "-lm" +// CHECK: "--push-state" +// CHECK: "--as-needed" +// CHECK: "-lc++" +// CHECK: "-lm" +// CHECK: "--pop-state" // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a" // CHECK: "-lc" // CHECK-NOT: crtend.o @@ -29,8 +33,10 @@ // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \ // RUN: -fuse-ld=lld 2>&1 \ // RUN: | FileCheck %s -check-prefix=CHECK-STATIC -// CHECK-STATIC: "-Bstatic" +// CHECK-STATIC: "--push-state" +// CHECK-STATIC: "--as-needed" +// CHECK-STATIC: "-static" // CHECK-STATIC: "-lc++" -// CHECK-STATIC: "-Bdynamic" // CHECK-STATIC: "-lm" +// CHECK-STATIC: "--pop-state" // CHECK-STATIC: "-lc" Index: clang/lib/Driver/ToolChains/Fuchsia.cpp === --- clang/lib/Driver/ToolChains/Fuchsia.cpp +++ clang/lib/Driver/ToolChains/Fuchsia.cpp @@ -122,13 +122,14 @@ if (ToolChain.ShouldLinkCXXStdlib(Args)) { bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) && !Args.hasArg(options::OPT_static); +CmdArgs.push_back("--push-state"); +CmdArgs.push_back("--as-needed"); if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); -if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bdynamic"); +CmdArgs.push_back("-lm"); +CmdArgs.push_back("--pop-state"); } - CmdArgs.push_back("-lm"); } AddRunTimeLibs(ToolChain, D, CmdArgs, Args); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
mcgrathr accepted this revision. mcgrathr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:125 !Args.hasArg(options::OPT_static); +CmdArgs.push_back("-push-state"); +CmdArgs.push_back("-as-needed"); I'd use the `--` version of all these GNU-compatible options since that's the GNU-compatible syntax. (But `-static` is still single-dash.) Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:132 } CmdArgs.push_back("-lm"); } `-lm` belongs inside the `--as-needed` umbrella too. Repository: rC Clang https://reviews.llvm.org/D53854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
phosek created this revision. phosek added a reviewer: mcgrathr. Herald added a reviewer: EricWF. Herald added a subscriber: cfe-commits. This avoids introducing unnecessary DT_NEEDED entries when using C++ driver for linking C code or C++ code that doesn't use C++ standard library. Repository: rC Clang https://reviews.llvm.org/D53854 Files: clang/lib/Driver/ToolChains/Fuchsia.cpp clang/test/Driver/fuchsia.cpp Index: clang/test/Driver/fuchsia.cpp === --- clang/test/Driver/fuchsia.cpp +++ clang/test/Driver/fuchsia.cpp @@ -15,7 +15,11 @@ // CHECK-NOT: crti.o // CHECK-NOT: crtbegin.o // CHECK: "-L[[SYSROOT]]{{/|}}lib" -// CHECK: "-lc++" "-lm" +// CHECK: "-push-state" +// CHECK: "-as-needed" +// CHECK: "-lc++" +// CHECK: "-pop-state" +// CHECK: "-lm" // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a" // CHECK: "-lc" // CHECK-NOT: crtend.o @@ -29,8 +33,10 @@ // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \ // RUN: -fuse-ld=lld 2>&1 \ // RUN: | FileCheck %s -check-prefix=CHECK-STATIC -// CHECK-STATIC: "-Bstatic" +// CHECK-STATIC: "-push-state" +// CHECK-STATIC: "-as-needed" +// CHECK-STATIC: "-static" // CHECK-STATIC: "-lc++" -// CHECK-STATIC: "-Bdynamic" +// CHECK-STATIC: "-pop-state" // CHECK-STATIC: "-lm" // CHECK-STATIC: "-lc" Index: clang/lib/Driver/ToolChains/Fuchsia.cpp === --- clang/lib/Driver/ToolChains/Fuchsia.cpp +++ clang/lib/Driver/ToolChains/Fuchsia.cpp @@ -122,11 +122,12 @@ if (ToolChain.ShouldLinkCXXStdlib(Args)) { bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) && !Args.hasArg(options::OPT_static); +CmdArgs.push_back("-push-state"); +CmdArgs.push_back("-as-needed"); if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); -if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bdynamic"); +CmdArgs.push_back("-pop-state"); } CmdArgs.push_back("-lm"); } Index: clang/test/Driver/fuchsia.cpp === --- clang/test/Driver/fuchsia.cpp +++ clang/test/Driver/fuchsia.cpp @@ -15,7 +15,11 @@ // CHECK-NOT: crti.o // CHECK-NOT: crtbegin.o // CHECK: "-L[[SYSROOT]]{{/|}}lib" -// CHECK: "-lc++" "-lm" +// CHECK: "-push-state" +// CHECK: "-as-needed" +// CHECK: "-lc++" +// CHECK: "-pop-state" +// CHECK: "-lm" // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a" // CHECK: "-lc" // CHECK-NOT: crtend.o @@ -29,8 +33,10 @@ // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \ // RUN: -fuse-ld=lld 2>&1 \ // RUN: | FileCheck %s -check-prefix=CHECK-STATIC -// CHECK-STATIC: "-Bstatic" +// CHECK-STATIC: "-push-state" +// CHECK-STATIC: "-as-needed" +// CHECK-STATIC: "-static" // CHECK-STATIC: "-lc++" -// CHECK-STATIC: "-Bdynamic" +// CHECK-STATIC: "-pop-state" // CHECK-STATIC: "-lm" // CHECK-STATIC: "-lc" Index: clang/lib/Driver/ToolChains/Fuchsia.cpp === --- clang/lib/Driver/ToolChains/Fuchsia.cpp +++ clang/lib/Driver/ToolChains/Fuchsia.cpp @@ -122,11 +122,12 @@ if (ToolChain.ShouldLinkCXXStdlib(Args)) { bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) && !Args.hasArg(options::OPT_static); +CmdArgs.push_back("-push-state"); +CmdArgs.push_back("-as-needed"); if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bstatic"); + CmdArgs.push_back("-static"); ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs); -if (OnlyLibstdcxxStatic) - CmdArgs.push_back("-Bdynamic"); +CmdArgs.push_back("-pop-state"); } CmdArgs.push_back("-lm"); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits