[lld] r316501 - [COFF] Clean up boolean flag handling

Shoaib Meenai via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 24 14:17:16 PDT 2017


Author: smeenai
Date: Tue Oct 24 14:17:16 2017
New Revision: 316501

URL: http://llvm.org/viewvc/llvm-project?rev=316501&view=rev
Log:
[COFF] Clean up boolean flag handling

LLD's handling of boolean flags is suboptimal:
* All boolean flags have a corresponding `:no` flag to turn the flag
  off, and the linker should scan for both the non-suffixed and suffixed
  flags (and the last one should win), but right now it only scans for
  either the suffixed or non-suffixed flag (depending on the default
  flag value).
* The `B` multiclass only allows specifying help text for the suffixed
  (`:no`) flag, but for some flags (e.g. `/appcontainer`) the help text
  should be associated with the non-suffixed flag instead.

Extend the `B` multiclass to have help text for both non-suffixed and
suffixed flag variants, and alter the existing help text accordingly in
some cases. Scan for both the non-suffixed and suffixed variants in the
driver and set config values accordingly.

This should mostly have no behavior change, apart from the added help
text and the modified argument scanning. Some flags are handled slightly
differently now, however; for example, LLD would previously always treat
64-bit images as large address aware, whereas `/largeaddressaware:no` is
now respected for 64-bit images (which is also how link.exe behaves).

Differential Revision: https://reviews.llvm.org/D39216

Modified:
    lld/trunk/COFF/Driver.cpp
    lld/trunk/COFF/Options.td

Modified: lld/trunk/COFF/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Driver.cpp?rev=316501&r1=316500&r2=316501&view=diff
==============================================================================
--- lld/trunk/COFF/Driver.cpp (original)
+++ lld/trunk/COFF/Driver.cpp Tue Oct 24 14:17:16 2017
@@ -818,9 +818,18 @@ void LinkerDriver::link(ArrayRef<const c
     Config->ManifestID = 2;
   }
 
