[Analysis] Prevent revisiting block when searching for noreturn vars

[spavloff]

When searching for noreturn variable initializations, do not visit CFG blocks that are already visited, it prevents hanging the analysis.

It must fix #150336.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Serge Pavlov (spavloff)

Changes

When searching for noreturn variable initializations, do not visit CFG blocks that are already visited, it prevents hanging the analysis.

It must fix Ihttps://github.com//issues/150336.

---

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

2 Files Affected:

• (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+4)
• (modified) clang/test/SemaCXX/noreturn-vars.cpp (+17)

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 829c81bab16f5..35ad0b59ad0fc 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -503,8 +503,12 @@ static bool areAllValuesNoReturn(const VarDecl *VD, const CFGBlock &VarBlk,
 
   TransferFunctions TF(VD);
   BackwardDataflowWorklist Worklist(*AC.getCFG(), AC);
+  llvm::DenseSet<const CFGBlock *> Visited;
   Worklist.enqueueBlock(&VarBlk);
   while (const CFGBlock *B = Worklist.dequeue()) {
+    if (Visited.contains(B))
+      continue;
+    Visited.insert(B);
     // First check the current block.
     for (CFGBlock::const_reverse_iterator ri = B->rbegin(), re = B->rend();
          ri != re; ++ri) {
diff --git a/clang/test/SemaCXX/noreturn-vars.cpp b/clang/test/SemaCXX/noreturn-vars.cpp
index ca65fcf5ca31d..1bf074149f04c 100644
--- a/clang/test/SemaCXX/noreturn-vars.cpp
+++ b/clang/test/SemaCXX/noreturn-vars.cpp
@@ -225,3 +225,20 @@ extern void abc_02(func_type *);
   abc_02(&func_ptr);
   func_ptr();
 } // expected-warning {{function declared 'noreturn' should not return}}
+
+namespace Issue150336 {
+void free(void *);
+typedef void (*sel_freefunc)(void *);
+struct gmx_ana_selmethod_t {
+  sel_freefunc free;
+  int nparams;
+  int *param;
+};
+void gmx_selelem_free_method(struct gmx_ana_selmethod_t* method, void* mdata) {
+    sel_freefunc free_func = 0;
+    for (int i = 0; i < method->nparams; ++i)
+        free(&method->param[i]);
+    if (mdata && free_func)
+        free_func(mdata);
+}
+}

[pawosm-arm]

I've tried this patch and indeed I can build complete gromacs (with -Weverything) without compiler hanging on any part of its code.

One small comment (feel free to ignore if feels irrelevant): the gmx_-prefixed names used in the noreturn-vars.cpp test file clearly come from the gromacs project. It is open-sourced project so it shouldn't matter, but I wonder what the general rule is, can the names in the test cases refer back to the piece of software where the problem we test against originate from.

[erichkeane]

Once the test is good, so am I.

[pawosm-arm]

/cherry-pick 330b40e

[pawosm-arm]

This bug affect release/21.x too, so this commit must be cherry-picked

[llvmbot]

/pull-request #151381