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

for loop on undefined variable falls into infinite loop (should throw an error or terminate immediately) #13934

Closed
20kano opened this issue Dec 14, 2022 · 14 comments
Labels
bug Observed behavior contradicts documented or intended behavior

Comments

@20kano
Copy link

20kano commented Dec 14, 2022

Zig Version

0.11.0-dev.753+331861161

Steps to Reproduce and Observed Behavior

const std = @import("std");
const print = std.debug.print;

pub fn main() !void {
	var w = Window.init();
	w.run();
}

pub const Window = struct {
    _layout: Layout = undefined,
    pub fn init() Window {
        var w = Window{};
        w._layout = Layout.init(&w);
        return w;
    }
    pub fn run(self: *Window) void {
		self._layout.draw();
    }
};

pub const Layout = struct{
    _view: ViewBase,
    _views: []IView = undefined,

    pub fn init(win: *Window) Layout{
        return Layout{
			._view = ViewBase.init(win)
        };
    }
    pub fn draw(self: Layout) void{
		print("{}\n", .{self._views.len});
		for (self._views)|v, i|{
			print("{},{}\n", .{v, i});
		}
    }
};

pub const ViewBase = struct {
    _window: *Window,
    pub fn init(win: *Window) ViewBase{
        return ViewBase{
			._window = win
		};
    }
};

pub const IView = struct {
    impl: *anyopaque,
    drawFn: *const fn (*anyopaque) void,
    pub fn draw(iface: *const IView) void {
        return iface.drawFn(iface.impl);
    }
};
> zig build run
12297829382473034410
Segmentation fault at address 0x0
/zigprojects/tradeapp/src/bug.zig:33:9: 0x20d6c5 in draw (tradeapp)
                        print("{},{}\n", .{v, i});
        ^
/zigprojects/tradeapp/src/bug.zig:17:20: 0x20c998 in run (tradeapp)
                self._layout.draw();
                   ^
/zigprojects/tradeapp/src/bug.zig:6:7: 0x20c8c8 in main (tradeapp)
        w.run();
      ^
