[ELF] Introduce ReportPolicy to handle -z *-report options. NFC

[MaskRay]

Use an enum to replace string comparison.

[llvmbot]

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Fangrui Song (MaskRay)

Changes

Use an enum to replace string comparison.

---

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

3 Files Affected:

• (modified) lld/ELF/Config.h (+16-5)
• (modified) lld/ELF/Driver.cpp (+18-22)
• (modified) lld/ELF/Writer.cpp (+2-6)

diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index b5872b85efd3a..d14faca31d58f 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -136,6 +136,9 @@ enum LtoKind : uint8_t {UnifiedThin, UnifiedRegular, Default};
 // For -z gcs=
 enum class GcsPolicy { Implicit, Never, Always };
 
+// For some options that resemble -z bti-report={none,warning,error}
+enum class ReportPolicy { None, Warning, Error };
+
 struct SymbolVersion {
   llvm::StringRef name;
   bool isExternCpp;
@@ -225,11 +228,11 @@ struct Config {
   llvm::StringRef whyExtract;
   llvm::StringRef cmseInputLib;
   llvm::StringRef cmseOutputLib;
-  StringRef zBtiReport = "none";
-  StringRef zCetReport = "none";
-  StringRef zPauthReport = "none";
-  StringRef zGcsReport = "none";
-  StringRef zExecuteOnlyReport = "none";
+  ReportPolicy zBtiReport = ReportPolicy::None;
+  ReportPolicy zCetReport = ReportPolicy::None;
+  ReportPolicy zPauthReport = ReportPolicy::None;
+  ReportPolicy zGcsReport = ReportPolicy::None;
+  ReportPolicy zExecuteOnlyReport = ReportPolicy::None;
   bool ltoBBAddrMap;
   llvm::StringRef ltoBasicBlockSections;
   std::pair<llvm::StringRef, llvm::StringRef> thinLTOObjectSuffixReplace;
@@ -748,6 +751,14 @@ ELFSyncStream InternalErr(Ctx &ctx, const uint8_t *buf);
 
 #define CHECK2(E, S) lld::check2((E), [&] { return toStr(ctx, S); })
 
+inline DiagLevel toDiagLevel(ReportPolicy policy) {
+  if (policy == ReportPolicy::Error)
+    return DiagLevel::Err;
+  else if (policy == ReportPolicy::Warning)
+    return DiagLevel::Warn;
+  return DiagLevel::None;
+}
+
 } // namespace lld::elf
 
 #endif
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index c5de522daa177..18f9fed0d08e2 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -396,18 +396,18 @@ static void checkOptions(Ctx &ctx) {
       ErrAlways(ctx) << "-z pac-plt only supported on AArch64";
     if (ctx.arg.zForceBti)
       ErrAlways(ctx) << "-z force-bti only supported on AArch64";
-    if (ctx.arg.zBtiReport != "none")
+    if (ctx.arg.zBtiReport != ReportPolicy::None)
       ErrAlways(ctx) << "-z bti-report only supported on AArch64";
-    if (ctx.arg.zPauthReport != "none")
+    if (ctx.arg.zPauthReport != ReportPolicy::None)
       ErrAlways(ctx) << "-z pauth-report only supported on AArch64";
-    if (ctx.arg.zGcsReport != "none")
+    if (ctx.arg.zGcsReport != ReportPolicy::None)
       ErrAlways(ctx) << "-z gcs-report only supported on AArch64";
     if (ctx.arg.zGcs != GcsPolicy::Implicit)
       ErrAlways(ctx) << "-z gcs only supported on AArch64";
   }
 
   if (ctx.arg.emachine != EM_AARCH64 && ctx.arg.emachine != EM_ARM &&
-      ctx.arg.zExecuteOnlyReport != "none")
+      ctx.arg.zExecuteOnlyReport != ReportPolicy::None)
     ErrAlways(ctx)
         << "-z execute-only-report only supported on AArch64 and ARM";
 
@@ -423,7 +423,7 @@ static void checkOptions(Ctx &ctx) {
     ErrAlways(ctx) << "--relax-gp is only supported on RISC-V targets";
 
   if (ctx.arg.emachine != EM_386 && ctx.arg.emachine != EM_X86_64 &&
-      ctx.arg.zCetReport != "none")
+      ctx.arg.zCetReport != ReportPolicy::None)
     ErrAlways(ctx) << "-z cet-report only supported on X86 and X86_64";
 
   if (ctx.arg.pie && ctx.arg.shared)
