From 91989e70ba68e3543acffef079d97c9416b5259c Mon Sep 17 00:00:00 2001 From: Matthew McAllister Date: Tue, 12 Feb 2019 21:22:16 -0800 Subject: [PATCH 1/2] Fix lvalue dereference type checking Previously, if a dereference instruction was an lvalue, it would fail to typecheck that the value being dereferenced was indeed a pointer. Although a little clunky, this change obviates the need for redundant type checks scattered about the analysis. --- src/all_types.hpp | 11 ++++++----- src/ir.cpp | 39 ++++++++++++++++++--------------------- test/compile_errors.zig | 24 +++++++++++++++++++++--- 3 files changed, 45 insertions(+), 29 deletions(-) diff --git a/src/all_types.hpp b/src/all_types.hpp index 6fbd987b9..1304106f0 100644 --- a/src/all_types.hpp +++ b/src/all_types.hpp @@ -2091,6 +2091,11 @@ struct IrBasicBlock { IrInstruction *must_be_comptime_source_instr; }; +enum LVal { + LValNone, + LValPtr, +}; + // These instructions are in transition to having "pass 1" instructions // and "pass 2" instructions. The pass 1 instructions are suffixed with Src // and pass 2 are suffixed with Gen. @@ -2354,6 +2359,7 @@ struct IrInstructionUnOp { IrUnOp op_id; IrInstruction *value; + LVal lval; }; enum IrBinOp { @@ -3090,11 +3096,6 @@ struct IrInstructionTypeName { IrInstruction *type_value; }; -enum LVal { - LValNone, - LValPtr, -}; - struct IrInstructionDeclRef { IrInstruction base; diff --git a/src/ir.cpp b/src/ir.cpp index 0fcbb60fe..5e6ff16f8 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -1307,6 +1307,7 @@ static IrInstruction *ir_build_un_op(IrBuilder *irb, Scope *scope, AstNode *sour IrInstructionUnOp *br_instruction = ir_build_instruction(irb, scope, source_node); br_instruction->op_id = op_id; br_instruction->value = value; + br_instruction->lval = LValNone; ir_ref_instruction(value, irb->current_basic_block); @@ -7223,7 +7224,13 @@ static IrInstruction *ir_gen_node_raw(IrBuilder *irb, AstNode *node, Scope *scop if (value == irb->codegen->invalid_instruction) return value; - return ir_build_un_op(irb, scope, node, IrUnOpDereference, value); + // We essentially just converted any lvalue from &(x.*) to (&x).*; + // this inhibits checking that x is a pointer later, so we directly + // record whether the pointer check is needed + IrInstructionUnOp *result = (IrInstructionUnOp*)ir_build_un_op(irb, scope, node, IrUnOpDereference, value); + result->lval = lval; + + return &result->base; } case NodeTypeUnwrapOptional: { AstNode *expr_node = node->data.unwrap_optional.expr; @@ -11437,7 +11444,7 @@ static IrInstruction *ir_get_deref(IrAnalyze *ira, IrInstruction *source_instruc return load_ptr_instruction; } else { ir_add_error_node(ira, source_instruction->source_node, - buf_sprintf("attempt to dereference non pointer type '%s'", + buf_sprintf("attempt to dereference non-pointer type '%s'", buf_ptr(&type_entry->name))); return ira->codegen->invalid_instruction; } @@ -13616,12 +13623,6 @@ no_mem_slot: static IrInstruction *ir_analyze_store_ptr(IrAnalyze *ira, IrInstruction *source_instr, IrInstruction *ptr, IrInstruction *uncasted_value) { - if (ptr->value.type->id != ZigTypeIdPointer) { - ir_add_error(ira, ptr, - buf_sprintf("attempt to dereference non pointer type '%s'", buf_ptr(&ptr->value.type->name))); - return ira->codegen->invalid_instruction; - } - if (ptr->value.data.x_ptr.special == ConstPtrSpecialDiscard) { return ir_const_void(ira, source_instr); } @@ -14550,11 +14551,18 @@ static IrInstruction *ir_analyze_instruction_un_op(IrAnalyze *ira, IrInstruction buf_ptr(&ptr_type->name))); return ira->codegen->invalid_instruction; } - // this dereference is always an rvalue because in the IR gen we identify lvalue and emit - // one of the ptr instructions + IrInstruction *result = ir_get_deref(ira, &instruction->base, ptr); if (result == ira->codegen->invalid_instruction) return ira->codegen->invalid_instruction; + + // If the result needs to be an lvalue, type check it + if (instruction->lval == LValPtr && result->value.type->id != ZigTypeIdPointer) { + ir_add_error(ira, &instruction->base, + buf_sprintf("attempt to dereference non-pointer type '%s'", buf_ptr(&result->value.type->name))); + return ira->codegen->invalid_instruction; + } + return result; } case IrUnOpOptional: @@ -15380,12 +15388,6 @@ static IrInstruction *ir_analyze_instruction_field_ptr(IrAnalyze *ira, IrInstruc if (type_is_invalid(container_ptr->value.type)) return ira->codegen->invalid_instruction; - if (container_ptr->value.type->id != ZigTypeIdPointer) { - ir_add_error_node(ira, field_ptr_instruction->base.source_node, - buf_sprintf("attempt to dereference non-pointer type '%s'", - buf_ptr(&container_ptr->value.type->name))); - return ira->codegen->invalid_instruction; - } ZigType *container_type = container_ptr->value.type->data.pointer.child_type; Buf *field_name = field_ptr_instruction->field_name_buffer; @@ -16596,11 +16598,6 @@ static IrInstruction *ir_analyze_instruction_switch_target(IrAnalyze *ira, return ir_const_type(ira, &switch_target_instruction->base, ptr_type->data.pointer.child_type); } - if (target_value_ptr->value.type->id != ZigTypeIdPointer) { - ir_add_error(ira, target_value_ptr, buf_sprintf("invalid deref on switch target")); - return ira->codegen->invalid_instruction; - } - ZigType *target_type = target_value_ptr->value.type->data.pointer.child_type; ConstExprValue *pointee_val = nullptr; if (instr_is_comptime(target_value_ptr)) { diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 9ef4af416..f2f08c432 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -137,6 +137,24 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { ".tmp_source.zig:3:15: error: C pointers cannot point to non-C-ABI-compatible type 'Foo'", ); + cases.addTest( + "assign to invalid dereference", + \\export fn entry() void { + \\ 'a'.* = 1; + \\} + , + ".tmp_source.zig:2:8: error: attempt to dereference non-pointer type 'comptime_int'", + ); + + cases.addTest( + "take slice of invalid dereference", + \\export fn entry() void { + \\ const x = 'a'.*[0..]; + \\} + , + ".tmp_source.zig:2:18: error: attempt to dereference non-pointer type 'comptime_int'", + ); + cases.addTest( "@truncate undefined value", \\export fn entry() void { @@ -447,7 +465,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ _ = a.*.len; \\} , - ".tmp_source.zig:3:12: error: attempt to dereference non-pointer type '[]u8'", + ".tmp_source.zig:3:10: error: attempt to dereference non-pointer type '[]u8'", ); cases.add( @@ -1158,7 +1176,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ Filled, \\}; , - ".tmp_source.zig:3:17: error: invalid deref on switch target", + ".tmp_source.zig:3:17: error: attempt to dereference non-pointer type 'Tile'", ); cases.add( @@ -4000,7 +4018,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ \\export fn entry() usize { return @sizeOf(@typeOf(pass)); } , - ".tmp_source.zig:4:10: error: attempt to dereference non pointer type '[10]u8'", + ".tmp_source.zig:4:10: error: attempt to dereference non-pointer type '[10]u8'", ); cases.add( From c8ce351ec982608d3ea0c60a34c7b18d894dee01 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 19 Feb 2019 15:34:44 -0500 Subject: [PATCH 2/2] pull request fixups --- src/ir.cpp | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/ir.cpp b/src/ir.cpp index e5f09dd93..3e48779b9 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -1303,15 +1303,23 @@ static IrInstruction *ir_build_ptr_type(IrBuilder *irb, Scope *scope, AstNode *s return &ptr_type_of_instruction->base; } -static IrInstruction *ir_build_un_op(IrBuilder *irb, Scope *scope, AstNode *source_node, IrUnOp op_id, IrInstruction *value) { - IrInstructionUnOp *br_instruction = ir_build_instruction(irb, scope, source_node); - br_instruction->op_id = op_id; - br_instruction->value = value; - br_instruction->lval = LValNone; +static IrInstruction *ir_build_un_op_lval(IrBuilder *irb, Scope *scope, AstNode *source_node, IrUnOp op_id, + IrInstruction *value, LVal lval) +{ + IrInstructionUnOp *instruction = ir_build_instruction(irb, scope, source_node); + instruction->op_id = op_id; + instruction->value = value; + instruction->lval = lval; ir_ref_instruction(value, irb->current_basic_block); - return &br_instruction->base; + return &instruction->base; +} + +static IrInstruction *ir_build_un_op(IrBuilder *irb, Scope *scope, AstNode *source_node, IrUnOp op_id, + IrInstruction *value) +{ + return ir_build_un_op_lval(irb, scope, source_node, op_id, value, LValNone); } static IrInstruction *ir_build_container_init_list(IrBuilder *irb, Scope *scope, AstNode *source_node, @@ -7227,10 +7235,7 @@ static IrInstruction *ir_gen_node_raw(IrBuilder *irb, AstNode *node, Scope *scop // We essentially just converted any lvalue from &(x.*) to (&x).*; // this inhibits checking that x is a pointer later, so we directly // record whether the pointer check is needed - IrInstructionUnOp *result = (IrInstructionUnOp*)ir_build_un_op(irb, scope, node, IrUnOpDereference, value); - result->lval = lval; - - return &result->base; + return ir_build_un_op_lval(irb, scope, node, IrUnOpDereference, value, lval); } case NodeTypeUnwrapOptional: { AstNode *expr_node = node->data.unwrap_optional.expr; @@ -13685,6 +13690,8 @@ no_mem_slot: static IrInstruction *ir_analyze_store_ptr(IrAnalyze *ira, IrInstruction *source_instr, IrInstruction *ptr, IrInstruction *uncasted_value) { + assert(ptr->value.type->id == ZigTypeIdPointer); + if (ptr->value.data.x_ptr.special == ConstPtrSpecialDiscard) { return ir_const_void(ira, source_instr); }