Clang.jl
Clang.jl copied to clipboard
Generators: Add padding between non consecutive fields
Hello :wave:
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:
struct B {
uint64_t ba;
uint64_t bb;
};
union union_B {
struct B b;
};
struct A {
char a;
union union_B b;
};
/**/
sizeof(struct A) // 24
sizeof(struct B) // 16
sizeof(struct A) != sizeof(char) + sizeof(struct B) // true
The generated Julia struct is:
struct B
ba::UInt64
bb::UInt64
end
struct union_B
data::NTuple{16, UInt8}
end
struct A
a::Cchar
b::union_B
end
#==#
sizeof(A) # 17
sizeof(union_B) # 16
sizeof(A) != sizeof(Cchar) + sizeof(union_B) # false
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:
struct B
ba::UInt64
bb::UInt64
end
struct union_B
data::NTuple{16, UInt8}
end
struct A
a::Cchar
var"##pad0#274"::NTuple{7, UInt8}
b::union_B
A(a, b) = new(a, Tuple((zero(UInt8) for _ = 1:7)), b)
end
#==#
sizeof(A) # 24
sizeof(union_B) # 16
sizeof(A) != sizeof(Cchar) + sizeof(union_B) # true
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 for clang-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 different sizeof
.
Anyway, thanks for making this project!
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
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.
It would be great if you could contribute a set of test cases. There were some simple tests as mentioned in https://github.com/JuliaInterop/Clang.jl/issues/317#issuecomment-884837470 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 NTuple{X, UInt8}
which does not require a different alignment. I updated the example as such.
An alternative fix would be marking these kinda structs as StructLayout
in the preprocessing pass like what we do for nested_anonymous_type
s. In this way, we redirect its codegen pass to here, so don't need to change anything in the CodeGen pass.
struct A
a::Cchar
var"##pad0#274"::NTuple{7, UInt8}
b::union_B
A(a, b) = new(a, Tuple((zero(UInt8) for _ = 1:7)), b)
end
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 StructLayout
and writing a new codegen pass for it.
https://github.com/JuliaInterop/Clang.jl/pull/368#issuecomment-1009602873 has been implemented in #370 and merged to the master.
#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 StructLayout
. What do you think ?