[foreign-jextract] Generated MemoryLayout contains padding instead of fields

Filip Krakowski krakowski at hhu.de
Sun Sep 6 11:07:15 UTC 2020 

Hi Jorn,

I think this consists of two separate bugs.

1. I noticed that the VarHandle uses the wrong MemoryLayout.

static final MemoryLayout tm$struct$LAYOUT_ = MemoryLayout.ofStruct(
         C_INT.withName("tm_sec"),
         C_INT.withName("tm_min"),
         C_INT.withName("tm_hour"),
         C_INT.withName("tm_mday"),
         C_INT.withName("tm_mon"),
         C_INT.withName("tm_year"),
         C_INT.withName("tm_wday"),
         C_INT.withName("tm_yday"),
         C_INT.withName("tm_isdst"),
         MemoryLayout.ofPaddingBits(32),
         C_LONG.withName("tm_gmtoff"),
         C_POINTER.withName("tm_zone")
     ).withName("tm");

This MemoryLayout has the same name like the nested one within 
"ibv_ops_wr$struct$LAYOUT_" from my last mail. Somehow both of them get 
mixed up in the generated code. As you can see in my last mail the 
VarHandle for "unexpected_cnt" uses "tm$struct$LAYOUT_" as its layout 
and an Exception is thrown at runtime since this layout does not contain 
the specified field. I assume this bug appears whenever two structs with 
the same name are declared at different levels of nesting. Here is the 
source for the "ibv_ops_wr" struct : 
https://github.com/linux-rdma/rdma-core/blob/c768684046865c7f02aef347b450fa053fe8a2ac/libibverbs/verbs.h#L1155-L1171.

2. Only the last (most inner) part of the layout path elements is 
specified in the generated function call.

I changed

    static final VarHandle tm$unexpected_cnt$VH_ =
    tm$struct$LAYOUT_.varHandle(int.class,
    MemoryLayout.PathElement.groupElement("unexpected_cnt"));


to

    static final VarHandle tm$unexpected_cnt$VH_ =
    ibv_ops_wr$struct$LAYOUT_.varHandle(int.class,
    MemoryLayout.PathElement.groupElement("tm"),
    MemoryLayout.PathElement.groupElement("unexpected_cnt"));


and the Exception for this specific field is gone, although the 
varHandle call for the next field inside the nested "tm" struct throws 
the same kind of Exception because of the same reasons.

The following snippet should reproduce the mentioned bugs without the 
need to install "rdma-core".

    struct ibv_ops_wr {
         struct {
             int     value
         } tm;
    };

    struct tm {
         int     dummy
    };

