Commit 53ae03ebe9e0aa56bca1c9219a47d124b0435258 | zig

author	Andrew Kelley <andrew@ziglang.org> 2019-09-23 15:02:38-04:00
committer	Andrew Kelley <andrew@ziglang.org> 2019-09-23 15:02:38-04:00
log	53ae03ebe9e0aa56bca1c9219a47d124b0435258
tree	2f69fc4aeb6bdbc9567338b033337c4556f5c0c0
parent	29b82d20a4534fc7e3393c21ad637d44d20449a2
signature	Commit is signed but in an unrecognized format.

make type_allowed_in_extern more robust

Previously if the type parameter was a pointer, it would assert that the
size of the type was resolved. It used to be that the size of pointers was
always resolved, however with lazy values, pointers gained the
possibility of not having their size resolved.

Now, type_allowed_in_extern triggers the resolution of whether a pointer
is zero bits, and returns a possible error if the resolution fails.

This fixes a compiler assertion when building the
[zootdeck project](https://github.com/donpdonp/zootdeck). I do not have
a test case reduction for the issue.

3 files changed, 105 insertions(+), 53 deletions(-)

src/analyze.cpp+91-49

 @@ -1538,7 +1538,8 @@ static Error emit_error_unless_type_allowed_in_packed_union(CodeGen *g, ZigType
     return emit_error_unless_type_allowed_in_packed_container(g, type_entry, source_node, "union");
 }
-bool type_allowed_in_extern(CodeGen *g, ZigType *type_entry) {
+Error type_allowed_in_extern(CodeGen *g, ZigType *type_entry, bool *result) {
+    Error err;
     switch (type_entry->id) {
         case ZigTypeIdInvalid:
             zig_unreachable();
 @@ -1555,11 +1556,13 @@ bool type_allowed_in_extern(CodeGen *g, ZigType *type_entry) {
         case ZigTypeIdVoid:
         case ZigTypeIdFnFrame:
         case ZigTypeIdAnyFrame:
-            return false;
+            *result = false;
+            return ErrorNone;
         case ZigTypeIdOpaque:
         case ZigTypeIdUnreachable:
         case ZigTypeIdBool:
-            return true;
+            *result = true;
+            return ErrorNone;
         case ZigTypeIdInt:
             switch (type_entry->data.integral.bit_count) {
                 case 8:
 @@ -1567,37 +1570,52 @@ bool type_allowed_in_extern(CodeGen *g, ZigType *type_entry) {
                 case 32:
                 case 64:
                 case 128:
-                    return true;
+                    *result = true;
+                    return ErrorNone;
                 default:
-                    return false;
+                    *result = false;
+                    return ErrorNone;
             }
         case ZigTypeIdVector:
-            return type_allowed_in_extern(g, type_entry->data.vector.elem_type);
+            return type_allowed_in_extern(g, type_entry->data.vector.elem_type, result);
         case ZigTypeIdFloat:
-            return true;
+            *result = true;
+            return ErrorNone;
         case ZigTypeIdArray:
-            return type_allowed_in_extern(g, type_entry->data.array.child_type);
+            return type_allowed_in_extern(g, type_entry->data.array.child_type, result);
         case ZigTypeIdFn:
-            return type_entry->data.fn.fn_type_id.cc == CallingConventionC ||
+            *result = type_entry->data.fn.fn_type_id.cc == CallingConventionC ||
                  type_entry->data.fn.fn_type_id.cc == CallingConventionStdcall;
+            return ErrorNone;
         case ZigTypeIdPointer:
-            if (type_size(g, type_entry) == 0)
+            if ((err = type_resolve(g, type_entry, ResolveStatusZeroBitsKnown)))
-                return false;
+                return err;
-            return true;
+            if (!type_has_bits(type_entry)) {
+                *result = false;
+                return ErrorNone;
+            }
+            *result = true;
+            return ErrorNone;
         case ZigTypeIdStruct:
-            return type_entry->data.structure.layout == ContainerLayoutExtern || type_entry->data.structure.layout == ContainerLayoutPacked;
+            *result = type_entry->data.structure.layout == ContainerLayoutExtern ||
-        case ZigTypeIdOptional:
+                type_entry->data.structure.layout == ContainerLayoutPacked;
-            {
+            return ErrorNone;
-                ZigType *child_type = type_entry->data.maybe.child_type;
+        case ZigTypeIdOptional: {
-                if (child_type->id != ZigTypeIdPointer && child_type->id != ZigTypeIdFn) {
+            ZigType *child_type = type_entry->data.maybe.child_type;
-                    return false;
+            if (child_type->id != ZigTypeIdPointer && child_type->id != ZigTypeIdFn) {
-                }
+                *result = false;
-                return type_allowed_in_extern(g, child_type);
+                return ErrorNone;
             }
+            return type_allowed_in_extern(g, child_type, result);
+        }
         case ZigTypeIdEnum:
-            return type_entry->data.enumeration.layout == ContainerLayoutExtern || type_entry->data.enumeration.layout == ContainerLayoutPacked;
+            *result = type_entry->data.enumeration.layout == ContainerLayoutExtern ||
+                type_entry->data.enumeration.layout == ContainerLayoutPacked;
+            return ErrorNone;
         case ZigTypeIdUnion:
-            return type_entry->data.unionation.layout == ContainerLayoutExtern || type_entry->data.unionation.layout == ContainerLayoutPacked;
+            *result = type_entry->data.unionation.layout == ContainerLayoutExtern ||
+                type_entry->data.unionation.layout == ContainerLayoutPacked;
+            return ErrorNone;
     }
     zig_unreachable();
 }
 @@ -1687,12 +1705,17 @@ static ZigType *analyze_fn_type(CodeGen *g, AstNode *proto_node, Scope *child_sc
             }
         }
-        if (!calling_convention_allows_zig_types(fn_type_id.cc) && !type_allowed_in_extern(g, type_entry)) {
+        if (!calling_convention_allows_zig_types(fn_type_id.cc)) {
-            add_node_error(g, param_node->data.param_decl.type,
+            bool ok_type;
-                    buf_sprintf("parameter of type '%s' not allowed in function with calling convention '%s'",
+            if ((err = type_allowed_in_extern(g, type_entry, &ok_type)))
-                        buf_ptr(&type_entry->name),
+                return g->builtin_types.entry_invalid;
-                        calling_convention_name(fn_type_id.cc)));
+            if (!ok_type) {
-            return g->builtin_types.entry_invalid;
+                add_node_error(g, param_node->data.param_decl.type,
+                        buf_sprintf("parameter of type '%s' not allowed in function with calling convention '%s'",
+                            buf_ptr(&type_entry->name),
+                            calling_convention_name(fn_type_id.cc)));
+                return g->builtin_types.entry_invalid;
+            }
         }
         switch (type_entry->id) {
 @@ -1809,7 +1832,10 @@ static ZigType *analyze_fn_type(CodeGen *g, AstNode *proto_node, Scope *child_sc
     {
         if ((err = type_resolve(g, fn_type_id.return_type, ResolveStatusSizeKnown)))
             return g->builtin_types.entry_invalid;
-        if (!type_allowed_in_extern(g, fn_type_id.return_type)) {
+        bool ok_type;
+        if ((err = type_allowed_in_extern(g, fn_type_id.return_type, &ok_type)))
+            return g->builtin_types.entry_invalid;
+        if (!ok_type) {
             add_node_error(g, fn_proto->return_type,
                     buf_sprintf("return type '%s' not allowed in function with calling convention '%s'",
                         buf_ptr(&fn_type_id.return_type->name),
 @@ -2128,14 +2154,19 @@ static Error resolve_struct_type(CodeGen *g, ZigType *struct_type) {
             return err;
         }
-        if (struct_type->data.structure.layout == ContainerLayoutExtern &&
+        if (struct_type->data.structure.layout == ContainerLayoutExtern) {
-            !type_allowed_in_extern(g, field_type))
+            bool ok_type;
-        {
+            if ((err = type_allowed_in_extern(g, field_type, &ok_type))) {
-            add_node_error(g, field->decl_node,
+                struct_type->data.structure.resolve_status = ResolveStatusInvalid;
-                    buf_sprintf("extern structs cannot contain fields of type '%s'",
+                return ErrorSemanticAnalyzeFail;
-                        buf_ptr(&field_type->name)));
+            }
-            struct_type->data.structure.resolve_status = ResolveStatusInvalid;
+            if (!ok_type) {
-            return ErrorSemanticAnalyzeFail;
+                add_node_error(g, field->decl_node,
+                        buf_sprintf("extern structs cannot contain fields of type '%s'",
+                            buf_ptr(&field_type->name)));
+                struct_type->data.structure.resolve_status = ResolveStatusInvalid;
+                return ErrorSemanticAnalyzeFail;
+            }
         }
     }
 @@ -2368,19 +2399,22 @@ static Error resolve_union_type(CodeGen *g, ZigType *union_type) {
     return ErrorNone;
 }
-static bool type_is_valid_extern_enum_tag(CodeGen *g, ZigType *ty) {
+static Error type_is_valid_extern_enum_tag(CodeGen *g, ZigType *ty, bool *result) {
     // Only integer types are allowed by the C ABI
-    if(ty->id != ZigTypeIdInt)
+    if(ty->id != ZigTypeIdInt) {
-        return false;
+        *result = false;
+        return ErrorNone;
+    }
     // According to the ANSI C standard the enumeration type should be either a
     // signed char, a signed integer or an unsigned one. But GCC/Clang allow
     // other integral types as a compiler extension so let's accomodate them
     // aswell.
-    return type_allowed_in_extern(g, ty);
+    return type_allowed_in_extern(g, ty, result);
 }
 static Error resolve_enum_zero_bits(CodeGen *g, ZigType *enum_type) {
+    Error err;
     assert(enum_type->id == ZigTypeIdEnum);
     if (enum_type->data.enumeration.resolve_status == ResolveStatusInvalid)
 @@ -2443,15 +2477,23 @@ static Error resolve_enum_zero_bits(CodeGen *g, ZigType *enum_type) {
             enum_type->data.enumeration.resolve_status = ResolveStatusInvalid;
             add_node_error(g, decl_node->data.container_decl.init_arg_expr,
                 buf_sprintf("expected integer, found '%s'", buf_ptr(&wanted_tag_int_type->name)));
-        } else if (enum_type->data.enumeration.layout == ContainerLayoutExtern &&
-                   !type_is_valid_extern_enum_tag(g, wanted_tag_int_type)) {
-            enum_type->data.enumeration.resolve_status = ResolveStatusInvalid;
-            ErrorMsg *msg = add_node_error(g, decl_node->data.container_decl.init_arg_expr,
-                buf_sprintf("'%s' is not a valid tag type for an extern enum",
-                            buf_ptr(&wanted_tag_int_type->name)));
-            add_error_note(g, msg, decl_node->data.container_decl.init_arg_expr,
-                buf_sprintf("any integral type of size 8, 16, 32, 64 or 128 bit is valid"));
         } else {
+            if (enum_type->data.enumeration.layout == ContainerLayoutExtern) {
+                bool ok_type;
+                if ((err = type_is_valid_extern_enum_tag(g, wanted_tag_int_type, &ok_type))) {
+                    enum_type->data.enumeration.resolve_status = ResolveStatusInvalid;
+                    return err;
+                }
+                if (!ok_type) {
+                    enum_type->data.enumeration.resolve_status = ResolveStatusInvalid;
+                    ErrorMsg *msg = add_node_error(g, decl_node->data.container_decl.init_arg_expr,
+                        buf_sprintf("'%s' is not a valid tag type for an extern enum",
+                                    buf_ptr(&wanted_tag_int_type->name)));
+                    add_error_note(g, msg, decl_node->data.container_decl.init_arg_expr,
+                        buf_sprintf("any integral type of size 8, 16, 32, 64 or 128 bit is valid"));
+                    return ErrorNone;
+                }
+            }
             tag_int_type = wanted_tag_int_type;
         }
     }

src/analyze.hpp+1-1

...	@@ -42,7 +42,7 @@ ZigType get_any_frame_type(CodeGen g, ZigType *result_type);	...	@@ -42,7 +42,7 @@ ZigType get_any_frame_type(CodeGen g, ZigType *result_type);
42	bool handle_is_ptr(ZigType *type_entry);	42	bool handle_is_ptr(ZigType *type_entry);
43		43
44	bool type_has_bits(ZigType *type_entry);	44	bool type_has_bits(ZigType *type_entry);
45	bool type_allowed_in_extern(CodeGen g, ZigType type_entry);	45	Error type_allowed_in_extern(CodeGen g, ZigType type_entry, bool *result);
46	bool ptr_allows_addr_zero(ZigType *ptr_type);	46	bool ptr_allows_addr_zero(ZigType *ptr_type);
47	bool type_is_nonnull_ptr(ZigType *type);	47	bool type_is_nonnull_ptr(ZigType *type);
48		48

src/ir.cpp+13-3

 @@ -14837,6 +14837,8 @@ static IrInstruction *ir_analyze_instruction_decl_var(IrAnalyze *ira,
 }
 static IrInstruction *ir_analyze_instruction_export(IrAnalyze *ira, IrInstructionExport *instruction) {
+    Error err;
     IrInstruction *name = instruction->name->child;
     Buf *symbol_name = ir_resolve_str(ira, name);
     if (symbol_name == nullptr) {
 @@ -14933,8 +14935,12 @@ static IrInstruction *ir_analyze_instruction_export(IrAnalyze *ira, IrInstructio
                 want_var_export = true;
             }
             break;
-        case ZigTypeIdArray:
+        case ZigTypeIdArray: {
-            if (!type_allowed_in_extern(ira->codegen, target->value.type->data.array.child_type)) {
+            bool ok_type;
+            if ((err = type_allowed_in_extern(ira->codegen, target->value.type->data.array.child_type, &ok_type)))
+                return ira->codegen->invalid_instruction;
+            if (!ok_type) {
                 ir_add_error(ira, target,
                     buf_sprintf("array element type '%s' not extern-compatible",
                         buf_ptr(&target->value.type->data.array.child_type->name)));
 @@ -14942,6 +14948,7 @@ static IrInstruction *ir_analyze_instruction_export(IrAnalyze *ira, IrInstructio
                 want_var_export = true;
             }
             break;
+        }
         case ZigTypeIdMetaType: {
             ZigType *type_value = target->value.data.x_type;
             switch (type_value->id) {
 @@ -26720,7 +26727,10 @@ static Error ir_resolve_lazy_raw(AstNode *source_node, ConstExprValue *val) {
                         buf_create_from_str("unknown-length pointer to opaque"));
                 return ErrorSemanticAnalyzeFail;
             } else if (lazy_ptr_type->ptr_len == PtrLenC) {
-                if (!type_allowed_in_extern(ira->codegen, elem_type)) {
+                bool ok_type;
+                if ((err = type_allowed_in_extern(ira->codegen, elem_type, &ok_type)))
+                    return err;
+                if (!ok_type) {
                     ir_add_error(ira, lazy_ptr_type->elem_type,
                         buf_sprintf("C pointers cannot point to non-C-ABI-compatible type '%s'",
                             buf_ptr(&elem_type->name)));