llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.98k stars 11.95k forks source link

Clang-CL can not compile `TraceLoggingProvider.h` #31369

Open steveire opened 7 years ago

steveire commented 7 years ago
Bugzilla Link 32021
Version trunk
OS Windows NT
Depends On llvm/llvm-project#30280
CC @majnemer,@DougGregor,@zmodem,@mstorsjo,@nico,@rnk

Extended Description

The example at

https://msdn.microsoft.com/en-us/library/windows/desktop/dn904627(v=vs.85).aspx

can not be compiled.

The first error is

    ..\SimpleTraceLoggingExample.cpp(6,1):  error: use of undeclared identifier '__annotation'
    TRACELOGGING_DEFINE_PROVIDER(
    ^
    C:\Program Files (x86)\Windows Kits\10\include\10.0.10586.0\shared\TraceLoggingProvider.h(182,5):  note: expanded from macro 'TRACELOGGING_DEFINE_PROVIDER'
        _TlgDefineProvider_annotation(handleVariable, _Tlg##handleVariable##Prov, 1, providerName); \
        ^
    C:\Program Files (x86)\Windows Kits\10\include\10.0.10586.0\shared\TraceLoggingProvider.h(3448,9):  note: expanded from macro '_TlgDefineProvider_annotation'
            __annotation( \
            ^

However, further errors prevent compilation too

    ..\SimpleTraceLoggingExample.cpp(6,1):  error: use of undeclared identifier '_Tlgg_hMyComponentProviderProv'; did you mean 'g_hMyComponentProvider'?
    C:\Program Files (x86)\Windows Kits\10\include\10.0.10586.0\shared\TraceLoggingProvider.h(183,29):  note: expanded from macro 'TRACELOGGING_DEFINE_PROVIDER'
        _TlgProviderStorage_imp(_Tlg##handleVariable##Prov, providerName, providerId, 1, __VA_ARGS__); \
                                ^

I have not been able to deconstruct the macros enough to find out what the problem is.

mstorsjo commented 5 years ago

Created attachment 21564 [details] 64-bit build log

Actually, I can't build this in 64-bit using my regular win sdk either. Attaching build log from using the 10.0.16299.0 win sdk.

If I use a more modern sdk (10.0.17763.0) I only get warnings about the execution_character_set pragma, no errors.

Ok, thanks - that explains the situation more. So apparently this header has been changing quite a bit not all that long ago.

I fixed the compiler-rt build failure in SVN r355397 at least.

zmodem commented 5 years ago

64-bit build log Actually, I can't build this in 64-bit using my regular win sdk either. Attaching build log from using the 10.0.16299.0 win sdk.

If I use a more modern sdk (10.0.17763.0) I only get warnings about the execution_character_set pragma, no errors.

mstorsjo commented 5 years ago

Created attachment 21563 [details] build log

Thanks for checking and updating. I think that's good enough.

I ran into this today when trying to build a new Clang snapshot for http://llvm.org/builds/

Martin: Maybe you were doing a 64-bit build, and the issue only occurs with the 32-bit version of the headers or due to some other 32-bit thing?

Yes, this seems to be the case, I only tested compiler-rt with clang-cl in 64 bit mode.

However, this is now actually a different issue than this bug originally. Originally, an example from MSDN failed to compile, and I rechecked that example and it works fine (even in 32 bit mode). In that case, the error was about __annotation attributes being undeclared.

The same issue as in compiler-rt can be reproduced with the example from MSDN, if compiling with -Z7.

mstorsjo commented 5 years ago

Created attachment 21563 [details] build log

Thanks for checking and updating. I think that's good enough.

I ran into this today when trying to build a new Clang snapshot for http://llvm.org/builds/

Martin: Maybe you were doing a 64-bit build, and the issue only occurs with the 32-bit version of the headers or due to some other 32-bit thing?

Yes, this seems to be the case, I only tested compiler-rt with clang-cl in 64 bit mode.

However, this is now actually a different issue than this bug originally. Originally, an example from MSDN failed to compile, and I rechecked that example and it works fine (even in 32 bit mode). In that case, the error was about __annotation attributes being undeclared.

This is a real problem because compiler-rt started using this header in llvm.org/r355045, and since we bootstrap the snapshot builds, this means I can't build any new snapshots.

I get different errors depending on what Windows SDK I use. Attaching the output from using 10.0.17763.0 (the errors are at the bottom and look pretty weird).

It should be easy to avoid this by extending the define from https://reviews.llvm.org/rCRT355236 from this state:

#if defined(SANITIZER_WINDOWS) && defined(_MSC_VER)
#define SANITIZER_WIN_TRACE 1

To:

#if defined(SANITIZER_WINDOWS) && defined(_MSC_VER) && !defined(__clang__)
#define SANITIZER_WIN_TRACE 1
zmodem commented 5 years ago

build log

Thanks for checking and updating. I think that's good enough.

I ran into this today when trying to build a new Clang snapshot for http://llvm.org/builds/

Martin: Maybe you were doing a 64-bit build, and the issue only occurs with the 32-bit version of the headers or due to some other 32-bit thing?

This is a real problem because compiler-rt started using this header in llvm.org/r355045, and since we bootstrap the snapshot builds, this means I can't build any new snapshots.

I get different errors depending on what Windows SDK I use. Attaching the output from using 10.0.17763.0 (the errors are at the bottom and look pretty weird).

steveire commented 5 years ago

Thanks for checking and updating. I think that's good enough.

mstorsjo commented 5 years ago

FWIW, I recently built code including TraceLoggingProvider.h (in sanitizers in compiler-rt, added last week) with a very recent clang-cl with headers from WinSDK 10.0.17763.0. I also tested the linked example from MSDN, and that also builds fine.

nico commented 7 years ago

For the reduced sample, if it's spelled like:

#define FLATTEN(...) __VA_ARGS__
#define ID(...) BRACE(FLATTEN __VA_ARGS__)
#define BRACE(...) { __VA_ARGS__ }
int a[] = ID((0x3970f9cf, 0x2c0c));

I.e.

#define ID(...) BRACE(FLATTEN __VA_ARGS__)

instead of

#define ID(...) BRACE FLATTEN(__VA_ARGS__)

then it works with both compilers.

nico commented 7 years ago

Reduced: This snippet

#define FLATTEN(a) a
#define ID(a) BRACE FLATTEN(a)
#define BRACE(a, b) { a, b }
int a[] = ID((0x3970f9cf, 0x2c0c));

is converted to

int a[] = BRACE (0x3970f9cf, 0x2c0c);

by clang-cl but to

int a[] = { 0x3970f9cf, 0x2c0c };

with cl.exe.

(The same is true for the slightly-less-reduced

#define FLATTEN(...) __VA_ARGS__
#define ID(...) BRACE FLATTEN(__VA_ARGS__)
#define BRACE(a, b) { a, b }
int a[] = ID((0x3970f9cf, 0x2c0c));

)

nico commented 7 years ago

If I manually replace

TRACELOGGING_DECLARE_PROVIDER(g_hMyComponentProvider);
TRACELOGGING_DEFINE_PROVIDER(g_hMyComponentProvider,
                             "SimpleTraceLoggingProvider",
                             (0x3970f9cf, 0x2c0c, 0x4f11, 0xb1, 0xcc, 0xe3,
                              0xa1, 0xe9, 0x95, 0x88, 0x33));

with what clang-cl /E outputs for it, and then do that again, the sample compiles fine. So it looks like clang-cl just stops earlier than cl when expanding macros (probably due to varargs) but otherwise behaves identical.

nico commented 7 years ago

For the preprocessor issue:

TRACELOGGING_DECLARE_PROVIDER(g_hMyComponentProvider);
TRACELOGGING_DEFINE_PROVIDER(g_hMyComponentProvider,
                             "SimpleTraceLoggingProvider",
                             (0x3970f9cf, 0x2c0c, 0x4f11, 0xb1, 0xcc, 0xe3,
                              0xa1, 0xe9, 0x95, 0x88, 0x33));

gets expanded to

extern const TraceLoggingHProvider g_hMyComponentProvider;

static void __cdecl _TlgDefineProvider_annotation__Tlgg_hMyComponentProviderProv(
    void) {
  __annotation(L"_TlgDefineProvider:|"
               L"11"
               L"|"
               L"g_hMyComponentProvider"
               L"|"
               L"SimpleTraceLoggingProvider");
};
_TlgProviderStorage_imp0(_Tlgg_hMyComponentProviderProv,
                         "SimpleTraceLoggingProvider",
                         (0x3970f9cf, 0x2c0c, 0x4f11, 0xb1, 0xcc, 0xe3, 0xa1,
                          0xe9, 0x95, 0x88, 0x33),
                         1);
extern const TraceLoggingHProvider g_hMyComponentProvider =
    &_Tlgg_hMyComponentProviderProv;

with clang-cl but to

extern const TraceLoggingHProvider g_hMyComponentProvider;

static void __cdecl _TlgDefineProvider_annotation__Tlgg_hMyComponentProviderProv(
    void) {
  __annotation(L"_TlgDefineProvider:|"
               L"7"
               L"|"
               L"g_hMyComponentProvider"
               L"|"
               L"SimpleTraceLoggingProvider");
};
__pragma(execution_character_set(push, "UTF-8"))
    __pragma(pack(push, 1)) static struct {
  struct _TlgProviderMetadata_t _TlgProv;
  char _TlgName[sizeof("SimpleTraceLoggingProvider")];
} const __declspec(allocate(".rdata$zETW2")) __declspec(align(1))
    _Tlgg_hMyComponentProviderProv_Meta = {
        {_TlgBlobProvider3,
         {0x3970f9cf, 0x2c0c, 0x4f11, 0xb1, 0xcc, 0xe3, 0xa1, 0xe9, 0x95, 0x88,
          0x33},
         sizeof(_Tlgg_hMyComponentProviderProv_Meta) - 1 - 16},
        ("SimpleTraceLoggingProvider")};
__pragma(pack(pop)) __pragma(execution_character_set(
    pop)) static struct _TlgProvider_t _Tlgg_hMyComponentProviderProv = {
    0, &_Tlgg_hMyComponentProviderProv_Meta._TlgProv.RemainingSize,  0, 0, 0, 0,
    0, &_TlgDefineProvider_annotation__Tlgg_hMyComponentProviderProv};
extern const TraceLoggingHProvider g_hMyComponentProvider =
    &_Tlgg_hMyComponentProviderProv;

with cl.exe. It looks like clang-cl stops macro expansion too early for what TraceLoggingProvider.h intends to do.

rnk commented 7 years ago

The pre-processor issues that Stephen mentioned aren't trivial. Let's undupe and mark it blocked on __annotation.

991901f3-cc14-4404-b340-165691b62a58 commented 7 years ago

This bug has been marked as a duplicate of bug llvm/llvm-project#30280

steveire commented 7 years ago

That is one of the issues, yes. The other one I posted seems to be some preprocessor difference in behavior. I don't know what it's trying to do.

991901f3-cc14-4404-b340-165691b62a58 commented 7 years ago

Looks like we are missing support for __annotation. If we'd want to correctly implement it, we'd need some CodeView support for it.