[sanitizer] Refactor -f(no-)?sanitize-recover parsing

[thurstond]

This moves the -f(no-)?sanitize-recover parsing into a generic parseSanitizerArgs function, and then applies it to parse -f(no-)?sanitize-recover and -f(no-)?sanitize-trap.

N.B. parseSanitizeTrapArgs does not remove non-TrappingSupported arguments. This maintains the legacy behavior of '-fsanitize=undefined -fsanitize-trap=undefined' (clang/test/Driver/fsanitize.c), which is that vptr is not enabled at all (not even in recover mode) in order to avoid the need for a ubsan runtime.

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Thurston Dang (thurstond)

Changes

This moves the functionality into a generic parseSanitizerArgs function, and then uses it for parsing both -f(no-)?sanitize-recover and -f(no-)?sanitize-trap.

Note: for backwards compatibility, we compute the sanitizer mask as:
Default + AlwaysIn + Arguments - AlwaysOut
instead of:
Default + Arguments + AlwaysIn - AlwaysOut

---

Full diff: https://github.com/llvm/llvm-project/pull/119819.diff

1 Files Affected:

• (modified) clang/lib/Driver/SanitizerArgs.cpp (+56-64)

diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 355dea5fad80bf..3e881fdf03ed7d 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -247,48 +247,72 @@ static SanitizerMask setGroupBits(SanitizerMask Kinds) {
   return Kinds;
 }
 
-// Computes the sanitizer mask based on the default plus opt-in (if supported)
-// minus opt-out.
+// Computes the sanitizer mask as:
+//     Default + AlwaysIn + Arguments - AlwaysOut
+// with arguments parsed from left to right.
+//
+// Error messages are optionally printed if the AlwaysIn or AlwaysOut
+// invariants are violated.
 static SanitizerMask
 parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
