Re: RFC: Make dllimport/dllexport imply default visibility

[Mark Mitchell <mark at codesourcery dot com>]

Chris Lattner wrote:
>>> That is a limited view of things based on the current implementation of
>>> GCC.  When future developments (e.g. LTO) occur, this will change: types
>>> certainly do live in object files.
>>
>> I don't see how LTO changes this.  Yes, type definitions will appear in
>> one or more object files.  But, the intended semantics of LTO are just
>> to do what the linker would do -- plus some consistency checking.
> 
> The consistency checking is the hard part.  :)  I'm not claiming it's
> impossible, it just adds another tricky case to get right.  How does
> this impact TBAA?  Are they the same types or not?

At present, GCC LTO is designed to do a step that the linker would do --
but with optimization.  The linker never sees two (members of) types on
both sides of a visibility barrier; it's either linking things with one
shared object, or within a main program, but it never combines two
shared objects.

If we make GCC LTO smarter than that (and we have rather a ways to
go...) so that (for example) it can optimize a main program by inlining
a function from a shared object, then, yes, this is an issue.  To tell
whether the class named "C" in one translation unit is the same as the
one named "C" in another, imagine that you had said "typeid(C)" in both
translation units, and then compared the results with
"std::type_info::operator==".  (So, for the typical plugin case where
each plugin defines a "MyPlugin" class, with hidden visibility, the
answer is that the types are not the same.)

After determining same-ness, you have to do consistency check that you
would do even within a shared object: check that the "same" types have
different sizes, etc.  The corner cases you get (like, the types have
the same size, but there are now two implementations of "C::f", because
you had a hidden "C::f" in each shared object, but only a single virtual
table for "C") are the same that you get within a single shared object
with different definitions of inline functions.  You get to decide how
strict you want to be that at LTO-time; you can either just pick one
implementation at run-time, or declare the combination un-LTO-able.

> Okay, it sounds like I'm misunderstanding the meaning of the design of
> hidden visibility here.  I've heard it most often described as a tool to
> limit the visibility of vtables and rtti objects to allow plugins from
> different vendors to work right.

That's a consequence, but not the core concept.  After all, ELF
visibility makes just as much sense for C programs, so that two shared
objects can each have their own "myFunction" function.  The impacts on
C++ are a natural consequence, including the ability to do things that
are forbidden in C++.

> While this is obviously not the case you are concerned about, I find it
> quite surprising that classes with vtables  (or non-virtual methods)
> should have completely different rules applied to them than classes with
> vtables and virtual methods.

No, there's no difference.  In both cases:

>> "The visibility attribute to a class specifies the default visibility
>> for all of its members, including compiler-generated functions and
>> variables.  You can override that default by explicitly specifying a
>> different visibility for the members."

Unless I am mistaken, those are in fact the semantics of the compiler at
present, and modulo outright bugs.

> This description sounds fine, and seems appropriate for the manual. 
> However, it seems that you'd want some caveats in this.  For example, is
> it valid for the class to be hidden but have mixed dllimport/hidden
> virtual methods?  What if one of the dllimport virtual method uses the
> typeid of "this"?  Does it get the same as a hidden implementation that
> calls typeid(*this)?  There are a number of tricky cases that need to be
> described (or mentioned) if you want to permit this sort of thing.

We already permit them.  We just mishandle some of them, like the
original one I posted.

Of course, you're right: documenting the interactions is desirable.  For
example, a "dllimport, hidden" function *should* be an error; that makes
no sense.

(That's exactly the bug I posted; the user's saying dllimport, but we're
emitting the function as hidden.  By the way, another way to get the bug
is to compile with -fdefault-visibility=hidden and then do:

  struct S {
    __declspec(dllimport) void f();
  };

It would be weird to force people to put the declspec on the class,
rather than on the only member, to make this code valid.  (And, again
there's a large body of existing code that doesn't.)  But, it would also
be weird to say -fdefault-visibility=hidden means something other than
putting a hidden visibility attribute on everything without an explicit
visibility, including classes.)

>> We have accepted this code:
>>
>>>> struct __attribute__((visibility("hidden"))) S {
>>>>   __attribute__((visibility("default"))) void f();
>>>>   void g();
>>>> };
>>
>> for quite some time.  It would be surprising to make that an error now.
> 
> I also consider that quite surprising, but not a fatal error.  As long
> as "f" is only used/defined within the same shared object as the S type
> is defined, it would be consistent (with my notions).  However,
> using/defining the 'f' symbol without having access to its type seems
> very strange, because you couldn't create the object to pass in as this
> (presumably the ctor etc are also hidden) without violating ODR/TBAA rules.

Well, of course, you could also have a default-visibility factory
function to hand you instances of that class, so that you could call
"S::f" on them.  And that factory could be a non-member of the class.

But, the other impact of default visibility is not just that you can
*use* "S::f" outside of the shared library which defines "S::g".  In
particular, you can *implement* it outside of that shared library.  This
goes back to dllimport sketch I gave earlier.  In more detail, suppose
something like the following:

1. I have a DLL that plays MPEG movies, but ...

2. ... needs you to implement a low-level "drawScreen" primitive.

One way I can do this is to have you pass in a ScreenDrawer* at some
point, and I can then call "theDrawer->drawScreen()" from appropriate
places.  A second way is that I can have you implement
"ScreenDrawer::drawScreen".  Of course, there are tradeoffs here
depending on whether "drawScreen" is a function that's universal for all
software on a given system (and therefore might make sense to provide in
a system-specific DLL, which all programs using my DLL would also use)
or different for every program (and therefore might make sense to pass
in).

But, sometimes, allowing some system DLL to define Drawer::drawScreen is
what people want.  The factorization of what-DLL-provides-what need not
always be at the whole-class level.

> a.cpp:
> 
> struct __attribute__((visibility("hidden"))) S {
>   __attribute__((visibility("default"))) void f();
>   void g();
> };
> 
> void S::f() { g(); }
> 
> 
> b.cpp:
> 
> struct __attribute__((visibility("hidden"))) S {
>   __attribute__((visibility("default"))) void f();
>   void g();
> };
> 
> void S::g() { f(); }
> 
> 
> Because these two types are defined as hidden in both .so's, I consider
> them to be different types.  However, S::f now passes in a "a.cpp::S*"
> pointer to S::g, which expects a "b.cpp::S*".

Yes, using my semantics above, these are different types.  And, yes, the
call that you suggest is a type error, for the reason that you say.
It's QoI for LTO to diagnose that, but, hopefully, it will.

>> The question which prompted this debate was where "f" has been marked
>> "dllimport" rather than with an explicit visibility.  But, "dllimport"
>> certainly implies default visibility.  It would be inconsistent to
>> accept the case directly above, but not the "dllimport" case.
> 
> dllimport implies default visibility, but it also requires (afaik) that
> the implementation be outside the current shlib. 

On the systems I've used, it just affords that option.  If the function
ends up being in the same shared library, you may have slower access to
it, because you end up going through an indirection you don't need, but
it works.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713