"Global is marked as dllimport, but not external"

[tritao]

Bugzilla Link	13450
Resolution	FIXED
Resolved on	Jun 03, 2014 19:32
Version	trunk
OS	Windows NT
Blocks	llvm/llvm-bugzilla-archive#13707
CC	@AaronBallman,@asl,@compnerd,@DougGregor,@rnk

Extended Description

compiling the following code:

extern __declspec(dllimport) void(*test)();
void(*test)();

int main()
{
}

produces:

Global is marked as dllimport, but not external
void ()** @​test
Broken module found, compilation aborted!
clang: error: clang frontend command failed with exit code 3 (use -v to see invocation)

[asl]

It seems that something is wrong with merging of attributes.

[llvmbot]

I will try to take a look.

[tritao]

This is also triggered on some other simple cases like a dllimport global variable.

char __declspec(dllimport) c;

[tritao]

I think the issue is that VarDecl::isThisDeclarationADefinition does not take into account the existence of a dllimport attribute.

According to MSDN, a dllimport attribute implies a declaration.

[llvmbot]

So this is not a problem with attribute merging. From Comment 4 it looks like that clang should be parsing this as

extern __declspec(dllimport) void(*test)();
extern void(*test)();

int main()
{
}

João, do you have a the link for where on MSDN this is documented? It is a really strange extension.

[AaronBallman]

http://msdn.microsoft.com/en-us/library/y4h7bcy6.aspx

Specifically:

"The use of dllexport implies a definition, while dllimport implies a declaration. You must use the extern keyword with dllexport to force a declaration; otherwise, a definition is implied."

[tritao]

Yep, that was the quote I was referring to.

I've been refactoring some of the DLL attributes semantic handling, added some diagnostics for global variables and invalid storage classes / linkages types, and am now adding some support for class exports.

As a side effect I also seem to have fixed this issue.

Found a few edge cases that we did not handle yet like:

void func()
{
// Okay; this is a declaration.
__declspec( dllimport ) int m;
}

In that case the compiler should assume an extern linkage.

I'll post a patch with these improvements once I clean it up a bit and add a few more test cases.

[tritao]

Refactor of DLL attributes handling

[tritao]

Attached the work-in-progress patch.

Still need to organize the test cases I've been using and integrate them in the Clang test suite.

I think the things that still need some work to be fully compliant with MSVC are exported inline functions, and additional semantic processing for selective member imports/exports.

-- notes:

I opted to export all static and non-static members in exported classes, but I'm not sure if that is the right approach, according to MSDN:

"When you declare a class dllexport, all its member functions and static data members are exported."

"As a rule, everything that is accessible to the DLL's client (according to C++ access rules) should be part of the exportable interface. This includes private data members referenced in inline functions."

Not sure if class friends can also affect what members get exported. Thoughts?

[llvmbot]

Getting dllimport/dllexport to work with inline is more complicated. See http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-March/060646.html for a brief discussion.

I haven't seen MSVC generate anything for export/imported member variables. But it does ignore accessibility, which is sensible because of inline member functions and friends.

[llvmbot]

Repro for this bug

[llvmbot]

The issue with this bug is that redeclarations with/without dllimport are not properly handled (the attribute should either be dropped or rejected). I have a patches forthcoming to deal with this.

The issue in comment 11 reduces to the following minimized test case:

template
struct X {
__declspec(dllimport) static int id;
};

template
int X::id;

template int X::id;

int foo() { return X::id; }

Usually definitions of dllimport static data are forbidden. And in fact without the explicit instantiation, MSVC expectedly emits an error:

t.cpp(12) : warning C4273: 'id' : inconsistent dll linkage
t.cpp(3) : see previous definition of 'public: static int X::id'
t.cpp(3) : while compiling class template static data member 'int X::id'
t.cpp(11) : see reference to class template instantiation 'X' being compiled
t.cpp(12) : error C2491: 'X::id' : definition of dllimport static data member not allowed

With the explicit instantiation MSVC still only emits a symbol when X::id is actually used, but does not emit an error.

The correct action would be to diagnose and reject the X::id definition. But this makes unusable. So I think a better solution would be to give such imported static data members available_externally linkage, similar to imported inline functions. This would avoid special casing explicit instantiations.

[llvmbot]

With all the work that has been committed recently for dll attributes all snippets presented here work as expected. Please file separate bugs for any remaining edge cases.

[tritao]

mentioned in issue llvm/llvm-bugzilla-archive#13707