[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

llvm-svn: 316501

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 168d99093c5ff471ca9a7212b0715824da348402..804827c467712eb56d3da43742aba600357c5c0a 100644 (file)
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -818,9 +818,18 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
     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 char *> ArgsArr) {
     }
   }
 
-  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 char *> ArgsArr) {
   }
 
   // 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 char *> ArgsArr) {
                                    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 char *> ArgsArr) {
   }
 
   // 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");

diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index 8df7a0f1d6b369ed531e70483dfe60a202dd9aa4..7ad269c1b9c44ac1925b94fd00b65dca261ac982 100644 (file)
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -9,10 +9,11 @@ class F<string name> : Flag<["/", "-", "-?"], name>;
 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>;