Skip to content

Presumable compiler bug? A segfault that's easily circumvented by minimally re-arranging stuff without changing code meaning at all: #4228

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

Closed
metaleap opened this issue Jan 18, 2020 · 4 comments
Labels
bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@metaleap
Copy link
Contributor

metaleap commented Jan 18, 2020

(Using nightly zig-linux-x86_64-0.5.0+b72f85819 from 2020-01-18.)

Behold, a maximally minimal (really!) zig runnable main.zig repro case, extracted from bigger project --- but first, skip down to the upshot / short description just below this 88-line snippet:

const std = @import("std");

pub fn main() !void {
    var mem = std.heap.ArenaAllocator.init(std.heap.page_allocator);
    defer mem.deinit();

    const list = try listFromStr(&mem.allocator, "Hail Zig =)");
    std.debug.warn("{s}", .{try listToStr(&mem.allocator, list)});
}

fn listFromStr(mem: *std.mem.Allocator, from: []const u8) !Expr {
    var ret = Expr{ .FuncRef = @enumToInt(StdFunc.Nil) }; // all type defs at bottom of snippet
    var i = from.len;
    while (i > 0) {
        i -= 1;
        ret = Expr{
            .Call = try enHeap(mem, ExprCall{
                .Callee = Expr{ .FuncRef = @enumToInt(StdFunc.Cons) },
                .Args = try std.mem.dupe(mem, Expr, &[_]Expr{ ret, Expr{ .NumInt = from[i] } }),
            }),
        };
    }
    return ret;
}

fn listToStr(mem: *std.mem.Allocator, expr: Expr) !?[]const u8 {
    const maybenumlist = try maybeList(mem, expr);
    return if (maybenumlist) |it| listToBytes(mem, it) else null;
}

fn maybeList(mem: *std.mem.Allocator, self: Expr) !?[]const Expr {
    var list = try std.ArrayList(Expr).initCapacity(mem, 1024);
    errdefer list.deinit();
    var next = self;
    while (true) {
        switch (next) {
            .FuncRef => |f| if (f == @enumToInt(StdFunc.Nil))
                return list.toOwnedSlice(),
            .Call => |c| if (c.Args.len == 2) switch (c.Callee) {
                .FuncRef => |f| if (f == @enumToInt(StdFunc.Cons)) {
                    try list.append(c.Args[1]);
                    next = c.Args[0];
                    continue;
                },
                else => {},
            },
            else => {},
        }
        break;
    }
    list.deinit();
    return null;
}

fn listToBytes(mem: *std.mem.Allocator, maybeNumList: []const Expr) !?[]const u8 {
    var ok = false;
    const bytes = try mem.alloc(u8, maybeNumList.len);
    defer if (!ok) mem.free(bytes);
    for (maybeNumList) |expr, i| switch (expr) {
        .NumInt => |n| if (n < 0 or n > 255) return null else bytes[i] = @intCast(u8, n),
        else => return null,
    };
    ok = true;
    return bytes;
}

inline fn enHeap(mem: *std.mem.Allocator, it: var) !*@TypeOf(it) {
    var ret = try mem.create(@TypeOf(it));
    ret.* = it;
    return ret;
}

const ExprCall = struct {
    Callee: Expr,
    Args: []Expr,
};

Expr = union(enum) {
    NumInt: isize,
    FuncRef: isize,
    Call: *const ExprCall,
};

const StdFunc = enum(isize) {
    Nil = 3,
    Cons = 4,
};

With the above, doing zig run main.zig:

Segmentation fault at address 0x1b
~/tmp/repros/segfault_2020jan18/main.zig:39:41: 0x22bfbe in maybeList (run)
            .Call => |c| if (c.Args.len == 2) switch (c.Callee) {
                                        ^

Now to circumvent this (until fixed/explained =), from what I have found one has 2 options, either one by itself alone will already do the trick:

  • either "manually inline" the already-auto-inlined fn enHeap into the callsite directly / by hand
  • or line 19: take out the expression assigned to .Args = , put it between line 15 & 16 to be assigned to a new local const tmpargs = ..., then use that local for the .Args = initializer

Both ways the program will work as expected, printing out the original input string from line 7. These "fixes" are mere re-arrangements / different placements without (AFAICT) any semantic differences. (So shouldn't have to dig around for such silly workarounds =)

@metaleap metaleap changed the title Presumable compiler bug? A segfault on the == in foo.len == 2 that's easily circumvented by minimally re-arranging stuff without changing code meaning at all: Presumable compiler bug? A segfault that's easily circumvented by minimally re-arranging stuff without changing code meaning at all: Jan 18, 2020
@andrewrk andrewrk added this to the 0.7.0 milestone Jan 28, 2020
@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend. labels Jan 28, 2020
@andrewrk
Copy link
Member

andrewrk commented Jan 28, 2020

                    next = c.Args[0];

If you didn't want next and c to alias each other here, you will have to introduce a manual copy. Can't remember right now where is the canonical issue for this. This is not a stable part of the language yet (meaning whether or not c is a copy might change).

@metaleap
Copy link
Contributor Author

If you didn't want next and c to alias each other here, you will have to introduce a manual copy.

Thanks for the reminder! In the above case c would always be a (heap not stack) pointer because the Expr.Call union-member is one. Unless word-sized ints and addresses can get potentially-"pointerized" behind the curtains currently..

@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 Nov 6, 2020
@SpexGuy
Copy link
Contributor

SpexGuy commented Apr 20, 2021

I think this is now always a copy, can this be closed?

@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 May 19, 2021
@Vexu
Copy link
Member

Vexu commented Dec 30, 2022

Fixed by #4454

@Vexu Vexu closed this as completed Dec 30, 2022
@andrewrk andrewrk modified the milestones: 0.12.0, 0.11.0 Dec 30, 2022
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 miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

No branches or pull requests

4 participants