it is easy confuse CLI options / command line parameters -D and -O when switching between zig build and zig build-exe

[sno2]

Zig Version

0.10.0-dev.3445

Steps to Reproduce

Access the Godbolt playground for the following code or compile on your local and observe using a disassembler:

const std = @import("std");

pub fn main() !void {
    var message = try std.heap.page_allocator.dupe(u8, "Hello, world!");
    std.heap.page_allocator.destroy(message.ptr);
    message.ptr.* = undefined;
}

Compile using -DReleaseFast.

Expected Behavior

Assigning pointers to undefined should be optimized out in ReleaseFast. Assigning pointers to undefined is extremely useful to catch use after free UB in development, especially in more complicated deinit() functions to make access to the self after destruction fail. However, it can be a pointless hit to performance when it is not optimized out in production.

Actual Behavior

Compiled with -DReleaseFast the relevant assembly code is the following:

main:
        push    rbp
        mov     rbp, rsp
        sub     rsp, 128
        mov     rsi, rdi
        mov     qword ptr [rbp - 112], rsi
        lea     rdi, [rbp - 80]
        movabs  rdx, offset __unnamed_21
        movabs  rcx, offset __unnamed_22
        call    std.mem.Allocator.dupe
        cmp     word ptr [rbp - 64], 0
        je      .LBB62_2
        mov     rdi, qword ptr [rbp - 112]
        mov     ax, word ptr [rbp - 64]
        mov     word ptr [rbp - 114], ax
        mov     word ptr [rbp - 50], ax
        call    __zig_return_error
        mov     ax, word ptr [rbp - 114]
        add     rsp, 128
        pop     rbp
        ret
.LBB62_2:
        mov     rax, qword ptr [rbp - 80]
        mov     qword ptr [rbp - 96], rax
        mov     rax, qword ptr [rbp - 72]
        mov     qword ptr [rbp - 88], rax

        ; std.heap.page_allocator.destroy(message.ptr);
        mov     rsi, qword ptr [rbp - 96]
        mov     qword ptr [rbp - 104], rsi
        movabs  rdi, offset __unnamed_23
        call    std.mem.Allocator.destroy

        ;  message.ptr.* = undefined;
        mov     rdi, qword ptr [rbp - 96]
        mov     qword ptr [rbp - 128], rdi
        mov     esi, 170
        mov     edx, 1
        call    memset@PLT
        mov     rax, qword ptr [rbp - 128]
        mov     qword ptr [rbp - 48], 1296236545
        mov     qword ptr [rbp - 40], rax
        mov     qword ptr [rbp - 32], 1
        mov     qword ptr [rbp - 24], 0
        mov     qword ptr [rbp - 16], 0
        mov     qword ptr [rbp - 8], 0
        lea     rax, [rbp - 48]
        xor     ecx, ecx
        mov     edx, ecx
        rol     rdi, 3
        rol     rdi, 13
        rol     rdi, 61
        rol     rdi, 51
        xchg    rbx, rbx

        mov     word ptr [rbp - 50], 0
        xor     eax, eax
        add     rsp, 128
        pop     rbp
        ret

Note how it still included the assembly for the message.ptr.* = undefined instruction.

[andrewrk]

Usage:

  -O [mode]                 Choose what to optimize for
    Debug                   (default) Optimizations off, safety on
    ReleaseFast             Optimizations on, safety off
    ReleaseSafe             Optimizations on, safety on
    ReleaseSmall            Optimize for small binary, safety off
...
  -D[macro]=[value]         Define C [macro] to [value] (1 if [value] omitted)

Sorry for the confusion, -D is used when using zig build but means something different when using the CLI.

Leaving the issue open to track this problem.

[sno2]

Ah that makes more sense. Thanks for the quick response

[mlugg]

The more general issue here is that the meaning of -D is overloaded between zig build and zig build-{exe,obj,lib}. The ideal solution would be to rename zig build's -D option to use some other character - importantly, one which is unused by build-exe etc, so they can emit errors for it.

[xdBronch]

was helping someone earlier and came across this problem (yet again), i think a potential solution would be to only allow the C style -Dmacro=value inside of the CLIs -cflags <flags> -- system. potential problem with that would be that that only applies to C file that are passed after the -cflags, maybe a warning could be issued if a user passes -cflags but no subsequent C files