[clang-tools-extra] r308974 - [clang-tidy] clang-apply-replacements: Don't insert null entry
Kevin Funk via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 25 07:28:16 PDT 2017
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
More information about the cfe-commits
mailing list