Skip to content

Fixed struct 'posix.system__struct_6652' has no member named 'MSF' compile error for other OS target #20833

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Fixed struct 'posix.system__struct_6652' has no member named 'MSF' compile error for other OS target #20833

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/std/debug.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ pub const StackIterator = struct {

const have_msync = switch (native_os) {
.wasi, .emscripten, .windows => false,
else => true,
else => posix.system.ucontext_t != void,
Copy link
Contributor

@rootbeer rootbeer Jul 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see how this fixes your problem, but I don't think msync existence should logically depend on ucontext_t existence.

I believe the approach going forward is supposed to be something more direct, like:

const have_msync = @TypeOf(posix.msync) != void;

But that won't work here (yet?) because posix.zig unilaterally defines an msync.... So you can either fix posix.zig to make that msync void when the underlying system.msync is missing, or just check for @TypeOf(posix.system.msync) != void in debug.zig, and then make sure the default system in posix.zig has a {} msync entry (see #20728 where I fixed a similar issue with freestanding use of debug.zig).

OTOH, the isValidMemory function in debug.zig that your backtrace goes through starts with:

        // We are unable to determine validity of memory for freestanding targets
        if (native_os == .freestanding or native_os == .uefi) return true;

Your native_os is "other", so maybe you can add "other" to this list here. Or, change your target to "freestanding"? (I'm not sure what the difference between "other" and "freestanding" targets is meant to be, so I'm not sure which of those might be the right suggestion.)

Overall, it seems worthwhile to make debug.zig a good example of how to do feature detection "correctly" in Zig, so cleaning it up to have a simple const have_msync = @TypeOf(posix.system.msync) != void (and fixing posix.zig to export msync symbols cleanly) is the right way to go, but I'm definitely not an authoritative source of such opinions.

EDIT: Added missing @TypeOf() on msync checks.

Copy link
Member

@alexrp alexrp Jul 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention with the other OS is that you can do the whole "bring your own OS" thing. See #3784. This is distinct from freestanding which is an environment where there explicitly is no OS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestions here were not quite as straightforward as I made it seem... I've put together a change at #20845. This should solve your specific problem (getting isValidMemory to compile on the other target OS), and I think it does so in the right way, and it includes a test too. But we'll have to wait and see if the Zig core folks think its good...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rootbeer #20845 fixes my issue and is definitely a higher quality fix than mine. Closing this PR

};

pub fn writeCurrentStackTrace(
Expand Down