Tcl Source Code

Here is a patch, which makes the MODULE_SCOPE
machinery unnecessary when using gcc. It makes
everything compile with the gcc option
-fvisibility=hidden (when available), so the EXPORT
macro controls which symbols are exported.

I am very interested to know whether Darwin, on
which MODULE_SCOPE is currently translated by
"__private_extern__" has a similar compiler options,
making all symbols hidden by default, except the
ones explicitely exported. Then MODULE_SCOPE
can fully be deprecated.

Reported by Stuart Cassoff: ================================= Hi!

Here's some snippets from an obsd discussion on symbol visibility. I thought you might find it interesting.

Stu

...

2) declares the internal functions explicitly hidden, instead of using -fvisibility=hidden and default declarations in sndio.h

Why (2)? Well, -fvisibility=hidden doesn't actually get you the full performance benefits, as it only affects definitions and not declarations. The result is that inter-file file calls still use the PLT unless you explicitly declare the internal functions hidden.

allow_comments - 1

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

Adding additional warning options seems out-of-scope for this Issue. If everything works fine, this simply should be closed. Agreed?

Maybe we should consider adding -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes (at least for gcc in debug mode; don't need to worry random builders, but do want to be able to audit for blunders)

Thanks, Donal. But it might be that,
because of this change, some symbols
that were exported before, are not
exported any more. I found and fixed
2 such cases in Tk.

Every such difference is a bug, unrelated to this
change! I can see 2 possibilities for this to happen:
- A missing #include, so the EXTERN in
  this include is not picked up by the compiler
- symbols which neither have EXTERN nor
  MODULE_SCOPE.

So, could anyone do a build (both Tcl and Tk)
with the current "configure" script and the previous
one, and compare the list of exported symbols.
It should be exactly the same!

Seems to function correctly for me on OSX (as in "a clean configure and build with it works"). Great!

Patch was still not 100% correct, but now it is.
DKF's and DAS's remark handled.
Checked in for HEAD. 

Assigning to das, could you please check if everything
works on mac as well? I am pretty sure it works as is,
but checking never hurts.

Thanks!

File Added - 365189: visibility_hidden.patch

File Deleted - 365183:

New version of patch, reverting to the current
behavior if -fvisibility=hidden is not available.
So, no regression.

File Added - 365183: visibility_hidden.patch

File Deleted - 364439:

Since we're going to be supporting all sorts of different compilers (on some platforms I use, I've got gcc 2.* still) for a long time across lots of platforms, I can't see that removing MODULE_SCOPE is necessarily a good move. The "get the overall visibility right" is an audit requirement, and we have make targets to support it.

If you want to rewrite configure so as to allow for the case you describe (compiler flag, etc.) then that could be trialled. But that's a configure change and not a change to the Tcl and Tk sources.

This means, in fact, that the title of this Issue is
wrong. The goal is not to phase out MODULE_SCOPE,
the goal is to add support for compilers who, like gcc 4.x
have a command line option to make all symbols hidden
except especially decorated ones.

>All that said, I am strongly in favor of using -fvisibility=hidden when
>available, I just think it would be a shame to loose the extra flexibility
>the current scheme affords us w.r.t non-gcc compilers/linkers.

Yes, I fully agree with that. My goal is first get gcc 4.x working with
-fvisibility=hidden, and for all other compilers keep using the
current MODULE_SCOPE method. Then later we can always
extend it to let other compilers use their -fvisiliby=hidden variant.

as mentioned, IMO it would be better to have a configure check for __attribute__ ((visibility("default"))), gcc > 3 is not the only compiler that supports this, so the check in tcl.h will miss some cases and be a regression over the current state of things.

as far as discovering mistakenly exported symbols, that's what the 'checkexports' and 'checkstubs' makefile targets are there for... I try to remember to run these shortly before a release to make sure that we didn't forget any decorations.

All that said, I am strongly in favor of using -fvisibility=hidden when available, I just think it would be a shame to loose  the extra flexibility the current scheme affords us w.r.t non-gcc compilers/linkers.

Thanks for the reaction!

- The handling of __attribute__((__visibility__("default"))) is done
  in tcl.h, and this is already available. Also, the needed
  -DBUILD_tcl and -DBUILD_tk is already part of the current
  Makefile.in. It should work for Darwin as is....

> After all the work adding MODULE_SCOPE everywhere, where is the
> need/benefit to removing it again?
I wouldn't start removing it, until it's been well tested on
all platforms. (I called it 'deprecate', not 'remove'). However,
being obliged to decorate all exported symbols with
EXTERN and all others with MODULE_SCOPE,
that's asking for trouble forgetting one. Before
starting with this, I compared the list of Tk exported
symbols, and found two problematic symbols.
One that didn't respect the EXTERN because
of a missing include (TkpCmapStressed), another
symbol that was not supposed to be exported at
all (TkSetTransientFor). I fixed those two, but with
-fvisibility=hidden, it would already have been
discovered long ago....

Anyway, it's not high priority. I agree that
the attribute should be kept as fallback.

Any more remarks?

with recent enough gcc, we use visibility hidden on Darwin as well, but with older gcc that did not support visibility attributes yet we fall back to the Apple extension private_extern.

MODULE_SCOPE affords us the flexibility to accommodate compiler/linkers that don't have an option to hide symbols by default and only export certain marked symbols.

After all the work adding MODULE_SCOPE everywhere, where is the need/benefit to removing it again?

Note that there are compilers (e.g. clang) that support __attribute__((__visibility__("hidden"))) but do not have the same commandline flags as gcc, your patch would be a regression there, the check for the attribute should remain as a fallback if the test for -fvisibility=hidden fails.

Also, the current patch does not seem to test for __attribute__((__visibility__("default"))), which you will presumably need for the EXPORT macro ? (also not part of the patch)

File Added - 364439: visibility_hidden.patch