[foreign] RFR: Windows support

Thu Jan 10 14:24:27 UTC 2019

2.1) and 2.2) is the srcgen.sh thing. I'm not sure why 
com/sun/tools/jextract/jclang-ffi/TestJextractFFI.java failed, it looks 
like your link to the test report got stripped from the email? I changed 
that test so that it grabs the libclang file name from a system property 
since it differs on the different platforms ("clang" vs "libclang").

I'll work on an updated webrev for 1) and 2.1) 2.2) in the mean time.

Jorn

Sundararajan Athijegannathan schreef op 2019-01-10 15:18:
> There are two issues:
> 
> 1) I had to change make/test/JtregNativeJdk.gmk
> 
> +    ifeq ($(OPENJDK_TARGET_OS), windows)
> +        # windows flags
> +        BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libVector := /arch:AVX
> +        BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libUpcall := /arch:AVX
> +        BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libGlobalVariable :=
> /arch:AVX
> +    else
> +        # Enable AVX for vector (Long*) tests
> +        BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libVector := -mavx
> +        BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libUpcall := -mavx
> +        BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libGlobalVariable := -mavx
> +    endif
> 
> Because I was getting "clang: error: no such file or directory:
> '/arch:AVX' on Mac. With the above change, I managed to get passed
> that failure.
> 
> 2) Three tests failed.
> 
> com/sun/tools/jextract/TestDowncall.java [1]:
> com/sun/tools/jextract/TestUpcall.java [2]:
> com/sun/tools/jextract/jclang-ffi/TestJextractFFI.java [3]:
> 
> I think that is srcgen.sh issue you're talking about. If files have to
> be generated, either we need to automate or pre-generate and be
> included as part of patch. We cannot expect shell scripts to be
> manually run before tests (in nightly testing for eg).
> 
> Please incorporate the above changes and send RFR against the test
> changes bug. Also please check my change above is fine on Windows.
> 
> Thanks,
> -Sundar
> 
> On 10/01/19, 7:24 PM, Jorn Vernee wrote:
> 
>>> PS. I'm running the test patch on Mac and Linux to make sure that
>>> the
>>> testing is fine on other platforms.
>> 
>> Thanks.
>> 
>> One thing I forgot to mention; the generated source files for
>> UpcallTest and DowncallTest needed changes, so I've removed them and
>> created the srcgen.sh script in test/jdk/com/sun/tools/jextract to
>> generate the source files instead. I had this hooked up through
>> jtreg, but since the native files are compiled before the sources
>> are generated this didn't generate the files in time, and I never
>> got around to figuring out how to hook the generation into the build
>> system.
>> 
>> So, just be aware that after applying the test-tweaks patch you
>> still have to run that srcgen.sh script (relies on java 11 on PATH)
>> to generate the test sources, since the patch deletes them.
>> 
>> Jorn
>> 
>> Sundararajan Athijegannathan schreef op 2019-01-10 14:26:
>> The 4 bugs (including the preexisting one) are as follows:
>> 
>> 1) Library.getDefault() does not work on Windows
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8211060
>> 
>> 2) Binder changes are needed to support paname foreign API on
>> Windows
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8216485
>> 
>> 3)  jextract LayoutUtils has to be modified for Windows
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8216484
>> 
>> 4) jextract tests have to be modified to run on Windows platform
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8216483
>> 
>> PS. I'm running the test patch on Mac and Linux to make sure that
>> the
>> testing is fine on other platforms.
>> 
>> Thanks,
>> -Sundar
>> 
>> On 10/01/19, 6:14 PM, Maurizio Cimadamore wrote:
>> Hi Jorn,
>> Thanks for the hard work; we are in the process of reviewing the
>> changes. Here's what we'll do:
>> 
>> 1) we will create issues for each of the patches (Sundar is doing
>> that as we speak)
>> 2) we will try to get easier changes in sooner rather  than later
>> (that is, patches 1, 3, 4)
>> 3) in parallel, I'll be doing an in-depth review of your binder
>> changes and perhaps start a separate thread on that
>> 
>> Thanks
>> Maurizio
>> 
>> On 09/01/2019 21:17, Jorn Vernee wrote:
>> 
>> Hi,
>> 
>> I have fixed the last few failing tests on Windows so this is an RFR
>> for adding Windows support to Panama FFI. There's a lot of changes
>> so I've split it into 4 patches. We should probably focus on one of
>> these one at a time, but I wanted to include all the changes needed
>> to pass the tests at least. It's probably best to start with [2],
>> since that implements the core functionality.
>> 
>> Default lib fixes [1] (needed for the default library to work on
>> Windows)
>> Hotspot, binder tweaks [2] (Changes to the stub generation and
>> shared binder code. Includes the Windows implementation of
>> SystemABI)
>> jextract tweaks [3] (minor jextract portability changes)
>> test tweaks [4] (changes needed to run, and pass the tests on
>> Windows)
>> 
>> There are some tests that are disabled:
>> - EmptyStructTest; Empty structs are a GCC extension.
>> - BadBitfieldTest; this is testing for a problem the MSVC ABI
>> doesn't seem to have.
>> - StdioTest; relies on standard system header path, which on
>> Windows is more tricky to find.
>> - ComplexTest; Windows complex types work very differently in C
>> source, so the current test doesn't compile.
>> - LongDoubleTest; long double is only 64 bits on Windows. This
>> might need a replacement test still, but it should work the same as
>> `double` on Windows.
>> - Getpid; No getpid on Windows in the C standard.
>> - UnixSystem; Windows is not unix.
>> 
>> (Some tests specifically for Windows could be added, but I'm leaving
>> that for a different RFR)
>> 
>> Changes for [1]:
>> - replace default library handle with a findEntryInProcess since
>> Windows doesn't have an RTLD_DEFAULT equivalent.
>> 
>> Changes for [2]:
>> - changed upcall/downcall stub generation to follow the MS ABI on
>> windows (using compiler switches)
>> - added SKIPs to shuffle recipe generation when e.g. the first
>> float is passed in the second float register (which can be the case
>> on windows).
>> - moved boxValue and unboxValue into SystemABI, since they turned
>> out to be ABI specific, and made tweaks to the windows version for
>> handeling structs being passed by value.
>> - moved upcall stub natives into shared ABI package.
>> - Let each ABI have it's own VarargsInvoker implementation, since
>> we need to propagate more info about which arguments are variadic on
>> windows.
>> - CallingSequence::storageOffset is no longer relying on
>> Storage::getStorageIndex, since that is not the same as what's
>> actually needed; being the index of the binding. (the 2 can differ
>> on windows, but not on sysv).
>> - Added Windows ABI implementation classes based on the SysV ones.
>> 
>> - Let UniversalUpcallHandler write the in-memory-return pointer to
>> RAX since that is required on Windows (and I think this change is
>> portable).
>> 
>> Changes for [3]:
>> - instead of hardcoding the size of `long` `unsigned long` and
>> `long double`, delegate to libclang to tell us the size, since these
>> types have different sizes on the 2 ABIs. (This is pretty ad-hoc, so
>> probably needs to be improved. But, I have little experience with
>> the jextract code, so I'm open to suggestion of how to better fix
>> this)
>> 
>> Changes for [4]:
>> - added dll exports for native symbols using compiler switches.
>> - added test for small (fits-in-register) struct by-value passing,
>> since windows has special handling for those.
>> - removed depencies on POSIX functions from some tests where it
>> wasn't really needed, so they could run on windows.
>> - propagate clang library name to TestJextractFFI, since on
>> Windows we need to load "libclang".
>> - let Runner test use windows specific files to compare with for
>> bitfields.h and simple.h
>> - use `long long` instead of `long` in some cases since `long
>> long` is 64 bits on SysV and MSx64
>> 
>> Thanks,
>> Jorn
>> 
>> [1] :
>> 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8211060/webrev.01/
>> 
>> [2] :
>> 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/windows/webrev.03/
>> 
>> [3] :
>> 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/windows_jextract/webrev.00/
>> [4] :
>> 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/windows_tests/webrev.02/
> 
> 
> Links:
> ------
> [1]
> http://webmail.xs4all.nl/../../../test-support/jtreg_test_jdk_jdk_foreign/com/sun/tools/jextract/TestDowncall.jtr
> [2]
> http://webmail.xs4all.nl/../../../test-support/jtreg_test_jdk_jdk_foreign/com/sun/tools/jextract/TestUpcall.jtr
> [3]
> http://webmail.xs4all.nl/../../../test-support/jtreg_test_jdk_jdk_foreign/com/sun/tools/jextract/jclang-ffi/TestJextractFFI.jtr