-  // Handle /fixed
-  if (Args.hasArg(OPT_fixed)) {
-    if (Args.hasArg(OPT_dynamicbase)) {
+  // Handle /dynamicbase and /fixed. We can't use hasFlag for /dynamicbase
+  // because we need to explicitly check whether that option or its inverse was
+  // present in the argument list in order to handle /fixed.
+  auto *DynamicBaseArg = Args.getLastArg(OPT_dynamicbase, OPT_dynamicbase_no);
+  if (DynamicBaseArg &&
+      DynamicBaseArg->getOption().getID() == OPT_dynamicbase_no)
+    Config->DynamicBase = false;
+
+  bool Fixed = Args.hasFlag(OPT_fixed, OPT_fixed_no, false);
+  if (Fixed) {
+    if (DynamicBaseArg &&
+        DynamicBaseArg->getOption().getID() == OPT_dynamicbase) {
       error("/fixed must not be specified with /dynamicbase");
     } else {
       Config->Relocatable = false;
@@ -828,8 +837,9 @@ void LinkerDriver::link(ArrayRef<const c
     }
   }
 
-  if (Args.hasArg(OPT_appcontainer))
-    Config->AppContainer = true;
+  // Handle /appcontainer
+  Config->AppContainer =
+      Args.hasFlag(OPT_appcontainer, OPT_appcontainer_no, false);
 
   // Handle /machine
   if (auto *Arg = Args.getLastArg(OPT_machine))
@@ -984,16 +994,11 @@ void LinkerDriver::link(ArrayRef<const c
   }
 
   // Handle miscellaneous boolean flags.
-  if (Args.hasArg(OPT_allowbind_no))
-    Config->AllowBind = false;
-  if (Args.hasArg(OPT_allowisolation_no))
-    Config->AllowIsolation = false;
-  if (Args.hasArg(OPT_dynamicbase_no))
-    Config->DynamicBase = false;
-  if (Args.hasArg(OPT_nxcompat_no))
-    Config->NxCompat = false;
-  if (Args.hasArg(OPT_tsaware_no))
-    Config->TerminalServerAware = false;
+  Config->AllowBind = Args.hasFlag(OPT_allowbind, OPT_allowbind_no, true);
+  Config->AllowIsolation =
+      Args.hasFlag(OPT_allowisolation, OPT_allowisolation_no, true);
+  Config->NxCompat = Args.hasFlag(OPT_nxcompat, OPT_nxcompat_no, true);
+  Config->TerminalServerAware = Args.hasFlag(OPT_tsaware, OPT_tsaware_no, true);
   if (Args.hasArg(OPT_nosymtab))
     Config->WriteSymtab = false;
 
@@ -1048,12 +1053,13 @@ void LinkerDriver::link(ArrayRef<const c
                                    ArrayRef<StringRef>(SearchPaths).slice(1)));
 
   // Handle /largeaddressaware
-  if (Config->is64() || Args.hasArg(OPT_largeaddressaware))
-    Config->LargeAddressAware = true;
+  Config->LargeAddressAware = Args.hasFlag(
+      OPT_largeaddressaware, OPT_largeaddressaware_no, Config->is64());
 
   // Handle /highentropyva
-  if (Config->is64() && !Args.hasArg(OPT_highentropyva_no))
-    Config->HighEntropyVA = true;
+  Config->HighEntropyVA =
+      Config->is64() &&
+      Args.hasFlag(OPT_highentropyva, OPT_highentropyva_no, true);
 
   // Handle /entry and /dll
   if (auto *Arg = Args.getLastArg(OPT_entry)) {
@@ -1208,7 +1214,7 @@ void LinkerDriver::link(ArrayRef<const c
   }
 
   // Handle /safeseh.
-  if (Args.hasArg(OPT_safeseh)) {
+  if (Args.hasFlag(OPT_safeseh, OPT_safeseh_no, false)) {
     for (ObjFile *File : ObjFile::Instances)
       if (!File->SEHCompat)
         error("/safeseh: " + File->getName() + " is not compatible with SEH");

Modified: lld/trunk/COFF/Options.td
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Options.td?rev=316501&r1=316500&r2=316501&view=diff
==============================================================================
--- lld/trunk/COFF/Options.td (original)
+++ lld/trunk/COFF/Options.td Tue Oct 24 14:17:16 2017
@@ -9,10 +9,11 @@ class F<string name> : Flag<["/", "-", "
 class P<string name, string help> :
       Joined<["/", "-", "-?"], name#":">, HelpText<help>;
 
-// Boolean flag suffixed by ":no".
-multiclass B<string name, string help> {
-  def "" : F<name>;
-  def _no : F<name#":no">, HelpText<help>;
+// Boolean flag which can be suffixed by ":no". Using it unsuffixed turns the
+// flag on and using it suffixed by ":no" turns it off.
+multiclass B<string name, string help_on, string help_off> {
+  def "" : F<name>, HelpText<help_on>;
+  def _no : F<name#":no">, HelpText<help_off>;
 }
 
 def align   : P<"align", "Section alignment">;
@@ -85,18 +86,31 @@ def force : F<"force">,
     HelpText<"Allow undefined symbols when creating executables">;
 def force_unresolved : F<"force:unresolved">;
 
-defm allowbind: B<"allowbind", "Disable DLL binding">;
-defm allowisolation : B<"allowisolation", "Set NO_ISOLATION bit">;
+defm allowbind : B<"allowbind", "Enable DLL binding (default)",
+                   "Disable DLL binding">;
+defm allowisolation : B<"allowisolation", "Enable DLL isolation (default)",
+                        "Disable DLL isolation">;
 defm appcontainer : B<"appcontainer",
-                      "Image can only be run in an app container">;
-defm dynamicbase : B<"dynamicbase",
-                     "Disable address space layout randomization">;
-defm fixed    : B<"fixed", "Enable base relocations">;
-defm highentropyva : B<"highentropyva", "Set HIGH_ENTROPY_VA bit">;
-defm largeaddressaware : B<"largeaddressaware", "Disable large addresses">;
-defm nxcompat : B<"nxcompat", "Disable data execution provention">;
-defm safeseh : B<"safeseh", "Produce an image with Safe Exception Handler">;
-defm tsaware  : B<"tsaware", "Create non-Terminal Server aware executable">;
+                      "Image can only be run in an app container",
+                      "Image can run outside an app container (default)">;
+defm dynamicbase : B<"dynamicbase", "Enable ASLR (default unless /fixed)",
+                     "Disable ASLR (default when /fixed)">;
+defm fixed : B<"fixed", "Disable base relocations",
+               "Enable base relocations (default)">;
+defm highentropyva : B<"highentropyva",
+                       "Enable 64-bit ASLR (default on 64-bit)",
+                       "Disable 64-bit ASLR">;
+defm largeaddressaware : B<"largeaddressaware",
+                           "Enable large addresses (default on 64-bit)",
+                           "Disable large addresses (default on 32-bit)">;
+defm nxcompat : B<"nxcompat", "Enable data execution prevention (default)",
+                  "Disable data execution provention">;
+defm safeseh : B<"safeseh",
+                 "Produce an image with Safe Exception Handler (only for x86)",
+                 "Don't produce an image with Safe Exception Handler">;
+defm tsaware  : B<"tsaware",
+                  "Create Terminal Server aware executable (default)",
+                  "Create non-Terminal Server aware executable">;
 
 def help : F<"help">;
 def help_q : Flag<["/?", "-?"], "">, Alias<help>;




More information about the llvm-commits mailing list