@@ -1272,11 +1272,6 @@ static void parseClangOption(Ctx &ctx, StringRef opt, const Twine &msg) {
   ErrAlways(ctx) << msg << ": " << StringRef(err).trim();
 }
 
-// Checks the parameter of the bti-report and cet-report options.
-static bool isValidReportString(StringRef arg) {
-  return arg == "none" || arg == "warning" || arg == "error";
-}
-
 // Process a remap pattern 'from-glob=to-file'.
 static bool remapInputs(Ctx &ctx, StringRef line, const Twine &location) {
   SmallVector<StringRef, 0> fields;
@@ -1638,12 +1633,17 @@ static void readConfigs(Ctx &ctx, opt::InputArgList &args) {
       if (option.first != reportArg.first)
         continue;
       arg->claim();
-      if (!isValidReportString(option.second)) {
+      if (option.second == "none")
+        *reportArg.second = ReportPolicy::None;
+      else if (option.second == "warning")
+        *reportArg.second = ReportPolicy::Warning;
+      else if (option.second == "error")
+        *reportArg.second = ReportPolicy::Error;
+      else {
         ErrAlways(ctx) << "unknown -z " << reportArg.first
                        << "= value: " << option.second;
         continue;
       }
-      *reportArg.second = option.second;
     }
   }
 
@@ -2820,17 +2820,13 @@ static void readSecurityNotes(Ctx &ctx) {
   bool hasValidPauthAbiCoreInfo = llvm::any_of(
       ctx.aarch64PauthAbiCoreInfo, [](uint8_t c) { return c != 0; });
 
-  auto report = [&](StringRef config) -> ELFSyncStream {
-    if (config == "error")
-      return {ctx, DiagLevel::Err};
-    else if (config == "warning")
-      return {ctx, DiagLevel::Warn};
-    return {ctx, DiagLevel::None};
+  auto report = [&](ReportPolicy policy) -> ELFSyncStream {
+    return {ctx, toDiagLevel(policy)};
   };
-  auto reportUnless = [&](StringRef config, bool cond) -> ELFSyncStream {
+  auto reportUnless = [&](ReportPolicy policy, bool cond) -> ELFSyncStream {
     if (cond)
       return {ctx, DiagLevel::None};
-    return report(config);
+    return {ctx, toDiagLevel(policy)};
   };
   for (ELFFileBase *f : ctx.objectFiles) {
     uint32_t features = f->andFeatures;
@@ -2860,13 +2856,13 @@ static void readSecurityNotes(Ctx &ctx) {
 
     if (ctx.arg.zForceBti && !(features & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)) {
       features |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
-      if (ctx.arg.zBtiReport == "none")
+      if (ctx.arg.zBtiReport == ReportPolicy::None)
         Warn(ctx) << f
                   << ": -z force-bti: file does not have "
                      "GNU_PROPERTY_AARCH64_FEATURE_1_BTI property";
     } else if (ctx.arg.zForceIbt &&
                !(features & GNU_PROPERTY_X86_FEATURE_1_IBT)) {
-      if (ctx.arg.zCetReport == "none")
+      if (ctx.arg.zCetReport == ReportPolicy::None)
         Warn(ctx) << f
                   << ": -z force-ibt: file does not have "
                      "GNU_PROPERTY_X86_FEATURE_1_IBT property";
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index f249bb198e98d..2cea6a44b391a 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2181,17 +2181,13 @@ template <class ELFT> void Writer<ELFT>::checkExecuteOnly() {
 // Check which input sections of RX output sections don't have the
 // SHF_AARCH64_PURECODE or SHF_ARM_PURECODE flag set.
 template <class ELFT> void Writer<ELFT>::checkExecuteOnlyReport() {
-  if (ctx.arg.zExecuteOnlyReport == "none")
+  if (ctx.arg.zExecuteOnlyReport == ReportPolicy::None)
     return;
 
   auto reportUnless = [&](bool cond) -> ELFSyncStream {
     if (cond)
       return {ctx, DiagLevel::None};
-    if (ctx.arg.zExecuteOnlyReport == "error")
-      return {ctx, DiagLevel::Err};
-    if (ctx.arg.zExecuteOnlyReport == "warning")
-      return {ctx, DiagLevel::Warn};
-    return {ctx, DiagLevel::None};
+    return {ctx, toDiagLevel(ctx.arg.zExecuteOnlyReport)};
   };
 
   uint64_t purecodeFlag =

[smithp35]

LGTM. Looks like a good clean up.

[Stylie777]

LGTM