[PATCH] D124621: [Analyzer] Fix assumptions about const field with member-initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl accepted this revision.
r.stahl added a comment.

I can confirm the issue with my patch, so this piece of code needs to be 
removed.

As long as the following test still succeeds, this looks good to me. Back then, 
the analyzer was not able to cover that case without that addition.

https://github.com/llvm/llvm-project/blob/main/clang/test/Analysis/globals.cpp#L110


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124621/new/

https://reviews.llvm.org/D124621

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
r.stahl marked an inline comment as done.
Closed by commit rGf625a8a250b3: [clang-format][tests] Explicitly specify style 
in some tests (authored by r.stahl).

Changed prior to commit:
  https://reviews.llvm.org/D61001?vs=196212=209500#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61001/new/

https://reviews.llvm.org/D61001

Files:
  clang/test/Format/adjust-indent.cpp
  clang/test/Format/disable-include-sorting.cpp
  clang/test/Format/language-detection.cpp
  clang/test/Format/xmloutput.cpp


Index: clang/test/Format/xmloutput.cpp
===
--- clang/test/Format/xmloutput.cpp
+++ clang/test/Format/xmloutput.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format -output-replacements-xml -sort-includes %s \
+// RUN: clang-format -style=LLVM -output-replacements-xml -sort-includes %s \
 // RUN:   | FileCheck -strict-whitespace %s
 
 // CHECK: >>= b;$}}
 // CHECK2: {{^a >> >= b;$}}
Index: clang/test/Format/disable-include-sorting.cpp
===
--- clang/test/Format/disable-include-sorting.cpp
+++ clang/test/Format/disable-include-sorting.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format %s | FileCheck %s
+// RUN: clang-format %s -style=LLVM | FileCheck %s
 // RUN: clang-format %s -sort-includes -style="{SortIncludes: false}" | 
FileCheck %s
 // RUN: clang-format %s -sort-includes=false | FileCheck %s 
-check-prefix=NOT-SORTED
 
Index: clang/test/Format/adjust-indent.cpp
===
--- clang/test/Format/adjust-indent.cpp
+++ clang/test/Format/adjust-indent.cpp
@@ -1,4 +1,4 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -lines=2:2 \
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM -lines=2:2 \
 // RUN:   | FileCheck -strict-whitespace %s
 
 void  f() {


Index: clang/test/Format/xmloutput.cpp
===
--- clang/test/Format/xmloutput.cpp
+++ clang/test/Format/xmloutput.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format -output-replacements-xml -sort-includes %s \
+// RUN: clang-format -style=LLVM -output-replacements-xml -sort-includes %s \
 // RUN:   | FileCheck -strict-whitespace %s
 
 // CHECK: >>= b;$}}
 // CHECK2: {{^a >> >= b;$}}
Index: clang/test/Format/disable-include-sorting.cpp
===
--- clang/test/Format/disable-include-sorting.cpp
+++ clang/test/Format/disable-include-sorting.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format %s | FileCheck %s
+// RUN: clang-format %s -style=LLVM | FileCheck %s
 // RUN: clang-format %s -sort-includes -style="{SortIncludes: false}" | FileCheck %s
 // RUN: clang-format %s -sort-includes=false | FileCheck %s -check-prefix=NOT-SORTED
 
Index: clang/test/Format/adjust-indent.cpp
===
--- clang/test/Format/adjust-indent.cpp
+++ clang/test/Format/adjust-indent.cpp
@@ -1,4 +1,4 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -lines=2:2 \
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM -lines=2:2 \
 // RUN:   | FileCheck -strict-whitespace %s
 
 void  f() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl marked 2 inline comments as done.
r.stahl added inline comments.



Comment at: test/Format/language-detection.cpp:2
 // RUN: grep -Ev "// *[A-Z0-9_]+:" %s \
-// RUN:   | clang-format -style=llvm -assume-filename=foo.js \
 // RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s

lebedev.ri wrote:
> What's wrong with `-style=llvm`?
I looked at all the tests in that folder and saw that LLVM is the more common 
usage and unified it.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61001/new/

https://reviews.llvm.org/D61001



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added reviewers: MyDeveloperDay, krasimir.
r.stahl added a comment.

This should be trivial enough to just commit, but I'd just be more comfortable 
if someone looked at it before, because this is my first commit in this area of 
clang.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61001/new/

https://reviews.llvm.org/D61001



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

@xazax.hun Yes, I planned to just commit. Set you as Subscriber not Reviewer in 
Arc. Was just a bit confused why it fails at first :)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61002/new/

https://reviews.llvm.org/D61002



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl abandoned this revision.
r.stahl added a comment.

Was already done: 
https://github.com/llvm-mirror/clang/commit/bacdda22396c39181aa0e641182e01a0b3cf43ea


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61002/new/

https://reviews.llvm.org/D61002



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.
Herald added a project: clang.

Missing break/fallthrough


Repository:
  rC Clang

https://reviews.llvm.org/D61002

Files:
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp


Index: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -90,6 +90,7 @@
   case UniqueExternalLinkage:
 if (SM.isInMainFile(defStart))
   Index[LookupName] = CurrentFileName;
+break;
   default:
 break;
   }


Index: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -90,6 +90,7 @@
   case UniqueExternalLinkage:
 if (SM.isInMainFile(defStart))
   Index[LookupName] = CurrentFileName;
+break;
   default:
 break;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: djasper, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes broken tests when doing an out-of-source build that picks up a 
random .clang-format on the file system due to the default "file" style.


Repository:
  rC Clang

https://reviews.llvm.org/D61001

Files:
  test/Format/adjust-indent.cpp
  test/Format/disable-include-sorting.cpp
  test/Format/language-detection.cpp
  test/Format/xmloutput.cpp


Index: test/Format/xmloutput.cpp
===
--- test/Format/xmloutput.cpp
+++ test/Format/xmloutput.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format -output-replacements-xml -sort-includes %s \
+// RUN: clang-format -style=LLVM -output-replacements-xml -sort-includes %s \
 // RUN:   | FileCheck -strict-whitespace %s
 
 // CHECK: >>= b;$}}
 // CHECK2: {{^a >> >= b;$}}
Index: test/Format/disable-include-sorting.cpp
===
--- test/Format/disable-include-sorting.cpp
+++ test/Format/disable-include-sorting.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format %s | FileCheck %s
+// RUN: clang-format %s -style=LLVM | FileCheck %s
 // RUN: clang-format %s -sort-includes -style="{SortIncludes: false}" | 
FileCheck %s
 // RUN: clang-format %s -sort-includes=false | FileCheck %s 
-check-prefix=NOT-SORTED
 
Index: test/Format/adjust-indent.cpp
===
--- test/Format/adjust-indent.cpp
+++ test/Format/adjust-indent.cpp
@@ -1,4 +1,4 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -lines=2:2 \
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM -lines=2:2 \
 // RUN:   | FileCheck -strict-whitespace %s
 
 void  f() {


Index: test/Format/xmloutput.cpp
===
--- test/Format/xmloutput.cpp
+++ test/Format/xmloutput.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format -output-replacements-xml -sort-includes %s \
+// RUN: clang-format -style=LLVM -output-replacements-xml -sort-includes %s \
 // RUN:   | FileCheck -strict-whitespace %s
 
 // CHECK: >>= b;$}}
 // CHECK2: {{^a >> >= b;$}}
Index: test/Format/disable-include-sorting.cpp
===
--- test/Format/disable-include-sorting.cpp
+++ test/Format/disable-include-sorting.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format %s | FileCheck %s
+// RUN: clang-format %s -style=LLVM | FileCheck %s
 // RUN: clang-format %s -sort-includes -style="{SortIncludes: false}" | FileCheck %s
 // RUN: clang-format %s -sort-includes=false | FileCheck %s -check-prefix=NOT-SORTED
 
Index: test/Format/adjust-indent.cpp
===
--- test/Format/adjust-indent.cpp
+++ test/Format/adjust-indent.cpp
@@ -1,4 +1,4 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -lines=2:2 \
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM -lines=2:2 \
 // RUN:   | FileCheck -strict-whitespace %s
 
 void  f() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358968: [analyzer][CrossTU] Extend CTU to VarDecls with 
initializer (authored by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46421?vs=196075=196207#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp
  test/Analysis/redecl.c
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -158,6 +158,27 @@
   return Result.str();
 }
 
+bool containsConst(const VarDecl *VD, const ASTContext ) {
+  CanQualType CT = ACtx.getCanonicalType(VD->getType());
+  if (!CT.isConstQualified()) {
+const RecordType *RTy = CT->getAs();
+if (!RTy || !RTy->hasConstFields())
+  return false;
+  }
+  return true;
+}
+
+static bool hasBodyOrInit(const FunctionDecl *D, const FunctionDecl *) {
+  return D->hasBody(DefD);
+}
+static bool hasBodyOrInit(const VarDecl *D, const VarDecl *) {
+  return D->getAnyInitializer(DefD);
+}
+template  static bool hasBodyOrInit(const T *D) {
+  const T *Unused;
+  return hasBodyOrInit(D, Unused);
+}
+
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance )
 : CI(CI), Context(CI.getASTContext()) {}
 
@@ -165,48 +186,50 @@
 
 std::string CrossTranslationUnitContext::getLookupName(const NamedDecl *ND) {
   SmallString<128> DeclUSR;
-  bool Ret = index::generateUSRForDecl(ND, DeclUSR); (void)Ret;
+  bool Ret = index::generateUSRForDecl(ND, DeclUSR);
+  (void)Ret;
   assert(!Ret && "Unable to generate USR");
   return DeclUSR.str();
 }
 
