GeneralPurposeAllocator passes uninitialized bytes to msync()

j-hui commented 1 year ago

Zig Version

0.11.0-dev.2892+fd6200eda

Steps to Reproduce and Observed Behavior

Initialize a project with zig init-exe, and build the following program with zig build:

// main.zig
const std = @import("std");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    defer _ = gpa.deinit();
    const allocator = gpa.allocator();
    const args = try std.process.argsAlloc(allocator);
    defer std.process.argsFree(allocator, args);
    std.debug.print("Args: {s}\n", .{args});
}

and then execute it with Valgrind:

$ valgrind ./zig-out/bin/ziggy
==2780956== Memcheck, a memory error detector
==2780956== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2780956== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==2780956== Command: ./zig-out/bin/ziggy
==2780956==
==2780956== Syscall param msync(start) points to uninitialised byte(s)
==2780956==    at 0x24CEBB: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x25CC28: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x2530AA: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x24DAC6: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x23F51B: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x22FE8E: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x232FE5: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x24614D: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x233F6D: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x245C77: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x231AD7: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x231638: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==  Address 0x1ffefff000 is on thread 1's stack
==2780956==
==2780956== Syscall param msync(start) points to unaddressable byte(s)
==2780956==    at 0x24CEBB: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x25CC28: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x2530AA: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x24DAC6: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x23F51B: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x22FE8E: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x232FE5: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x24614D: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x233F6D: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x2339CF: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x2120CD: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==    by 0x20E2F3: ??? (in /home/j-hui/ziggy/zig-out/bin/ziggy)
==2780956==  Address 0x1ffefff000 is on thread 1's stack
==2780956==  680 bytes below stack pointer
==2780956==
Args: { ./zig-out/bin/ziggy }
==2780956==
==2780956== HEAP SUMMARY:
==2780956==     in use at exit: 0 bytes in 0 blocks
==2780956==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==2780956==
==2780956== All heap blocks were freed -- no leaks are possible
==2780956==
==2780956== Use --track-origins=yes to see where uninitialised values come from
==2780956== For lists of detected and suppressed errors, rerun with: -s
==2780956== ERROR SUMMARY: 63 errors from 2 contexts (suppressed: 0 from 0)

Expected Behavior

No memory errors (:

squeek502 commented 1 year ago

Here's the stack trace with proper debug info (see https://github.com/ziglang/zig/issues/15254):

==3189443== Syscall param msync(start) points to unaddressable byte(s)
==3189443==    at 0x24DD9B: os.linux.x86_64.syscall3 (x86_64.zig:46)
==3189443==    by 0x25D6F8: os.linux.msync (linux.zig:432)
==3189443==    by 0x25417A: os.msync (os.zig:4454)
==3189443==    by 0x24E9E6: debug.StackIterator.isValidMemory (debug.zig:496)
==3189443==    by 0x23FF7B: debug.StackIterator.next_internal (debug.zig:539)
==3189443==    by 0x2307CE: debug.StackIterator.next (debug.zig:475)
==3189443==    by 0x233915: debug.captureStackTrace (debug.zig:239)
==3189443==    by 0x246C53: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).collectStackTrace (general_purpose_allocator.zig:456)
==3189443==    by 0x23489D: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).BucketHeader.captureStackTrace (general_purpose_allocator.zig:278)
==3189443==    by 0x246797: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).allocSlot (general_purpose_allocator.zig:501)
==3189443==    by 0x232417: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).allocInner (general_purpose_allocator.zig:986)
==3189443==    by 0x231F78: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).alloc (general_purpose_allocator.zig:942)
==3189443==  Address 0x1ffeffe000 is on thread 1's stack
==3189443==  904 bytes below stack pointer
==3189443== 
==3189443== Syscall param msync(start) points to uninitialised byte(s)
==3189443==    at 0x24DD9B: os.linux.x86_64.syscall3 (x86_64.zig:46)
==3189443==    by 0x25D6F8: os.linux.msync (linux.zig:432)
==3189443==    by 0x25417A: os.msync (os.zig:4454)
==3189443==    by 0x24E9E6: debug.StackIterator.isValidMemory (debug.zig:496)
==3189443==    by 0x23FF7B: debug.StackIterator.next_internal (debug.zig:539)
==3189443==    by 0x23086B: debug.StackIterator.next (debug.zig:479)
==3189443==    by 0x233915: debug.captureStackTrace (debug.zig:239)
==3189443==    by 0x246C53: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).collectStackTrace (general_purpose_allocator.zig:456)
==3189443==    by 0x23489D: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).BucketHeader.captureStackTrace (general_purpose_allocator.zig:278)
==3189443==    by 0x246797: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).allocSlot (general_purpose_allocator.zig:501)
==3189443==    by 0x232417: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).allocInner (general_purpose_allocator.zig:986)
==3189443==    by 0x231F78: heap.general_purpose_allocator.GeneralPurposeAllocator(.{.stack_trace_frames = 4, .enable_memory_limit = false, .safety = true, .thread_safe = true, .MutexType = null, .never_unmap = false, .retain_metadata = false, .verbose_log = false}).alloc (general_purpose_allocator.zig:942)
==3189443==  Address 0x1ffefff000 is on thread 1's stack

