This is an archive of the discontinued LLVM Phabricator instance.

[libclang] Add clang_File_tryGetRealPathName
ClosedPublic

Authored by MaskRay on Feb 4 2018, 11:14 AM.

Details

Summary

clang_getFileName() may return a path relative to WorkingDir.
On Arch Linux, during clang_indexTranslationUnit(), clang_getFileName() on
CXIdxIncludedIncludedFileInfo::file may return
"/../lib64/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/string",
for #include <string>.

I presume WorkingDir is somehow changed to /usr/lib or /usr/include and
clang_getFileName() returns a path relative to WorkingDir.

clang_File_tryGetRealPathName() returns a better file name more useful for
the indexer in this case.

Diff Detail

Event Timeline

MaskRay created this revision.Feb 4 2018, 11:14 AM
MaskRay updated this revision to Diff 132773.Feb 4 2018, 11:15 AM

description

MaskRay updated this revision to Diff 132774.Feb 4 2018, 11:20 AM

Increase CINDEX_VERSION_MINOR

MaskRay updated this revision to Diff 132775.Feb 4 2018, 11:21 AM

description

Harbormaster completed remote builds in B14601: Diff 132775.

On Arch Linux, ../lib64/gcc/x86_64-pc-linux-gnu/7.2.1/../../../../include/c++/7.2.1 (Name) resolves to a path that requires leading path components (/usr/include), this kind of resembles jailbreak.

jbcoe requested changes to this revision.Mar 31 2018, 11:01 AM

Can you add some simple tests?

This revision now requires changes to proceed.Mar 31 2018, 11:01 AM

Can you add some simple tests?

Where should I add tests? I cannot find tests for other clang_File_* functions.

There is no unittest but I have verified with a C++ language server on Arch Linux. Without using this function, cquery/ccls will not be able to index system header files because of excessive .. in the path returned by clang_getFileName.

jbcoe added a comment.Apr 7 2018, 12:33 AM

I’ll see if I can find a suitable location for a test.

jbcoe added a comment.Apr 7 2018, 9:07 AM

Tests for libclang are in clang/unittests/libclang/LibclangTest.cpp

Given the surgical nature of this change I hope it will be quick to add a test.

MaskRay updated this revision to Diff 141496.Apr 7 2018, 11:04 AM

Add unittests/libclang/LibclangTest.cpp test

Done.

ninja -C ~/Dev/llvm/debug unittests/libclang/libclangTests
~/Dev/llvm/debug/tools/clang/unittests/libclang/libclangTests
jbcoe accepted this revision.Apr 7 2018, 12:31 PM
This revision is now accepted and ready to land.Apr 7 2018, 12:31 PM
This revision was automatically updated to reflect the committed changes.