-/// Recursively visits the function decls of a DeclContext, and looks up a
-/// function based on USRs.
-const FunctionDecl *
-CrossTranslationUnitContext::findFunctionInDeclContext(const DeclContext *DC,
-   StringRef LookupFnName) {
+/// Recursively visits the decls of a DeclContext, and returns one with the
+/// given USR.
+template 
+const T *
+CrossTranslationUnitContext::findDefInDeclContext(const DeclContext *DC,
+  StringRef LookupName) {
   assert(DC && "Declaration Context must not be null");
   for (const Decl *D : DC->decls()) {
 const auto *SubDC = dyn_cast(D);
 if (SubDC)
-  if (const auto *FD = findFunctionInDeclContext(SubDC, LookupFnName))
-return FD;
+  if (const auto *ND = findDefInDeclContext(SubDC, LookupName))
+return ND;
 
-const auto *ND = dyn_cast(D);
-const FunctionDecl *ResultDecl;
-if (!ND || !ND->hasBody(ResultDecl))
+const auto *ND = dyn_cast(D);
+const T *ResultDecl;
+if (!ND || !hasBodyOrInit(ND, ResultDecl))
   continue;
-if (getLookupName(ResultDecl) != LookupFnName)
+if (getLookupName(ResultDecl) != LookupName)
   continue;
 return ResultDecl;
   }
   return nullptr;
 }
 
-llvm::Expected
-CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
-  StringRef CrossTUDir,
-  StringRef IndexName,
-  bool DisplayCTUProgress) {
-  assert(FD && "FD is missing, bad call to this function!");
-  assert(!FD->hasBody() && "FD has a definition in current translation unit!");
+template 
+llvm::Expected CrossTranslationUnitContext::getCrossTUDefinitionImpl(
+const T *D, StringRef CrossTUDir, StringRef IndexName,
+bool DisplayCTUProgress) {
+  assert(D && "D is missing, bad call to this function!");
+  assert(!hasBodyOrInit(D) &&
+ "D has a body or init in current translation unit!");
   ++NumGetCTUCalled;
-  const std::string LookupFnName = getLookupName(FD);
-  if (LookupFnName.empty())
+  const std::string LookupName = getLookupName(D);
+  if (LookupName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
   llvm::Expected ASTUnitOrError =
-  loadExternalAST(LookupFnName, CrossTUDir, IndexName, DisplayCTUProgress);
+  loadExternalAST(LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -262,12 +285,29 @@
   }
 
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
-  if (const FunctionDecl *ResultDecl =
-  findFunctionInDeclContext(TU, LookupFnName))
+  if 

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 196075.
r.stahl added a comment.

@xazax.hun good point and that actually fixes a bug since that branch should 
also do the deep check. Added that to the tests.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp
  test/Analysis/redecl.c
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -34,20 +34,22 @@
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
   MapExtDefNamesConsumer(ASTContext )
-  : SM(Context.getSourceManager()) {}
+  : Ctx(Context), SM(Context.getSourceManager()) {}
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
 llvm::outs() << createCrossTUIndexString(Index);
   }
 
-  void HandleTranslationUnit(ASTContext ) override {
-handleDecl(Ctx.getTranslationUnitDecl());
+  void HandleTranslationUnit(ASTContext ) override {
+handleDecl(Context.getTranslationUnitDecl());
   }
 
 private:
   void handleDecl(const Decl *D);
+  void addIfInMain(const DeclaratorDecl *DD, SourceLocation defStart);
 
+  ASTContext 
   SourceManager 
   llvm::StringMap Index;
   std::string CurrentFileName;
@@ -58,30 +60,13 @@
 return;
 
   if (const auto *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
-  if (const Stmt *Body = FD->getBody()) {
-if (CurrentFileName.empty()) {
-  CurrentFileName =
-  SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
-  if (CurrentFileName.empty())
-CurrentFileName = "invalid_file";
-}
-
-switch (FD->getLinkageInternal()) {
-case ExternalLinkage:
-case VisibleNoLinkage:
-case UniqueExternalLinkage:
-  if (SM.isInMainFile(Body->getBeginLoc())) {
-std::string LookupName =
-CrossTranslationUnitContext::getLookupName(FD);
-Index[LookupName] = CurrentFileName;
-  }
-  break;
-default:
-  break;
-}
-  }
-}
+if (FD->isThisDeclarationADefinition())
+  if (const Stmt *Body = FD->getBody())
+addIfInMain(FD, Body->getBeginLoc());
+  } else if (const auto *VD = dyn_cast(D)) {
+if (cross_tu::containsConst(VD, Ctx) && VD->hasInit())
+  if (const Expr *Init = VD->getInit())
+addIfInMain(VD, Init->getBeginLoc());
   }
 
   if (const auto *DC = dyn_cast(D))
@@ -89,6 +74,27 @@
   handleDecl(D);
 }
 
+void MapExtDefNamesConsumer::addIfInMain(const DeclaratorDecl *DD,
+ SourceLocation defStart) {
+  std::string LookupName = CrossTranslationUnitContext::getLookupName(DD);
+  if (CurrentFileName.empty()) {
+CurrentFileName =
+SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+if (CurrentFileName.empty())
+  CurrentFileName = "invalid_file";
+  }
+
+  switch (DD->getLinkageInternal()) {
+  case ExternalLinkage:
+  case VisibleNoLinkage:
+  case UniqueExternalLinkage:
+if (SM.isInMainFile(defStart))
+  Index[LookupName] = CurrentFileName;
+  default:
+break;
+  }
+}
+
 class MapExtDefNamesAction : public ASTFrontendAction {
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
Index: test/Analysis/redecl.c
===
--- /dev/null
+++ test/Analysis/redecl.c
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// XFAIL: *
+
+void clang_analyzer_eval(int);
+
+extern const int extInt;
+
+int main()
+{
+clang_analyzer_eval(extInt == 2); // expected-warning{{TRUE}}
+}
+
+extern const int extInt = 2;
Index: test/Analysis/func-mapping-test.cpp
===
--- test/Analysis/func-mapping-test.cpp
+++ test/Analysis/func-mapping-test.cpp
@@ -1,7 +1,43 @@
-// RUN: %clang_extdef_map %s -- | FileCheck %s
+// RUN: %clang_extdef_map %s -- | FileCheck --implicit-check-not "c:@y" --implicit-check-not "c:@z" %s
 
 int f(int) {
   return 0;
 }
+// CHECK-DAG: c:@F@f#I#
 
-// CHECK: c:@F@f#I#
+extern const int x = 5;
+// CHECK-DAG: c:@x
+
+// Non-const variables should not be collected.
+int y = 5;
+
+// In C++, const implies internal linkage, so not collected.
+const int z = 5;
+
+struct S {
+  int a;
+};
+extern S const s = {.a = 2};
+// CHECK-DAG: c:@s
+
+struct SF {
+  const int 

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 194285.
r.stahl marked 4 inline comments as done.
r.stahl added a comment.

- addressed review comments
- created cross_tu::containsConst
- fixed bug that unions were not exported
- added TODO test for constructor case


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp
  test/Analysis/redecl.c
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -34,20 +34,22 @@
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
   MapExtDefNamesConsumer(ASTContext )
-  : SM(Context.getSourceManager()) {}
+  : Ctx(Context), SM(Context.getSourceManager()) {}
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
 llvm::outs() << createCrossTUIndexString(Index);
   }
 
-  void HandleTranslationUnit(ASTContext ) override {
-handleDecl(Ctx.getTranslationUnitDecl());
+  void HandleTranslationUnit(ASTContext ) override {
+handleDecl(Context.getTranslationUnitDecl());
   }
 
 private:
   void handleDecl(const Decl *D);
+  void addIfInMain(const DeclaratorDecl *DD, SourceLocation defStart);
 
+  ASTContext 
   SourceManager 
   llvm::StringMap Index;
   std::string CurrentFileName;
@@ -58,30 +60,13 @@
 return;
 
   if (const auto *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
-  if (const Stmt *Body = FD->getBody()) {
-if (CurrentFileName.empty()) {
-  CurrentFileName =
-  SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
-  if (CurrentFileName.empty())
-CurrentFileName = "invalid_file";
-}
-
-switch (FD->getLinkageInternal()) {
-case ExternalLinkage:
-case VisibleNoLinkage:
-case UniqueExternalLinkage:
-  if (SM.isInMainFile(Body->getBeginLoc())) {
-std::string LookupName =
-CrossTranslationUnitContext::getLookupName(FD);
-Index[LookupName] = CurrentFileName;
-  }
-  break;
-default:
-  break;
-}
-  }
-}
+if (FD->isThisDeclarationADefinition())
+  if (const Stmt *Body = FD->getBody())
+addIfInMain(FD, Body->getBeginLoc());
+  } else if (const auto *VD = dyn_cast(D)) {
+if (cross_tu::containsConst(VD, Ctx) && VD->hasInit())
+  if (const Expr *Init = VD->getInit())
+addIfInMain(VD, Init->getBeginLoc());
   }
 
   if (const auto *DC = dyn_cast(D))
@@ -89,6 +74,27 @@
   handleDecl(D);
 }
 
+void MapExtDefNamesConsumer::addIfInMain(const DeclaratorDecl *DD,
+ SourceLocation defStart) {
+  std::string LookupName = CrossTranslationUnitContext::getLookupName(DD);
+  if (CurrentFileName.empty()) {
+CurrentFileName =
+SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+if (CurrentFileName.empty())
+  CurrentFileName = "invalid_file";
+  }
+
+  switch (DD->getLinkageInternal()) {
+  case ExternalLinkage:
+  case VisibleNoLinkage:
+  case UniqueExternalLinkage:
+if (SM.isInMainFile(defStart))
+  Index[LookupName] = CurrentFileName;
+  default:
+break;
+  }
+}
+
 class MapExtDefNamesAction : public ASTFrontendAction {
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
Index: test/Analysis/redecl.c
===
--- /dev/null
+++ test/Analysis/redecl.c
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// XFAIL: *
+
+void clang_analyzer_eval(int);
+
+extern const int extInt;
+
+int main()
+{
+clang_analyzer_eval(extInt == 2); // expected-warning{{TRUE}}
+}
+
+extern const int extInt = 2;
Index: test/Analysis/func-mapping-test.cpp
===
--- test/Analysis/func-mapping-test.cpp
+++ test/Analysis/func-mapping-test.cpp
@@ -1,7 +1,43 @@
-// RUN: %clang_extdef_map %s -- | FileCheck %s
+// RUN: %clang_extdef_map %s -- | FileCheck --implicit-check-not "c:@y" --implicit-check-not "c:@z" %s
 
 int f(int) {
   return 0;
 }
+// CHECK-DAG: c:@F@f#I#
 
-// CHECK: c:@F@f#I#
+extern const int x = 5;
+// CHECK-DAG: c:@x
+
+// Non-const variables should not be collected.
+int y = 5;
+
+// In C++, const implies internal linkage, so not collected.
+const int z = 5;
+
+struct S {
+  int a;
+};
+extern S const s = 

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:357
+  return true;
+if (!RTy->hasConstFields())
+  return true;

xazax.hun wrote:
> I wonder what would happen with types that has const fields and user written 
> constructors. In case we will not simulate the effect of the constructor and 
> will not be able to set the const fields maybe we should exclude those as 
> well?
I added a test for that and it doesn't seem to work. It just ends up as unknown 
which is fine, right? Eventually it would be nice to do that of course.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl abandoned this revision.
r.stahl added a comment.
Herald added a reviewer: shafik.

ASTContext::getParents should not be used this way. This use-case is solved by 
function-scoped ParentMaps or AnalysisDeclContext::getParentMap. Discussion: 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061944.html


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46940/new/

https://reviews.llvm.org/D46940



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl marked 3 inline comments as done.
r.stahl added a comment.

In D46421#1456171 , @xazax.hun wrote:

> My last set of comments are also unresolved. Most of them are minor nits, but 
> I would love to get rid of the code duplication between ClangExtDefMapGen and 
> the Clang Static Analyzer regarding when do we consider a variable worth to 
> import. Otherwise the patch looks good to me.


Sorry, was too long ago, thought this was done!

What would be a good place for this? I'm thinking ExprClassification in the AST 
lib. There is a tail in `IsModifiable` that tests the `CanQualType` for const 
that could be moved out. And then make a new function `bool ContainsConst(const 
VarDecl *VD)` mabye? Shouldn't that be a new patch, because it touches the more 
critical AST lib?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Okay so I would suggest to go ahead and commit this. Rebased it succeeds 
without modification.

Still leaves the open problems with the redecls. Should I add the failing test 
from https://reviews.llvm.org/D46421#1375147 in the same commit marked as 
expected to fail? Or what is the procedure here?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:255
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||

This is likely to become a bug in the future, but I didn't see a better way to 
compare dialects.

Is there a way to add a test that lights up once there is a new dialect?

Would it be too pessimistic to compare the entire LangOpts? Some stuff in there 
should even still produce errors, right? For example "GnuMode". I skimmed over 
them and didn't find an obvious one that would differ between translation units 
of the same project. Maybe the template depth could be set selectively, but to 
fix that we could mask "COMPATIBLE_" and "BENIGN_" opts.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57906/new/

https://reviews.llvm.org/D57906



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Thanks for the comments! All good points. Nice idea with the constructor, but 
that can probably happen in a follow up patch.

In D46421#1380619 , @xazax.hun wrote:

> I know you probably tired of me making you split all the patches but I think 
> if it is independent of CTU we should submit this fix as a separate commit. 
> This way we could be more focused having one commit doing one thing.


No problem if I should do that. However I'm concerned whether this change is 
correct. Previously this was not required since all VarDecls were canonical. 
Not sure if this change was intended. I did some digging, but am not familiar 
enough with the code base to figure out what changed. Does anyone have an idea 
about this?

But I agree that if it wasn't a deliberate change, we could canonicalize in the 
DeclRegion constructor - or VarRegion since only those are redeclarable.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In D46421#1374807 , @NoQ wrote:

> At the same time, i don't have any test cases for the actual change in 
> behavior that such canonicalization causes. If the test case that you had in 
> mind is indeed demonstrating this problem, i'd love to have it. If it turns 
> out that your test case doesn't allow us to demonstrate the problem without 
> CTU, then probably it has something to do with `ASTImporter` accidentally 
> canonicalizing the the declaration in `DeclRefExpr` more rarely than the 
> vanilla AST.


This seems unrelated to CTU. The following subset of my test demonstrates this:

  // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-verify %s
  
  void clang_analyzer_eval(int);
  
  extern const int extInt;
  
  int main()
  {
  clang_analyzer_eval(extInt == 2); // expected-warning{{TRUE}}
  }
  
  extern const int extInt = 2;



  Breakpoint 1, (anonymous namespace)::RegionStoreManager::getBindingForVar 
(this=0xa7b420, B=..., R=0xa7d348)
  at 
/data/work/commitllvm/llvm/tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1948
  1948if (const Expr *Init = VD->getAnyInitializer()) {
  (gdb) p VD->getInit()
  $1 = (const clang::Expr *) 0x0
  (gdb) p VD->getAnyInitializer()
  $2 = (const clang::Expr *) 0xa4b630


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 181263.
r.stahl marked 12 inline comments as done.
r.stahl added a comment.

Strip name changes (see D56441 ); addressed 
review comments

In my old version I seemed to get away with the tests, but they failed after 
rebasing. It seems like earlier there was only a single VarDecl for the 
imported ones with InitExpr. Now after importing there is one without init and 
a redecl with the init. This is why I changed getInit() in RegionStore to 
getAnyInititializer. I think these three should be enough, but I'm not sure 
where else in the analyzer this would have to be changed.

I also noticed that nested struct inits don't work, but this is unrelated to 
this patch, so I commented out the test for now and will send a separate patch.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -48,6 +48,7 @@
 
 private:
   void handleDecl(const Decl *D);
+  void addIfInMain(const DeclaratorDecl *DD, SourceLocation defStart);
 
   SourceManager 
   llvm::StringMap Index;
@@ -59,30 +60,19 @@
 return;
 
   if (const auto *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
-  if (const Stmt *Body = FD->getBody()) {
-if (CurrentFileName.empty()) {
-  CurrentFileName =
-  SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
-  if (CurrentFileName.empty())
-CurrentFileName = "invalid_file";
-}
-
-switch (FD->getLinkageInternal()) {
-case ExternalLinkage:
-case VisibleNoLinkage:
-case UniqueExternalLinkage:
-  if (SM.isInMainFile(Body->getBeginLoc())) {
-std::string LookupName =
-CrossTranslationUnitContext::getLookupName(FD);
-Index[LookupName] = CurrentFileName;
-  }
-  break;
-default:
-  break;
-}
-  }
-}
+if (FD->isThisDeclarationADefinition())
+  if (const Stmt *Body = FD->getBody())
+addIfInMain(FD, Body->getBeginLoc());
+  } else if (const auto *VD = dyn_cast(D)) {
+QualType VTy = VD->getType();
+bool containsConst = VTy.isConstQualified();
+if (!containsConst && !VTy.isNull())
+  if (const RecordType *RTy = VTy->getAsStructureType())
+containsConst = RTy->hasConstFields();
+
+if (containsConst && VD->hasInit())
+  if (const Expr *Init = VD->getInit())
+addIfInMain(VD, Init->getBeginLoc());
   }
 
   if (const auto *DC = dyn_cast(D))
@@ -90,6 +80,27 @@
   handleDecl(D);
 }
 
+void MapExtDefNamesConsumer::addIfInMain(const DeclaratorDecl *DD,
+ SourceLocation defStart) {
+  std::string LookupName = CrossTranslationUnitContext::getLookupName(DD);
+  if (CurrentFileName.empty()) {
+CurrentFileName =
+SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+if (CurrentFileName.empty())
+  CurrentFileName = "invalid_file";
+  }
+
+  switch (DD->getLinkageInternal()) {
+  case ExternalLinkage:
+  case VisibleNoLinkage:
+  case UniqueExternalLinkage:
+if (SM.isInMainFile(defStart))
+  Index[LookupName] = CurrentFileName;
+  default:
+break;
+  }
+}
+
 class MapExtDefNamesAction : public ASTFrontendAction {
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
Index: test/Analysis/func-mapping-test.cpp
===
--- test/Analysis/func-mapping-test.cpp
+++ test/Analysis/func-mapping-test.cpp
@@ -1,7 +1,36 @@
-// RUN: %clang_extdef_map %s -- | FileCheck %s
+// RUN: %clang_extdef_map %s -- | FileCheck --implicit-check-not "c:@y" --implicit-check-not "c:@z" %s
 
 int f(int) {
   return 0;
 }
+// CHECK-DAG: c:@F@f#I#
 
-// CHECK: c:@F@f#I#
+extern const int x = 5;
+// CHECK-DAG: c:@x
+
+// Non-const variables should not be collected.
+int y = 5;
+
+// In C++, const implies internal linkage, so not collected.
+const int z = 5;
+
+struct S {
+  int a;
+};
+extern S const s = {.a = 2};
+// CHECK-DAG: c:@s
+
+struct SF {
+  const int a;
+};
+SF sf = {.a = 2};
+// CHECK-DAG: c:@sf
+
+struct SStatic {
+  static const int a = 4;
+};
+const int SStatic::a;
+// CHECK-DAG: c:@S@SStatic@a
+
+extern int const arr[5] = { 0, 1 };
+// CHECK-DAG: c:@arr
\ No 

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:120
+}
+template  static bool hasDefinition(const T *D) {
+  const T *Unused;

martong wrote:
> `hasDefinitionOrInit` ?
I simply made it `hasBodyOrInit` now to be as clear as possible


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350852: [analyzer][CrossTU][NFC] Generalize to external 
definitions instead of external… (authored by r.stahl, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56441?vs=181073=181079#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56441/new/

https://reviews.llvm.org/D56441

Files:
  cfe/trunk/include/clang/Basic/DiagnosticCrossTUKinds.td
  cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
  cfe/trunk/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  cfe/trunk/test/Analysis/Inputs/ctu-other.c.externalFnMap.txt
  cfe/trunk/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  cfe/trunk/test/Analysis/Inputs/ctu-other.cpp.externalFnMap.txt
  cfe/trunk/test/Analysis/analyzer-config.c
  cfe/trunk/test/Analysis/ctu-different-triples.cpp
  cfe/trunk/test/Analysis/ctu-main.c
  cfe/trunk/test/Analysis/ctu-main.cpp
  cfe/trunk/test/Analysis/ctu-unknown-parts-in-triples.cpp
  cfe/trunk/test/Analysis/func-mapping-test.cpp
  cfe/trunk/test/CMakeLists.txt
  cfe/trunk/test/lit.cfg.py
  cfe/trunk/tools/CMakeLists.txt
  cfe/trunk/tools/clang-extdef-mapping/CMakeLists.txt
  cfe/trunk/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
  cfe/trunk/tools/clang-func-mapping/CMakeLists.txt
  cfe/trunk/tools/clang-func-mapping/ClangFnMapGen.cpp
  cfe/trunk/tools/scan-build-py/README.md
  cfe/trunk/tools/scan-build-py/libscanbuild/__init__.py
  cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
  cfe/trunk/tools/scan-build-py/libscanbuild/arguments.py
  cfe/trunk/tools/scan-build-py/libscanbuild/clang.py
  cfe/trunk/tools/scan-build-py/tests/unit/test_analyze.py
  cfe/trunk/tools/scan-build-py/tests/unit/test_clang.py

Index: cfe/trunk/test/Analysis/ctu-different-triples.cpp
===
--- cfe/trunk/test/Analysis/ctu-different-triples.cpp
+++ cfe/trunk/test/Analysis/ctu-different-triples.cpp
@@ -2,7 +2,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -triple powerpc64-montavista-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: cfe/trunk/test/Analysis/ctu-main.c
===
--- cfe/trunk/test/Analysis/ctu-main.c
+++ cfe/trunk/test/Analysis/ctu-main.c
@@ -2,7 +2,7 @@
 // RUN: mkdir -p %t/ctudir2
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c
-// RUN: cp %S/Inputs/ctu-other.c.externalFnMap.txt %t/ctudir2/externalFnMap.txt
+// RUN: cp %S/Inputs/ctu-other.c.externalDefMap.txt %t/ctudir2/externalDefMap.txt
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 -analyze \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: cfe/trunk/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
===
--- cfe/trunk/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
+++ cfe/trunk/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
@@ -0,0 +1,15 @@
+c:@N@chns@F@chf1#I# ctu-other.cpp.ast
+c:@N@myns@N@embed_ns@F@fens#I# ctu-other.cpp.ast
+c:@F@g#I# ctu-other.cpp.ast
+c:@S@mycls@F@fscl#I#S ctu-other.cpp.ast
+c:@S@mycls@F@fcl#I# ctu-other.cpp.ast
+c:@N@myns@S@embed_cls@F@fecl#I# ctu-other.cpp.ast
+c:@S@mycls@S@embed_cls2@F@fecl2#I# ctu-other.cpp.ast
+c:@F@f#I# ctu-other.cpp.ast
+c:@N@myns@F@fns#I# ctu-other.cpp.ast
+c:@F@h#I# ctu-other.cpp.ast
+c:@F@h_chain#I# ctu-chain.cpp.ast
+c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
+c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
+c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+c:@F@other_macro_diag#I# ctu-other.cpp.ast
Index: cfe/trunk/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
===
--- cfe/trunk/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
+++ cfe/trunk/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
@@ -0,0 +1,6 @@
+c:@F@inlineAsm ctu-other.c.ast
+c:@F@g ctu-other.c.ast
+c:@F@f ctu-other.c.ast
+c:@F@enumCheck ctu-other.c.ast
+c:@F@identImplicit ctu-other.c.ast
+c:@F@structInProto ctu-other.c.ast
Index: cfe/trunk/test/Analysis/func-mapping-test.cpp
===
--- cfe/trunk/test/Analysis/func-mapping-test.cpp
+++ 

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 181073.
r.stahl added a comment.

addressed review comments


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56441/new/

https://reviews.llvm.org/D56441

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  test/Analysis/Inputs/ctu-other.c.externalFnMap.txt
  test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  test/Analysis/Inputs/ctu-other.cpp.externalFnMap.txt
  test/Analysis/analyzer-config.c
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-main.c
  test/Analysis/ctu-main.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp
  test/Analysis/func-mapping-test.cpp
  test/CMakeLists.txt
  test/lit.cfg.py
  tools/CMakeLists.txt
  tools/clang-extdef-mapping/
  tools/clang-extdef-mapping/CMakeLists.txt
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
  tools/clang-func-mapping/CMakeLists.txt
  tools/clang-func-mapping/ClangFnMapGen.cpp
  tools/scan-build-py/README.md
  tools/scan-build-py/libscanbuild/__init__.py
  tools/scan-build-py/libscanbuild/analyze.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-build-py/libscanbuild/clang.py
  tools/scan-build-py/tests/unit/test_analyze.py
  tools/scan-build-py/tests/unit/test_clang.py

Index: tools/scan-build-py/tests/unit/test_clang.py
===
--- tools/scan-build-py/tests/unit/test_clang.py
+++ tools/scan-build-py/tests/unit/test_clang.py
@@ -96,7 +96,7 @@
 
 class ClangIsCtuCapableTest(unittest.TestCase):
 def test_ctu_not_found(self):
-is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping')
+is_ctu = sut.is_ctu_capable('not-found-clang-extdef-mapping')
 self.assertFalse(is_ctu)
 
 
Index: tools/scan-build-py/tests/unit/test_analyze.py
===
--- tools/scan-build-py/tests/unit/test_analyze.py
+++ tools/scan-build-py/tests/unit/test_analyze.py
@@ -349,14 +349,14 @@
 class MergeCtuMapTest(unittest.TestCase):
 
 def test_no_map_gives_empty(self):
-pairs = sut.create_global_ctu_function_map([])
+pairs = sut.create_global_ctu_extdef_map([])
 self.assertFalse(pairs)
 
 def test_multiple_maps_merged(self):
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun3#I# ast/fun3.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
 self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs)
@@ -366,7 +366,7 @@
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun1#I# ast/fun7.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
@@ -376,28 +376,28 @@
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun1#I# ast/fun1.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
 self.assertEqual(2, len(pairs))
 
 def test_space_handled_in_source(self):
 concat_map = ['c:@F@fun1#I# ast/f un.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs)
 self.assertEqual(1, len(pairs))
 
 
-class FuncMapSrcToAstTest(unittest.TestCase):
+class ExtdefMapSrcToAstTest(unittest.TestCase):
 
 def test_empty_gives_empty(self):
-fun_ast_lst = sut.func_map_list_src_to_ast([])
+fun_ast_lst = sut.extdef_map_list_src_to_ast([])
 self.assertFalse(fun_ast_lst)
 
 def test_sources_to_asts(self):
 fun_src_lst = ['c:@F@f1#I# ' + os.path.join(os.sep + 'path', 'f1.c'),
'c:@F@f2#I# ' + os.path.join(os.sep + 'path', 'f2.c')]
-fun_ast_lst = sut.func_map_list_src_to_ast(fun_src_lst)
+fun_ast_lst = sut.extdef_map_list_src_to_ast(fun_src_lst)
 self.assertTrue('c:@F@f1#I# ' +

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Thanks for the quick response! Will wait a couple days to see if someone else 
has something to add.

But I think something is wrong here...

When trying to "arc diff" I got the following error:

   Exception 
  Command failed with error #1!
  COMMAND
  svn propget 'svn:mime-type' 
'/data/work/commitllvm/llvm/tools/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp'@
  
  STDOUT
  (empty)
  
  STDERR
  svn: warning: W200017: Property 'svn:mime-type' not found on 
'/data/work/commitllvm/llvm/tools/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp@'
  svn: E20: A problem occurred; see other errors for details

So I did:

  svn propset svn:mime-type text/plain 
tools/clang-extdef-mapping/ClangExtDefMapGen.cpp 

This was on all the new/renamed files.

Now this diff shows some weird "svn:mime-type" changes which are probably 
unwanted. But only on two of the new files (?).

I only found this online, but weird that no one else is having this issue: 
https://secure.phabricator.com/T10608


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56441/new/

https://reviews.llvm.org/D56441



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: NoQ, xazax.hun, martong, a.sidorin.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, rnkovacs, szepet, baloghadamsoftware, whisperity, mgorny.
Herald added a reviewer: george.karpenkov.
Herald added a reviewer: serge-sans-paille.

This is just changing naming and documentation to be general about external 
definitions that can be imported for cross translation unit analysis. There is 
at least a plan to add VarDecls: D46421 


Repository:
  rC Clang

https://reviews.llvm.org/D56441

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  test/Analysis/Inputs/ctu-other.c.externalFnMap.txt
  test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  test/Analysis/Inputs/ctu-other.cpp.externalFnMap.txt
  test/Analysis/analyzer-config.c
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-main.c
  test/Analysis/ctu-main.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp
  test/Analysis/func-mapping-test.cpp
  test/CMakeLists.txt
  test/lit.cfg.py
  tools/CMakeLists.txt
  tools/clang-extdef-mapping/
  tools/clang-extdef-mapping/CMakeLists.txt
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
  tools/clang-func-mapping/CMakeLists.txt
  tools/clang-func-mapping/ClangFnMapGen.cpp
  tools/scan-build-py/README.md
  tools/scan-build-py/libscanbuild/__init__.py
  tools/scan-build-py/libscanbuild/analyze.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-build-py/libscanbuild/clang.py
  tools/scan-build-py/tests/unit/test_analyze.py
  tools/scan-build-py/tests/unit/test_clang.py

Index: tools/scan-build-py/tests/unit/test_clang.py
===
--- tools/scan-build-py/tests/unit/test_clang.py
+++ tools/scan-build-py/tests/unit/test_clang.py
@@ -96,7 +96,7 @@
 
 class ClangIsCtuCapableTest(unittest.TestCase):
 def test_ctu_not_found(self):
-is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping')
+is_ctu = sut.is_ctu_capable('not-found-clang-extdef-mapping')
 self.assertFalse(is_ctu)
 
 
Index: tools/scan-build-py/tests/unit/test_analyze.py
===
--- tools/scan-build-py/tests/unit/test_analyze.py
+++ tools/scan-build-py/tests/unit/test_analyze.py
@@ -349,14 +349,14 @@
 class MergeCtuMapTest(unittest.TestCase):
 
 def test_no_map_gives_empty(self):
-pairs = sut.create_global_ctu_function_map([])
+pairs = sut.create_global_ctu_extdef_map([])
 self.assertFalse(pairs)
 
 def test_multiple_maps_merged(self):
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun3#I# ast/fun3.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
 self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs)
@@ -366,7 +366,7 @@
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun1#I# ast/fun7.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
@@ -376,28 +376,28 @@
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun1#I# ast/fun1.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
 self.assertEqual(2, len(pairs))
 
 def test_space_handled_in_source(self):
 concat_map = ['c:@F@fun1#I# ast/f un.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs)
 self.assertEqual(1, len(pairs))
 
 
-class FuncMapSrcToAstTest(unittest.TestCase):
+class ExtdefMapSrcToAstTest(unittest.TestCase):
 
 def test_empty_gives_empty(self):
-fun_ast_lst = sut.func_map_list_src_to_ast([])
+fun_ast_lst = sut.extdef_map_list_src_to_ast([])
 self.assertFalse(fun_ast_lst)
 

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350528: [analyzer] Pass the correct loc Expr from 
VisitIncDecOp to evalStore (authored by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55701?vs=180487=180489#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55701/new/

https://reviews.llvm.org/D55701

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1052,7 +1052,7 @@
   // Perform the store, so that the uninitialized value detection happens.
   Bldr.takeNodes(*I);
   ExplodedNodeSet Dst3;
-  evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+  evalStore(Dst3, U, Ex, *I, state, loc, V2_untested);
   Bldr.addNodes(Dst3);
 
   continue;
@@ -1120,7 +1120,7 @@
 // Perform the store.
 Bldr.takeNodes(*I);
 ExplodedNodeSet Dst3;
-evalStore(Dst3, U, U, *I, state, loc, Result);
+evalStore(Dst3, U, Ex, *I, state, loc, Result);
 Bldr.addNodes(Dst3);
   }
   Dst.insert(Dst2);
Index: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -21,16 +21,7 @@
 namespace ento {
 namespace {
 
-class CustomChecker : public Checker {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager ,
-BugReporter ) const {
-BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
-   "Custom diagnostic description",
-   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
-  }
-};
-
+template 
 class TestAction : public ASTFrontendAction {
   class DiagConsumer : public PathDiagnosticConsumer {
 llvm::raw_ostream 
@@ -59,23 +50,55 @@
 Compiler.getAnalyzerOpts()->CheckersControlList = {
 {"custom.CustomChecker", true}};
 AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry ) {
-  Registry.addChecker("custom.CustomChecker", "Description",
- "");
+  Registry.addChecker("custom.CustomChecker", "Description", "");
 });
 return std::move(AnalysisConsumer);
   }
 };
 
+template 
+bool runCheckerOnCode(const std::string , std::string ) {
+  llvm::raw_string_ostream OS(Diags);
+  return tooling::runToolOnCode(new TestAction(OS), Code);
+}
+template 
+bool runCheckerOnCode(const std::string ) {
+  std::string Diags;
+  return runCheckerOnCode(Code, Diags);
+}
+
+
+class CustomChecker : public Checker {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager ,
+BugReporter ) const {
+BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+   "Custom diagnostic description",
+   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  }
+};
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
-  {
-llvm::raw_string_ostream OS(Diags);
-EXPECT_TRUE(tooling::runToolOnCode(new TestAction(OS), "void f() {;}"));
-  }
+  EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
   EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description");
 }
 
+class LocIncDecChecker : public Checker {
+public:
+  void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+ CheckerContext ) const {
+auto UnaryOp = dyn_cast(S);
+if (UnaryOp && !IsLoad)
+  EXPECT_FALSE(UnaryOp->isIncrementOp());
+  }
+};
+
+TEST(RegisterCustomCheckers, CheckLocationIncDec) {
+  EXPECT_TRUE(
+  runCheckerOnCode("void f() { int *p; (*p)++; }"));
+}
+
 }
 }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

I tried adding isGLValue to evalStore and the test suite didn't complain. For 
evalLoad (on BoundEx) it failed in pretty much every test. Should the evalStore 
assert also go into trunk?

Since the analyzer behavior itself remains unchanged, I don't believe any other 
checker should be affected.

For myself it was pretty obvious that there is something weird going on when I 
didn't get the expected Stmt in my checker callback. So I added the following 
workaround. With this patch, the workaround is safely skipped.

The only change that this patch might cause "in the wild" is when someone used 
the Stmt as location, but didn't test with IncDecOps. However, as far as I can 
tell this should only have positive outcome.

Do I have any other means to check if other checkers were affected than to run 
the test suite?

  // Workaround for an inconsistency with IncDecOps: The statement is not the 
location expr.
  if (auto unaryE = dyn_cast(S))
  {
if (unaryE->isIncrementDecrementOp())
{
  S = unaryE->getSubExpr();
}
  }


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55701/new/

https://reviews.llvm.org/D55701



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 180487.
r.stahl added a comment.

rebased


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55701/new/

https://reviews.llvm.org/D55701

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -21,16 +21,7 @@
 namespace ento {
 namespace {
 
-class CustomChecker : public Checker {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager ,
-BugReporter ) const {
-BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
-   "Custom diagnostic description",
-   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
-  }
-};
-
+template 
 class TestAction : public ASTFrontendAction {
   class DiagConsumer : public PathDiagnosticConsumer {
 llvm::raw_ostream 
@@ -59,23 +50,55 @@
 Compiler.getAnalyzerOpts()->CheckersControlList = {
 {"custom.CustomChecker", true}};
 AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry ) {
-  Registry.addChecker("custom.CustomChecker", "Description",
- "");
+  Registry.addChecker("custom.CustomChecker", "Description", "");
 });
 return std::move(AnalysisConsumer);
   }
 };
 
+template 
+bool runCheckerOnCode(const std::string , std::string ) {
+  llvm::raw_string_ostream OS(Diags);
+  return tooling::runToolOnCode(new TestAction(OS), Code);
+}
+template 
+bool runCheckerOnCode(const std::string ) {
+  std::string Diags;
+  return runCheckerOnCode(Code, Diags);
+}
+
+
+class CustomChecker : public Checker {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager ,
+BugReporter ) const {
+BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+   "Custom diagnostic description",
+   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  }
+};
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
-  {
-llvm::raw_string_ostream OS(Diags);
-EXPECT_TRUE(tooling::runToolOnCode(new TestAction(OS), "void f() {;}"));
-  }
+  EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
   EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description");
 }
 
