Recognize scope of thread local variables in CFGBuilder

[kparzysz]

Summary

Compiling the following testcase causes a crash:

struct A {
  ~A() {}
};

int fred(char *p, int *q) {
  if (p != 0) {
    goto bail;
  }
  thread_local A a;
  *q = 0;
bail:
  return 0;
}

The crash is caused by CFGBuilder thinking that the variable a is local (in terms of storage scope), and trying to perform some sort of a lifetime analysis on it. The problem is that the program would not have been legal if a was actually local, and the problem would have been diagnosed earlier, preventing CFGBuilder from running in the first place.

This fix recognizes locally declared C++11 thread_local variables as static.

Assertion failed: (F != const_iterator() && "L iterator is not reachable from F iterator."), function distance, file /w/src/llvm.org/clang/lib/Analysis/CFG.cpp, line 334.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: /w/c/org/bin/clang++ -c dd.cc
1.      <eof> parser at end of file
2.      dd.cc:5:27: parsing function body 'fred'
 #0 0x000000000561c8aa llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/w/c/org/bin/clang+++0x561c8aa)
 #1 0x000000000561a788 llvm::sys::RunSignalHandlers() (/w/c/org/bin/clang+++0x561a788)
 #2 0x000000000557edd4 (anonymous namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) CrashRecoveryContext.cpp:0:0
 #3 0x000000000557efa0 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #4 0x00000008091f35b0 handle_signal /usr/src/lib/libthr/thread/thr_sig.c:0:3
 #5 0x00000008091f2b6f thr_sighandler /usr/src/lib/libthr/thread/thr_sig.c:247:1
 #6 0x00007ffffffff2d3 ([vdso]+0x2d3)
 #7 0x000000080950f14a thr_kill /usr/obj/usr/src/amd64.amd64/lib/libc/thr_kill.S:4:0
 #8 0x0000000809487c34 _raise /usr/src/lib/libc/gen/raise.c:0:10
 #9 0x0000000809538e99 abort /usr/src/lib/libc/stdlib/abort.c:73:17
