-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generators: Add padding between non consecutive fields #368
Conversation
Could you share some system info? I can't reproduce the wrong padding result on the Julia side. julia> using Clang.Generators
julia> args = get_default_args("x86_64-linux-gnu")
5-element Vector{String}:
"-isystem/Users/gnimuc/.julia/ar" ⋯ 56 bytes ⋯ "/x86_64-linux-gnu/4.8.5/include"
"-isystem/Users/gnimuc/.julia/ar" ⋯ 62 bytes ⋯ "4-linux-gnu/4.8.5/include-fixed"
"-isystem/Users/gnimuc/.julia/ar" ⋯ 42 bytes ⋯ "e3d045/x86_64-linux-gnu/include"
"-isystem/Users/gnimuc/.julia/ar" ⋯ 55 bytes ⋯ "-linux-gnu/sys-root/usr/include"
"--target=x86_64-unknown-linux-gnu"
shell> cat test.h
#include <stdint.h>
struct A {
char a;
struct B {
uint64_t ba;
uint64_t bb;
} b;
};
julia> ctx = create_context("test.h", args);
[ Info: Parsing headers...
julia> build!(ctx)
[ Info: Processing header: test.h
[ Info: Building the DAG...
┌ Warning: default libname: ":libxxx" is being used, did you forget to set `library_name` in the toml file?
└ @ Clang.Generators ~/.julia/dev/Clang/src/generator/audit.jl:16
[ Info: Emit Julia expressions...
struct B
ba::UInt64
bb::UInt64
end
struct A
a::Cchar
b::B
end
[ Info: Done!
Context(...)
julia> struct B
ba::UInt64
bb::UInt64
end
julia> struct A
a::Cchar
b::B
end
julia> sizeof(A)
24
julia> sizeof(B)
16 |
It would be great if you could contribute a set of test cases. There were some simple tests as mentioned in #317 (comment) and a repo for monitoring the sanity of the generated code. |
oh I simplified my example a bit too much. It only happens when the re-aligned field is an union because in Julia they are represented as |
|
||
current_offset = 0 | ||
pads = [] | ||
for (i, field_cursor) in enumerate(field_cursors) | ||
field_sym = make_symbol_safe(name(field_cursor)) | ||
field_ty = getCursorType(field_cursor) | ||
field_offset = getOffsetOfField(field_cursor) ÷ 8 | ||
field_size = getSizeOf(field_ty) | ||
@assert field_offset >= current_offset | ||
|
||
if field_offset != current_offset | ||
pad_size = field_offset - current_offset | ||
pad_name = Symbol("pad", length(pads)) |> gensym | ||
push!(block.args, Expr(:(::), pad_name, :(NTuple{$pad_size, UInt8}))) | ||
push!(pads, (;narg = i + length(pads), size = pad_size)) | ||
end | ||
|
||
current_offset += field_size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix only handles generic cases, the same fix should also apply for StructMutualRef
.
An alternative fix would be marking these kinda structs as |
I think this solution is also good. We can add an option in the .toml file to let users choose what they'd like to generate. You could implement this by adding a new type like |
#368 (comment) has been implemented in #370 and merged to the master. |
Awesome! Sorry i didn't had time to update on this. I don't know whether or not it's worth adding the different codegen with paddings now that it uses a |
Hello 👋
I had an issue with Clang.jl where the created Julia struct would not match the C representation of the same struct because of field alignments issues.
Consider the following struct:
The generated Julia struct is:
The C and Julia version of field
b::B
are not aligned (C is+8
whereas Julia is only+1
). This PR tries to address this problem by adding new fields on the Julia side and adding a custom constructor to the struct:I have noticed that there are only a few tests for
Clang.Generators
so I would be interested on having other codebase to test this code on. For example when generating the Julia forclang-c
into LibClang some struct are added padding fields. Also, should I add test for this ? It would eval the generated julia code and check the differentsizeof
.Anyway, thanks for making this project!