zig icon indicating copy to clipboard operation
zig copied to clipboard

Switching on pointer segfaults the compiler

Open hollmmax opened this issue 5 years ago • 10 comments

The following example segfaults during compilation on both 0.7.0 and master.

pub fn main() !void {
    var foo = "a";
    switch (foo) {
        "a" => {},
        "b" => {},
        else => {},
    }
}

Regardless whether this should be valid Zig, it shouldn't cause a segfault without any error report. Do note that this example compiles and works if one of the string branches is removed. What is the official stance on slice matching with switch?

hollmmax avatar Dec 09 '20 05:12 hollmmax

Your example is not a slice, it is trying to switch on a pointer-to-array.

daurnimator avatar Dec 09 '20 05:12 daurnimator

It seems to be pointers in general that have this problem. This segfaults too:

pub fn main() !void {
    const i: i32 = 0;
    const j: i32 = 1;
    const k: i32 = 2;

    var foo = &i;
    const a = &j;
    const b = &k;

    switch (foo) {
        a => {},
        b => {},
        else => {},
    }
}

ghost avatar Dec 09 '20 05:12 ghost

It seems to be pointers in general that have this problem.

LLVM doesn't like switching on anything but integral types, I tried to extend this to pointers (see ""switch on pointer type" test in behavior/switch.zig) but that works only for non-relocatable pointers.

We can always lower this kind of switch using some custom LLVM IR if switching on pointers is wanted.

cc @spexguy: the language specification doesn't cover what types switch works with.

LemonBoy avatar Dec 09 '20 08:12 LemonBoy

Your example is not a slice, it is trying to switch on a pointer-to-array.

You're correct. I'm used to slices being valid only as pointer-to, therefor I use the terms interchangeably. Title changed.

hollmmax avatar Dec 09 '20 09:12 hollmmax

the language specification doesn't cover what types switch works with.

Good point.

My gut reaction is to say it should only work for bool, int, comptime_int, error, enum, enum literal, tagged union, and probably optional containing any of the defined types. There's a potential argument for allowing float and comptime_float, and maybe void and type. These rules make sense based on how switch is meant to be used. Switch labels must be comptime known, and it's a pretty weird use case to switch on one of a set of comptime known pointers. Since comptime-known pointers aren't always actually comptime known, there is the possibility that two extern symbols have the same address and the compiler will not catch that two cases have the same condition.

That said, the simple definition says that it should work for any type for which == is defined and evaluates to bool. So type, void, bool, int, float, non-slice pointer, comptime_float, comptime_int, null, error, enum, tagged union, fn, frame, anyframe, vector, enumliteral, or an optional containing any of these types. Basically everything except noreturn, array, struct, undefined, untagged union, slice pointer, error union, bound fn, vector, and opaque. In practice frame and anyframe can't be comptime known so they are effectively disallowed.

I'd have to defer to @andrewrk to decide between these two general options.

P.S. I don't think == is defined on error unions or noreturn but I might be wrong about this. If so they would be included in the second option.

Edit: Add enum literal to the first option, that is pretty unambiguous and makes sense.

SpexGuy avatar Dec 09 '20 09:12 SpexGuy

My preference is for the second option minus float and comptime_float, adding floating point numbers in the mix is not worth it IMO.

In practice frame and anyframe can't be comptime known so they are effectively disallowed.

Is == actually defined for those types?

LemonBoy avatar Dec 09 '20 09:12 LemonBoy

Ah, you're right, it's not defined for frame. I think it's defined for anyframe, since that's a pointer. But I haven't tested it.

SpexGuy avatar Dec 09 '20 09:12 SpexGuy

Matching on float using == doesn't really make sense, but it would be useful with ranges. Although I would like to be able to match slices, I understand it would go against the Zen of Zig.

hollmmax avatar Dec 09 '20 09:12 hollmmax

Imho we should allow float as matching to ranges would be quite nice, also switching on type makes generics way more readable, i use that a lot

ikskuh avatar Dec 09 '20 10:12 ikskuh

trying ghost's example yields the following trace on 0.12.0-dev.1836+dd189a354

zig: /home/meghan/src/llvm-17/llvm/include/llvm/Support/Casting.h:578: decltype(auto) llvm::cast(From*) [with To = ConstantInt; From = Value]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
Aborted (core dumped)