+class LocIncDecChecker : public Checker {
+public:
+  void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+ CheckerContext ) const {
+auto UnaryOp = dyn_cast(S);
+if (UnaryOp && !IsLoad)
+  EXPECT_FALSE(UnaryOp->isIncrementOp());
+  }
+};
+
+TEST(RegisterCustomCheckers, CheckLocationIncDec) {
+  EXPECT_TRUE(
+  runCheckerOnCode("void f() { int *p; (*p)++; }"));
+}
+
 }
 }
 }
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1052,7 +1052,7 @@
   // Perform the store, so that the uninitialized value detection happens.
   Bldr.takeNodes(*I);
   ExplodedNodeSet Dst3;
-  evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+  evalStore(Dst3, U, Ex, *I, state, loc, V2_untested);
   Bldr.addNodes(Dst3);
 
   continue;
@@ -1120,7 +1120,7 @@
 // Perform the store.
 Bldr.takeNodes(*I);
 ExplodedNodeSet Dst3;
-evalStore(Dst3, U, U, *I, state, loc, Result);
+evalStore(Dst3, U, Ex, *I, state, loc, Result);
 Bldr.addNodes(Dst3);
   }
   Dst.insert(Dst2);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.
Herald added subscribers: dkrupp, donat.nagy, Szelethus, mikhail.ramalho, 
baloghadamsoftware.

Please let me know if there is anything else that should be addressed.

Note that even though this diff is quite large, most is just renaming things to 
stay consistent.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46421/new/

https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

cfe-dev thread with short discussion: 
http://lists.llvm.org/pipermail/cfe-dev/2018-April/057562.html


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55701/new/

https://reviews.llvm.org/D55701



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: NoQ, dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

The LocationE parameter of evalStore is documented as "The location expression 
that is stored to". When storing from an increment / decrement operator this 
was not satisfied. In user code this causes an inconsistency between the SVal 
and Stmt parameters of checkLocation.


Repository:
  rC Clang

https://reviews.llvm.org/D55701

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -21,16 +21,7 @@
 namespace ento {
 namespace {
 
-class CustomChecker : public Checker {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager ,
-BugReporter ) const {
-BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
-   "Custom diagnostic description",
-   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
-  }
-};
-
+template 
 class TestAction : public ASTFrontendAction {
   class DiagConsumer : public PathDiagnosticConsumer {
 llvm::raw_ostream 
@@ -59,22 +50,55 @@
 Compiler.getAnalyzerOpts()->CheckersControlList = {
 {"custom.CustomChecker", true}};
 AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry ) {
-  Registry.addChecker("custom.CustomChecker", "Description");
+  Registry.addChecker("custom.CustomChecker", "Description");
 });
 return std::move(AnalysisConsumer);
   }
 };
 
+template 
+bool runCheckerOnCode(const std::string , std::string ) {
+  llvm::raw_string_ostream OS(Diags);
+  return tooling::runToolOnCode(new TestAction(OS), Code);
+}
+template 
+bool runCheckerOnCode(const std::string ) {
+  std::string Diags;
+  return runCheckerOnCode(Code, Diags);
+}
+
+
+class CustomChecker : public Checker {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager ,
+BugReporter ) const {
+BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+   "Custom diagnostic description",
+   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  }
+};
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
-  {
-llvm::raw_string_ostream OS(Diags);
-EXPECT_TRUE(tooling::runToolOnCode(new TestAction(OS), "void f() {;}"));
-  }
+  EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
   EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description");
 }
 
+class LocIncDecChecker : public Checker {
+public:
+  void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+ CheckerContext ) const {
+auto UnaryOp = dyn_cast(S);
+if (UnaryOp && !IsLoad)
+  EXPECT_FALSE(UnaryOp->isIncrementOp());
+  }
+};
+
+TEST(RegisterCustomCheckers, CheckLocationIncDec) {
+  EXPECT_TRUE(
+  runCheckerOnCode("void f() { int *p; (*p)++; }"));
+}
+
 }
 }
 }
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1052,7 +1052,7 @@
   // Perform the store, so that the uninitialized value detection happens.
   Bldr.takeNodes(*I);
   ExplodedNodeSet Dst3;
-  evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+  evalStore(Dst3, U, Ex, *I, state, loc, V2_untested);
   Bldr.addNodes(Dst3);
 
   continue;
@@ -1120,7 +1120,7 @@
 // Perform the store.
 Bldr.takeNodes(*I);
 ExplodedNodeSet Dst3;
-evalStore(Dst3, U, U, *I, state, loc, Result);
+evalStore(Dst3, U, Ex, *I, state, loc, Result);
 Bldr.addNodes(Dst3);
   }
   Dst.insert(Dst2);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

@rsmith Yes, this should generally better be handled outside of the ASTContext. 
That would be out of scope of this patch. Is it fine to proceed with this one 
for now?

For my usage scenario it actually is convenient this way. In my checker I use 
getParents, but if I would replace this with some tooling functions I wouldn't 
know when the maps become invalid since the invalidation happens on a lower 
level within the analyzer. I'd have to search the parents on every callback 
invocation which would be a massive performance issue.

Once an external parent utility exists, we would still need some invalidation 
notification. If anyone uses the analyzer and parent maps he'd want to know 
when they become invalid for performance reasons. Ideally only when necessary 
like the invalidateParents calls in this patch.