/lang/zig/lib/std/start.zig:614:37: 0x20cdb9 in main (tradeapp)
            const result = root.main() catch |err| {
                                    ^

Expected Behavior

expected it prints like this

> zig build run
0

This works.

const std = @import("std");
const print = std.debug.print;

pub fn main() !void {
	var w = Window.init();
	w.run();
}

pub const Window = struct {
    _layout: Layout = undefined,
    pub fn init() Window {
        var w = Window{};
        w._layout = Layout.init(&w);
        return w;
    }
    pub fn run(self: *Window) void {
		self._layout.draw();
    }
};

pub const Layout = struct{
    _view: ViewBase,
    _views: []IView = undefined,

    pub fn init(win: *Window) Layout{
        return Layout{
			._view = ViewBase.init(win)
        };
    }
    pub fn draw(self: Layout) void{
		print("{}\n", .{self._views.len});
		for (self._views)|v, i|{
			print("{},{}\n", .{v, i});
		}
    }
};

pub const ViewBase = struct {
    // _window: *Window,
    pub fn init(win: *Window) ViewBase{
		_ = win;
        return ViewBase{
			// ._window = win
		};
    }
};

pub const IView = struct {
    impl: *anyopaque,
    drawFn: *const fn (*anyopaque) void,
    pub fn draw(iface: *const IView) void {
        return iface.drawFn(iface.impl);
    }
};

Also this.

const std = @import("std");
const print = std.debug.print;

pub fn main() !void {
	var w = Window.init();
	w.run();
}

pub const Window = struct {
    _layout: Layout = undefined,
    pub fn init() Window {
        var w = Window{};
        w._layout = Layout.init(&w);
        return w;
    }
    pub fn run(self: *Window) void {
		self._layout.draw();
    }
};

pub const Layout = struct{
    // _view: ViewBase,
    _views: []IView = undefined,

    pub fn init(win: *Window) Layout{
		_ = win;
        return Layout{
			// ._view = ViewBase.init(win)
        };
    }
    pub fn draw(self: Layout) void{
		print("{}\n", .{self._views.len});
		for (self._views)|v, i|{
			print("{},{}\n", .{v, i});
		}
    }
};

pub const ViewBase = struct {
    _window: *Window,
    pub fn init(win: *Window) ViewBase{
        return ViewBase{
			._window = win
		};
    }
};

pub const IView = struct {
    impl: *anyopaque,
    drawFn: *const fn (*anyopaque) void,
    pub fn draw(iface: *const IView) void {
        return iface.drawFn(iface.impl);
    }
};
@20kano 20kano added the bug Observed behavior contradicts documented or intended behavior label Dec 14, 2022
@IntegratedQuantum
Copy link
Contributor

Setting something to undefined means that the value can be absolutely anything.
In this case _views.len just so happens to be 12297829382473034410.

If you want _views.len to be 0 then you need to explicitely set it to zero.
This can for example be done in the init function like this:

    pub fn init(win: *Window) Layout{
        var result = Layout{
			._view = ViewBase.init(win)
        };
        result._views.len = 0;
        return result;
    }

@squeek502
Copy link
Collaborator

squeek502 commented Dec 14, 2022

Or, even better, initialize it to a zero length slice. This is what, for example, std.ArrayList does:

.items = &[_]T{},

In your case, you'd want:

    _views: []IView = &[_]IView{},

By the way, in the future, it'd be better to seek help with problems like this in one of the community spaces first: https://github.com/ziglang/zig/wiki/Community

@20kano
Copy link
Author

20kano commented Dec 14, 2022

Thanks, since this works as expected, then I thought this is just a bug.

const Foo = struct {
    arr: []u8 = undefined,
    fn foo(self: Foo) void {
        print("undefined arr.len = {}\n", .{self.arr.len});
        for (self.arr)|a|{
            print("{}\n", .{a});
        }
    }
};

test "array undefined" {
    const f= Foo{};
    f.foo();
}

@Vexu Vexu closed this as completed Dec 14, 2022
@20kano
Copy link
Author

20kano commented Dec 14, 2022

If you want _views.len to be 0 then you need to explicitely set it to zero.

My problem was not that I wanted len==0.
Commenting out the part that has nothing to do with it works correctly, as in the two examples below.
I considered this inconsistency to be a bug.

@tato
Copy link

tato commented Dec 14, 2022

Setting _views to undefined can make it be anything, even a zero-length slice. That makes your examples work, but only as a coincidence.

@20kano
Copy link
Author

20kano commented Dec 14, 2022

I see, I misunderstood that in the case of undefined, the for loop is supposed to terminate immediately.
That's why I get this became an infinite loop..

pub fn draw(self: Layout) void{
    for (self._views)|v, i|{
        _ = v;
       print("{}\n", .{i});
    }
}

Then I thought this is a undefined behavior of for loop.

This is because if an undefined variable represents nothing, it seems strange for a for loop to work on a variable that represents nothing.

The for loop should exit without executing the loop if the given variable is undefined, or should throw an error.

@20kano 20kano changed the title broken array for loop on undefined variable Dec 14, 2022
@20kano 20kano changed the title for loop on undefined variable for loop on undefined variable should throw an error or terminate immediately Dec 14, 2022
@20kano 20kano changed the title for loop on undefined variable should throw an error or terminate immediately for loop on undefined variable falls into infinite loop (should throw an error or terminate immediately) Dec 14, 2022
@mlugg
Copy link
Member

mlugg commented Dec 14, 2022

The for loop should exit without executing the loop if the given variable is undefined, or should throw an error.

undefined values can't be detected by the language at runtime: that's pretty much the whole point, it tells the language that you don't care what the value is for now. It's unrelated to undefined in languages like JS, where it's a distinct value you can detect. It should only be used when you're not going to use that value at all before later assigning it.

@20kano
Copy link
Author

20kano commented Dec 14, 2022

Thanks for the explanation.
I had seen this explanation and thought it was technically detectable.

In Debug mode, Zig writes 0xaa bytes to undefined memory. This is to catch bugs early, and to help detect use of undefined memory in a debugger.

@mlugg
Copy link
Member

mlugg commented Dec 14, 2022

I had seen this explanation and thought it was technically detectable.

That's a strategy to make it noticeable to humans while debugging, since you'll very rarely have that byte pattern in normal code, but it's still a valid value. You can set a u8 to the value 0xAA and it'll be indistinguishable from a debug-mode undefined.

@20kano
Copy link
Author

20kano commented Dec 14, 2022

I'm sorry, I may have misunderstood. Is it a for loop you are talking about?

@mlugg
Copy link
Member

mlugg commented Dec 14, 2022

I'm just talking about undefined values in general. In debug builds, Zig initializes all bytes of undefined memory to 0xAA - that's not so it can be accurately detected at runtime (because it can't! it's indistinguishable from an actual value of 0xAA), but instead so that if you're looking at values in a debugger, you'll notice this value in places you wouldn't expect it to be and find whatever bug you're looking for.
for loops aren't anything special here - looping over an undefined slice is undefined behaviour. In a debug build, it'll try to iterate over 0xAAAAAAAAAAAAAAAA elements starting at the address 0xAAAAAAAAAAAAAAAA, normally resulting in either a segfault (if you try to deref the pointer) or a seemingly-infinite loop (since that's a ridiculously large number of elements).

@Jarred-Sumner
Copy link
Contributor

Wouldn't this be a useful safety check in debug mode? Is there a situation where a for loop on an undefined value is desirable?

@Vexu
Copy link
Member

Vexu commented Dec 16, 2022

Wouldn't this be a useful safety check in debug mode? Is there a situation where a for loop on an undefined value is desirable?

It would be, that's why #211 exists.

@20kano
Copy link
Author

20kano commented Dec 16, 2022

    pub fn draw(self: Layout) void {
        for (self._views) |i| {
            _ = i;
        }
    }
pub const IView = packed struct {

btw, with packed, soon Segmentation fault, but without packed, infinite.

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
Projects
None yet
Development

No branches or pull requests

7 participants