#0  0x00007fffea4a1adc in __pthread_kill_implementation () from /nix/store/dg8mpqqykmw9c7l0bgzzb5znkymlbfjw-glibc-2.37-8/lib/libc.so.6
#1  0x00007fffea452cb6 in raise () from /nix/store/dg8mpqqykmw9c7l0bgzzb5znkymlbfjw-glibc-2.37-8/lib/libc.so.6
#2  0x00007fffea43c8ba in abort () from /nix/store/dg8mpqqykmw9c7l0bgzzb5znkymlbfjw-glibc-2.37-8/lib/libc.so.6
#3  0x00007fffea43c7d9 in __assert_fail_base.cold () from /nix/store/dg8mpqqykmw9c7l0bgzzb5znkymlbfjw-glibc-2.37-8/lib/libc.so.6
#4  0x00007fffea44b9c6 in __assert_fail () from /nix/store/dg8mpqqykmw9c7l0bgzzb5znkymlbfjw-glibc-2.37-8/lib/libc.so.6
#5  0x00007fffeb2d4b03 in llvm::cast<llvm::ConstantInt, llvm::Value> (Val=<optimized out>) at /home/meghan/src/llvm-17/llvm/include/llvm/Support/Casting.h:578
#6  0x00007fffeb2e19b6 in llvm::cast<llvm::ConstantInt, llvm::Value> (Val=<optimized out>) at /home/meghan/src/llvm-17/llvm/lib/IR/Core.cpp:3226
#7  llvm::unwrap<llvm::ConstantInt> (P=<optimized out>) at /home/meghan/src/llvm-17/llvm/include/llvm/IR/Value.h:1039
#8  LLVMAddCase (Switch=<optimized out>, OnVal=<optimized out>, Dest=<optimized out>) at /home/meghan/src/llvm-17/llvm/lib/IR/Core.cpp:3226
#9  0x00000000018b0a66 in codegen.llvm.Builder.WipFunction.WipSwitch.addCase (self=0x7ffffffeaa58, val=(none | unknown: 0x4), dest=(unknown: 0x3), wip=0x7ffffffee478) at /home/meghan/src/zig/src/codegen/llvm/Builder.zig:5024
#10 0x00000000018af657 in codegen.llvm.FuncGen.airSwitchBr (self=0x7ffffffee400, inst=(unknown: 0x27)) at /home/meghan/src/zig/src/codegen/llvm.zig:5970
#11 0x0000000001359d5c in codegen.llvm.FuncGen.genBody (self=0x7ffffffee400, body=...) at /home/meghan/src/zig/src/codegen/llvm.zig:4961
#12 0x00000000018aba2e in codegen.llvm.FuncGen.airBlock (self=0x7ffffffee400, inst=(unknown: 0x17)) at /home/meghan/src/zig/src/codegen/llvm.zig:5758
#13 0x0000000001359c4a in codegen.llvm.FuncGen.genBody (self=0x7ffffffee400, body=...) at /home/meghan/src/zig/src/codegen/llvm.zig:4959
#14 0x00000000013540f6 in codegen.llvm.Object.updateFunc (o=0x35bfac0, mod=0x35be3f8, func_index=304, air=..., liveness=...) at /home/meghan/src/zig/src/codegen/llvm.zig:1648
#15 0x0000000001367cd7 in link.Elf.updateFunc (self=0x35beff0, mod=0x35be3f8, func_index=304, air=..., liveness=...) at /home/meghan/src/zig/src/link/Elf.zig:3174
#16 0x000000000101abc3 in link.File.updateFunc (base=0x35bf240, module=0x35be3f8, func_index=304, air=..., liveness=...) at link.zig:624
#17 0x0000000000d99c7b in Module.ensureFuncBodyAnalyzed (mod=0x35be3f8, func_index=304) at Module.zig:3390
#18 0x0000000000d9762e in Compilation.processOneJob (comp=0x35bcfe0, job=..., prog_node=0x7fffffff12a8) at Compilation.zig:3761
#19 0x0000000000b69529 in Compilation.performAllTheWork (comp=0x35bcfe0, main_progress_node=0x7fffffff12a8) at Compilation.zig:3698
#20 0x0000000000b64d83 in Compilation.update (comp=0x35bcfe0, main_progress_node=0x7fffffff12a8) at Compilation.zig:2505
#21 0x0000000000b95483 in main.updateModule (comp=0x35bcfe0) at main.zig:4283
#22 0x0000000000bb8043 in main.buildOutputType (gpa=..., arena=..., all_args=..., arg_mode=...) at main.zig:3684
#23 0x00000000009c18c8 in main.mainArgs (gpa=..., arena=..., args=...) at main.zig:284
#24 0x00000000009be8c6 in main.main () at main.zig:222

nektro avatar Dec 21 '23 01:12 nektro

I am not ready to decide on a full list but the language should allow switch on pointers, because this use case is perfectly valid:

const special_ptr_foo: *const T = @ptrFromInt(0x1);
const special_ptr_bar: *const T = @ptrFromInt(0x2);

fn baz(x: *T) void {
    switch (x) {
        special_ptr_foo => { ... },
        special_ptr_bar => { ... },
        else => { ... },
    }
}

andrewrk avatar Jan 10 '24 07:01 andrewrk

@andrewrk I know the 0.12.0 release is near, but could this feature be added to the 0.12.0 release in case you decide to include it in the language? It would be GREAT to have the ability to switch over strings

Ev1lT3rm1nal avatar Mar 31 '24 19:03 Ev1lT3rm1nal

Switching over strings is not going to be a feature of the language.

Vexu avatar Mar 31 '24 20:03 Vexu