Is it safe to access struct fields based on @offsetOf at runtime?

Help
1

When writing comptime code that is based on the structure of a struct I often find myself using a construction similar to this:

fn runtimeFieldPointer(ptr: anytype, comptime field_name: []const u8) *@TypeOf(@field(ptr.*, field_name)) {
    const field_offset = @offsetOf(@TypeOf(ptr.*), field_name);
    const base_ptr: [*]u8 = @ptrCast(ptr);
    return @ptrCast(@alignCast(&base_ptr[field_offset]));
}

All it does is uses @offsetOf to make a pointer to a field based on it’s name at runtime. It looks a bit crazy to me, but it works nicely in most cases I have thrown at it (here is an example where I use it to generate an imgui UI based on the structure of my Entity struct: gamedev-playground/src/internal/debug.zig at df6186e30ba6da170b6cae56c35fbcee9e9988e4 · zoeesilcock/gamedev-playground · GitHub). I have now run into an “Illegal instruction” error when trying to use this same code on a different struct (the game.State struct in that same project) but I’m not sure what the root of that is.

Is there anything obviously wrong with this function or some reason why it would be a bad idea? I feel like alignment could be an issue in some cases perhaps?

2

Why not use @field? (You already wrote the code, just to get the type of that value)

5 Likes
3

to make @dimdin’s point a little clearer, these two lines of code are attempting to do equivalent things.

const ptr = runtimeFieldPointer(parent, field_name);
const ptr = &@field(parent, field_name);

as you saw with your code, there are correctness issues (i would guess having to do with alignment) in the function you wrote.

4 Likes
4

To add onto the @field suggestion, it seems like you’d also be interested in @typeInfo and then inline for to iterate over the fields of a struct type. This is what std.fmt.format does, for example:

(info in the above here is @typeInfo(T).@"struct", [it uses a switch on @typeInfo(T)])

which allows formatted printing of arbitrary struct types:

const T = struct {
    hello: bool,
};
const Foo = struct {
    bar: u32,
    baz: T,
};
const foo = Foo{ .bar = 123, .baz = .{ .hello = false } };
std.debug.print("{}\n", .{foo});

which prints:

main.Foo{ .bar = 123, .baz = main.T{ .hello = false } }

This is probably not what you want exactly, but the concept (@typeInfo → inline for → @field) seems useful for what you’re doing.

EDIT: In re-reading the OP it seems like you probably know about this part, but no harm in mentioning it still I guess :upside_down_face:

1 Like
5

That’s a very good question. My guess would be that I came to @field while trying to implement getRuntimePointer and missed that it in fact was doing the exact thing I needed. Since many of these builtins are strictly comptime I probably assumed @field would give me a comptime pointer, but the docs are pretty clear so that’s a flimsy excuse at best.

I have replaced my custom function with calls to @field and it works in all the same places that my function did. This points to my current issue being unrelated.

I’m still getting an “Illegal instruction” error when using a pointer to @field, specifically for the bools in game.State which feels like a clue. Bools work fine on the Entity component fields, so I’m still stumped. I’ll keep investigating and post again with the exact code if I can’t make progress.

6

Could you post the full error? The most common cause of this is from the on-by-default-in-Debug-mode undefined behavior sanitizer detecting undefined behavior in a C/C++ dependency, but nowadays that should provide a proper error instead of just ‘illegal instruction’

1 Like
7

Sure, the full error looks like this:

Illegal instruction at address 0x7ffc1fe8b40c
...\gamedev-playground\src\dcimgui\dcimgui.cpp:915:0: 0x7ffc1fc3c62e in ImGui_Checkbox (dcimgui_sdl.lib)
    return ::ImGui::Checkbox(label, v);

...\gamedev-playground\src\internal\debug.zig:613:25: 0x7ffc1fc31dce in inputBool (playground.dll.obj)
    _ = c.ImGui_Checkbox(heading, value);
                        ^
...\gamedev-playground\src\internal\debug.zig:590:26: 0x7ffc1fc30d95 in inspectState (playground.dll.obj)
                inputBool(state_field.name, entry);
                         ^
...\gamedev-playground\src\internal\debug.zig:526:21: 0x7ffc1fc2e910 in drawDebugUI (playground.dll.obj)
        inspectState(state);
                    ^
...\gamedev-playground\src\game.zig:749:30: 0x7ffc1fc2d08c in draw (playground.dll.obj)
            debug.drawDebugUI(state);
                             ^
...\gamedev-playground\src\main.zig:99:17: 0x7ff6a86c4cef in main (gamedev-playground.exe.obj)
        gameDraw(state);
                ^
...\.zvm\0.14.1\lib\std\start.zig:635:28: 0x7ff6a86c53fa in main (gamedev-playground.exe.obj)
    return callMainWithArgs(@as(usize, @intCast(c_argc)), @as([*][*:0]u8, @ptrCast(c_argv)), envp);
                           ^
...\.zvm\0.14.1\lib\libc\mingw\crt\crtexe.c:266:0: 0x7ff6a86e8dbb in __tmainCRTStartup (crt2.obj)
    mainret = _tmain (argc, argv, envp);

...\.zvm\0.14.1\lib\libc\mingw\crt\crtexe.c:186:0: 0x7ff6a86e8e1b in mainCRTStartup (crt2.obj)
  ret = __tmainCRTStartup ();

???:?:?: 0x7ffcc59be8d6 in ??? (KERNEL32.DLL)
???:?:?: 0x7ffcc695c5db in ??? (ntdll.dll)
run
└─ run gamedev-playground failure
error: the following command exited with error code 3:
C:\Users\zoee\Developer\gamedev-playground\zig-out\bin\gamedev-playground.exe

When I use a debugger I can see that the line that causes the error is dereferencing the pointer that I passed into ImGui_Checkbox:

bool checked = *v;

I have compared the pointer address and it remains the same all the way and if I skip bools (there are two on the struct in question) all the remaining number and vector fields work perfectly, including writing changes back to the struct when the UI is changed.

1 Like
8

Is it possible that the issue is that you’re not initializing these boolean fields to valid values in game.init() and that they end up with their padding bits set to something other than the default (usually 0 but technically implementation defined), which will invoke UB in C/C++?

Whether or not it’s the cause, I’m a strong advocate for initializing allocated struct instances using foo.* = .{ ... } instead of assigning each field one by one, because it catches potential issues like this (forgetting to initialize fields) at compile time and also lets you take advantage of default field values. As a crutch you could instead use std.mem.zeroes() or std.mem.zeroInit() before assigning each field, but assigning the entire struct instance is still the preferred way to go.

5 Likes
9

That was it, well caught! This is a really fascinating detail about uninitialized variables that I wasn’t aware of.

Thanks for the suggestion about initializing, I usually prefer that syntax for struct inits, but didn’t use it here due to it being a pointer. I didn’t think of dereferencing the pointer, that will be much better than my current approach.

1 Like