ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
35.03k stars 2.56k forks source link

valgrind doesnt detect struct with undefined default field initializer #19148

Open nektro opened 8 months ago

nektro commented 8 months ago

Zig Version

0.12.0-dev.3142+9d500bda2

Steps to Reproduce and Observed Behavior

const std = @import("std");

pub fn main() !void {
    const alloc = std.heap.c_allocator;
    const int = try alloc.create(S);
    int.* = .{
        .b = 7,
    };

    if (int.a == 20) {
        std.debug.print("xxx\n", .{});
    } else {
        std.debug.print("yyy\n", .{});
    }
}

const S = struct {
    a: u32 = undefined,
    b: u32,
};
[meghan@nixos:~/src/zig]$ zig2 build-exe ./test2.zig -lc && nix-run valgrind ./test2
==778096== Memcheck, a memory error detector
==778096== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==778096== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==778096== Command: ./test2
==778096== 
yyy
==778096== 
==778096== HEAP SUMMARY:
==778096==     in use at exit: 8 bytes in 1 blocks
==778096==   total heap usage: 1 allocs, 0 frees, 8 bytes allocated
==778096== 
==778096== LEAK SUMMARY:
==778096==    definitely lost: 8 bytes in 1 blocks
==778096==    indirectly lost: 0 bytes in 0 blocks
==778096==      possibly lost: 0 bytes in 0 blocks
==778096==    still reachable: 0 bytes in 0 blocks
==778096==         suppressed: 0 bytes in 0 blocks
==778096== Rerun with --leak-check=full to see details of leaked memory
==778096== 
==778096== For lists of detected and suppressed errors, rerun with: -s
==778096== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Expected Behavior

==780046== Conditional jump or move depends on uninitialised value(s)
==780046==    at 0x102FD6A: test2.main (test2.zig:10)
mlugg commented 8 months ago

(Per #19149, it doesn't matter whether the undefined field is a default or explicit init.)

This happens only when the entire struct value is comptime-known. The problem is that logic in Sema.validateStructInit (and similarly for unions) is performing an optimization: if all RLS field stores are comptime-known, the entire struct is converted into a comptime constant. That's great for backends, since it gives them more freedom in lowering, but it's a case not correctly detected by the LLVM backend's undefined detection logic for valgrind.