From 096f79260b025ab53d77c4943f237676abb5b7d8 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 15 Feb 2020 21:17:39 +0100 Subject: [PATCH 1/3] ir: Prevent crash when indexing undefined ptr to array Closes #4471 --- src/ir.cpp | 44 ++++++++++++++++++++++++----------------- test/compile_errors.zig | 9 +++++++++ 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/ir.cpp b/src/ir.cpp index 323fa4ba6f76..a29a5b694864 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -20638,12 +20638,12 @@ static IrInstGen *ir_analyze_instruction_elem_ptr(IrAnalyze *ira, IrInstSrcElemP if (type_is_invalid(array_ptr->value->type)) return ira->codegen->invalid_inst_gen; - ZigValue *orig_array_ptr_val = array_ptr->value; - IrInstGen *elem_index = elem_ptr_instruction->elem_index->child; if (type_is_invalid(elem_index->value->type)) return ira->codegen->invalid_inst_gen; + ZigValue *orig_array_ptr_val = array_ptr->value; + ZigType *ptr_type = orig_array_ptr_val->type; assert(ptr_type->id == ZigTypeIdPointer); @@ -20653,23 +20653,25 @@ static IrInstGen *ir_analyze_instruction_elem_ptr(IrAnalyze *ira, IrInstSrcElemP // We will adjust return_type's alignment before returning it. ZigType *return_type; - if (type_is_invalid(array_type)) { + if (type_is_invalid(array_type)) return ira->codegen->invalid_inst_gen; - } else if (array_type->id == ZigTypeIdArray || - (array_type->id == ZigTypeIdPointer && - array_type->data.pointer.ptr_len == PtrLenSingle && - array_type->data.pointer.child_type->id == ZigTypeIdArray)) + + if (array_type->id == ZigTypeIdPointer && + array_type->data.pointer.ptr_len == PtrLenSingle && + array_type->data.pointer.child_type->id == ZigTypeIdArray) { - if (array_type->id == ZigTypeIdPointer) { - array_type = array_type->data.pointer.child_type; - ptr_type = ptr_type->data.pointer.child_type; - if (orig_array_ptr_val->special != ConstValSpecialRuntime) { - orig_array_ptr_val = const_ptr_pointee(ira, ira->codegen, orig_array_ptr_val, - elem_ptr_instruction->base.base.source_node); - if (orig_array_ptr_val == nullptr) - return ira->codegen->invalid_inst_gen; - } - } + IrInstGen *ptr_value = ir_get_deref(ira, &elem_ptr_instruction->base.base, + array_ptr, nullptr); + if (type_is_invalid(ptr_value->value->type)) + return ira->codegen->invalid_inst_gen; + + array_type = array_type->data.pointer.child_type; + ptr_type = ptr_type->data.pointer.child_type; + + orig_array_ptr_val = ptr_value->value; + } + + if (array_type->id == ZigTypeIdArray) { if (array_type->data.array.len == 0) { ir_add_error_node(ira, elem_ptr_instruction->base.base.source_node, buf_sprintf("index 0 outside array of size 0")); @@ -20807,8 +20809,14 @@ static IrInstGen *ir_analyze_instruction_elem_ptr(IrAnalyze *ira, IrInstSrcElemP orig_array_ptr_val->data.x_ptr.special != ConstPtrSpecialHardCodedAddr && (orig_array_ptr_val->data.x_ptr.mut != ConstPtrMutRuntimeVar || array_type->id == ZigTypeIdArray)) { + if ((err = ir_resolve_const_val(ira->codegen, ira->new_irb.exec, + elem_ptr_instruction->base.base.source_node, orig_array_ptr_val, UndefBad))) + { + return ira->codegen->invalid_inst_gen; + } + ZigValue *array_ptr_val = const_ptr_pointee(ira, ira->codegen, orig_array_ptr_val, - elem_ptr_instruction->base.base.source_node); + elem_ptr_instruction->base.base.source_node); if (array_ptr_val == nullptr) return ira->codegen->invalid_inst_gen; diff --git a/test/compile_errors.zig b/test/compile_errors.zig index cc2863a0460b..6d46afc979c1 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -3,6 +3,15 @@ const builtin = @import("builtin"); const Target = @import("std").Target; pub fn addCases(cases: *tests.CompileErrorContext) void { + cases.addTest("access of undefined pointer to array", + \\const ram_u32: *[4096]u32 = undefined; + \\export fn entry() void { + \\ @ptrCast(*u32, &(ram_u32[0])) = &(ram_u32[0]); + \\} + , &[_][]const u8{ + "tmp.zig:3:29: error: use of undefined value here causes undefined behavior", + }); + cases.addTest("duplicate field in anonymous struct literal", \\export fn entry() void { \\ const anon = .{ From 59a243ce24e858260d289c5c40f3430223b7d98b Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 15 Feb 2020 21:33:59 +0100 Subject: [PATCH 2/3] std: Remove now-superflous hack --- lib/std/json.zig | 4 +--- lib/std/zig/tokenizer.zig | 8 ++------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/std/json.zig b/lib/std/json.zig index 74e008d76e17..b04861eaae01 100644 --- a/lib/std/json.zig +++ b/lib/std/json.zig @@ -1026,10 +1026,8 @@ pub const TokenStream = struct { pub fn next(self: *TokenStream) Error!?Token { if (self.token) |token| { - // TODO: Audit this pattern once #2915 is closed - const copy = token; self.token = null; - return copy; + return token; } var t1: ?Token = undefined; diff --git a/lib/std/zig/tokenizer.zig b/lib/std/zig/tokenizer.zig index 327700f59141..f6c71479e7be 100644 --- a/lib/std/zig/tokenizer.zig +++ b/lib/std/zig/tokenizer.zig @@ -414,10 +414,8 @@ pub const Tokenizer = struct { pub fn next(self: *Tokenizer) Token { if (self.pending_invalid_token) |token| { - // TODO: Audit this pattern once #2915 is closed - const copy = token; self.pending_invalid_token = null; - return copy; + return token; } const start_index = self.index; var state = State.Start; @@ -1270,10 +1268,8 @@ pub const Tokenizer = struct { if (result.id == Token.Id.Eof) { if (self.pending_invalid_token) |token| { - // TODO: Audit this pattern once #2915 is closed - const copy = token; self.pending_invalid_token = null; - return copy; + return token; } } From 6b74fd2e12b117e90197db5f4fa1ea51aa246248 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 16 Feb 2020 20:19:30 +0100 Subject: [PATCH 3/3] ir: Avoid invalidating the decl_table iterator Collect the declarations to resolve first and run resolve_top_level_decl on them later. Closes #4310 --- src/ir.cpp | 43 +++++++++++++++++++++++++++++------------ test/compile_errors.zig | 10 +++++----- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/ir.cpp b/src/ir.cpp index a29a5b694864..8544fe175889 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -23653,14 +23653,13 @@ static Error ir_make_type_info_decls(IrAnalyze *ira, IrInst* source_instr, ZigVa if ((err = type_resolve(ira->codegen, type_info_fn_decl_inline_type, ResolveStatusSizeKnown))) return err; - // Loop through our declarations once to figure out how many declarations we will generate info for. + // The unresolved declarations are collected in a separate queue to avoid + // modifying decl_table while iterating over it + ZigList resolve_decl_queue{}; + auto decl_it = decls_scope->decl_table.entry_iterator(); decltype(decls_scope->decl_table)::Entry *curr_entry = nullptr; - int declaration_count = 0; - while ((curr_entry = decl_it.next()) != nullptr) { - // If the declaration is unresolved, force it to be resolved again. - resolve_top_level_decl(ira->codegen, curr_entry->value, curr_entry->value->source_node, false); if (curr_entry->value->resolution == TldResolutionInvalid) { return ErrorSemanticAnalyzeFail; } @@ -23670,16 +23669,36 @@ static Error ir_make_type_info_decls(IrAnalyze *ira, IrInst* source_instr, ZigVa return ErrorSemanticAnalyzeFail; } + // If the declaration is unresolved, force it to be resolved again. + if (curr_entry->value->resolution == TldResolutionUnresolved) + resolve_decl_queue.append(curr_entry->value); + } + + for (size_t i = 0; i < resolve_decl_queue.length; i++) { + Tld *decl = resolve_decl_queue.at(i); + resolve_top_level_decl(ira->codegen, decl, decl->source_node, false); + if (decl->resolution == TldResolutionInvalid) { + return ErrorSemanticAnalyzeFail; + } + } + + resolve_decl_queue.deinit(); + + // Loop through our declarations once to figure out how many declarations we will generate info for. + int declaration_count = 0; + decl_it = decls_scope->decl_table.entry_iterator(); + while ((curr_entry = decl_it.next()) != nullptr) { // Skip comptime blocks and test functions. - if (curr_entry->value->id != TldIdCompTime) { - if (curr_entry->value->id == TldIdFn) { - ZigFn *fn_entry = ((TldFn *)curr_entry->value)->fn_entry; - if (fn_entry->is_test) - continue; - } + if (curr_entry->value->id == TldIdCompTime) + continue; - declaration_count += 1; + if (curr_entry->value->id == TldIdFn) { + ZigFn *fn_entry = ((TldFn *)curr_entry->value)->fn_entry; + if (fn_entry->is_test) + continue; } + + declaration_count += 1; } ZigValue *declaration_array = ira->codegen->pass1_arena->create(); diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 6d46afc979c1..febc93648624 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -3,13 +3,13 @@ const builtin = @import("builtin"); const Target = @import("std").Target; pub fn addCases(cases: *tests.CompileErrorContext) void { - cases.addTest("access of undefined pointer to array", - \\const ram_u32: *[4096]u32 = undefined; - \\export fn entry() void { - \\ @ptrCast(*u32, &(ram_u32[0])) = &(ram_u32[0]); + cases.addTest("", + \\const A = B; + \\test "Crash" { + \\ _ = @typeInfo(@This()).Struct.decls; \\} , &[_][]const u8{ - "tmp.zig:3:29: error: use of undefined value here causes undefined behavior", + "tmp.zig:1:11: error: use of undeclared identifier 'B'", }); cases.addTest("duplicate field in anonymous struct literal",