Skip to content

Support -fvisibility=hidden#1207

Closed
DemiMarie wants to merge 4 commits intoocaml:trunkfrom
DemiMarie:visibility-fixes
Closed

Support -fvisibility=hidden#1207
DemiMarie wants to merge 4 commits intoocaml:trunkfrom
DemiMarie:visibility-fixes

Conversation

@DemiMarie
Copy link
Copy Markdown
Contributor

CAMLprim and CAMLexport currently expand to nothing, but it can be
made to export the associated declaration even when
-fvisibility=hidden is used.

Also, eliminate the need for CAMLnoreturn_end by moving its
expansion to that of CAMLnoreturn_start.

@xavierleroy
Copy link
Copy Markdown
Contributor

I'm curious to learn more about what you're trying to achieve. I have a feeling I'll like it in the end, but please tell us more about it.

@DemiMarie
Copy link
Copy Markdown
Contributor Author

Currently, the documented way to write C stubs doesn't work with -fvisibility=hidden or in C++. Furthermore, it is necessary to write CAMLnoreturn_start and CAMLnoreturn_end, which is clumsy.

Functions that use CAMLprim are meant to be called from OCaml, which calls them using the C ABI and is in a different object than the C stubs are in most cases. Therefore, it is necessary to export these. However, most other functions generally should not be exported from a shared library, for reasons that can be found in the GCC wiki. CAMLprim is documented to ensure that a C function can be called by OCaml. Currently, it doesn't do that, which is a bug.

@dra27
Copy link
Copy Markdown
Member

dra27 commented Jun 20, 2017

I don't think the CAML_noreturn part is related to the CAMLprim change - please could they be separate commits. I like the look of that change, but I wonder if there are exotic platforms where this could be a problem.

For the CAMLprim change, AppVeyor seems to be failing - I haven't looked in detail, but is this change appropriate for Windows/Cygwin given FlexDLL?

CAMLprim and CAMLexport currently expand to nothing, but it can be
made to export the associated declaration even when
-fvisibility=hidden is used.
CAMLnoreturn_end expands to an __attribute__ annotation when a
GNU-compatible compiler is in use, and nothing otherwise.  But
attributes can also go before a declaration, where CAMLnoreturn_start
is placed.  Therefore, moving the __attribute__ to before the
declaration allows eliminating CAMLnoreturn_end on all platforms.
This allows writing OCaml primitives in C++ without having to manually
place extern "C" around them.
@dra27
Copy link
Copy Markdown
Member

dra27 commented Jul 7, 2017

The Windows build is still broken.

Thanks for splitting the original commit, although with the extra one you have added, this now feels like 3 pull requests merged into one. What happens with the change for C++ for C++ which have already manually annotated extern "C" CAMLprim ...?

@xavierleroy
Copy link
Copy Markdown
Contributor

Concerning CAMLnoreturn_end, I believe it was introduced by @bschommer and @gasche in an attempt to support both GCC and MSVC. Maybe they would like to comment on the proposal.

@xavierleroy
Copy link
Copy Markdown
Contributor

Concerning Windows support, I'm still concerned that the dllexport attribute doesn't play nice with FlexDLL. @alainfrisch would know.

@alainfrisch
Copy link
Copy Markdown
Contributor

dllexport instructs the linker to put the symbol in the export table when generating the PE image (.exe/.dll). I think this is equivalent to creating a .def file which collects all marked symbols.

FlexDLL does not rely on the visibility of symbols in the PE format and implements its own dynamic linker and export/import tables. In particular, it does not honor the dllexport export (but should happily forward it to the underlying linker). Currently, all public symbols in the DLL will end up in the FlexDLL export table.

So as far as I can tell, the change does not "fix" anything and will only export more symbols in the generated images (at the PE level). At least for CAMLprim, the attribute seems useless. For CAMLexport, I don't know. One could argue that if the final image (linked in a DLL) is intended to be used by non-OCaml code, the attribute could be used to explicitly tell the linker to make the symbol accessible from outside the OCaml world. Is that the intended usage of CAMLexport?

@bschommer
Copy link
Copy Markdown
Contributor

If I remember correctly the CAMLnoreturn_end was introduced for cosmetic reasons only.

@xavierleroy
Copy link
Copy Markdown
Contributor

Once more, I think there are good ideas in there but they need to be developed and discussed more. Either we do this or we close this PR.

@damiendoligez damiendoligez added this to the 4.07-or-later milestone Sep 25, 2017
@damiendoligez damiendoligez removed this from the consider-for-4.07 milestone Jun 5, 2018
@gasche
Copy link
Copy Markdown
Member

gasche commented Apr 28, 2021

Quoting the last message in 2017:

Once more, I think there are good ideas in there but they need to be developed and discussed more. Either we do this or we close this PR.

The discussion has not happened, so closing. @DemiMarie, feel free to actually discuss things if you still want to, we can reopen if relevant.

@gasche gasche closed this Apr 28, 2021
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Co-authored-by: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com>
Co-authored-by: Thibaut Mattio <thibaut.mattio@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants