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

Small structs passed by value to a C library function contain address of value instead #1411

Closed
hcff opened this Issue Aug 24, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@hcff
Contributor

hcff commented Aug 24, 2018

I got this problem when using SDL2_ttf and was wondering why the surface returned by TTF_RenderUTF8_Solid(doc) used a random color for the text instead of the one I gave.

Here's a C library that show the problem :

  • libcolor.h
#include <stdint.h>

struct Color {
    uint8_t r;
    uint8_t g;
    uint8_t b;
    uint8_t a;
};

void printColor(struct Color col);
void printColorPtr(const struct Color* col);
  • libcolor.c :
#include "libcolor.h"
#include <stdio.h>

void printColor(struct Color col) {
    printf("C Color : %02x %02x %02x %02x\n", col.r, col.g, col.b, col.a);
}

void printColorPtr(const struct Color* col) {
    printf("C Color ptr : %02x %02x %02x %02x\n", col->r, col->g, col->b, col->a);
}

And a zig program that use this library :

  • uselib.zig
const std = @import("std");
const c = @cImport({ @cInclude("libcolor.h"); });

pub fn main() void {
    const col = c.Color{ .r = 0x12, .g = 0x34, .b = 0x56, .a = 0x78 };
    std.debug.warn("zig Color : {x2} {x2} {x2} {x2}\n", col.r, col.g, col.b, col.a);
    std.debug.warn("address of Color : {x8}\n", @ptrToInt(&col));

    c.printColor(col);
    c.printColorPtr(@ptrCast([*]const c.Color, &col));
}

The three lines "zig Color", "C Color" and "C Color ptr" should all have "12 34 56 78", right ?
Here's what I got on my system (Fedora 28 x86-64, zig version 0.2.0+4b68ef45) :

 $ gcc -Wall -fPIC -shared libcolor.c -o libcolor.so
 $ zig build-exe uselib.zig -isystem . --library color --library-path .
 $ ./uselib
zig Color : 12 34 56 78
address of Color: 00204480
C Color : 80 44 20 00
C Color ptr : 12 34 56 78

It seems passing a pointer to the struct works fine, but if you try to pass the struct itself, zig will silently pass the pointer instead.
Except if you replace uint8_t with uint64_t (and %02x with %02lx), then it works fine, and the C library does get the value of col :

zig Color : 12 34 56 78
address of Color : 002044a0
C Color : 12 34 56 78
C Color ptr : 12 34 56 78
@isaachier

This comment has been minimized.

Contributor

isaachier commented Aug 24, 2018

@hcff

This comment has been minimized.

Contributor

hcff commented Aug 24, 2018

If a type is a container, error union, optional type, slice, or array, then its handle is a pointer, and everywhere we refer to a value of this type we refer to a pointer.
Parameters and return values are always passed as handles.

For Zig functions, sure. But printColor is an extern function, so it should use the C ABI.
In C, structs can be passed to a function by value. And TTF_RenderUTF8_Solid, for example, require the SDL_Color struct to be given this way.
And according to the documentation, Zig is supposed to let you pass a struct by value to an extern function : https://ziglang.org/documentation/master/#Pass-by-value-Parameters

For extern functions, Zig follows the C ABI for passing structs and unions by value.

Here, the Zig program is passing the address of the struct, while the C library is expecting the struct itself.
So it's impossible to give the color you want to printColor.

@andrewrk andrewrk added this to the 0.3.0 milestone Aug 24, 2018

@andrewrk andrewrk added the bug label Aug 24, 2018

@andrewrk

This comment has been minimized.

Member

andrewrk commented Aug 24, 2018

This is a really nasty bug. My apologies.

@andrewrk andrewrk referenced this issue Sep 6, 2018

Open

stage1 C ABI compatibility #1481

6 of 11 tasks complete

andrewrk added a commit that referenced this issue Sep 7, 2018

stage1: compile error instead of incorrect code
for unimplemented C ABI

See #1411
See #1481

@andrewrk andrewrk closed this in 7505529 Sep 7, 2018

@andrewrk

This comment has been minimized.

Member

andrewrk commented Sep 7, 2018

See #1481 for more details on the state of C ABI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment