On Mon, 25 Mar 2024 09:11:40 GMT, Magnus Ihse Bursie <[email protected]> wrote:
> > And neither should we compile or link it with "-fvisibility=hidden". That
> > is the root of this problem.
>
> If you suggest that we should not compile hsdis with hidden visibility, I
> disagree.
Yes, that's what I would do.
> I have been working hard on unifying build of native libraries across the
> entire product, to fix holes where we have not used a consistent way of
> compiling and/or linking. There is no reason to tread hsdis differently. If I
> restore using hidden visibility as an option that all native libraries,
> except hsdis, must opt in to, then we are just back to square one, and
> suddenly someone will forget about it. Instead, now we set
> -fvisibility=hidden in configure so nobody can forget about it.
OK, OK! So please can we get this fix in?
> Robbin proposes to change this to
>
> ```
> #if defined(_WIN32)
> __declspec(dllexport)
> #elif defined(_GNU_SOURCE)
> __attribute__ ((visibility ("default")))
> #endif
> ```
>
> My counter-proposal was to replace it with just `JNIEXPORT`. Surely you can't
> say that is a worse solution?
JNIEXPORT is better, I guess, but it does mean that hsdis is no longer
standalone, and IMO it should have a pathological dependency on some JVM header
file. That's the problem here.
But really, whatever works is good with me. The last thing I want to do is
delay this fix any further.
Robbin has asked "How would you add jni.h ?" Is anyone going to answer?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/18400#issuecomment-2037672904
