<div dir="ltr">Reverted, found the memory leak and will put up a patch to fix it & reland in a bit. Thanks!</div><br><div class="gmail_quote"><div dir="ltr">On Wed, May 9, 2018 at 3:13 PM Evgenii Stepanov <<a href="mailto:eugeni.stepanov@gmail.com">eugeni.stepanov@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>HI,</div><div><br></div><div>ASan says there is a use-after-free after this change:</div><a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/5410/steps/check-clang%20asan/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/5410/steps/check-clang%20asan/logs/stdio</a><br><div><br></div><div>MSan also sees a problem, but ASan's is likely closer to the root cause:</div><div><a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/4529/steps/check-clang%20msan/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/4529/steps/check-clang%20msan/logs/stdio</a><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 9, 2018 at 11:27 AM, Julie Hockett via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: juliehockett<br>
Date: Wed May  9 11:27:37 2018<br>
New Revision: 331905<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=331905&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=331905&view=rev</a><br>
Log:<br>
[tools] Updating PPCallbacks::InclusionDirective calls<br>
<br>
[revision] added SrcMgr::CharacteristicKind to the InclusionDirective<br>
callback, this revision updates instances of it in clang-tools-extra.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D46615" rel="noreferrer" target="_blank">https://reviews.llvm.org/D46615</a><br>
<br>
Modified:<br>
    clang-tools-extra/trunk/clang-move/ClangMove.cpp<br>
    clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp<br>
    clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp<br>
    clang-tools-extra/trunk/clang-tidy/utils/IncludeInserter.cpp<br>
    clang-tools-extra/trunk/clangd/ClangdUnit.cpp<br>
    clang-tools-extra/trunk/clangd/Headers.cpp<br>
    clang-tools-extra/trunk/modularize/CoverageChecker.cpp<br>
    clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp<br>
    clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp<br>
    clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h<br>
<br>
Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Wed May  9 11:27:37 2018<br>
@@ -131,7 +131,8 @@ public:<br>
                           clang::CharSourceRange FilenameRange,<br>
                           const clang::FileEntry * /*File*/,<br>
                           StringRef SearchPath, StringRef /*RelativePath*/,<br>
-                          const clang::Module * /*Imported*/) override {<br>
+                          const clang::Module * /*Imported*/,<br>
+                          SrcMgr::CharacteristicKind /*FileType*/) override {<br>
     if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc)))<br>
       MoveTool->addIncludes(FileName, IsAngled, SearchPath,<br>
                             FileEntry->getName(), FilenameRange, SM);<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp Wed May  9 11:27:37 2018<br>
@@ -28,7 +28,8 @@ public:<br>
                           StringRef FileName, bool IsAngled,<br>
                           CharSourceRange FilenameRange, const FileEntry *File,<br>
                           StringRef SearchPath, StringRef RelativePath,<br>
-                          const Module *Imported) override;<br>
+                          const Module *Imported,<br>
+                          SrcMgr::CharacteristicKind FileType) override;<br>
   void EndOfMainFile() override;<br>
<br>
 private:<br>
@@ -76,7 +77,8 @@ static int getPriority(StringRef Filenam<br>
 void IncludeOrderPPCallbacks::InclusionDirective(<br>
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,<br>
     bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,<br>
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported) {<br>
+    StringRef SearchPath, StringRef RelativePath, const Module *Imported,<br>
+    SrcMgr::CharacteristicKind FileType) {<br>
   // We recognize the first include as a special main module header and want<br>
   // to leave it in the top position.<br>
   IncludeDirective ID = {HashLoc, FilenameRange, FileName, IsAngled, false};<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp Wed May  9 11:27:37 2018<br>
@@ -30,7 +30,8 @@ public:<br>
                           StringRef FileName, bool IsAngled,<br>
                           CharSourceRange FilenameRange, const FileEntry *File,<br>
                           StringRef SearchPath, StringRef RelativePath,<br>
-                          const Module *Imported) override;<br>
+                          const Module *Imported,<br>
+                          SrcMgr::CharacteristicKind FileType) override;<br>
<br>
 private:<br>
   ClangTidyCheck &Check;<br>
@@ -94,7 +95,8 @@ IncludeModernizePPCallbacks::IncludeMode<br>
 void IncludeModernizePPCallbacks::InclusionDirective(<br>
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,<br>
     bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,<br>
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported) {<br>
+    StringRef SearchPath, StringRef RelativePath, const Module *Imported,<br>
+    SrcMgr::CharacteristicKind FileType) {<br>
   // FIXME: Take care of library symbols from the global namespace.<br>
   //<br>
   // Reasonable options for the check:<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/utils/IncludeInserter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/IncludeInserter.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/IncludeInserter.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/utils/IncludeInserter.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/utils/IncludeInserter.cpp Wed May  9 11:27:37 2018<br>
@@ -25,7 +25,8 @@ public:<br>
                           bool IsAngled, CharSourceRange FileNameRange,<br>
                           const FileEntry * /*IncludedFile*/,<br>
                           StringRef /*SearchPath*/, StringRef /*RelativePath*/,<br>
-                          const Module * /*ImportedModule*/) override {<br>
+                          const Module * /*ImportedModule*/,<br>
+                          SrcMgr::CharacteristicKind /*FileType*/) override {<br>
     Inserter->AddInclude(FileNameRef, IsAngled, HashLocation,<br>
                          IncludeToken.getEndLoc());<br>
   }<br>
