RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

On 2018-03-23 18:24, Volker Simonis wrote:
> Hi Magnus,
>
> thanks for addressing this long standing issue! I haven't looked at
> the changes, but just want to share some general and historical notes:
>
> - Compiling with "-fvisibility=hidden" which hides all symbols expect
> the ones explicitly exported with
> "__attribute__((visibility("default")))" has been requested by SAP
> back in 2007 even before we had OpenJDK (see "Use -fvisibility=hidden
> for gcc compiles" https://bugs.openjdk.java.net/browse/JDK-6588413)
> and finally pushed into the OpenJKD around 2010.
> - "-fvisibility=hidden" gave us performance improvements of about 5%
> (JBB2005) and 2% (JVM98) on Linux/IA64 and 1,5% (JBB2005) and 0,5%
> (JVM98) on Linux/PPC64 because the compiler could use faster calls for
> non exported symbols. This improvement was only very small on x86
> tough.
That's a nice side effect! Although my main purpose here is 
maintainability, gaining performance is nothing I say no to. :)
> - "-fvisibility=hidden"/"__attribute__((visibility("default")))"
> applies BEFORE using the map files in the linking step (i.e. hidden
> symbols can't be exported any more even if mentioned in the map file)
> - because of the performance improvements we got by using
> "-fvisibility=hidden" it was worth while using it even though we had
> the mapfiles at the end of the process.
>
> Then we had several mail threads (which you probably remember because
> you were involved :) where we discussed to either remove the map files
> completely or instead generate them automatically during the build:
>
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12412
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12628
>
> The main arguments against removing the map files at that time were:
>
> 1. the danger to re-export all symbols of statically linked libraries
> (notably libstdc++ at that time)
> 2. loosing exports of compiler generated symbols like vtables which
> are required by the Serviceability Agent
>
> Point 1 is not a problem today, because I don't think we do any static
> linking any more. If we still do it under some circumstances, this
> problem should be re-evaluated.
Well, we do static linking with libstdc++ on linux, in certain 
circumstances. See "--with-stdc++lib=<static>,<dynamic>,<default>". 
Fortunately, this is not a problem. The linker can be told not to 
include symbols from statically linked libraries, which is exactly what 
I do with LDFLAGS_JDKLIB += -Wl,--exclude-libs,ALL.

The corresponding feature does not exist for the solstudio linker, but 
fortunately we do not use statically linked libraries there.

> Point 2 is only relevant for HotSpot. But because of "8034065: GCC 4.3
> and later doesn't export vtable symbols any more which seem to be
> needed by SA" (https://bugs.openjdk.java.net/browse/JDK-8034065),
> exporting such symbols trough a map files doesn't work any more
> anyway. So this isn't a problem either.
In any case, that's a question for another day. :) There were reasons I 
left Hotspot out of this fix, and the question about the SA agent is one 
of them. :) As you say, I think they do not apply anymore, but I'll 
return to consider Hotspot later on.

> So to cut a long story short - I think the time is ripe to get rid of
> the map files. Thumbs up from me (meant as moral support, not as a
> concrete review :)
Thanks for the kind words!

/Magnus

