[foreign-jextract] Wrong MemoryLayout and VarHandle carrier for integer field

Fri Aug 28 09:09:18 UTC 2020

Hi,

amazing! I am really impressed how fast bugs are fixed. Thank you all!

Best regards,
Filip

On 27.08.20 20:02, Maurizio Cimadamore wrote:
> Hi Filip,
> Sundar has just fixed this; I have verified that extraction of verbs.h 
> no longer shows the issue.
>
> Cheers
> Maurizio
>
> On 27/08/2020 15:55, Maurizio Cimadamore wrote:
>> We got at the bottom of this; there are multiple structs defining 
>> fields with same name, and in that case jextract always resolves the 
>> var handle to the first one.
>>
>> Thanks for catching this and helping us reproducing.
>>
>> Cheers
>> Maurizio
>>
>> On 27/08/2020 15:45, Maurizio Cimadamore wrote:
>>> I see now
>>>
>>> we have two:
>>>
>>> sources/rdmi/verbs_h$constants$2.java: C_BOOL.withName("sq_sig_all"),
>>> sources/rdmi/verbs_h$constants$6.java: C_INT.withName("sq_sig_all"),
>>> sources/rdmi/verbs_h$constants$6.java: C_INT.withName("sq_sig_all"),
>>>
>>> Maurizio
>>>
>>> On 27/08/2020 15:42, Filip Krakowski wrote:
>>>> Hi,
>>>>
>>>> the layout looks the same in my case 
>>>> (C_INT.withName("sq_sig_all")). Getters and setters use byte and 
>>>> the VarHandles MemoryLayout is C_BOOL.
>>>>
>>>> I think I found out what happens. Another struct named 
>>>> "ib_uverbs_ex_create_qp" uses the same field name 
>>>> (https://urldefense.com/v3/__https://github.com/linux-rdma/rdma-core/blob/b4d8c5a8102316476c44829db1a91b672fbfef5d/kernel-headers/rdma/ib_user_verbs.h*L613__;Iw!!GqivPVa7Brio!IQvwdAz0HL_NhTchq2aUomxq3y3bMMzDaj3s_BukxhHoRqWh1Yt0LhGUM6MbU4B5KI3R0oE$ 
>>>> ). It seems the getters and setters for the field inside 
>>>> "ibv_qp_init_attr" 
>>>> (https://urldefense.com/v3/__https://github.com/linux-rdma/rdma-core/blob/master/libibverbs/verbs.h*L887__;Iw!!GqivPVa7Brio!IQvwdAz0HL_NhTchq2aUomxq3y3bMMzDaj3s_BukxhHoRqWh1Yt0LhGUM6MbU4B5ClheUYc$ 
>>>> ) use the VarHandle generated for the field inside 
>>>> "ib_uverbs_ex_create_qp" (which is correctly using byte).
>>>>
>>>> Best regards,
>>>> Filip
>>>>
>>>> On 27.08.20 16:20, Maurizio Cimadamore wrote:
>>>>>
>>>>> On 27/08/2020 15:07, Maurizio Cimadamore wrote:
>>>>>> What do I need to have installed to try this out? It seems like 
>>>>>> the header you linked has some dependencies on <infiniband> 
>>>>>> headers which are not in the github repo. Could you please point 
>>>>>> me at how you set up your machine to get those?
>>>>>
>>>>> To get around it, I redefined in place a bunch of enums which were 
>>>>> defined elsewhere and was able to get jextract working; this is 
>>>>> what I got:
>>>>>
>>>>>
>>>>> private static final MemoryLayout ibv_qp_init_attr$struct$LAYOUT_ 
>>>>> = MemoryLayout.ofStruct(
>>>>>         C_POINTER.withName("qp_context"),
>>>>>         C_POINTER.withName("send_cq"),
>>>>>         C_POINTER.withName("recv_cq"),
>>>>>         C_POINTER.withName("srq"),
>>>>>         MemoryLayout.ofStruct(
>>>>>             C_INT.withName("max_send_wr"),
>>>>>             C_INT.withName("max_recv_wr"),
>>>>>             C_INT.withName("max_send_sge"),
>>>>>             C_INT.withName("max_recv_sge"),
>>>>>             C_INT.withName("max_inline_data")
>>>>>         ).withName("cap"),
>>>>>         C_INT.withName("qp_type"),
>>>>>         C_INT.withName("sq_sig_all"),
>>>>>         MemoryLayout.ofPaddingBits(32)
>>>>>     ).withName("ibv_qp_init_attr");
>>>>>
>>>>>
>>>>> No C_BOOL here... odd
>>>>>
>>>>> Maurizio
>>>>>
>>>>>
>>>>>>
>>>>>> Maurizio
>>>>>>
>>>>>> On 27/08/2020 14:01, Filip Krakowski wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> this is source file generation. I noticed that the constants 
>>>>>>> class file is no longer generated (in source mode) and instead 
>>>>>>> several constants source files are created.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Filip
>>>>>>>
>>>>>>> On 27.08.20 14:54, Maurizio Cimadamore wrote:
>>>>>>>> Out of curiousity - is this source or classfile generation?
>>>>>>>>
>>>>>>>> Maurizio
>>>>>>>>
>>>>>>>> On 27/08/2020 12:56, Filip Krakowski wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I already use the latest build (commit 06675b build yesterday 
>>>>>>>>> 10:28PM - 
>>>>>>>>> https://urldefense.com/v3/__https://coconucos.cs.hhu.de/forschung/jdk/panama-20200826202808-06675b.zip__;!!GqivPVa7Brio!LZ_9YbqVSPqVFBgvUpbtN5NZ8u_rK2LJJ7qkmpZ3wI-4vNinKdeqRIz1TVuiE2o7wL_IUwA$ 
>>>>>>>>> ). Other structs containing "int" fields (for example 
>>>>>>>>> "ibv_device_attr") work well and use "C_INT". I also checked 
>>>>>>>>> my local header files.
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> Filip
>>>>>>>>>
>>>>>>>>> On 27.08.20 13:26, sundararajan.athijegannathan at oracle.com wrote:
>>>>>>>>>> I don't have the complete dependencies on my machine
>>>>>>>>>>
>>>>>>>>>> I locally filled the struct definition you gave with 
>>>>>>>>>> appropriate dependencies (dummy struct, enum declarations) 
>>>>>>>>>> and jextracted it.
>>>>>>>>>>
>>>>>>>>>> I see C_INT is being generated as layout for sq_sig_all. I 
>>>>>>>>>> suspect most likely you're using old panama build. I'd 
>>>>>>>>>> recommend building latest panama build from repo and trying 
>>>>>>>>>> your header with the same.
>>>>>>>>>>
>>>>>>>>>> -Sundar
>>>>>>>>>>
>>>>>>>>>> On 27/08/20 4:14 pm, Filip Krakowski wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> I would like to report a bug regarding jextract. The header 
>>>>>>>>>>> file 
>>>>>>>>>>> (https://urldefense.com/v3/__https://github.com/linux-rdma/rdma-core/blob/master/libibverbs/verbs.h__;!!GqivPVa7Brio!LZ_9YbqVSPqVFBgvUpbtN5NZ8u_rK2LJJ7qkmpZ3wI-4vNinKdeqRIz1TVuiE2o7emilnN0$ 
>>>>>>>>>>> ) I give to jextract contains the following struct.
>>>>>>>>>>>
>>>>>>>>>>>    struct ibv_qp_init_attr {
>>>>>>>>>>>             void *qp_context;
>>>>>>>>>>>             struct ibv_cq          *send_cq;
>>>>>>>>>>>             struct ibv_cq          *recv_cq;
>>>>>>>>>>>             struct ibv_srq         *srq;
>>>>>>>>>>>             struct ibv_qp_cap       cap;
>>>>>>>>>>>             enum ibv_qp_type        qp_type;
>>>>>>>>>>>             int sq_sig_all;
>>>>>>>>>>>    };
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> The Code generated for the "sq_sig_all" field looks like this.
>>>>>>>>>>>
>>>>>>>>>>>    private static final MemoryLayout sq_sig_all$LAYOUT_ 
>>>>>>>>>>> =C_BOOL;
>>>>>>>>>>>    private static final VarHandle sq_sig_all$VH_ 
>>>>>>>>>>> =sq_sig_all$LAYOUT_.varHandle(byte.class);
>>>>>>>>>>>
>>>>>>>>>>>    public static int sq_sig_all$get(MemorySegment seg) {
>>>>>>>>>>>         return 
>>>>>>>>>>> (int)verbs_h$constants$3.sq_sig_all$VH().get(seg);
>>>>>>>>>>>    }
>>>>>>>>>>>
>>>>>>>>>>>    public static void 
>>>>>>>>>>> sq_sig_all$set(jdk.incubator.foreign.MemorySegment seg,int x) {
>>>>>>>>>>> verbs_h$constants$3.sq_sig_all$VH().set(seg, x);
>>>>>>>>>>>    }
>>>>>>>>>>>
>>>>>>>>>>> As you can see jextract chooses "C_BOOL" as the MemoryLayout 
>>>>>>>>>>> for the "sq_sig_all" field and uses "byte" for the 
>>>>>>>>>>> VarHandle's carrier. In contrast to this, getters and 
>>>>>>>>>>> setters work with "int" which leads to an Exception during 
>>>>>>>>>>> runtime.
>>>>>>>>>>>
>>>>>>>>>>>    java.lang.invoke.WrongMethodTypeException: cannot convert
>>>>>>>>>>> MethodHandle(VarHandle,MemorySegment,byte)void to
>>>>>>>>>>>    (VarHandle,MemorySegment,int)void
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>