OikawaKirie updated this revision to Diff 390653.
OikawaKirie marked an inline comment as done.
OikawaKirie edited the summary of this revision.
OikawaKirie added a comment.
Herald added a subscriber: manas.
It has been a long period since the last discussion, I hope you can still
remember this bug. And apologize for the delay.
Updated as required, the lookup name generator
`CrossTranslationUnitContext::getLookupName` and parser `parseCrossTUIndex` are
modified.
And assertions are added before accessing the CTU index mapping for input
lookup names to be searched.
Corresponding test cases and documents are also updated.
Please let me know if there are other files to be updated.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102669/new/
https://reviews.llvm.org/D102669
Files:
clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
clang/include/clang/Basic/DiagnosticCrossTUKinds.td
clang/lib/CrossTU/CrossTranslationUnit.cpp
clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
clang/test/Analysis/Inputs/ctu-lookup-name-with-space.cpp
clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
clang/test/Analysis/Inputs/plist-macros-with-expansion-ctu.c.externalDefMap.txt
clang/test/Analysis/ctu-inherited-default-ctor.cpp
clang/test/Analysis/ctu-lookup-name-with-space.cpp
clang/test/Analysis/func-mapping-test.cpp
clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===================================================================
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -61,7 +61,7 @@
ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
IndexFileName));
llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
- IndexFile.os() << "c:@F@f#I# " << ASTFileName << "\n";
+ IndexFile.os() << "9:c:@F@f#I# " << ASTFileName << "\n";
IndexFile.os().flush();
EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
@@ -154,9 +154,9 @@
TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
llvm::StringMap<std::string> Index;
- Index["a"] = "/b/f1";
- Index["c"] = "/d/f2";
- Index["e"] = "/f/f3";
+ Index["1:a"] = "/b/f1";
+ Index["1:c"] = "/d/f2";
+ Index["1:e"] = "/f/f3";
std::string IndexText = createCrossTUIndexString(Index);
int IndexFD;
Index: clang/test/Analysis/func-mapping-test.cpp
===================================================================
--- clang/test/Analysis/func-mapping-test.cpp
+++ clang/test/Analysis/func-mapping-test.cpp
@@ -3,10 +3,10 @@
int f(int) {
return 0;
}
-// CHECK-DAG: c:@F@f#I#
+// CHECK-DAG: 9:c:@F@f#I#
extern const int x = 5;
-// CHECK-DAG: c:@x
+// CHECK-DAG: 4:c:@x
// Non-const variables should not be collected.
int y = 5;
@@ -18,29 +18,29 @@
int a;
};
extern S const s = {.a = 2};
-// CHECK-DAG: c:@s
+// CHECK-DAG: 4:c:@s
struct SF {
const int a;
};
SF sf = {.a = 2};
-// CHECK-DAG: c:@sf
+// CHECK-DAG: 5:c:@sf
struct SStatic {
static const int a = 4;
};
const int SStatic::a;
-// CHECK-DAG: c:@S@SStatic@a
+// CHECK-DAG: 14:c:@S@SStatic@a
extern int const arr[5] = { 0, 1 };
-// CHECK-DAG: c:@arr
+// CHECK-DAG: 6:c:@arr
union U {
const int a;
const unsigned int b;
};
U u = {.a = 6};
-// CHECK-DAG: c:@u
+// CHECK-DAG: 4:c:@u
// No USR can be generated for this.
// Check for no crash in this case.
Index: clang/test/Analysis/ctu-lookup-name-with-space.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/ctu-lookup-name-with-space.cpp
@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: cp %s %t/trigger.cpp
+// RUN: cp %S/Inputs/ctu-lookup-name-with-space.cpp %t/importee.cpp
+// RUN: echo '"%t/importee.cpp" : ["g++", "-c", "%t/importee.cpp"]' > %t/invocations.yaml
+// RUN: echo '[{"directory": "%t", "command": "g++ -c %t/importee.cpp", "file": "%t/importee.cpp"}, {"directory": "%t", "command": "g++ -c %t/trigger.cpp", "file": "%t/trigger.cpp"}]' > %t/compile_commands.json
+
+// RUN: %clang_extdef_map -p %t %t/importee.cpp > %t/externalDefMap.txt
+
+// RUN: cd %t && %clang_cc1 -fsyntax-only -analyze \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN: -analyzer-config ctu-dir=. \
+// RUN: -analyzer-config ctu-invocation-list=invocations.yaml \
+// RUN: -verify trigger.cpp 2>&1 | FileCheck --allow-empty importee.cpp
+
+int importee(int);
+
+void trigger() {
+ importee(0); // expected-warn...@importee.cpp:13 {{Division by zero}}
+}
Index: clang/test/Analysis/ctu-inherited-default-ctor.cpp
===================================================================
--- clang/test/Analysis/ctu-inherited-default-ctor.cpp
+++ clang/test/Analysis/ctu-inherited-default-ctor.cpp
@@ -4,7 +4,7 @@
// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
// RUN: -emit-pch -o %t/ctudir/ctu-inherited-default-ctor-other.cpp.ast \
// RUN: %S/Inputs/ctu-inherited-default-ctor-other.cpp
-// RUN: echo "c:@N@clang@S@DeclContextLookupResult@SingleElementDummyList ctu-inherited-default-ctor-other.cpp.ast" \
+// RUN: echo "59:c:@N@clang@S@DeclContextLookupResult@SingleElementDummyList ctu-inherited-default-ctor-other.cpp.ast" \
// RUN: > %t/ctudir/externalDefMap.txt
//
// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
Index: clang/test/Analysis/Inputs/plist-macros-with-expansion-ctu.c.externalDefMap.txt
===================================================================
--- clang/test/Analysis/Inputs/plist-macros-with-expansion-ctu.c.externalDefMap.txt
+++ clang/test/Analysis/Inputs/plist-macros-with-expansion-ctu.c.externalDefMap.txt
@@ -1,4 +1,4 @@
-c:@F@F1 plist-macros-ctu.c.ast
-c:@F@F2 plist-macros-ctu.c.ast
-c:@F@F3 plist-macros-ctu.c.ast
-c:@F@F_H plist-macros-ctu.c.ast
+7:c:@F@F1 plist-macros-ctu.c.ast
+7:c:@F@F2 plist-macros-ctu.c.ast
+7:c:@F@F3 plist-macros-ctu.c.ast
+8:c:@F@F_H plist-macros-ctu.c.ast
Index: clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
===================================================================
--- clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
+++ clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
@@ -1,30 +1,30 @@
-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:@S@mycls@F@fvcl#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:@S@derived@F@fvcl#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
-c:@extInt ctu-other.cpp.ast
-c:@N@intns@extInt ctu-other.cpp.ast
-c:@extS ctu-other.cpp.ast
-c:@S@A@a ctu-other.cpp.ast
-c:@extSC ctu-other.cpp.ast
-c:@S@ST@sc ctu-other.cpp.ast
-c:@extSCN ctu-other.cpp.ast
-c:@extSubSCN ctu-other.cpp.ast
-c:@extSCC ctu-other.cpp.ast
-c:@extU ctu-other.cpp.ast
-c:@S@TestAnonUnionUSR@Test ctu-other.cpp.ast
-c:@F@testImportOfIncompleteDefaultParmDuringImport#I# ctu-other.cpp.ast
-c:@F@testImportOfDelegateConstructor#I# ctu-other.cpp.ast
\ No newline at end of file
+19:c:@N@chns@F@chf1#I# ctu-other.cpp.ast
+30:c:@N@myns@N@embed_ns@F@fens#I# ctu-other.cpp.ast
+9:c:@F@g#I# ctu-other.cpp.ast
+21:c:@S@mycls@F@fscl#I#S ctu-other.cpp.ast
+19:c:@S@mycls@F@fcl#I# ctu-other.cpp.ast
+20:c:@S@mycls@F@fvcl#I# ctu-other.cpp.ast
+31:c:@N@myns@S@embed_cls@F@fecl#I# ctu-other.cpp.ast
+34:c:@S@mycls@S@embed_cls2@F@fecl2#I# ctu-other.cpp.ast
+22:c:@S@derived@F@fvcl#I# ctu-other.cpp.ast
+9:c:@F@f#I# ctu-other.cpp.ast
+18:c:@N@myns@F@fns#I# ctu-other.cpp.ast
+9:c:@F@h#I# ctu-other.cpp.ast
+15:c:@F@h_chain#I# ctu-chain.cpp.ast
+27:c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
+19:c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
+29:c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+24:c:@F@other_macro_diag#I# ctu-other.cpp.ast
+9:c:@extInt ctu-other.cpp.ast
+17:c:@N@intns@extInt ctu-other.cpp.ast
+7:c:@extS ctu-other.cpp.ast
+8:c:@S@A@a ctu-other.cpp.ast
+8:c:@extSC ctu-other.cpp.ast
+10:c:@S@ST@sc ctu-other.cpp.ast
+9:c:@extSCN ctu-other.cpp.ast
+12:c:@extSubSCN ctu-other.cpp.ast
+9:c:@extSCC ctu-other.cpp.ast
+7:c:@extU ctu-other.cpp.ast
+26:c:@S@TestAnonUnionUSR@Test ctu-other.cpp.ast
+53:c:@F@testImportOfIncompleteDefaultParmDuringImport#I# ctu-other.cpp.ast
+39:c:@F@testImportOfDelegateConstructor#I# ctu-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
===================================================================
--- clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
+++ clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
@@ -1,7 +1,7 @@
-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
-c:@F@switchWithoutCases ctu-other.c.ast
+14:c:@F@inlineAsm ctu-other.c.ast
+6:c:@F@g ctu-other.c.ast
+6:c:@F@f ctu-other.c.ast
+14:c:@F@enumCheck ctu-other.c.ast
+18:c:@F@identImplicit ctu-other.c.ast
+18:c:@F@structInProto ctu-other.c.ast
+23:c:@F@switchWithoutCases ctu-other.c.ast
Index: clang/test/Analysis/Inputs/ctu-lookup-name-with-space.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-lookup-name-with-space.cpp
@@ -0,0 +1,14 @@
+void f(int (*)(char));
+void f(bool (*)(char));
+
+struct G {
+ G() {
+ // CHECK-NOT: multiple definitions are found for the same key in index
+ f([](char) -> int { return 42; });
+ f([](char) -> bool { return true; });
+ }
+};
+
+int importee(int X) {
+ return 1 / X;
+}
Index: clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
===================================================================
--- clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
+++ clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
@@ -1 +1 @@
-c:@F@testStaticImplicit ctu-import.c.ast
+23:c:@F@testStaticImplicit ctu-import.c.ast
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===================================================================
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -149,6 +149,36 @@
return std::error_code(static_cast<int>(Code), *Category);
}
+static bool parseCrossTUIndexItem(StringRef LineRef, StringRef &LookupName,
+ StringRef &FilePath) {
+ // Find the length delimiter.
+ const size_t LengthDelimiter = LineRef.find(':');
+ if (StringRef::npos == LengthDelimiter)
+ return false;
+
+ // Parse the length of LookupName as USRLength.
+ size_t USRLength = 0;
+ if (LineRef.substr(0, LengthDelimiter).consumeInteger(10, USRLength))
+ return false;
+
+ // Check LookupName length out of bound and incorrect delimiter.
+ const size_t Delimiter = LengthDelimiter + USRLength + 1;
+ if (USRLength <= 0 || Delimiter >= LineRef.size() ||
+ ' ' != LineRef[Delimiter])
+ return false;
+
+ LookupName = LineRef.substr(0, Delimiter);
+ FilePath = LineRef.substr(Delimiter + 1);
+ return true;
+}
+
+#define IS_CTU_INDEX_KEY_VALID(FUNCTION_NAME) \
+ ([](StringRef N) { \
+ std::string FN = N.str() + ' '; \
+ StringRef LN, FP; \
+ return parseCrossTUIndexItem(FN, LN, FP) && N == LN && FP.empty(); \
+ }(FUNCTION_NAME))
+
llvm::Expected<llvm::StringMap<std::string>>
parseCrossTUIndex(StringRef IndexPath) {
std::ifstream ExternalMapFile{std::string(IndexPath)};
@@ -160,24 +190,23 @@
std::string Line;
unsigned LineNo = 1;
while (std::getline(ExternalMapFile, Line)) {
- StringRef LineRef{Line};
- const size_t Delimiter = LineRef.find(' ');
- if (Delimiter > 0 && Delimiter != std::string::npos) {
- StringRef LookupName = LineRef.substr(0, Delimiter);
-
- // Store paths with posix-style directory separator.
- SmallString<32> FilePath(LineRef.substr(Delimiter + 1));
- llvm::sys::path::native(FilePath, llvm::sys::path::Style::posix);
-
- bool InsertionOccured;
- std::tie(std::ignore, InsertionOccured) =
- Result.try_emplace(LookupName, FilePath.begin(), FilePath.end());
- if (!InsertionOccured)
- return llvm::make_error<IndexError>(
- index_error_code::multiple_definitions, IndexPath.str(), LineNo);
- } else
+ // Split lookup name and file path
+ StringRef LookupName, FilePathInIndex;
+ if (!parseCrossTUIndexItem(Line, LookupName, FilePathInIndex))
return llvm::make_error<IndexError>(
index_error_code::invalid_index_format, IndexPath.str(), LineNo);
+
+ // Store paths with posix-style directory separator.
+ SmallString<32> FilePath(FilePathInIndex);
+ llvm::sys::path::native(FilePath, llvm::sys::path::Style::posix);
+
+ bool InsertionOccured;
+ std::tie(std::ignore, InsertionOccured) =
+ Result.try_emplace(LookupName, FilePath.begin(), FilePath.end());
+ if (!InsertionOccured)
+ return llvm::make_error<IndexError>(
+ index_error_code::multiple_definitions, IndexPath.str(), LineNo);
+
++LineNo;
}
return Result;
@@ -223,7 +252,11 @@
bool Ret = index::generateUSRForDecl(ND, DeclUSR);
if (Ret)
return {};
- return std::string(DeclUSR.str());
+
+ SmallString<128> LNBuf;
+ llvm::raw_svector_ostream LNOS(LNBuf);
+ LNOS << DeclUSR.size() << ':' << DeclUSR;
+ return std::string(LNOS.str());
}
/// Recursively visits the decls of a DeclContext, and returns one with the
@@ -429,6 +462,8 @@
return std::move(IndexLoadError);
// Check if there is and entry in the index for the function.
+ assert(IS_CTU_INDEX_KEY_VALID(FunctionName) &&
+ "Invalid USR function name when accessing CTU index.");
if (!NameFileMap.count(FunctionName)) {
++NumNotInOtherTU;
return llvm::make_error<IndexError>(index_error_code::missing_definition);
@@ -457,6 +492,9 @@
StringRef FunctionName, StringRef CrossTUDir, StringRef IndexName) {
if (llvm::Error IndexLoadError = ensureCTUIndexLoaded(CrossTUDir, IndexName))
return std::move(IndexLoadError);
+
+ assert(IS_CTU_INDEX_KEY_VALID(FunctionName) &&
+ "Invalid USR function name when accessing CTU index.");
return NameFileMap[FunctionName];
}
Index: clang/include/clang/Basic/DiagnosticCrossTUKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticCrossTUKinds.td
+++ clang/include/clang/Basic/DiagnosticCrossTUKinds.td
@@ -12,7 +12,7 @@
"error opening '%0': required by the CrossTU functionality">;
def err_extdefmap_parsing : Error<
- "error parsing index file: '%0' line: %1 'UniqueID filename' format "
+ "error parsing index file: '%0' line: %1 'USR-Length:USR File-Path' format "
"expected">;
def err_multiple_def_index : Error<
Index: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
===================================================================
--- clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
+++ clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
@@ -81,12 +81,12 @@
$
The next step is to create a CTU index file which holds the `USR` name and location of external definitions in the
-source files:
+source files in format `USR-Length:USR File-Path`:
.. code-block:: bash
$ clang-extdef-mapping -p . foo.cpp
- c:@F@foo# /path/to/your/project/foo.cpp
+ 9:c:@F@foo# /path/to/your/project/foo.cpp
$ clang-extdef-mapping -p . foo.cpp > externalDefMap.txt
We have to modify `externalDefMap.txt` to contain the name of the `.ast` files instead of the source files:
@@ -283,7 +283,7 @@
.. code-block:: bash
$ clang-extdef-mapping -p . foo.cpp
- c:@F@foo# /path/to/your/project/foo.cpp
+ 9:c:@F@foo# /path/to/your/project/foo.cpp
$ clang-extdef-mapping -p . foo.cpp > externalDefMap.txt
Now everything is available for the CTU analysis.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
