`field_access_method_list` is not architecture-independent

[JamesWrigley]

I came across a bug when testing some code on 32bit CI:

c_attrs = LibSSH.lib.sftp_attributes_struct(Ptr{Int8} @0x00000000, Ptr{Int8} @0x00000000, 0x0000000f, 0x01, 0x0000000000000000, 0x000003e9, 0x0000007f, Ptr{Int8} @0x00000000, Ptr{Int8} @0x00000000, 0x00008180, 0x0000000000000000, 0x671c0a45, 0x00000000, 0x0000000000000000, 0x00000000, 0x0000000000000000, 0x671c0a45, 0x00000000, Ptr{LibSSH.lib.ssh_string_struct} @0x00000000, 0x00000000, Ptr{LibSSH.lib.ssh_string_struct} @0x00000000, Ptr{LibSSH.lib.ssh_string_struct} @0x00000000)
c_attrs.owner = Ptr{Int8} @0x00000000
x = Ptr{Int8} @0x00008180

[2065] signal (11.1): Segmentation fault
in expression starting at none:1
unknown function (ip: 0xe7ea162a)

c_attrs is a struct that has been unsafe_load()'d from a pointer. c_attrs.owner is a pointer field of c_attrs, and x is the result of calling unsafe_load(getproperty(::Ptr{sftp_attributes_struct}, :owner)). The problem is that c_attrs.owner and x do not have the same value, and I believe this is because our getproperty() implementations don't take into account the fact that sizeof(Ptr) is different on 32bit/64bit platforms.

This is the struct definition generated by Clang.jl:

Code

mutable struct sftp_attributes_struct
    name::Ptr{Cchar}
    longname::Ptr{Cchar}
    flags::UInt32
    type::UInt8
    size::UInt64
    uid::UInt32
    gid::UInt32
    owner::Ptr{Cchar}
    group::Ptr{Cchar}
    permissions::UInt32
    atime64::UInt64
    atime::UInt32
    atime_nseconds::UInt32
    createtime::UInt64
    createtime_nseconds::UInt32
    mtime64::UInt64
    mtime::UInt32
    mtime_nseconds::UInt32
    acl::ssh_string
    extended_count::UInt32
    extended_type::ssh_string
    extended_data::ssh_string
end
function Base.getproperty(x::Ptr{sftp_attributes_struct}, f::Symbol)
    f === :name && return Ptr{Ptr{Cchar}}(x + 0)
    f === :longname && return Ptr{Ptr{Cchar}}(x + 8)
    f === :flags && return Ptr{UInt32}(x + 16)
    f === :type && return Ptr{UInt8}(x + 20)
    f === :size && return Ptr{UInt64}(x + 24)
    f === :uid && return Ptr{UInt32}(x + 32)
    f === :gid && return Ptr{UInt32}(x + 36)
    f === :owner && return Ptr{Ptr{Cchar}}(x + 40)
    f === :group && return Ptr{Ptr{Cchar}}(x + 48)
    f === :permissions && return Ptr{UInt32}(x + 56)
    f === :atime64 && return Ptr{UInt64}(x + 64)
    f === :atime && return Ptr{UInt32}(x + 72)
    f === :atime_nseconds && return Ptr{UInt32}(x + 76)
    f === :createtime && return Ptr{UInt64}(x + 80)
    f === :createtime_nseconds && return Ptr{UInt32}(x + 88)
    f === :mtime64 && return Ptr{UInt64}(x + 96)
    f === :mtime && return Ptr{UInt32}(x + 104)
    f === :mtime_nseconds && return Ptr{UInt32}(x + 108)
    f === :acl && return Ptr{ssh_string}(x + 112)
    f === :extended_count && return Ptr{UInt32}(x + 120)
    f === :extended_type && return Ptr{ssh_string}(x + 128)
    f === :extended_data && return Ptr{ssh_string}(x + 136)
    return getfield(x, f)
end

function Base.setproperty!(x::Ptr{sftp_attributes_struct}, f::Symbol, v)
    unsafe_store!(getproperty(x, f), v)
end

The first field is a pointer and the offset for the second field is hardcoded to 8 bytes, but that's only valid on 64bit systems (which the bindings were generated on). The problem is coming from codegen where we just use the field offset from Clang, which presumably returns the field offset on the host system:

Clang.jl/src/generator/codegen.jl

Lines 311 to 320 in 8ed2dd2

	offset = getOffsetOf(getCursorType(root_cursor), n)
	if isBitField(field_cursor)
	w = getFieldDeclBitWidth(field_cursor)
	@assert w <= 32 # Bit fields should not be larger than int(32 bits)
	d, r = divrem(offset, 32)
	ex = :(f === $(QuoteNode(fsym)) && return (Ptr{$ty}(x + $(4d)), $r, $w))
	else
	d = offset ÷ 8
	ex = :(f === $(QuoteNode(fsym)) && return Ptr{$ty}(x + $d))
	end

It's slightly annoying to fix since we'll need to keep track of how many pointer fields there are and take into account all of their offsets. But I'm guessing not many people use 32bit machines anymore, especially in scientific computing, so this probably isn't very high-priority.

[Gnimuc]

The problem is coming from codegen where we just use the field offset from Clang, which presumably returns the field offset on the host system:

Does this happen when a multiplatform setup is used? e.g. https://github.com/JuliaGPU/VulkanCore.jl/tree/master/lib

[JamesWrigley]

Ah hah, no it doesn't :) Maybe we should warn users that if they're not generating code for all targets they must at least generate a 64bit and 32bit target?

[Gnimuc]

Yes, we should issue a warning whenever _emit_getproperty_ptr! is called. However, we only provide two options: generating a single target(unsafe) or generating all targets(extremely safe) that Julia currently runs on.