Skip to content

Revert "[clang][analyzer] Fix a nullptr dereference when -ftime-trace is used"#139936

Merged
qinkunbao merged 1 commit intollvm:mainfrom
fangyi-zhou:revert-139820-fix-time-trace-crash
May 14, 2025
Merged

Revert "[clang][analyzer] Fix a nullptr dereference when -ftime-trace is used"#139936
qinkunbao merged 1 commit intollvm:mainfrom
fangyi-zhou:revert-139820-fix-time-trace-crash

Conversation

@fangyi-zhou
Copy link
Contributor

@fangyi-zhou fangyi-zhou commented May 14, 2025

Reverts #139820

Reverting due to buildbot failures in asan

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels May 14, 2025
@llvmbot
Copy link
Member

llvmbot commented May 14, 2025

@llvm/pr-subscribers-clang

Author: Fangyi Zhou (fangyi-zhou)

Changes

Reverts llvm/llvm-project#139820

Reverting due to buildbot failures in asan

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

2 Files Affected:

  • (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h (+1-4)
  • (removed) clang/test/Analysis/ftime-trace-no-init.cpp (-5)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 2e06e71f7be5f..9e7c98fdded17 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -103,10 +103,7 @@ class SymbolConjured : public SymbolData {
   const Stmt *getStmt() const {
     switch (Elem->getKind()) {
     case CFGElement::Initializer:
-      if (const auto *Init = Elem->castAs<CFGInitializer>().getInitializer()) {
-        return Init->getInit();
-      }
-      return nullptr;
+      return Elem->castAs<CFGInitializer>().getInitializer()->getInit();
     case CFGElement::ScopeBegin:
       return Elem->castAs<CFGScopeBegin>().getTriggerStmt();
     case CFGElement::ScopeEnd:
diff --git a/clang/test/Analysis/ftime-trace-no-init.cpp b/clang/test/Analysis/ftime-trace-no-init.cpp
deleted file mode 100644
index 7fb289b19da78..0000000000000
--- a/clang/test/Analysis/ftime-trace-no-init.cpp
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling %s -ftime-trace=%t.raw.json -verify
-// expected-no-diagnostics
-
-// GitHub issue 139779
-struct {} a; // no-crash

@llvmbot
Copy link
Member

llvmbot commented May 14, 2025

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

Author: Fangyi Zhou (fangyi-zhou)

Changes

Reverts llvm/llvm-project#139820

Reverting due to buildbot failures in asan

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

2 Files Affected:

  • (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h (+1-4)
  • (removed) clang/test/Analysis/ftime-trace-no-init.cpp (-5)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 2e06e71f7be5f..9e7c98fdded17 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -103,10 +103,7 @@ class SymbolConjured : public SymbolData {
   const Stmt *getStmt() const {
     switch (Elem->getKind()) {
     case CFGElement::Initializer:
-      if (const auto *Init = Elem->castAs<CFGInitializer>().getInitializer()) {
-        return Init->getInit();
-      }
-      return nullptr;
+      return Elem->castAs<CFGInitializer>().getInitializer()->getInit();
     case CFGElement::ScopeBegin:
       return Elem->castAs<CFGScopeBegin>().getTriggerStmt();
     case CFGElement::ScopeEnd:
diff --git a/clang/test/Analysis/ftime-trace-no-init.cpp b/clang/test/Analysis/ftime-trace-no-init.cpp
deleted file mode 100644
index 7fb289b19da78..0000000000000
--- a/clang/test/Analysis/ftime-trace-no-init.cpp
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling %s -ftime-trace=%t.raw.json -verify
-// expected-no-diagnostics
-
-// GitHub issue 139779
-struct {} a; // no-crash

@qinkunbao qinkunbao merged commit baf2cfa into llvm:main May 14, 2025
9 of 13 checks passed
@fangyi-zhou fangyi-zhou mentioned this pull request May 14, 2025
Merged
@steakhal steakhal added the skip-precommit-approval PR for CI feedback, not intended for review label May 15, 2025
steakhal pushed a commit that referenced this pull request May 15, 2025
@fangyi-zhou
[clang][analyzer] Fix a nullptr dereference when -ftime-trace is used…
81d48e0 
… (Reland) (#139980)

Fixes #139779.

The bug was introduced in #137355 in `SymbolConjured::getStmt`, when
trying to obtain a statement for a CFG initializer without an
initializer.  This commit adds a null check before access.

Previous PR #139820, Revert #139936

Additional notes since previous PR:

When conjuring a symbol, sometimes there is no valid CFG element, e.g.
in the file causing the crash, there is no element at all in the CFG. In
these cases, the CFG element reference in the expression engine will be
invalid. As a consequence, there needs to be extra checks to ensure the
validity of the CFG element reference.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

clang:static analyzer clang Clang issues not falling into any other category skip-precommit-approval PR for CI feedback, not intended for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@fangyi-zhou @llvmbot @steakhal @qinkunbao