On 2020-02-26 14:31, Bob Vandette wrote:

On Feb 26, 2020, at 7:31 AM, Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> 
wrote:

On 2020-02-26 08:41, David Holmes wrote:
Hi Bob,

Adding build-dev.
Thanks for noticing that we were missing, David!
Sorry, I should have included you folks.

On 26/02/2020 6:37 am, Bob Vandette wrote:
Please review this RFE that alters the visibility of JNI entrypoints to hidden 
when a shared library
is created using static JDK libraries.

RFE:

https://bugs.openjdk.java.net/browse/JDK-8239563

WEBREV:

http://cr.openjdk.java.net/~bobv/8239563/webrev.00/

CSR:

https://bugs.openjdk.java.net/browse/JDK-8239791

All JNI entrypoints that exist in JDK static libraries are declared as exported 
or visible.
If a dynamic library is built from these static libraries, we end up with many 
exported
functions that we don't want to provide access to,

This RFE will change the definition of JNIEXPORT for libraries built when 
JNI_STATIC_BUILD
is defined.  When defined, functions declared with JNIEXPORT will not be 
exported when
linked into shared or dynamic libraries.  This will still allow linking of 
these functions into
dynamic libraries but will not export the JDK symbols outside of the shared 
library.

A CSR has been filed (https://bugs.openjdk.java.net/browse/JDK-8239791) to add 
the JNI_STATIC_BUILD
define support in jni.h.
I have reservations about turning this into something we have to expose and 
support, rather than being something totally handled within the OpenJDK build 
system.
I fully agree. The JNI headers are an exported interface. Our internal build 
mechanisms have nothing to do there.

I'm tempted to suggest the header files be adjusted to have:

     #ifndef JNIEXPORT
     <JNIEXPORT basic definitions as they are now >
     #endif

and then we define the modified JNIEXPORT via the build system when doing a 
static build.

Is that feasible?
It's definitely doable, and a far better solution.
Yes, I like this solution.

A patch something akin to this would be needed:

diff --git a/make/autoconf/flags-cflags.m4 b/make/autoconf/flags-cflags.m4
--- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -709,7 +709,10 @@
    # JDK libraries.
    STATIC_LIBS_CFLAGS="-DSTATIC_BUILD=1"
    if test "x$TOOLCHAIN_TYPE" = xgcc || test "x$TOOLCHAIN_TYPE" = xclang; then
-    STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -ffunction-sections 
-fdata-sections"
+    STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -ffunction-sections \
+        -fdata-sections -DJNIEXPORT=__attribute__((visibility(\"hidden\")))"
+  else
+    STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -DJNIEXPORT="
    fi
    if test "x$TOOLCHAIN_TYPE" = xgcc; then
      # Disable relax-relocation to enable compatibility with older linkers

(With the reservation that I just wrote this on the fly and have not tested it 
-- things like quoting might be off. Also, I'm not sure if the match of
compilers is correct -- it might be the case that all compilers except 
Microsoft defines __GNUC__, so maybe the addition of this -D flag might need
a separate if statement to cover all our compilers correctly.)
Most of the STATIC_BUILD support is done in jni_util.h.  We could define 
JNIEXPORT in that header file after allowing it to be overridden in jni.h.
I'm not sure I understand you correctly here. Do you mean that you'd like to re-define JNIEXPORT inside jni_util.h instead of using compiler command line flags? I don't think that'd work -- all libraries using JNIEXPORT that does not include jni_util.h first would then export their symbols just the same. Even if you fixed those, the system would be very fragile.

/Magnus

Bob.

/Magnus
Thanks,
David

BACKGROUND:

In JDK8 the JNI specification and JDK implementation was enhanced to support 
static JNI libraries
but we didn’t consider the issue of exportibility of JNI entrypoint symbols.

    https://bugs.openjdk.java.net/browse/JDK-8005716

If developers use these static JDK libraries in order to produce a custom 
shared library, all of the
JNIEXPORTS will be exposed by this library even if the developer did not choose 
to export these.
This is a security issue and a potential problem if this library is mixed with 
other libraries containing
these symbols.

Bob.




Reply via email to