-                  bool DiagnoseErrors, SanitizerMask Supported,
-                  SanitizerMask Default, int OptInID, int OptOutID) {
-  SanitizerMask Remove; // During the loop below, the accumulated set of
-                        // sanitizers disabled by the current sanitizer
-                        // argument or any argument after it.
-  SanitizerMask Kinds;
-  SanitizerMask SupportedWithGroups = setGroupBits(Supported);
-
-  for (const llvm::opt::Arg *Arg : llvm::reverse(Args)) {
+                  bool DiagnoseErrors, SanitizerMask Default,
+                  SanitizerMask AlwaysIn, SanitizerMask AlwaysOut, int OptInID,
+                  int OptOutID) {
+
+  SanitizerMask Output = Default | AlwaysIn;
+  // Keep track of which violations we have already reported, to avoid
+  // duplicate error messages.
+  SanitizerMask DiagnosedAlwaysInViolations;
+  SanitizerMask DiagnosedAlwaysOutViolations;
+  for (const auto *Arg : Args) {
     if (Arg->getOption().matches(OptInID)) {
-      Arg->claim();
-      SanitizerMask Add = parseArgValues(D, Arg, true);
-      Add &= ~Remove;
-      SanitizerMask InvalidValues = Add & ~SupportedWithGroups;
-      if (InvalidValues && DiagnoseErrors) {
-        SanitizerSet S;
-        S.Mask = InvalidValues;
-        D.Diag(diag::err_drv_unsupported_option_argument)
-            << Arg->getSpelling() << toString(S);
+      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors);
+      // Report error if user explicitly tries to opt-in to an always-out
+      // sanitizer.
+      if (SanitizerMask KindsToDiagnose =
+              Add & AlwaysOut & ~DiagnosedAlwaysOutViolations) {
+        if (DiagnoseErrors) {
+          SanitizerSet SetToDiagnose;
+          SetToDiagnose.Mask |= KindsToDiagnose;
+          D.Diag(diag::err_drv_unsupported_option_argument)
+              << Arg->getSpelling() << toString(SetToDiagnose);
+          DiagnosedAlwaysOutViolations |= KindsToDiagnose;
+        }
       }
-      Kinds |= expandSanitizerGroups(Add) & ~Remove;
+      Output |= expandSanitizerGroups(Add);
+      Arg->claim();
     } else if (Arg->getOption().matches(OptOutID)) {
+      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors);
+      // Report error if user explicitly tries to opt-out of an always-in
+      // sanitizer.
+      if (SanitizerMask KindsToDiagnose =
+              Remove & AlwaysIn & ~DiagnosedAlwaysInViolations) {
+        if (DiagnoseErrors) {
+          SanitizerSet SetToDiagnose;
+          SetToDiagnose.Mask |= KindsToDiagnose;
+          D.Diag(diag::err_drv_unsupported_option_argument)
+              << Arg->getSpelling() << toString(SetToDiagnose);
+          DiagnosedAlwaysInViolations |= KindsToDiagnose;
+        }
+      }
+      Output &= ~expandSanitizerGroups(Remove);
       Arg->claim();
-      Remove |= expandSanitizerGroups(parseArgValues(D, Arg, DiagnoseErrors));
     }
   }
 
-  // Apply default behavior.
-  Kinds |= Default & ~Remove;
+  Output &= ~AlwaysOut;
 
-  return Kinds;
+  return Output;
 }
 
 static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
                                            const llvm::opt::ArgList &Args,
                                            bool DiagnoseErrors) {
-  return parseSanitizeArgs(D, Args, DiagnoseErrors, TrappingSupported,
-                           TrappingDefault, options::OPT_fsanitize_trap_EQ,
+  SanitizerMask AlwaysTrap; // Empty
+  SanitizerMask NeverTrap = ~(setGroupBits(TrappingSupported));
+
+  return parseSanitizeArgs(D, Args, DiagnoseErrors, TrappingDefault, AlwaysTrap,
+                           NeverTrap, options::OPT_fsanitize_trap_EQ,
                            options::OPT_fno_sanitize_trap_EQ);
 }
 
@@ -652,44 +676,12 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   // default in ASan?
 
   // Parse -f(no-)?sanitize-recover flags.
-  SanitizerMask RecoverableKinds = RecoverableByDefault | AlwaysRecoverable;
-  SanitizerMask DiagnosedUnrecoverableKinds;
-  SanitizerMask DiagnosedAlwaysRecoverableKinds;
-  for (const auto *Arg : Args) {
-    if (Arg->getOption().matches(options::OPT_fsanitize_recover_EQ)) {
-      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors);
-      // Report error if user explicitly tries to recover from unrecoverable
-      // sanitizer.
-      if (SanitizerMask KindsToDiagnose =
-              Add & Unrecoverable & ~DiagnosedUnrecoverableKinds) {
-        SanitizerSet SetToDiagnose;
-        SetToDiagnose.Mask |= KindsToDiagnose;
-        if (DiagnoseErrors)
-          D.Diag(diag::err_drv_unsupported_option_argument)
-              << Arg->getSpelling() << toString(SetToDiagnose);
-        DiagnosedUnrecoverableKinds |= KindsToDiagnose;
-      }
-      RecoverableKinds |= expandSanitizerGroups(Add);
-      Arg->claim();
-    } else if (Arg->getOption().matches(options::OPT_fno_sanitize_recover_EQ)) {
-      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors);
-      // Report error if user explicitly tries to disable recovery from
-      // always recoverable sanitizer.
-      if (SanitizerMask KindsToDiagnose =
-              Remove & AlwaysRecoverable & ~DiagnosedAlwaysRecoverableKinds) {
-        SanitizerSet SetToDiagnose;
-        SetToDiagnose.Mask |= KindsToDiagnose;
-        if (DiagnoseErrors)
-          D.Diag(diag::err_drv_unsupported_option_argument)
-              << Arg->getSpelling() << toString(SetToDiagnose);
-        DiagnosedAlwaysRecoverableKinds |= KindsToDiagnose;
-      }
-      RecoverableKinds &= ~expandSanitizerGroups(Remove);
-      Arg->claim();
-    }
-  }
+  SanitizerMask RecoverableKinds = parseSanitizeArgs(
+      D, Args, DiagnoseErrors, RecoverableByDefault, AlwaysRecoverable,
+      Unrecoverable, options::OPT_fsanitize_recover_EQ,
+      options::OPT_fno_sanitize_recover_EQ);
+
   RecoverableKinds &= Kinds;
-  RecoverableKinds &= ~Unrecoverable;
 
   TrappingKinds &= Kinds;
   RecoverableKinds &= ~TrappingKinds;