llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Balázs Benics (steakhal) <details> <summary>Changes</summary> This is a use-after-free. Codegen would drop the AST before starting the optimizations on the LLVM IR level. This means that the ASTConsumers of the SSAF extractors only had dangling TU Decls etc. For now, let's override this option to force-keep the AST alive. Note that PluginActions already did the same if their consumers were added after the main frontend-action. See: https://github.com/llvm/llvm-project/blob/69e0367e8221b8002b5d438fb70ff3daf36257fc/clang/lib/Frontend/FrontendAction.cpp#L470 ```c++ CI.getCodeGenOpts().ClearASTBeforeBackend = false; ``` Long term, we could think about the stability implications of running the extractors before codegen to be able to drop the AST, thus save memory for codegen. However, that would mean that if any of the virtual callbacks of the ASTConsumers (extractors) would crash/fail, it would drag down the whole process. If extractors would only use the HandleTranslationUnitDecl callback, then we could wrap our `TUSummaryRunner` multiplexer in a crash-safe `CrashRecoveryContext` - which would ensure that only the runner would fail, but would continue for codegen. Personally, I don't really like this continue-after-extractors-crash, because I'd not put my life on the binary generated such an event. And if that's the case, then what's the point... Nevertheless, let's force keep the AST for now. --- Full diff: https://github.com/llvm/llvm-project/pull/191058.diff 2 Files Affected: - (modified) clang/lib/ScalableStaticAnalysisFramework/Frontend/TUSummaryExtractorFrontendAction.cpp (+1) - (added) clang/test/Analysis/Scalable/extraction-works-alongside-compilation.cpp (+16) ``````````diff diff --git a/clang/lib/ScalableStaticAnalysisFramework/Frontend/TUSummaryExtractorFrontendAction.cpp b/clang/lib/ScalableStaticAnalysisFramework/Frontend/TUSummaryExtractorFrontendAction.cpp index d4bf8d4a63fa4..1c5170e354fb5 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Frontend/TUSummaryExtractorFrontendAction.cpp +++ b/clang/lib/ScalableStaticAnalysisFramework/Frontend/TUSummaryExtractorFrontendAction.cpp @@ -175,6 +175,7 @@ TUSummaryExtractorFrontendAction::CreateASTConsumer(CompilerInstance &CI, return nullptr; if (auto Runner = TUSummaryRunner::create(CI, InFile)) { + CI.getCodeGenOpts().ClearASTBeforeBackend = false; std::vector<std::unique_ptr<ASTConsumer>> Consumers; Consumers.reserve(2); Consumers.push_back(std::move(WrappedConsumer)); diff --git a/clang/test/Analysis/Scalable/extraction-works-alongside-compilation.cpp b/clang/test/Analysis/Scalable/extraction-works-alongside-compilation.cpp new file mode 100644 index 0000000000000..0077d557e31bc --- /dev/null +++ b/clang/test/Analysis/Scalable/extraction-works-alongside-compilation.cpp @@ -0,0 +1,16 @@ +// DEFINE: %{filecheck} = FileCheck %s --match-full-lines --check-prefix +// DEFINE: %{codegen} = %clang -c %s -o %t.o -mllvm -debug-only=codegenaction 2>&1 + +// The codegen action should not clear the AST before running codegen. +// Codegen should otherwise clear the AST if ran without extraction. + +// RUN: rm -rf %t.o %t.json +// RUN: %{codegen} | grep "Clearing AST" + +// RUN: rm -rf %t.o %t.json +// RUN: %{codegen} \ +// RUN: --ssaf-extract-summaries=CallGraph \ +// RUN: --ssaf-tu-summary-file=%t.json \ +// RUN: | not grep "Clearing AST" + +void empty() {} `````````` </details> https://github.com/llvm/llvm-project/pull/191058 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
