RFC: Fix libclang default include paths on FreeBSD.
AbandonedPublic

Authored by arrowd on Jun 19 2018, 3:14 PM.

Details

Reviewers
mwolff
aaronpuchert
Group Reviewers
KDevelop
Summary

This change fixes test_problems test on FreeBSD. Without that /usr/include isn't added to the default include paths and many system headers become missing.
I do understand that this is probably wrong place to make such a change, but after some research it turns out that Clang devs didn't put the logic for properly setting
default include paths into libclang. It is present in clang itself, but not libclang.

Relevant thread in another libclang consumer: https://github.com/Valloric/YouCompleteMe/issues/2330

So, what would be the proper fix for that?

Diff Detail

Repository
R32 KDevelop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 864
Build 877: arc lint + arc unit
arrowd created this revision.Jun 19 2018, 3:14 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptJun 19 2018, 3:14 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
arrowd requested review of this revision.Jun 19 2018, 3:14 PM

The include directories should be provided by the custom-definesandincludes plugin, specifically the compiler provider. See GccLikeCompiler::includes in plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp. Why does that not work?

The include directories should be provided by the custom-definesandincludes plugin, specifically the compiler provider. See GccLikeCompiler::includes in plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp. Why does that not work?

Yes, and it, presumably, works in the wild. But test_problems test doesn't use this analyzing the code. Maybe the code for the test itself should be changed?

But test_problems test doesn't use this analyzing the code. Maybe the code for the test itself should be changed?

Not entirely sure, but this seems to be the right thing then. Because in the actual application, the include directories should come from the includes&defines-plugin, so there should be no need to add /usr/include. The only directory that makes sense to add here is the clang-internal include directory (something like /usr/lib64/clang/<version>/include usually), which GccLikeCompiler::includes will not produce if the compiler is GCC.

mwolff requested changes to this revision.Jun 21 2018, 3:59 PM
mwolff added a subscriber: mwolff.

I also believe that this is the wrong way for fixing this.

This revision now requires changes to proceed.Jun 21 2018, 3:59 PM

I also believe that this is the wrong way for fixing this.

I was asking what's the right way. Do you think I should change the test?

I was asking what's the right way. Do you think I should change the test?

I think it would be a good first step to understand the problem, because the test seems to work on Linux. Which test specifically fails and how? I see that some tests use system headers or C++ standard library headers (e.g. testMissingInclude_data uses <vector>), but they all work on my machine.

I was asking what's the right way. Do you think I should change the test?

I think it would be a good first step to understand the problem, because the test seems to work on Linux. Which test specifically fails and how? I see that some tests use system headers or C++ standard library headers (e.g. testMissingInclude_data uses <vector>), but they all work on my machine.

Yes, it is exactly testMissingInclude that fails. It should generate only one problem, but on FreeBSD it also contains:

Error: 'sys/endian.h' file not found in /usr/include/c++/v1/__config:[(212,10),(212,24)]: <no explanation> (found by Semantic analysis)" ,
"Error: 'wchar.h' file not found in /usr/include/c++/v1/wchar.h:[(118,14),(118,23)]: <no explanation> (found by Semantic analysis)" ,
"Error: 'stddef.h' file not found in /usr/include/c++/v1/cstddef:[(43,14),(43,24)]: <no explanation> (found by Semantic analysis)" ,
"Error: 'string.h' file not found in /usr/include/c++/v1/string.h:[(60,14),(60,24)]: <no explanation> (found by Semantic analysis)" ,
"Error: 'stdint.h' file not found in /usr/include/c++/v1/stdint.h:[(118,14),(118,24)]: <no explanation> (found by Semantic analysis)" ,
"Error: 'stdlib.h' file not found in /usr/include/c++/v1/stdlib.h:[(93,14),(93,24)]: <no explanation> (found by Semantic analysis)" ,
"Error: 'assert.h' file not found in /usr/include/c++/v1/cassert:[(20,9),(20,19)]: <no explanation> (found by Semantic analysis)" ,
"Error: 'limits.h' file not found in /usr/include/c++/v1/limits.h:[(57,14),(57,24)]: <no explanation> (found by Semantic analysis)" ,
"Error: 'limits.h' file not found in /usr/include/c++/v1/limits.h:[(61,14),(61,24)]: <no explanation> (found by Semantic analysis)"

I think, the cause is that standard C++ library on FreeBSD is libc++, and it uses libc headers under the hood.

