[CHERI_CSA] ProvenanceSource: suppress with -Wno-cheri-provenance

If [-Wcheri-provenance] compiler warning is disabled, do not emit ambiguous provenance warning for cases when the default choice of LHS as a source of provenance will probably be fine. Binary operations with provenance-carrying RHS are reported anyway, as this may indicate a real bug.
rems-project · Nov 17, 2023 · 13fe607 · 13fe607
1 parent f309723
commit 13fe607
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 5 deletions.
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1694,7 +1694,14 @@ def ProvenanceSourceChecker : Checker<"ProvenanceSource">,
                   "ShowFixIts",
                   "Enable fix-it hints for this checker",
                   "false",
-                  InAlpha>
+                  InAlpha>,
+    CmdLineOption<Boolean,
+                  "ReportForAmbiguousProvenance",
+                  "Report for binary operations with ambiguous provenance "
+                  "for which the default capability derivation from LHS is fine. "
+                  "Disabled if [-Wcheri-provenance] is disabled.",
+                  "true",
+                  Released>
   ]>,
   Documentation<NotDocumented>;
 

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -24,6 +24,7 @@
 #include "PS4CPU.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include <clang/Basic/DiagnosticSema.h>
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
@@ -3057,7 +3058,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
 
 static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
                                   const llvm::Triple &Triple,
-                                  const InputInfo &Input) {
+                                  const InputInfo &Input,
+                                  DiagnosticsEngine &Diags) {
   // Enable region store model by default.
   CmdArgs.push_back("-analyzer-store=region");
 
@@ -3116,6 +3118,16 @@ static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
         (Triple.isRISCV() && tools::riscv::isCheriPurecap(Args, Triple)) ||
         (Triple.isAArch64() && tools::aarch64::isPurecap(Args, Triple))) {
       CmdArgs.push_back("-analyzer-checker=cheri");
+
+      // disable AmbiguousProvenance war if [-Wcheri-provenance] is disabled
+      if (Diags.getDiagnosticLevel(
+              diag::warn_ambiguous_provenance_capability_binop,
+              SourceLocation()) < DiagnosticsEngine::Warning) {
+        CmdArgs.push_back("-analyzer-config");
+        CmdArgs.push_back(
+            "cheri.ProvenanceSource:ReportForAmbiguousProvenance=false");
+      }
+
       CmdArgs.push_back("-analyzer-checker=optin.portability.PointerAlignment");
       CmdArgs.push_back("-analyzer-checker=alpha.core.PointerSub");
     }
@@ -4758,7 +4770,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back("-DUNICODE");
 
   if (isa<AnalyzeJobAction>(JA))
-    RenderAnalyzerOptions(Args, CmdArgs, Triple, Input);
+    RenderAnalyzerOptions(Args, CmdArgs, Triple, Input, D.getDiags());
 
   if (isa<AnalyzeJobAction>(JA) ||
       (isa<PreprocessJobAction>(JA) && Args.hasArg(options::OPT__analyze)))

diff --git a/clang/lib/StaticAnalyzer/Checkers/CHERI/ProvenanceSourceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CHERI/ProvenanceSourceChecker.cpp
@@ -13,10 +13,11 @@
 
 #include "CHERIUtils.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
-#include <clang/StaticAnalyzer/Core/BugReporter/BugReporter.h>
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include <clang/Basic/DiagnosticSema.h>
+#include <clang/StaticAnalyzer/Core/BugReporter/BugReporter.h>
 
 using namespace clang;
 using namespace ento;
@@ -81,6 +82,7 @@ class ProvenanceSourceChecker : public Checker<check::PostStmt<CastExpr>,
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 
   bool ShowFixIts = false;
+  bool ReportForAmbiguousProvenance = true;
 
 private:
   // Utility
@@ -379,8 +381,8 @@ ExplodedNode *ProvenanceSourceChecker::emitAmbiguousProvenanceWarn(
     const BinaryOperator *BO, CheckerContext &C,
     bool LHSIsAddr, bool LHSIsNullDerived,
     bool RHSIsAddr, bool RHSIsNullDerived) const {
-  bool const IsUnsigned = BO->getType()->isUnsignedIntegerType();
 
+  bool const IsUnsigned = BO->getType()->isUnsignedIntegerType();
   SmallString<350> ErrorMessage;
   llvm::raw_svector_ostream OS(ErrorMessage);
 
@@ -485,6 +487,14 @@ void ProvenanceSourceChecker::checkPostStmt(const BinaryOperator *BO,
   ExplodedNode *N;
   bool InvalidCap;
   if (LHSActiveProv && RHSActiveProv && !IsSub) {
+    if (!RHSIsAddr && !this->ReportForAmbiguousProvenance) {
+        // No strong evidence of a real bug here. This code will probably
+        // be fine with the default choice of LHS for capability derivation.
+        // Don't report if [-Wcheri-provenance] compiler warning is disabled
+        // and don't propagate ambiguous provenance flag further
+        return;
+    }
+
     N = emitAmbiguousProvenanceWarn(BO, C, LHSIsAddr, LHSIsNullDerived,
                                       RHSIsAddr, RHSIsNullDerived);
     if (!N)
@@ -649,6 +659,9 @@ void ento::registerProvenanceSourceChecker(CheckerManager &mgr) {
   auto *Checker = mgr.registerChecker<ProvenanceSourceChecker>();
   Checker->ShowFixIts = mgr.getAnalyzerOptions().getCheckerBooleanOption(
       Checker, "ShowFixIts");
+  Checker->ReportForAmbiguousProvenance =
+      mgr.getAnalyzerOptions().getCheckerBooleanOption(
+          Checker, "ReportForAmbiguousProvenance");
 }
 
 bool ento::shouldRegisterProvenanceSourceChecker(const CheckerManager &Mgr) {