RFR: JDK-8200178 Remove mapfiles for JDK native libraries (original) (raw)

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Fri Mar 23 21:08:35 UTC 2018

On 2018-03-23 18:33, Phil Race wrote:

There are a lot of changes in the desktop libraries. Well, yes and no. While there are multiple touched files, the resulting native shared libraries that are built have very minimal changes in them. (That's the view point from the build guy, you know :))

Doing mach5 tier1/2/3 testing is not nearly sufficient to cover those since only tier3 has any UI tests and it barely uses anything that's touched here. So since testing seems to be wise, then I think you should do a jtreg desktop group run on Linux & Windows. There is next to no difference for java.desktop on Windows. The only, very subtle, difference, is that awt.dll now exports 18 more functions (totalling 800, instead of 782). I can't even begin to imagine how anything could fail due to this additional exporting. Not even the disassembly of the machine code of awt.dll is different from before, not a single byte. So I don't buy it that I need to do extensive client testing on Windows.

You can probably skip Mac since it is unaffected and I think Linux will cover Solaris here. You should also do some headless testing. I agree that it seems prudent to do some Linux/Solaris testing, since changes there are more wide spread. Could you please point me to some guidance on how to run these tests? (You can do it off list) It could take some time to review this properly and decide what changes are OK, what changes are something we should clean up later, and what changes are something that ought to be addressed now .. As I said, I am going to file follow-up bugs for suspicious handling of exported symbols. These follow-up bugs will be separated per component team, unlike this fix, which by necessity addresses all JDK libraries at once. So you will get plenty of time to consider ways of cleaning up any exports handling that you do not like. It would be a pity if this entire checkin was delayed since the client team could not accept the changes needed in client libraries. :-(

And frankly, I believe the java.desktop libs needs some serious refactoring to get to grip with the exported symbols situation. The major cause of problems is, I believe, rooted in a non-optimal split of functionality between libawt, libawt_xawt and libawt_headless. This is not likely something that can be addressed in this change.

I think I'd be mainly concerned that something fails due to a missing symbol, or that for newly exported symbols if we ended up with duplicate symbols as a result.

Once again, I've run the COMPARE_BUILD script on this patch. Let me explain a bit more in detail what it does, since that might be known only to us in the build team. This script analyses the build result, the jmods, the lib*.so files, etc. The basic idea here is that a change in the build system, which does not produce a change in the build result, is "transparent" to the product. There is e.g. no reason to run any further testing, since we're in effect testing the same bits. For many changes in the build system, we hold this as the gold standard.

For this particular change, to achieve this kind of fidelity would have come with a too high price in code complexity, so I have allowed certain small deviations. These are really minimal, and should in most cases be undetectable by the product. The changes in Linux and Solaris that have occured, is those that I listed in my review mail. Basically, for some libraries, additional symbols are exported. I could fix this, but only at the expense of more complex code. While it's a good thing to minimize the functions exported, a handful extra symbols is not a disaster. (We have more important issues to address in our native libraries.)

For the AWT libraries, most of the duplicates are coming from the source code that are shared between libraries, in java.desktop/share/native/common. This means that the same function is compiled into -- and now also exported from -- multiple libraries. This is not a big deal. Even if we were to link with two libraries defining the same symbol, the dynamic linker will arbitrarily chose one of them, but since they are identical, it does not matter. (It's another thing if they implement different functions, as you noted yourself in the bugs about linking with awt_xawt vs doing a runtime linking to awt_headless).

Also, I guarantee you that in no way are there missing symbols in the refactored build. I've checked, double-checked and triple-checked that.

The results of a test run will add confidence here.

BTW I don't think you are right that java.desktop:/libawtheadless: The following symbols are now also exported due to JNIEXPORT (they were previously ..  X11SurfaceDataGetOps It looks to me like it was previously exported. You are correct, it was previously exported in libawt_headless. I meant that it is now also exported for libawt_xawt due to the JNIEXPORT. Sorry for mixing this up.

/Magnus

-phil.

On 03/23/2018 06:56 AM, 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 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 $(EXPORTALLSYMBOLS) to these libraries. Hopefully, we can remove this flag and fix proper exported symbols in the future. I have run the complete build using COMPAREBUILD, 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 bssstart, 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 SUNWprivate1.1 is not included, and thus neither the .gnu.versiond 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 dofoo() { ... }", but dofoo 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, dofoo() 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 libawtxawt and libawtheadless, 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 jiofprintf. (I have not listed the libjvm re-exports below.) Note that  JavajavaioFileOutputStreamclose0 in java.base/unix/native/libjava/FileOutputStreammd.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: JavajavaioFileOutputStreamclose0 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.)  JLIAddArgsFromEnvVar  JLICmdToArgs  JLIGetAppArgIndex  JLIGetStdArgc  JLIGetStdArgs  JLIInitArgProcessing  JLILaunch  JLIListadd  JLIListnew  JLIManifestIterate  JLIMemAlloc  JLIMemFree  JLIPreprocessArg  JLIReportErrorMessage  JLIReportErrorMessageSys  JLIReportExceptionDescription  JLIReportMessage  JLISetTraceLauncher  JLIStringDup java.desktop:/libawtxawt: The following symbols are now also exported on linux and solaris due to JNIEXPORT:  awtDrawingSurfaceFreeDrawingSurfaceInfo  awtDrawingSurfaceGetDrawingSurfaceInfo  awtDrawingSurfaceLock  awtDrawingSurfaceUnlock  awtGetColor The following symbols are now also exported on linux and solaris due to JNIEXPORT (they were previously  exported only in libawt):  JavasunawtDebugSettingssetCTracingOn_Z  JavasunawtDebugSettingssetCTracingOn_ZLjavalangString2  JavasunawtDebugSettingssetCTracingOn_ZLjavalangString2I  JavasunawtX11GraphicsConfiggetNumColors java.desktop:/libawtheadless: The following symbols are now also exported due to JNIEXPORT (they were previously  exported only in libawtxawt and/or libawt):  Javasunjava2dopenglGLXGraphicsConfiggetGLXConfigInfo  Javasunjava2dopenglGLXGraphicsConfiggetOGLCapabilities  Javasunjava2dx11X11PMBlitLoopsupdateBitmask  Javasunjava2dx11X11SurfaceDataisShmPMAvailable  X11SurfaceDataGetOps java.desktop/libawt: The following symbols are now also exported on Windows, due to added JNIEXPORT:  SurfaceDataInitOps  mul8table  div8table  doDrawPath  doFillPath  gCMpDataID  initInverseGrayLut  makeditherarrays  makeunsorderedditherarray  path2DFloatCoordsID  path2DNumTypesID  path2DTypesID  path2DWindingRuleID  sg2dStrokeHintID  stdimgodablue  stdimgodagreen  stdimgodared  stdodascomputed  sunHintsINTVALSTROKEPURE 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:  ByteGrayToIntArgbPreConvertF  ByteGrayToIntArgbPreScaleConvertF  IntArgbBmToFourByteAbgrPreScaleXparOverF  IntArgbToIntRgbXorBlitF  IntBgrToIntBgrAlphaMaskBlitF java.desktop/libawt on solaris: These are now also exported due to JNIEXPORT in libmlibimage.  j2dmlibImageCreate  j2dmlibImageCreateStruct  j2dmlibImageDelete java.desktop/libawt on solaris: This is now also exported due to JNIEXPORT:  GrPrimCompGetXorColor  SurfaceDataGetOpsNoSetup  SurfaceDataIntersectBoundsXYWH  SurfaceDataSetOps  TransformGetInfo  Transformtransform java.desktop/libsplashscreen: JNIOnLoad 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 libmlibimage . jdk.sctp/libsctp: handleSocketError is now exported on linux and solaris due to JNIEXPORT in libnio. java.instrument:/libinstrument: AgentOnUnload is now also exported on linux and solaris platforms due to JNIEXPORT. JLIManifestIterate is now also exported on Windows, due to added JNIEXPORT in libjli. jdk.management/libmanagementext: JavacomsunmanagementinternalFlagsetDoubleValue is now also exported on linux and solaris platforms due to JNIEXPORT. /Magnus

More information about the serviceability-dev mailing list