[PATCH] D59841: [Gnu Driver] Let -static-pie win if it is specified along with -pie or -static.

[Siva Chandra via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361312: Let -static-pie win if it is specified along with 
-pie or -static. (authored by sivachandra, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59841?vs=200577=200584#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59841/new/

https://reviews.llvm.org/D59841

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/linux-ld.c

Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -311,7 +311,7 @@
 
 static bool getPIE(const ArgList , const toolchains::Linux ) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
@@ -321,6 +321,26 @@
   return A->getOption().matches(options::OPT_pie);
 }
 
+static bool getStaticPIE(const ArgList ,
+ const toolchains::Linux ) {
+  bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
+  // -no-pie is an alias for -nopie. So, handling -nopie takes care of
+  // -no-pie as well.
+  if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
+const Driver  = ToolChain.getDriver();
+const llvm::opt::OptTable  = D.getOpts();
+const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
+D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
+  }
+  return HasStaticPIE;
+}
+
+static bool getStatic(const ArgList ) {
+  return Args.hasArg(options::OPT_static) &&
+  !Args.hasArg(options::OPT_static_pie);
+}
+
 void tools::gnutools::Linker::ConstructJob(Compilation , const JobAction ,
const InputInfo ,
const InputInfoList ,
@@ -336,7 +356,8 @@
   const bool isAndroid = ToolChain.getTriple().isAndroid();
   const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
   const bool IsPIE = getPIE(Args, ToolChain);
-  const bool IsStaticPIE = Args.hasArg(options::OPT_static_pie);
+  const bool IsStaticPIE = getStaticPIE(Args, ToolChain);
+  const bool IsStatic = getStatic(Args);
   const bool HasCRTBeginEndFiles =
   ToolChain.getTriple().hasEnvironment() ||
   (ToolChain.getTriple().getVendor() != llvm::Triple::MipsTechnologies);
@@ -408,7 +429,7 @@
 return;
   }
 
-  if (Args.hasArg(options::OPT_static)) {
+  if (IsStatic) {
 if (Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb ||
 Arch == llvm::Triple::thumb || Arch == llvm::Triple::thumbeb)
   CmdArgs.push_back("-Bstatic");
@@ -418,7 +439,7 @@
 CmdArgs.push_back("-shared");
   }
 
-  if (!Args.hasArg(options::OPT_static)) {
+  if (!IsStatic) {
 if (Args.hasArg(options::OPT_rdynamic))
   CmdArgs.push_back("-export-dynamic");
 
@@ -465,7 +486,7 @@
   }
   if (P.empty()) {
 const char *crtbegin;
-if (Args.hasArg(options::OPT_static))
+if (IsStatic)
   crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
 else if (Args.hasArg(options::OPT_shared))
   crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
@@ -520,7 +541,7 @@
 
   if (!Args.hasArg(options::OPT_nostdlib)) {
 if (!Args.hasArg(options::OPT_nodefaultlibs)) {
-  if (Args.hasArg(options::OPT_static) || IsStaticPIE)
+  if (IsStatic || IsStaticPIE)
 CmdArgs.push_back("--start-group");
 
   if (NeedsSanitizerDeps)
@@ -556,7 +577,7 @@
   if (IsIAMCU)
 CmdArgs.push_back("-lgloss");
 
-  if (Args.hasArg(options::OPT_static) || IsStaticPIE)
+  if (IsStatic || IsStaticPIE)
 CmdArgs.push_back("--end-group");
   else
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -458,4 +458,5 @@
   "command line to use the libc++ standard library instead">,
   InGroup>;
 
+def err_drv_cannot_mix_options : Error<"cannot specify '%1' along with '%0'">;
 }
Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -194,6 +194,39 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -static-pie -pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: 

[PATCH] D59841: [Gnu Driver] Let -static-pie win if it is specified along with -pie or -static.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra marked an inline comment as done.
sivachandra added a comment.

PTAL




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:311
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;

saugustine wrote:
> It's not clear to me that the command line -static-pie -no-pie should result 
> in static-pie, given the way the rest of that function is coded.
> 
> I might make it a ternary enum: "nothing" "pie" "static-pie" with the last 
> one winning. That method seems more consistent with current behavior.
> 
> This would allow anyone checking the state of pie to use a single function 
> and just check the result.
static-pie is now handled in its own separate function below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59841/new/

https://reviews.llvm.org/D59841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59841: [Gnu Driver] Let -static-pie win if it is specified along with -pie or -static.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 200577.
sivachandra added a comment.

Let -static-pie win if specified along with -pie or -static.

Also, treat specifying -nopie/-no-pie along with -static-pie as an error.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59841/new/

https://reviews.llvm.org/D59841

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c

Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -194,6 +194,39 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -static-pie -pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-PIE %s
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "-static"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "-pie"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
+// RUN: %clang -static-pie -static -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-STATIC %s
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "-static"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "-pie"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "--no-dynamic-linker"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
+// RUN: %clang -static-pie -nopie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
+// CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -311,7 +311,7 @@
 
 static bool getPIE(const ArgList , const toolchains::Linux ) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
@@ -321,6 +321,26 @@
   return A->getOption().matches(options::OPT_pie);
 }
 
+static bool getStaticPIE(const ArgList ,
+ const toolchains::Linux ) {
+  bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
+  // -no-pie is an alias for -nopie. So, handling -nopie takes care of
+  // -no-pie as well.
+  if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
+const Driver  = ToolChain.getDriver();
+const llvm::opt::OptTable  = D.getOpts();
+const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
+D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
+  }
+  return HasStaticPIE;
+}
+
+static bool getStatic(const ArgList ) {
+  return Args.hasArg(options::OPT_static) &&
+  !Args.hasArg(options::OPT_static_pie);
+}
+
 void tools::gnutools::Linker::ConstructJob(Compilation , const JobAction ,
const InputInfo ,
const InputInfoList ,
@@ -336,7 +356,8 @@
   const bool isAndroid = ToolChain.getTriple().isAndroid();
   const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
   const bool IsPIE = getPIE(Args, ToolChain);
-  const bool IsStaticPIE = Args.hasArg(options::OPT_static_pie);
+  const bool IsStaticPIE = getStaticPIE(Args, ToolChain);
+  const bool IsStatic = getStatic(Args);
   const bool HasCRTBeginEndFiles =
   ToolChain.getTriple().hasEnvironment() ||
   (ToolChain.getTriple().getVendor() !=