Skip to content
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

translate-c: incorrect type for pointers to cv-qualified values #23329

Open
theofabilous opened this issue Mar 23, 2025 · 2 comments · May be fixed by #23394
Open

translate-c: incorrect type for pointers to cv-qualified values #23329

theofabilous opened this issue Mar 23, 2025 · 2 comments · May be fixed by #23394
Labels
bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport)

Comments

@theofabilous
Copy link

theofabilous commented Mar 23, 2025

Zig Version

0.15.0-dev.33+539f3effd

Steps to Reproduce and Observed Behavior

zig translate-c does not correctly translate pointer types when the pointee is cv-qualified in some cases. Unlike Zig, C can have const/volatile qualified values, so cv-qualified non-pointer types are translated without those qualifiers (as expected). However, pointers to such values do not always translate to correctly-qualified pointers, even if the qualified pointer type is representable in zig.

Example C code and zig translation that demonstrate this in various ways:

typedef volatile int mmio_int;
typedef mmio_int *mmio_int_ptr;
typedef struct {
    mmio_int reg;
} hw_t;

extern hw_t *hw;

static int hardware_read(void) {
    // volatile read, equivalent to *(&hw->reg)
    return hw->reg;
}

static int hardware_read_v2(void) {
    // volatile int *
    typeof(&hw->reg) ptr = &hw->reg;
    return *ptr;
}

static int hardware_read_v3(void) {
    // volatile int * (explicit)
    mmio_int_ptr ptr = &hw->reg;
    return *ptr;
}
pub const mmio_int = c_int;
pub const mmio_int_ptr = [*c]volatile mmio_int; // pointer to cv-qual value explicitly appears in source, ok
pub const hw_t = extern struct {
    reg: mmio_int = @import("std").mem.zeroes(mmio_int),
};
pub extern var hw: [*c]hw_t;
pub fn hardware_read() callconv(.c) c_int {
    // non-volatile read (should be volatile)
    return hw.*.reg;
}
pub fn hardware_read_v2() callconv(.c) c_int {
    // [*c]c_int (wrong type)
    var ptr: @TypeOf(&hw.*.reg) = &hw.*.reg;
    _ = &ptr;
    // non-volatile read (should be volatile)
    return ptr.*;
}
pub fn hardware_read_v3() callconv(.c) c_int {
    // correctly volatile-qualified
    var ptr: mmio_int_ptr = &hw.*.reg;
    _ = &ptr;
    // volatile read (ok)
    return ptr.*;
}

AFAIK this isn't esoteric C code - its pretty common in the embedded world (e.g. official arm CMSIS tooling generates this kind of thing).

Expected Behavior

Expected: Pointers to cv-qualified values in C-source code should be correctly translated.

It's hard to say what the exact output should be. For consistency with how other untranslateable C concepts are handled in translation, I would expect to see an explicit type-cast to the correct type whenever the address of a cv-qualified value-type is taken (directly or indirectly, ie. with & or ->), and would expect the type to appear without qualifiers when it appears a value-type (status quo). Something like this:

pub const mmio_int = c_int; // status quo: wrong, but expected because no equivalent concept exists in zig
pub const hw_t = extern struct {
    reg: mmio_int = @import("std").mem.zeroes(mmio_int),
};
pub extern var hw: [*c]hw_t;
pub fn hardware_read() callconv(.c) c_int {
    // now that we have obtained a pointer to the cv-qual value,
    // the correct type is representable: explicit cast.
    // c-translation engine knows this at this point.
    return @as([*c]volatile mmio_int, @ptrCast(&hw.*.reg)).*;
}
pub fn hardware_read_v2() callconv(.c) c_int {
    // ugly, but correct
    const ptr: @TypeOf(@as([*c]volatile mmio_int, @ptrCast(&hw.*.reg))) = @as([*c]volatile mmio_int, @ptrCast(&hw.*.reg));
    return ptr.*;
}

Another approach could be something like how flexible array struct members are handled, which is to say such types are supported by std.zig.c_translation somehow.


More specifically, I would expect internal code in the translation that uses pointers to volatile qualified value-types to be correctly typed. I think it should be left to the user (for now, at least) to properly handle pointer-qualifiers themselves when they consume the API, it's trivial to write some wrapper functions/types to handle that. But there is no easy user-land solution to diagnose internal usages of incorrectly-qualified pointers without having to literally rewrite those functions entirely. For HAL libraries and SDKs where such code is abundant (e.g. pico-sdk), there can be hundreds (if not thousands) of inline functions (i.e. function body is mis-translated by zig) using wrongly-qualified pointers that call into each other. So in that case, basically the entire code base would have to be manually rewritten.

@theofabilous theofabilous added the bug Observed behavior contradicts documented or intended behavior label Mar 23, 2025
@alexrp alexrp added the translate-c C to Zig source translation feature (@cImport) label Mar 23, 2025
@theofabilous
Copy link
Author

I'd be willing to work on this. I've marginally familiarized myself with the c-translation source code and have a primitive implementation that produces something similar to the example I've outlined in the "expected behavior" section, it's pretty straightforward and I don't think the full implementation would be particularly complex.

Just want to make sure this is the desired solution before sinking time into this approach.

@Vexu
Copy link
Member

Vexu commented Mar 27, 2025

As long as there is a proper cast to a volatile pointer type it should be good. mmio_int could also automatically be renamed to volatile_mmio_int or something to remind how it is supposed to be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants