Re: RFC: Make dllimport/dllexport imply default visibility
On 03/07/2007, at 9:18 PM, Mark Mitchell wrote:
Geoffrey Keating wrote:
On 03/07/2007, at 7:37 PM, Mark Mitchell wrote:
Geoffrey Keating wrote:
Yes. __attribute__((visibility)) has consistent GNU semantics, and
other features (eg. -fvisibility-ms-compat, __dllspec) match other
compilers.
The only semantics that make sense on SymbianOS are the ones that
allow
default visibility within a hidden class.
I think we're talking past each other. What semantics exactly are
these? Who created them? Where are they implemented? Are they
documented? Who documented them? Why can't they be changed?
SymbianOS allows you to declare a member dllimport or dllexport even
though the class is declared with hidden visibility. dllimport and
dllexport imply default visibility.
OK. So we are *not* talking about __attribute__((visibility)), we are
talking about dllimport, dllexport, and (importantly) notshared.
The semantics, as I described earlier in this thread, are simple: the
visibility you specify for the class is the default for all members,
including vtables and other compiler-generated stuff, but the user may
explicitly override that by specifying a visibility for the member.
... these are not exactly 'semantics'. They are a description of an
implementation, and rely on other undescribed features of the
implementation, like "this implementation does not strictly check the
ODR". If the implementation changed in future, it would be very
unclear as to how, or whether, to preserve these properties.
The
semantics I describe are a conservative extension to the semantics you
describe; they are operationally identical, except that the semantics
you describe forbid giving a member more visibility than its class.
The
change that I made was to stop G++ from silently ignoring such a
request.
I'm not sure you've really described the semantics. For this
extension to be actually useful, you're going to want an ODR exception
like the one for -fvisibility-ms-compat, otherwise you won't be able
to do things like access fields of the class or other members without
invoking undefined behaviour. Then, what happens if you write this
and it turns out to not quite be the same as what the other compiler
does, but people are already relying on the meaning you documented/
implemented?
So, whether or not there's a
command-line option, it's going to be on by default, and therefore
is
going to be inconsistent with a system on which GCC disallows (or
ignores) that.
Maybe, and maybe not. In particular, an option that changes
defaults is
one thing, but if the user overrides the default with GCC-specific
syntax and the compiler does something different, that's another
thing
altogether.
Here, the compiler was silently ignoring an attribute explicitly given
by the user. I changed the compiler not to ignore the attribute.
That
did not alter the GNU semantics, unless we consider it an intentional
feature that we ignored the attribute. (I can't imagine that we would
consider that a feature; if for some reason we are going to refuse to
let users declare members with more visibility than specified for the
class, we should at least issue an error message.)
Yes, you've convinced me that there should be an error message.
RealView 3.0.x doesn't support the visibility attribute, but it
does
support other GNU attributes.
So it can't conflict.
I don't find that a convincing argument. The two compilers have
different syntaxes for specifying ELF visibility. But, they meant the
same thing in all respects (so far as I know) except for this case.
I expect they're different in a lot of other cases too.
For example, in the GNU semantics it's clear that you can have:
struct S __attribute__((visibility("hidden")));
void f(struct S *p) { };
in two different shared libraries, and they do not conflict, and it so
happens that on ELF systems this is implemented by automatically
marking 'f' as hidden.
It also so happens that if you write
struct S __attribute__((visibility("hidden")));
inline void f() {
struct S * p;
};
that the compiler is permitted to optimise this by making 'f' hidden,
because 'f' can be defined and used only in this shared object. The
compiler does not presently do this, but it could, and in future it
probably will.
This is why I was asking about documentation. From GCC's visibility
documentation, you can deduce what is a valid program, what is an
invalid program, and what the valid programs do.
We
could invent some alternative attribute to mark a class "hidden, but
in
the SymbianOS way that allows you to override the visibility of
members,
not in the GNU way that doesn't", but that seems needlessly complex.
It's really not that complex. In fact, we already have syntax for
this attribute, "__declspec(notshared)". Or, at least, it's only as
complex as implementing the semantics in the first place.
Concretely, are
Re: RFC: Make dllimport/dllexport imply default visibility
Geoffrey Keating wrote:
> On 03/07/2007, at 7:37 PM, Mark Mitchell wrote:
>
>> Geoffrey Keating wrote:
>>
>>> Yes. __attribute__((visibility)) has consistent GNU semantics, and
>>> other features (eg. -fvisibility-ms-compat, __dllspec) match other
>>> compilers.
>>
>> The only semantics that make sense on SymbianOS are the ones that allow
>> default visibility within a hidden class.
>
> I think we're talking past each other. What semantics exactly are
> these? Who created them? Where are they implemented? Are they
> documented? Who documented them? Why can't they be changed?
SymbianOS allows you to declare a member dllimport or dllexport even
though the class is declared with hidden visibility. dllimport and
dllexport imply default visibility.
The semantics, as I described earlier in this thread, are simple: the
visibility you specify for the class is the default for all members,
including vtables and other compiler-generated stuff, but the user may
explicitly override that by specifying a visibility for the member. The
semantics I describe are a conservative extension to the semantics you
describe; they are operationally identical, except that the semantics
you describe forbid giving a member more visibility than its class. The
change that I made was to stop G++ from silently ignoring such a request.
I don't know who created the SymbianOS semantics, but they are
implemented in ARM's RealView compiler, which is the compiler used to
build SymbianOS and most of the applications for that system. Those
semantics go back to pre-ELF SymbianOS but are preserved in the current
ELF-based SymbianOS explicitly in terms of ELF visibility. They're
documented, at least partially, in various places, including the ARM
EABI and the RealView compiler. They can't be changed because there are
millions of lines of code that require them.
>> So, whether or not there's a
>> command-line option, it's going to be on by default, and therefore is
>> going to be inconsistent with a system on which GCC disallows (or
>> ignores) that.
>
> Maybe, and maybe not. In particular, an option that changes defaults is
> one thing, but if the user overrides the default with GCC-specific
> syntax and the compiler does something different, that's another thing
> altogether.
Here, the compiler was silently ignoring an attribute explicitly given
by the user. I changed the compiler not to ignore the attribute. That
did not alter the GNU semantics, unless we consider it an intentional
feature that we ignored the attribute. (I can't imagine that we would
consider that a feature; if for some reason we are going to refuse to
let users declare members with more visibility than specified for the
class, we should at least issue an error message.)
>>> I hope you don't mean that there are other system's compilers using the
>>> '__attribute__((visibility))' syntax in a way that's incompatible with
>>> GCC. If there are, I think the appropriate response is a combination of
>>> fixincludes and a polite e-mail asking them to stop.
>>
>> I don't know if there are, but I certainly wouldn't be surprised. The
>> GNU attribute syntax is implemented in the EDG front end and there are
>> lots of EDG-based compilers.
>
> This doesn't quite count, because EDG implements it to be compatible
> with GCC, and I'm sure if you find it isn't then they'll fix it (or at
> least acknowledge it as a bug).
EDG licensees routinely modify the front end. The attribute handling is
specifically designed to be easy to extend. ARM has already confirmed
(on this list) that the RealView behavior is intentional. It seems
unlikely that it will go away.
>> RealView 3.0.x doesn't support the visibility attribute, but it does
>> support other GNU attributes.
>
> So it can't conflict.
I don't find that a convincing argument. The two compilers have
different syntaxes for specifying ELF visibility. But, they meant the
same thing in all respects (so far as I know) except for this case. We
could invent some alternative attribute to mark a class "hidden, but in
the SymbianOS way that allows you to override the visibility of members,
not in the GNU way that doesn't", but that seems needlessly complex.
Concretely, are you arguing that my patch was a bad change? If so, do
you feel that silently discarding the attribute on members was a good
thing? Do you feel that preventing users from making members more
visible than classes must be an error, rather than just considering it
in questionable taste?
> If you have -fvisibility-ms-compat switched on, then
>
> struct S {
> void f() __dllspec(dllimport);
> };
That, however, is not quite the case in question; rather it's:
struct S __declspec(notshared) {
void f() __declspec(dllimport); // Or dllexport
};
The class itself is explicitly declared with hidden visibility. And, as
far as I know, there's no desire on SymbianOS to make vtables and such
visible, even when class members are not, which seems to be
Re: RFC: Make dllimport/dllexport imply default visibility
On 03/07/2007, at 7:37 PM, Mark Mitchell wrote:
Geoffrey Keating wrote:
Yes. __attribute__((visibility)) has consistent GNU semantics, and
other features (eg. -fvisibility-ms-compat, __dllspec) match other
compilers.
The only semantics that make sense on SymbianOS are the ones that
allow
default visibility within a hidden class.
I think we're talking past each other. What semantics exactly are
these? Who created them? Where are they implemented? Are they
documented? Who documented them? Why can't they be changed?
I don't know what you mean by 'make sense'. Clearly these semantics
do make sense; no-one's brain has exploded yet. You probably mean
'compatible with the system headers' or possibly 'that I like' or
perhaps 'that my users like' or 'that my users understand'.
So, whether or not there's a
command-line option, it's going to be on by default, and therefore is
going to be inconsistent with a system on which GCC disallows (or
ignores) that.
Maybe, and maybe not. In particular, an option that changes defaults
is one thing, but if the user overrides the default with GCC-specific
syntax and the compiler does something different, that's another
thing altogether.
I hope you don't mean that there are other system's compilers
using the
'__attribute__((visibility))' syntax in a way that's incompatible
with
GCC. If there are, I think the appropriate response is a
combination of
fixincludes and a polite e-mail asking them to stop.
I don't know if there are, but I certainly wouldn't be surprised. The
GNU attribute syntax is implemented in the EDG front end and there are
lots of EDG-based compilers.
This doesn't quite count, because EDG implements it to be compatible
with GCC, and I'm sure if you find it isn't then they'll fix it (or
at least acknowledge it as a bug).
RealView 3.0.x doesn't support the visibility attribute, but it does
support other GNU attributes.
So it can't conflict.
__declspec(dllexport) and
__attribute__((visibility("default"))) are synonyms on SymbianOS.
As far as I can tell they're synonyms everywhere, although my
understanding is subject to change through bug reports. The question
is about "__attribute__((visibility("hidden")))".
If you have -fvisibility-ms-compat switched on, then
struct S {
void f() __dllspec(dllimport);
};
is entirely valid and does what you want to do. So if you're just
trying to be compatible with Visual Studio, or trying to be
compatible with people trying to be compatible with Visual Studio,
that's already implemented.
And likewise, for new code, the answer is to not make 'S' hidden in
the first place, which by coincidence is also:
struct S {
void f() __dllspec(dllimport);
};
or so.
smime.p7s
Description: S/MIME cryptographic signature
Re: RFC: Make dllimport/dllexport imply default visibility
Geoffrey Keating wrote:
> Yes. __attribute__((visibility)) has consistent GNU semantics, and
> other features (eg. -fvisibility-ms-compat, __dllspec) match other
> compilers.
The only semantics that make sense on SymbianOS are the ones that allow
default visibility within a hidden class. So, whether or not there's a
command-line option, it's going to be on by default, and therefore is
going to be inconsistent with a system on which GCC disallows (or
ignores) that.
However, since this is a conservative extension to your semantics (all
valid programs remain valid and have the same meaning), I don't
understand why you think this is a problem. The SymbianOS semantics
give the programmer more choices, which, as with most such things, the
programmer can use in good ways or bad.
The good news is that the change that I made caused the compiler to stop
silently ignoring explicit requests from the programmer and without
changing the meaning of any valid programs. So, we can (and do) still
have consistent GNU semantics across platforms with this change.
> I hope you don't mean that there are other system's compilers using the
> '__attribute__((visibility))' syntax in a way that's incompatible with
> GCC. If there are, I think the appropriate response is a combination of
> fixincludes and a polite e-mail asking them to stop.
I don't know if there are, but I certainly wouldn't be surprised. The
GNU attribute syntax is implemented in the EDG front end and there are
lots of EDG-based compilers.
RealView 3.0.x doesn't support the visibility attribute, but it does
support other GNU attributes. __declspec(dllexport) and
__attribute__((visibility("default"))) are synonyms on SymbianOS. It
would be odd for them to be different because dllexport is defined in
terms of visibility. It seems unlikely to me that RealView would add
the visibility attribute but disallow usage that is valid with the
__declspec syntax, but, of course, I can't speak for ARM.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
On 03/07/2007, at 5:13 PM, Mark Mitchell wrote: Geoffrey Keating wrote: GCC's concept of visibility is very different to that of some other compilers. Yes, and that may be a problem. For some features, we want to have GNU semantics that are consistent that across platforms; for others, we want to match other compilers on a particular platform. Yes. __attribute__((visibility)) has consistent GNU semantics, and other features (eg. -fvisibility-ms-compat, __dllspec) match other compilers. To be clear, I don't have any objection to the semantics you stated, from the point of view of first principles of language design. But, they do not match existing practice on various systems -- and I consider that a serious problem. It's not possible for any semantics to match existing practise on every system, since systems differ. As I said, I think that it would be best for GCC to have one standard consistent set of semantics for __attribute__((visibility)), and then if it's desirable to match existing practise on other systems that should be done with other features explicitly labelled as such. I hope you don't mean that there are other system's compilers using the '__attribute__((visibility))' syntax in a way that's incompatible with GCC. If there are, I think the appropriate response is a combination of fixincludes and a polite e-mail asking them to stop. smime.p7s Description: S/MIME cryptographic signature
Re: RFC: Make dllimport/dllexport imply default visibility
Geoffrey Keating wrote:
> GCC's concept of visibility is very different to that of some other
> compilers.
Yes, and that may be a problem. For some features, we want to have GNU
semantics that are consistent that across platforms; for others, we want
to match other compilers on a particular platform.
To be clear, I don't have any objection to the semantics you stated,
from the point of view of first principles of language design. But,
they do not match existing practice on various systems -- and I consider
that a serious problem. In particular, ARM SymbianOS is a system that
has specifically defined DLL attributes in terms of ELF visibility.
There's no choice about the semantics on that system, including the fact
that the first "S" below is a valid class on that system.
Fortunately, for:
> In respect of this specific example, the compiler should not give any
> effect to the attribute on 'f' in:
>
> struct __attribute__((visibility("hidden"))) S {
> void f() __attribute__((visibility("default")));
> }
>
> because another shared library can define:
>
> struct __attribute__((visibility("default"))) S {
> void f();
> }
we can be generous. In particular, we can do what the user asks with
the first "S", i.e., give "f" default visibility. There's no inherent
problem there. We can then detect that there are two "S::f" when both
shared libraries are loaded (if we think that's a useful check), or we
can just call it undefined behavior, as it would be if there were no
visibility markers in this example, but there were conflicting
definitions of "S::f".
We could of course issue a warning, or, I guess, an error. I see this
as a style issue, so a warning seems better to me, but if Darwin wants
to forbid giving member functions more visibility than their containing
classes, that's not for me to say.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
Mark Mitchell <[EMAIL PROTECTED]> writes:
> But, the visibility attribute is only specified in terms of its effects
> on ELF symbols, not as having C++ semantics per se.
[Sorry I'm so late with this reply; I've been busy and am behind on reading
mailing lists.]
The documentation for the visibility attribute was rewritten a year
ago to be in terms of C++ semantics. It now says:
@item hidden
Hidden visibility indicates that the entity declared will have a new
form of linkage, which we'll call ``hidden linkage''. Two
declarations of an object with hidden linkage refer to the same object
if they are in the same shared object.
...
In C++, the visibility attribute applies to types as well as functions
and objects, because in C++ types have linkage. ...
It became essential to do this because the previous documentation was
incomprehensible for users who knew C++ but did not understand every
detail of the C++ linkage model---and insane for users who did
understand every detail of some other system's C++ linkage model and
expected that GCC behaved similarly.
GCC's concept of visibility is very different to that of some other
compilers. I think this should be handled by features the purpose of
which is specifically to emulate those other compilers. __dllspec
sounds like one of those features; -fvisibility-ms-compat is another.
That has the advantage that since those other compilers are often not
precisely documented, then as our understanding of those compilers
improves and we adjust the feature to match, the amount of code that
need be broken is minimised.
In respect of this specific example, the compiler should not give any
effect to the attribute on 'f' in:
struct __attribute__((visibility("hidden"))) S {
void f() __attribute__((visibility("default")));
}
because another shared library can define:
struct __attribute__((visibility("default"))) S {
void f();
}
but the f() functions are different, because they are in different
name spaces; one is in the name space of S in one shared library,
another is in the name space of a different type S that is visible
globally. I don't have a strong opinion on whether this should be an
error, a warning that the attribute is ignored, or just silently
ignoring the attribute.
Re: RFC: Make dllimport/dllexport imply default visibility
Chris Lattner wrote:
> This description also makes sense, but is different than what was
> described before. To me, this description/implementation is extremely
> problematic, because the extension cannot be described without
> describing the implementation (specifically presence of vtables etc),
> which is unlike any standard C++ feature.
That's because it's an ELF-level attribute. It's like dirty tricks you
can play with the "alias" attribute (means two functions can have the
same address) or making things weak (means addresses of objects with
static storage duration can be NULL). These things are all unappealing
from a language theory point of view; they explicitly go behind the back
of the language to do odd things. This is the ugly reality of C/C++,
and also part of why the languages are so useful.
> Some more specific questions:
I'll answer with what I think should happen and with what I know about
what G++ does. I'm not sure what RealView does in all of these cases.
> 1. If a class is hidden, does that default all the members (not just the
> metadata) to be notshared?
Yes. (This is what G++ does.)
> 2. If a class with vtable is hidden, what visibility constraints exist
> on virtual methods?
None. In particular, the virtual methods may be imported from another
shared object. (However, if a virtual function is hidden, then the
vtable must also be defined in the same shared object, as otherwise you
will get a link error.)
> 3. What does 'notshared' on a class without a vtable mean, what effect
> does it have?
It means that all the members have hidden visibility, unless otherwise
specified. (This is what G++ does.)
> 4. If classes with vtables have different behavior than those without,
> is this something we want?
There's no difference.
The notshared attribute means that all of the members,
compiler-generated or otherwise, have hidden visibility, unless
otherwise explicitly specified. It doesn't say anything about the type
per se, just about the various members.
> 5. How does this impact the ODR?
It's beyond the scope of the ODR. Even for two hidden functions "f",
each defined in different shared objects, the name of both is "::f" --
but is that an ODR violation? We're way outside the standard at this point.
In practice, people use all of these facilities to do things that
explicitly violate the ODR. For example, in an implementation DLL:
struct S {
__attribute__((visibility("hidden"))) void f();
__declspec(dllexport) void g();
};
In other DLLs, the header for S looks like:
struct S {
__declspec(dllimport) void g();
};
and "f" is not even declared. Certainly, the intent is that these are
the same types, but as a literal reading of the standard that's an ODR
violation. (Even if you left "f" in, it would be an ODR violation in a
literal sense due to the change from dllexport to dllimport, unless you
modify the ODR rules to be more structural than literal. They actually
require token-for-token identity.)
That's why the right way to understand these attributes is purely in
terms of what they do to object files. Yes, that's crossing an
abstraction barrier, and yes, it means that as a programmer, you need to
understand that "there's some vtables and stuff out there", but that's
they way people use these bits in practice.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
On Jun 19, 2007, at 7:49 AM, Richard Earnshaw wrote:
On Mon, 2007-06-18 at 10:04 -0700, Mark Mitchell wrote:
I suspect that the realview compiler accepts
this as an oversight or a bug, not as an intentional feature.
Let's ask.
Richard E., is the fact that RealView 3.0SP1 accepts:
class __declspec(notshared) S {
__declspec(dllimport) void f();
};
a bug or a feature? If this is considered a bug, is it something
that
RealView is likely to change in a future release, or will it be
preserved for the forseeable future for backwards compatibility?
This is well beyond my sphere of expertise, so I've asked one of the
original developers of the spec. He asserts that the above is
supported
and intentional. Hopefully I've correctly represented his position
below.
His key point is that 'notshared' on a class is not the same as making
the whole class hidden: only the class impedimenta (vtables, rtti) is
hidden, but the rest of the class can be exported as normal. And that
since it can be exported, there's no reason why definitions of member
functions can't be imported.
This description also makes sense, but is different than what was
described before. To me, this description/implementation is
extremely problematic, because the extension cannot be described
without describing the implementation (specifically presence of
vtables etc), which is unlike any standard C++ feature.
Some more specific questions:
1. If a class is hidden, does that default all the members (not just
the metadata) to be notshared?
2. If a class with vtable is hidden, what visibility constraints
exist on virtual methods?
3. What does 'notshared' on a class without a vtable mean, what
effect does it have?
4. If classes with vtables have different behavior than those
without, is this something we want?
5. How does this impact the ODR?
-Chris
Re: RFC: Make dllimport/dllexport imply default visibility
Richard Earnshaw wrote:
> On Mon, 2007-06-18 at 10:04 -0700, Mark Mitchell wrote:
>>> I suspect that the realview compiler accepts
>>> this as an oversight or a bug, not as an intentional feature.
>> Let's ask.
>>
>> Richard E., is the fact that RealView 3.0SP1 accepts:
>>
>> class __declspec(notshared) S {
>> __declspec(dllimport) void f();
>> };
>>
>> a bug or a feature? If this is considered a bug, is it something that
>> RealView is likely to change in a future release, or will it be
>> preserved for the forseeable future for backwards compatibility?
>
> This is well beyond my sphere of expertise, so I've asked one of the
> original developers of the spec. He asserts that the above is supported
> and intentional. Hopefully I've correctly represented his position
> below.
Thank you for following up on this!
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
On Mon, 2007-06-18 at 10:04 -0700, Mark Mitchell wrote:
> > I suspect that the realview compiler accepts
> > this as an oversight or a bug, not as an intentional feature.
>
> Let's ask.
>
> Richard E., is the fact that RealView 3.0SP1 accepts:
>
> class __declspec(notshared) S {
> __declspec(dllimport) void f();
> };
>
> a bug or a feature? If this is considered a bug, is it something that
> RealView is likely to change in a future release, or will it be
> preserved for the forseeable future for backwards compatibility?
This is well beyond my sphere of expertise, so I've asked one of the
original developers of the spec. He asserts that the above is supported
and intentional. Hopefully I've correctly represented his position
below.
His key point is that 'notshared' on a class is not the same as making
the whole class hidden: only the class impedimenta (vtables, rtti) is
hidden, but the rest of the class can be exported as normal. And that
since it can be exported, there's no reason why definitions of member
functions can't be imported.
R.
Re: RFC: Make dllimport/dllexport imply default visibility
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 surp
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
Re: RFC: Make dllimport/dllexport imply default visibility
Chris Lattner wrote:
[Richard E., please see below for a question re. RealView's behavior.]
>> You and Chris are taking the view that "the type" has a location. But,
>> a lot of people don't look at it this way. The things that have
>> locations are variables (including class data) and functions. After
>> all, types don't appear in object files; only variables and functions do.
>
> 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.
>> 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.
>> But, the visibility attribute is only specified in terms of its effects
>> on ELF symbols, not as having C++ semantics per se. The hidden
>> visibility attribute says that all members of the class have hidden
>> visibility, unless otherwise specified.
>
> I'll paraphrase this as saying: "this is already an extension, not a
> standard - we can extend the extension without remorse".
Currently, the compiler generates wrong code: it generates a hidden
reference to a dllimport'd function. There are two ways to fix that:
declare the construct invalid, or make the compiler generate a
non-hidden reference. The second option might be an "extension to an
extension" but it might also just be "a bug fix".
>> 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."
ELF operates at a level below C++, and can be used to do things that C++
does not allow. 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.
> I suspect that the realview compiler accepts
> this as an oversight or a bug, not as an intentional feature.
Let's ask.
Richard E., is the fact that RealView 3.0SP1 accepts:
class __declspec(notshared) S {
__declspec(dllimport) void f();
};
a bug or a feature? If this is considered a bug, is it something that
RealView is likely to change in a future release, or will it be
preserved for the forseeable future for backwards compatibility?
> 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 so
Re: RFC: Make dllimport/dllexport imply default visibility
On Jun 17, 2007, at 6:40 PM, Dave Korn wrote: On 17 June 2007 18:17, Ross Ridge wrote: Daniel Jacobowitz writes: The minimum I'd want to accept this code would be a complete and useful example in the manual; since Mark and Danny say this happens a lot on Windows I don't understand how this issue can come up at all on Windows. As far I know, visibility is an ELF-only thing, while DLLs are a PE-COFF- only thing. Is there some platform that supports both sets of attributes? I'm fairly sure it's a non-issue for windows. I think that dllimport/dllexport are implemented on more than just windows but I'm not sure. What they mean on any other platform is anyone's guess, but one thing's for sure: visibility attributes apply to ELF symbols and not anything else at all. They also apply to macho symbols on darwin. -Chris
RE: RFC: Make dllimport/dllexport imply default visibility
On 17 June 2007 18:17, Ross Ridge wrote: > Daniel Jacobowitz writes: >> The minimum I'd want to accept this code would be a complete and useful >> example in the manual; since Mark and Danny say this happens a lot >> on Windows > > I don't understand how this issue can come up at all on Windows. As far > I know, visibility is an ELF-only thing, while DLLs are a PE-COFF-only > thing. Is there some platform that supports both sets of attributes? I'm fairly sure it's a non-issue for windows. I think that dllimport/dllexport are implemented on more than just windows but I'm not sure. What they mean on any other platform is anyone's guess, but one thing's for sure: visibility attributes apply to ELF symbols and not anything else at all. cheers, DaveK -- Can't think of a witty .sigline today
Re: RFC: Make dllimport/dllexport imply default visibility
Daniel Jacobowitz writes: >The minimum I'd want to accept this code would be a complete and useful >example in the manual; since Mark and Danny say this happens a lot >on Windows I don't understand how this issue can come up at all on Windows. As far I know, visibility is an ELF-only thing, while DLLs are a PE-COFF-only thing. Is there some platform that supports both sets of attributes? Ross Ridge
Re: RFC: Make dllimport/dllexport imply default visibility
On Sat, Jun 16, 2007 at 10:13:34PM -0700, Chris Lattner wrote: > Because there is no standard to reference, I think it's important to consider > these things in terms of explainability. Chris said everything I could have said about this (better, too). I want to particularly highlight this part. The minimum I'd want to accept this code would be a complete and useful example in the manual; since Mark and Danny say this happens a lot on Windows, I'm sure there must be one. -- Daniel Jacobowitz CodeSourcery
Re: RFC: Make dllimport/dllexport imply default visibility
On Jun 16, 2007, at 11:52 AM, Mark Mitchell wrote:
Daniel Jacobowitz wrote:
This doesn't make a lick of sense to me. If the type is hidden, how
on earth can it get a member function _of that type_ from another
library? That library would, by definition, have to have a type of
the same name... but it would be a "different" type.
You and Chris are taking the view that "the type" has a location.
But,
a lot of people don't look at it this way. The things that have
locations are variables (including class data) and functions. After
all, types don't appear in object files; only variables and
functions do.
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.
Even now, "types" can live in object files through debug info, RTTI,
vtables, etc. Obviously there is a subset of cases where these don't
apply and where the approach you propose can make sense.
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.
But, the visibility attribute is only specified in terms of its
effects
on ELF symbols, not as having C++ semantics per se. The hidden
visibility attribute says that all members of the class have hidden
visibility, unless otherwise specified.
I'll paraphrase this as saying: "this is already an extension, not a
standard - we can extend the extension without remorse".
We also allow:
struct __attribute__((visibility("hidden"))) S {
__attribute__((visibility("default"))) void f();
void g();
};
which may seem equally odd: it says that you can call a function from
outside this translation unit even though you can't see the type.
But,
code like this is reasonable; it says that within the defining shared
object you can call all the functions, but that elsewhere you can call
"f" but not "g". (That's orthogonal to C++ access specifiers; private
functions can have default visibility, and public ones can have hidden
visibility.)
The above class should be exactly the same as:
struct S {
__attribute__((visibility("default"))) void f();
__attribute__((visibility("hidden"))) void g();
};
since there is no virtual table or other class data.
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).
I notice that your original patch did not include a documentation
patch explaining the new semantics of the extension. I believe it
would be very difficult to convey the proposed semantics to people
who are not familiar with the Itanium ABI class layout and other
related details of the G++ implementation. I suspect that the
realview compiler accepts this as an oversight or a bug, not as an
intentional feature.
For better or worse (usually worse), people's understanding of the
language they code in is often defined by what their compiler happens
to accept. Because this is an (one of many) il-documented extension,
the *only* documentation is in what the compiler accepts.
How does that serve our users?
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.
I believe that rejecting this by default is important because people
often define what is valid by what their compiler accepts, not what
the standard says or what is logically consistent. :)
-Chris
RE: RFC: Make dllimport/dllexport imply default visibility
> -Original Message-
> From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On
> Behalf Of Mark Mitchell
> Sent: Saturday, 16 June 2007 11:47 a.m.
>
> Chris Lattner wrote:
>
> > This construct seems like it should be rejected by the C++
> front-end.
> > The source is making two contradictory claims: the struct
> is not visible
> > outside this library, but part of it is implemented outside of it.
>
> I don't think there's a contradiction. The declaration on
> the structure
> is the default for the members and applies to the vtable and
> other class
> data. There's no reason the members shouldn't be implemented
> elsewhere,
> and there's certainly existing code (in Windows, SymbianOS, and other
> DLL-based operating systems, whether or not there is on
> GNU/Linux) that
> implements different class members in different DLLs, while still not
> exporting the class from its home DLL. One situation where this is
> useful is when the class members are actually shared between multiple
> classes, or are also callable as C functions, etc.
>
>
In windows dll's the default visibility of a symbol is hidden (GNU ld as
--export-all-extension to override this default, but that is not the
compiler's fault).
So this, in a dll module source:
>> Consider:
>>
>> struct __attribute__((vsibility ("hidden"))) S {
>>void __declspec(dllimport) f();
>> };
is equivalent to
>> struct __attribute__ S {
>>void __declspec(dllimport) f();
>> };
And certainly that is meaningful on window's targets
Danny
Re: RFC: Make dllimport/dllexport imply default visibility
Daniel Jacobowitz wrote:
> This doesn't make a lick of sense to me. If the type is hidden, how
> on earth can it get a member function _of that type_ from another
> library? That library would, by definition, have to have a type of
> the same name... but it would be a "different" type.
You and Chris are taking the view that "the type" has a location. But,
a lot of people don't look at it this way. The things that have
locations are variables (including class data) and functions. After
all, types don't appear in object files; only variables and functions do.
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.
But, the visibility attribute is only specified in terms of its effects
on ELF symbols, not as having C++ semantics per se. The hidden
visibility attribute says that all members of the class have hidden
visibility, unless otherwise specified.
We also allow:
struct __attribute__((visibility("hidden"))) S {
__attribute__((visibility("default"))) void f();
void g();
};
which may seem equally odd: it says that you can call a function from
outside this translation unit even though you can't see the type. But,
code like this is reasonable; it says that within the defining shared
object you can call all the functions, but that elsewhere you can call
"f" but not "g". (That's orthogonal to C++ access specifiers; private
functions can have default visibility, and public ones can have hidden
visibility.)
The above class should be exactly the same as:
struct S {
__attribute__((visibility("default"))) void f();
__attribute__((visibility("hidden"))) void g();
};
since there is no virtual table or other class data.
As a motivating example for things like the original case I posted, this
is a technique for permitting users to provide member functions for
classes that you provide in a DLL. If your DLL needs some
system-specific routines, one way to get them is to have the user
provide another DLL containing implementations of various functions in
the class hierarchy. That can be more efficient at run-time than
passing in a separate callback object for the DLL to use, because it
avoids indirecting through the vtable of the callback object.
The bottom line is that you're taking the view that a construct with
well-defined semantics (namely, that the GNU visibility attribute map
directly to the ELF visibility without any further linguistic
implications) and which is easy to implement should be considered
invalid, despite conflicting with existing practice of other compilers
and large bodies of application code.
How does that serve our users?
Note: I'm not arguing about issuing a warning about this with "-W", if
people want to do that. These mixtures of visibility are certainly not
the typical case.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
On Jun 15, 2007, at 4:47 PM, Mark Mitchell wrote:
Chris Lattner wrote:
This construct seems like it should be rejected by the C++ front-end.
The source is making two contradictory claims: the struct is not
visible
outside this library, but part of it is implemented outside of it.
I don't think there's a contradiction. The declaration on the
structure
is the default for the members and applies to the vtable and other
class
data.
If we separate implementation details from the logical semantics, I
see that anonymous namespaces and hidden visibility, when applied to
a type, global impact the visibility of the type. If the *type* is
hidden, no members of that type, nor any other functions/methods that
take or refer to it, should be able to be dllimport.
In your example:
So, why not:
struct S __attribute__((visibility("hidden")) {
void f();
void g() __attribute__((dllimport));
};
void S::f() { S::g(); };
This may happen to work, but remember that there is an implicit
"this" pointer in "g". Regardless of whether "g" dereferences the
this pointer, it still takes one. I agree with Andrew that it is an
ODR violation, regardless of whether these cases happen to not get
caught in some cases. The this pointer in S::f and the this pointer
in S::g are two different types.
I'm not sure why you say that the code is broken. It works.
"working" and "making sense" are two different things :)
As far as I know it doesn't violate any specification.
Is there any formal spec at all that defines dllimport, hidden
visibility, and their interaction?
There's no reason the members shouldn't be implemented elsewhere,
and there's certainly existing code (in Windows, SymbianOS, and other
DLL-based operating systems, whether or not there is on GNU/Linux)
that
implements different class members in different DLLs, while still not
exporting the class from its home DLL. One situation where this is
useful is when the class members are actually shared between multiple
classes, or are also callable as C functions, etc.
IMO, regardless of whether this is accepted by other compilers, this
is a serious logical inconsistency and should be rejected by
default. For compatibility with other compilers, I agree that it
would be reasonable to add a compatibility flag to enable this more
lenient behavior.
-Chris
Re: RFC: Make dllimport/dllexport imply default visibility
On Fri, Jun 15, 2007 at 04:47:04PM -0700, Mark Mitchell wrote: > data. There's no reason the members shouldn't be implemented elsewhere, > and there's certainly existing code (in Windows, SymbianOS, and other > DLL-based operating systems, whether or not there is on GNU/Linux) that > implements different class members in different DLLs, while still not > exporting the class from its home DLL. One situation where this is > useful is when the class members are actually shared between multiple > classes, or are also callable as C functions, etc. This doesn't make a lick of sense to me. If the type is hidden, how on earth can it get a member function _of that type_ from another library? That library would, by definition, have to have a type of the same name... but it would be a "different" type. -- Daniel Jacobowitz CodeSourcery
Re: RFC: Make dllimport/dllexport imply default visibility
Andrew Pinski wrote:
>> In any case, in practice, ARM's RealView compiler accepts:
>>
>> struct __declspec(notshared) S {
>> __declspec(dllimport) void f();
>> void g();
>> };
>>
>> void S::g() {
>> f();
>> }
>>
>> And, there's a large body of code that uses this.
>
> Because you missed typeinfo is also hidden (not just vtables).
That has nothing to do with the fact that there's a lot of existing code
out there depending on this.
> So in an user program you use typeid(S), you will get a link failure.
> If f and g are switch around, you will then not get a link failure but
> doing a "throw S();" with a catch on the outside, will cause the throw
> to be caught by a try{ } catch (S &a) ...
Sure. But, so what? If you try to access hidden variables from C,
things won't work then too.
You seem to be saying that because you can do things that don't work, we
should disallow a useful construct.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
On 6/15/07, Mark Mitchell <[EMAIL PROTECTED]> wrote:
So, why not:
struct S __attribute__((visibility("hidden")) {
void f();
void g() __attribute__((dllimport));
};
void S::f() { S::g(); };
In any case, in practice, ARM's RealView compiler accepts:
struct __declspec(notshared) S {
__declspec(dllimport) void f();
void g();
};
void S::g() {
f();
}
And, there's a large body of code that uses this.
Because you missed typeinfo is also hidden (not just vtables).
So if you have (which is what you showed):
struct S __attribute__((visibility("hidden")) {
void f();
void g() __attribute__((dllimport));
};
void S::f() { S::g(); };
The typeinfo will be hidden and only in the shared library and not
exported (and if it is then that is a bug).
So in an user program you use typeid(S), you will get a link failure.
If f and g are switch around, you will then not get a link failure but
doing a "throw S();" with a catch on the outside, will cause the throw
to be caught by a try{ } catch (S &a) ...
Thanks,
Andrew Pinski
Re: RFC: Make dllimport/dllexport imply default visibility
Andrew Pinski wrote:
> Well this allows for easier violating of ODR. I guess I am just a bit
> off of what is going on here but I agree with Chris in that this
> really should be rejected as you have stuff which is hidden and then
> you call a non hidden member function. How can the vtable be hidden
> while the member functions not be? I think if you try to throw that
> class across boundaries, it will never be caught as the typeinfos are
> different.
There's nothing that says that the class even has a vtable. Or, this
might be a static member function. Obviously, if that function tries to
access the vtable, it will fail to link -- but it might very well not
need to access the hidden stuff.
> So really I think this is just bad pratice and should at least get a
> warning, even though code in real life exists, the code is broken to
> say the least.
I'm not sure why you say that the code is broken. It works. As far as
I know it doesn't violate any specification. What's broken about it?
This is valid:
void f() __attribute__((visibility("hidden")));
void g() __attribute__((dllimport));
void f() { g(); };
So is:
struct S {
void f() __attribute__((visibility("hidden"));
void g() __attribute__((dllimport));
};
void S::f() { S::g(); };
So, why not:
struct S __attribute__((visibility("hidden")) {
void f();
void g() __attribute__((dllimport));
};
void S::f() { S::g(); };
In any case, in practice, ARM's RealView compiler accepts:
struct __declspec(notshared) S {
__declspec(dllimport) void f();
void g();
};
void S::g() {
f();
}
And, there's a large body of code that uses this.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
On 6/15/07, Mark Mitchell <[EMAIL PROTECTED]> wrote: I don't think there's a contradiction. The declaration on the structure is the default for the members and applies to the vtable and other class data. There's no reason the members shouldn't be implemented elsewhere, and there's certainly existing code (in Windows, SymbianOS, and other DLL-based operating systems, whether or not there is on GNU/Linux) that implements different class members in different DLLs, while still not exporting the class from its home DLL. One situation where this is useful is when the class members are actually shared between multiple classes, or are also callable as C functions, etc. Well this allows for easier violating of ODR. I guess I am just a bit off of what is going on here but I agree with Chris in that this really should be rejected as you have stuff which is hidden and then you call a non hidden member function. How can the vtable be hidden while the member functions not be? I think if you try to throw that class across boundaries, it will never be caught as the typeinfos are different. So really I think this is just bad pratice and should at least get a warning, even though code in real life exists, the code is broken to say the least. -- Pinski
Re: RFC: Make dllimport/dllexport imply default visibility
On Jun 15, 2007, at 3:45 PM, Mark Mitchell wrote:
Bill Wendling wrote:
Perhaps I'm mistaken, but the above seems to indicate to me that the
structure (and, therefore, all of its fields) are hidden, one of its
functions is from an external and visible source.
Yes. And, therefore, emitting a undefined reference to S::f with
hidden
linkage in the current translation unit causes S::f to have hidden
visibility in the shared object containing this translation unit. For
example, if the translation unit goes on to say:
void g() {
S s;
s.f();
}
we will now have an undefined reference to S::f with hidden
visibility.
As a result, S::f will have hidden visibility in the shared object
containing this translation unit. Thus, despite dllimport, the user
cannot actually import a function of a hidden class from another DLL.
That seems bad.
Okay. What I mentioned was based on the docs for 4.0.1 where it says:
"On Microsoft Windows and Symbian OS targets, the dllimport attribute
causes the compiler to reference a function or variable via a global
pointer to a pointer that is set up by the DLL exporting the symbol."
So my thoughts were, "because a hidden structure could still have a
pointer which points to global data, then this should be okay." But
the scenario you described is clearly bad.
-bw
Re: RFC: Make dllimport/dllexport imply default visibility
Chris Lattner wrote: > This construct seems like it should be rejected by the C++ front-end. > The source is making two contradictory claims: the struct is not visible > outside this library, but part of it is implemented outside of it. I don't think there's a contradiction. The declaration on the structure is the default for the members and applies to the vtable and other class data. There's no reason the members shouldn't be implemented elsewhere, and there's certainly existing code (in Windows, SymbianOS, and other DLL-based operating systems, whether or not there is on GNU/Linux) that implements different class members in different DLLs, while still not exporting the class from its home DLL. One situation where this is useful is when the class members are actually shared between multiple classes, or are also callable as C functions, etc. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
On Jun 15, 2007, at 3:45 PM, Mark Mitchell wrote:
Bill Wendling wrote:
On Jun 15, 2007, at 12:48 AM, Mark Mitchell wrote:
Consider:
struct __attribute__((vsibility ("hidden"))) S {
void __declspec(dllimport) f();
};
At present, we give "f" hidden visibility. That seems odd since the
user has explicitly told us that the symbol is coming from another
shared library.
I'm planning to make any dllimport or dllexport attribute imply
default
visibility. Is that a bad idea?
Yes. And, therefore, emitting a undefined reference to S::f with
hidden
linkage in the current translation unit causes S::f to have hidden
visibility in the shared object containing this translation unit. For
example, if the translation unit goes on to say:
void g() {
S s;
s.f();
}
we will now have an undefined reference to S::f with hidden
visibility.
As a result, S::f will have hidden visibility in the shared object
containing this translation unit. Thus, despite dllimport, the user
cannot actually import a function of a hidden class from another DLL.
This construct seems like it should be rejected by the C++ front-
end. The source is making two contradictory claims: the struct is
not visible outside this library, but part of it is implemented
outside of it. This should be rejected by the front-end, not the
linker IMO.
-Chris
Re: RFC: Make dllimport/dllexport imply default visibility
Bill Wendling wrote:
> On Jun 15, 2007, at 12:48 AM, Mark Mitchell wrote:
>
>> Consider:
>>
>> struct __attribute__((vsibility ("hidden"))) S {
>>void __declspec(dllimport) f();
>> };
>>
>> At present, we give "f" hidden visibility. That seems odd since the
>> user has explicitly told us that the symbol is coming from another
>> shared library.
>>
>> I'm planning to make any dllimport or dllexport attribute imply default
>> visibility. Is that a bad idea?
>>
> Perhaps I'm mistaken, but the above seems to indicate to me that the
> structure (and, therefore, all of its fields) are hidden, one of its
> functions is from an external and visible source.
Yes. And, therefore, emitting a undefined reference to S::f with hidden
linkage in the current translation unit causes S::f to have hidden
visibility in the shared object containing this translation unit. For
example, if the translation unit goes on to say:
void g() {
S s;
s.f();
}
we will now have an undefined reference to S::f with hidden visibility.
As a result, S::f will have hidden visibility in the shared object
containing this translation unit. Thus, despite dllimport, the user
cannot actually import a function of a hidden class from another DLL.
That seems bad.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: RFC: Make dllimport/dllexport imply default visibility
On Jun 15, 2007, at 12:48 AM, Mark Mitchell wrote:
Consider:
struct __attribute__((vsibility ("hidden"))) S {
void __declspec(dllimport) f();
};
At present, we give "f" hidden visibility. That seems odd since the
user has explicitly told us that the symbol is coming from another
shared library.
I'm planning to make any dllimport or dllexport attribute imply
default
visibility. Is that a bad idea?
Perhaps I'm mistaken, but the above seems to indicate to me that the
structure (and, therefore, all of its fields) are hidden, one of its
functions is from an external and visible source. Because nothing
outside of the translation unit that has S can access f directly (only
through a function pointer), I don't see a problem with marking it as
a hidden part of S. That is, as long as it doesn't affect the original
f()'s visibility.
I'll ask the opposite question: what would be gained by giving this
default visibility?
-bw
