[PATCH] [clang-tidy] Fix ClangTidyMessage for files included with relative paths

[Aaron Wishnick]

Thanks for taking a look. Here are steps to reproduce the issue. In short,
it's important that clang-tidy is run in a separate working directory from
the source code. In the stack trace, the issue is that the SourceManager
can't find the header. In ErrorReporter::getLocation(), FilePath is
"../header.h", and the working directory is
"/Users/awishnick/clang-tidy/build_debug".

awishnick-mac:build_debug awishnick$ cat
/Users/awishnick/clang-tidy-bug/header.h
void f(int n) {
  int *p = 0;
  *p = n;
}
awishnick-mac:build_debug awishnick$ cat
/Users/awishnick/clang-tidy-bug/src/source.c
#include "header.h"
int main() { f(4); }
awishnick-mac:build_debug awishnick$ cat
/Users/awishnick/clang-tidy-bug/src/compile_commands.json
[
{
  "directory": "/Users/awishnick/clang-tidy-bug/src",
  "command": "clang -I../ source.c",
  "file": "/Users/awishnick/clang-tidy-bug/src/source.c"
}
]
awishnick-mac:build_debug awishnick$ bin/clang-tidy -p
/Users/awishnick/clang-tidy-bug/src/compile_commands.json
/Users/awishnick/clang-tidy-bug/src/source.c
Assertion failed: (FileEnt && "Didn't specify a file entry to use?"),
function getOrCreateContentCache, file
/Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/SourceManager.cpp,
line 428.

On Fri, Nov 14, 2014 at 7:47 AM, Alexander Kornienko <alexfh at google.com>
wrote:

> This sounds like something that could break after multiple translation
> units support was fixed in r221600. Your analysis is sound, but I couldn't
> easily reproduce this issue on LLVM. Can you provide an example of a
> compilation database where this happens (or just the relevant part of it)?
>
> Also, the solution you propose will not work with a virtual file system
> which may be the case when clang-tidy is used as a library. We need a way
> to recover correct path while keeping the file names exactly as they came
> from the SourceManager. Maybe store some additional information with
> clang-tidy error messages or something.
>
> But first, I'd like to be able to reproduce this.
>
> On Thu, Nov 13, 2014 at 8:26 PM, Aaron Wishnick <
> aaron.s.wishnick at gmail.com> wrote:
>
>> I have an updated patch for this; I found one issue when a diagnostic is
>> emitted where the SourceManager doesn't have a corresponding filename.
>>
>> On Wed, Nov 12, 2014 at 5:40 PM, Aaron Wishnick <
>> aaron.s.wishnick at gmail.com> wrote:
>>
>>> I found a bug when running clang-tidy with a compilation database that
>>> specified its include paths as a relative path. The bug occurred when a
>>> diagnostic was emitted with a note in a file it included, where the file's
>>> include path was specified in the compilation database as a relative path
>>> (i.e. "-I../include"). When the ClangTidyMessage is first created in
>>> emitDiagnosticMessage, it asks the SourceManager for the filename
>>> corresponding to the location, and stores only that.
>>>
>>> The bug is that this could be a relative path. Later on, when it goes to
>>> render the diagnostic, the FileManager can't find the file by its relative
>>> path, causing an assertion in SourceManager.cpp, line 428: "Didn't specify
>>> a file entry to use?". This happens because ClangTool::run() changes to the
>>> directory specified in the compilation database by calling chdir(), and
>>> then restores the previous working directory. When the ClangTidyMessage is
>>> created, the working directory is the one specified in the compilation
>>> database, so the relative path works, but when it's rendered later on, the
>>> working directory has been restored to clang-tidy's initial working
>>> directory.
>>>
>>> My fix is for ClangTidyMessage to store an absolute FilePath in its
>>> constructor. I've attached the patch.
>>>
>>> Thanks,
>>> Aaron
>>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141114/95f8549e/attachment.html>