disallow unknown-length pointer to opaque

This also means that translate-c has to detect when a pointer to opaque is happening, and use `*` instead of `[*]`. See #1059
ziglang · Jun 5, 2018 · 652f4bd · 652f4bd
1 parent 7a09482
commit 652f4bd
Show file tree

Hide file tree

Showing 14 changed files with 89 additions and 50 deletions.
diff --git a/src/analyze.cpp b/src/analyze.cpp
@@ -384,6 +384,7 @@ TypeTableEntry *get_pointer_to_type_extra(CodeGen *g, TypeTableEntry *child_type
         bool is_volatile, PtrLen ptr_len, uint32_t byte_alignment, uint32_t bit_offset, uint32_t unaligned_bit_count)
 {
     assert(!type_is_invalid(child_type));
+    assert(ptr_len == PtrLenSingle || child_type->id != TypeTableEntryIdOpaque);
 
     TypeId type_id = {};
     TypeTableEntry **parent_pointer = nullptr;

diff --git a/src/ir.cpp b/src/ir.cpp
@@ -4620,11 +4620,8 @@ static IrInstruction *ir_lval_wrap(IrBuilder *irb, Scope *scope, IrInstruction *
 
 static IrInstruction *ir_gen_pointer_type(IrBuilder *irb, Scope *scope, AstNode *node) {
     assert(node->type == NodeTypePointerType);
-    // The null check here is for C imports which don't set a token on the AST node. We could potentially
-    // update that code to create a fake token and then remove this check.
-    PtrLen ptr_len = (node->data.pointer_type.star_token != nullptr &&
-            (node->data.pointer_type.star_token->id == TokenIdStar ||
-             node->data.pointer_type.star_token->id == TokenIdStarStar)) ? PtrLenSingle : PtrLenUnknown;
+    PtrLen ptr_len = (node->data.pointer_type.star_token->id == TokenIdStar ||
+             node->data.pointer_type.star_token->id == TokenIdStarStar) ? PtrLenSingle : PtrLenUnknown;
     bool is_const = node->data.pointer_type.is_const;
     bool is_volatile = node->data.pointer_type.is_volatile;
     AstNode *expr_node = node->data.pointer_type.op_expr;
@@ -18973,6 +18970,9 @@ static TypeTableEntry *ir_analyze_instruction_ptr_type(IrAnalyze *ira, IrInstruc
     if (child_type->id == TypeTableEntryIdUnreachable) {
         ir_add_error(ira, &instruction->base, buf_sprintf("pointer to noreturn not allowed"));
         return ira->codegen->builtin_types.entry_invalid;
+    } else if (child_type->id == TypeTableEntryIdOpaque && instruction->ptr_len == PtrLenUnknown) {
+        ir_add_error(ira, &instruction->base, buf_sprintf("unknown-length pointer to opaque"));
+        return ira->codegen->builtin_types.entry_invalid;
     }
 
     uint32_t align_bytes;

diff --git a/src/tokenizer.hpp b/src/tokenizer.hpp
@@ -170,6 +170,8 @@ struct Token {
         TokenCharLit char_lit;
     } data;
 };
+// work around conflicting name Token which is also found in libclang
+typedef Token ZigToken;
 
 struct Tokenization {
     ZigList<Token> *tokens;

diff --git a/src/translate_c.cpp b/src/translate_c.cpp
@@ -276,8 +276,11 @@ static AstNode *maybe_suppress_result(Context *c, ResultUsed result_used, AstNod
         node);
 }
 
-static AstNode *trans_create_node_ptr_type(Context *c, bool is_const, bool is_volatile, AstNode *child_node) {
+static AstNode *trans_create_node_ptr_type(Context *c, bool is_const, bool is_volatile, AstNode *child_node, PtrLen ptr_len) {
     AstNode *node = trans_create_node(c, NodeTypePointerType);
+    node->data.pointer_type.star_token = allocate<ZigToken>(1);
+    node->data.pointer_type.star_token->id = (ptr_len == PtrLenSingle) ? TokenIdStar: TokenIdBracketStarBracket;
+    node->data.pointer_type.is_const = is_const;
     node->data.pointer_type.is_const = is_const;
     node->data.pointer_type.is_volatile = is_volatile;
     node->data.pointer_type.op_expr = child_node;
@@ -731,6 +734,30 @@ static bool qual_type_has_wrapping_overflow(Context *c, QualType qt) {
     }
 }
 
+static bool type_is_opaque(Context *c, const Type *ty, const SourceLocation &source_loc) {
+    switch (ty->getTypeClass()) {
+        case Type::Builtin: {
+            const BuiltinType *builtin_ty = static_cast<const BuiltinType*>(ty);
+            return builtin_ty->getKind() == BuiltinType::Void;
+        }
+        case Type::Record: {
+            const RecordType *record_ty = static_cast<const RecordType*>(ty);
+            return record_ty->getDecl()->getDefinition() == nullptr;
+        }
+        case Type::Elaborated: {
+            const ElaboratedType *elaborated_ty = static_cast<const ElaboratedType*>(ty);
+            return type_is_opaque(c, elaborated_ty->getNamedType().getTypePtr(), source_loc);
+        }
+        case Type::Typedef: {
+            const TypedefType *typedef_ty = static_cast<const TypedefType*>(ty);
+            const TypedefNameDecl *typedef_decl = typedef_ty->getDecl();
+            return type_is_opaque(c, typedef_decl->getUnderlyingType().getTypePtr(), source_loc);
+        }
+        default:
+            return false;
+    }
+}
+
 static AstNode *trans_type(Context *c, const Type *ty, const SourceLocation &source_loc) {
     switch (ty->getTypeClass()) {
         case Type::Builtin:
@@ -855,8 +882,10 @@ static AstNode *trans_type(Context *c, const Type *ty, const SourceLocation &sou
                     return trans_create_node_prefix_op(c, PrefixOpMaybe, child_node);
                 }
 
+                PtrLen ptr_len = type_is_opaque(c, child_qt.getTypePtr(), source_loc) ? PtrLenSingle : PtrLenUnknown;
+
                 AstNode *pointer_node = trans_create_node_ptr_type(c, child_qt.isConstQualified(),
-                        child_qt.isVolatileQualified(), child_node);
+                        child_qt.isVolatileQualified(), child_node, ptr_len);
                 return trans_create_node_prefix_op(c, PrefixOpMaybe, pointer_node);
             }
         case Type::Typedef:
@@ -1041,7 +1070,7 @@ static AstNode *trans_type(Context *c, const Type *ty, const SourceLocation &sou
                     return nullptr;
                 }
                 AstNode *pointer_node = trans_create_node_ptr_type(c, child_qt.isConstQualified(),
-                        child_qt.isVolatileQualified(), child_type_node);
+                        child_qt.isVolatileQualified(), child_type_node, PtrLenUnknown);
                 return pointer_node;
             }
         case Type::BlockPointer:
@@ -4448,7 +4477,7 @@ static AstNode *parse_ctok_suffix_op_expr(Context *c, CTokenize *ctok, size_t *t
         } else if (first_tok->id == CTokIdAsterisk) {
             *tok_i += 1;
 
-            node = trans_create_node_ptr_type(c, false, false, node);
+            node = trans_create_node_ptr_type(c, false, false, node, PtrLenUnknown);
         } else {
             return node;
         }

diff --git a/std/c/index.zig b/std/c/index.zig
@@ -20,11 +20,11 @@ pub extern "c" fn @"fstat$INODE64"(fd: c_int, buf: *Stat) c_int;
 pub extern "c" fn lseek(fd: c_int, offset: isize, whence: c_int) isize;
 pub extern "c" fn open(path: [*]const u8, oflag: c_int, ...) c_int;
 pub extern "c" fn raise(sig: c_int) c_int;
-pub extern "c" fn read(fd: c_int, buf: [*]c_void, nbyte: usize) isize;
+pub extern "c" fn read(fd: c_int, buf: *c_void, nbyte: usize) isize;
 pub extern "c" fn stat(noalias path: [*]const u8, noalias buf: *Stat) c_int;
-pub extern "c" fn write(fd: c_int, buf: [*]const c_void, nbyte: usize) isize;
-pub extern "c" fn mmap(addr: ?[*]c_void, len: usize, prot: c_int, flags: c_int, fd: c_int, offset: isize) ?[*]c_void;
-pub extern "c" fn munmap(addr: [*]c_void, len: usize) c_int;
+pub extern "c" fn write(fd: c_int, buf: *const c_void, nbyte: usize) isize;
+pub extern "c" fn mmap(addr: ?*c_void, len: usize, prot: c_int, flags: c_int, fd: c_int, offset: isize) ?*c_void;
+pub extern "c" fn munmap(addr: *c_void, len: usize) c_int;
 pub extern "c" fn unlink(path: [*]const u8) c_int;
 pub extern "c" fn getcwd(buf: [*]u8, size: usize) ?[*]u8;
 pub extern "c" fn waitpid(pid: c_int, stat_loc: *c_int, options: c_int) c_int;
@@ -48,15 +48,15 @@ pub extern "c" fn setreuid(ruid: c_uint, euid: c_uint) c_int;
 pub extern "c" fn setregid(rgid: c_uint, egid: c_uint) c_int;
 pub extern "c" fn rmdir(path: [*]const u8) c_int;
 
-pub extern "c" fn aligned_alloc(alignment: usize, size: usize) ?[*]c_void;
-pub extern "c" fn malloc(usize) ?[*]c_void;
-pub extern "c" fn realloc([*]c_void, usize) ?[*]c_void;
-pub extern "c" fn free([*]c_void) void;
-pub extern "c" fn posix_memalign(memptr: *[*]c_void, alignment: usize, size: usize) c_int;
+pub extern "c" fn aligned_alloc(alignment: usize, size: usize) ?*c_void;
+pub extern "c" fn malloc(usize) ?*c_void;
+pub extern "c" fn realloc(*c_void, usize) ?*c_void;
+pub extern "c" fn free(*c_void) void;
+pub extern "c" fn posix_memalign(memptr: **c_void, alignment: usize, size: usize) c_int;
 
 pub extern "pthread" fn pthread_create(noalias newthread: *pthread_t, noalias attr: ?*const pthread_attr_t, start_routine: extern fn (?*c_void) ?*c_void, noalias arg: ?*c_void) c_int;
 pub extern "pthread" fn pthread_attr_init(attr: *pthread_attr_t) c_int;
-pub extern "pthread" fn pthread_attr_setstack(attr: *pthread_attr_t, stackaddr: [*]c_void, stacksize: usize) c_int;
+pub extern "pthread" fn pthread_attr_setstack(attr: *pthread_attr_t, stackaddr: *c_void, stacksize: usize) c_int;
 pub extern "pthread" fn pthread_attr_destroy(attr: *pthread_attr_t) c_int;
 pub extern "pthread" fn pthread_join(thread: pthread_t, arg_return: ?*?*c_void) c_int;
 

diff --git a/std/heap.zig b/std/heap.zig
@@ -22,7 +22,7 @@ fn cAlloc(self: *Allocator, n: usize, alignment: u29) ![]u8 {
 }
 
 fn cRealloc(self: *Allocator, old_mem: []u8, new_size: usize, alignment: u29) ![]u8 {
-    const old_ptr = @ptrCast([*]c_void, old_mem.ptr);
+    const old_ptr = @ptrCast(*c_void, old_mem.ptr);
     if (c.realloc(old_ptr, new_size)) |buf| {
         return @ptrCast([*]u8, buf)[0..new_size];
     } else if (new_size <= old_mem.len) {
@@ -33,7 +33,7 @@ fn cRealloc(self: *Allocator, old_mem: []u8, new_size: usize, alignment: u29) ![
 }
 
 fn cFree(self: *Allocator, old_mem: []u8) void {
-    const old_ptr = @ptrCast([*]c_void, old_mem.ptr);
+    const old_ptr = @ptrCast(*c_void, old_mem.ptr);
     c.free(old_ptr);
 }
 
@@ -140,7 +140,7 @@ pub const DirectAllocator = struct {
                 const old_adjusted_addr = @ptrToInt(old_mem.ptr);
                 const old_record_addr = old_adjusted_addr + old_mem.len;
                 const root_addr = @intToPtr(*align(1) usize, old_record_addr).*;
-                const old_ptr = @intToPtr([*]c_void, root_addr);
+                const old_ptr = @intToPtr(*c_void, root_addr);
                 const amt = new_size + alignment + @sizeOf(usize);
                 const new_ptr = os.windows.HeapReAlloc(??self.heap_handle, 0, old_ptr, amt) ?? blk: {
                     if (new_size > old_mem.len) return error.OutOfMemory;
@@ -170,7 +170,7 @@ pub const DirectAllocator = struct {
             Os.windows => {
                 const record_addr = @ptrToInt(bytes.ptr) + bytes.len;
                 const root_addr = @intToPtr(*align(1) usize, record_addr).*;
-                const ptr = @intToPtr([*]c_void, root_addr);
+                const ptr = @intToPtr(*c_void, root_addr);
                 _ = os.windows.HeapFree(??self.heap_handle, 0, ptr);
             },
             else => @compileError("Unsupported OS"),

diff --git a/std/os/darwin.zig b/std/os/darwin.zig
@@ -327,25 +327,25 @@ pub fn raise(sig: i32) usize {
 }
 
 pub fn read(fd: i32, buf: [*]u8, nbyte: usize) usize {
-    return errnoWrap(c.read(fd, @ptrCast([*]c_void, buf), nbyte));
+    return errnoWrap(c.read(fd, @ptrCast(*c_void, buf), nbyte));
 }
 
 pub fn stat(noalias path: [*]const u8, noalias buf: *stat) usize {
     return errnoWrap(c.stat(path, buf));
 }
 
 pub fn write(fd: i32, buf: [*]const u8, nbyte: usize) usize {
-    return errnoWrap(c.write(fd, @ptrCast([*]const c_void, buf), nbyte));
+    return errnoWrap(c.write(fd, @ptrCast(*const c_void, buf), nbyte));
 }
 
 pub fn mmap(address: ?[*]u8, length: usize, prot: usize, flags: u32, fd: i32, offset: isize) usize {
-    const ptr_result = c.mmap(@ptrCast([*]c_void, address), length, @bitCast(c_int, c_uint(prot)), @bitCast(c_int, c_uint(flags)), fd, offset);
+    const ptr_result = c.mmap(@ptrCast(*c_void, address), length, @bitCast(c_int, c_uint(prot)), @bitCast(c_int, c_uint(flags)), fd, offset);
     const isize_result = @bitCast(isize, @ptrToInt(ptr_result));
     return errnoWrap(isize_result);
 }
 
 pub fn munmap(address: usize, length: usize) usize {
-    return errnoWrap(c.munmap(@intToPtr([*]c_void, address), length));
+    return errnoWrap(c.munmap(@intToPtr(*c_void, address), length));
 }
 
 pub fn unlink(path: [*]const u8) usize {

diff --git a/std/os/file.zig b/std/os/file.zig
@@ -334,7 +334,7 @@ pub const File = struct {
             while (index < buffer.len) {
                 const want_read_count = windows.DWORD(math.min(windows.DWORD(@maxValue(windows.DWORD)), buffer.len - index));
                 var amt_read: windows.DWORD = undefined;
-                if (windows.ReadFile(self.handle, @ptrCast([*]c_void, buffer.ptr + index), want_read_count, &amt_read, null) == 0) {
+                if (windows.ReadFile(self.handle, @ptrCast(*c_void, buffer.ptr + index), want_read_count, &amt_read, null) == 0) {
                     const err = windows.GetLastError();
                     return switch (err) {
                         windows.ERROR.OPERATION_ABORTED => continue,

diff --git a/std/os/index.zig b/std/os/index.zig
@@ -2362,7 +2362,7 @@ pub const Thread = struct {
         },
         builtin.Os.windows => struct {
             handle: windows.HANDLE,
-            alloc_start: [*]c_void,
+            alloc_start: *c_void,
             heap_handle: windows.HANDLE,
         },
         else => @compileError("Unsupported OS"),
@@ -2533,7 +2533,7 @@ pub fn spawnThread(context: var, comptime startFn: var) SpawnThreadError!*Thread
 
         // align to page
         stack_end -= stack_end % os.page_size;
-        assert(c.pthread_attr_setstack(&attr, @intToPtr([*]c_void, stack_addr), stack_end - stack_addr) == 0);
+        assert(c.pthread_attr_setstack(&attr, @intToPtr(*c_void, stack_addr), stack_end - stack_addr) == 0);
 
         const err = c.pthread_create(&thread_ptr.data.handle, &attr, MainFuncs.posixThreadMain, @intToPtr(*c_void, arg));
         switch (err) {

diff --git a/std/os/windows/index.zig b/std/os/windows/index.zig
@@ -101,17 +101,17 @@ pub extern "kernel32" stdcallcc fn GetSystemTimeAsFileTime(?*FILETIME) void;
 
 pub extern "kernel32" stdcallcc fn HeapCreate(flOptions: DWORD, dwInitialSize: SIZE_T, dwMaximumSize: SIZE_T) ?HANDLE;
 pub extern "kernel32" stdcallcc fn HeapDestroy(hHeap: HANDLE) BOOL;
-pub extern "kernel32" stdcallcc fn HeapReAlloc(hHeap: HANDLE, dwFlags: DWORD, lpMem: [*]c_void, dwBytes: SIZE_T) ?[*]c_void;
-pub extern "kernel32" stdcallcc fn HeapSize(hHeap: HANDLE, dwFlags: DWORD, lpMem: [*]const c_void) SIZE_T;
-pub extern "kernel32" stdcallcc fn HeapValidate(hHeap: HANDLE, dwFlags: DWORD, lpMem: [*]const c_void) BOOL;
+pub extern "kernel32" stdcallcc fn HeapReAlloc(hHeap: HANDLE, dwFlags: DWORD, lpMem: *c_void, dwBytes: SIZE_T) ?*c_void;
+pub extern "kernel32" stdcallcc fn HeapSize(hHeap: HANDLE, dwFlags: DWORD, lpMem: *const c_void) SIZE_T;
+pub extern "kernel32" stdcallcc fn HeapValidate(hHeap: HANDLE, dwFlags: DWORD, lpMem: *const c_void) BOOL;
 pub extern "kernel32" stdcallcc fn HeapCompact(hHeap: HANDLE, dwFlags: DWORD) SIZE_T;
 pub extern "kernel32" stdcallcc fn HeapSummary(hHeap: HANDLE, dwFlags: DWORD, lpSummary: LPHEAP_SUMMARY) BOOL;
 
 pub extern "kernel32" stdcallcc fn GetStdHandle(in_nStdHandle: DWORD) ?HANDLE;
 
-pub extern "kernel32" stdcallcc fn HeapAlloc(hHeap: HANDLE, dwFlags: DWORD, dwBytes: SIZE_T) ?[*]c_void;
+pub extern "kernel32" stdcallcc fn HeapAlloc(hHeap: HANDLE, dwFlags: DWORD, dwBytes: SIZE_T) ?*c_void;
 
-pub extern "kernel32" stdcallcc fn HeapFree(hHeap: HANDLE, dwFlags: DWORD, lpMem: [*]c_void) BOOL;
+pub extern "kernel32" stdcallcc fn HeapFree(hHeap: HANDLE, dwFlags: DWORD, lpMem: *c_void) BOOL;
 
 pub extern "kernel32" stdcallcc fn MoveFileExA(
     lpExistingFileName: LPCSTR,
@@ -127,7 +127,7 @@ pub extern "kernel32" stdcallcc fn PathFileExists(pszPath: ?LPCTSTR) BOOL;
 
 pub extern "kernel32" stdcallcc fn ReadFile(
     in_hFile: HANDLE,
-    out_lpBuffer: [*]c_void,
+    out_lpBuffer: *c_void,
     in_nNumberOfBytesToRead: DWORD,
     out_lpNumberOfBytesRead: *DWORD,
     in_out_lpOverlapped: ?*OVERLAPPED,
@@ -150,7 +150,7 @@ pub extern "kernel32" stdcallcc fn WaitForSingleObject(hHandle: HANDLE, dwMillis
 
 pub extern "kernel32" stdcallcc fn WriteFile(
     in_hFile: HANDLE,
-    in_lpBuffer: [*]const c_void,
+    in_lpBuffer: *const c_void,
     in_nNumberOfBytesToWrite: DWORD,
     out_lpNumberOfBytesWritten: ?*DWORD,
     in_out_lpOverlapped: ?*OVERLAPPED,

diff --git a/std/os/windows/util.zig b/std/os/windows/util.zig
@@ -42,7 +42,7 @@ pub const WriteError = error{
 };
 
 pub fn windowsWrite(handle: windows.HANDLE, bytes: []const u8) WriteError!void {
-    if (windows.WriteFile(handle, @ptrCast([*]const c_void, bytes.ptr), u32(bytes.len), null, null) == 0) {
+    if (windows.WriteFile(handle, @ptrCast(*const c_void, bytes.ptr), u32(bytes.len), null, null) == 0) {
         const err = windows.GetLastError();
         return switch (err) {
             windows.ERROR.INVALID_USER_BUFFER => WriteError.SystemResources,

diff --git a/test/compare_output.zig b/test/compare_output.zig
@@ -284,7 +284,7 @@ pub fn addCases(cases: *tests.CompareOutputContext) void {
     cases.addC("expose function pointer to C land",
         \\const c = @cImport(@cInclude("stdlib.h"));
         \\
-        \\export fn compare_fn(a: ?[*]const c_void, b: ?[*]const c_void) c_int {
+        \\export fn compare_fn(a: ?*const c_void, b: ?*const c_void) c_int {
         \\    const a_int = @ptrCast(*const i32, @alignCast(@alignOf(i32), a));
         \\    const b_int = @ptrCast(*const i32, @alignCast(@alignOf(i32), b));
         \\    if (a_int.* < b_int.*) {
@@ -299,7 +299,7 @@ pub fn addCases(cases: *tests.CompareOutputContext) void {
         \\export fn main() c_int {
         \\    var array = []u32{ 1, 7, 3, 2, 0, 9, 4, 8, 6, 5 };
         \\
-        \\    c.qsort(@ptrCast(?[*]c_void, array[0..].ptr), c_ulong(array.len), @sizeOf(i32), compare_fn);
+        \\    c.qsort(@ptrCast(?*c_void, array[0..].ptr), c_ulong(array.len), @sizeOf(i32), compare_fn);
         \\
         \\    for (array) |item, i| {
         \\        if (item != i) {

diff --git a/test/compile_errors.zig b/test/compile_errors.zig
@@ -1,6 +1,13 @@
 const tests = @import("tests.zig");
 
 pub fn addCases(cases: *tests.CompileErrorContext) void {
+    cases.add(
+        "unknown length pointer to opaque",
+        \\export const T = [*]@OpaqueType();
+    ,
+        ".tmp_source.zig:1:18: error: unknown-length pointer to opaque",
+    );
+
     cases.add(
         "error when evaluating return type",
         \\const Foo = struct {