>
> Regards,
> Volker
>
> On Fri, Mar 23, 2018 at 5:05 PM, mandy chung <mandy.chung at oracle.com> wrote:
>> This is a very good change and no more mapfile to maintain!!
>>
>> Please do file JBS issues for the component teams to clean up their exports.
>>
>> Mandy
>>
>>
>> On 3/23/18 7:30 AM, Erik Joelsson wrote:
>>> I have looked at the build changes and they look good.
>>>
>>> Will you file followups for each component team to look over their
>>> exported symbols, at least for the libraries with $(EXPORT_ALL_SYMBOLS)? It
>>> sure looks like there is some technical debt laying around here.
>>>
>>> /Erik
>>>
>>>
>>> On 2018-03-23 06:56, Magnus Ihse Bursie wrote:
>>>> With modern compilers, we can use compiler directives (such as
>>>> _attribute__((visibility("default"))), or __declspec(dllexport)) to control
>>>> symbol visibility, directly in the source code. This has historically not
>>>> been present on all compilers, so we had to resort to using mapfiles (also
>>>> known as linker scripts).
>>>>
>>>> This is no longer the case. Now all compilers we use support symbol
>>>> visibility directives, in one form or another. We should start using this.
>>>> Since this has been the only way to control symbol visibility on Windows,
>>>> for most of the shared code, we already have proper JNIEXPORT decorations in
>>>> place.
>>>>
>>>> If we fix the remaining platform-specific files to have proper JNIEXPORT
>>>> tagging, then we can finally get rid of mapfiles.
>>>>
>>>> This fix removed mapfiles for all JDK libraries. It does not touch
>>>> hotspot libraries nor JDK executables; they will have to wait for a future
>>>> fix -- this was complex enough. This change will not have any impact on
>>>> macosx, since we do not use mapfiles there, but instead export all symbols.
>>>> (This is not a good idea, but I'll address that separately.) This change
>>>> will also have a minimal impact on Windows. The only reason Windows is
>>>> impacted at all, is that some changes needed by Solaris and Linux were
>>>> simpler to fix for all platforms.
>>>>
>>>> I have strived for this change to have no impact on the actual generated
>>>> code. Unfortunately, this was not possible to fully achieve. I do not
>>>> believe that these changes will have any actual impact on the product,
>>>> though. I will present the differences more in detail further down. Those
>>>> who are not interested can probably skip that.
>>>>
>>>> The patch has passed tier1 testing and is currently running tier2 and
>>>> tier3. Since the running code is more or less (see caveat below) unmodified,
>>>> I don't expect any testing issues.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
>>>> WebRev:
>>>> http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01
>>>>
>>>> Details on changes:
>>>> Most of the source code changes are (unsurprisingly) in java.base and
>>>> java.desktop. Remaining changes are in jdk.crypto.ucrypto,
>>>> jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.
>>>>
>>>> Source code changes does almost to 100% consists in decorating an
>>>> exported function with JNIEXPORT. I have also followed the long-standing
>>>> convention of adding JNICALL. This is a no-op on non-Windows platforms, so
>>>> for most of the changes this is purely cosmetic (and possibly adding in
>>>> robustness, should the function ever be used on Windows in the future). I
>>>> have also followed the stylistic convention of putting "JNIEXPORT <return
>>>> type> JNICALL" on a separate line. For some functions, however, this might
>>>> cause a change in calling convention on Windows. Since this can not apply to
>>>> exported functions on Windows (otherwise they would already have had
>>>> JNIEXPORT), I do not think this matters anything.
>>>>
>>>> A few libraries did not have a mapfile, on Linux and/or Solaris. This
>>>> actually meant that all symbols were exported. It is highly unclear if this
>>>> was known and intended by the original make rule writer. I have emulated
>>>> this by adding the flag $(EXPORT_ALL_SYMBOLS) to these libraries. Hopefully,
>>>> we can remove this flag and fix proper exported symbols in the future.
>>>>
>>>> I have run the complete build using COMPARE_BUILD, and made a thourough
>>>> analysis of the differences for Linux and Solaris. All native libraries have
>>>> symbol differences, but most of them are trivial and/or harmless. As a
>>>> result, most libraries have disasm differences as well, but these too seem
>>>> trivial and harmless. The differences in symbols that are common to all
>>>> libraries include:
>>>>   * Internal symbols such as __bss_start, _edata, _end and _fini are now
>>>> global. (They are imported as such from the compiler libraries/archives, and
>>>> we have no linker script to override this behavior).
>>>>   * The versioning tag SUNWprivate_1.1 is not included, and thus neither
>>>> the .gnu.version_d symbol.
>>>>   * There are a few differences in the symbol and/or mangling of some
>>>> local functions. I'm not sure what's causing this,
>>>> but it's unlikely to have any effect on the product.
>>>>
>>>> Another common source for change in symbols is due to previous platform
>>>> differences. For instance, if we had "JNIEXPORT int JNICALL do_foo() { ...
>>>> }", but do_foo was not in the mapfile, the symbol was exported on Windows
>>>> but not on Linux and Solaris. (Presumable since it was not needed there,
>>>> even though it was compiled for those platforms as well.) Now, with the
>>>> mapfiles gone, do_foo() will be exported on all platforms. And contrary,
>>>> functions that are compiled on all platforms, and were exported in mapfiles,
>>>> but now have gotten an JNIEXPORT decoration, will now be visible even on
>>>> Windows. (This accounts for half of the noticed symbol differences on
>>>> Windows.) I could have made the JNIEXPORT conditional on OS, but I didn't
>>>> think the mess in source code were worth the keeping of binary confidence
>>>> with the old build.
>>>>
>>>> A third common source for change in symbols is due to exported functions
>>>> "leaking" across library borders. For instance, some functions in
>>>> java.desktop is compiled in both libawt_xawt and libawt_headless, but they
>>>> were previously only included in the mapfile for one of these libraries.
>>>> Now, since the visibility is determined by the source code itself, it gets
>>>> exported in both libraries. A variant of this is when a library depends on
>>>> another JDK library, and includes the header file from that other library,
>>>> which in turn declares a function as JNIEXPORT. This will cause the
>>>> including library to also export the function. This accounts for the other
>>>> half of the changes on Windows. A typical example of this is that multiple
>>>> libraries now re-export hotspot symbols from libjvm.so, like jio_fprintf. (I
>>>> have not listed the libjvm re-exports below.)
>>>>
>>>> Note that  Java_java_io_FileOutputStream_close0 in
>>>> java.base/unix/native/libjava/FileOutputStream_md.c is no longer exported,
>>>> and can probably be removed.
>>>>
>>>> Here is a detailed table showing and accounting for all the remaining
>>>> differences found on Linux and Solaris:
>>>> java.base/unix/native/libjava: Java_java_io_FileOutputStream_close0 is
>>>> now also exported on unix platforms due to JNIEXPORT.
>>>>
>>>> java.base/jspawnlauncher: On solaris, we also include
>>>> libjava/childproc.o, which
>>>> now exports less functions than it used to (it used to export all
>>>> functions, now it is compiled with visibility=hidden).
>>>>
>>>> java.base/java(w).exe: Is now also exporting the following symbols due to
>>>> added JNIEXPORT in libjli on Windows:
>>>> (Yes, executables can export symbols on Windows. Confusing, I know.)
>>>>   JLI_AddArgsFromEnvVar
>>>>   JLI_CmdToArgs
>>>>   JLI_GetAppArgIndex
>>>>   JLI_GetStdArgc
>>>>   JLI_GetStdArgs
>>>>   JLI_InitArgProcessing
>>>>   JLI_Launch
>>>>   JLI_List_add
>>>>   JLI_List_new
>>>>   JLI_ManifestIterate
>>>>   JLI_MemAlloc
>>>>   JLI_MemFree
>>>>   JLI_PreprocessArg
>>>>   JLI_ReportErrorMessage
>>>>   JLI_ReportErrorMessageSys
>>>>   JLI_ReportExceptionDescription
>>>>   JLI_ReportMessage
>>>>   JLI_SetTraceLauncher
>>>>   JLI_StringDup
>>>>
>>>> java.desktop:/libawt_xawt: The following symbols are now also exported on
>>>> linux and solaris due to JNIEXPORT:
>>>>   awt_DrawingSurface_FreeDrawingSurfaceInfo
>>>>   awt_DrawingSurface_GetDrawingSurfaceInfo
>>>>   awt_DrawingSurface_Lock
>>>>   awt_DrawingSurface_Unlock
>>>>   awt_GetColor
>>>>
>>>> The following symbols are now also exported on linux and solaris due to
>>>> JNIEXPORT (they were previously
>>>>   exported only in libawt):
>>>>   Java_sun_awt_DebugSettings_setCTracingOn__Z
>>>>   Java_sun_awt_DebugSettings_setCTracingOn__ZLjava_lang_String_2
>>>>   Java_sun_awt_DebugSettings_setCTracingOn__ZLjava_lang_String_2I
>>>>   Java_sun_awt_X11GraphicsConfig_getNumColors
>>>>
>>>> java.desktop:/libawt_headless: The following symbols are now also
>>>> exported due to JNIEXPORT (they were previously
>>>>   exported only in libawt_xawt and/or libawt):
>>>>   Java_sun_java2d_opengl_GLXGraphicsConfig_getGLXConfigInfo
>>>>   Java_sun_java2d_opengl_GLXGraphicsConfig_getOGLCapabilities
>>>>   Java_sun_java2d_x11_X11PMBlitLoops_updateBitmask
>>>>   Java_sun_java2d_x11_X11SurfaceData_isShmPMAvailable
>>>>   X11SurfaceData_GetOps
>>>>
>>>> java.desktop/libawt: The following symbols are now also exported on
>>>> Windows, due to added
>>>> JNIEXPORT:
>>>>   SurfaceData_InitOps
>>>>   mul8table
>>>>   div8table
>>>>   doDrawPath
>>>>   doFillPath
>>>>   g_CMpDataID
>>>>   initInverseGrayLut
>>>>   make_dither_arrays
>>>>   make_uns_ordered_dither_array
>>>>   path2DFloatCoordsID
>>>>   path2DNumTypesID
>>>>   path2DTypesID
>>>>   path2DWindingRuleID
>>>>   sg2dStrokeHintID
>>>>   std_img_oda_blue
>>>>   std_img_oda_green
>>>>   std_img_oda_red
>>>>   std_odas_computed
>>>>   sunHints_INTVAL_STROKE_PURE
>>>>
>>>> java.desktop/libawt on solaris:
>>>> A number of "#pragma weak" directives was previously overridden by the
>>>> mapfile.
>>>> Now these directives are respected, so these symbols are now weak instead
>>>> of local:
>>>>   ByteGrayToIntArgbPreConvert_F
>>>>   ByteGrayToIntArgbPreScaleConvert_F
>>>>   IntArgbBmToFourByteAbgrPreScaleXparOver_F
>>>>   IntArgbToIntRgbXorBlit_F
>>>>   IntBgrToIntBgrAlphaMaskBlit_F
>>>>
>>>> java.desktop/libawt on solaris: These are now also exported due to
>>>> JNIEXPORT in libmlib_image.
>>>>   j2d_mlib_ImageCreate
>>>>   j2d_mlib_ImageCreateStruct
>>>>   j2d_mlib_ImageDelete
>>>>
>>>> java.desktop/libawt on solaris: This is now also exported due to
>>>> JNIEXPORT:
>>>>   GrPrim_CompGetXorColor
>>>>   SurfaceData_GetOpsNoSetup
>>>>   SurfaceData_IntersectBoundsXYWH
>>>>   SurfaceData_SetOps
>>>>   Transform_GetInfo
>>>>   Transform_transform
>>>>
>>>> java.desktop/libsplashscreen: JNI_OnLoad is now exported on linux and
>>>> solaris due to JNIEXPORT.
>>>> libspashscreen also had JNIEXPORT (actually a pure _declspec(dllexport))
>>>> but no JNICALL, which I added as
>>>> a part of converting to JNIEXPORT. The same goes for libmlib_image .
>>>>
>>>> jdk.sctp/libsctp: handleSocketError is now exported on linux and solaris
>>>> due to JNIEXPORT in libnio.
>>>>
>>>> java.instrument:/libinstrument: Agent_OnUnload is now also exported on
>>>> linux and solaris platforms due to JNIEXPORT.
>>>> JLI_ManifestIterate is now also exported on Windows, due to added
>>>> JNIEXPORT in libjli.
>>>>
>>>> jdk.management/libmanagement_ext:
>>>> Java_com_sun_management_internal_Flag_setDoubleValue is now also exported on
>>>> linux and solaris platforms due to JNIEXPORT.
>>>>
>>>> /Magnus
>>>>
>>>>