abeserminji created this revision.
abeserminji added reviewers: petarj, atanasyan, MatzeB, mstojanovic.
Herald added subscribers: jfb, arichardson, sdardis, srhines.
When executing test-suite tests with -static option, we get stdthreadbug test
to fail.
Sometimes it segfault, sometimes it gets stuck in an infinite loop.
The reason for such behavior is that pthread library uses weak symbols for some
functions
(ex. __pthread_mutex_lock), and such functions are not included in the
executable when
-static option is used. To force the linker to include these functions as well,
we use the following
options: //-Wl,--whole-archive -lpthread -Wl,--no-whole-archive//.
I proposed adding this options to the test-suite for the stdthreadbug test here
D52878 <https://reviews.llvm.org/D52878>,
and got a recommendation to try to teach the clang to handle this case by
always including
given options when -pthread is used.
This patch does exactly that.
My concern about this is that we are forcing whole pthread into an executable,
and user
does not have an option do decide if he wants to use lpthread or something
else. Now,
I am not sure what exactly that something else could be, and maybe there isn't
something else
and my concerns aren't justified.
Repository:
rC Clang
https://reviews.llvm.org/D56836
Files:
include/clang/Driver/Options.td
lib/Driver/ToolChains/Gnu.cpp
Index: lib/Driver/ToolChains/Gnu.cpp
===================================================================
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -501,6 +501,7 @@
bool WantPthread = Args.hasArg(options::OPT_pthread) ||
Args.hasArg(options::OPT_pthreads);
+ bool WantLPthread = Args.hasArg(options::OPT_lpthread);
// FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
// require librt. Most modern Linux platforms do, but some may not.
@@ -513,8 +514,35 @@
AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
- if (WantPthread && !isAndroid)
- CmdArgs.push_back("-lpthread");
+ if ((WantPthread || WantLPthread) && !isAndroid) {
+ if (Triple.isMIPS() && Args.hasArg(options::OPT_static)) {
+ // When -pthread option is used in combination with -static,
+ // we want to avoid problems which weak symbols may cause,
+ // and therefore we include whole lpthread library
+ bool wholeArchiveFlag = false;
+ bool lpthreadAsWholeArchive = false;
+ for (Arg *A : Args) {
+ if (A->getOption().matches(options::OPT_whole_archive))
+ wholeArchiveFlag = true;
+ if (A->getOption().matches(options::OPT_no_whole_archive))
+ wholeArchiveFlag = false;
+ if (wholeArchiveFlag) {
+ if (A->getOption().matches(options::OPT_lpthread)) {
+ lpthreadAsWholeArchive = true;
+ break;
+ }
+ }
+ }
+
+ if (!lpthreadAsWholeArchive) {
+ CmdArgs.push_back("--whole-archive");
+ CmdArgs.push_back("-lpthread");
+ CmdArgs.push_back("--no-whole-archive");
+ }
+ } else {
+ CmdArgs.push_back("-lpthread");
+ }
+ }
if (Args.hasArg(options::OPT_fsplit_stack))
CmdArgs.push_back("--wrap=pthread_create");
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2491,6 +2491,7 @@
def pthread : Flag<["-"], "pthread">, Flags<[CC1Option]>,
HelpText<"Support POSIX threads in generated code">;
def no_pthread : Flag<["-"], "no-pthread">, Flags<[CC1Option]>;
+def lpthread : Flag<["-"], "lpthread">;
def p : Flag<["-"], "p">;
def pie : Flag<["-"], "pie">;
def read__only__relocs : Separate<["-"], "read_only_relocs">;
@@ -2602,6 +2603,8 @@
def weak__library : Separate<["-"], "weak_library">, Flags<[LinkerInput]>;
def weak__reference__mismatches : Separate<["-"], "weak_reference_mismatches">;
def whatsloaded : Flag<["-"], "whatsloaded">;
+def whole_archive : Flag<["--"], "whole-archive">, Group<Link_Group>;
+def no_whole_archive : Flag<["--"], "no-whole-archive">, Group<Link_Group>;
def whyload : Flag<["-"], "whyload">;
def w : Flag<["-"], "w">, HelpText<"Suppress all warnings">,
Flags<[CC1Option]>;
def x : JoinedOrSeparate<["-"], "x">, Flags<[DriverOption,CC1Option]>,
Index: lib/Driver/ToolChains/Gnu.cpp
===================================================================
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -501,6 +501,7 @@
bool WantPthread = Args.hasArg(options::OPT_pthread) ||
Args.hasArg(options::OPT_pthreads);
+ bool WantLPthread = Args.hasArg(options::OPT_lpthread);
// FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
// require librt. Most modern Linux platforms do, but some may not.
@@ -513,8 +514,35 @@
AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
- if (WantPthread && !isAndroid)
- CmdArgs.push_back("-lpthread");
+ if ((WantPthread || WantLPthread) && !isAndroid) {
+ if (Triple.isMIPS() && Args.hasArg(options::OPT_static)) {
+ // When -pthread option is used in combination with -static,
+ // we want to avoid problems which weak symbols may cause,
+ // and therefore we include whole lpthread library
+ bool wholeArchiveFlag = false;
+ bool lpthreadAsWholeArchive = false;
+ for (Arg *A : Args) {
+ if (A->getOption().matches(options::OPT_whole_archive))
+ wholeArchiveFlag = true;
+ if (A->getOption().matches(options::OPT_no_whole_archive))
+ wholeArchiveFlag = false;
+ if (wholeArchiveFlag) {
+ if (A->getOption().matches(options::OPT_lpthread)) {
+ lpthreadAsWholeArchive = true;
+ break;
+ }
+ }
+ }
+
+ if (!lpthreadAsWholeArchive) {
+ CmdArgs.push_back("--whole-archive");
+ CmdArgs.push_back("-lpthread");
+ CmdArgs.push_back("--no-whole-archive");
+ }
+ } else {
+ CmdArgs.push_back("-lpthread");
+ }
+ }
if (Args.hasArg(options::OPT_fsplit_stack))
CmdArgs.push_back("--wrap=pthread_create");
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2491,6 +2491,7 @@
def pthread : Flag<["-"], "pthread">, Flags<[CC1Option]>,
HelpText<"Support POSIX threads in generated code">;
def no_pthread : Flag<["-"], "no-pthread">, Flags<[CC1Option]>;
+def lpthread : Flag<["-"], "lpthread">;
def p : Flag<["-"], "p">;
def pie : Flag<["-"], "pie">;
def read__only__relocs : Separate<["-"], "read_only_relocs">;
@@ -2602,6 +2603,8 @@
def weak__library : Separate<["-"], "weak_library">, Flags<[LinkerInput]>;
def weak__reference__mismatches : Separate<["-"], "weak_reference_mismatches">;
def whatsloaded : Flag<["-"], "whatsloaded">;
+def whole_archive : Flag<["--"], "whole-archive">, Group<Link_Group>;
+def no_whole_archive : Flag<["--"], "no-whole-archive">, Group<Link_Group>;
def whyload : Flag<["-"], "whyload">;
def w : Flag<["-"], "w">, HelpText<"Suppress all warnings">, Flags<[CC1Option]>;
def x : JoinedOrSeparate<["-"], "x">, Flags<[DriverOption,CC1Option]>,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