I guess this comes from the second test row ("ignore-moc-at-end")? I'm not sure if <vector> includes system headers, but I replaced it with <cstdlib>, which obviously includes <stdlib.h> from /usr/include. That still works here.

Apparently /usr/include/c++/v1 contains files with the same names as the system headers, for example stdlib.h, which includes another stdlib.h. There are not so many wrappers on my machine, /usr/include/c++/8/*.h matches only six files. But among them is stdlib.h, which includes /usr/include/stdlib.h with this weird #include_next mechanism.

stdlib.h:

#if !defined __cplusplus || defined _GLIBCXX_INCLUDE_NEXT_C_HEADERS
# include_next <stdlib.h>
#else
# include <cstdlib>
// ...

cstdlib:

// Need to ensure this finds the C library's <stdlib.h> not a libstdc++
// wrapper that might already be installed later in the include search path.
#define _GLIBCXX_INCLUDE_NEXT_C_HEADERS
#include_next <stdlib.h>
#undef _GLIBCXX_INCLUDE_NEXT_C_HEADERS

That doesn't seem to be a problem.

What does KDevelop say when you include <vector>? Does it show any problems? I can install libc++ on my system, but I'm not sure how I could convince DUChain/libclang to use it.

I guess this comes from the second test row ("ignore-moc-at-end")?

Right.

I'm not sure if <vector> includes system headers, but I replaced it with <cstdlib>, which obviously includes <stdlib.h> from /usr/include. That still works here.

In the test source?

What does KDevelop say when you include <vector>? Does it show any problems? I can install libc++ on my system, but I'm not sure how I could convince DUChain/libclang to use it.

KDevelop doesn't show any problem, because it draws -I/usr/include from CMake (I think). It is only test_problems that fails.

I'm not sure if <vector> includes system headers, but I replaced it with <cstdlib>, which obviously includes <stdlib.h> from /usr/include. That still works here.

In the test source?

Exactly. I wanted to find out if /usr/include is searched on my system.

What does KDevelop say when you include <vector>? Does it show any problems? I can install libc++ on my system, but I'm not sure how I could convince DUChain/libclang to use it.

KDevelop doesn't show any problem, because it draws -I/usr/include from CMake (I think). It is only test_problems that fails.

But then the libc++ include directory (/usr/include/c++/v1/) should also come from CMake, and we'd see earlier errors, or am I mistaken? Maybe you can find out which include directories are used?

What about standalone clang++ without an explicit -I/usr/include? Compiling with -v lists the implicit include directories.

But then the libc++ include directory (/usr/include/c++/v1/) should also come from CMake, and we'd see earlier errors, or am I mistaken?

These directories are built-in to libclang, so they don't need to come from CMake. And in CMake case we don't see those errors, because -I/usr/include is present.

Maybe you can find out which include directories are used?

Sure, here is the line that test_problems uses:

clang -ferror-limit=100 -fspell-checking  -Wunused-parameter -Wunreachable-code -Wall -std=c++11 -nostdinc -nostdinc++ -xc++ -isystem/usr/include/c++/v1 -isystem/usr/lib/clang/6.0.0/include -isystem /usr/local/llvm60/lib/clang/6.0.0/include -imacros /tmp/test_problems.zWlSBE /tmp/testfile_HVezBz.cpp

No /usr/include, as you see.

What about standalone clang++ without an explicit -I/usr/include? Compiling with -v lists the implicit include directories.

As I wrote in the summary, clang has its own logic for setting default include paths. Clang's logic works fine and adds -I/usr/include, while libclang don't.

That's the part I don't understand. Why is the C++ standard library include directory /usr/include/c++/v1/ builtin, but the C standard library directory /usr/include isn't? Here's what I get:

clang -ferror-limit=100 -fspell-checking  -Wunused-parameter -Wunreachable-code -Wall -std=c++11 -nostdinc -nostdinc++ -xc++ -isystem/usr/include/c++/8 -isystem/usr/include/c++/8/x86_64-suse-linux -isystem/usr/include/c++/8/backward -isystem/usr/local/include -isystem/usr/include -isystem /usr/lib64/clang/6.0.0/include -imacros /tmp/test_problems.HeYwcT /tmp/testfile_LwSiAq.cpp
plugins/clang/duchain/parsesession.cpp
269 ↗(On Diff #36345)

This is where /usr/include comes from on Linux. Can you find out why includes.system is (apparently) incomplete on FreeBSD?

mwolff added a comment.Jul 2 2018, 9:07 PM

Could it be that the issue is that on your system, libc++ and the clang compiler builtin-headers are installed in the same prefix? On Linux at least, that is usually not the case afaik. Thus we didn't run into this issue yet.

I mean, compare: https://packages.ubuntu.com/bionic/amd64/libc++-dev/filelist vs. https://packages.ubuntu.com/bionic/amd64/libclang-common-6.0-dev/filelist

The clang builtins are in /usr/lib/clang/6.0.0/include
The libc++ headers are in /usr/include/c++/v1/

Your last comment seems to indicate that the builtins are also in the latter path on FreeBSD?

clang -ferror-limit=100 -fspell-checking  -Wunused-parameter -Wunreachable-code -Wall -std=c++11 -nostdinc -nostdinc++ -xc++ -isystem/usr/include/c++/v1 -isystem/usr/lib/clang/6.0.0/include -isystem /usr/local/llvm60/lib/clang/6.0.0/include -imacros /tmp/test_problems.zWlSBE /tmp/testfile_HVezBz.cpp

Actually we seem to have 2 builtin directories here: /usr/lib/clang/6.0.0/include and /usr/local/llvm60/lib/clang/6.0.0/include. It looks like the latter comes from a local installation, probably the one used to compile KDevelop. (It's the last argument.) But I'm not sure how this is related to the missing /usr/include.

arrowd added a comment.Jul 3 2018, 8:36 AM

Could it be that the issue is that on your system, libc++ and the clang compiler builtin-headers are installed in the same prefix? On Linux at least, that is usually not the case afaik. Thus we didn't run into this issue yet.

I mean, compare: https://packages.ubuntu.com/bionic/amd64/libc++-dev/filelist vs. https://packages.ubuntu.com/bionic/amd64/libclang-common-6.0-dev/filelist

The clang builtins are in /usr/lib/clang/6.0.0/include
The libc++ headers are in /usr/include/c++/v1/

Your last comment seems to indicate that the builtins are also in the latter path on FreeBSD?

Nope, it is the same as for Ubuntu. libc++ headers are in /usr/include/c++/v1/ and come with FreeBSD itself, and Clang is installed into /usr/local/llvm60 prefix. So builtins are in /usr/local/llvm60/lib/clang/6.0.0/include and there are no libc++ headers in there.

clang -ferror-limit=100 -fspell-checking  -Wunused-parameter -Wunreachable-code -Wall -std=c++11 -nostdinc -nostdinc++ -xc++ -isystem/usr/include/c++/v1 -isystem/usr/lib/clang/6.0.0/include -isystem /usr/local/llvm60/lib/clang/6.0.0/include -imacros /tmp/test_problems.zWlSBE /tmp/testfile_HVezBz.cpp

Actually we seem to have 2 builtin directories here: /usr/lib/clang/6.0.0/include and /usr/local/llvm60/lib/clang/6.0.0/include. It looks like the latter comes from a local installation, probably the one used to compile KDevelop. (It's the last argument.) But I'm not sure how this is related to the missing /usr/include.

Hum. I just noticed that /usr/lib/clang/6.0.0/include dir. FreeBSD ships with base compiler, which is also Clang and that dir is builtin dir for the base clang. While it shouldn't be there, I doubt it is the cause of the problem.

Can you find out why includes.system is (apparently) incomplete on FreeBSD?

Still working on this.

arrowd updated this revision to Diff 37668.Jul 12 2018, 7:43 PM

Proper fix for the testcase.

custom-definesandincludes plugin used varargs.h file to check if the include path is built-in one. On FreeBSD even the Clang's built-in path doesn't have this header, so use another one.

I wonder why it is varargs.h. There is another bug which broke whole C++ support on FreeBSD due to check for the same file: https://bugs.kde.org/show_bug.cgi?id=393779

On my system,

> find /usr/lib64/ -name vadefs.h
/usr/lib64/clang/6.0.0/include/vadefs.h
> find /usr/lib64/ -name varargs.h
/usr/lib64/clang/6.0.0/include/varargs.h
/usr/lib64/gcc/x86_64-suse-linux/8/include/varargs.h

Meaning that with vadefs.h we wouldn't be able to exclude the GCC builtin directory. The intersection of both on my system is:

> sort <(ls /usr/lib64/clang/6.0.0/include/) <(ls /usr/lib64/gcc/x86_64-suse-linux/8/include/) | uniq -d                 
adxintrin.h
ammintrin.h
avx2intrin.h
avx512bitalgintrin.h
avx512bwintrin.h
avx512cdintrin.h
avx512dqintrin.h
avx512erintrin.h
avx512fintrin.h
avx512ifmaintrin.h
avx512ifmavlintrin.h
avx512pfintrin.h
avx512vbmi2intrin.h
avx512vbmiintrin.h
avx512vbmivlintrin.h
avx512vlbwintrin.h
avx512vldqintrin.h
avx512vlintrin.h
avx512vnniintrin.h
avx512vpopcntdqintrin.h
avx512vpopcntdqvlintrin.h
avxintrin.h
bmi2intrin.h
bmiintrin.h
cetintrin.h
clflushoptintrin.h
clwbintrin.h
clzerointrin.h
cpuid.h
emmintrin.h
f16cintrin.h
float.h
fma4intrin.h
fmaintrin.h
fxsrintrin.h
gfniintrin.h
ia32intrin.h
immintrin.h
iso646.h
lwpintrin.h
lzcntintrin.h
mm3dnow.h
mmintrin.h
mm_malloc.h
mwaitxintrin.h
nmmintrin.h
omp.h
pkuintrin.h
pmmintrin.h
popcntintrin.h
prfchwintrin.h
rdseedintrin.h
rtmintrin.h
sanitizer
shaintrin.h
smmintrin.h
stdalign.h
stdarg.h
stdatomic.h
stdbool.h
stddef.h
stdint.h
stdnoreturn.h
tbmintrin.h
tmmintrin.h
unwind.h
vaesintrin.h
varargs.h
vpclmulqdqintrin.h
wmmintrin.h
x86intrin.h
xmmintrin.h
xopintrin.h
xsavecintrin.h
xsaveintrin.h
xsaveoptintrin.h
xsavesintrin.h
xtestintrin.h

Maybe you can intersect that with the files on your system, so that we can find a better candidate.

plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
193–196

My understanding is that this is primarily supposed to exclude non-Clang builtin directories, especially the GCC builtin directory which Clang might not understand.

Maybe you can intersect that with the files on your system, so that we can find a better candidate.

Files that are absent on FreeBSD are listed here. So, let's just use first one in lexicographics order: adxintrin.h.

plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
193–196

I don't have GCC built-in directories in output of my clang. However, this fixes the problem for me.

Files that are absent on FreeBSD are listed here. So, let's just use first one in lexicographics order: adxintrin.h.

Do you know why these files are patched out?

The intrinsics could be architecture-specific (I guess we both have an x86), and I'm not sure whether they will be installed on other architectures like PowerPC. What about iso646.h or cpuid.h?

Or maybe you have a better idea how to recognize the builtin directory.

plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
193–196

Not in test_problems. But it could happen in the actual application.

Files that are absent on FreeBSD are listed here. So, let's just use first one in lexicographics order: adxintrin.h.

Do you know why these files are patched out?

AFAIK, FreeBSD base system also contains these headers, and these are conflicting. I suspect it is the same problem that Gentoo/FreeBSD faced there: https://bugs.llvm.org/show_bug.cgi?id=12957

The intrinsics could be architecture-specific (I guess we both have an x86), and I'm not sure whether they will be installed on other architectures like PowerPC. What about iso646.h or cpuid.h?

FreeBSD base doesn't contain iso646.h, so cpuid.h is what's left for us.

Or maybe you have a better idea how to recognize the builtin directory.

This is what https://bugs.kde.org/show_bug.cgi?id=393779 is about, basically. Besides looking at clang/<numeric_version>/ in the path I have no idea.

arrowd updated this revision to Diff 37795.Jul 15 2018, 10:36 AM

Use cpuid.h header instead of vadefs.h.

aaronpuchert accepted this revision.Jul 15 2018, 1:14 PM

Maybe @mwolff wants to have a look again, but I think it's fine.

this can now be abandoned, right? the other patch seems to supersede this?

Then close it manually. You wrote "Diffirential" instead of "Differential" in the commit message. It's also missing the "Reviewed By" field.

If you have arcanist installed, you can use arc patch Dxxxx to get the commit message right.

arrowd abandoned this revision.Jul 26 2018, 4:52 AM

Without @mwolff approval I can't close this, only abandon.