https://reviews.llvm.org/D46940



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC336523: [ASTImporter] import FunctionDecl end locations 
(authored by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48941?vs=154215=154549#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48941

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2554,16 +2554,16 @@
D->isInlineSpecified(),
FromConversion->isExplicit(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else if (auto *Method = dyn_cast(D)) {
 ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
cast(DC),
InnerLocStart,
NameInfo, T, TInfo,
Method->getStorageClass(),
Method->isInlineSpecified(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else {
 ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
   InnerLocStart,
@@ -2580,6 +2580,7 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2554,16 +2554,16 @@
D->isInlineSpecified(),
FromConversion->isExplicit(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else if (auto *Method = dyn_cast(D)) {
 ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
cast(DC),
InnerLocStart,
NameInfo, T, TInfo,
Method->getStorageClass(),
Method->isInlineSpecified(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else {
 ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
   InnerLocStart,
@@ -2580,6 +2580,7 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 154215.
r.stahl marked 2 inline comments as done.
r.stahl added a comment.

Alright, but then I would suggest to pass an invalid loc to the constructors 
instead to make it more explicit and save a map lookup in the import function.


Repository:
  rC Clang

https://reviews.llvm.org/D48941

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2563,16 +2563,16 @@
D->isInlineSpecified(),
FromConversion->isExplicit(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else if (auto *Method = dyn_cast(D)) {
 ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
cast(DC),
InnerLocStart,
NameInfo, T, TInfo,
Method->getStorageClass(),
Method->isInlineSpecified(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else {
 ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
   InnerLocStart,
@@ -2589,6 +2589,7 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2563,16 +2563,16 @@
D->isInlineSpecified(),
FromConversion->isExplicit(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else if (auto *Method = dyn_cast(D)) {
 ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
cast(DC),
InnerLocStart,
NameInfo, T, TInfo,
Method->getStorageClass(),
Method->isInlineSpecified(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else {
 ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
   InnerLocStart,
@@ -2589,6 +2589,7 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In https://reviews.llvm.org/D47698#1141871, @r.stahl wrote:

> improved code quality; added nested macro test. it "works", but is disabled 
> because it revealed another bug: the function end location is not imported. 
> will send a patch


Related to this.


Repository:
  rC Clang

https://reviews.llvm.org/D48941



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: martong, a.sidorin, balazske, xazax.hun.
Herald added subscribers: cfe-commits, rnkovacs.

On constructors that do not take the end source location, it was not imported. 
Fixes test from https://reviews.llvm.org/D47698 / 
https://reviews.llvm.org/rC336269.


Repository:
  rC Clang

https://reviews.llvm.org/D48941

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2533,6 +2533,7 @@
 D->isInlineSpecified(), 
 D->isImplicit(),
 D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
 if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
   SmallVector CtorInitializers;
   for (auto *I : FromConstructor->inits()) {
@@ -2555,6 +2556,7 @@
NameInfo, T, TInfo,
D->isInlineSpecified(),
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {
 ToFunction = CXXConversionDecl::Create(Importer.getToContext(), 
cast(DC),
@@ -2580,6 +2582,7 @@
   D->isInlineSpecified(),
   D->hasWrittenPrototype(),
   D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   }
 
   // Import the qualifier, if any.


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2533,6 +2533,7 @@
 D->isInlineSpecified(), 
 D->isImplicit(),
 D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
 if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
   SmallVector CtorInitializers;
   for (auto *I : FromConstructor->inits()) {
@@ -2555,6 +2556,7 @@
NameInfo, T, TInfo,
D->isInlineSpecified(),
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {
 ToFunction = CXXConversionDecl::Create(Importer.getToContext(), 
cast(DC),
@@ -2580,6 +2582,7 @@
   D->isInlineSpecified(),
   D->hasWrittenPrototype(),
   D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   }
 
   // Import the qualifier, if any.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48474: [analyzer][ctu] fix unsortable diagnostics

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC336275: [analyzer][ctu] fix unsortable diagnostics (authored 
by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48474?vs=152433=154107#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48474

Files:
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-hdr.h
  test/Analysis/ctu-main.cpp


Index: test/Analysis/ctu-hdr.h
===
--- test/Analysis/ctu-hdr.h
+++ test/Analysis/ctu-hdr.h
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();
Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -4,6 +4,8 @@
 // RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze 
-analyzer-checker=core,debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
 
+#include "ctu-hdr.h"
+
 void clang_analyzer_eval(int);
 
 int f(int);
@@ -41,6 +43,7 @@
 }
 
 int fun_using_anon_struct(int);
+int other_macro_diag(int);
 
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@
 
   clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
   clang_analyzer_eval(fun_using_anon_struct(8) == 8); // 
expected-warning{{TRUE}}
+
+  clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
+  // expected-warning@Inputs/ctu-other.cpp:75{{REACHABLE}}
+  MACRODIAG(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/Inputs/externalFnMap.txt
===
--- test/Analysis/Inputs/externalFnMap.txt
+++ test/Analysis/Inputs/externalFnMap.txt
@@ -12,3 +12,4 @@
 c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
 c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
 c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+c:@F@other_macro_diag#I# ctu-other.cpp.ast
Index: test/Analysis/Inputs/ctu-other.cpp
===
--- test/Analysis/Inputs/ctu-other.cpp
+++ test/Analysis/Inputs/ctu-other.cpp
@@ -1,3 +1,5 @@
+#include "../ctu-hdr.h"
+
 int callback_to_main(int x);
 int f(int x) {
   return x - 1;
@@ -68,3 +70,8 @@
 
 typedef struct { int n; } Anonymous;
 int fun_using_anon_struct(int n) { Anonymous anon; anon.n = n; return anon.n; }
+
+int other_macro_diag(int x) {
+  MACRODIAG();
+  return x;
+}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -406,11 +406,15 @@
   std::pair InSameTU = SM.isInTheSameTranslationUnit(XOffs, YOffs);
   if (InSameTU.first)
 return XL.isBeforeInTranslationUnitThan(YL);
-  const FileEntry *XFE = SM.getFileEntryForID(XL.getFileID());
-  const FileEntry *YFE = SM.getFileEntryForID(YL.getFileID());
+  const FileEntry *XFE = SM.getFileEntryForID(XL.getSpellingLoc().getFileID());
+  const FileEntry *YFE = SM.getFileEntryForID(YL.getSpellingLoc().getFileID());
   if (!XFE || !YFE)
 return XFE && !YFE;
-  return XFE->getName() < YFE->getName();
+  int NameCmp = XFE->getName().compare(YFE->getName());
+  if (NameCmp != 0)
+return NameCmp == -1;
+  // Last resort: Compare raw file IDs that are possibly expansions.
+  return XL.getFileID() < YL.getFileID();
 }
 
 static bool compare(const PathDiagnostic , const PathDiagnostic ) {


Index: test/Analysis/ctu-hdr.h
===
--- test/Analysis/ctu-hdr.h
+++ test/Analysis/ctu-hdr.h
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();
Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -4,6 +4,8 @@
 // RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir -verify %s
 
+#include "ctu-hdr.h"
+
 void clang_analyzer_eval(int);
 
 int f(int);
@@ -41,6 +43,7 @@
 }
 
 int fun_using_anon_struct(int);
+int other_macro_diag(int);
 
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@
 
   clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
   clang_analyzer_eval(fun_using_anon_struct(8) == 8); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
+  // 

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
r.stahl marked an inline comment as done.
Closed by commit rC336269: [ASTImporter] import macro source locations 
(authored by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47698?vs=152633=154103#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47698

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7164,61 +7164,70 @@
 return {};
 
   SourceManager  = FromContext.getSourceManager();
-  
-  // For now, map everything down to its file location, so that we
-  // don't have to import macro expansions.
-  // FIXME: Import macro expansions!
-  FromLoc = FromSM.getFileLoc(FromLoc);
+
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  SourceManager  = ToContext.getSourceManager();
   FileID ToFileID = Import(Decomposed.first);
   if (ToFileID.isInvalid())
 return {};
-  SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
-   .getLocWithOffset(Decomposed.second);
-  return ret;
+  SourceManager  = ToContext.getSourceManager();
+  return ToSM.getComposedLoc(ToFileID, Decomposed.second);
 }
 
 SourceRange ASTImporter::Import(SourceRange FromRange) {
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
 FileID ASTImporter::Import(FileID FromID) {
-  llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+  llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
-  
+
   SourceManager  = FromContext.getSourceManager();
   SourceManager  = ToContext.getSourceManager();
   const SrcMgr::SLocEntry  = FromSM.getSLocEntry(FromID);
-  assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-  
-  // Include location of this file.
-  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-  
-  // Map the FileID for to the "to" source manager.
+
+  // Map the FromID to the "to" source manager.
   FileID ToID;
-  const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-// FIXME: We probably want to use getVirtualFile(), so we don't hit the
-// disk again
-// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-// than mmap the files several times.
-const FileEntry *Entry = ToFileManager.getFile(Cache->OrigEntry->getName());
-if (!Entry)
-  return {};
-ToID = ToSM.createFileID(Entry, ToIncludeLoc, 
- FromSLoc.getFile().getFileCharacteristic());
+  if (FromSLoc.isExpansion()) {
+const SrcMgr::ExpansionInfo  = FromSLoc.getExpansion();
+SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc());
+SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());
+unsigned TokenLen = FromSM.getFileIDSize(FromID);
+SourceLocation MLoc;
+if (FromEx.isMacroArgExpansion()) {
+  MLoc = ToSM.createMacroArgExpansionLoc(ToSpLoc, ToExLocS, TokenLen);
+} else {
+  SourceLocation ToExLocE = Import(FromEx.getExpansionLocEnd());
+  MLoc = ToSM.createExpansionLoc(ToSpLoc, ToExLocS, ToExLocE, TokenLen,
+ FromEx.isExpansionTokenRange());
+}
+ToID = ToSM.getFileID(MLoc);
   } else {
-// FIXME: We want to re-use the existing MemoryBuffer!
-const llvm::MemoryBuffer *
-FromBuf = Cache->getBuffer(FromContext.getDiagnostics(), FromSM);
-std::unique_ptr ToBuf
-  = llvm::MemoryBuffer::getMemBufferCopy(FromBuf->getBuffer(),
- FromBuf->getBufferIdentifier());
-ToID = ToSM.createFileID(std::move(ToBuf),
- FromSLoc.getFile().getFileCharacteristic());
+// Include location of this file.
+SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
+
+const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
+if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+  // FIXME: We probably want to use getVirtualFile(), so we don't hit the
+  // disk again
+  // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
+  // than mmap the files several times.
+  const FileEntry *Entry =
+  ToFileManager.getFile(Cache->OrigEntry->getName());
+  if (!Entry)
+return {};
+  ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+   FromSLoc.getFile().getFileCharacteristic());
+} else {
+  // FIXME: We want to re-use the existing MemoryBuffer!
+  const llvm::MemoryBuffer *FromBuf =
+  Cache->getBuffer(FromContext.getDiagnostics(), FromSM);
+  std::unique_ptr ToBuf =
+  

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

@rsmith do you have a chance to take a look or assign someone else?


https://reviews.llvm.org/D46940



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl marked an inline comment as done.
r.stahl added inline comments.



Comment at: lib/AST/ASTImporter.cpp:7058
+const SrcMgr::ExpansionInfo  = FromSLoc.getExpansion();
+SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc());
+SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());

a.sidorin wrote:
> r.stahl wrote:
> > martong wrote:
> > > Let's say we import a `SourceLocation` with 
> > > `ASTImporter::Import(SourceLocation FromLoc)`.
> > > That calls into `ASTImporter::Import(FileID FromID)` where we again 
> > > import other source locations.
> > > Could the initial `FromLoc` be equal with any of these locations 
> > > (`FromEx.getSpellingLoc()`, `FromEx.getExpansionLocStart()`) ?
> > > My understanding is that this is not possible because we cannot have 
> > > recursive macros, but please confirm.
> > Yes, that was my understanding as well. If some compiler error is a macro 
> > expansion it eventually stops at some point while walking this chain.
> If we have a macro referring another macro in the same file, will we import 
> their FileID twice? First, during `Import(getSpellingLoc())` and then in this 
> caller?
That is taken care of by the ImportedFileIDs check.


Repository:
  rC Clang

https://reviews.llvm.org/D47698



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 152633.
r.stahl marked 5 inline comments as done.
r.stahl added a comment.

improved code quality; added nested macro test. it "works", but is disabled 
because it revealed another bug: the function end location is not imported. 
will send a patch


Repository:
  rC Clang

https://reviews.llvm.org/D47698

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1518,6 +1518,66 @@
 ToTU, cxxRecordDecl(unless(isImplicit();
 }
 
+static void CompareSourceLocs(FullSourceLoc Loc1, FullSourceLoc Loc2) {
+  EXPECT_EQ(Loc1.getExpansionLineNumber(), Loc2.getExpansionLineNumber());
+  EXPECT_EQ(Loc1.getExpansionColumnNumber(), Loc2.getExpansionColumnNumber());
+  EXPECT_EQ(Loc1.getSpellingLineNumber(), Loc2.getSpellingLineNumber());
+  EXPECT_EQ(Loc1.getSpellingColumnNumber(), Loc2.getSpellingColumnNumber());
+}
+static void CompareSourceRanges(SourceRange Range1, SourceRange Range2,
+SourceManager , SourceManager ) {
+  CompareSourceLocs(FullSourceLoc{ Range1.getBegin(), SM1 },
+FullSourceLoc{ Range2.getBegin(), SM2 });
+  CompareSourceLocs(FullSourceLoc{ Range1.getEnd(), SM1 },
+FullSourceLoc{ Range2.getEnd(), SM2 });
+}
+TEST_P(ASTImporterTestBase, ImportSourceLocs) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  #define MFOO(arg) arg = arg + 1
+
+  void foo() {
+int a = 5;
+MFOO(a);
+  }
+  )",
+  Lang_CXX);
+  auto FromD = FirstDeclMatcher().match(FromTU, functionDecl());
+  auto ToD = Import(FromD, Lang_CXX);
+
+  auto ToLHS = LastDeclMatcher().match(ToD, declRefExpr());
+  auto FromLHS = LastDeclMatcher().match(FromTU, declRefExpr());
+  auto ToRHS = LastDeclMatcher().match(ToD, integerLiteral());
+  auto FromRHS =
+  LastDeclMatcher().match(FromTU, integerLiteral());
+
+  SourceManager  = ToAST->getASTContext().getSourceManager();
+  SourceManager  = FromD->getASTContext().getSourceManager();
+  CompareSourceRanges(ToD->getSourceRange(), FromD->getSourceRange(), ToSM,
+  FromSM);
+  CompareSourceRanges(ToLHS->getSourceRange(), FromLHS->getSourceRange(), ToSM,
+  FromSM);
+  CompareSourceRanges(ToRHS->getSourceRange(), FromRHS->getSourceRange(), ToSM,
+  FromSM);
+}
+
+TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  #define FUNC_INT void declToImport
+  #define FUNC FUNC_INT
+  FUNC(int a);
+  )",
+  Lang_CXX);
+  auto FromD = FirstDeclMatcher().match(FromTU, functionDecl());
+  auto ToD = Import(FromD, Lang_CXX);
+
+  SourceManager  = ToAST->getASTContext().getSourceManager();
+  SourceManager  = FromD->getASTContext().getSourceManager();
+  CompareSourceRanges(ToD->getSourceRange(), FromD->getSourceRange(), ToSM,
+  FromSM);
+}
+
 TEST_P(
 ASTImporterTestBase,
 ImportDefinitionOfClassTemplateSpecIfThereIsAnExistingFwdDeclAndDefinition)
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7029,61 +7029,70 @@
 return {};
 
   SourceManager  = FromContext.getSourceManager();
-  
-  // For now, map everything down to its file location, so that we
-  // don't have to import macro expansions.
-  // FIXME: Import macro expansions!
-  FromLoc = FromSM.getFileLoc(FromLoc);
+
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  SourceManager  = ToContext.getSourceManager();
   FileID ToFileID = Import(Decomposed.first);
   if (ToFileID.isInvalid())
 return {};
-  SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
-   .getLocWithOffset(Decomposed.second);
-  return ret;
+  SourceManager  = ToContext.getSourceManager();
+  return ToSM.getComposedLoc(ToFileID, Decomposed.second);
 }
 
 SourceRange ASTImporter::Import(SourceRange FromRange) {
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
 FileID ASTImporter::Import(FileID FromID) {
-  llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+  llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
-  
+
   SourceManager  = FromContext.getSourceManager();
   SourceManager  = ToContext.getSourceManager();
   const SrcMgr::SLocEntry  = FromSM.getSLocEntry(FromID);
-  assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-  
-  // Include location of this file.
-  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-  
-  // Map the FileID for to the "to" source manager.
+
+  // Map the FromID to the "to" source manager.
   FileID ToID;
-  const 

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In https://reviews.llvm.org/D47698#1140629, @thakis wrote:

> This code is live when reading pchs, correct? Does this have any measurable 
> perf impact on deserializing pchs for, say, Cocoa.h or Windows.h?


I don't know for sure, but it should be used - yes. I have not measured a 
possible performance impact. Do you have a suggestion how I could do this on a 
Linux setup?

Note that I did not implement this as nice-to-have feature, but for fixing a 
concrete issue: https://reviews.llvm.org/D26054#1085413




Comment at: unittests/AST/ASTImporterTest.cpp:1534
+}
+TEST_P(ASTImporterTestBase, ImportSourceLocs) {
+  Decl *FromTU = getTuDecl(

balazske wrote:
> This test causes every case for expansion (macro, macro arg) to be executed 
> at import?
Yes, the last DeclRef will be "arg" on the RHS which is a macro argument 
expansion and the last IntegerLiteral will be the "1" which is a non-argument 
macro expansion.


Repository:
  rC Clang

https://reviews.llvm.org/D47698



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 152436.
r.stahl marked 5 inline comments as done.
r.stahl added a comment.

addressed review comment, but there is nothing like FullSourceRange to improve 
everything


Repository:
  rC Clang

https://reviews.llvm.org/D47698

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1518,6 +1518,49 @@
 ToTU, cxxRecordDecl(unless(isImplicit();
 }
 
+static void CompareSourceLocs(FullSourceLoc Loc1, FullSourceLoc Loc2) {
+  EXPECT_EQ(Loc1.getExpansionLineNumber(), Loc2.getExpansionLineNumber());
+  EXPECT_EQ(Loc1.getExpansionColumnNumber(), Loc2.getExpansionColumnNumber());
+  EXPECT_EQ(Loc1.getSpellingLineNumber(), Loc2.getSpellingLineNumber());
+  EXPECT_EQ(Loc1.getSpellingColumnNumber(), Loc2.getSpellingColumnNumber());
+}
+static void CompareSourceRanges(SourceRange Range1, SourceRange Range2,
+SourceManager , SourceManager ) {
+  CompareSourceLocs(FullSourceLoc{ Range1.getBegin(), SM1 },
+FullSourceLoc{ Range2.getBegin(), SM2 });
+  CompareSourceLocs(FullSourceLoc{ Range1.getEnd(), SM1 },
+FullSourceLoc{ Range2.getEnd(), SM2 });
+}
+TEST_P(ASTImporterTestBase, ImportSourceLocs) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  #define MFOO(arg) arg = arg + 1
+
+  void foo() {
+int a = 5;
+MFOO(a);
+  }
+  )",
+  Lang_CXX);
+  auto FromD = FirstDeclMatcher().match(FromTU, functionDecl());
+  auto ToD = Import(FromD, Lang_CXX);
+
+  auto ToLHS = LastDeclMatcher().match(ToD, declRefExpr());
+  auto FromLHS = LastDeclMatcher().match(FromTU, declRefExpr());
+  auto ToRHS = LastDeclMatcher().match(ToD, integerLiteral());
+  auto FromRHS =
+  LastDeclMatcher().match(FromTU, integerLiteral());
+
+  SourceManager  = ToAST->getASTContext().getSourceManager();
+  SourceManager  = FromD->getASTContext().getSourceManager();
+  CompareSourceRanges(ToD->getSourceRange(), FromD->getSourceRange(), ToSM,
+  FromSM);
+  CompareSourceRanges(ToLHS->getSourceRange(), FromLHS->getSourceRange(), ToSM,
+  FromSM);
+  CompareSourceRanges(ToRHS->getSourceRange(), FromRHS->getSourceRange(), ToSM,
+  FromSM);
+}
+
 TEST_P(
 ASTImporterTestBase,
 ImportDefinitionOfClassTemplateSpecIfThereIsAnExistingFwdDeclAndDefinition)
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7029,61 +7029,70 @@
 return {};
 
   SourceManager  = FromContext.getSourceManager();
-  
-  // For now, map everything down to its file location, so that we
-  // don't have to import macro expansions.
-  // FIXME: Import macro expansions!
-  FromLoc = FromSM.getFileLoc(FromLoc);
+
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  SourceManager  = ToContext.getSourceManager();
   FileID ToFileID = Import(Decomposed.first);
   if (ToFileID.isInvalid())
 return {};
-  SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
-   .getLocWithOffset(Decomposed.second);
-  return ret;
+  SourceManager  = ToContext.getSourceManager();
+  return ToSM.getComposedLoc(ToFileID, Decomposed.second);
 }
 
 SourceRange ASTImporter::Import(SourceRange FromRange) {
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
 FileID ASTImporter::Import(FileID FromID) {
-  llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+  llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
-  
+
   SourceManager  = FromContext.getSourceManager();
   SourceManager  = ToContext.getSourceManager();
   const SrcMgr::SLocEntry  = FromSM.getSLocEntry(FromID);
-  assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-  
-  // Include location of this file.
-  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-  
+
   // Map the FileID for to the "to" source manager.
   FileID ToID;
-  const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-// FIXME: We probably want to use getVirtualFile(), so we don't hit the
-// disk again
-// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-// than mmap the files several times.
-const FileEntry *Entry = ToFileManager.getFile(Cache->OrigEntry->getName());
-if (!Entry)
-  return {};
-ToID = ToSM.createFileID(Entry, ToIncludeLoc, 
- FromSLoc.getFile().getFileCharacteristic());
+  if (FromSLoc.isExpansion()) {
+const SrcMgr::ExpansionInfo  = FromSLoc.getExpansion();
+SourceLocation 

[PATCH] D48474: [analyzer][ctu] fix unsortable diagnostics

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In https://reviews.llvm.org/D30691#879838, @xazax.hun wrote:

> In https://reviews.llvm.org/D30691#878830, @r.stahl wrote:
>
> > For my purposes I replaced the return statement of the 
> > compareCrossTUSourceLocs function with:
> >
> >   return XL.getFileID() < YL.getFileID();
> >
> >
> > A more correct fix would create only one unique diagnostic for both cases.
>
>
> Thank you for the report, I could reproduce this after modifying the null 
> dereference checker. My fear of using file IDs is that I don't know whether 
> they are stable. So in subsequent runs, different paths might be chosen by 
> the analyzer and this could be confusing to the user.  I will think about a 
> workaround that both stable and solves this assertion.
>
> I see two possible ways to do the proper fix. One is to check explicitly for 
> this case when the same function appears in multiple translation units. A 
> better approach would be to have the ASTImporter handle this case. I think 
> the proper fix is better addressed in a separate patch.


Here is my improper fix with test case. Since CTU is still an experimental 
feature and this is a rare case to encounter I believe it's okay to risk 
confusing the user, rather than keep it broken.

Unfortunately I will not have the time to work on a proper fix.


Repository:
  rC Clang

https://reviews.llvm.org/D48474



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48474: [analyzer][ctu] fix unsortable diagnostics

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: xazax.hun, NoQ, dcoughlin.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, rnkovacs, 
szepet.
Herald added a reviewer: george.karpenkov.

In the provided test case the PathDiagnostic compare function was not able to 
find a difference.


Repository:
  rC Clang

https://reviews.llvm.org/D48474

Files:
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-hdr.h
  test/Analysis/ctu-main.cpp


Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -4,6 +4,8 @@
 // RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze 
-analyzer-checker=core,debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
 
+#include "ctu-hdr.h"
+
 void clang_analyzer_eval(int);
 
 int f(int);
@@ -41,6 +43,7 @@
 }
 
 int fun_using_anon_struct(int);
+int other_macro_diag(int);
 
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@
 
   clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
   clang_analyzer_eval(fun_using_anon_struct(8) == 8); // 
expected-warning{{TRUE}}
+
+  clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
+  // expected-warning@Inputs/ctu-other.cpp:75{{REACHABLE}}
+  MACRODIAG(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/ctu-hdr.h
===
--- /dev/null
+++ test/Analysis/ctu-hdr.h
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();
Index: test/Analysis/Inputs/externalFnMap.txt
===
--- test/Analysis/Inputs/externalFnMap.txt
+++ test/Analysis/Inputs/externalFnMap.txt
@@ -12,3 +12,4 @@
 c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
 c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
 c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+c:@F@other_macro_diag#I# ctu-other.cpp.ast
Index: test/Analysis/Inputs/ctu-other.cpp
===
--- test/Analysis/Inputs/ctu-other.cpp
+++ test/Analysis/Inputs/ctu-other.cpp
@@ -1,3 +1,5 @@
+#include "../ctu-hdr.h"
+
 int callback_to_main(int x);
 int f(int x) {
   return x - 1;
@@ -68,3 +70,8 @@
 
 typedef struct { int n; } Anonymous;
 int fun_using_anon_struct(int n) { Anonymous anon; anon.n = n; return anon.n; }
+
+int other_macro_diag(int x) {
+  MACRODIAG();
+  return x;
+}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -406,11 +406,15 @@
   std::pair InSameTU = SM.isInTheSameTranslationUnit(XOffs, YOffs);
   if (InSameTU.first)
 return XL.isBeforeInTranslationUnitThan(YL);
-  const FileEntry *XFE = SM.getFileEntryForID(XL.getFileID());
-  const FileEntry *YFE = SM.getFileEntryForID(YL.getFileID());
+  const FileEntry *XFE = SM.getFileEntryForID(XL.getSpellingLoc().getFileID());
+  const FileEntry *YFE = SM.getFileEntryForID(YL.getSpellingLoc().getFileID());
   if (!XFE || !YFE)
 return XFE && !YFE;
-  return XFE->getName() < YFE->getName();
+  int NameCmp = XFE->getName().compare(YFE->getName());
+  if (NameCmp != 0)
+return NameCmp == -1;
+  // Last resort: Compare raw file IDs that are possibly expansions.
+  return XL.getFileID() < YL.getFileID();
 }
 
 static bool compare(const PathDiagnostic , const PathDiagnostic ) {


Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -4,6 +4,8 @@
 // RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir -verify %s
 
+#include "ctu-hdr.h"
+
 void clang_analyzer_eval(int);
 
 int f(int);
@@ -41,6 +43,7 @@
 }
 
 int fun_using_anon_struct(int);
+int other_macro_diag(int);
 
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@
 
   clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
   clang_analyzer_eval(fun_using_anon_struct(8) == 8); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
+  // expected-warning@Inputs/ctu-other.cpp:75{{REACHABLE}}
+  MACRODIAG(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/ctu-hdr.h
===
--- /dev/null
+++ 

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/AST/ASTImporter.cpp:7058
+const SrcMgr::ExpansionInfo  = FromSLoc.getExpansion();
+SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc());
+SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());

martong wrote:
> Let's say we import a `SourceLocation` with 
> `ASTImporter::Import(SourceLocation FromLoc)`.
> That calls into `ASTImporter::Import(FileID FromID)` where we again import 
> other source locations.
> Could the initial `FromLoc` be equal with any of these locations 
> (`FromEx.getSpellingLoc()`, `FromEx.getExpansionLocStart()`) ?
> My understanding is that this is not possible because we cannot have 
> recursive macros, but please confirm.
Yes, that was my understanding as well. If some compiler error is a macro 
expansion it eventually stops at some point while walking this chain.



Comment at: unittests/AST/ASTImporterTest.cpp:1521
 
+static void CompareSourceLocs(SourceLocation Loc1, SourceLocation Loc2,
+  SourceManager , SourceManager ) {

martong wrote:
> Perhaps it would be more concise and less error prone to use a 
> `FullSourceLoc` which wraps one simple `SourceLocation` and a `SourceManager`.
Will do, thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D47698



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 151138.
r.stahl added a comment.

remove stray include


Repository:
  rC Clang

https://reviews.llvm.org/D47698

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1518,6 +1518,50 @@
 ToTU, cxxRecordDecl(unless(isImplicit();
 }
 
+static void CompareSourceLocs(SourceLocation Loc1, SourceLocation Loc2,
+  SourceManager , SourceManager ) {
+  EXPECT_EQ(SM1.getExpansionLineNumber(Loc1), SM2.getExpansionLineNumber(Loc2));
+  EXPECT_EQ(SM1.getExpansionColumnNumber(Loc1),
+SM2.getExpansionColumnNumber(Loc2));
+  EXPECT_EQ(SM1.getSpellingLineNumber(Loc1), SM2.getSpellingLineNumber(Loc2));
+  EXPECT_EQ(SM1.getSpellingColumnNumber(Loc1),
+SM2.getSpellingColumnNumber(Loc2));
+}
+static void CompareSourceRanges(SourceRange Range1, SourceRange Range2,
+SourceManager , SourceManager ) {
+  CompareSourceLocs(Range1.getBegin(), Range2.getBegin(), SM1, SM2);
+  CompareSourceLocs(Range1.getEnd(), Range2.getEnd(), SM1, SM2);
+}
+TEST_P(ASTImporterTestBase, ImportSourceLocs) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  #define MFOO(arg) arg = arg + 1
+
+  void foo() {
+int a = 5;
+MFOO(a);
+  }
+  )",
+  Lang_CXX);
+  auto FromD = FirstDeclMatcher().match(FromTU, functionDecl());
+  auto ToD = Import(FromD, Lang_CXX);
+
+  auto ToLHS = LastDeclMatcher().match(ToD, declRefExpr());
+  auto FromLHS = LastDeclMatcher().match(FromTU, declRefExpr());
+  auto ToRHS = LastDeclMatcher().match(ToD, integerLiteral());
+  auto FromRHS =
+  LastDeclMatcher().match(FromTU, integerLiteral());
+
+  SourceManager  = ToAST->getASTContext().getSourceManager();
+  SourceManager  = FromD->getASTContext().getSourceManager();
+  CompareSourceRanges(ToD->getSourceRange(), FromD->getSourceRange(), ToSM,
+  FromSM);
+  CompareSourceRanges(ToLHS->getSourceRange(), FromLHS->getSourceRange(), ToSM,
+  FromSM);
+  CompareSourceRanges(ToRHS->getSourceRange(), FromRHS->getSourceRange(), ToSM,
+  FromSM);
+}
+
 TEST_P(
 ASTImporterTestBase,
 ImportDefinitionOfClassTemplateSpecIfThereIsAnExistingFwdDeclAndDefinition)
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7029,61 +7029,70 @@
 return {};
 
   SourceManager  = FromContext.getSourceManager();
-  
-  // For now, map everything down to its file location, so that we
-  // don't have to import macro expansions.
-  // FIXME: Import macro expansions!
-  FromLoc = FromSM.getFileLoc(FromLoc);
+
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  SourceManager  = ToContext.getSourceManager();
   FileID ToFileID = Import(Decomposed.first);
   if (ToFileID.isInvalid())
 return {};
-  SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
-   .getLocWithOffset(Decomposed.second);
-  return ret;
+  SourceManager  = ToContext.getSourceManager();
+  return ToSM.getComposedLoc(ToFileID, Decomposed.second);
 }
 
 SourceRange ASTImporter::Import(SourceRange FromRange) {
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
 FileID ASTImporter::Import(FileID FromID) {
-  llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+  llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
-  
+
   SourceManager  = FromContext.getSourceManager();
   SourceManager  = ToContext.getSourceManager();
   const SrcMgr::SLocEntry  = FromSM.getSLocEntry(FromID);
-  assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-  
-  // Include location of this file.
-  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-  
+
   // Map the FileID for to the "to" source manager.
   FileID ToID;
-  const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-// FIXME: We probably want to use getVirtualFile(), so we don't hit the
-// disk again
-// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-// than mmap the files several times.
-const FileEntry *Entry = ToFileManager.getFile(Cache->OrigEntry->getName());
-if (!Entry)
-  return {};
-ToID = ToSM.createFileID(Entry, ToIncludeLoc, 
- FromSLoc.getFile().getFileCharacteristic());
+  if (FromSLoc.isExpansion()) {
+const SrcMgr::ExpansionInfo  = FromSLoc.getExpansion();
+SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc());
+SourceLocation ToExLocS = 

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 149959.
r.stahl added a comment.

- add missing AnalysisConsumer diff
- only collect const qualified vardecls in the extdef index and adjust test


https://reviews.llvm.org/D46421

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalDefMap.txt
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp
  test/CMakeLists.txt
  test/lit.cfg.py
  tools/CMakeLists.txt
  tools/clang-extdef-mapping/CMakeLists.txt
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
  tools/clang-func-mapping/CMakeLists.txt
  tools/clang-func-mapping/ClangFnMapGen.cpp
  tools/scan-build-py/README.md
  tools/scan-build-py/libscanbuild/__init__.py
  tools/scan-build-py/libscanbuild/analyze.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-build-py/libscanbuild/clang.py
  tools/scan-build-py/tests/unit/test_analyze.py
  tools/scan-build-py/tests/unit/test_clang.py

Index: tools/scan-build-py/tests/unit/test_clang.py
===
--- tools/scan-build-py/tests/unit/test_clang.py
+++ tools/scan-build-py/tests/unit/test_clang.py
@@ -96,7 +96,7 @@
 
 class ClangIsCtuCapableTest(unittest.TestCase):
 def test_ctu_not_found(self):
-is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping')
+is_ctu = sut.is_ctu_capable('not-found-clang-extdef-mapping')
 self.assertFalse(is_ctu)
 
 
Index: tools/scan-build-py/tests/unit/test_analyze.py
===
--- tools/scan-build-py/tests/unit/test_analyze.py
+++ tools/scan-build-py/tests/unit/test_analyze.py
@@ -349,14 +349,14 @@
 class MergeCtuMapTest(unittest.TestCase):
 
 def test_no_map_gives_empty(self):
-pairs = sut.create_global_ctu_function_map([])
+pairs = sut.create_global_ctu_extdef_map([])
 self.assertFalse(pairs)
 
 def test_multiple_maps_merged(self):
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun3#I# ast/fun3.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
 self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs)
@@ -366,7 +366,7 @@
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun1#I# ast/fun7.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
@@ -376,28 +376,28 @@
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun1#I# ast/fun1.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
 self.assertEqual(2, len(pairs))
 
 def test_space_handled_in_source(self):
 concat_map = ['c:@F@fun1#I# ast/f un.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs)
 self.assertEqual(1, len(pairs))
 
 
-class FuncMapSrcToAstTest(unittest.TestCase):
+class ExtdefMapSrcToAstTest(unittest.TestCase):
 
 def test_empty_gives_empty(self):
-fun_ast_lst = sut.func_map_list_src_to_ast([])
+fun_ast_lst = sut.extdef_map_list_src_to_ast([])
 self.assertFalse(fun_ast_lst)
 
 def test_sources_to_asts(self):
 fun_src_lst = ['c:@F@f1#I# ' + os.path.join(os.sep + 'path', 'f1.c'),
'c:@F@f2#I# ' + os.path.join(os.sep + 'path', 'f2.c')]
-fun_ast_lst = sut.func_map_list_src_to_ast(fun_src_lst)
+fun_ast_lst = sut.extdef_map_list_src_to_ast(fun_src_lst)
 self.assertTrue('c:@F@f1#I# ' +
 os.path.join('ast', 'path', 'f1.c.ast')
 in fun_ast_lst)
@@ -408,7 +408,7 @@
 
 def test_spaces_handled(self):
 fun_src_lst = ['c:@F@f1#I# ' + 

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In https://reviews.llvm.org/D46421#1119098, @xazax.hun wrote:

> Sorry for the limited activity. Unfortunately, I have very little time 
> reviewing patches lately.


Thanks for getting around to it!

> I think we need to answer the following questions:
> 
> - Does this change affect the analysis of the constructors of global objects? 
> If so, how?

The intention was to only allow the analyzer to resolve constants of arrays, 
structs and integral values. It seems to me that construction would have to be 
symbolically executed at the correct point in time and cannot be retroactively 
looked up - as is done with the constants here.

> - Do we want to import non-const object's initializer expressions? The 
> analyzer might not end up using the value anyways.

It seems like a good idea to not do that, since non-const values are not used. 
It might become useful if we ever do some kind of straight line execution from 
static initialization to main.
However for structs it is enough if one of their fields is declared const.

> - How big can the index get with this modification for large projects?

That depends a lot on global usage of course. In LLVM for example there is 
little impact, but I don't have any numbers, it's taking too long. Taking care 
of the previous point helps here as well.

> Overall the direction looks good to me, this will be a very useful addition, 
> thanks for working on this.

I will prepare something to not store unnecessary entries in the index. I also 
noticed the AnalysisConsumer change has gone missing with my latest diff - will 
be fixed.


https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

The split-token case should be covered by this, because it takes the correct 
TokenLen and the isTokenRange flag. Other than that the split-token 
ExpansionInfo is indistinguishable.

What about loaded source locations? Do they need to be handled specifically or 
does this cover everything now?


Repository:
  rC Clang

https://reviews.llvm.org/D47698



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: a.sidorin, klimek, martong.
Herald added subscribers: cfe-commits, rnkovacs.

Implement full import of macro expansion info with spelling and expansion 
locations.


Repository:
  rC Clang

https://reviews.llvm.org/D47698

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1518,6 +1518,50 @@
 ToTU, cxxRecordDecl(unless(isImplicit();
 }
 
+static void CompareSourceLocs(SourceLocation Loc1, SourceLocation Loc2,
+  SourceManager , SourceManager ) {
+  EXPECT_EQ(SM1.getExpansionLineNumber(Loc1), SM2.getExpansionLineNumber(Loc2));
+  EXPECT_EQ(SM1.getExpansionColumnNumber(Loc1),
+SM2.getExpansionColumnNumber(Loc2));
+  EXPECT_EQ(SM1.getSpellingLineNumber(Loc1), SM2.getSpellingLineNumber(Loc2));
+  EXPECT_EQ(SM1.getSpellingColumnNumber(Loc1),
+SM2.getSpellingColumnNumber(Loc2));
+}
+static void CompareSourceRanges(SourceRange Range1, SourceRange Range2,
+SourceManager , SourceManager ) {
+  CompareSourceLocs(Range1.getBegin(), Range2.getBegin(), SM1, SM2);
+  CompareSourceLocs(Range1.getEnd(), Range2.getEnd(), SM1, SM2);
+}
+TEST_P(ASTImporterTestBase, ImportSourceLocs) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  #define MFOO(arg) arg = arg + 1
+
+  void foo() {
+int a = 5;
+MFOO(a);
+  }
+  )",
+  Lang_CXX);
+  auto FromD = FirstDeclMatcher().match(FromTU, functionDecl());
+  auto ToD = Import(FromD, Lang_CXX);
+
+  auto ToLHS = LastDeclMatcher().match(ToD, declRefExpr());
+  auto FromLHS = LastDeclMatcher().match(FromTU, declRefExpr());
+  auto ToRHS = LastDeclMatcher().match(ToD, integerLiteral());
+  auto FromRHS =
+  LastDeclMatcher().match(FromTU, integerLiteral());
+
+  SourceManager  = ToAST->getASTContext().getSourceManager();
+  SourceManager  = FromD->getASTContext().getSourceManager();
+  CompareSourceRanges(ToD->getSourceRange(), FromD->getSourceRange(), ToSM,
+  FromSM);
+  CompareSourceRanges(ToLHS->getSourceRange(), FromLHS->getSourceRange(), ToSM,
+  FromSM);
+  CompareSourceRanges(ToRHS->getSourceRange(), FromRHS->getSourceRange(), ToSM,
+  FromSM);
+}
+
 TEST_P(
 ASTImporterTestBase,
 ImportDefinitionOfClassTemplateSpecIfThereIsAnExistingFwdDeclAndDefinition)
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -52,6 +52,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Lex/Lexer.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
@@ -7029,61 +7030,70 @@
 return {};
 
   SourceManager  = FromContext.getSourceManager();
-  
-  // For now, map everything down to its file location, so that we
-  // don't have to import macro expansions.
-  // FIXME: Import macro expansions!
-  FromLoc = FromSM.getFileLoc(FromLoc);
+
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  SourceManager  = ToContext.getSourceManager();
   FileID ToFileID = Import(Decomposed.first);
   if (ToFileID.isInvalid())
 return {};
-  SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
-   .getLocWithOffset(Decomposed.second);
-  return ret;
+  SourceManager  = ToContext.getSourceManager();
+  return ToSM.getComposedLoc(ToFileID, Decomposed.second);
 }
 
 SourceRange ASTImporter::Import(SourceRange FromRange) {
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
 FileID ASTImporter::Import(FileID FromID) {
-  llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+  llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
-  
+
   SourceManager  = FromContext.getSourceManager();
   SourceManager  = ToContext.getSourceManager();
   const SrcMgr::SLocEntry  = FromSM.getSLocEntry(FromID);
-  assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-  
-  // Include location of this file.
-  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-  
+
   // Map the FileID for to the "to" source manager.
   FileID ToID;
-  const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-// FIXME: We probably want to use getVirtualFile(), so we don't hit the
-// disk again
-// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-// than mmap the files several times.
-const FileEntry *Entry = 

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333417: [analyzer] const init: handle non-explicit cases 
more accurately (authored by r.stahl, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D46823

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/initialization.c
  test/Analysis/initialization.cpp

Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
   // The array index has to be known.
   if (auto CI = R->getIndex().getAs()) {
 int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
-  return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+  return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
+  if (CAT->getSize().sle(i))
+return UndefinedVal();
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+  return svalBuilder.makeZeroVal(R->getElementType());
 
 if (const Expr *ElemInit = InitList->getInit(i))
   if (Optional V = svalBuilder.getConstantVal(ElemInit))
@@ -1715,11 +1724,15 @@
 // Either the record variable or the field has to be const qualified.
 if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
   if (const Expr *Init = VD->getInit())
-if (const auto *InitList = dyn_cast(Init))
-  if (Index < InitList->getNumInits())
+if (const auto *InitList = dyn_cast(Init)) {
+  if (Index < InitList->getNumInits()) {
 if (const Expr *FieldInit = InitList->getInit(Index))
   if (Optional V = svalBuilder.getConstantVal(FieldInit))
 return *V;
+  } else {
+return svalBuilder.makeZeroVal(Ty);
+  }
+}
   }
 
   return getBindingForFieldOrElementCommon(B, R, Ty);
Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
 
 void initbug() {
   const union { float a; } u = {};
   (void)u.a; // no-crash
 }
+
+int const parr[2] = {1};
+void constarr() {
+  int i = 2;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+  i = 1;
+  clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+  i = -1;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+  int a;
+  int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+  clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: test/Analysis/initialization.cpp
===
--- test/Analysis/initialization.cpp
+++ test/Analysis/initialization.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int a = 3;
+};
+S const sarr[2] = {};
+void definit() {
+  int i = 1;
+  // FIXME: Should recognize that it is 3.
+  clang_analyzer_eval(sarr[i].a); // expected-warning{{UNKNOWN}}
+}
+
+int const arr[2][2] = {};
+void arr2init() {
+  int i = 1;
+  // FIXME: Should recognize that it is 0.
+  clang_analyzer_eval(arr[i][0]); // expected-warning{{UNKNOWN}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2139
+  CXXRecordDecl *Injected = nullptr;
+  for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);

balazske wrote:
> r.stahl wrote:
> > The only thing I'm wondering is whether the Decls looked up here are always 
> > known to be already imported.
> These are in the 'To' context. It may be that the `Injected` is not found 
> here, probably because not yet imported (in this case the import may be part 
> of a not completed recursive process).
As far as I understand that corner case could be covered by doing the lookup on 
`DCXX` instead and then importing the injected decl. But then you wouldn't find 
it if it is only in the To context (if that is possible).

I mean if a user calls ImportDecl in another order specifically. But such a 
case is probably really artificial and I'm not sure if it's even makes sense or 
is already covered by ImportDeclParts.

It should be fine as it is.


Repository:
  rC Clang

https://reviews.llvm.org/D47450



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 148809.
r.stahl added a comment.

Added FIXME tests.

I know my example didn't exercise your scenario, but it was the only one I 
could think of where returning zero would have been straight up wrong. Note 
that I default initialized `a` to 3 there, so `sarr` is not just 
zero-initialized.

I do not have access yet, but applied today!


https://reviews.llvm.org/D46823

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/initialization.c
  test/Analysis/initialization.cpp

Index: test/Analysis/initialization.cpp
===
--- test/Analysis/initialization.cpp
+++ test/Analysis/initialization.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int a = 3;
+};
+S const sarr[2] = {};
+void definit() {
+  int i = 1;
+  // FIXME: Should recognize that it is 3.
+  clang_analyzer_eval(sarr[i].a); // expected-warning{{UNKNOWN}}
+}
+
+int const arr[2][2] = {};
+void arr2init() {
+  int i = 1;
+  // FIXME: Should recognize that it is 0.
+  clang_analyzer_eval(arr[i][0]); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
 
 void initbug() {
   const union { float a; } u = {};
   (void)u.a; // no-crash
 }
+
+int const parr[2] = {1};
+void constarr() {
+  int i = 2;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+  i = 1;
+  clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+  i = -1;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+  int a;
+  int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+  clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
   // The array index has to be known.
   if (auto CI = R->getIndex().getAs()) {
 int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
-  return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+  return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
+  if (CAT->getSize().sle(i))
+return UndefinedVal();
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+  return svalBuilder.makeZeroVal(R->getElementType());
 
 if (const Expr *ElemInit = InitList->getInit(i))
   if (Optional V = svalBuilder.getConstantVal(ElemInit))
@@ -1715,11 +1724,15 @@
 // Either the record variable or the field has to be const qualified.
 if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
   if (const Expr *Init = VD->getInit())
-if (const auto *InitList = dyn_cast(Init))
-  if (Index < InitList->getNumInits())
+if (const auto *InitList = dyn_cast(Init)) {
+  if (Index < InitList->getNumInits()) {
 if (const Expr *FieldInit = InitList->getInit(Index))
   if (Optional V = svalBuilder.getConstantVal(FieldInit))
 return *V;
+  } else {
+return svalBuilder.makeZeroVal(Ty);
+  }
+}
   }
 
   return getBindingForFieldOrElementCommon(B, R, Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl accepted this revision.
r.stahl added a comment.

LGTM!

I'm not really too confident to approve changes yet, but with a second opinion 
it should be fine.




Comment at: lib/AST/ASTImporter.cpp:2139
+  CXXRecordDecl *Injected = nullptr;
+  for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);

The only thing I'm wondering is whether the Decls looked up here are always 
known to be already imported.


Repository:
  rC Clang

https://reviews.llvm.org/D47450



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/AST/ASTImporter.cpp:6776
+  // been invalidated to avoid repeatedly calling this.
+  ToContext.invalidateParents();
+

martong wrote:
> Can an `Expr` has a parent too? If yes, why not invalidate the parents in 
> `Import(Expr*)` ?
> I am also a bit concerned to call the invalidation during the import of all 
> `Stmt` (and possible during the import of `Expr`s too).
> Isn't it enough to invalidate only during `Import(Decl*)`?
The Import(Expr) function just defers to Import(Stmt) so a call there would be 
redundant as Alexey commented.

I don't think it would be enough to invalidate in Import(Decl), because 
Import(Stmt) is part of the public API. I don't know if it makes sense to just 
import Stmts and if there is any user code doing that, but it is theoretically 
possible. In that case we should make Import(Stmt) and Import(Expr) private.

I wouldn't be too concerned about too many calls to invalidate, since the 
function doesn't do anything once the parent info has been released and it will 
only be rebuilt once someone calls ACtx::getParents - which is not the case 
while importing.


https://reviews.llvm.org/D46940



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 147268.
r.stahl marked 2 inline comments as done.
r.stahl added a comment.

addressed review comments


https://reviews.llvm.org/D46940

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1431,6 +1431,36 @@
   MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern));
 }
 
+TEST_P(ASTImporterTestBase, ValidParents) {
+  // Create parent cache in To context.
+  Decl *PreTU = getTuDecl("struct S {};", Lang_CXX, "pre.cc");
+  RecordDecl *FromRD = FirstDeclMatcher().match(PreTU, recordDecl());
+  auto ToRD = cast(Import(FromRD, Lang_CXX));
+  auto  = ToAST->getASTContext();
+  ToACtx.getParents(*ToRD);
+
+  Decl *FromTU = getTuDecl("void f() {}", Lang_CXX);
+  CompoundStmt *FromCS = FirstDeclMatcher().match(FromTU,
+compoundStmt());
+  FunctionDecl *FromFD = FirstDeclMatcher().match(FromTU,
+functionDecl());
+
+  auto FromPs = FromFD->getASTContext().getParents(*FromCS);
+  ASSERT_TRUE(!FromPs.empty());
+  auto FromP = FromPs[0].get();
+  EXPECT_EQ(FromP, FromFD);
+
+  auto ToFD = cast(Import(FromFD, Lang_CXX));
+
+  Decl *ToTU = ToACtx.getTranslationUnitDecl();
+  auto ToCS = FirstDeclMatcher().match(ToTU, compoundStmt());
+
+  auto ToPs = ToACtx.getParents(*ToCS);
+  ASSERT_TRUE(!ToPs.empty());
+  auto ToP = ToPs[0].get();
+  EXPECT_EQ(ToP, ToFD);
+}
+
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ASTImporterTestBase,
 ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -6686,6 +6686,8 @@
   if (!ToD)
 return nullptr;
 
+  ToContext.invalidateParents();
+
   // Record the imported declaration.
   ImportedDecls[FromD] = ToD;
   ToD->IdentifierNamespace = FromD->IdentifierNamespace;
@@ -6762,13 +6764,17 @@
   llvm::DenseMap::iterator Pos = ImportedStmts.find(FromS);
   if (Pos != ImportedStmts.end())
 return Pos->second;
-  
+
   // Import the type
   ASTNodeImporter Importer(*this);
   Stmt *ToS = Importer.Visit(FromS);
   if (!ToS)
 return nullptr;
 
+  // FIXME: We could add a separate function that assumes parents have already
+  // been invalidated to avoid repeatedly calling this.
+  ToContext.invalidateParents();
+
   // Record the imported declaration.
   ImportedStmts[FromS] = ToS;
   return ToS;
@@ -6779,52 +6785,60 @@
 return nullptr;
 
   NestedNameSpecifier *prefix = Import(FromNNS->getPrefix());
+  NestedNameSpecifier *ToNNS = nullptr;
 
   switch (FromNNS->getKind()) {
   case NestedNameSpecifier::Identifier:
 if (IdentifierInfo *II = Import(FromNNS->getAsIdentifier())) {
-  return NestedNameSpecifier::Create(ToContext, prefix, II);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, II);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::Namespace:
 if (auto *NS = 
 cast_or_null(Import(FromNNS->getAsNamespace( {
-  return NestedNameSpecifier::Create(ToContext, prefix, NS);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, NS);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::NamespaceAlias:
 if (auto *NSAD = 
   cast_or_null(Import(FromNNS->getAsNamespaceAlias( {
-  return NestedNameSpecifier::Create(ToContext, prefix, NSAD);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, NSAD);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::Global:
-return NestedNameSpecifier::GlobalSpecifier(ToContext);
+ToNNS = NestedNameSpecifier::GlobalSpecifier(ToContext);
+break;
 
   case NestedNameSpecifier::Super:
 if (auto *RD =
 cast_or_null(Import(FromNNS->getAsRecordDecl( {
-  return NestedNameSpecifier::SuperSpecifier(ToContext, RD);
+  ToNNS = NestedNameSpecifier::SuperSpecifier(ToContext, RD);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::TypeSpec:
   case NestedNameSpecifier::TypeSpecWithTemplate: {
   QualType T = Import(QualType(FromNNS->getAsType(), 0u));
   if (!T.isNull()) {
 bool bTemplate = FromNNS->getKind() == 
  NestedNameSpecifier::TypeSpecWithTemplate;
-return NestedNameSpecifier::Create(ToContext, prefix, 
+ToNNS = NestedNameSpecifier::Create(ToContext, prefix, 
bTemplate, T.getTypePtr());
   }
+  break;
 }
-  return nullptr;
+
+  default:
+llvm_unreachable("Invalid nested name specifier kind");
   }
 
-  llvm_unreachable("Invalid nested name specifier 

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/AST/ASTImporter.cpp:6755
+  auto ToE = cast_or_null(Import(cast(FromE)));
+  if (ToE)
+ToContext.invalidateParents();

a.sidorin wrote:
> We already do the invalidation in Import(Stmt), so it looks redundant here.
Yes, sorry this should actually be in Import(Decl) as I wrote in the 
description!


Repository:
  rC Clang

https://reviews.llvm.org/D46940



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: a.sidorin, klimek.
Herald added subscribers: cfe-commits, martong.

If an AST node is imported after a call to getParents in the ToCtx, it was no 
longer possible to retrieve its parents since the old results were cached.

Valid types for getParents:

- Decl, Stmt, NestedNameSpecifier: handled explicitly
- Type: not supported by ast importer
- TypeLoc: not imported
- NestedNameSpecifierLoc: calls import for NNS


Repository:
  rC Clang

https://reviews.llvm.org/D46940

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1431,6 +1431,36 @@
   MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern));
 }
 
+TEST_P(ASTImporterTestBase, ValidParents) {
+  // Create parent cache in To context.
+  Decl *PreTU = getTuDecl("struct S {};", Lang_CXX, "pre.cc");
+  RecordDecl *FromRD = FirstDeclMatcher().match(PreTU, recordDecl());
+  auto ToRD = cast(Import(FromRD, Lang_CXX));
+  auto  = ToAST->getASTContext();
+  ToACtx.getParents(*ToRD);
+
+  Decl *FromTU = getTuDecl("void f() {}", Lang_CXX);
+  CompoundStmt *FromCS = FirstDeclMatcher().match(FromTU,
+compoundStmt());
+  FunctionDecl *FromFD = FirstDeclMatcher().match(FromTU,
+functionDecl());
+
+  auto FromPs = FromFD->getASTContext().getParents(*FromCS);
+  ASSERT_TRUE(!FromPs.empty());
+  auto FromP = FromPs[0].get();
+  EXPECT_EQ(FromP, FromFD);
+
+  auto ToFD = cast(Import(FromFD, Lang_CXX));
+
+  Decl *ToTU = ToACtx.getTranslationUnitDecl();
+  auto ToCS = FirstDeclMatcher().match(ToTU, compoundStmt());
+
+  auto ToPs = ToACtx.getParents(*ToCS);
+  ASSERT_TRUE(!ToPs.empty());
+  auto ToP = ToPs[0].get();
+  EXPECT_EQ(ToP, ToFD);
+}
+
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ASTImporterTestBase,
 ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -6751,7 +6751,11 @@
   if (!FromE)
 return nullptr;
 
-  return cast_or_null(Import(cast(FromE)));
+  auto ToE = cast_or_null(Import(cast(FromE)));
+  if (ToE)
+ToContext.invalidateParents();
+
+  return ToE;
 }
 
 Stmt *ASTImporter::Import(Stmt *FromS) {
@@ -6762,13 +6766,15 @@
   llvm::DenseMap::iterator Pos = ImportedStmts.find(FromS);
   if (Pos != ImportedStmts.end())
 return Pos->second;
-  
+
   // Import the type
   ASTNodeImporter Importer(*this);
   Stmt *ToS = Importer.Visit(FromS);
   if (!ToS)
 return nullptr;
 
+  ToContext.invalidateParents();
+
   // Record the imported declaration.
   ImportedStmts[FromS] = ToS;
   return ToS;
@@ -6779,52 +6785,60 @@
 return nullptr;
 
   NestedNameSpecifier *prefix = Import(FromNNS->getPrefix());
+  NestedNameSpecifier *ToNNS = nullptr;
 
   switch (FromNNS->getKind()) {
   case NestedNameSpecifier::Identifier:
 if (IdentifierInfo *II = Import(FromNNS->getAsIdentifier())) {
-  return NestedNameSpecifier::Create(ToContext, prefix, II);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, II);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::Namespace:
 if (auto *NS = 
 cast_or_null(Import(FromNNS->getAsNamespace( {
-  return NestedNameSpecifier::Create(ToContext, prefix, NS);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, NS);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::NamespaceAlias:
 if (auto *NSAD = 
   cast_or_null(Import(FromNNS->getAsNamespaceAlias( {
-  return NestedNameSpecifier::Create(ToContext, prefix, NSAD);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, NSAD);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::Global:
-return NestedNameSpecifier::GlobalSpecifier(ToContext);
+ToNNS = NestedNameSpecifier::GlobalSpecifier(ToContext);
+break;
 
   case NestedNameSpecifier::Super:
 if (auto *RD =
 cast_or_null(Import(FromNNS->getAsRecordDecl( {
-  return NestedNameSpecifier::SuperSpecifier(ToContext, RD);
+  ToNNS = NestedNameSpecifier::SuperSpecifier(ToContext, RD);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::TypeSpec:
   case NestedNameSpecifier::TypeSpecWithTemplate: {
   QualType T = Import(QualType(FromNNS->getAsType(), 0u));
   if (!T.isNull()) {
 bool bTemplate = FromNNS->getKind() == 
  NestedNameSpecifier::TypeSpecWithTemplate;
-return NestedNameSpecifier::Create(ToContext, prefix, 
+ToNNS = NestedNameSpecifier::Create(ToContext, 

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())

NoQ wrote:
> NoQ wrote:
> > Would this work correctly if the element is not of an integral or 
> > enumeration type? I think this needs an explicit check.
> What if we have an out-of-bounds access to a variable-length array? I don't 
> think it'd yield zero.
I'm getting "variable-sized object may not be initialized", so this case should 
not be possible.



Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650-1652
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+  return svalBuilder.makeZeroVal(R->getElementType());

r.stahl wrote:
> NoQ wrote:
> > NoQ wrote:
> > > Would this work correctly if the element is not of an integral or 
> > > enumeration type? I think this needs an explicit check.
> > What if we have an out-of-bounds access to a variable-length array? I don't 
> > think it'd yield zero.
> I'm getting "variable-sized object may not be initialized", so this case 
> should not be possible.
I'm having a hard time reproducing this either.


```
struct S {
  int a = 3;
};
S const sarr[2] = {};
void definit() {
  int i = 1;
  clang_analyzer_dump(sarr[i].a); // expected-warning{{test}}
}
```

results in a symbolic value, because makeZeroVal returns an empty SVal list for 
arrays, records, vectors and complex types. Otherwise it just returns 
UnknownVal (for example for not-yet-implemented floats).

Can you think of a case where this would be an issue?


https://reviews.llvm.org/D46823



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 146809.
r.stahl marked 2 inline comments as done.
r.stahl added a comment.

updated test


https://reviews.llvm.org/D46823

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/initialization.c


Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze 
-analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
 
 void initbug() {
   const union { float a; } u = {};
   (void)u.a; // no-crash
 }
+
+int const parr[2] = {1};
+void constarr() {
+  int i = 2;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+  i = 1;
+  clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+  i = -1;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+  int a;
+  int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+  clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
   // The array index has to be known.
   if (auto CI = R->getIndex().getAs()) {
 int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
-  return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+  return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
+  if (CAT->getSize().sle(i))
+return UndefinedVal();
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+  return svalBuilder.makeZeroVal(R->getElementType());
 
 if (const Expr *ElemInit = InitList->getInit(i))
   if (Optional V = svalBuilder.getConstantVal(ElemInit))
@@ -1715,11 +1724,15 @@
 // Either the record variable or the field has to be const qualified.
 if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
   if (const Expr *Init = VD->getInit())
-if (const auto *InitList = dyn_cast(Init))
-  if (Index < InitList->getNumInits())
+if (const auto *InitList = dyn_cast(Init)) {
+  if (Index < InitList->getNumInits()) {
 if (const Expr *FieldInit = InitList->getInit(Index))
   if (Optional V = svalBuilder.getConstantVal(FieldInit))
 return *V;
+  } else {
+return svalBuilder.makeZeroVal(Ty);
+  }
+}
   }
 
   return getBindingForFieldOrElementCommon(B, R, Ty);


Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
 
 void initbug() {
   const union { float a; } u = {};
   (void)u.a; // no-crash
 }
+
+int const parr[2] = {1};
+void constarr() {
+  int i = 2;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+  i = 1;
+  clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+  i = -1;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+  int a;
+  int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+  clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
   // The array index has to be known.
   if (auto CI = R->getIndex().getAs()) {
 int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
-  return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+  return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: include/clang/CrossTU/CrossTranslationUnit.h:114
+  llvm::Expected
+  getCrossTUDefinition(const VarDecl *VD, StringRef CrossTUDir,
+   StringRef IndexName);

xazax.hun wrote:
> I wonder if we need these overloads. Maybe having only the templated version 
> and having a static assert for the supported types is better? Asking the 
> other reviewers as well.
They are not required, but I thought they make the interface more clear and 
help prevent implementation in headers.



Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:354
+if (!VD->hasExternalStorage() ||
+VD->getAnyInitializer() != nullptr)
+  return true;

xazax.hun wrote:
> Will this work for zero initialized globals (i.e. not having an initializer 
> expression) that are defined in the current translation unit? It would be 
> great to have a test case for that.
If a variable has a definition in the current TU, it may not have another one 
else where, but you're correct that this case will continue here. It will 
however only result in a failed lookup.

Similar but backwards: If an extern var is defined in another TU with global 
zero init.

Overall I'd say that default initializing a constant to zero is pretty uncommon 
(and invalid in C++), so that in my opinion it's fine to not support it for 
now. It seems like there is no transparent way to get the initializer including 
that case or am I missing something?


https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 146789.
r.stahl marked 3 inline comments as done.
r.stahl edited the summary of this revision.
r.stahl added a comment.
Herald added subscribers: whisperity, mgorny.

I looked through the original patches and found quite a few more occurrences of 
"function map" and renamed them - including the tool "clang-func-mapping". 
There is one comment "by using the 'clang-extdef-mapping' packaged with Xcode 
(on OS X)". Is this implicit from the install targets that the tool name 
changed or do I have to inform someone?


https://reviews.llvm.org/D46421

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalDefMap.txt
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp
  test/CMakeLists.txt
  test/lit.cfg.py
  tools/CMakeLists.txt
  tools/clang-extdef-mapping/CMakeLists.txt
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
  tools/clang-func-mapping/CMakeLists.txt
  tools/clang-func-mapping/ClangFnMapGen.cpp
  tools/scan-build-py/README.md
  tools/scan-build-py/libscanbuild/__init__.py
  tools/scan-build-py/libscanbuild/analyze.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-build-py/libscanbuild/clang.py
  tools/scan-build-py/tests/unit/test_analyze.py
  tools/scan-build-py/tests/unit/test_clang.py

Index: tools/scan-build-py/tests/unit/test_clang.py
===
--- tools/scan-build-py/tests/unit/test_clang.py
+++ tools/scan-build-py/tests/unit/test_clang.py
@@ -96,7 +96,7 @@
 
 class ClangIsCtuCapableTest(unittest.TestCase):
 def test_ctu_not_found(self):
-is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping')
+is_ctu = sut.is_ctu_capable('not-found-clang-extdef-mapping')
 self.assertFalse(is_ctu)
 
 
Index: tools/scan-build-py/tests/unit/test_analyze.py
===
--- tools/scan-build-py/tests/unit/test_analyze.py
+++ tools/scan-build-py/tests/unit/test_analyze.py
@@ -349,14 +349,14 @@
 class MergeCtuMapTest(unittest.TestCase):
 
 def test_no_map_gives_empty(self):
-pairs = sut.create_global_ctu_function_map([])
+pairs = sut.create_global_ctu_extdef_map([])
 self.assertFalse(pairs)
 
 def test_multiple_maps_merged(self):
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun3#I# ast/fun3.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
 self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs)
@@ -366,7 +366,7 @@
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun1#I# ast/fun7.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
@@ -376,28 +376,28 @@
 concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
   'c:@F@fun2#I# ast/fun2.c.ast',
   'c:@F@fun1#I# ast/fun1.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
 self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
 self.assertEqual(2, len(pairs))
 
 def test_space_handled_in_source(self):
 concat_map = ['c:@F@fun1#I# ast/f un.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
 self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs)
 self.assertEqual(1, len(pairs))
 
 
-class FuncMapSrcToAstTest(unittest.TestCase):
+class ExtdefMapSrcToAstTest(unittest.TestCase):
 
 def test_empty_gives_empty(self):
-fun_ast_lst = sut.func_map_list_src_to_ast([])
+fun_ast_lst = sut.extdef_map_list_src_to_ast([])
 self.assertFalse(fun_ast_lst)
 
 def test_sources_to_asts(self):
 fun_src_lst = ['c:@F@f1#I# ' + os.path.join(os.sep + 'path', 'f1.c'),
'c:@F@f2#I# ' + os.path.join(os.sep + 'path', 'f2.c')]
-fun_ast_lst = sut.func_map_list_src_to_ast(fun_src_lst)

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 146609.
r.stahl marked an inline comment as done.
r.stahl added a comment.

- added tests
- updated doc and var naming
- addressed review comment


https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp

Index: test/Analysis/func-mapping-test.cpp
===
--- test/Analysis/func-mapping-test.cpp
+++ test/Analysis/func-mapping-test.cpp
@@ -3,5 +3,16 @@
 int f(int) {
   return 0;
 }
+// CHECK-DAG: c:@F@f#I#
 
-// CHECK: c:@F@f#I#
+int x = 5;
+// CHECK-DAG: c:@x
+
+struct S {
+  int a;
+};
+S s = {.a = 2};
+// CHECK-DAG: c:@s
+
+int arr[5] = { 0, 1 };
+// CHECK-DAG: c:@arr
Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -42,6 +42,15 @@
 
 int fun_using_anon_struct(int);
 
+extern const int extInt;
+namespace intns {
+extern const int extInt;
+}
+struct S {
+  int a;
+};
+extern const S extS;
+
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
   clang_analyzer_eval(f(4) == 3); // expected-warning{{TRUE}}
@@ -58,4 +67,8 @@
 
   clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
   clang_analyzer_eval(fun_using_anon_struct(8) == 8); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(extInt == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(intns::extInt == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(extS.a == 4); // expected-warning{{TRUE}}
 }
Index: test/Analysis/Inputs/externalFnMap.txt
===
--- test/Analysis/Inputs/externalFnMap.txt
+++ test/Analysis/Inputs/externalFnMap.txt
@@ -12,3 +12,6 @@
 c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
 c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
 c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+c:@extInt ctu-other.cpp.ast
+c:@N@intns@extInt ctu-other.cpp.ast
+c:@extS ctu-other.cpp.ast
Index: test/Analysis/Inputs/ctu-other.cpp
===
--- test/Analysis/Inputs/ctu-other.cpp
+++ test/Analysis/Inputs/ctu-other.cpp
@@ -68,3 +68,12 @@
 
 typedef struct { int n; } Anonymous;
 int fun_using_anon_struct(int n) { Anonymous anon; anon.n = n; return anon.n; }
+
+extern const int extInt = 2;
+namespace intns {
+extern const int extInt = 3;
+}
+struct S {
+  int a;
+};
+extern const S extS = {.a = 4};
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -346,6 +346,27 @@
 return true;
   }
 
+  bool VisitVarDecl(VarDecl *VD) {
+if (!Opts->naiveCTUEnabled())
+  return true;
+
+if (!VD->hasExternalStorage() ||
+VD->getAnyInitializer() != nullptr)
+  return true;
+
+llvm::Expected CTUDeclOrError =
+  CTU.getCrossTUDefinition(VD, Opts->getCTUDir(), Opts->getCTUIndexName());
+
+if (!CTUDeclOrError) {
+  handleAllErrors(CTUDeclOrError.takeError(),
+  [&](const cross_tu::IndexError ) {
+CTU.emitCrossTUDiagnostics(IE);
+  });
+}
+
+return true;
+  }
+
   bool VisitFunctionDecl(FunctionDecl *FD) {
 IdentifierInfo *II = FD->getIdentifier();
 if (II && II->getName().startswith("__inline"))
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -87,14 +87,14 @@
 const size_t Pos = Line.find(" ");
 if (Pos > 0 && Pos != std::string::npos) {
   StringRef LineRef{Line};
-  StringRef FunctionLookupName = LineRef.substr(0, Pos);
-  if (Result.count(FunctionLookupName))
+  StringRef LookupName = LineRef.substr(0, Pos);
+  if (Result.count(LookupName))
 return llvm::make_error(
 index_error_code::multiple_definitions, IndexPath.str(), LineNo);
   StringRef FileName = LineRef.substr(Pos + 1);
   SmallString<256> FilePath = CrossTUDir;
   llvm::sys::path::append(FilePath, FileName);
-  Result[FunctionLookupName] = FilePath.str().str();
+  Result[LookupName] = FilePath.str().str();
 } else
   return llvm::make_error(
   index_error_code::invalid_index_format, IndexPath.str(), LineNo);
@@ -111,52 +111,65 @@
   return Result.str();
 }
 
+bool HasDefinition(const FunctionDecl *D, const FunctionDecl *) {
+  return D->hasBody(DefD);
+}
+bool HasDefinition(const VarDecl *D, const VarDecl *) {
+  return D->getAnyInitializer(DefD) != 

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: NoQ, xazax.hun, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.

If the access is out of bounds, return UndefinedVal. If it is missing an 
explicit init, return the implicit zero value it must have.


Repository:
  rC Clang

https://reviews.llvm.org/D46823

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/initialization.c


Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze 
-analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(int);
 
 void initbug() {
   const union { float a; } u = {};
   (void)u.a; // no-crash
 }
+
+int const parr[2] = {1};
+void constarr() {
+  int i = 2;
+  clang_analyzer_dump(parr[i]); // expected-warning{{Undefined}}
+  i = 1;
+  clang_analyzer_dump(parr[i]); // expected-warning{{0 S32b}}
+  i = -1;
+  clang_analyzer_dump(parr[i]); // expected-warning{{Undefined}}
+}
+
+struct SM {
+  int a;
+  int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+  clang_analyzer_dump(sm.a); // expected-warning{{1 S32b}}
+  clang_analyzer_dump(sm.b); // expected-warning{{0 S32b}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
   // The array index has to be known.
   if (auto CI = R->getIndex().getAs()) {
 int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
-  return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+  return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
+  if (CAT->getSize().sle(i))
+return UndefinedVal();
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+  return svalBuilder.makeZeroVal(R->getElementType());
 
 if (const Expr *ElemInit = InitList->getInit(i))
   if (Optional V = svalBuilder.getConstantVal(ElemInit))
@@ -1715,11 +1724,15 @@
 // Either the record variable or the field has to be const qualified.
 if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
   if (const Expr *Init = VD->getInit())
-if (const auto *InitList = dyn_cast(Init))
-  if (Index < InitList->getNumInits())
+if (const auto *InitList = dyn_cast(Init)) {
+  if (Index < InitList->getNumInits()) {
 if (const Expr *FieldInit = InitList->getInit(Index))
   if (Optional V = svalBuilder.getConstantVal(FieldInit))
 return *V;
+  } else {
+return svalBuilder.makeZeroVal(Ty);
+  }
+}
   }
 
   return getBindingForFieldOrElementCommon(B, R, Ty);


Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(int);
 
 void initbug() {
   const union { float a; } u = {};
   (void)u.a; // no-crash
 }
+
+int const parr[2] = {1};
+void constarr() {
+  int i = 2;
+  clang_analyzer_dump(parr[i]); // expected-warning{{Undefined}}
+  i = 1;
+  clang_analyzer_dump(parr[i]); // expected-warning{{0 S32b}}
+  i = -1;
+  clang_analyzer_dump(parr[i]); // expected-warning{{Undefined}}
+}
+
+struct SM {
+  int a;
+  int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+  clang_analyzer_dump(sm.a); // expected-warning{{1 S32b}}
+  clang_analyzer_dump(sm.b); // expected-warning{{0 S32b}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
   // The array index has to be known.
   if (auto CI = R->getIndex().getAs()) {
 int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
-  return UnknownVal();
+// If it is known that the index is out of bounds, we can 

[PATCH] D46633: [analyzer] add range check for InitList lookup

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: alexfh, NoQ, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.

Fixes issue introduced by https://reviews.llvm.org/rC331556.

Closes bug: https://bugs.llvm.org/show_bug.cgi?id=37357


Repository:
  rC Clang

https://reviews.llvm.org/D46633

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/initialization.c


Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -0,0 +1,8 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void initbug() {
+  const union { float a; } u = {};
+  (void)u.a; // no-crash
+}
+
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1711,13 +1711,15 @@
   if (const auto *VR = dyn_cast(superR)) {
 const VarDecl *VD = VR->getDecl();
 QualType RecordVarTy = VD->getType();
+unsigned Index = FD->getFieldIndex();
 // Either the record variable or the field has to be const qualified.
 if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
   if (const Expr *Init = VD->getInit())
 if (const auto *InitList = dyn_cast(Init))
-  if (const Expr *FieldInit = InitList->getInit(FD->getFieldIndex()))
-if (Optional V = svalBuilder.getConstantVal(FieldInit))
-  return *V;
+  if (Index < InitList->getNumInits())
+if (const Expr *FieldInit = InitList->getInit(Index))
+  if (Optional V = svalBuilder.getConstantVal(FieldInit))
+return *V;
   }
 
   return getBindingForFieldOrElementCommon(B, R, Ty);


Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -0,0 +1,8 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void initbug() {
+  const union { float a; } u = {};
+  (void)u.a; // no-crash
+}
+
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1711,13 +1711,15 @@
   if (const auto *VR = dyn_cast(superR)) {
 const VarDecl *VD = VR->getDecl();
 QualType RecordVarTy = VD->getType();
+unsigned Index = FD->getFieldIndex();
 // Either the record variable or the field has to be const qualified.
 if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
   if (const Expr *Init = VD->getInit())
 if (const auto *InitList = dyn_cast(Init))
-  if (const Expr *FieldInit = InitList->getInit(FD->getFieldIndex()))
-if (Optional V = svalBuilder.getConstantVal(FieldInit))
-  return *V;
+  if (Index < InitList->getNumInits())
+if (const Expr *FieldInit = InitList->getInit(Index))
+  if (Optional V = svalBuilder.getConstantVal(FieldInit))
+return *V;
   }
 
   return getBindingForFieldOrElementCommon(B, R, Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 145660.
r.stahl added a comment.

Didn't see that overload, thanks!

I need someone to commit this for me.


https://reviews.llvm.org/D46115

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  test/Import/attr/Inputs/S.cpp
  test/Import/attr/test.cpp

Index: test/Import/attr/Inputs/S.cpp
===
--- test/Import/attr/Inputs/S.cpp
+++ test/Import/attr/Inputs/S.cpp
@@ -0,0 +1,13 @@
+extern void f() __attribute__((const));
+
+struct S {
+  struct {
+int a __attribute__((packed));
+  };
+};
+
+void stmt() {
+#pragma unroll
+  for (;;)
+;
+}
Index: test/Import/attr/test.cpp
===
--- test/Import/attr/test.cpp
+++ test/Import/attr/test.cpp
@@ -0,0 +1,26 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s | FileCheck %s
+// CHECK: FunctionDecl
+// CHECK-SAME: S.cpp:1:1, col:13
+// CHECK-NEXT: ConstAttr
+// CHECK-SAME: col:32
+
+// CHECK: IndirectFieldDecl
+// CHECK-NEXT: Field
+// CHECK-NEXT: Field
+// CHECK-NEXT: PackedAttr
+// CHECK-SAME: col:26
+
+// CHECK: AttributedStmt
+// CHECK-NEXT: LoopHintAttr
+// CHECK-SAME: line:10:9
+
+extern void f() __attribute__((const));
+
+struct S;
+
+void stmt();
+
+void expr() {
+  f();
+  struct S s;
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2646,8 +2646,8 @@
   Importer.getToContext(), DC, Loc, Name.getAsIdentifierInfo(), T,
   {NamedChain, D->getChainingSize()});
 
-  for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Attr->clone(Importer.getToContext()));
+  for (const auto *A : D->attrs())
+ToIndirectField->addAttr(Importer.Import(A));
 
   ToIndirectField->setAccess(D->getAccess());
   ToIndirectField->setLexicalDeclContext(LexicalDC);
@@ -4721,15 +4721,8 @@
   SourceLocation ToAttrLoc = Importer.Import(S->getAttrLoc());
   ArrayRef FromAttrs(S->getAttrs());
   SmallVector ToAttrs(FromAttrs.size());
-  ASTContext &_ToContext = Importer.getToContext();
-  std::transform(FromAttrs.begin(), FromAttrs.end(), ToAttrs.begin(),
-[&_ToContext](const Attr *A) -> const Attr * {
-  return A->clone(_ToContext);
-});
-  for (const auto *ToA : ToAttrs) {
-if (!ToA)
-  return nullptr;
-  }
+  if (ImportArrayChecked(FromAttrs, ToAttrs.begin()))
+return nullptr;
   Stmt *ToSubStmt = Importer.Import(S->getSubStmt());
   if (!ToSubStmt && S->getSubStmt())
 return nullptr;
@@ -6544,6 +6537,12 @@
Import(FromTSI->getTypeLoc().getLocStart()));
 }
 
+Attr *ASTImporter::Import(const Attr *FromAttr) {
+  Attr *ToAttr = FromAttr->clone(ToContext);
+  ToAttr->setRange(Import(FromAttr->getRange()));
+  return ToAttr;
+}
+
 Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
   llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
   if (Pos != ImportedDecls.end()) {
@@ -7199,8 +7198,8 @@
 
 Decl *ASTImporter::Imported(Decl *From, Decl *To) {
   if (From->hasAttrs()) {
-for (auto *FromAttr : From->getAttrs())
-  To->addAttr(FromAttr->clone(To->getASTContext()));
+for (const auto *FromAttr : From->getAttrs())
+  To->addAttr(Import(FromAttr));
   }
   if (From->isUsed()) {
 To->setIsUsed();
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -41,6 +41,7 @@
 class Stmt;
 class TagDecl;
 class TypeSourceInfo;
+class Attr;
 
   /// \brief Imports selected nodes from one AST context into another context,
   /// merging AST nodes where appropriate.
@@ -130,6 +131,12 @@
 /// context, or NULL if an error occurred.
 TypeSourceInfo *Import(TypeSourceInfo *FromTSI);
 
+/// \brief Import the given attribute from the "from" context into the
+/// "to" context.
+///
+/// \returns the equivalent attribute in the "to" context.
+Attr *Import(const Attr *FromAttr);
+
 /// \brief Import the given declaration from the "from" context into the 
 /// "to" context.
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: include/clang/AST/ASTImporter.h:137
+///
+/// \returns the equivalent attribute in the "to" context, or NULL if an
+/// error occurred.

a.sidorin wrote:
> nullptr
I tried to stay consistent with the other descriptions. Will be removed anyway 
(see below).



Comment at: lib/AST/ASTImporter.cpp:2650
+  for (const auto *A : D->attrs())
+ToIndirectField->addAttr(Importer.Import(const_cast(A)));
 

a.sidorin wrote:
> Could we just remove 'const' qualifier from `A` to avoid `const_cast`? (Same 
> below)
I made the interface const instead.



Comment at: lib/AST/ASTImporter.cpp:6547
+Attr *ASTImporter::Import(Attr *FromAttr) {
+  if (!FromAttr)
+return nullptr;

a.sidorin wrote:
> Is it possible to get into a situation where a nullptr attribute is imported?
I looked at the Decl variant and thought it should act similarly, but as far as 
I can tell, null attributes should not exist.


https://reviews.llvm.org/D46115



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 145641.
r.stahl marked 6 inline comments as done.

https://reviews.llvm.org/D46115

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  test/Import/attr/Inputs/S.cpp
  test/Import/attr/test.cpp

Index: test/Import/attr/Inputs/S.cpp
===
--- test/Import/attr/Inputs/S.cpp
+++ test/Import/attr/Inputs/S.cpp
@@ -0,0 +1,13 @@
+extern void f() __attribute__((const));
+
+struct S {
+  struct {
+int a __attribute__((packed));
+  };
+};
+
+void stmt() {
+#pragma unroll
+  for (;;)
+;
+}
Index: test/Import/attr/test.cpp
===
--- test/Import/attr/test.cpp
+++ test/Import/attr/test.cpp
@@ -0,0 +1,26 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s | FileCheck %s
+// CHECK: FunctionDecl
+// CHECK-SAME: S.cpp:1:1, col:13
+// CHECK-NEXT: ConstAttr
+// CHECK-SAME: col:32
+
+// CHECK: IndirectFieldDecl
+// CHECK-NEXT: Field
+// CHECK-NEXT: Field
+// CHECK-NEXT: PackedAttr
+// CHECK-SAME: col:26
+
+// CHECK: AttributedStmt
+// CHECK-NEXT: LoopHintAttr
+// CHECK-SAME: line:10:9
+
+extern void f() __attribute__((const));
+
+struct S;
+
+void stmt();
+
+void expr() {
+  f();
+  struct S s;
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2646,8 +2646,8 @@
   Importer.getToContext(), DC, Loc, Name.getAsIdentifierInfo(), T,
   {NamedChain, D->getChainingSize()});
 
-  for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Attr->clone(Importer.getToContext()));
+  for (const auto *A : D->attrs())
+ToIndirectField->addAttr(Importer.Import(A));
 
   ToIndirectField->setAccess(D->getAccess());
   ToIndirectField->setLexicalDeclContext(LexicalDC);
@@ -4721,15 +4721,8 @@
   SourceLocation ToAttrLoc = Importer.Import(S->getAttrLoc());
   ArrayRef FromAttrs(S->getAttrs());
   SmallVector ToAttrs(FromAttrs.size());
-  ASTContext &_ToContext = Importer.getToContext();
-  std::transform(FromAttrs.begin(), FromAttrs.end(), ToAttrs.begin(),
-[&_ToContext](const Attr *A) -> const Attr * {
-  return A->clone(_ToContext);
-});
-  for (const auto *ToA : ToAttrs) {
-if (!ToA)
-  return nullptr;
-  }
+  if (ImportArrayChecked(FromAttrs.begin(), FromAttrs.end(), ToAttrs.begin()))
+return nullptr;
   Stmt *ToSubStmt = Importer.Import(S->getSubStmt());
   if (!ToSubStmt && S->getSubStmt())
 return nullptr;
@@ -6544,6 +6537,12 @@
Import(FromTSI->getTypeLoc().getLocStart()));
 }
 
+Attr *ASTImporter::Import(const Attr *FromAttr) {
+  Attr *ToAttr = FromAttr->clone(ToContext);
+  ToAttr->setRange(Import(FromAttr->getRange()));
+  return ToAttr;
+}
+
 Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
   llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
   if (Pos != ImportedDecls.end()) {
@@ -7199,8 +7198,8 @@
 
 Decl *ASTImporter::Imported(Decl *From, Decl *To) {
   if (From->hasAttrs()) {
-for (auto *FromAttr : From->getAttrs())
-  To->addAttr(FromAttr->clone(To->getASTContext()));
+for (const auto *FromAttr : From->getAttrs())
+  To->addAttr(Import(FromAttr));
   }
   if (From->isUsed()) {
 To->setIsUsed();
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -41,6 +41,7 @@
 class Stmt;
 class TagDecl;
 class TypeSourceInfo;
+class Attr;
 
   /// \brief Imports selected nodes from one AST context into another context,
   /// merging AST nodes where appropriate.
@@ -130,6 +131,12 @@
 /// context, or NULL if an error occurred.
 TypeSourceInfo *Import(TypeSourceInfo *FromTSI);
 
+/// \brief Import the given attribute from the "from" context into the
+/// "to" context.
+///
+/// \returns the equivalent attribute in the "to" context.
+Attr *Import(const Attr *FromAttr);
+
 /// \brief Import the given declaration from the "from" context into the 
 /// "to" context.
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 145486.
r.stahl edited the summary of this revision.
r.stahl added a comment.

full patch with test


https://reviews.llvm.org/D46115

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  test/Import/attr/Inputs/S.cpp
  test/Import/attr/test.cpp

Index: test/Import/attr/Inputs/S.cpp
===
--- test/Import/attr/Inputs/S.cpp
+++ test/Import/attr/Inputs/S.cpp
@@ -0,0 +1,14 @@
+extern void f() __attribute__((const));
+
+struct S
+{
+struct {
+int a __attribute__((packed));
+};
+};
+
+void stmt()
+{
+#pragma unroll
+for (;;);
+}
Index: test/Import/attr/test.cpp
===
--- test/Import/attr/test.cpp
+++ test/Import/attr/test.cpp
@@ -0,0 +1,26 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s | FileCheck %s
+// CHECK: FunctionDecl
+// CHECK-SAME: S.cpp:1:1, col:13
+// CHECK-NEXT: ConstAttr
+// CHECK-SAME: col:32
+
+// CHECK: IndirectFieldDecl
+// CHECK-NEXT: Field
+// CHECK-NEXT: Field
+// CHECK-NEXT: PackedAttr
+// CHECK-SAME: col:30
+
+// CHECK: AttributedStmt
+// CHECK-NEXT: LoopHintAttr
+// CHECK-SAME: line:12:13
+
+extern void f() __attribute__((const));
+
+struct S;
+
+void stmt();
+
+void expr() {
+  f();
+  struct S s;
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2646,8 +2646,8 @@
   Importer.getToContext(), DC, Loc, Name.getAsIdentifierInfo(), T,
   {NamedChain, D->getChainingSize()});
 
-  for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Attr->clone(Importer.getToContext()));
+  for (const auto *A : D->attrs())
+ToIndirectField->addAttr(Importer.Import(const_cast(A)));
 
   ToIndirectField->setAccess(D->getAccess());
   ToIndirectField->setLexicalDeclContext(LexicalDC);
@@ -4721,10 +4721,9 @@
   SourceLocation ToAttrLoc = Importer.Import(S->getAttrLoc());
   ArrayRef FromAttrs(S->getAttrs());
   SmallVector ToAttrs(FromAttrs.size());
-  ASTContext &_ToContext = Importer.getToContext();
   std::transform(FromAttrs.begin(), FromAttrs.end(), ToAttrs.begin(),
-[&_ToContext](const Attr *A) -> const Attr * {
-  return A->clone(_ToContext);
+[this](const Attr *A) -> const Attr * {
+  return Importer.Import(const_cast(A));
 });
   for (const auto *ToA : ToAttrs) {
 if (!ToA)
@@ -6544,6 +6543,15 @@
Import(FromTSI->getTypeLoc().getLocStart()));
 }
 
+Attr *ASTImporter::Import(Attr *FromAttr) {
+  if (!FromAttr)
+return nullptr;
+
+  Attr *ToAttr = FromAttr->clone(ToContext);
+  ToAttr->setRange(Import(FromAttr->getRange()));
+  return ToAttr;
+}
+
 Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
   llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
   if (Pos != ImportedDecls.end()) {
@@ -7200,7 +7208,7 @@
 Decl *ASTImporter::Imported(Decl *From, Decl *To) {
   if (From->hasAttrs()) {
 for (auto *FromAttr : From->getAttrs())
-  To->addAttr(FromAttr->clone(To->getASTContext()));
+  To->addAttr(Import(FromAttr));
   }
   if (From->isUsed()) {
 To->setIsUsed();
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -41,6 +41,7 @@
 class Stmt;
 class TagDecl;
 class TypeSourceInfo;
+class Attr;
 
   /// \brief Imports selected nodes from one AST context into another context,
   /// merging AST nodes where appropriate.
@@ -130,6 +131,13 @@
 /// context, or NULL if an error occurred.
 TypeSourceInfo *Import(TypeSourceInfo *FromTSI);
 
+/// \brief Import the given attribute from the "from" context into the
+/// "to" context.
+///
+/// \returns the equivalent attribute in the "to" context, or NULL if an
+/// error occurred.
+Attr *Import(Attr *FromAttr);
+
 /// \brief Import the given declaration from the "from" context into the 
 /// "to" context.
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Tests and doc fixes pending. First I'm interested in your thoughts to this 
change.

It allows to bind more symbolic values to constants if they have been defined 
and initialized in another TU:

use.c:

  extern int * const p;
  extern struct S * const s;

def.c:

  int * const p = 0;
  struct S * const s = { /* complex init */ };


Repository:
  rC Clang

https://reviews.llvm.org/D46421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: NoQ, dcoughlin, xazax.hun, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.

The existing CTU mechanism imports FunctionDecls where the definition is 
available in another TU. This patch extends that to VarDecls, to bind more 
constants.

- Add VarDecl importing functionality to CrossTranslationUnitContext
- Import Decls while traversing them in AnalysisConsumer
- Add VarDecls to CTU external mappings generator


Repository:
  rC Clang

https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  tools/clang-func-mapping/ClangFnMapGen.cpp

Index: tools/clang-func-mapping/ClangFnMapGen.cpp
===
--- tools/clang-func-mapping/ClangFnMapGen.cpp
+++ tools/clang-func-mapping/ClangFnMapGen.cpp
@@ -54,6 +54,7 @@
 
 private:
   void handleDecl(const Decl *D);
+  void addIfInMain(const DeclaratorDecl *DD, SourceLocation defStart);
 
   ASTContext 
   llvm::StringMap Index;
@@ -65,35 +66,42 @@
 return;
 
   if (const auto *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
-  if (const Stmt *Body = FD->getBody()) {
-std::string LookupName = CrossTranslationUnitContext::getLookupName(FD);
-const SourceManager  = Ctx.getSourceManager();
-if (CurrentFileName.empty()) {
-  CurrentFileName =
-  SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
-  if (CurrentFileName.empty())
-CurrentFileName = "invalid_file";
-}
-
-switch (FD->getLinkageInternal()) {
-case ExternalLinkage:
-case VisibleNoLinkage:
-case UniqueExternalLinkage:
-  if (SM.isInMainFile(Body->getLocStart()))
-Index[LookupName] = CurrentFileName;
-default:
-  break;
-}
-  }
-}
+if (FD->isThisDeclarationADefinition())
+  if (const Stmt *Body = FD->getBody())
+addIfInMain(FD, Body->getLocStart());
+  } else if (const auto *VD = dyn_cast(D)) {
+if (VD->hasInit())
+  if (const Expr *Init = VD->getInit())
+addIfInMain(VD, Init->getLocStart());
   }
 
   if (const auto *DC = dyn_cast(D))
 for (const Decl *D : DC->decls())
   handleDecl(D);
 }
 
+void MapFunctionNamesConsumer::addIfInMain(const DeclaratorDecl *DD,
+   SourceLocation defStart) {
+  std::string LookupName = CrossTranslationUnitContext::getLookupName(DD);
+  const SourceManager  = Ctx.getSourceManager();
+  if (CurrentFileName.empty()) {
+CurrentFileName =
+SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+if (CurrentFileName.empty())
+  CurrentFileName = "invalid_file";
+  }
+
+  switch (DD->getLinkageInternal()) {
+  case ExternalLinkage:
+  case VisibleNoLinkage:
+  case UniqueExternalLinkage:
+if (SM.isInMainFile(defStart))
+  Index[LookupName] = CurrentFileName;
+  default:
+break;
+  }
+}
+
 class MapFunctionNamesAction : public ASTFrontendAction {
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -346,6 +346,27 @@
 return true;
   }
 
+  bool VisitVarDecl(VarDecl *VD) {
+if (!Opts->naiveCTUEnabled())
+  return true;
+
+if (!VD->hasExternalStorage() ||
+VD->getAnyInitializer() != nullptr)
+  return true;
+
+llvm::Expected CTUDeclOrError =
+  CTU.getCrossTUDefinition(VD, Opts->getCTUDir(), Opts->getCTUIndexName());
+
+if (!CTUDeclOrError) {
+  handleAllErrors(CTUDeclOrError.takeError(),
+  [&](const cross_tu::IndexError ) {
+CTU.emitCrossTUDiagnostics(IE);
+  });
+}
+
+return true;
+  }
+
   bool VisitFunctionDecl(FunctionDecl *FD) {
 IdentifierInfo *II = FD->getIdentifier();
 if (II && II->getName().startswith("__inline"))
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -111,52 +111,65 @@
   return Result.str();
 }
 
+bool HasDefinition(const FunctionDecl *D, const FunctionDecl *) {
+  return D->hasBody(DefD);
+}
+bool HasDefinition(const VarDecl *D, const VarDecl *) {
+  return D->getAnyInitializer(DefD) != nullptr;
+}
+template  bool HasDefinition(const T *D) {
+  const T *Unused;
+  return HasDefinition(D, Unused);
+}
+
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance )
 : CI(CI), Context(CI.getASTContext()) {}
 
 

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1723
+if (const InitListExpr *InitList = dyn_cast(Init)) {
+  if (const Expr *FieldInit = InitList->getInit(FD->getFieldIndex())) {
+if (Optional V = svalBuilder.getConstantVal(FieldInit))

NoQ wrote:
> This method crashes when the index is out of range. Not sure - is it possible 
> to skip a few fields in the list? Would the list's AST automatically contain 
> any placeholders in this case?
Good catch, but I don't think this is an issue. When testing InitListExpr with 
missing inits, they get filled with ImplicitValueInitExpr.


https://reviews.llvm.org/D45774



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 145156.
r.stahl marked 3 inline comments as done.
r.stahl added a comment.

addressed the comments, thanks!

If it looks good, please commit for me.


https://reviews.llvm.org/D45774

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  test/Analysis/globals.cpp

Index: test/Analysis/globals.cpp
===
--- test/Analysis/globals.cpp
+++ test/Analysis/globals.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+
+static const unsigned long long scull = 0;
+void static_int()
+{
+*(int*)scull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull = 0;
+void const_int()
+{
+*(int*)cull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+static int * const spc = 0;
+void static_ptr()
+{
+*spc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc = 0;
+void const_ptr()
+{
+*pc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull_nonnull = 4;
+void nonnull_int()
+{
+*(int*)(cull_nonnull - 4) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc_nonnull = (int*)sizeof(int);
+void nonnull_ptr()
+{
+*(pc_nonnull - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const constcast = const_cast((int*)sizeof(int));
+void cast1()
+{
+*(constcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const recast = reinterpret_cast(sizeof(int));
+void cast2()
+{
+*(recast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const staticcast = static_cast((int*)sizeof(int));
+void cast3()
+{
+*(staticcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct Foo { int a; };
+Foo * const dyncast = dynamic_cast((Foo*)sizeof(Foo));
+void cast4()
+{
+// Do not handle dynamic_cast for now, because it may change the pointer value.
+(dyncast - 1)->a = 0; // no-warning
+}
+
+typedef int * const intptrconst;
+int * const funccast = intptrconst(sizeof(int));
+void cast5()
+{
+*(funccast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S1
+{
+int * p;
+};
+const S1 s1 = {
+.p = (int*)sizeof(int)
+};
+void conststruct()
+{
+*(s1.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S2
+{
+int * const p;
+};
+S2 s2 = {
+.p = (int*)sizeof(int)
+};
+void constfield()
+{
+*(s2.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const parr[1] = { (int*)sizeof(int) };
+void constarr()
+{
+*(parr[0] - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S3
+{
+int * p = (int*)sizeof(int);
+};
+void recordinit()
+{
+S3 s3;
+*(s3.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1606,7 +1606,7 @@
   const MemRegion* superR = R->getSuperRegion();
 
   // Check if the region is an element region of a string literal.
-  if (const StringRegion *StrR=dyn_cast(superR)) {
+  if (const StringRegion *StrR = dyn_cast(superR)) {
 // FIXME: Handle loads from strings where the literal is treated as
 // an integer, e.g., *((unsigned int*)"hello")
 QualType T = Ctx.getAsArrayType(StrR->getValueType())->getElementType();
@@ -1629,6 +1629,27 @@
   char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
   return svalBuilder.makeIntVal(c, T);
 }
+  } else if (const VarRegion *VR = dyn_cast(superR)) {
+// Check if the containing array is const and has an initialized value.
+const VarDecl *VD = VR->getDecl();
+// Either the array or the array element has to be const.
+if (VD->getType().isConstQualified() || R->getElementType().isConstQualified()) {
+  if (const Expr *Init = VD->getInit()) {
+if (const auto *InitList = dyn_cast(Init)) {
+  // The array index has to be known.
+  if (auto CI = R->getIndex().getAs()) {
+int64_t i = CI->getValue().getSExtValue();
+// Return unknown value if index is out of bounds.
+if (i < 0 || i >= InitList->getNumInits())
+  return UnknownVal();
+
+if (const Expr *ElemInit = InitList->getInit(i))
+  if (Optional V = svalBuilder.getConstantVal(ElemInit))
+return *V;
+  }
+}
+  }
+}
   }
 
   // Check for loads from a code text region.  For such loads, just give up.
@@ -1678,7 +1699,28 @@
   if (const Optional  = B.getDirectBinding(R))
 return *V;
 
-  QualType Ty = R->getValueType();
+  // Is the field declared constant and has an in-class initializer?
+  const FieldDecl *FD = 

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Maybe this is a user error of CrossTU, but it seemed to import a FuncDecl with 
attributes, causing the imported FuncDecl to have all those attributes twice. 
That's why I thought merging would maybe make sense. However I did not 
encounter any issue with the duplicate attributes. Only the wrong source 
locations produced odd crashes.

Thanks for the input, then I will prepare the full patch.


Repository:
  rC Clang

https://reviews.llvm.org/D46115



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26054: Use `getFileLoc()` instead of `getSpellingLoc()` in the ASTImporter

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

It puts everything at the start of the marco expansion.

  `-FunctionDecl 0x12f5218 prev 0x12f4fa0  line:12:5 used 
foo 'int ()'
`-CompoundStmt 0x12f5600 
  |-DeclStmt 0x12f5508 
  | `-VarDecl 0x12f5470  col:15 used s 'struct S *' cinit
  |   `-ImplicitCastExpr 0x12f54f0  'struct S *' 
  | `-IntegerLiteral 0x12f54d0  'unsigned int' 2147516416
  `-ParenExpr 0x12f55e0  'int'
`-BinaryOperator 0x12f55b8  'int' '='
  |-MemberExpr 0x12f5560  'int' lvalue ->a 0x12f5378
  | `-ImplicitCastExpr 0x12f5548  'struct S *' 
  |   `-DeclRefExpr 0x12f5520  'struct S *' lvalue Var 0x12f5470 
's' 'struct S *'
  `-IntegerLiteral 0x12f5598  'int' 0


https://reviews.llvm.org/D26054



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26054: Use `getFileLoc()` instead of `getSpellingLoc()` in the ASTImporter

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.
Herald added a subscriber: martong.

I encountered an issue caused by this change.

In the scenario as shown below the call to getFileLoc causes the startLoc of 
the BinaryOp to be after the endLoc, because the LHS (`s->a`) is located in the 
marco argument while the RHS (`0`) is located at the beginning of the marco. 
See below: `BinaryOperator 0x12f45b8 `

This is an issue because the diagnostics machinery asserts on correctly ordered 
sourceranges. Namely:

`llvm/tools/clang/lib/Frontend/TextDiagnostic.cpp:1015: void 
highlightRange(const clang::CharSourceRange&, unsigned int, clang::FileID, 
const {anonymous}::SourceColumnMap&, std::__cxx11::string&, const 
clang::SourceManager&, const clang::LangOptions&): Assertion 'StartColNo <= 
EndColNo && "Invalid range!"' failed.`

I have tested the solution proposed by @a.sidorin and it works for my tests and 
the regression tests still pass.

code:

  01|#include 
  02|
  03|
  04|#define CLR_BIT(reg) (reg = 0)
  05|
  06|
  07|struct S
  08|{
  09|int a;
  10|};
  11|
  12|int foo()
  13|{
  14|struct S *s = 0x80008000;
  15|CLR_BIT(s->a);
  16|}

original:

  `-FunctionDecl 0xd29850  line:12:5 foo 'int ()'
`-CompoundStmt 0xd312d8 
  |-DeclStmt 0xd311e0 
  | `-VarDecl 0xd299e0  col:15 used s 'struct S *' cinit
  |   `-ImplicitCastExpr 0xd29a60  'struct S *' 
  | `-IntegerLiteral 0xd29a40  'unsigned int' 2147516416
  `-ParenExpr 0xd312b8  'int'
`-BinaryOperator 0xd31290  'int' '='
  |-MemberExpr 0xd31238  'int' lvalue ->a 0xd297b8
  | `-ImplicitCastExpr 0xd31220  'struct S *' 
  |   `-DeclRefExpr 0xd311f8  'struct S *' lvalue Var 0xd299e0 
's' 'struct S *'
  `-IntegerLiteral 0xd31270  'int' 0

imported:

  `-FunctionDecl 0x12f4218 prev 0x12f3fa0  line:12:5 used 
foo 'int ()'
`-CompoundStmt 0x12f4600 
  |-DeclStmt 0x12f4508 
  | `-VarDecl 0x12f4470  col:15 used s 'struct S *' cinit
  |   `-ImplicitCastExpr 0x12f44f0  'struct S *' 
  | `-IntegerLiteral 0x12f44d0  'unsigned int' 2147516416
  `-ParenExpr 0x12f45e0  'int'
`-BinaryOperator 0x12f45b8  'int' '='
  |-MemberExpr 0x12f4560  'int' lvalue ->a 0x12f4378
  | `-ImplicitCastExpr 0x12f4548  'struct S *' 
  |   `-DeclRefExpr 0x12f4520  'struct S *' lvalue Var 
0x12f4470 's' 'struct S *'
  `-IntegerLiteral 0x12f4598  'int' 0


https://reviews.llvm.org/D26054



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

This is unfinished. Posting to make you aware of the issue. There are other 
occurances of imported attrs without importing the srcloc in:

- VisitIndirectFieldDecl
- VisitAttributedStmt

So I was thinking this would suggest to add a public API 
`ASTImporter::Import(Attr *)`. Does it make sense to add this? Is there 
anything else I need to do to change public API?

Merging of Attr would also be missing and I wouldn't know how to approach this.

Testing also added soon!


Repository:
  rC Clang

https://reviews.llvm.org/D46115



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: NoQ, dcoughlin, xazax.hun, george.karpenkov.
Herald added subscribers: cfe-commits, martong, a.sidorin, rnkovacs.

The ASTImporter was failing to import the SourceLocation field of Attrs.


Repository:
  rC Clang

https://reviews.llvm.org/D46115

Files:
  lib/AST/ASTImporter.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7199,8 +7199,11 @@
 
 Decl *ASTImporter::Imported(Decl *From, Decl *To) {
   if (From->hasAttrs()) {
-for (auto *FromAttr : From->getAttrs())
-  To->addAttr(FromAttr->clone(To->getASTContext()));
+for (auto *FromAttr : From->getAttrs()) {
+  Attr *ToAttr = FromAttr->clone(To->getASTContext());
+  ToAttr->setRange(Import(FromAttr->getRange()));
+  To->addAttr(ToAttr);
+}
   }
   if (From->isUsed()) {
 To->setIsUsed();


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7199,8 +7199,11 @@
 
 Decl *ASTImporter::Imported(Decl *From, Decl *To) {
   if (From->hasAttrs()) {
-for (auto *FromAttr : From->getAttrs())
-  To->addAttr(FromAttr->clone(To->getASTContext()));
+for (auto *FromAttr : From->getAttrs()) {
+  Attr *ToAttr = FromAttr->clone(To->getASTContext());
+  ToAttr->setRange(Import(FromAttr->getRange()));
+  To->addAttr(ToAttr);
+}
   }
   if (From->isUsed()) {
 To->setIsUsed();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Not sure how to proceed about these:

- CXXDynamicCastExpr: I don't think evalCast would handle this correctly, does 
it?
- ObjCBridgedCastExpr: I don't know ObjectiveC
- CastKinds for floating point: Floats cannot yet be handled by the analyzer, 
right?


Repository:
  rC Clang

https://reviews.llvm.org/D45774



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: NoQ, dcoughlin, xazax.hun.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet, mehdi_amini.
Herald added a reviewer: george.karpenkov.

- SValBuilder::getConstantVal stopped processing an initialization if there are 
intermediate casts. Now additionally handles CStyleCastExpr, CXXConstCastExpr, 
CXXReinterpretCastExpr, CXXStaticCastExpr, CXXFunctionalCastExpr. Additional 
CastKinds are CK_NoOp and CK_IntegralToPointer - the others are already handled 
by EvaluateAsInt.
- RegionStoreManager::getBindingForElement now resolves a constant array 
initialization.
- RegionStoreManager::getBindingForField now resolves constant in-class 
initializers and constant element-wise initialization.
- Added tests for all new cases.


Repository:
  rC Clang

https://reviews.llvm.org/D45774

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  test/Analysis/globals.cpp

Index: test/Analysis/globals.cpp
===
--- test/Analysis/globals.cpp
+++ test/Analysis/globals.cpp
@@ -0,0 +1,101 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+
+static const unsigned long long scull = 0;
+void static_int()
+{
+*(int*)scull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull = 0;
+void const_int()
+{
+*(int*)cull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+static int * const spc = 0;
+void static_ptr()
+{
+*spc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc = 0;
+void const_ptr()
+{
+*pc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull_nonnull = 4;
+void nonnull_int()
+{
+*(int*)(cull_nonnull - 4) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc_nonnull = (int*)sizeof(int);
+void nonnull_ptr()
+{
+*(pc_nonnull - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const constcast = const_cast((int*)sizeof(int));
+void cast1()
+{
+*(constcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const recast = reinterpret_cast(sizeof(int));
+void cast2()
+{
+*(recast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const staticcast = static_cast((int*)sizeof(int));
+void cast3()
+{
+*(staticcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct Foo { int a; };
+Foo * const dyncast = dynamic_cast((Foo*)sizeof(Foo));
+void cast4()
+{
+// Do not handle dynamic_cast for now, because it may change the pointer value.
+(dyncast - 1)->a = 0; // no-warning
+}
+
+typedef int * const intptrconst;
+int * const funccast = intptrconst(sizeof(int));
+void cast5()
+{
+*(funccast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S1
+{
+int * p;
+};
+const S1 s1 = {
+.p = (int*)sizeof(int)
+};
+void conststruct()
+{
+*(s1.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S2
+{
+int * const p;
+};
+S2 s2 = {
+.p = (int*)sizeof(int)
+};
+void constfield()
+{
+*(s2.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const parr[1] = { (int*)sizeof(int) };
+void constarr()
+{
+*(parr[0] - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1606,7 +1606,7 @@
   const MemRegion* superR = R->getSuperRegion();
 
   // Check if the region is an element region of a string literal.
-  if (const StringRegion *StrR=dyn_cast(superR)) {
+  if (const StringRegion *StrR = dyn_cast(superR)) {
 // FIXME: Handle loads from strings where the literal is treated as
 // an integer, e.g., *((unsigned int*)"hello")
 QualType T = Ctx.getAsArrayType(StrR->getValueType())->getElementType();
@@ -1629,6 +1629,29 @@
   char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
   return svalBuilder.makeIntVal(c, T);
 }
+  } else if (const VarRegion *VR = dyn_cast(superR)) {
+// Check if the containing array is const and has an initialized value.
+const VarDecl *VD = VR->getDecl();
+// Either the array or the array element has to be const.
+if (VD->getType().isConstQualified() || R->getElementType().isConstQualified()) {
+  if (const Expr *Init = VD->getInit()) {
+if (const InitListExpr *InitList = dyn_cast(Init)) {
+  // The array index has to be known.
+  if (Optional CI = R->getIndex().getAs()) {
+int64_t i = CI->getValue().getSExtValue();
+// Return unknown value if index is out of bounds.
+if (i < 0 || i >= InitList->getNumInits()) {
+  return UnknownVal();
+}
+
+  

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 142381.
r.stahl edited the summary of this revision.
r.stahl added a comment.

addressed review comments.

I created a new test because certain checkers would cause early exits in the 
engine (because of undefined func ptr) and not cause the crash.

Since I don't have commit access, please commit for me.


https://reviews.llvm.org/D45564

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/undef-call.c


Index: test/Analysis/undef-call.c
===
--- test/Analysis/undef-call.c
+++ test/Analysis/undef-call.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze 
-analyzer-checker=debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
+// expected-no-diagnostics
+
+struct S {
+  void (*fp)();
+};
+
+int main() {
+  struct S s;
+  // This will cause the analyzer to look for a function definition that has
+  // no FunctionDecl. It used to cause a crash in 
AnyFunctionCall::getRuntimeDefinition.
+  // It would only occur when CTU analysis is enabled.
+  s.fp();
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,32 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD)
+return {};
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions  = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext  =
   *Engine->getCrossTranslationUnitContext();


Index: test/Analysis/undef-call.c
===
--- test/Analysis/undef-call.c
+++ test/Analysis/undef-call.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze -analyzer-checker=debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir -verify %s
+// expected-no-diagnostics
+
+struct S {
+  void (*fp)();
+};
+
+int main() {
+  struct S s;
+  // This will cause the analyzer to look for a function definition that has
+  // no FunctionDecl. It used to cause a crash in AnyFunctionCall::getRuntimeDefinition.
+  // It would only occur when CTU analysis is enabled.
+  s.fp();
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,32 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD)
+return {};
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

I encountered this with a construct like this:

  struct S
  {
  void (*fp)();
  };
  
  int main()
  {
  struct S s;
  s.fp();
  }


Repository:
  rC Clang

https://reviews.llvm.org/D45564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: xazax.hun, dcoughlin, a.sidorin, george.karpenkov.
Herald added subscribers: cfe-commits, rnkovacs, szepet.

In https://reviews.llvm.org/D30691 code was added to getRuntimeDefinition that 
does not handle the case when FD==nullptr.


Repository:
  rC Clang

https://reviews.llvm.org/D45564

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,33 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD) {
+return {};
+  }
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions  = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext  =
   *Engine->getCrossTranslationUnitContext();


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,33 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD) {
+return {};
+  }
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions  = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext  =
   *Engine->getCrossTranslationUnitContext();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38943: [ASTImporter] import SubStmt of CaseStmt

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.
Herald added a subscriber: martong.



Comment at: cfe/trunk/unittests/AST/ASTImporterTest.cpp:100
 
+  // This traverses the AST to catch certain bugs like poorly or not
+  // implemented subtrees.

a.sidorin wrote:
> I just saw this change and I cannot find the reason, why do we need to print 
> the imported declaration after we have printed the entire translation unit? 
> Is there some special case?
Sorry for the late reply.

The particular bug here would only be detected when dumping, but not when 
printing. The output of printing was just not listing the missing AST nodes, 
but dumping crashed and therefore failed the test as expected.

I'm not familiar with the inner workings of print and dump, so I cannot explain 
why that is.


Repository:
  rL LLVM

https://reviews.llvm.org/D38943



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38842: [CrossTU] Fix handling of Cross Translation Unit directory path

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl accepted this revision.
r.stahl added a comment.
This revision is now accepted and ready to land.

I'm gonna go ahead and approve this now, because I reported the issue. Note 
that I'm not a regular contributor, yet!




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:99
   else
 llvm::sys::path::append(FilePath, FileName);
   Result[FunctionLookupName] = FilePath.str().str();

Here you could drop the if/else completely and just append.


https://reviews.llvm.org/D38842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38943: [ASTImporter] import SubStmt of CaseStmt

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

If all is good, I will need someone to commit this for me please.




Comment at: unittests/AST/ASTImporterTest.cpp:100
 
+  // This might catch other cases.
+  Imported->dump(ToNothing);

xazax.hun wrote:
> r.stahl wrote:
> > xazax.hun wrote:
> > > I would elaborate a bit more on the purpose of the code below.
> > I will need help for that, since I do not have a better rationale myself. 
> > My guess would be that print has different assumptions and error checking 
> > than dump, so executing both increases coverage.
> So the reason you need this to traverse the AST and crash on the null 
> pointers (when the patch is not applied right?)
> I think you could comment something like traverse the AST to catch certain 
> bugs like poorly (not) imported subtrees. 
> If we do not want to actually print the whole tree (because it might be 
> noisy) you can also try using a no-op recursive AST visitor.
Yes, of course this referred to the case when the patch is not applied.

There is no noise since the dump is done on a buffer that will be discarded.


https://reviews.llvm.org/D38943



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38943: [ASTImporter] import SubStmt of CaseStmt

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Thanks for the fast response. See inline comment.




Comment at: unittests/AST/ASTImporterTest.cpp:100
 
+  // This might catch other cases.
+  Imported->dump(ToNothing);

xazax.hun wrote:
> I would elaborate a bit more on the purpose of the code below.
I will need help for that, since I do not have a better rationale myself. My 
guess would be that print has different assumptions and error checking than 
dump, so executing both increases coverage.


https://reviews.llvm.org/D38943



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38943: [ASTImporter] import SubStmt of CaseStmt

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.

This fixes importing of CaseStmts, because the importer was not importing its 
SubStmt.

A test was added and the test procedure was adjusted to also dump the imported 
Decl, because otherwise the bug would not be detected.


https://reviews.llvm.org/D38943

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -97,6 +97,9 @@
   llvm::raw_svector_ostream ToNothing(ImportChecker);
   ToCtx.getTranslationUnitDecl()->print(ToNothing);
 
+  // This might catch other cases.
+  Imported->dump(ToNothing);
+
   return Verifier.match(Imported, AMatcher);
 }
 
@@ -267,6 +270,15 @@
   
hasUnaryOperand(cxxThisExpr();
 }
 
+TEST(ImportExpr, ImportSwitch) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+  testImport("void declToImport() { int b; switch (b) { case 1: break; } 
}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(hasBody(compoundStmt(
+ has(switchStmt(has(compoundStmt(has(caseStmt()));
+}
+
 TEST(ImportExpr, ImportStmtExpr) {
   MatchVerifier Verifier;
   // NOTE: has() ignores implicit casts, using hasDescendant() to match it
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -3983,12 +3983,16 @@
   Expr *ToRHS = Importer.Import(S->getRHS());
   if (!ToRHS && S->getRHS())
 return nullptr;
+  Stmt *ToSubStmt = Importer.Import(S->getSubStmt());
+  if (!ToSubStmt && S->getSubStmt())
+return nullptr;
   SourceLocation ToCaseLoc = Importer.Import(S->getCaseLoc());
   SourceLocation ToEllipsisLoc = Importer.Import(S->getEllipsisLoc());
   SourceLocation ToColonLoc = Importer.Import(S->getColonLoc());
-  return new (Importer.getToContext()) CaseStmt(ToLHS, ToRHS,
-ToCaseLoc, ToEllipsisLoc,
-ToColonLoc);
+  CaseStmt *ToStmt = new (Importer.getToContext())
+  CaseStmt(ToLHS, ToRHS, ToCaseLoc, ToEllipsisLoc, ToColonLoc);
+  ToStmt->setSubStmt(ToSubStmt);
+  return ToStmt;
 }
 
 Stmt *ASTNodeImporter::VisitDefaultStmt(DefaultStmt *S) {


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -97,6 +97,9 @@
   llvm::raw_svector_ostream ToNothing(ImportChecker);
   ToCtx.getTranslationUnitDecl()->print(ToNothing);
 
+  // This might catch other cases.
+  Imported->dump(ToNothing);
+
   return Verifier.match(Imported, AMatcher);
 }
 
@@ -267,6 +270,15 @@
   hasUnaryOperand(cxxThisExpr();
 }
 
+TEST(ImportExpr, ImportSwitch) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+  testImport("void declToImport() { int b; switch (b) { case 1: break; } }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(hasBody(compoundStmt(
+ has(switchStmt(has(compoundStmt(has(caseStmt()));
+}
+
 TEST(ImportExpr, ImportStmtExpr) {
   MatchVerifier Verifier;
   // NOTE: has() ignores implicit casts, using hasDescendant() to match it
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -3983,12 +3983,16 @@
   Expr *ToRHS = Importer.Import(S->getRHS());
   if (!ToRHS && S->getRHS())
 return nullptr;
+  Stmt *ToSubStmt = Importer.Import(S->getSubStmt());
+  if (!ToSubStmt && S->getSubStmt())
+return nullptr;
   SourceLocation ToCaseLoc = Importer.Import(S->getCaseLoc());
   SourceLocation ToEllipsisLoc = Importer.Import(S->getEllipsisLoc());
   SourceLocation ToColonLoc = Importer.Import(S->getColonLoc());
-  return new (Importer.getToContext()) CaseStmt(ToLHS, ToRHS,
-ToCaseLoc, ToEllipsisLoc,
-ToColonLoc);
+  CaseStmt *ToStmt = new (Importer.getToContext())
+  CaseStmt(ToLHS, ToRHS, ToCaseLoc, ToEllipsisLoc, ToColonLoc);
+  ToStmt->setSubStmt(ToSubStmt);
+  return ToStmt;
 }
 
 Stmt *ASTNodeImporter::VisitDefaultStmt(DefaultStmt *S) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Since I do not have commit access, it would be nice if someone committed this 
for me. Thanks!


https://reviews.llvm.org/D37478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 118182.
r.stahl marked an inline comment as done.
r.stahl edited the summary of this revision.
r.stahl added a comment.
Herald added a subscriber: szepet.

addressed review comments. updated summary.


https://reviews.llvm.org/D37478

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/pointer-arithmetic.c


Index: test/Analysis/pointer-arithmetic.c
===
--- test/Analysis/pointer-arithmetic.c
+++ test/Analysis/pointer-arithmetic.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+int test1() {
+  int *p = (int *)sizeof(int);
+  p -= 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test2() {
+  int *p = (int *)sizeof(int);
+  p -= 2;
+  p += 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test3() {
+  int *p = (int *)sizeof(int);
+  p++;
+  p--;
+  p--;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test4() {
+  // This is a special case where pointer arithmetic is not calculated to
+  // preserve useful warnings on dereferences of null pointers.
+  int *p = 0;
+  p += 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -922,6 +922,10 @@
   if (rhs.isZeroConstant())
 return lhs;
 
+  // Perserve the null pointer so that it can be found by the DerefChecker.
+  if (lhs.isZeroConstant())
+return lhs;
+
   // We are dealing with pointer arithmetic.
 
   // Handle pointer arithmetic on constant values.
@@ -937,6 +941,8 @@
 
   // Offset the increment by the pointer size.
   llvm::APSInt Multiplicand(rightI.getBitWidth(), /* isUnsigned */ true);
+  QualType pointeeType = resultTy->getPointeeType();
+  Multiplicand = 
getContext().getTypeSizeInChars(pointeeType).getQuantity();
   rightI *= Multiplicand;
 
   // Compute the adjusted pointer.


Index: test/Analysis/pointer-arithmetic.c
===
--- test/Analysis/pointer-arithmetic.c
+++ test/Analysis/pointer-arithmetic.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+int test1() {
+  int *p = (int *)sizeof(int);
+  p -= 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test2() {
+  int *p = (int *)sizeof(int);
+  p -= 2;
+  p += 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test3() {
+  int *p = (int *)sizeof(int);
+  p++;
+  p--;
+  p--;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test4() {
+  // This is a special case where pointer arithmetic is not calculated to
+  // preserve useful warnings on dereferences of null pointers.
+  int *p = 0;
+  p += 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -922,6 +922,10 @@
   if (rhs.isZeroConstant())
 return lhs;
 
+  // Perserve the null pointer so that it can be found by the DerefChecker.
+  if (lhs.isZeroConstant())
+return lhs;
+
   // We are dealing with pointer arithmetic.
 
   // Handle pointer arithmetic on constant values.
@@ -937,6 +941,8 @@
 
   // Offset the increment by the pointer size.
   llvm::APSInt Multiplicand(rightI.getBitWidth(), /* isUnsigned */ true);
+  QualType pointeeType = resultTy->getPointeeType();
+  Multiplicand = getContext().getTypeSizeInChars(pointeeType).getQuantity();
   rightI *= Multiplicand;
 
   // Compute the adjusted pointer.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34512: Add preliminary Cross Translation Unit support library

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In https://reviews.llvm.org/D34512#877643, @xazax.hun wrote:

> - Unittests now creates temporary files at the correct location
> - Temporary files are also cleaned up when the process is terminated
> - Absolute paths are handled correctly by the library


I think there is an issue with this change.

First, the function map generation writes absolute paths to the map file. Now 
when the `parseCrossTUIndex` function parses it, it will no longer prepend the 
paths with the CTUDir and therefore expect the precompiled AST files at the 
exact path of the source file. This seems like a bad requirement, because the 
user would have to overwrite his source files.

If I understand your intention correctly, a fix would be to replace the 
`is_absolute` check with a check for `CTUDir == ""` in the `parseCrossTUIndex` 
function.


Repository:
  rL LLVM

https://reviews.llvm.org/D34512



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In a similar case, static inline functions are an issue.

inc.h

  void foo();
  static inline void wow()
  {
  int a = *(int*)0;
  }

main.c

  #include "inc.h"
  void moo()
  {
  foo();
  wow();
  }

other.c

  #include "inc.h"
  void foo()
  {
  wow();
  }

The inline function is inlined into each calling AST as different AST objects. 
This causes the PathDiagnostics to be distinct, while pointing to the exact 
same source location.

When the compareCrossTUSourceLocs function tries to compare the FullSourceLocs, 
it cannot find a difference and FlushDiagnostics will assert on an erroneous 
compare function.

For my purposes I replaced the return statement of the compareCrossTUSourceLocs 
function with:

  return XL.getFileID() < YL.getFileID();

A more correct fix would create only one unique diagnostic for both cases.


https://reviews.llvm.org/D30691



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

While testing this I stumbled upon a crash with the following test case:

inc.h

  #define BASE ((int*)0)
  void foo();

main.c:

  #include "inc.h"
  void moo()
  {
  int a = BASE[0];
  foo();
  }

other.c

  #include "inc.h"
  void foo()
  {
  int a = BASE[0];
  }

Note that I used a custom checker that did not stop on the path like the 
DerefChecker would here. I did not know how to reproduce it with official 
checkers, but the issue should be understandable without reproduction.

With the given test a checker may produce two results for the null dereference 
in moo() and foo(). When analyzing main.c they will both be found and therefore 
sorted with PathDiagnostic.cpp "compareCrossTUSourceLocs".

If either of the FullSourceLocs is a MacroID, the call 
SM.getFileEntryForID(XL.getFileID()) will return a null pointer. The null 
pointer will crash the program when attempting to call ->getName() on it.

My solution was to add the following lines before the .getFileID() calls:

  XL = XL.getExpansionLoc();
  YL = YL.getExpansionLoc();




Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:391
+return XL.isBeforeInTranslationUnitThan(YL);
+  return SM.getFileEntryForID(XL.getFileID())->getName() <
+ SM.getFileEntryForID(YL.getFileID())->getName();

see comment


https://reviews.llvm.org/D30691



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 114126.
r.stahl added a comment.

addressed the review comments


https://reviews.llvm.org/D37478

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/explain-svals.cpp
  test/Analysis/inlining/inline-defensive-checks.c
  test/Analysis/pointer-arithmetic.c


Index: test/Analysis/pointer-arithmetic.c
===
--- test/Analysis/pointer-arithmetic.c
+++ test/Analysis/pointer-arithmetic.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+int test1() {
+  int *p = (int *)sizeof(int);
+  p -= 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test2() {
+  int *p = (int *)sizeof(int);
+  p -= 2;
+  p += 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test3() {
+  int *p = (int *)sizeof(int);
+  p++;
+  p--;
+  p--;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test4() {
+  // It would be fine if this warns about a null dereference, but the
+  // symbolic value of p at the end should still be sizeof(int).
+  int *p = 0;
+  p += 1;
+  return *p; // no-warning
+}
Index: test/Analysis/inlining/inline-defensive-checks.c
===
--- test/Analysis/inlining/inline-defensive-checks.c
+++ test/Analysis/inlining/inline-defensive-checks.c
@@ -140,6 +140,10 @@
   idcTriggerZeroValueThroughCall(z);
 }
 
+
+// Intuitively the following tests should all warn about a null dereference,
+// since the object pointer the operations are based on can be null.
+
 struct S {
   int f1;
   int f2;
@@ -159,8 +163,7 @@
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
   idc(s);
   int *x = &(s->f2) - 1;
-  // FIXME: Should not warn.
-  *x = 7; // expected-warning{{Dereference of null pointer}}
+  *x = 7; // no-warning
 }
 
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -59,8 +59,7 @@
   clang_analyzer_explain([5].y[3]); // expected-warning-re^pointer to 
element of type 'int' with index 3 of field 'y' of base object 'S::S3' inside 
element of type 'struct S::S2' with index 5 of field 's2' of parameter 's'$
   if (!s.s2[7].x) {
 clang_analyzer_explain(s.s2[7].x); // expected-warning-re^concrete 
memory address '0'$
-// FIXME: we need to be explaining '1' rather than '0' here; not explainer 
bug.
-clang_analyzer_explain(s.s2[7].x + 1); // expected-warning-re^concrete 
memory address '0'$
+clang_analyzer_explain(s.s2[7].x + 1); // expected-warning-re^concrete 
memory address '4'$
   }
 }
 
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -935,6 +935,8 @@
 
   // Offset the increment by the pointer size.
   llvm::APSInt Multiplicand(rightI.getBitWidth(), /* isUnsigned */ true);
+  QualType pointeeType = resultTy->getPointeeType();
+  Multiplicand = 
getContext().getTypeSizeInChars(pointeeType).getQuantity();
   rightI *= Multiplicand;
 
   // Compute the adjusted pointer.


Index: test/Analysis/pointer-arithmetic.c
===
--- test/Analysis/pointer-arithmetic.c
+++ test/Analysis/pointer-arithmetic.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+int test1() {
+  int *p = (int *)sizeof(int);
+  p -= 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test2() {
+  int *p = (int *)sizeof(int);
+  p -= 2;
+  p += 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test3() {
+  int *p = (int *)sizeof(int);
+  p++;
+  p--;
+  p--;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test4() {
+  // It would be fine if this warns about a null dereference, but the
+  // symbolic value of p at the end should still be sizeof(int).
+  int *p = 0;
+  p += 1;
+  return *p; // no-warning
+}
Index: test/Analysis/inlining/inline-defensive-checks.c
===
--- test/Analysis/inlining/inline-defensive-checks.c
+++ test/Analysis/inlining/inline-defensive-checks.c
@@ -140,6 +140,10 @@
   idcTriggerZeroValueThroughCall(z);
 }
 
+
+// Intuitively the following tests should all warn about a null dereference,
+// since the object pointer the operations are based on can be null.
+
 struct S {
   int f1;
   int f2;
@@ -159,8 +163,7 @@
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
   idc(s);
   int *x = &(s->f2) - 1;
-  // FIXME: Should 

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

To be honest I was quite surprised that this change in behavior didn't cause 
more test failures, because for detecting null dereferences the old behavior is 
definitely more useful. Since it did not, I was convinced that this change is 
desired.

We use the analyzer for finding dereferences to fixed addresses - very similar 
to the FixedAddressChecker. For this purpose it is crucial that the execution 
engine works as perfect as possible, without "swallowing" any arithmetic.

For the struct example you mentioned you can still get the final address by 
asking the ASTContext if needed, but with pointer arithmetic the information is 
lost forever. Information is lost either way here. Either you forget that the 
arithmetic was based on a null pointer or you lose whatever was added to or 
subtracted from it.

So unless you can somehow tag the information in the SVal when an operation was 
based on a null pointer, this is pretty difficult. You also could introduce a 
heuristic that defines all dereferences around zero as null dereferences, but 
it would be very arbitrary and platform dependent. Or maybe the 
DereferenceChecker should explicitly break early on all statements that do 
arithmetic on pointers constrained to null. Overall I don't know enough about 
the analyzer to suggest more here.

Thanks for the comments, I will address them soon.


https://reviews.llvm.org/D37478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
Herald added a subscriber: eraman.

The "Multiplicand" variable in SimpleSValBuilder::evalBinOpLN was always 
initialized to zero, causing all pointer arithmetic on constant values to be 
no-ops.

This fixes two FIXMEs in existing tests. The added tests were all failing 
before this change.


https://reviews.llvm.org/D37478

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/explain-svals.cpp
  test/Analysis/inlining/inline-defensive-checks.c
  test/Analysis/pointer-arithmetic.c


Index: test/Analysis/pointer-arithmetic.c
===
--- test/Analysis/pointer-arithmetic.c
+++ test/Analysis/pointer-arithmetic.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+int test1() {
+  int *p = (int *)sizeof(int);
+  p -= 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test2() {
+  int *p = (int *)sizeof(int);
+  p -= 2;
+  p += 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test3() {
+  int *p = (int *)sizeof(int);
+  p++;
+  p--;
+  p--;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test4() {
+  int *p = 0;
+  p += 1;
+  return *p; // no-warning
+}
Index: test/Analysis/inlining/inline-defensive-checks.c
===
--- test/Analysis/inlining/inline-defensive-checks.c
+++ test/Analysis/inlining/inline-defensive-checks.c
@@ -159,8 +159,7 @@
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
   idc(s);
   int *x = &(s->f2) - 1;
-  // FIXME: Should not warn.
-  *x = 7; // expected-warning{{Dereference of null pointer}}
+  *x = 7; // no-warning
 }
 
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -59,8 +59,7 @@
   clang_analyzer_explain([5].y[3]); // expected-warning-re^pointer to 
element of type 'int' with index 3 of field 'y' of base object 'S::S3' inside 
element of type 'struct S::S2' with index 5 of field 's2' of parameter 's'$
   if (!s.s2[7].x) {
 clang_analyzer_explain(s.s2[7].x); // expected-warning-re^concrete 
memory address '0'$
-// FIXME: we need to be explaining '1' rather than '0' here; not explainer 
bug.
-clang_analyzer_explain(s.s2[7].x + 1); // expected-warning-re^concrete 
memory address '0'$
+clang_analyzer_explain(s.s2[7].x + 1); // expected-warning-re^concrete 
memory address '4'$
   }
 }
 
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -935,6 +935,8 @@
 
   // Offset the increment by the pointer size.
   llvm::APSInt Multiplicand(rightI.getBitWidth(), /* isUnsigned */ true);
+  QualType PteeTy = 
resultTy.getTypePtr()->castAs()->getPointeeType();
+  Multiplicand = getContext().getTypeSizeInChars(PteeTy).getQuantity();
   rightI *= Multiplicand;
 
   // Compute the adjusted pointer.


Index: test/Analysis/pointer-arithmetic.c
===
--- test/Analysis/pointer-arithmetic.c
+++ test/Analysis/pointer-arithmetic.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+int test1() {
+  int *p = (int *)sizeof(int);
+  p -= 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test2() {
+  int *p = (int *)sizeof(int);
+  p -= 2;
+  p += 1;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test3() {
+  int *p = (int *)sizeof(int);
+  p++;
+  p--;
+  p--;
+  return *p; // expected-warning {{Dereference of null pointer}}
+}
+
+int test4() {
+  int *p = 0;
+  p += 1;
+  return *p; // no-warning
+}
Index: test/Analysis/inlining/inline-defensive-checks.c
===
--- test/Analysis/inlining/inline-defensive-checks.c
+++ test/Analysis/inlining/inline-defensive-checks.c
@@ -159,8 +159,7 @@
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
   idc(s);
   int *x = &(s->f2) - 1;
-  // FIXME: Should not warn.
-  *x = 7; // expected-warning{{Dereference of null pointer}}
+  *x = 7; // no-warning
 }
 
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -59,8 +59,7 @@
   clang_analyzer_explain([5].y[3]); // expected-warning-re^pointer to element of type 'int' with index 3 of field 'y' of base object 'S::S3' inside element