runtime safety to detect branch on undefined

[srccde]

I've written a very simple argument "parser" that just reads in the first console argument given (after skipping argv[0]):

var code_file: ?[]u8 = undefined;

var args = std.process.args();
// Skip argv[0]
assert(args.skip() == true);
if (args.next(allocator)) |arg_err| {
    code_file = try arg_err;
}

if (code_file) |_| {} else {
    println("Usage: bfi [FILE]");
    return;
}

While rewriting some of the code I found that if I build with release-fast or release-small, the 2nd if-else{} is entirely optimized out.

Another check later on is also completely voided:

try intp.loadFile(code_file orelse return);

This will just try to load the file even if code_file is null.

Interestingly, I found out that if you used the variable code_file somewhere before the checks (e.g. print it to stdout), everything works as expected. Debug builds and release-safe are fine, too.

The build command is just simply "zig build", as well as, "zig build -Drelease-fast/small/safe" respectively, on a default created zig init-exe build.zig. The target is x86_64-linux-gnu, there's no actual dependency on libc though.
"zig version" is:

0.5.0+d6dec8026

The compiler was compiled in "ReleaseWithDebInfo" config mode.

[andrewrk]

The control flow of your logic, when there is no command line parameter passed, is:

var code_file: ?[]u8 = undefined;
if (code_file) |_| {} else {

This is a branch on undefined, which is Illegal Behavior (#2402). ReleaseSmall and ReleaseFast build modes assume no illegal behavior, which makes this Undefined Behavior in these modes.

Here's a fully self-contained example:

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

pub fn main() anyerror!void {
    var code_file: ?[]u8 = undefined;

    var args = std.process.args();
    // Skip argv[0]
    assert(args.skip() == true);
    if (args.next(std.heap.direct_allocator)) |arg_err| {
        code_file = try arg_err;
    }

    if (code_file) |_| {} else {
        std.debug.warn("Usage: bfi [FILE]\n");
        return;
    }
}

In safe builds, it compiles and outputs:

Usage: bfi [FILE]

It's planned for safe modes to detect this branch on undefined and output instead "branch on undefined value" plus a stack trace. This will turn Undetectable Illegal Behavior into Detectable Illegal Behavior.

I'll leave this issue open until this is solved. For your project, you want null, not undefined.

[srccde]

Well, one thing is certain: I'm never gonna mix up undefined and null again.

[andrewrk]

One more thing - Zig has valgrind integration with regards to undefined, so if you ran your debug build code with valgrind you will see the problem:

[nix-shell:~/Downloads/zig/build]$ valgrind ./test
==9367== Memcheck, a memory error detector
==9367== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9367== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==9367== Command: ./test
==9367== 
==9367== Conditional jump or move depends on uninitialised value(s)
==9367==    at 0x228627: main (test.zig:14)
==9367== 
Usage: bfi [FILE]
==9367== 
==9367== HEAP SUMMARY:
==9367==     in use at exit: 0 bytes in 0 blocks
==9367==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==9367== 
==9367== All heap blocks were freed -- no leaks are possible
==9367== 
==9367== Use --track-origins=yes to see where uninitialised values come from
==9367== For lists of detected and suppressed errors, rerun with: -s
==9367== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

[N00byEdge]

var i: i64 = undefined;

What about

if((i & 1) == 0) {
  i |= 3;
}

Should the above be considered Illegal Behavior?
What about

if((i & 1) == 0) {
  i |= 1;
}

which can be rewritten just as

i |= 1;

?

[ifreund]

Closing as a duplicate of #63