[foreign] RFR: Windows support

Thu Jan 10 15:09:27 UTC 2019

Well, that's the problem, the .dll doesn't follow the standard library 
name format. i.e. it's not called clang.dll but libclang.dll

Jorn

Sundararajan Athijegannathan schreef op 2019-01-10 16:19:
> clang.dll / libclang.so / libclang.dylib is already taken care by
> native library loading mechanism. There should not be any need for
> System property.
> 
> The test failure stack trace on Mac is as follows:
> 
> Loading LibClang FFI
> Exception in thread "main" java.lang.UnsatisfiedLinkError: no libclang
> in java.library.path: [/Users/SATHIJEG/bin/llvm/lib]
>     at
> java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2700)
>     at java.base/java.lang.Runtime.loadLibrary0(Runtime.java:849)
>     at java.base/java.lang.Runtime.loadLibrary(Runtime.java:829)
>     at java.base/java.lang.System$2.loadLibrary(System.java:2280)
>     at
> java.base/jdk.internal.foreign.LibrariesHelper.loadLibrary(LibrariesHelper.java:169)
>     at
> java.base/java.foreign.Libraries.loadLibrary(Libraries.java:100)
>     at
> jdk.internal.clang/jdk.internal.clang.LibClang.<clinit>(LibClang.java:43)
>     at
> jdk.jextract/com.sun.tools.jextract.parser.Parser.parse(Parser.java:71)
>     at
> jdk.jextract/com.sun.tools.jextract.JextractTool.processHeaders(JextractTool.java:58)
>     at jdk.jextract/com.sun.tools.jextract.Main.run(Main.java:264)
>     at jdk.jextract/com.sun.tools.jextract.Main.main(Main.java:334)
> 
> -Sundar
> 
> On 10/01/19, 7:54 PM, Jorn Vernee wrote:
> 
>> 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