This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: Make dllimport/dllexport imply default visibility


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?


Furthermore, you're taking the view that:

__attribute__((visibility ("hidden"))

on a type means something about visibility of the type in a linguistic
sense, i.e., that it provides some kind of scoping, perhaps like an
anonymous namespace that is different in each shared library.

Yes.

That's a possible meaning, but it's not the meaning that was intended. As Danny has said, it's not the meaning that Windows users want. It's also not the meaning that SymbianOS users want.

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.


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.

We also allow:

struct __attribute__((visibility("hidden"))) S {
  __attribute__((visibility("default"))) void f();
  void g();
};

Because there is no standard to reference, I think it's important to
consider these things in terms of explainability. It is very easy (and
common) to explain visibility and anon namespaces in terms of types
(when applied to a type).

Here would be my explanation:


"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."

That seems acceptably simple to me. As long as we allow visibility
specifications for the members that are different from the class
(independently of whether that is narrower or wider visibility) an
explanation in terms of namespaces will require a caveat. For example:


"Giving a class hidden visibility is similar to putting it in an
anonymous namespace shared not just within a single translation unit,
but across all translation units in a shared object. However, if you
override the visibility of the members of the class, then they may have
more or less visibility than specified by the class."

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.


ELF operates at a level below C++, and can be used to do things that C++
does not allow.

Of course. Again, hidden visibility is also supported on darwin, so it is not an ELF-only feature. The implementation works the same way on both systems though.


For example, the C++ standard (via the ODR) forbids a
single program from having two classes with the same name. But, one of
the goals of ELF hidden visibility is to allow that, so that, for
example, two plugins can have classes with the same name without
conflicting. You can also give two C++ functions the same address via
appropriate ELF magic. These sorts of things must be done with care,
but they are techniques used by many real programs, and in the hands of
experts, useful.

Right. The usual way I have heard (or imagined :) this described is as adding an extra level of (anonymous) namespace around hidden types/ symbols across a shared library. With this description it is an extra-linguistic extension, but doesn't change the core concepts like the ODR.


There are two conflicting goals to balance:

1. Define our extensions as well as possible and make their semantics as
explainable and logical as possible.
2. Compile existing code with maximum compatibility.


To me, the best way to handle this is to reject this by default (based
on #1). To handle #2, add a flag (defaulting to off) to enable this
extended extension. In the diagnostic, tell the user about the option,
and in the manual document the option and the issue.

Good; at this point we've agreed that we should accept the code. Now
we're just arguing about whether we accept it by default. That's a less
important issue, since at least there will be some way to get the
behavior that users want.


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.



To be completely explicit, consider two .cpp files linked into two different .so's:


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*".





In contrast, consider:


c.cpp:

struct __attribute__((visibility("hidden"))) S {
  __attribute__((visibility("default"))) void f();
  void g();
};
void S::f() { g(); }
void S::g() { f(); }


Just because 'f' has default visibility, it doesn't mean that it will be called outside the current shared library. To me, it would be logical if the compiler "optimized" f in this case, giving it hidden visibility, because there is no way to pass a "c.cpp::S" struct into f from outside the shared library.


Note that this is exactly the functionality that Jason added for 4.2, but it apparently only overrides symbols without an explicit visibility set?


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. default visibility implies that it could be accessed outside the shlib, but it may still be implemented/accessed within the shlib (in which case the construct is ok).



In any case, if you'd like to make this enabled by default, I'm obviously not going to stop you :), but these cases all sound very strange. How could something outside the current shlib call S::f if it can't create an S? If there is a good answer for these questions, they should definitely go in the manual, with detailed explanation of which cases are and are not ok.


-Chris


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]