<br>
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed May  9 11:27:37 2018<br>
@@ -93,7 +93,8 @@ public:<br>
                           StringRef FileName, bool IsAngled,<br>
                           CharSourceRange FilenameRange, const FileEntry *File,<br>
                           StringRef SearchPath, StringRef RelativePath,<br>
-                          const Module *Imported) override {<br>
+                          const Module *Imported,<br>
+                          SrcMgr::CharacteristicKind FileType) override {<br>
     auto SR = FilenameRange.getAsRange();<br>
     if (SR.isInvalid() || !File || File->tryGetRealPathName().empty())<br>
       return;<br>
<br>
Modified: clang-tools-extra/trunk/clangd/Headers.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/Headers.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/Headers.cpp Wed May  9 11:27:37 2018<br>
@@ -34,7 +34,8 @@ public:<br>
                           CharSourceRange /*FilenameRange*/,<br>
                           const FileEntry *File, llvm::StringRef /*SearchPath*/,<br>
                           llvm::StringRef /*RelativePath*/,<br>
-                          const Module * /*Imported*/) override {<br>
+                          const Module * /*Imported*/,<br>
+                          SrcMgr::CharacteristicKind /*FileType*/) override {<br>
     WrittenHeaders.insert(<br>
         (IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").str());<br>
     if (File != nullptr && !File->tryGetRealPathName().empty())<br>
<br>
Modified: clang-tools-extra/trunk/modularize/CoverageChecker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/CoverageChecker.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/CoverageChecker.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/modularize/CoverageChecker.cpp (original)<br>
+++ clang-tools-extra/trunk/modularize/CoverageChecker.cpp Wed May  9 11:27:37 2018<br>
@@ -90,7 +90,8 @@ public:<br>
                           StringRef FileName, bool IsAngled,<br>
                           CharSourceRange FilenameRange, const FileEntry *File,<br>
                           StringRef SearchPath, StringRef RelativePath,<br>
-                          const Module *Imported) override {<br>
+                          const Module *Imported,<br>
+                          SrcMgr::CharacteristicKind FileType) override {<br>
     Checker.collectUmbrellaHeaderHeader(File->getName());<br>
   }<br>
<br>
<br>
Modified: clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp (original)<br>
+++ clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp Wed May  9 11:27:37 2018<br>
@@ -750,7 +750,8 @@ public:<br>
                           const clang::FileEntry *File,<br>
                           llvm::StringRef SearchPath,<br>
                           llvm::StringRef RelativePath,<br>
-                          const clang::Module *Imported) override;<br>
+                          const clang::Module *Imported,<br>
+                          clang::SrcMgr::CharacteristicKind FileType) override;<br>
   void FileChanged(clang::SourceLocation Loc,<br>
                    clang::PPCallbacks::FileChangeReason Reason,<br>
                    clang::SrcMgr::CharacteristicKind FileType,<br>
@@ -1289,7 +1290,7 @@ void PreprocessorCallbacks::InclusionDir<br>
     llvm::StringRef FileName, bool IsAngled,<br>
     clang::CharSourceRange FilenameRange, const clang::FileEntry *File,<br>
     llvm::StringRef SearchPath, llvm::StringRef RelativePath,<br>
-    const clang::Module *Imported) {<br>
+    const clang::Module *Imported, clang::SrcMgr::CharacteristicKind FileType) {<br>
   int DirectiveLine, DirectiveColumn;<br>
   std::string HeaderPath = getSourceLocationFile(PP, HashLoc);<br>
   getSourceLocationLineAndColumn(PP, HashLoc, DirectiveLine, DirectiveColumn);<br>
<br>
Modified: clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp (original)<br>
+++ clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp Wed May  9 11:27:37 2018<br>
@@ -139,7 +139,7 @@ void PPCallbacksTracker::InclusionDirect<br>
     llvm::StringRef FileName, bool IsAngled,<br>
     clang::CharSourceRange FilenameRange, const clang::FileEntry *File,<br>
     llvm::StringRef SearchPath, llvm::StringRef RelativePath,<br>
-    const clang::Module *Imported) {<br>
+    const clang::Module *Imported, clang::SrcMgr::CharacteristicKind FileType) {<br>
   beginCallback("InclusionDirective");<br>
   appendArgument("IncludeTok", IncludeTok);<br>
   appendFilePathArgument("FileName", FileName);<br>
<br>
Modified: clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h?rev=331905&r1=331904&r2=331905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h?rev=331905&r1=331904&r2=331905&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h (original)<br>
+++ clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h Wed May  9 11:27:37 2018<br>
@@ -102,7 +102,8 @@ public:<br>
                           const clang::FileEntry *File,<br>
                           llvm::StringRef SearchPath,<br>
                           llvm::StringRef RelativePath,<br>
-                          const clang::Module *Imported) override;<br>
+                          const clang::Module *Imported,<br>
+                          clang::SrcMgr::CharacteristicKind FileType) override;<br>
   void moduleImport(clang::SourceLocation ImportLoc, clang::ModuleIdPath Path,<br>
                     const clang::Module *Imported) override;<br>
   void EndOfMainFile() override;<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</blockquote></div>