Hi Martin,

I can see that both places already include headers from 
src/java.base/share/native/libjava/. I suggest adding the define in jni_util.h. 
WindowsPreferences.c already includes it.

Best regards
Christoph

From: Doerr, Martin <martin.do...@sap.com>
Sent: Donnerstag, 5. Dezember 2019 12:00
To: Thomas Stüfe <thomas.stu...@gmail.com>; Langer, Christoph 
<christoph.lan...@sap.com>
Cc: core-libs-dev@openjdk.java.net; security-dev 
<security-...@openjdk.java.net>; Lindenmaier, Goetz <goetz.lindenma...@sap.com>
Subject: RE: RFR(S): 8220348: [ntintel] asserts about copying unalinged array

Hi Thomas and Christoph,

thanks for the reviews.

Other code in java.security.jgss also uses these #defined checks:
src/java.security.jgss/share/native/libj2gss/gssapi.h:#if defined (_WIN32) && 
defined (_MSC_VER)

I’d like to have it consistent with that.

@Christoph: I think having ATTRIBUTE_ALIGNED(x) would be nice. It could get 
defined easily for Visual Studio and GCC, but some other compilers may be more 
difficult. Note that this macro is only defined for a selected set of compilers 
in hotspot. If we wanted to add it, where should we define it?

Windows 32 bit seems to be the only platform which is affected by the problem 
that jlongs on stack are not 8 byte aligned.
(AFAIK, GCC uses -malign-double by default on 32 bit which should do the job 
for jlongs, too:
https://gcc.gnu.org/onlinedocs/gcc-4.5.3/gcc/i386-and-x86_002d64-Options.html)

Best regards,
Martin


From: Thomas Stüfe <thomas.stu...@gmail.com<mailto:thomas.stu...@gmail.com>>
Sent: Mittwoch, 4. Dezember 2019 17:56
To: Doerr, Martin <martin.do...@sap.com<mailto:martin.do...@sap.com>>
Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>; 
security-dev 
<security-...@openjdk.java.net<mailto:security-...@openjdk.java.net>>; 
Lindenmaier, Goetz <goetz.lindenma...@sap.com<mailto:goetz.lindenma...@sap.com>>
Subject: Re: RFR(S): 8220348: [ntintel] asserts about copying unalinged array

Hi Martin,

this makes sense. This is the right way to force alignment. I do not like the 
platform code in the shared file but do not think this is a big deal.

+#if defined (_WIN32) && defined (_MSC_VER)

Why do you think we need _MSC_VER too? Is OpenJDK on Windows even buildable 
with anything other than VC++?

Cheers, Thomas

On Mon, Dec 2, 2019 at 4:14 PM Doerr, Martin 
<martin.do...@sap.com<mailto:martin.do...@sap.com>> wrote:
Hi,

I'd like to propose a fix for an old issue on 32 bit Windows (also for an 11u 
backport):
https://bugs.openjdk.java.net/browse/JDK-8220348

Some jdk native methods use jni_SetLongArrayRegion with a stack allocated 
buffer.
jni_SetLongArrayRegion uses Copy::conjoint_jlongs_atomic which requires jlongs 
to be 8 byte aligned (asserted).
However, Windows 32 bit only uses 4 byte alignment for jlong arrays by default.
I found such issues in the following files:
src/java.prefs/windows/native/libprefs/WindowsPreferences.c
src/java.security.jgss/share/native/libj2gss/GSSLibStub.c
I suggest to use __declspec(align(8)) there.

Webrev:
http://cr.openjdk.java.net/~mdoerr/8220348_ntintel_stack_align/webrev.00/
Please review.

I think using 8 byte alignment is not a disadvantage for 64 bit.

I guess there are still people interested in this platform with jdk14. 
Otherwise I could contribute it as 11u only fix.

Is there a better way to force 8 byte alignment for jlongs or jlong arrays on 
stack?
Best regards,
Martin

Reply via email to