[clang] [clang] Fix null pointer related issues in StaticAnalyzer (PR #210823)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Ian Li (ianayl)

<details>
<summary>Changes</summary>

This PR addresses issues found by Coverity in Clang's StaticAnalyzer related to potential null pointer dereferences:

- Adds null guards to variable accesses in:
  - `clang/lib/StaticAnalyzer/Core/MemRegion.cpp` in `MemRegionManager::getVarRegion`, where the newly guarded `SF` argument can potentially be null (as hinted [here](https://github.com/ianayl/llvm-project/blob/f5f0e3ca73d1e94ac16c5c634701150ba6a10b74/clang/lib/StaticAnalyzer/Core/MemRegion.cpp#L1758-L1763))
  - `clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp` in `reportBug`, where the newly guarded `V` argument can potentially be null (as hinted [here](https://github.com/ianayl/llvm-project/blob/f5f0e3ca73d1e94ac16c5c634701150ba6a10b74/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp#L332-L334))
- Initializes Clang StaticAnalyzer's `CallEvent::Data` to `nullptr` to prevent potential UB from improper access

**Note** on `clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp`: 
For the `reportBug` function, [I am using the `SourceLocation` from the `ParmVarDecl`](https://github.com/ianayl/llvm-project/blob/f5f0e3ca73d1e94ac16c5c634701150ba6a10b74/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp#L422) as fallback in the case that the associated `Expr V` is null. Please let me know if an assertion that the `Expr` must exist would be a better solution.

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


3 Files Affected:

- (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h (+1-1) 
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp (+4-2) 
- (modified) clang/lib/StaticAnalyzer/Core/MemRegion.cpp (+3-1) 


``````````diff
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 3252421414181..2010e4b0da84b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -162,7 +162,7 @@ class CallEvent {
 
 protected:
   // This is user data for subclasses.
-  const void *Data;
+  const void *Data = nullptr;
 
   // This is user data for subclasses.
   // This should come right before RefCount, so that the two fields can be
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index a624b7d686734..c005cd5ac8164 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -419,9 +419,11 @@ class RawPtrRefLocalVarsChecker
       Os << " is a ";
       printPointerTypeAndType(Os, V->getType());
 
-      PathDiagnosticLocation BSLoc(Value->getExprLoc(), BR->getSourceManager());
+      SourceLocation ExprLoc = (Value) ? Value->getExprLoc() : V->getLocation();
+      PathDiagnosticLocation BSLoc(ExprLoc, BR->getSourceManager());
       auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
-      Report->addRange(Value->getSourceRange());
+      if (Value)
+        Report->addRange(Value->getSourceRange());
       BR->emitReport(std::move(Report));
     } else {
       if (V->hasLocalStorage())
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index f3fe21101792c..4c45fa814d2f5 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1039,7 +1039,9 @@ const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D,
   const auto *PVD = dyn_cast<ParmVarDecl>(D);
   if (PVD) {
     unsigned Index = PVD->getFunctionScopeIndex();
-    const Expr *CallSite = SF->getCallSite();
+    const Expr *CallSite = nullptr;
+    if (SF)
+      CallSite = SF->getCallSite();
     if (CallSite) {
       const Decl *CalleeDecl = SF->getDecl();
       bool CurrentParam = true;

``````````

</details>


https://github.com/llvm/llvm-project/pull/210823