crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.42k stars 1.62k forks source link

Binding to C flexible array members #10598

Open nickbclifford opened 3 years ago

nickbclifford commented 3 years ago

I was trying to work with a C library that has flexible array members declared in its structs and ran into trouble with using them in a lib declaration.

Given a hypothetical list.c:

typedef struct list {
  unsigned length;
  int data[];
} list_t;

list_t* new_list() {
    list_t* new = malloc(sizeof(list_t) + 8 * sizeof(int));
    new->length = 8;

    for (int i = 0; i <= 7; i++) {
        new->data[i] = i + 1;
    }

    return new;
}

Trying to treat it as a normal pointer causes a segfault:

@[Link(ldflags: "#{__DIR__}/list.o")]
lib LibList
    struct ListT
        length : LibC::UInt
        data : LibC::Int*
    end

    fun new_list : ListT*
end

ptr = LibList.new_list
list = ptr.value
p Slice.new(list.data, list.length)
$ gcc -c test.c -o test.o
$ crystal test.cr
Slice[Invalid memory access (signal 11) at address 0x300000002
[0x55a6ea68a1d6] *Exception::CallStack::print_backtrace:(Int32 | Nil) +118
[0x55a6ea67d9fc] __crystal_sigfault_handler +316
[0x7f3da8dc1960] ???
[0x55a6ea6e14b6] *Pointer(Int32) +6
[0x55a6ea6e8b5a] *Slice(Int32) +10
[0x55a6ea6e8ab2] *Slice(Int32) +418
[0x55a6ea6e8906] *Slice(Int32) +6
[0x55a6ea67da71] *p<Slice(Int32)>:Slice(Int32) +65
[0x55a6ea66fa6b] __crystal_main +1163
[0x55a6ea6e9ba6] *Crystal::main_user_code<Int32, Pointer(Pointer(UInt8))>:Nil +6
[0x55a6ea6e9a1c] *Crystal::main<Int32, Pointer(Pointer(UInt8))>:Int32 +44
[0x55a6ea67aee6] main +6
[0x7f3da8b81b25] __libc_start_main +213
[0x55a6ea66f50e] _start +46
[0x0] ???

Replacing the pointer with LibC::Int[] gives a parse failure.

crystal --version:

Crystal 1.0.0 (2021-03-29)

LLVM: 10.0.1
Default target: x86_64-pc-linux-gnu
asterite commented 3 years ago

Does that mean, in C, that the array is inlined in the struct?

lbguilherme commented 3 years ago

The array is inlined in the struct. There is no extra pointer. Maybe you could take the struct pointer, offset to that member and cast types. It will be safe.

asterite commented 3 years ago

This works:

@[Link(ldflags: "#{__DIR__}/list.o")]
lib LibList
  struct ListT
    length : LibC::UInt
    data : LibC::Int
  end

  fun new_list : ListT*
end

list_ptr = LibList.new_list
length = list_ptr.value.length
data = (list_ptr.as(LibC::UInt*) + 1).as(LibC::Int*)
p Slice.new(data, length)

It's not ideal but it gets the job done.

Alternatively I think pointerof(list_ptr.value.data) should work, but it currently triggers a compiler bug.

oprypin commented 3 years ago

This works as well

@[Extern]
struct ListT
  @length : LibC::UInt = 0
  @data : LibC::Int = 0

  def to_slice
    Slice.new(pointerof(@data), @length)
  end
end

@[Link(ldflags: "#{__DIR__}/list.o")]
lib LibList
  fun new_list : ListT*
end

list_ptr = LibList.new_list
p list_ptr.value.to_slice
asterite commented 3 years ago

It works!

Also this:

@[Link(ldflags: "#{__DIR__}/list.o")]
lib LibList
  struct ListT
    length : LibC::UInt
    data : LibC::Int
  end

  fun new_list : ListT*
end

struct LibList::ListT
  def to_slice
    Slice.new(pointerof(@data), @length)
  end
end

list_ptr = LibList.new_list
p list_ptr.value.to_slice
nickbclifford commented 3 years ago

Interesting, which should I use as a canonical solution? Frankly, now I wonder why declaring the member as a pointer itself doesn't work.

asterite commented 3 years ago

Because it's not a pointer. The array is inlined in the struct. Any of those solutions is fine, there's no canonical way for it.

asterite commented 3 years ago

Maybe we could make it inline with an annotation. Let's leave this open.

HertzDevil commented 3 years ago

If we follow C's semantics, the VLA member will be ignored by sizeof and assignments, so a closer approximation would be something like:

lib LibList
  struct ListT
    length : LibC::UInt
    data : LibC::Int[0]
  end
end

struct LibList::ListT
  def to_slice
    Slice.new(@data.to_unsafe, @length)
  end
end

If there is an annotation for VLA members we could also check for constructs not allowed in C (e.g. VLAs not the end of a lib struct, VLA-containing structs as members of other structs), but that isn't necessary.

nickbclifford commented 3 years ago

Just ran into this for some extern constants, appears to be the same issue, but @HertzDevil's solution works beautifully.

nickbclifford commented 3 years ago

Update: I seem to be having trouble with getting either @asterite's second solution or @HertzDevil's solution to work with more complex structs (specifically DeviceVersion and KeyblocksInfo here), although it works fine with these IndexedString struct globals.