[foreign-jextract] Generated MemoryLayout contains padding instead of fields
Jorn Vernee
jorn.vernee at oracle.com
Mon Sep 7 09:41:57 UTC 2020
Hi Filip,
Thanks for looking into this. I also found bug 1. last week:
https://bugs.openjdk.java.net/browse/JDK-8252799
Number 2. looks like it's behaving as intended though. If you have a
struct like
struct ibv_ops_wr {
struct {
int value
} tm;
};
You would access the 'value' field like:
struct ibv_ops_wr o;
o.tm.value = ...
The panama equivalent would be to get a MemorySegment for tm from the
ibv_ops_wr struct, and then pass that MS to the generated getter. i.e.
doing a two-stage dereference.
If you'd rather be able to access 'value' directly from ibv_ops_wr, you
would have to create your own VarHandle and getter for now.
(maybe we can generate a getter that works on the top-level struct as
well, but we have some technical debt in that area which needs to be
resolved first).
Jorn
On 06/09/2020 13:07, Filip Krakowski wrote:
> 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
> <https://urldefense.com/v3/__https://github.com/linux-rdma/rdma-core/blob/c768684046865c7f02aef347b450fa053fe8a2ac/libibverbs/verbs.h*L1155-L1171__;Iw!!GqivPVa7Brio!PgD2mgX21pdmUjQifwiut_7taheQN0CS2F2HxkumIBEIgsdICRqJrU-2-JxZR_BH$>.
>
> 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