[foreign] RFR: Windows support

Thu Jan 10 14:13:07 UTC 2019

Maurizio Cimadamore schreef op 2019-01-10 15:08:
> On 10/01/2019 13:54, 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.
> 
> Why not just updating the already existing Java files for generating
> the tests, TestUpcall/DownCallGenerator ? These are in Java and can be
> executed using the recent java launcher feature (where you can pass a
> .java file to the 'java' launcher). Relatively easy to maintain.

I did that. The script just calls those 2 java generators 4 times to 
generate all the files :)

> In other words, the idea is that (similarly to what  has been done for
> some tests in the vector API), whenever we need to tweak the
> generators we:
> 
> 1) change the generators
> 2) rerun them so as to change the headers/impl files used by
> Upcall/DownCall tests
> 3) commit all changes

I wanted to avoid having huge files/diffs for something that can be 
generated automatically.

I will update the patch to include these files.

Jorn

> So, (3) will see changes in both the generated files and in the 
> generators.
> 
> Maurizio
> 
>> 
>> 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/