Hi Matthias, Magnus,
The problem is with JNICALL added to the functions. JNICALL expands to
__stdcall [1] on Windows. On 64 bit, the modifier has no effect and is
ignored. On 32 bit, the modifier changes the way parameters are pass and
the function name is decorated with an underscore and with @ + the size
of arguments.
If I remove JNICALL modifier from the exported functions, they're
exported with plain name as in source code (plus, __cdecl [2] calling
convention is used.)
zip.dll and jimage.dll are affected by this. It's because the exported
functions are looked up by name rather than using a header file with
JNIIMPORT. See
http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/classLoader.cpp#l1155
http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/classLoader.cpp#l1194
JNICALL modifier must also be removed from type declarations for
functions from zip.dll:
http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/classLoader.cpp#l81
I'm attaching the patch to zip and jimage as well as classLoader.cpp
which takes these changes into account. It does not replace Matthias'
patch but complements it.
Alternatively, if keeping JNICALL / __stdcall, it's possible to use
pragma comments [3][4] to export undecorated names. But this is compiler
specific and may require if's for other platforms.
Regards,
Alexey
[1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx
[2] https://msdn.microsoft.com/en-us/library/zkwh89ks.aspx
[3] https://docs.microsoft.com/en-ie/cpp/build/reference/exports
[4] https://docs.microsoft.com/en-ie/cpp/preprocessor/comment-c-cpp
On 09/04/2018 12:42, Magnus Ihse Bursie wrote:
Those were added by my patch that removed the map files.
/Magnus
9 apr. 2018 kl. 13:38 skrev Baesken, Matthias <matthias.baes...@sap.com>:
I did not add JNICALL decorations to any libzip functions , please see my
webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/
, the problem here
is the added JNICALL decoration, which affects only win32 (and incorrectly,
that is).
so I wonder which added JNICALL decoration you are refering to .
Best regards, Matthias
-----Original Message-----
From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com]
Sent: Montag, 9. April 2018 13:29
To: Baesken, Matthias <matthias.baes...@sap.com>
Cc: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev <build-
d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com>
Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in function
declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
some places in function declarations/implementations
Reinstating the -export command line options is not the way forward here.
As I understood it from private conversation with Alexey, the problem here
is the added JNICALL decoration, which affects only win32 (and incorrectly,
that is).
/Magnus
9 apr. 2018 kl. 12:46 skrev Baesken, Matthias <matthias.baes...@sap.com>:
I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as
this function is correctly decorated in CRC32.c and is exported.
Hi Alexey, you are correct on this one . The added declaration does not
help with the "Corrupted ZIP library" error .
This one can be fixed by bringing back the exports in the makefile
make/lib/CoreLibraries.gmk (same for some JIMAGE functions) :
--- a/make/lib/CoreLibraries.gmk Sun Apr 08 17:01:20 2018 +0800
+++ b/make/lib/CoreLibraries.gmk Mon Apr 09 12:44:08 2018 +0200
@@ -191,6 +191,9 @@
DISABLED_WARNINGS_gcc := implicit-fallthrough, \
LDFLAGS := $(LDFLAGS_JDKLIB) \
$(call SET_SHARED_LIBRARY_ORIGIN), \
+ LDFLAGS_windows := -export:ZIP_Open -export:ZIP_Close -
export:ZIP_FindEntry \
+ -export:ZIP_ReadEntry -export:ZIP_GetNextEntry \
+ -export:ZIP_InflateFully -export:ZIP_CRC32 -export:ZIP_FreeEntry, \
LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \
LIBS_windows := jvm.lib $(WIN_JAVA_LIB), \
))
@@ -221,6 +224,10 @@
CFLAGS_unix := -UDEBUG, \
LDFLAGS := $(LDFLAGS_JDKLIB) $(LDFLAGS_CXX_JDK) \
$(call SET_SHARED_LIBRARY_ORIGIN), \
+ LDFLAGS_windows := -export:JIMAGE_Open -export:JIMAGE_Close \
+ -export:JIMAGE_PackageToModule \
+ -export:JIMAGE_FindResource -export:JIMAGE_GetResource \
+ -export:JIMAGE_ResourceIterator -export:JIMAGE_ResourcePath, \
LIBS_unix := -ljvm -ldl $(LIBCXX), \
LIBS_macosx := -lc++, \
LIBS_windows := jvm.lib, \
I wonder why the 64bit windows build can live without the exports , any
ideas ?
Best regards , Matthias
-----Original Message-----
From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
Sent: Samstag, 7. April 2018 00:14
To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken,
Matthias <matthias.baes...@sap.com>
Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin
<martin.do...@sap.com>
Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in
function
declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
some places in function declarations/implementations
Hi Magnus, Matthias,
I tried to build 32 bit Windows but it fails to run for me with
Corrupted ZIP library:
c:\work\jdk-dev\build\windows-x86-normal-server-
release\jdk\bin\zip.dll
The problem is that zip.dll exports all symbols as C++ rather than C.
For example, ZIP_CRC32 is exported as _ZIP_CRC32@12, and
classLoader.cpp
cannot find the function.
I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as
this function is correctly decorated in CRC32.c and is exported.
Regards,
Alexey
On 06/04/2018 17:33, Magnus Ihse Bursie wrote:
I think it's reasonable to update the existing webrev.
/Magnus
6 apr. 2018 kl. 15:20 skrev Baesken, Matthias
<matthias.baes...@sap.com>:
Hello, I just noticed 2 additonal issues regarding mapfile-removal :
The follow up change that removed mapfiles for exes as well leads
on Win32 bit to this link error :
Creating library
C:/JVM/jdk_jdk_ntintel/support/native/java.base/java/java.lib and
object
C:/JVM/jdk_jdk_ntintel/support/native/java.base/java/java.exp
LIBCMT.lib(crt0.obj) : error LNK2019: unresolved external symbol _main
referenced in function ___tmainCRTStartup
C:/JVM/jdk_jdk_ntintel/support/native/java.base/java_objs/java.exe
:
fatal error LNK1120: 1 unresolved externals
Looks like we cannot have JNICALL in main.c :
diff -r 4f6887eade94 src/java.base/share/native/launcher/main.c
--- a/src/java.base/share/native/launcher/main.c Thu Apr 05
14:39:04
2018 -0700
+++ b/src/java.base/share/native/launcher/main.c Fri Apr 06
15:16:40
2018 +0200
@@ -93,7 +93,7 @@
__initenv = _environ;
#else /* JAVAW */
-JNIEXPORT int JNICALL
+JNIEXPORT int
main(int argc, char **argv)
{
int margc;
Zip-library has runtime issues when symbols are resolved in
src/hotspot/share/classfile/classLoader.cpp.
I added the declaration of the missing symbol, this seems to fix it .
diff -r 4f6887eade94 src/java.base/share/native/libzip/zip_util.h
--- a/src/java.base/share/native/libzip/zip_util.h Thu Apr 05 14:39:04
2018 -0700
+++ b/src/java.base/share/native/libzip/zip_util.h Fri Apr 06 15:16:40
2018 +0200
@@ -284,4 +284,7 @@
JNIEXPORT jboolean JNICALL
ZIP_InflateFully(void *inBuf, jlong inLen, void *outBuf, jlong outLen,
char
**pmsg);
+JNIEXPORT jint JNICALL
+ZIP_CRC32(jint crc, const jbyte *buf, jint len);
+
Should I prepare another bug, or add this to the existing one and
post
a second webrev ?
Best regards, Matthias
From: Baesken, Matthias
Sent: Freitag, 6. April 2018 09:54
To: 'Magnus Ihse Bursie' <magnus.ihse.bur...@oracle.com>
Cc: build-dev@openjdk.java.net; Doerr, Martin
<martin.do...@sap.com>
Subject: RFR: 8201226 missing JNIEXPORT / JNICALL at some places in
function declarations/implementations - was : RE: missing JNIEXPORT /
JNICALL at some places in function declarations/implementations
Hello, please review :
Bug :
https://bugs.openjdk.java.net/browse/JDK-8201226
change :
http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/
mostly I added JNIEXPORT / JNICALL at some places where the
win32bit
build missed it .
A difference is
src/java.desktop/share/native/libsplashscreen/splashscreen_impl.h
Where I removed a few declarations (one decl with one without
JNIEXPORT / JNICALL – looked a bit strange ) .
Best regards , Matthias
From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com]
Sent: Donnerstag, 5. April 2018 17:45
To: Baesken, Matthias <matthias.baes...@sap.com>
Cc: build-dev@openjdk.java.net; Doerr, Martin
<martin.do...@sap.com>
Subject: Re: missing JNIEXPORT / JNICALL at some places in function
declarations/implementations
That's most likely a result of the new JNIEXPORT I added as part of the
mapfile removal.
I tried to match header file and C file, but I can certainly have missed
cases. If I didn't get any warnings, it was hard to know what I missed.
Please do submit your patch.
I'm a bit surprised 32-bit Window is still buildable. :)
/Magnus
5 apr. 2018 kl. 17:20 skrev Baesken, Matthias
<matthias.baes...@sap.com>:
Hello, we noticed that at a number of places in the coding , the
JNIEXPORT and/or JNICALL modifiers do not match when one compares
the declaration and
The implementation of functions.
While this is ok on most platforms, it seems to fail on Windows 32 bit
and
leads to errors like this one :
e:/priv/openjdk/repos/jdk/src/java.desktop/share/native/libmlib_image/ml
ib_ImageConvKernelConvert.c(87) : error C2373:
'j2d_mlib_ImageConvKernelConvert' : redefinition; different type
modifiers
e:\priv\openjdk\repos\jdk\src\java.desktop\share\native\libmlib_image\ml
ib_image_proto.h(2630) : see declaration of
'j2d_mlib_ImageConvKernelConvert'
(there are quite a few of these e.g. in mlib / splashscreen etc.)
Another example is this one :
diff -r 4d98473ed33e src/java.base/share/native/libjimage/jimage.hpp
--- a/src/java.base/share/native/libjimage/jimage.hpp Thu Apr 05
09:55:16 2018 +0200
+++ b/src/java.base/share/native/libjimage/jimage.hpp Thu Apr
05 17:07:40 2018 +0200
@@ -126,7 +126,7 @@
* JImageLocationRef location = (*JImageFindResource)(image,
* "java.base", "9.0", "java/lang/String.class",
&size);
*/
-extern "C" JNIEXPORT JImageLocationRef
JIMAGE_FindResource(JImageFile* jimage,
+extern "C" JNIEXPORT JImageLocationRef JNICALL
JIMAGE_FindResource(JImageFile* jimage,
const char* module_name, const char* version, const char* name,
jlong* size);
Is there some generic way to get the same declarations /
impementations in the code ?
Or should I just add a patch with my findings ?
Best regards, Matthias
diff -r b9df14155468 src/hotspot/share/classfile/classLoader.cpp
--- a/src/hotspot/share/classfile/classLoader.cpp Thu Apr 05 19:08:48
2018 -0700
+++ b/src/hotspot/share/classfile/classLoader.cpp Mon Apr 09 14:36:33
2018 +0100
@@ -80,13 +80,13 @@
// Entry points in zip.dll for loading zip/jar file entries
-typedef void * * (JNICALL *ZipOpen_t)(const char *name, char **pmsg);
-typedef void (JNICALL *ZipClose_t)(jzfile *zip);
-typedef jzentry* (JNICALL *FindEntry_t)(jzfile *zip, const char *name, jint
*sizeP, jint *nameLen);
-typedef jboolean (JNICALL *ReadEntry_t)(jzfile *zip, jzentry *entry, unsigned
char *buf, char *namebuf);
-typedef jzentry* (JNICALL *GetNextEntry_t)(jzfile *zip, jint n);
-typedef jboolean (JNICALL *ZipInflateFully_t)(void *inBuf, jlong inLen, void
*outBuf, jlong outLen, char **pmsg);
-typedef jint (JNICALL *Crc32_t)(jint crc, const jbyte *buf, jint len);
+typedef void * * (*ZipOpen_t)(const char *name, char **pmsg);
+typedef void (*ZipClose_t)(jzfile *zip);
+typedef jzentry* (*FindEntry_t)(jzfile *zip, const char *name, jint *sizeP,
jint *nameLen);
+typedef jboolean (*ReadEntry_t)(jzfile *zip, jzentry *entry, unsigned char
*buf, char *namebuf);
+typedef jzentry* (*GetNextEntry_t)(jzfile *zip, jint n);
+typedef jboolean (*ZipInflateFully_t)(void *inBuf, jlong inLen, void *outBuf,
jlong outLen, char **pmsg);
+typedef jint (*Crc32_t)(jint crc, const jbyte *buf, jint len);
static ZipOpen_t ZipOpen = NULL;
static ZipClose_t ZipClose = NULL;
diff -r b9df14155468 src/java.base/share/native/libjimage/jimage.cpp
--- a/src/java.base/share/native/libjimage/jimage.cpp Thu Apr 05 19:08:48
2018 -0700
+++ b/src/java.base/share/native/libjimage/jimage.cpp Mon Apr 09 14:36:33
2018 +0100
@@ -55,7 +55,7 @@
* }
* ...
*/
-extern "C" JNIEXPORT JImageFile* JNICALL
+extern "C" JNIEXPORT JImageFile*
JIMAGE_Open(const char *name, jint* error) {
// TODO - return a meaningful error code
*error = 0;
@@ -72,7 +72,7 @@
* Ex.
* (*JImageClose)(image);
*/
-extern "C" JNIEXPORT void JNICALL
+extern "C" JNIEXPORT void
JIMAGE_Close(JImageFile* image) {
ImageFileReader::close((ImageFileReader*) image);
}
@@ -89,7 +89,7 @@
* tty->print_cr(package);
* -> java.base
*/
-extern "C" JNIEXPORT const char* JNICALL
+extern "C" JNIEXPORT const char*
JIMAGE_PackageToModule(JImageFile* image, const char* package_name) {
return ((ImageFileReader*)
image)->get_image_module_data()->package_to_module(package_name);
}
@@ -108,7 +108,7 @@
* JImageLocationRef location = (*JImageFindResource)(image,
* "java.base", "9.0",
"java/lang/String.class", &size);
*/
-extern "C" JNIEXPORT JImageLocationRef JNICALL
+extern "C" JNIEXPORT JImageLocationRef
JIMAGE_FindResource(JImageFile* image,
const char* module_name, const char* version, const char* name,
jlong* size) {
@@ -155,7 +155,7 @@
* char* buffer = new char[size];
* (*JImageGetResource)(image, location, buffer, size);
*/
-extern "C" JNIEXPORT jlong JNICALL
+extern "C" JNIEXPORT jlong
JIMAGE_GetResource(JImageFile* image, JImageLocationRef location,
char* buffer, jlong size) {
((ImageFileReader*) image)->get_resource((u4) location, (u1*) buffer);
@@ -184,7 +184,7 @@
* }
* (*JImageResourceIterator)(image, ctw_visitor, loader);
*/
-extern "C" JNIEXPORT void JNICALL
+extern "C" JNIEXPORT void
JIMAGE_ResourceIterator(JImageFile* image,
JImageResourceVisitor_t visitor, void* arg) {
ImageFileReader* imageFile = (ImageFileReader*) image;
@@ -226,7 +226,7 @@
* char path[JIMAGE_MAX_PATH];
* (*JImageResourcePath)(image, location, path, JIMAGE_MAX_PATH);
*/
-extern "C" JNIEXPORT bool JNICALL
+extern "C" JNIEXPORT bool
JIMAGE_ResourcePath(JImageFile* image, JImageLocationRef locationRef,
char* path, size_t max) {
ImageFileReader* imageFile = (ImageFileReader*) image;
diff -r b9df14155468 src/java.base/share/native/libjimage/jimage.hpp
--- a/src/java.base/share/native/libjimage/jimage.hpp Thu Apr 05 19:08:48
2018 -0700
+++ b/src/java.base/share/native/libjimage/jimage.hpp Mon Apr 09 14:36:33
2018 +0100
@@ -72,7 +72,7 @@
* ...
*/
-extern "C" JNIEXPORT JImageFile* JNICALL
+extern "C" JNIEXPORT JImageFile*
JIMAGE_Open(const char *name, jint* error);
typedef JImageFile* (*JImageOpen_t)(const char *name, jint* error);
@@ -87,7 +87,7 @@
* (*JImageClose)(image);
*/
-extern "C" JNIEXPORT void JNICALL
+extern "C" JNIEXPORT void
JIMAGE_Close(JImageFile* jimage);
typedef void (*JImageClose_t)(JImageFile* jimage);
@@ -106,7 +106,7 @@
* -> java.base
*/
-extern "C" JNIEXPORT const char * JNICALL
+extern "C" JNIEXPORT const char *
JIMAGE_PackageToModule(JImageFile* jimage, const char* package_name);
typedef const char* (*JImagePackageToModule_t)(JImageFile* jimage, const char*
package_name);
@@ -150,7 +150,7 @@
* char* buffer = new char[size];
* (*JImageGetResource)(image, location, buffer, size);
*/
-extern "C" JNIEXPORT jlong JNICALL
+extern "C" JNIEXPORT jlong
JIMAGE_GetResource(JImageFile* jimage, JImageLocationRef location,
char* buffer, jlong size);
@@ -185,7 +185,7 @@
const char* module_name, const char* version, const char* package,
const char* name, const char* extension, void* arg);
-extern "C" JNIEXPORT void JNICALL
+extern "C" JNIEXPORT void
JIMAGE_ResourceIterator(JImageFile* jimage,
JImageResourceVisitor_t visitor, void *arg);
@@ -202,7 +202,7 @@
* char path[JIMAGE_MAX_PATH];
* (*JImageResourcePath)(image, location, path, JIMAGE_MAX_PATH);
*/
-extern "C" JNIEXPORT bool JNICALL
+extern "C" JNIEXPORT bool
JIMAGE_ResourcePath(JImageFile* image, JImageLocationRef locationRef,
char* path, size_t max);
diff -r b9df14155468 src/java.base/share/native/libzip/CRC32.c
--- a/src/java.base/share/native/libzip/CRC32.c Thu Apr 05 19:08:48 2018 -0700
+++ b/src/java.base/share/native/libzip/CRC32.c Mon Apr 09 14:36:33 2018 +0100
@@ -54,7 +54,7 @@
return crc;
}
-JNIEXPORT jint JNICALL
+JNIEXPORT jint
ZIP_CRC32(jint crc, const jbyte *buf, jint len)
{
return crc32(crc, (Bytef*)buf, len);
diff -r b9df14155468 src/java.base/share/native/libzip/zip_util.c
--- a/src/java.base/share/native/libzip/zip_util.c Thu Apr 05 19:08:48
2018 -0700
+++ b/src/java.base/share/native/libzip/zip_util.c Mon Apr 09 14:36:33
2018 +0100
@@ -881,7 +881,7 @@
* set to the error message text if msg != 0. Otherwise, *msg will be
* set to NULL. Caller doesn't need to free the error message.
*/
-JNIEXPORT jzfile * JNICALL
+JNIEXPORT jzfile *
ZIP_Open(const char *name, char **pmsg)
{
jzfile *file = ZIP_Open_Generic(name, pmsg, O_RDONLY, 0);
@@ -895,7 +895,7 @@
/*
* Closes the specified zip file object.
*/
-JNIEXPORT void JNICALL
+JNIEXPORT void
ZIP_Close(jzfile *zip)
{
MLOCK(zfiles_lock);
@@ -1115,7 +1115,7 @@
* Returns the zip entry corresponding to the specified name, or
* NULL if not found.
*/
-JNIEXPORT jzentry * JNICALL
+JNIEXPORT jzentry *
ZIP_GetEntry(jzfile *zip, char *name, jint ulen)
{
if (ulen == 0) {
@@ -1238,7 +1238,7 @@
* Returns the n'th (starting at zero) zip file entry, or NULL if the
* specified index was out of range.
*/
-JNIEXPORT jzentry * JNICALL
+JNIEXPORT jzentry *
ZIP_GetNextEntry(jzfile *zip, jint n)
{
jzentry *result;
@@ -1439,7 +1439,7 @@
* The current implementation does not support reading an entry that
* has the size bigger than 2**32 bytes in ONE invocation.
*/
-JNIEXPORT jzentry * JNICALL
+JNIEXPORT jzentry *
ZIP_FindEntry(jzfile *zip, char *name, jint *sizeP, jint *nameLenP)
{
jzentry *entry = ZIP_GetEntry(zip, name, 0);
@@ -1456,7 +1456,7 @@
* Note: this is called from the separately delivered VM (hotspot/classic)
* so we have to be careful to maintain the expected behaviour.
*/
-JNIEXPORT jboolean JNICALL
+JNIEXPORT jboolean
ZIP_ReadEntry(jzfile *zip, jzentry *entry, unsigned char *buf, char *entryname)
{
char *msg;
@@ -1515,7 +1515,7 @@
return JNI_TRUE;
}
-JNIEXPORT jboolean JNICALL
+JNIEXPORT jboolean
ZIP_InflateFully(void *inBuf, jlong inLen, void *outBuf, jlong outLen, char
**pmsg)
{
z_stream strm;
diff -r b9df14155468 src/java.base/share/native/libzip/zip_util.h
--- a/src/java.base/share/native/libzip/zip_util.h Thu Apr 05 19:08:48
2018 -0700
+++ b/src/java.base/share/native/libzip/zip_util.h Mon Apr 09 14:36:33
2018 +0100
@@ -241,16 +241,16 @@
*/
#define ZIP_ENDCHAIN ((jint)-1)
-JNIEXPORT jzentry * JNICALL
+JNIEXPORT jzentry *
ZIP_FindEntry(jzfile *zip, char *name, jint *sizeP, jint *nameLenP);
-JNIEXPORT jboolean JNICALL
+JNIEXPORT jboolean
ZIP_ReadEntry(jzfile *zip, jzentry *entry, unsigned char *buf, char *entrynm);
-JNIEXPORT jzentry * JNICALL
+JNIEXPORT jzentry *
ZIP_GetNextEntry(jzfile *zip, jint n);
-JNIEXPORT jzfile * JNICALL
+JNIEXPORT jzfile *
ZIP_Open(const char *name, char **pmsg);
jzfile *
@@ -265,10 +265,10 @@
jzfile *
ZIP_Put_In_Cache0(const char *name, ZFILE zfd, char **pmsg, jlong
lastModified, jboolean usemmap);
-JNIEXPORT void JNICALL
+JNIEXPORT void
ZIP_Close(jzfile *zip);
-JNIEXPORT jzentry * JNICALL
+JNIEXPORT jzentry *
ZIP_GetEntry(jzfile *zip, char *name, jint ulen);
JNIEXPORT void JNICALL
ZIP_Lock(jzfile *zip);
@@ -281,7 +281,7 @@
jlong ZIP_GetEntryDataOffset(jzfile *zip, jzentry *entry);
jzentry * ZIP_GetEntry2(jzfile *zip, char *name, jint ulen, jboolean addSlash);
-JNIEXPORT jboolean JNICALL
+JNIEXPORT jboolean
ZIP_InflateFully(void *inBuf, jlong inLen, void *outBuf, jlong outLen, char
**pmsg);
#endif /* !_ZIP_H_ */
