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

design flaw: undefined zig pointers: are they non-null or not? #3325

Open
andrewrk opened this issue Sep 27, 2019 · 5 comments
Open

design flaw: undefined zig pointers: are they non-null or not? #3325

andrewrk opened this issue Sep 27, 2019 · 5 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior.
Milestone

Comments

@andrewrk
Copy link
Member

This issue is not related to allowzero.

A Zig pointer, such as *i32 or [*]u8, is guaranteed to be non-null. Optional pointers must be used to gain null as a possible value: ?*i32 or ?[*]u8. So we put the nonnull LLVM attribute on functions. However, that makes this simple Zig code undefined behavior, even in debug mode:

pub fn main() void {
    var not_even_used: *i32 = undefined;
    foo(not_even_used);
}

fn foo(not_even_used: *i32) void {}

Because not_even_used is undefined, that means it could be 0 which violates the nonnull attribute. This could be solved by not putting nonnull attribute on pointer parameters, though it would be unfortunate since most of the time nonnull is correct.

It's also a question of whether to put nonnull on loads, stores, and other places having to do with Zig pointers.

The more I think about it, the more I think the answer is that Zig pointers have a slightly different property than nonnull. The property is nonnull_or_undefined. This is not an LLVM concept that currently exists, but I have a gut instinct that it should. Any time that the nonnull attribute would allow the optimizer to do anything useful, if the value were nonnull_or_undefined it should allow the optimizer to do the same thing. The only issue is that undefined is violating nonnull.

Related:

I promise that the above Zig code snippet will not make it into the 1.0 language specification (#75) as undefined behavior. This will be resolved some way or another.

@andrewrk andrewrk added the breaking Implementing this issue could cause existing code to no longer compile or have different behavior. label Sep 27, 2019
@andrewrk andrewrk added this to the 0.6.0 milestone Sep 27, 2019
SpexGuy added a commit to SpexGuy/Zig-Vulkan-Headers that referenced this issue Feb 14, 2020
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Mar 12, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 17, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 Jun 4, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 24, 2021
@jlombera
Copy link

Slices are also affected by this. Following code causes segmentation fault at runtime instead of an out-of-bounds panic in debug mode:

fn foo(slice: []u8) void {
    const x = slice[0];
    _ = x;
}

pub fn main() void {
    var slice: []u8 = undefined;
    foo(slice);
}

I guess, in general, undefined is not safe around anything involving pointers at the moment.

@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@atmnk
Copy link

atmnk commented Mar 17, 2024

Not sure if this could be related but following code leads to seg fault with undefined pointer. Just exploring zig.

const Complex = struct {
    real: f32,
    imaginary: f32,
    fn add(self:Complex,other:Complex) Complex{
        return Complex{
            .real = self.real+other.real,
            .imaginary = self.imaginary+other.imaginary
        };
    }
};
test "simple test" {
    var a:*Complex=undefined;
    a.real = 1.0;
    const b =Complex{
        .real = 1.0,
        .imaginary = 1.0,
    };
    const c= b.add(a.*);
    try std.testing.expect(c.real==1.0);
    try std.testing.expect(c.imaginary==1.0);
}

The output
1/1 main.test.simple test... Segmentation fault at address 0xaaaaaaaaaaaaaaaa Panicked during a panic. Aborting. error: the following test command crashed: /Users/atmaramnaik/work/practice/zig/day3/zig-cache/o/718a27f2b8f12de912fa6156d0c7f660/test

@nektro
Copy link
Contributor

nektro commented Mar 17, 2024

yes that segfault is expected given your code

@nektro
Copy link
Contributor

nektro commented Mar 17, 2024

is there a use case where having an undefined pointer ever makes sense? could assigning undefined to a pointer be make a compile error? (sure there would be ways to trick it but it would make that it is illegal behavior a lot more obvious)

@InKryption
Copy link
Contributor

Generally speaking I think the way a pointer becomes undefined is most frequently as a field of a struct in an allocated slice, ie try allocator.alloc(struct { foo: []u8 }, n) - so, @memset & aliasing via @ptrCast. There could also possibly some generic code that initializes a value of a specified type T to undefined, which is initialized by a callback with some parameters.
I think it would be more useful to make it runtime illegal behaviour to dereference an undefined pointer, via usage of special values in safe modes (ie ptr=0). There's a proposal for that, however I can't find it at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior.
Projects
None yet
Development

No branches or pull requests

5 participants