#10 0x000000080946a9d1 (/lib/libc.so.7+0x929d1)
#11 0x0000000007f2fa66 (anonymous namespace)::CFGBuilder::addAutomaticObjHandling((anonymous namespace)::LocalScope::const_iterator, (anonymous namespace)::LocalScope::const_iterator, clang::Stmt*) CFG.cpp:0:0
#12 0x0000000007f2adc7 (anonymous namespace)::CFGBuilder::VisitGotoStmt(clang::GotoStmt*) CFG.cpp:0:0
#13 0x0000000007f2866f (anonymous namespace)::CFGBuilder::VisitCompoundStmt(clang::CompoundStmt*, bool) CFG.cpp:0:0
#14 0x0000000007f2b612 (anonymous namespace)::CFGBuilder::VisitIfStmt(clang::IfStmt*) CFG.cpp:0:0
#15 0x0000000007f2866f (anonymous namespace)::CFGBuilder::VisitCompoundStmt(clang::CompoundStmt*, bool) CFG.cpp:0:0
#16 0x0000000007f211e4 clang::CFG::buildCFG(clang::Decl const*, clang::Stmt*, clang::ASTContext*, clang::CFG::BuildOptions const&) (/w/c/org/bin/clang+++0x7f211e4)
#17 0x0000000007ef12ab clang::AnalysisDeclContext::getCFG() (/w/c/org/bin/clang+++0x7ef12ab)
#18 0x00000000076bb654 clang::sema::AnalysisBasedWarnings::IssueWarnings(clang::sema::AnalysisBasedWarnings::Policy, clang::sema::FunctionScopeInfo*, clang::Decl const*, clang::QualType) (/w/c/org/bin/clang+++0x76bb654)
#19 0x00000000076b05f7 clang::Sema::PopFunctionScopeInfo(clang::sema::AnalysisBasedWarnings::Policy const*, clang::Decl const*, clang::QualType) (/w/c/org/bin/clang+++0x76b05f7)
#20 0x0000000007817aa2 clang::Sema::ActOnFinishFunctionBody(clang::Decl*, clang::Stmt*, bool) (/w/c/org/bin/clang+++0x7817aa2)
#21 0x00000000075bb3f3 clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) (/w/c/org/bin/clang+++0x75bb3f3)
#22 0x0000000007590e34 clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) (/w/c/org/bin/clang+++0x7590e34)
#23 0x0000000007615d7d clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, clang::DeclaratorContext, clang::SourceLocation*, clang::Parser::ForRangeInit*) (/w/c/org/bin/clang+++0x7615d7d)
#24 0x000000000758f681 clang::Parser::ParseDeclOrFunctionDefInternal(clang::ParsedAttributes&, clang::ParsingDeclSpec&, clang::AccessSpecifier) (/w/c/org/bin/clang+++0x758f681)
#25 0x000000000758f0a2 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::ParsedAttributes&, clang::ParsingDeclSpec*, clang::AccessSpecifier) (/w/c/org/bin/clang+++0x758f0a2)
#26 0x000000000758de8b clang::Parser::ParseExternalDeclaration(clang::ParsedAttributes&, clang::ParsingDeclSpec*) (/w/c/org/bin/clang+++0x758de8b)
#27 0x000000000758bdcd clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) (/w/c/org/bin/clang+++0x758bdcd)
#28 0x0000000007585f7e clang::ParseAST(clang::Sema&, bool, bool) (/w/c/org/bin/clang+++0x7585f7e)
#29 0x0000000006082523 clang::FrontendAction::Execute() (/w/c/org/bin/clang+++0x6082523)
#30 0x000000000600aeef clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/w/c/org/bin/clang+++0x600aeef)
#31 0x000000000614c4b5 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/w/c/org/bin/clang+++0x614c4b5)
#32 0x0000000003244afc cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/w/c/org/bin/clang+++0x3244afc)
#33 0x00000000032421ea ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) driver.cpp:0:0
#34 0x0000000005eb48e7 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool*) const::$_1>(long) Job.cpp:0:0
#35 0x000000000557ecf0 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/w/c/org/bin/clang+++0x557ecf0)
#36 0x0000000005eb43f2 clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool*) const (/w/c/org/bin/clang+++0x5eb43f2)
#37 0x0000000005e7dba4 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&) const (/w/c/org/bin/clang+++0x5e7dba4)
#38 0x0000000005e7dfd9 clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*> >&) const (/w/c/org/bin/clang+++0x5e7dfd9)
#39 0x0000000005e953ac clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*> >&) (/w/c/org/bin/clang+++0x5e953ac)
clang-15: error: clang frontend command failed with exit code 134 (use -v to see invocation)
clang version 15.0.0 (https://github.com/llvm/llvm-project.git 0b2a2620b883cef1dac63f3ea4c839aba0e4d80e)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /w/c/org/bin
clang-15: note: diagnostic msg:
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-15: note: diagnostic msg: /tmp/dd-48e7c6.cpp
clang-15: note: diagnostic msg: /tmp/dd-48e7c6.sh
clang-15: note: diagnostic msg:

********************

[efriedma]

Can we just use VarDecl::hasLocalStorage()?

[kparzysz]

Replace the direct storage class examination with hasLocalStorage.

[kparzysz]

Use the proper diff file this time.

[efriedma]

LGTM

clang/lib/Analysis/CFG.cpp

LocalScope* CFGBuilder::addLocalScopeForVarDecl(VarDecl *VD,
                                                 LocalScope* Scope) {
   assert(!(BuildOpts.AddImplicitDtors && BuildOpts.AddLifetime) &&
          "AddImplicitDtors and AddLifetime cannot be used at the same time");
   if (!BuildOpts.AddImplicitDtors && !BuildOpts.AddLifetime &&
       !BuildOpts.AddScopes)
     return Scope;
 
   // Check if variable is local.
-  switch (VD->getStorageClass()) {
-  case SC_None:
-  case SC_Auto:
-  case SC_Register:
-    break;
-  default: return Scope;
-  }
+  if (!VD->hasLocalStorage())
+    return Scope;
 
   if (BuildOpts.AddImplicitDtors) {
     if (!hasTrivialDestructor(VD) || BuildOpts.AddScopes) {
       // Add the variable to scope
       Scope = createOrReuseLocalScope(Scope);
       Scope->addVar(VD);
       ScopePos = Scope->begin();
     }

clang/test/Analysis/cfg.cpp

namespace CommaTemp {
   struct A { ~A(); };
   struct B { ~B(); };
   void f();
 }
 void CommaTemp::f() {
   A(), B();
 }
 
+// CHECK-LABEL: int crash_with_thread_local(char *p, int *q)
+// CHECK:       [B7 (ENTRY)]
+// CHECK-NEXT:    Succs (1): B6
+// CHECK:       [B1]
+// CHECK-NEXT:   bail:
+// CHECK-NEXT:    1: 0
+// CHECK-NEXT:    2: return [B1.1];
+// CHECK-NEXT:    Preds (2): B2 B5
+// CHECK-NEXT:    Succs (1): B0
+// CHECK:       [B2]
+// CHECK-NEXT:    1: 0
+// CHECK-NEXT:    2: q
+// CHECK-NEXT:    3: [B2.2] (ImplicitCastExpr, LValueToRValue, int *)
+// CHECK-NEXT:    4: *[B2.3]
+// CHECK-NEXT:    5: [B2.4] = [B2.1]
+// CHECK-NEXT:    Preds (2): B3 B4
+// CHECK-NEXT:    Succs (1): B1
+// CHECK:       [B3]
+// WARNINGS-NEXT: 1:  (CXXConstructExpr, struct ClassWithDtor)
+// ANALYZER-NEXT: 1:  (CXXConstructExpr, [B3.2], struct ClassWithDtor)
+// CHECK-NEXT:    2: thread_local ClassWithDtor a;
+// CHECK-NEXT:    Preds (1): B4
+// CHECK-NEXT:    Succs (1): B2
+// CHECK:       [B4]
+// CHECK-NEXT:    T: static init a
+// CHECK-NEXT:    Preds (1): B6
+// CHECK-NEXT:    Succs (2): B2 B3
+// CHECK:       [B5]
+// CHECK-NEXT:    T: goto bail;
+// CHECK-NEXT:    Preds (1): B6
+// CHECK-NEXT:    Succs (1): B1
+// CHECK:       [B6]
+// CHECK-NEXT:    1: p
+// CHECK-NEXT:    2: [B6.1] (ImplicitCastExpr, LValueToRValue, char *)
+// CHECK-NEXT:    3: 0
+// CHECK-NEXT:    4: [B6.3] (ImplicitCastExpr, NullToPointer, char *)
+// CHECK-NEXT:    5: [B6.2] != [B6.4]
+// CHECK-NEXT:    T: if [B6.5]
+// CHECK-NEXT:    Preds (1): B7
+// CHECK-NEXT:    Succs (2): B5 B4
+// CHECK:       [B0 (EXIT)]
+// CHECK-NEXT:    Preds (1): B1
+
+struct ClassWithDtor {
+  ~ClassWithDtor() {}
+};
+
+int crash_with_thread_local(char *p, int *q) {
+  if (p != 0) {
+    goto bail;
+  }
+  thread_local ClassWithDtor a;
+  *q = 0;
+bail:
+  return 0;
+}
+
 // CHECK-LABEL: template<> int *PR18472<int>()
 // CHECK: [B2 (ENTRY)]
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1: 0
 // CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, NullToPointer, PR18472_t)
 // CHECK-NEXT:   3: (PR18472_t)[B1.2] (CStyleCastExpr, NoOp, PR18472_t)
 // CHECK-NEXT:   4: CFGNewAllocator(int *)