Failed to compile with clang 5

[ghost]

I am trying to build coreclr on ubuntu with llvm 5.0 and getting an error given below

coreclr/src/inc/cor.h:2210:1: error: __declspec attribute 'selectany' is not supported [-Werror,-Wignored-attributes]

is the latest version of llvm/clang officially supported?

[wateret]

According to build.sh, 4.0 looks the latest version officially supported.
https://github.com/dotnet/coreclr/blob/463c5b0e005e1fe779a1de855bcb350c0a7709ab/build.sh#L733-L760

FYI the default version is 3.9

[ghost]

Hm, I actually passed that step as it seems like these conditions only apply if we pass clangx.y as script argument? This system I am using has all the latest build tools installed.

final condition in build.sh that checks minimum version looks like this:

if ! [[ "$__ClangMajorVersion" -gt "3" || ( $__ClangMajorVersion == 3 && $__ClangMinorVersion == 9 ) ]];

Compilation begins (after checking all the required dependencies) then fails with the error mentioned above.

Is 5.0 too strict / missing features to compile coreclr? llvm folks are even going to release 5.0.1 in Q4 this year. Not sure why they had a leap from v4.0.1 to directly v5.0.0 all within one year. So I can understand it's hard to chase these versions (and perhaps no point to rush).

I will downgrade to 4.0. Thanks.

[wateret]

@kasper3 Probably if you don't specify clangx.y option, it will try find the default version(clang-3.9 and clang3.9) then just clang if not found. I guess you have clang(of verson 5.0) in your path. This is how you passed that step.

When we introduce new compliers, there is chance that the source cannot be compiled since compilers contains non-stanard stuff like __declspec which is AFAIK microsoft specific. So we need to resolve the issue in some way. It might be worth you try that.

% That condition you mentioned is only for arm32 build, so if you build it for x86 or something else the lower versions will work.

[janvorli]

@kasper3 the 5.0 is not supported yet, we haven't tried to make it work. From time to time, I go and make it work on newer clang versions. There are almost always some details that need to be fixed due to the compiler being more strict etc.
@wateret the __declspec selectany should work with clang 5.0 and clang 6.0, it is described here: https://clang.llvm.org/docs/AttributeReference.html#selectany-gnu-selectany.
@kasper3 I actually wonder if the build was really using clang++ 5.0. What happened to me few times was that clang installation created e.g. clang-3.9, but only clang++ (instead of matching clang++-3.9). For some reason, cmake then picked GCC as the c++ compiler instead of clang. And that one doesn't support the Microsoft extensions. You can see that during the initialization phase of the build, cmake dumps that to console.

[janvorli]

I have tried it locally and I can repro the problem. It turns out it is a bug in clang 5.0:
https://reviews.llvm.org/D33852

[janvorli]

The __declspec(selectany) is not the only thing preventing compiling coreclr using clang5.0. There is a couple of other issues. I've tried to build it with all __declspec(selectany) removed and there is one more problem in src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp:302. The function in there calls the constructor explicitly, but clang 5.0 doesn't like the syntax. And in fact, the call doesn't seem to make sense since all it does when built with clang3.9 or VS is that it creates a local temp of the type and destroys it right away. @BruceForstall, what is the purpose of this constructor invocation?
Then the clang5.0 should be added to build.sh options, src/ToolBox/SOS/lldbplugin/CMakeLists.txt should be updated to support lldb5.0 and finally there is an unused parameter in pal/inc/pal.h:5921 reported by clang5.0.

[am11]

Thanks for this info Jan, I am also trying to make sure when next release of Alpine (3.7) is out with LLVM 5, we don’t miss any patch for coreclr and corert.

The llvm patch you mentioned above is being applied to the ongoing llvm5 port in aports repo. Would you remove __declspec(selectany) from coreclr repo or was it an offline experiment? If it is the former, we can stop adding more patches to llvm50 in aports. 8-)

[janvorli]

@am11 it was just an experiment to see what else is possibly broken. I've also tried to replace it with the [[selectany]] attribute that I think would work even with the unpatched llvm 5.0, but that attribute needs to be used in a little different context (in front of a variable name instead of the whole declaration) and so I cannot just change the existing #define SELECTANY to use it.
The interesting thing though is that even with that define to changed from extern __declspec(selectany) to just extern and the other stuff fixed, the coreclr compiled and linked successfully. So I wonder if we need to define it at all (maybe we never end up with multiple instances of those variables). So if you'd prefer not adding the patch to llvm50, I can test it using coreclr / corefx tests to see if everything is ok and if it is then we could get rid of it for Unix, at least.

[BruceForstall]

@janvorli

void MethodContext::MethodInitHelper(unsigned char* buff2, unsigned int totalLen)
{
    MethodContext::MethodContext();

It seems like that's not necessary, or correct. Perhaps in the past it was, before all the PAL EH macros went in, for example.

@sandreenko Any ideas? Want to fix it?

[sandreenko]

@sandreenko Any ideas? Want to fix it?

Yes, I will clean the part with explicit constructors in the spmi code.

[sam0302]

I am also hit with exactly the same issue when trying to build coreclr in yocto for Raspberry pi. I raised an issue : https://github.com/dotnet/coreclr/issues/15197

Would like to know when can we expect a fix/patch for this?

[janvorli]

@sandreenko any progress on this?