Commit 1b41f2d77e

Alexandros Naskos <alex_naskos@hotmail.com>
2020-02-29 07:36:42
C pointer slices are no longer allowzero (#4462)
* Slices from C pointers are no longer allowzero but instead insert a runtime assertion. * Added a test, fixed code for cases with non-allowzero C pointers * Create new type when flipping allow_zero, sometimes we get a cached value back from adjust_ptr_len. * Added comments, changed panic message * Added runtime safety test.
1 parent a5a53a1
Changed files (4)
src/codegen.cpp
@@ -973,7 +973,7 @@ static Buf *panic_msg_buf(PanicMsgId msg_id) {
         case PanicMsgIdExactDivisionRemainder:
             return buf_create_from_str("exact division produced remainder");
         case PanicMsgIdUnwrapOptionalFail:
-            return buf_create_from_str("attempt to unwrap null");
+            return buf_create_from_str("attempt to use null value");
         case PanicMsgIdUnreachable:
             return buf_create_from_str("reached unreachable code");
         case PanicMsgIdInvalidErrorCode:
src/ir.cpp
@@ -20412,6 +20412,17 @@ static ZigType *adjust_ptr_len(CodeGen *g, ZigType *ptr_type, PtrLen ptr_len) {
             ptr_type->data.pointer.allow_zero);
 }
 
+static ZigType *adjust_ptr_allow_zero(CodeGen *g, ZigType *ptr_type, bool allow_zero) {
+    assert(ptr_type->id == ZigTypeIdPointer);
+    return get_pointer_to_type_extra(g,
+            ptr_type->data.pointer.child_type,
+            ptr_type->data.pointer.is_const, ptr_type->data.pointer.is_volatile,
+            ptr_type->data.pointer.ptr_len,
+            ptr_type->data.pointer.explicit_alignment,
+            ptr_type->data.pointer.bit_offset_in_host, ptr_type->data.pointer.host_int_bytes,
+            allow_zero);
+}
+
 static IrInstGen *ir_analyze_instruction_elem_ptr(IrAnalyze *ira, IrInstSrcElemPtr *elem_ptr_instruction) {
     Error err;
     IrInstGen *array_ptr = elem_ptr_instruction->array_ptr->child;
@@ -25966,6 +25977,8 @@ static IrInstGen *ir_analyze_instruction_slice(IrAnalyze *ira, IrInstSrcSlice *i
     ZigType *non_sentinel_slice_ptr_type;
     ZigType *elem_type;
 
+    bool generate_non_null_assert = false;
+
     if (array_type->id == ZigTypeIdArray) {
         elem_type = array_type->data.array.child_type;
         bool is_comptime_const = ptr_ptr->value->special == ConstValSpecialStatic &&
@@ -25993,6 +26006,14 @@ static IrInstGen *ir_analyze_instruction_slice(IrAnalyze *ira, IrInstSrcSlice *i
             elem_type = array_type->data.pointer.child_type;
             if (array_type->data.pointer.ptr_len == PtrLenC) {
                 array_type = adjust_ptr_len(ira->codegen, array_type, PtrLenUnknown);
+
+                // C pointers are allowzero by default.  
+                // However, we want to be able to slice them without generating an allowzero slice (see issue #4401).
+                // To achieve this, we generate a runtime safety check and make the slice type non-allowzero.
+                if (array_type->data.pointer.allow_zero) {
+                    array_type = adjust_ptr_allow_zero(ira->codegen, array_type, false);
+                    generate_non_null_assert = true;
+                }
             }
             ZigType *maybe_sentineled_slice_ptr_type = array_type;
             non_sentinel_slice_ptr_type = adjust_ptr_sentinel(ira->codegen, maybe_sentineled_slice_ptr_type, nullptr);
@@ -26264,7 +26285,6 @@ static IrInstGen *ir_analyze_instruction_slice(IrAnalyze *ira, IrInstSrcSlice *i
 
     IrInstGen *result_loc = ir_resolve_result(ira, &instruction->base.base, instruction->result_loc,
             return_type, nullptr, true, true);
-
     if (result_loc != nullptr) {
         if (type_is_invalid(result_loc->value->type) || result_loc->value->type->id == ZigTypeIdUnreachable) {
             return result_loc;
@@ -26277,8 +26297,17 @@ static IrInstGen *ir_analyze_instruction_slice(IrAnalyze *ira, IrInstSrcSlice *i
             return ira->codegen->invalid_inst_gen;
     }
 
-    return ir_build_slice_gen(ira, &instruction->base.base, return_type,
-        ptr_ptr, casted_start, end, instruction->safety_check_on, result_loc);
+    if (generate_non_null_assert) {
+        IrInstGen *ptr_val = ir_get_deref(ira, &instruction->base.base, ptr_ptr, nullptr);
+
+        if (type_is_invalid(ptr_val->value->type))
+            return ira->codegen->invalid_inst_gen;
+    
+        ir_build_assert_non_null(ira, &instruction->base.base, ptr_val);
+    }
+
+    return ir_build_slice_gen(ira, &instruction->base.base, return_type, ptr_ptr,
+            casted_start, end, instruction->safety_check_on, result_loc);
 }
 
 static IrInstGen *ir_analyze_instruction_has_field(IrAnalyze *ira, IrInstSrcHasField *instruction) {
test/stage1/behavior/slice.zig
@@ -43,6 +43,17 @@ test "C pointer" {
     expectEqualSlices(u8, "kjdhfkjdhf", slice);
 }
 
+test "C pointer slice access" {
+    var buf: [10]u32 = [1]u32{42} ** 10;
+    const c_ptr = @ptrCast([*c]const u32, &buf);
+
+    comptime expectEqual([]const u32, @TypeOf(c_ptr[0..1]));
+
+    for (c_ptr[0..5]) |*cl| {
+        expectEqual(@as(u32, 42), cl.*);
+    }
+}
+
 fn sliceSum(comptime q: []const u8) i32 {
     comptime var result = 0;
     inline for (q) |item| {
test/runtime_safety.zig
@@ -745,4 +745,17 @@ pub fn addCases(cases: *tests.CompareOutputContext) void {
         \\    (await p) catch unreachable;
         \\}
     );
+
+    // Slicing a C pointer returns a non-allowzero slice, thus we need to emit
+    // a safety check to ensure the pointer is not null.
+    cases.addRuntimeSafety("slicing null C pointer",
+        \\pub fn panic(message: []const u8, stack_trace: ?*@import("builtin").StackTrace) noreturn {
+        \\    @import("std").os.exit(126);
+        \\}
+        \\
+        \\pub fn main() void {
+        \\    var ptr: [*c]const u32 = null;
+        \\    var slice = ptr[0..3];
+        \\}
+    );
 }