From fa5f2ec781f6f26d2798b300b1e773beaa47a018 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Mon, 4 May 2015 14:59:20 +0000 Subject: [PATCH] Reapply "Frontend: Stop leaking when not -disable-free" This reverts commit r236422, effectively reapplying r236419. ASan helped me diagnose the problem: the non-leaking logic would free the ASTConsumer before freeing Sema whenever `isCurrentASTFile()`, causing a use-after-free in `Sema::~Sema()`. This version unconditionally frees Sema and the ASTContext before freeing the ASTConsumer. Without the fix, these were either being freed before the ASTConsumer was freed or leaked after, but they were always spiritually released so this isn't really a functionality change. I ran all of check-clang with ASan locally this time, so I'm hoping there aren't any more problems lurking. Original commit message: Try again to plug a leak that's been around since at least r128011 after coming across the FIXME. Nico Weber tried something similar in r207065 but had to revert in r207070 due to a bot failure. The build failure isn't visible anymore so I'm not sure what went wrong. I'm doing this slightly differently -- when not -disable-free I'm still resetting the members (just not leaking them) -- so maybe it will work out this time? Tests pass locally, anyway. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@236424 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Frontend/FrontendAction.cpp | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp index d42ca718c3..9bba75541f 100644 --- a/lib/Frontend/FrontendAction.cpp +++ b/lib/Frontend/FrontendAction.cpp @@ -468,16 +468,12 @@ void FrontendAction::EndSourceFile() { // FIXME: There is more per-file stuff we could just drop here? bool DisableFree = CI.getFrontendOpts().DisableFree; if (DisableFree) { - if (!isCurrentFileAST()) { - CI.resetAndLeakSema(); - CI.resetAndLeakASTContext(); - } + CI.resetAndLeakSema(); + CI.resetAndLeakASTContext(); BuryPointer(CI.takeASTConsumer().get()); } else { - if (!isCurrentFileAST()) { - CI.setSema(nullptr); - CI.setASTContext(nullptr); - } + CI.setSema(nullptr); + CI.setASTContext(nullptr); CI.setASTConsumer(nullptr); } @@ -494,13 +490,16 @@ void FrontendAction::EndSourceFile() { // FrontendAction. CI.clearOutputFiles(/*EraseFiles=*/shouldEraseOutputFiles()); - // FIXME: Only do this if DisableFree is set. if (isCurrentFileAST()) { - CI.resetAndLeakSema(); - CI.resetAndLeakASTContext(); - CI.resetAndLeakPreprocessor(); - CI.resetAndLeakSourceManager(); - CI.resetAndLeakFileManager(); + if (DisableFree) { + CI.resetAndLeakPreprocessor(); + CI.resetAndLeakSourceManager(); + CI.resetAndLeakFileManager(); + } else { + CI.setPreprocessor(nullptr); + CI.setSourceManager(nullptr); + CI.setFileManager(nullptr); + } } setCompilerInstance(nullptr); -- 2.50.1