In this case the generated sources do contain a MemoryLayout for 
"ibv_ops_wr" which is not used (expect in the layout's getter method) 
and the path layout for the "value" VarHandle is incomplete.

Best regards,
Filip

On 06.09.20 12:35, Filip Krakowski wrote:
> Hi Jorn,
>
> the MemoryLayout does contain all expected fields now. The code does 
> compile but it looks like the layout path elements do only consider 
> the first level of fields within a struct. I get the following 
> exception at runtime.
>
>     java.lang.IllegalArgumentException: Bad layout path: cannot
>     resolve 'unexpected_cnt' in layout
>     [b32(tm_sec)[abi/sysv/class=INTEGER,layout/name=tm_sec]b32(tm_min)[abi/sysv/class=INTEGER,layout/name=tm_min]b32(tm_hour)[abi/sysv/class=INTEGER,layout/name=tm_hour]b32(tm_mday)[abi/sysv/class=INTEGER,layout/name=tm_mday]b32(tm_mon)[abi/sysv/class=INTEGER,layout/name=tm_mon]b32(tm_year)[abi/sysv/class=INTEGER,layout/name=tm_year]b32(tm_wday)[abi/sysv/class=INTEGER,layout/name=tm_wday]b32(tm_yday)[abi/sysv/class=INTEGER,layout/name=tm_yday]b32(tm_isdst)[abi/sysv/class=INTEGER,layout/name=tm_isdst]x32b64(tm_gmtoff)[abi/sysv/class=INTEGER,layout/name=tm_gmtoff]b64(tm_zone)[abi/sysv/class=POINTER,layout/name=tm_zone]](tm)[layout/name=tm]
>
>
> The corresponding MemoryLayout looks like this.
>
>     static final MemoryLayout ibv_ops_wr$struct$LAYOUT_ =
>     MemoryLayout.ofStruct(
>             C_LONG.withName("wr_id"),
>             C_POINTER.withName("next"),
>             C_INT.withName("opcode"),
>             C_INT.withName("flags"),
>             MemoryLayout.ofStruct(
>                 C_INT.withName("unexpected_cnt"),
>                 C_INT.withName("handle"),
>                 MemoryLayout.ofStruct(
>                     C_LONG.withName("recv_wr_id"),
>                     C_POINTER.withName("sg_list"),
>                     C_INT.withName("num_sge"),
>                     MemoryLayout.ofPaddingBits(32),
>                     C_LONG.withName("tag"),
>                     C_LONG.withName("mask")
>                 ).withName("add")
>             ).withName("tm")
>         ).withName("ibv_ops_wr");
>
>
> As you can see the "unexpected_cnt" field is within a nested struct 
> named "tm".
>
>     static final VarHandle tm$unexpected_cnt$VH_ =
>     tm$struct$LAYOUT_.varHandle(int.class,
>     MemoryLayout.PathElement.groupElement("unexpected_cnt"));
>
>
> Since the "varHandle" method's signature allows specifying multiple 
> PathElements through Varargs I would expect "tm" to be in front of 
> "unexpected_cnt", although I'm not entirely sure how the "elements" 
> parameter works.
>
> Best regards,
> Filip
>
> On 03.09.20 15:17, Jorn Vernee wrote:
>> Hi Filip,
>>
>> I've fixed the issue, please give it a try.
>>
>> Jorn
>>
>> On 03/09/2020 12:48, Jorn Vernee wrote:
>>> Hi,
>>>
>>> The issue with clang was fixed in LLVM 9, which we already use, so 
>>> we can update jextract to handle incomplete arrays now.
>>>
>>> I've filed: https://bugs.openjdk.java.net/browse/JDK-8252756
>>>
>>> Jorn
>>>
>>> On 02/09/2020 22:07, Maurizio Cimadamore wrote:
>>>> Hi Filip,
>>>> I'm afraid this might be an issue we know about - and is ultimately 
>>>> caused by clang - e.g. libclang will, if the struct contains an 
>>>> unspecified-size array, refuse to give information about _other 
>>>> fields_ in that struct. To avoid completely omitting generation of 
>>>> the struct, we have a workaround which just emits a struct with 
>>>> some padding (enough to fit the struct size) - but it seems the 
>>>> workaround is then causing further issues, as jextract will still 
>>>> try to generate struct getters and setters even if no field is 
>>>> really there (which I suspect is a problem, again, only in source 
>>>> generation mode).
>>>>
>>>> Thanks
>>>> Maurizio
>>>>
>>>> On 02/09/2020 21:01, Filip Krakowski wrote:
>>>>> Hi,
>>>>>
>>>>> my last encountered bug seems to be fixed with 
>>>>> https://git.openjdk.java.net/panama-foreign/commit/1dc9149d 
>>>>> ("constants are made package static final instead of private 
>>>>> static final"). The code compiles now but throws an Exception 
>>>>> during runtime.
>>>>>
>>>>>    java.lang.IllegalArgumentException: Bad layout path: cannot 
>>>>> resolve
>>>>>    'cmsg_len' in layout [x128]
>>>>>
>>>>>
>>>>> I searched for the field and found that the associated 
>>>>> MemoryLayout for "cmsghdr" only consists of padding in the 
>>>>> generated code.
>>>>>
>>>>>    static final MemoryLayout cmsghdr$struct$LAYOUT_ =
>>>>>    MemoryLayout.ofStruct(
>>>>>       MemoryLayout.ofPaddingBits(128)
>>>>>    );
>>>>>
>>>>>    public static jdk.incubator.foreign.MemoryLayout
>>>>>    cmsghdr$struct$LAYOUT() { return cmsghdr$struct$LAYOUT_; }
>>>>>
>>>>>    static final MemoryLayout cmsghdr$cmsg_len$LAYOUT_ = C_LONG;
>>>>>    public static jdk.incubator.foreign.MemoryLayout
>>>>>    cmsghdr$cmsg_len$LAYOUT() { return cmsghdr$cmsg_len$LAYOUT_; }
>>>>>
>>>>>    static final VarHandle cmsghdr$cmsg_len$VH_ =
>>>>>    cmsghdr$struct$LAYOUT_.varHandle(long.class,
>>>>>    MemoryLayout.PathElement.groupElement("cmsg_len"));
>>>>>    public static java.lang.invoke.VarHandle cmsghdr$cmsg_len$VH() {
>>>>>    return cmsghdr$cmsg_len$VH_; }
>>>>>
>>>>>    ...
>>>>>
>>>>>
>>>>> According to https://man7.org/linux/man-pages/man3/cmsg.3.html the 
>>>>> "cmsghdr" struct looks like this.
>>>>>
>>>>>    struct cmsghdr {
>>>>>       size_t cmsg_len;    /* Data byte count, including header
>>>>>                                           (type is socklen_t in 
>>>>> POSIX) */
>>>>>       int    cmsg_level;  /* Originating protocol */
>>>>>       int    cmsg_type;   /* Protocol-specific type */
>>>>>       /* followed by
>>>>>       unsigned char cmsg_data[]; */
>>>>>    };
>>>>>
>>>>>
>>>>> Best regards,
>>>>> Filip
>>>>> Email Signature
>>>>> On 02.09.20 12:30, Filip Krakowski wrote:
>>>>>> Hi Jorn,
>>>>>>
>>>>>> I just gave it a try and it looks good.
>>>>>>
>>>>>> Now unfortunately I have another bug. A source file (in my case 
>>>>>> "constants$3") is referencing a private field in another source 
>>>>>> file ("constants$2"). It looks like the VarHandles for the struct 
>>>>>> got split in two different source files. I am using the same 
>>>>>> header file 
>>>>>> (https://github.com/linux-rdma/rdma-core/blob/master/libibverbs/verbs.h) 
>>>>>> as before.
>>>>>>
>>>>>> constants$2.java
>>>>>>
>>>>>>     private static final MemoryLayout
>>>>>>     ib_uverbs_alloc_mw_resp$struct$LAYOUT_ =MemoryLayout.ofStruct(
>>>>>>          C_INT.withName("mw_handle"),
>>>>>>          C_INT.withName("rkey"),
>>>>>> MemoryLayout.ofSequence(0,C_LONG).withName("driver_data")
>>>>>>     ).withName("ib_uverbs_alloc_mw_resp");
>>>>>>     public static jdk.incubator.foreign.MemoryLayout
>>>>>>     ib_uverbs_alloc_mw_resp$struct$LAYOUT() {return 
>>>>>> ib_uverbs_alloc_mw_resp$struct$LAYOUT_; }
>>>>>>
>>>>>>     private static final MemoryLayout
>>>>>>     ib_uverbs_alloc_mw_resp$mw_handle$LAYOUT_ =C_INT;
>>>>>>     public static jdk.incubator.foreign.MemoryLayout
>>>>>>     ib_uverbs_alloc_mw_resp$mw_handle$LAYOUT() {return 
>>>>>> ib_uverbs_alloc_mw_resp$mw_handle$LAYOUT_; }
>>>>>>
>>>>>>     private static final VarHandle 
>>>>>> ib_uverbs_alloc_mw_resp$mw_handle$VH_ 
>>>>>> =ib_uverbs_alloc_mw_resp$struct$LAYOUT_.varHandle(int.class,MemoryLayout.PathElement.groupElement("mw_handle"));
>>>>>>     public static java.lang.invoke.VarHandle
>>>>>>     ib_uverbs_alloc_mw_resp$mw_handle$VH() {return 
>>>>>> ib_uverbs_alloc_mw_resp$mw_handle$VH_; }
>>>>>>
>>>>>>
>>>>>> constants$3.java
>>>>>>
>>>>>>     private static final MemoryLayout
>>>>>>     ib_uverbs_alloc_mw_resp$rkey$LAYOUT_ =C_INT;
>>>>>>     public static jdk.incubator.foreign.MemoryLayout
>>>>>>     ib_uverbs_alloc_mw_resp$rkey$LAYOUT() {return 
>>>>>> ib_uverbs_alloc_mw_resp$rkey$LAYOUT_; }
>>>>>>
>>>>>>     private static final VarHandle 
>>>>>> ib_uverbs_alloc_mw_resp$rkey$VH_ 
>>>>>> =ib_uverbs_alloc_mw_resp$struct$LAYOUT_.varHandle(int.class,MemoryLayout.PathElement.groupElement("rkey"));
>>>>>>     public static java.lang.invoke.VarHandle
>>>>>>     ib_uverbs_alloc_mw_resp$rkey$VH() {return 
>>>>>> ib_uverbs_alloc_mw_resp$rkey$VH_; }
>>>>>>
>>>>>>
>>>>>> Since "ib_uverbs_alloc_mw_resp$struct$LAYOUT_" is private within 
>>>>>> "constants$2" the code does not compile.
>>>>>>
>>>>>> Best regards,
>>>>>> Filip
>>>>>>
>>>>>> On 01.09.20 15:08, Jorn Vernee wrote:
>>>>>>> Hi Filip,
>>>>>>>
>>>>>>> This issue should now be fixed [1], can you give it a try?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jorn
>>>>>>>
>>>>>>> [1] : https://github.com/openjdk/panama-foreign/commit/afdbc657
>>>>>>>
>>>>>>> On 31/08/2020 12:35, Filip Krakowski wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I just noticed that jextract's generated getters and setters 
>>>>>>>> always access the provided MemorySegment at offset 0. This 
>>>>>>>> looks like a bug to me.
>>>>>>>>
>>>>>>>> This is the struct I used to reproduce this issue.
>>>>>>>>
>>>>>>>>    struct coordinate {
>>>>>>>>       int x;
>>>>>>>>       int y;
>>>>>>>>    };
>>>>>>>>
>>>>>>>>
>>>>>>>> And this is the code jextract generated from it (source mode).
>>>>>>>>
>>>>>>>>    private static final MemoryLayout coordinate$struct$LAYOUT_ 
>>>>>>>> =MemoryLayout.ofStruct(
>>>>>>>>         C_INT.withName("x"),
>>>>>>>>         C_INT.withName("y")
>>>>>>>>    ).withName("coordinate");
>>>>>>>>
>>>>>>>>    public static jdk.incubator.foreign.MemoryLayout
>>>>>>>>    coordinate$struct$LAYOUT() {return coordinate$struct$LAYOUT_; }
>>>>>>>>
>>>>>>>>    private static final MemoryLayout coordinate$x$LAYOUT_ =C_INT;
>>>>>>>>    public static jdk.incubator.foreign.MemoryLayout 
>>>>>>>> coordinate$x$LAYOUT() {return coordinate$x$LAYOUT_; }
>>>>>>>>
>>>>>>>>    private static final VarHandle coordinate$x$VH_ 
>>>>>>>> =coordinate$x$LAYOUT_.varHandle(int.class);
>>>>>>>>    public static java.lang.invoke.VarHandle coordinate$x$VH() 
>>>>>>>> {return coordinate$x$VH_; }
>>>>>>>>
>>>>>>>>    private static final MemoryLayout coordinate$y$LAYOUT_ =C_INT;
>>>>>>>>    public static jdk.incubator.foreign.MemoryLayout 
>>>>>>>> coordinate$y$LAYOUT() {return coordinate$y$LAYOUT_; }
>>>>>>>>
>>>>>>>>    private static final VarHandle coordinate$y$VH_ 
>>>>>>>> =coordinate$y$LAYOUT_.varHandle(int.class);
>>>>>>>>    public static java.lang.invoke.VarHandle coordinate$y$VH() 
>>>>>>>> {return coordinate$y$VH_; }
>>>>>>>>
>>>>>>>>
>>>>>>>> Here I allocate the struct, set both fields and print them 
>>>>>>>> afterwards.
>>>>>>>>
>>>>>>>>    try (var segment =coordinate.allocate()) {
>>>>>>>>         coordinate.x$set(segment,1);
>>>>>>>>         coordinate.y$set(segment,2);
>>>>>>>>
>>>>>>>> System.out.println(coordinate.x$get(segment));
>>>>>>>> System.out.println(coordinate.y$get(segment));
>>>>>>>>    }
>>>>>>>>
>>>>>>>>
>>>>>>>> This will print "2" twice, although "x" should be set to "1" in 
>>>>>>>> my understanding.
>>>>>>>>
>>>>>>>> I am using the latest build (commit 
>>>>>>>> 4d7888c040767760b6250130ef6024ea16b43461).
>>>>>>>>
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Filip
>>>>>>
>>>>>
>

More information about the panama-dev mailing list