Hi Christoph, that would work, but I don’t want to pollute this file with compiler specific defines. In addition, I don’t like introducing a macro which works on some platforms and does nothing on other ones (which is the case for hotspot’s ATTRIBUTE_ALIGNED).
Because Windows 32 bit is the only affected platform, I prefer not to touch other ones. Are you ok with webrev.00 as it is? Best regards, Martin From: Langer, Christoph <christoph.lan...@sap.com> Sent: Donnerstag, 5. Dezember 2019 12:16 To: Doerr, Martin <martin.do...@sap.com> Cc: core-libs-dev@openjdk.java.net; security-dev <security-...@openjdk.java.net>; Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Thomas Stüfe <thomas.stu...@gmail.com> Subject: RE: RFR(S): 8220348: [ntintel] asserts about copying unalinged array 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<mailto:martin.do...@sap.com>> Sent: Donnerstag, 5. Dezember 2019 12:00 To: Thomas Stüfe <thomas.stu...@gmail.com<mailto:thomas.stu...@gmail.com>>; Langer, Christoph <christoph.lan...@sap.com<mailto:christoph.lan...@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 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
