RefactoringTool does not rewrite sources in another directory

[maleadt]

Bugzilla Link	22385
Version	3.6
OS	Linux
CC	@Sarcasm,@zmodem

Extended Description

If I use the RefactoringTool to match and rewrite files which reside in a subdirectory, matching works fine, but Rewriting fails because the sources are not found anymore (the directory component is stripped off).

For example, demonstrating using remove-cstr-calls from clang-tools-extra, if I have a file main.cpp in /tmp/test/src, and an accompanying compile_database.json (in the same directory) looking as follows: [ { "command": "g++ -c -o main.o main.cpp", "directory": "/tmp/test/src", "file": "/tmp/test/src/main.cpp" } ] (this compile database is generated using Bear)

If I now call the tooling binary from /tmp/test using remove-cstr-calls src src/main.cpp, I get:

fatal error: cannot open file 'main.cpp': No such file or directory remove-cstr-calls: ../../tools/clang/include/clang/Rewrite/Core/RewriteRope.h:203: void clang::RewriteRope::erase(unsigned int, unsigned int): Assertion `Offset+NumBytes <= size() && "Invalid region to erase!"' failed.

​0 0x7fc86c9a9c48 llvm::sys::PrintStackTrace(_IO_FILE*) ../../lib/Support/Unix/Signals.inc:422:15

​1 0x7fc86c9ab24b SignalHandler(int) ../../lib/Support/Unix/Signals.inc:198:28

​2 0x7fc86a7b4b20 __restore_rt (/usr/lib/libc.so.6+0x33b20)

​3 0x7fc86a7b4a97 __GI_raise (/usr/lib/libc.so.6+0x33a97)

​4 0x7fc86a7b5e6a __GI_abort (/usr/lib/libc.so.6+0x34e6a)

​5 0x7fc86a7ad8bd __assert_fail_base (/usr/lib/libc.so.6+0x2c8bd)

​6 0x7fc86a7ad972 (/usr/lib/libc.so.6+0x2c972)

​7 0x7fc86798f7ce (bin/../lib/../lib/libclangRewrite.so+0x87ce)

​8 0x7fc867990788 clang::Rewriter::ReplaceText(clang::SourceLocation, unsigned int, llvm::StringRef) ../../tools/clang/lib/Rewrite/Rewriter.cpp:311:3

​9 0x7fc86b35105d clang::tooling::Replacement::apply(clang::Rewriter&) const ../../tools/clang/lib/Tooling/Core/Replacement.cpp:73:28

​10 0x7fc86b351a3e clang::tooling::applyAllReplacements(std::set<clang::tooling::Replacement, std::less, std::allocator > const&, clang::Rewriter&) ../../tools/clang/lib/Tooling/Core/Replacement.cpp:234:16

​11 0x7fc86b567898 clang::tooling::RefactoringTool::runAndSave(clang::tooling::FrontendActionFactory*) ../../tools/clang/lib/Tooling/Refactoring.cpp:48:8

​12 0x404eed main ../../tools/clang/tools/extra/remove-cstr-calls/RemoveCStrCalls.cpp:236:10

​13 0x7fc86a7a1040 __libc_start_main (/usr/lib/libc.so.6+0x20040)

​14 0x402f78 _start (bin/remove-cstr-calls+0x402f78)

Aborted (core dumped)

If I call the binary from /tmp/test/src (the directory where main.cpp resides) using remove-cstr-calls . main.cpp, everything works fine and the file is properly rewritten.

I'm using LLVM/Clang from the 3.6 branch, at revision 227398.

[zmodem]

Hans, I think, this shouldn't block the release, as there's an easy workaround (run from the directory where the compilation database points to).

Sounds reasonable.

[llvmbot]

I meant clang-tidy crashes when outputting an error message in this case. And clang-check works fine.

[llvmbot]

Hans, I think, this shouldn't block the release, as there's an easy workaround (run from the directory where the compilation database points to).

But yeah, it would be nice to fix, and I'm looking into this. BTW, clang-check fails this way when trying to print an error message as well.

[maleadt]

It seems like Replacement::setFromSourceLocation fails to make the path absolute (fs::make_absolute fails) because the FileEntry it gets from the SourceManager does not contain a full path. In case of the test set-up I used above, the FileEntry contains file="main.cpp" dir="." I tried to look further but couldn't easily find where the SourceManager is populated.

[zmodem]

Not yet, unfortunately. Trying to switch to this.

Any update?

I'm not sure if this is worth blocking the release on, but it sounds like it would be nice to get fixed.

[llvmbot]

Not yet, unfortunately. Trying to switch to this.

[zmodem]

Has there been any progress on this one?

[llvmbot]

The problem is that the paths in replacements turn out to be relative. That seems strange to me, as Replacement::setFromSourceLocation which seems to be used there, makes paths absolute.

I'll take a look at this tomorrow.

[zmodem]

I've bisected this to r221600.

Alex, can you take a look?

[maleadt]

I've bisected this to r221600.

[maleadt]

I've tested this bug with remove-cstr-calls compiled against LLVM/Clang 3.5.1, and everything works as expected (refactoring files in a subdirectory, that is). So this seems like a regression from 3.5 to 3.6.

Some more details, the file I tested to rewrite:

----main.cpp--------------------

include

int main() { std::string Hello = "World"; std::out << std::string(Hello.c_str()) << std::endl;

return 1;

}

I checked out RemoveCstrCalls.cpp from release_35, compiled it against my system 3.5.1 LLVM installation, and called the binary as follows:

in /tmp/test: $ llvm-3.5/remove-cstr-calls src src/main.cpp

This works without error, while using remove-cstr-calls from 3.6 fails.