here's the workaround used to make the debug info recognized by valgrind:

// main.zig
const std = @import("std");

// force a .data section in the executable
export var foo: usize = 1;

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    defer _ = gpa.deinit();
    const allocator = gpa.allocator();
    const args = try std.process.argsAlloc(allocator);
    defer std.process.argsFree(allocator, args);
    std.debug.print("Args: {s}\n", .{args});
    foo += 1; // make sure the foo variable doesn't get optimized out
}

squeek502 commented 1 year ago

After looking into this a bit, I think it's ultimately harmless and am not sure if/how it can be avoided. This msync call is just checking if the address is mapped for the purposes of stack unwinding--the bytes at the address are never read or written to so the address pointing to unaddressable/uninitialized bytes shouldn't actually lead to a problem. To be more specific, msync needs a page-aligned address so std.debug.StackIterator.isValidMemory takes an address like 0x1ffefff600 and then gets its page-aligned address which in this case is 0x1ffefff000 and passes that to msync. We don't care what's actually at that address, we just want to know if it's part of a mapped page.

libunwind seems to have similar behavior judging from the google results of "Syscall param msync(start) points".

Also worth noting that this is not specific to GeneralPurposeAllocator, it's also reproducible with the following code:

const std = @import("std");

// force a .data section in the executable to ensure Valgrind debug info works,
// see https://github.com/ziglang/zig/issues/15254
export var foo: usize = 1;

pub fn main() !void {
    std.debug.dumpCurrentStackTrace(@returnAddress());
    foo += 1; // make sure the foo variable doesn't get optimized out
}

This will lead to:

==186236== Syscall param msync(start) points to unaddressable byte(s)
==186236==    at 0x23F25B: os.linux.x86_64.syscall3 (x86_64.zig:46)
==186236==    by 0x248188: os.linux.msync (linux.zig:432)
==186236==    by 0x2445BA: os.msync (os.zig:4454)
==186236==    by 0x2391E7: debug.StackIterator.isValidMemory (debug.zig:497)
==186236==    by 0x22CABB: debug.StackIterator.next_internal (debug.zig:542)
==186236==    by 0x229EBE: debug.StackIterator.next (debug.zig:475)
==186236==    by 0x229D47: debug.writeCurrentStackTrace__anon_4358 (debug.zig:575)
==186236==    by 0x20B6D3: debug.dumpCurrentStackTrace (debug.zig:157)
==186236==    by 0x20B55C: main.main (main.zig:6)
==186236==    by 0x20B064: callMain (start.zig:609)
==186236==    by 0x20B064: initEventLoopAndCallMain (start.zig:543)
==186236==    by 0x20B064: callMainWithArgs (start.zig:493)
==186236==    by 0x20B064: start.posixCallMainAndExit (start.zig:456)
==186236==    by 0x20AAF1: (below main) (start.zig:368)
==186236==  Address 0x1ffefff000 is on thread 1's stack
==186236==  280 bytes below stack pointer
==186236==

ziglang / zig