diff options
author | Kevin Funk <kfunk@kde.org> | 2017-07-25 14:28:16 +0000 |
---|---|---|
committer | Kevin Funk <kfunk@kde.org> | 2017-07-25 14:28:16 +0000 |
commit | bb5cfb5dd394f9f3eccf79ce8e2f18c50b016903 (patch) | |
tree | bd6fec8e797e14b6533494d3e50c86832a010d5b | |
parent | b6317007b4cddfa473e856968bcba43cbf0435eb (diff) | |
download | bcm5719-llvm-bb5cfb5dd394f9f3eccf79ce8e2f18c50b016903.tar.gz bcm5719-llvm-bb5cfb5dd394f9f3eccf79ce8e2f18c50b016903.zip |
[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
llvm-svn: 308974
3 files changed, 27 insertions, 12 deletions
diff --git a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp index 52234fed854..56482dc7394 100644 --- a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp +++ b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp @@ -288,13 +288,12 @@ bool mergeAndDeduplicate(const TUReplacements &TUs, 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 TUDiagnostics &TUs, 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); } } } diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml new file mode 100644 index 00000000000..f84964d9664 --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml @@ -0,0 +1,12 @@ +--- +MainSourceFile: '' +Replacements: + - FilePath: idontexist.h + Offset: 2669 + Length: 0 + ReplacementText: ' override' + - FilePath: idontexist.h + Offset: 2669 + Length: 0 + ReplacementText: ' override' +... diff --git a/clang-tools-extra/test/clang-apply-replacements/invalid-files.cpp b/clang-tools-extra/test/clang-apply-replacements/invalid-files.cpp new file mode 100644 index 00000000000..9d3dd537c87 --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/invalid-files.cpp @@ -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 |