[PATCH] Add full semantic support for dllimport/export attributes

[Nico Rieck]

On 07.07.2013 23:26, Reid Kleckner wrote:
>> 2. dllexport requires a definition. The extern keyword can be used to
>>     force a declaration.
>>
>
> This only applies to class or template definitions, right?  You can
> certainly put dllexport on function declarations in TUs that don't define
> the function.

Yes you can, but they won't be exported then. That should probably be 
clarified, as I think it would be nice to have this summary in the docs 
once this is integrated.

>> 6. Classes
>>    [...]
>>    c) Members can be imported or exported selectively.
>>
>
> This selectivity presumably allows you to split a class definition across
> multiple dlls.  What linkage does the class data (vtables, RTTI, etc) get
> then?  If I define two constructors for a class and split them into two
> dlls, do the typeid()s of two objects constructed from each dll compare
> equal?  I'm not actually sure if the standard says anything about that.
>
> I can see how this might work in an ELF shared object world because the
> loader will take the first symbol definition, but I don't see how this
> could work in a PE DLL world.  It might even be worth a warning.

Interesting. Let's see. If the split class is non-dynamic, typeinfo is 
emitted everywhere, and they should compare equal, as long as the 
typeinfo is not shared across module boundaries. MSVC appears to use 
string comparison to make it work. GCC opts for the address comparison, 
which seems to cause problems for some folks playing with visibility 
(see <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=23628>).

Now if the class is dynamic, you cannot actually use selective export
because the class data is not decorated. Note: this only applies when 
following the Itanium rules. GCC also fails here. MSVC never imports or 
exports the vtable.

This actually reminds me of another issue. Suppose a dynamic class X is 
exported from a shared library A and used in B. If A and B don't share 
the same operator new/delete, deleting an instance of X in B will invoke 
the wrong operator delete (the one in A). This can also happen with ELF 
on Linux if you give A's operators hidden visibility, though I'm not 
sure how "legal" that is.
That's the reason why MSVC emits a modified "local vftable" using the 
correct operator delete.

>>    f) Virtual tables and typeinfo follow the emission rules of the
>>       Itanium ABI, i.e. for an exported/imported dynamic class with key
>>       function, the needed globals are exported/imported. If there is no
>>       key function, they are emitted everywhere and are not exported/
>>       imported.
>>
>
> That's interesting!  So cl.exe *does* have key functions, but only at the
> DLL level, not the object file level.  I think that mostly handles the
> corner case I raised above.

Note that f) doesn't apply to MSVC's ABI. As noted above, vtable 
emission is done differently, and this point has to be revisited. The 
main motivation to start with Itanium was that it seems GCC on Windows 
follows it, Clang/LLVM's support for MSVC's ABI was still in its infancy 
(not sure how far this has come along), and AFAICS it's not currently 
possible to implement some C++11 features because MSVC's ABI does not 
implement them.
This actually begs the question, are the vtable layouts used by COM 
compatible between the two or would that require a modified Itanium ABI?

> Shouldn't you only add UsedAttr if this is an export?

Indeed.

>> +  if (IsRecordExported) {
>> +    // All base classes of an exportable class must be exportable.
>>
>
> What happens in the corner case where the separate members are exported,
> but the base classes are not exported?  Link errors, I guess?

This should probable read "should" instead of "must" as it's just a 
warning. Depending on usage, and whether the base class uses inline 
functions or selective export, you shouldn't get link errors.


> Ditto what Eli said, you can probably fix the clone() body emitted in
> EmitClangAttrImpl().

Yup, I have it locally already. I'll also address the other style issues 
brought up.

Thanks for looking at this.

-Nico