Skip to content

Commit 2b7781d

Browse files
LemonBoyandrewrk
authored andcommitted
stage1: Fix undefined assignment for bitfields
Prevents silent memory corruption. Closes #7055
1 parent 13cccdd commit 2b7781d

File tree

2 files changed

+54
-16
lines changed

2 files changed

+54
-16
lines changed

src/stage1/codegen.cpp

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ static void render_const_val_global(CodeGen *g, ZigValue *const_val, const char
8383
static LLVMValueRef gen_const_val(CodeGen *g, ZigValue *const_val, const char *name);
8484
static void generate_error_name_table(CodeGen *g);
8585
static bool value_is_all_undef(CodeGen *g, ZigValue *const_val);
86-
static void gen_undef_init(CodeGen *g, uint32_t ptr_align_bytes, ZigType *value_type, LLVMValueRef ptr);
86+
static void gen_undef_init(CodeGen *g, ZigType *ptr_type, ZigType *value_type, LLVMValueRef ptr);
8787
static LLVMValueRef build_alloca(CodeGen *g, ZigType *type_entry, const char *name, uint32_t alignment);
8888
static LLVMValueRef gen_await_early_return(CodeGen *g, IrInstGen *source_instr,
8989
LLVMValueRef target_frame_ptr, ZigType *result_type, ZigType *ptr_result_type,
@@ -3351,7 +3351,7 @@ static LLVMValueRef ir_render_ptr_of_array_to_slice(CodeGen *g, IrExecutableGen
33513351
gen_store_untyped(g, slice_start_ptr, ptr_field_ptr, 0, false);
33523352
} else if (ir_want_runtime_safety(g, &instruction->base)) {
33533353
LLVMValueRef ptr_field_ptr = LLVMBuildStructGEP(g->builder, result_loc, ptr_index, "");
3354-
gen_undef_init(g, slice_ptr_type->abi_align, slice_ptr_type, ptr_field_ptr);
3354+
gen_undef_init(g, slice_ptr_type, slice_ptr_type, ptr_field_ptr);
33553355
}
33563356

33573357
LLVMValueRef len_field_ptr = LLVMBuildStructGEP(g->builder, result_loc, len_index, "");
@@ -3816,22 +3816,35 @@ static void gen_valgrind_undef(CodeGen *g, LLVMValueRef dest_ptr, LLVMValueRef b
38163816
gen_valgrind_client_request(g, zero, req, ptr_as_usize, byte_count, zero, zero, zero);
38173817
}
38183818

3819-
static void gen_undef_init(CodeGen *g, uint32_t ptr_align_bytes, ZigType *value_type, LLVMValueRef ptr) {
3819+
static void gen_undef_init(CodeGen *g, ZigType *ptr_type, ZigType *value_type, LLVMValueRef ptr) {
38203820
assert(type_has_bits(g, value_type));
3821+
3822+
uint64_t ptr_align_bytes = get_ptr_align(g, ptr_type);
3823+
assert(ptr_align_bytes > 0);
38213824
uint64_t size_bytes = LLVMStoreSizeOfType(g->target_data_ref, get_llvm_type(g, value_type));
38223825
assert(size_bytes > 0);
3823-
assert(ptr_align_bytes > 0);
3824-
// memset uninitialized memory to 0xaa
3825-
LLVMTypeRef ptr_u8 = LLVMPointerType(LLVMInt8Type(), 0);
3826-
LLVMValueRef fill_char = LLVMConstInt(LLVMInt8Type(), 0xaa, false);
3827-
LLVMValueRef dest_ptr = LLVMBuildBitCast(g->builder, ptr, ptr_u8, "");
3828-
ZigType *usize = g->builtin_types.entry_usize;
3829-
LLVMValueRef byte_count = LLVMConstInt(usize->llvm_type, size_bytes, false);
3830-
ZigLLVMBuildMemSet(g->builder, dest_ptr, fill_char, byte_count, ptr_align_bytes, false);
3831-
// then tell valgrind that the memory is undefined even though we just memset it
3832-
if (g->valgrind_enabled) {
3833-
gen_valgrind_undef(g, dest_ptr, byte_count);
3826+
3827+
if (ptr_type->data.pointer.host_int_bytes == 0) {
3828+
// memset uninitialized memory to 0xaa
3829+
LLVMTypeRef ptr_u8 = LLVMPointerType(LLVMInt8Type(), 0);
3830+
LLVMValueRef fill_char = LLVMConstInt(LLVMInt8Type(), 0xaa, false);
3831+
LLVMValueRef dest_ptr = LLVMBuildBitCast(g->builder, ptr, ptr_u8, "");
3832+
ZigType *usize = g->builtin_types.entry_usize;
3833+
LLVMValueRef byte_count = LLVMConstInt(usize->llvm_type, size_bytes, false);
3834+
ZigLLVMBuildMemSet(g->builder, dest_ptr, fill_char, byte_count, ptr_align_bytes, false);
3835+
// then tell valgrind that the memory is undefined even though we just memset it
3836+
if (g->valgrind_enabled) {
3837+
gen_valgrind_undef(g, dest_ptr, byte_count);
3838+
}
3839+
return;
38343840
}
3841+
3842+
// This is a pointer into a packed struct, we can't use memset here.
3843+
// The jury is still out on what pattern should be written here so clear the
3844+
// old value and call it a day. Generating a 0xAA...AA mask for this n-bit
3845+
// value is left as an exercise for the (bored) reader.
3846+
LLVMValueRef zero = LLVMConstNull(get_llvm_type(g, value_type));
3847+
gen_assign_raw(g, ptr, ptr_type, zero);
38353848
}
38363849

38373850
static LLVMValueRef ir_render_store_ptr(CodeGen *g, IrExecutableGen *executable, IrInstGenStorePtr *instruction) {
@@ -3860,7 +3873,7 @@ static LLVMValueRef ir_render_store_ptr(CodeGen *g, IrExecutableGen *executable,
38603873
LLVMValueRef value = ir_llvm_value(g, instruction->value);
38613874
gen_assign_raw(g, ptr, ptr_type, value);
38623875
} else if (ir_want_runtime_safety(g, &instruction->base)) {
3863-
gen_undef_init(g, get_ptr_align(g, ptr_type), instruction->value->value->type,
3876+
gen_undef_init(g, ptr_type, instruction->value->value->type,
38643877
ir_llvm_value(g, instruction->ptr));
38653878
}
38663879
return nullptr;
@@ -5885,7 +5898,7 @@ static LLVMValueRef ir_render_slice(CodeGen *g, IrExecutableGen *executable, IrI
58855898
if (slice_start_ptr != nullptr) {
58865899
gen_store_untyped(g, slice_start_ptr, ptr_field_ptr, 0, false);
58875900
} else if (want_runtime_safety) {
5888-
gen_undef_init(g, slice_ptr_type->abi_align, slice_ptr_type, ptr_field_ptr);
5901+
gen_undef_init(g, slice_ptr_type, slice_ptr_type, ptr_field_ptr);
58895902
} else {
58905903
gen_store_untyped(g, LLVMGetUndef(get_llvm_type(g, slice_ptr_type)), ptr_field_ptr, 0, false);
58915904
}

test/stage1/behavior/struct.zig

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -884,3 +884,28 @@ test "type coercion of anon struct literal to struct" {
884884
S.doTheTest();
885885
comptime S.doTheTest();
886886
}
887+
888+
test "packed struct with undefined initializers" {
889+
const S = struct {
890+
const P = packed struct {
891+
a: u3,
892+
_a: u3 = undefined,
893+
b: u3,
894+
_b: u3 = undefined,
895+
c: u3,
896+
_c: u3 = undefined,
897+
};
898+
899+
fn doTheTest() void {
900+
var p: P = undefined;
901+
p = P{ .a = 2, .b = 4, .c = 6 };
902+
// Make sure the compiler doesn't touch the unprefixed fields.
903+
expectEqual(@as(u3, 2), p.a);
904+
expectEqual(@as(u3, 4), p.b);
905+
expectEqual(@as(u3, 6), p.c);
906+
}
907+
};
908+
909+
S.doTheTest();
910+
comptime S.doTheTest();
911+
}

0 commit comments

Comments
 (0)