Author: kfunk Date: Tue Jul 25 07:28:16 2017 New Revision: 308974 URL: http://llvm.org/viewvc/llvm-project?rev=308974&view=rev Log: [clang-tidy] clang-apply-replacements: Don't insert null entry
Summary: [clang-tidy] clang-apply-replacements: Don't insert null entry Fix crash when running clang-apply-replacements on YML files which contain an invalid file path. Make sure we never add a nullptr into the map. The previous code started adding nullptr to the map after the first warnings via errs() has been emitted. Backtrace: ``` Starting program: /home/kfunk/devel/build/llvm/bin/clang-apply-replacements /tmp/tmpIqtp7m [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Described file '.moc/../../../../../../src/qt5.8/qtremoteobjects/src/remoteobjects/qremoteobjectregistrysource_p.h' doesn't exist. Ignoring... ... Program received signal SIGSEGV, Segmentation fault. main (argc=<optimized out>, argv=<optimized out>) at /home/kfunk/devel/src/llvm/tools/clang/tools/extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:262 (gdb) p FileAndReplacements.first $1 = (const clang::FileEntry *) 0x0 (gdb) ``` Added tests. Before patch: ``` ******************** TEST 'Clang Tools :: clang-apply-replacements/invalid-files.cpp' FAILED ******************** Script: -- mkdir -p /home/kfunk/devel/build/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/invalid-files clang-apply-replacements /home/kfunk/devel/build/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/invalid-files ls -1 /home/kfunk/devel/build/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/invalid-files | FileCheck /home/kfunk/devel/src/llvm/tools/clang/tools/extra/test/clang-apply-replacements/invalid-files.cpp --check-prefix=YAML -- Exit Code: 139 Command Output (stderr): -- Described file 'idonotexist.h' doesn't exist. Ignoring... /home/kfunk/devel/build/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Output/invalid-files.cpp.script: line 4: 9919 Segmentation fault clang-apply-replacements /home/kfunk/devel/build/llvm/tools/clang/tools/extra/test/clang-apply- replacements/Output/Inputs/invalid-files -- ``` After Patch: ``` PASS: Clang Tools :: clang-apply-replacements/invalid-files.cpp (5 of 6) ``` Reviewers: alexfh, yawanng Reviewed By: alexfh Subscribers: cfe-commits, klimek, JDevlieghere, xazax.hun Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D35194 Added: clang-tools-extra/trunk/test/clang-apply-replacements/Inputs/invalid-files/ clang-tools-extra/trunk/test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml clang-tools-extra/trunk/test/clang-apply-replacements/invalid-files.cpp Modified: clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp Modified: clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp?rev=308974&r1=308973&r2=308974&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp (original) +++ clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp Tue Jul 25 07:28:16 2017 @@ -288,13 +288,12 @@ bool mergeAndDeduplicate(const TUReplace for (const tooling::Replacement &R : TU.Replacements) { // Use the file manager to deduplicate paths. FileEntries are // automatically canonicalized. - const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath()); - if (!Entry && Warned.insert(R.getFilePath()).second) { - errs() << "Described file '" << R.getFilePath() - << "' doesn't exist. Ignoring...\n"; - continue; + if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) { + GroupedReplacements[Entry].push_back(R); + } else if (Warned.insert(R.getFilePath()).second) { + errs() << "Described file '" << R.getFilePath() + << "' doesn't exist. Ignoring...\n"; } - GroupedReplacements[Entry].push_back(R); } } @@ -314,13 +313,12 @@ bool mergeAndDeduplicate(const TUDiagnos for (const tooling::Replacement &R : Fix.second) { // Use the file manager to deduplicate paths. FileEntries are // automatically canonicalized. - const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath()); - if (!Entry && Warned.insert(R.getFilePath()).second) { - errs() << "Described file '" << R.getFilePath() - << "' doesn't exist. Ignoring...\n"; - continue; + if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) { + GroupedReplacements[Entry].push_back(R); + } else if (Warned.insert(R.getFilePath()).second) { + errs() << "Described file '" << R.getFilePath() + << "' doesn't exist. Ignoring...\n"; } - GroupedReplacements[Entry].push_back(R); } } } Added: clang-tools-extra/trunk/test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml?rev=308974&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml (added) +++ clang-tools-extra/trunk/test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml Tue Jul 25 07:28:16 2017 @@ -0,0 +1,12 @@ +--- +MainSourceFile: '' +Replacements: + - FilePath: idontexist.h + Offset: 2669 + Length: 0 + ReplacementText: ' override' + - FilePath: idontexist.h + Offset: 2669 + Length: 0 + ReplacementText: ' override' +... Added: clang-tools-extra/trunk/test/clang-apply-replacements/invalid-files.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-apply-replacements/invalid-files.cpp?rev=308974&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-apply-replacements/invalid-files.cpp (added) +++ clang-tools-extra/trunk/test/clang-apply-replacements/invalid-files.cpp Tue Jul 25 07:28:16 2017 @@ -0,0 +1,5 @@ +// RUN: mkdir -p %T/invalid-files +// RUN: clang-apply-replacements %T/invalid-files +// +// Check that the yaml files are *not* deleted after running clang-apply-replacements without remove-change-desc-files. +// RUN: ls %T/Inputs/invalid-files/invalid-files.yaml _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits