[clang][ASTImporter] Fix possible crash at import of function template #124273
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
During import of a function template at specific conditions an assertion "'TemplateOrSpecialization.isNull()" can be triggered. This can happen when the new AST is already incompatible after import failures. Problem is fixed by returning import failure at the assert condition.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Balázs Kéri (balazske) ChangesDuring import of a function template at specific conditions an assertion Full diff: https://github.com/llvm/llvm-project/pull/124273.diff 4 Files Affected:
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 0669aa1b809c34..34b49963c00ec1 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6755,6 +6755,14 @@ ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
Params, TemplatedFD))
return ToFunc;
+ // Fail if TemplatedFD is already part of a template.
+ // The template should have been found by structural equivalence check before,
+ // or ToFunc should be already imported.
+ // If not, there is AST incompatibility that can be caused by previous import
+ // errors. (NameConflict is not exact here.)
+ if (TemplatedFD->getDescribedTemplate())
+ return make_error<ASTImportError>(ASTImportError::NameConflict);
+
TemplatedFD->setDescribedFunctionTemplate(ToFunc);
ToFunc->setAccess(D->getAccess());
diff --git a/clang/test/Analysis/Inputs/ctu-test-import-failure-import.cpp b/clang/test/Analysis/Inputs/ctu-test-import-failure-import.cpp
new file mode 100644
index 00000000000000..d5e6ba8852872e
--- /dev/null
+++ b/clang/test/Analysis/Inputs/ctu-test-import-failure-import.cpp
@@ -0,0 +1,56 @@
+namespace std {
+inline namespace __cxx11 {
+template <typename _CharT, typename = int, typename = _CharT>
+class basic_string;
+}
+template <typename, typename> class basic_istream;
+template <typename> struct __get_first_arg;
+template <typename _Ptr> using __ptr_traits_elem_t = __get_first_arg<_Ptr>;
+template <typename> struct __ptr_traits_impl;
+template <typename _Ptr>
+struct pointer_traits : __ptr_traits_impl<__ptr_traits_elem_t<_Ptr>> {};
+struct allocator_traits {
+ using type = pointer_traits<int>;
+};
+} // namespace std
+namespace std {
+inline namespace __cxx11 {
+template <typename, typename, typename> class basic_string {
+ allocator_traits _M_allocated_capacity;
+ void _M_assign();
+};
+} // namespace __cxx11
+} // namespace std
+namespace std {
+template <typename _CharT, typename _Alloc> void operator!=(_Alloc, _CharT);
+template <typename _CharT, typename _Traits, typename _Alloc>
+basic_istream<_CharT, _Traits> &getline(basic_istream<_CharT, _Traits> &,
+ basic_string<_CharT, _Traits, _Alloc> &,
+ _CharT);
+} // namespace std
+namespace std {
+template <typename _CharT, typename _Traits, typename _Alloc>
+void basic_string<_CharT, _Traits, _Alloc>::_M_assign() {
+ this != 0;
+}
+template <typename _CharT, typename _Traits, typename _Alloc>
+basic_istream<_CharT, _Traits> &getline(basic_istream<_CharT, _Traits> &,
+ basic_string<_CharT, _Traits, _Alloc> &,
+ _CharT) {}
+} // namespace std
+struct CommandLineOptionDefinition {
+ void *OutAddress;
+};
+struct CommandLineCommand {
+ CommandLineOptionDefinition Options;
+};
+namespace CommandLine {
+extern const CommandLineCommand RootCommands[];
+extern const int RootExamples[];
+} // namespace CommandLine
+using utf8 = char;
+using u8string = std::basic_string<utf8>;
+u8string _rct2DataPath;
+CommandLineOptionDefinition StandardOptions{&_rct2DataPath};
+const CommandLineCommand CommandLine::RootCommands[]{StandardOptions};
+const int CommandLine::RootExamples[]{};
diff --git a/clang/test/Analysis/Inputs/ctu-test-import-failure-import.cpp.externalDefMap.ast-dump.txt b/clang/test/Analysis/Inputs/ctu-test-import-failure-import.cpp.externalDefMap.ast-dump.txt
new file mode 100644
index 00000000000000..6ffb3795d3e36a
--- /dev/null
+++ b/clang/test/Analysis/Inputs/ctu-test-import-failure-import.cpp.externalDefMap.ast-dump.txt
@@ -0,0 +1,5 @@
+47:c:@N@std@S@allocator_traits@F@allocator_traits# ctu-test-import-failure-import.cpp.ast
+29:c:@N@CommandLine@RootCommands ctu-test-import-failure-import.cpp.ast
+55:c:@N@std@N@__cxx11@ST>3#T#T#T@basic_string@F@_M_assign# ctu-test-import-failure-import.cpp.ast
+97:c:@S@CommandLineOptionDefinition@F@CommandLineOptionDefinition#&1$@S@CommandLineOptionDefinition# ctu-test-import-failure-import.cpp.ast
+29:c:@N@CommandLine@RootExamples ctu-test-import-failure-import.cpp.ast
\ No newline at end of file
diff --git a/clang/test/Analysis/ctu-test-import-failure.cpp b/clang/test/Analysis/ctu-test-import-failure.cpp
new file mode 100644
index 00000000000000..c586b9aa9a3107
--- /dev/null
+++ b/clang/test/Analysis/ctu-test-import-failure.cpp
@@ -0,0 +1,34 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -std=c++17 \
+// RUN: -emit-pch -o %t/ctudir/ctu-test-import-failure-import.cpp.ast %S/Inputs/ctu-test-import-failure-import.cpp
+// RUN: cp %S/Inputs/ctu-test-import-failure-import.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -std=c++17 -analyze \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN: -analyzer-config ctu-dir=%t/ctudir \
+// RUN: -verify %s
+
+// Check that importing this code does not cause crash.
+// Import intentionally fails because mismatch of '__get_first_arg'.
+
+namespace std {
+inline namespace __cxx11 {}
+template <typename _CharT, typename> class basic_istream;
+struct __get_first_arg;
+inline namespace __cxx11 {
+template <typename, typename, typename> class basic_string;
+}
+template <typename _CharT, typename _Traits, typename _Alloc>
+basic_istream<_CharT, _Traits> &getline(basic_istream<_CharT, _Traits> &,
+ basic_string<_CharT, _Traits, _Alloc> &,
+ _CharT) {}
+} // namespace std
+namespace CommandLine {
+extern const int RootExamples[];
+}
+
+// expected-warning@Inputs/ctu-test-import-failure-import.cpp:18{{incompatible definitions}}
+// expected-warning@Inputs/ctu-test-import-failure-import.cpp:18{{incompatible definitions}}
+// expected-note@Inputs/ctu-test-import-failure-import.cpp:18{{no corresponding field here}}
+// expected-note@Inputs/ctu-test-import-failure-import.cpp:18{{no corresponding field here}}
|
Michael137
reviewed
Jan 24, 2025
There was a problem hiding this comment.
So in spirit this is kind of like an ODR violation right? Which the ASTImporter does try to handle, so this seems reasonable to me. For my own understanding, where did the crash end up happening?
Also, I know you've probably already reduced the reproducer substantially, but is there any chance of making the test even smaller? I assume a mismatching __get_first_arg is not all it takes to trigger the crash, so I understand if it's difficult to reduce even further.
|
If this case is not handled, the following |
Ah thanks for the context. LGTM Shorter test-case would be great, but not blocking this on it |
Michael137
approved these changes
Jan 27, 2025
|
It seems that this test is not reducible any further (except very small changes) (to get the original crash). I could remove only few lines. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/12562 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During import of a function template at specific conditions an assertion
"TemplateOrSpecialization.isNull()" can be triggered. This can
happen when the new AST is already incompatible after import failures.
Problem is fixed by returning import failure at the assert condition.