Skip to content

Commit 5699ab5

Browse files
andrewrkandrewrk
committed
C pointers: errors for nested pointer casting regarding null
See #1059
1 parent 270933b commit 5699ab5

File tree

4 files changed

+181
-89
lines changed

4 files changed

+181
-89
lines changed

‎src/all_types.hpp

+9-3
Original file line numberDiff line numberDiff line change
@@ -691,15 +691,17 @@ struct AstNodePointerType {
691691
AstNode *align_expr;
692692
BigInt *bit_offset_start;
693693
BigInt *host_int_bytes;
694+
AstNode *op_expr;
695+
Token *allow_zero_token;
694696
bool is_const;
695697
bool is_volatile;
696-
AstNode *op_expr;
697698
};
698699

699700
struct AstNodeArrayType {
700701
AstNode *size;
701702
AstNode *child_type;
702703
AstNode *align_expr;
704+
Token *allow_zero_token;
703705
bool is_const;
704706
bool is_volatile;
705707
};
@@ -1050,6 +1052,7 @@ struct ZigTypePointer {
10501052
uint32_t host_int_bytes; // size of host integer. 0 means no host integer; this field is aligned
10511053
bool is_const;
10521054
bool is_volatile;
1055+
bool allow_zero;
10531056
};
10541057

10551058
struct ZigTypeInt {
@@ -1499,11 +1502,12 @@ struct TypeId {
14991502
struct {
15001503
ZigType *child_type;
15011504
PtrLen ptr_len;
1502-
bool is_const;
1503-
bool is_volatile;
15041505
uint32_t alignment;
15051506
uint32_t bit_offset_in_host;
15061507
uint32_t host_int_bytes;
1508+
bool is_const;
1509+
bool is_volatile;
1510+
bool allow_zero;
15071511
} pointer;
15081512
struct {
15091513
ZigType *child_type;
@@ -2592,6 +2596,7 @@ struct IrInstructionPtrType {
25922596
PtrLen ptr_len;
25932597
bool is_const;
25942598
bool is_volatile;
2599+
bool allow_zero;
25952600
};
25962601

25972602
struct IrInstructionPromiseType {
@@ -2607,6 +2612,7 @@ struct IrInstructionSliceType {
26072612
IrInstruction *child_type;
26082613
bool is_const;
26092614
bool is_volatile;
2615+
bool allow_zero;
26102616
};
26112617

26122618
struct IrInstructionAsm {

‎src/analyze.cpp

+33-12
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,9 @@ ZigType *get_pointer_to_type_extra(CodeGen *g, ZigType *child_type, bool is_cons
433433
bool is_volatile, PtrLen ptr_len, uint32_t byte_alignment,
434434
uint32_t bit_offset_in_host, uint32_t host_int_bytes)
435435
{
436+
// TODO when implementing https://github.com/ziglang/zig/issues/1953
437+
// move this to a parameter
438+
bool allow_zero = (ptr_len == PtrLenC);
436439
assert(!type_is_invalid(child_type));
437440
assert(ptr_len != PtrLenUnknown || child_type->id != ZigTypeIdOpaque);
438441

@@ -452,7 +455,7 @@ ZigType *get_pointer_to_type_extra(CodeGen *g, ZigType *child_type, bool is_cons
452455

453456
TypeId type_id = {};
454457
ZigType **parent_pointer = nullptr;
455-
if (host_int_bytes != 0 || is_volatile || byte_alignment != 0 || ptr_len != PtrLenSingle) {
458+
if (host_int_bytes != 0 || is_volatile || byte_alignment != 0 || ptr_len != PtrLenSingle || allow_zero) {
456459
type_id.id = ZigTypeIdPointer;
457460
type_id.data.pointer.child_type = child_type;
458461
type_id.data.pointer.is_const = is_const;
@@ -461,6 +464,7 @@ ZigType *get_pointer_to_type_extra(CodeGen *g, ZigType *child_type, bool is_cons
461464
type_id.data.pointer.bit_offset_in_host = bit_offset_in_host;
462465
type_id.data.pointer.host_int_bytes = host_int_bytes;
463466
type_id.data.pointer.ptr_len = ptr_len;
467+
type_id.data.pointer.allow_zero = allow_zero;
464468

465469
auto existing_entry = g->type_table.maybe_get(type_id);
466470
if (existing_entry)
@@ -481,26 +485,38 @@ ZigType *get_pointer_to_type_extra(CodeGen *g, ZigType *child_type, bool is_cons
481485
const char *star_str = ptr_len_to_star_str(ptr_len);
482486
const char *const_str = is_const ? "const " : "";
483487
const char *volatile_str = is_volatile ? "volatile " : "";
488+
const char *allow_zero_str;
489+
if (ptr_len == PtrLenC) {
490+
assert(allow_zero);
491+
allow_zero_str = "";
492+
} else {
493+
allow_zero_str = allow_zero ? "allowzero " : "";
494+
}
484495
buf_resize(&entry->name, 0);
485496
if (host_int_bytes == 0 && byte_alignment == 0) {
486-
buf_appendf(&entry->name, "%s%s%s%s", star_str, const_str, volatile_str, buf_ptr(&child_type->name));
497+
buf_appendf(&entry->name, "%s%s%s%s%s",
498+
star_str, const_str, volatile_str, allow_zero_str, buf_ptr(&child_type->name));
487499
} else if (host_int_bytes == 0) {
488-
buf_appendf(&entry->name, "%salign(%" PRIu32 ") %s%s%s", star_str, byte_alignment,
489-
const_str, volatile_str, buf_ptr(&child_type->name));
500+
buf_appendf(&entry->name, "%salign(%" PRIu32 ") %s%s%s%s", star_str, byte_alignment,
501+
const_str, volatile_str, allow_zero_str, buf_ptr(&child_type->name));
490502
} else if (byte_alignment == 0) {
491-
buf_appendf(&entry->name, "%salign(:%" PRIu32 ":%" PRIu32 ") %s%s%s", star_str,
492-
bit_offset_in_host, host_int_bytes, const_str, volatile_str, buf_ptr(&child_type->name));
503+
buf_appendf(&entry->name, "%salign(:%" PRIu32 ":%" PRIu32 ") %s%s%s%s", star_str,
504+
bit_offset_in_host, host_int_bytes, const_str, volatile_str, allow_zero_str,
505+
buf_ptr(&child_type->name));
493506
} else {
494-
buf_appendf(&entry->name, "%salign(%" PRIu32 ":%" PRIu32 ":%" PRIu32 ") %s%s%s", star_str, byte_alignment,
495-
bit_offset_in_host, host_int_bytes, const_str, volatile_str, buf_ptr(&child_type->name));
507+
buf_appendf(&entry->name, "%salign(%" PRIu32 ":%" PRIu32 ":%" PRIu32 ") %s%s%s%s", star_str, byte_alignment,
508+
bit_offset_in_host, host_int_bytes, const_str, volatile_str, allow_zero_str,
509+
buf_ptr(&child_type->name));
496510
}
497511

498512
assert(child_type->id != ZigTypeIdInvalid);
499513

500514
entry->zero_bits = !type_has_bits(child_type);
501515

502516
if (!entry->zero_bits) {
503-
if (is_const || is_volatile || byte_alignment != 0 || ptr_len != PtrLenSingle || bit_offset_in_host != 0) {
517+
if (is_const || is_volatile || byte_alignment != 0 || ptr_len != PtrLenSingle ||
518+
bit_offset_in_host != 0 || allow_zero)
519+
{
504520
ZigType *peer_type = get_pointer_to_type_extra(g, child_type, false, false,
505521
PtrLenSingle, 0, 0, host_int_bytes);
506522
entry->type_ref = peer_type->type_ref;
@@ -534,6 +550,7 @@ ZigType *get_pointer_to_type_extra(CodeGen *g, ZigType *child_type, bool is_cons
534550
entry->data.pointer.explicit_alignment = byte_alignment;
535551
entry->data.pointer.bit_offset_in_host = bit_offset_in_host;
536552
entry->data.pointer.host_int_bytes = host_int_bytes;
553+
entry->data.pointer.allow_zero = allow_zero;
537554

538555
if (parent_pointer) {
539556
*parent_pointer = entry;
@@ -850,7 +867,7 @@ ZigType *get_slice_type(CodeGen *g, ZigType *ptr_type) {
850867

851868
ZigType *child_type = ptr_type->data.pointer.child_type;
852869
if (ptr_type->data.pointer.is_const || ptr_type->data.pointer.is_volatile ||
853-
ptr_type->data.pointer.explicit_alignment != 0)
870+
ptr_type->data.pointer.explicit_alignment != 0 || ptr_type->data.pointer.allow_zero)
854871
{
855872
ZigType *peer_ptr_type = get_pointer_to_type_extra(g, child_type, false, false,
856873
PtrLenUnknown, 0, 0, 0);
@@ -873,7 +890,7 @@ ZigType *get_slice_type(CodeGen *g, ZigType *ptr_type) {
873890
ZigType *child_ptr_type = child_type->data.structure.fields[slice_ptr_index].type_entry;
874891
assert(child_ptr_type->id == ZigTypeIdPointer);
875892
if (child_ptr_type->data.pointer.is_const || child_ptr_type->data.pointer.is_volatile ||
876-
child_ptr_type->data.pointer.explicit_alignment != 0)
893+
child_ptr_type->data.pointer.explicit_alignment != 0 || child_ptr_type->data.pointer.allow_zero)
877894
{
878895
ZigType *grand_child_type = child_ptr_type->data.pointer.child_type;
879896
ZigType *bland_child_ptr_type = get_pointer_to_type_extra(g, grand_child_type, false, false,
@@ -4053,7 +4070,9 @@ ZigType *get_src_ptr_type(ZigType *type) {
40534070
if (type->id == ZigTypeIdFn) return type;
40544071
if (type->id == ZigTypeIdPromise) return type;
40554072
if (type->id == ZigTypeIdOptional) {
4056-
if (type->data.maybe.child_type->id == ZigTypeIdPointer) return type->data.maybe.child_type;
4073+
if (type->data.maybe.child_type->id == ZigTypeIdPointer) {
4074+
return type->data.maybe.child_type->data.pointer.allow_zero ? nullptr : type->data.maybe.child_type;
4075+
}
40574076
if (type->data.maybe.child_type->id == ZigTypeIdFn) return type->data.maybe.child_type;
40584077
if (type->data.maybe.child_type->id == ZigTypeIdPromise) return type->data.maybe.child_type;
40594078
}
@@ -6289,6 +6308,7 @@ uint32_t type_id_hash(TypeId x) {
62896308
((x.data.pointer.ptr_len == PtrLenSingle) ? (uint32_t)1120226602 : (uint32_t)3200913342) +
62906309
(x.data.pointer.is_const ? (uint32_t)2749109194 : (uint32_t)4047371087) +
62916310
(x.data.pointer.is_volatile ? (uint32_t)536730450 : (uint32_t)1685612214) +
6311+
(x.data.pointer.allow_zero ? (uint32_t)3324284834 : (uint32_t)3584904923) +
62926312
(((uint32_t)x.data.pointer.alignment) ^ (uint32_t)0x777fbe0e) +
62936313
(((uint32_t)x.data.pointer.bit_offset_in_host) ^ (uint32_t)2639019452) +
62946314
(((uint32_t)x.data.pointer.host_int_bytes) ^ (uint32_t)529908881);
@@ -6339,6 +6359,7 @@ bool type_id_eql(TypeId a, TypeId b) {
63396359
a.data.pointer.ptr_len == b.data.pointer.ptr_len &&
63406360
a.data.pointer.is_const == b.data.pointer.is_const &&
63416361
a.data.pointer.is_volatile == b.data.pointer.is_volatile &&
6362+
a.data.pointer.allow_zero == b.data.pointer.allow_zero &&
63426363
a.data.pointer.alignment == b.data.pointer.alignment &&
63436364
a.data.pointer.bit_offset_in_host == b.data.pointer.bit_offset_in_host &&
63446365
a.data.pointer.host_int_bytes == b.data.pointer.host_int_bytes;

‎src/ir.cpp

+70-29
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ enum ConstCastResultId {
6161
ConstCastResultIdType,
6262
ConstCastResultIdUnresolvedInferredErrSet,
6363
ConstCastResultIdAsyncAllocatorType,
64-
ConstCastResultIdNullWrapPtr
64+
ConstCastResultIdBadAllowsZero,
6565
};
6666

6767
struct ConstCastOnly;
@@ -83,6 +83,7 @@ struct ConstCastErrUnionErrSetMismatch;
8383
struct ConstCastErrUnionPayloadMismatch;
8484
struct ConstCastErrSetMismatch;
8585
struct ConstCastTypeMismatch;
86+
struct ConstCastBadAllowsZero;
8687

8788
struct ConstCastOnly {
8889
ConstCastResultId id;
@@ -99,6 +100,7 @@ struct ConstCastOnly {
99100
ConstCastOnly *null_wrap_ptr_child;
100101
ConstCastArg fn_arg;
101102
ConstCastArgNoAlias arg_no_alias;
103+
ConstCastBadAllowsZero *bad_allows_zero;
102104
} data;
103105
};
104106

@@ -141,6 +143,12 @@ struct ConstCastErrSetMismatch {
141143
ZigList<ErrorTableEntry *> missing_errors;
142144
};
143145

146+
struct ConstCastBadAllowsZero {
147+
ZigType *wanted_type;
148+
ZigType *actual_type;
149+
};
150+
151+
144152
enum UndefAllowed {
145153
UndefOk,
146154
UndefBad,
@@ -8636,6 +8644,14 @@ static ZigType *get_error_set_intersection(IrAnalyze *ira, ZigType *set1, ZigTyp
86368644
return err_set_type;
86378645
}
86388646

8647+
static bool ptr_allows_addr_zero(ZigType *ptr_type) {
8648+
if (ptr_type->id == ZigTypeIdPointer) {
8649+
return ptr_type->data.pointer.allow_zero;
8650+
} else if (ptr_type->id == ZigTypeIdOptional) {
8651+
return true;
8652+
}
8653+
return false;
8654+
}
86398655

86408656
static ConstCastOnly types_match_const_cast_only(IrAnalyze *ira, ZigType *wanted_type,
86418657
ZigType *actual_type, AstNode *source_node, bool wanted_is_mutable)
@@ -8649,34 +8665,35 @@ static ConstCastOnly types_match_const_cast_only(IrAnalyze *ira, ZigType *wanted
86498665
if (wanted_type == actual_type)
86508666
return result;
86518667

8652-
// *T and [*]T may const-cast-only to ?*U and ?[*]U, respectively
8653-
// but not if we want a mutable pointer
8654-
// and not if the actual pointer has zero bits
8655-
if (!wanted_is_mutable && wanted_type->id == ZigTypeIdOptional &&
8656-
wanted_type->data.maybe.child_type->id == ZigTypeIdPointer &&
8657-
actual_type->id == ZigTypeIdPointer && type_has_bits(actual_type))
8658-
{
8659-
ConstCastOnly child = types_match_const_cast_only(ira,
8660-
wanted_type->data.maybe.child_type, actual_type, source_node, wanted_is_mutable);
8661-
if (child.id == ConstCastResultIdInvalid)
8662-
return child;
8663-
if (child.id != ConstCastResultIdOk) {
8664-
result.id = ConstCastResultIdNullWrapPtr;
8665-
result.data.null_wrap_ptr_child = allocate_nonzero<ConstCastOnly>(1);
8666-
*result.data.null_wrap_ptr_child = child;
8667-
}
8668-
return result;
8669-
}
8670-
8671-
// pointer const
8668+
// If pointers have the same representation in memory, they can be "const-casted".
8669+
// `const` attribute can be gained
8670+
// `volatile` attribute can be gained
8671+
// `allowzero` attribute can be gained (whether from explicit attribute, C pointer, or optional pointer)
8672+
// but only if !wanted_is_mutable
8673+
// alignment can be decreased
8674+
// bit offset attributes must match exactly
8675+
// PtrLenSingle/PtrLenUnknown must match exactly, but PtrLenC matches either one
86728676
ZigType *wanted_ptr_type = get_src_ptr_type(wanted_type);
86738677
ZigType *actual_ptr_type = get_src_ptr_type(actual_type);
8678+
bool wanted_allows_zero = ptr_allows_addr_zero(wanted_type);
8679+
bool actual_allows_zero = ptr_allows_addr_zero(actual_type);
86748680
bool wanted_is_c_ptr = wanted_type->id == ZigTypeIdPointer && wanted_type->data.pointer.ptr_len == PtrLenC;
86758681
bool actual_is_c_ptr = actual_type->id == ZigTypeIdPointer && actual_type->data.pointer.ptr_len == PtrLenC;
8676-
if ((wanted_type->id == ZigTypeIdPointer && actual_type->id == ZigTypeIdPointer) ||
8677-
(wanted_ptr_type != nullptr && actual_is_c_ptr) ||
8678-
(actual_ptr_type != nullptr && wanted_is_c_ptr))
8679-
{
8682+
bool wanted_opt_or_ptr = wanted_ptr_type != nullptr &&
8683+
(wanted_type->id == ZigTypeIdPointer || wanted_type->id == ZigTypeIdOptional);
8684+
bool actual_opt_or_ptr = actual_ptr_type != nullptr &&
8685+
(actual_type->id == ZigTypeIdPointer || actual_type->id == ZigTypeIdOptional);
8686+
if (wanted_opt_or_ptr && actual_opt_or_ptr) {
8687+
bool ok_allows_zero = (wanted_allows_zero &&
8688+
(actual_allows_zero || wanted_ptr_type->data.pointer.is_const)) ||
8689+
(!wanted_allows_zero && !actual_allows_zero);
8690+
if (!ok_allows_zero) {
8691+
result.id = ConstCastResultIdBadAllowsZero;
8692+
result.data.bad_allows_zero = allocate_nonzero<ConstCastBadAllowsZero>(1);
8693+
result.data.bad_allows_zero->wanted_type = wanted_type;
8694+
result.data.bad_allows_zero->actual_type = actual_type;
8695+
return result;
8696+
}
86808697
ConstCastOnly child = types_match_const_cast_only(ira, wanted_ptr_type->data.pointer.child_type,
86818698
actual_ptr_type->data.pointer.child_type, source_node, !wanted_ptr_type->data.pointer.is_const);
86828699
if (child.id == ConstCastResultIdInvalid)
@@ -8699,6 +8716,7 @@ static ConstCastOnly types_match_const_cast_only(IrAnalyze *ira, ZigType *wanted
86998716
}
87008717
bool ptr_lens_equal = actual_ptr_type->data.pointer.ptr_len == wanted_ptr_type->data.pointer.ptr_len;
87018718
if ((ptr_lens_equal || wanted_is_c_ptr || actual_is_c_ptr) &&
8719+
type_has_bits(wanted_type) == type_has_bits(actual_type) &&
87028720
(!actual_ptr_type->data.pointer.is_const || wanted_ptr_type->data.pointer.is_const) &&
87038721
(!actual_ptr_type->data.pointer.is_volatile || wanted_ptr_type->data.pointer.is_volatile) &&
87048722
actual_ptr_type->data.pointer.bit_offset_in_host == wanted_ptr_type->data.pointer.bit_offset_in_host &&
@@ -9922,7 +9940,7 @@ static ConstExprValue *ir_resolve_const(IrAnalyze *ira, IrInstruction *value, Un
99229940
if (undef_allowed == UndefOk) {
99239941
return &value->value;
99249942
} else {
9925-
ir_add_error(ira, value, buf_sprintf("use of undefined value"));
9943+
ir_add_error(ira, value, buf_sprintf("use of undefined value here causes undefined behavior"));
99269944
return nullptr;
99279945
}
99289946
}
@@ -10828,6 +10846,26 @@ static void report_recursive_error(IrAnalyze *ira, AstNode *source_node, ConstCa
1082810846
report_recursive_error(ira, source_node, cast_result->data.fn_arg.child, msg);
1082910847
break;
1083010848
}
10849+
case ConstCastResultIdBadAllowsZero: {
10850+
bool wanted_allows_zero = ptr_allows_addr_zero(cast_result->data.bad_allows_zero->wanted_type);
10851+
bool actual_allows_zero = ptr_allows_addr_zero(cast_result->data.bad_allows_zero->actual_type);
10852+
ZigType *wanted_ptr_type = get_src_ptr_type(cast_result->data.bad_allows_zero->wanted_type);
10853+
ZigType *actual_ptr_type = get_src_ptr_type(cast_result->data.bad_allows_zero->actual_type);
10854+
ZigType *wanted_elem_type = wanted_ptr_type->data.pointer.child_type;
10855+
ZigType *actual_elem_type = actual_ptr_type->data.pointer.child_type;
10856+
if (actual_allows_zero && !wanted_allows_zero) {
10857+
add_error_note(ira->codegen, parent_msg, source_node,
10858+
buf_sprintf("'%s' could have null values which are illegal in type '%s'",
10859+
buf_ptr(&actual_elem_type->name),
10860+
buf_ptr(&wanted_elem_type->name)));
10861+
} else {
10862+
add_error_note(ira->codegen, parent_msg, source_node,
10863+
buf_sprintf("mutable '%s' allows illegal null values stored to type '%s'",
10864+
buf_ptr(&cast_result->data.bad_allows_zero->wanted_type->name),
10865+
buf_ptr(&cast_result->data.bad_allows_zero->actual_type->name)));
10866+
}
10867+
break;
10868+
}
1083110869
case ConstCastResultIdFnAlign: // TODO
1083210870
case ConstCastResultIdFnCC: // TODO
1083310871
case ConstCastResultIdFnVarArgs: // TODO
@@ -10838,7 +10876,6 @@ static void report_recursive_error(IrAnalyze *ira, AstNode *source_node, ConstCa
1083810876
case ConstCastResultIdFnArgNoAlias: // TODO
1083910877
case ConstCastResultIdUnresolvedInferredErrSet: // TODO
1084010878
case ConstCastResultIdAsyncAllocatorType: // TODO
10841-
case ConstCastResultIdNullWrapPtr: // TODO
1084210879
break;
1084310880
}
1084410881
}
@@ -20589,12 +20626,14 @@ static IrInstruction *ir_analyze_ptr_cast(IrAnalyze *ira, IrInstruction *source_
2058920626
// We have a check for zero bits later so we use get_src_ptr_type to
2059020627
// validate src_type and dest_type.
2059120628

20592-
if (get_src_ptr_type(src_type) == nullptr) {
20629+
ZigType *src_ptr_type = get_src_ptr_type(src_type);
20630+
if (src_ptr_type == nullptr) {
2059320631
ir_add_error(ira, ptr, buf_sprintf("expected pointer, found '%s'", buf_ptr(&src_type->name)));
2059420632
return ira->codegen->invalid_instruction;
2059520633
}
2059620634

20597-
if (get_src_ptr_type(dest_type) == nullptr) {
20635+
ZigType *dest_ptr_type = get_src_ptr_type(dest_type);
20636+
if (dest_ptr_type == nullptr) {
2059820637
ir_add_error(ira, dest_type_src,
2059920638
buf_sprintf("expected pointer, found '%s'", buf_ptr(&dest_type->name)));
2060020639
return ira->codegen->invalid_instruction;
@@ -20606,6 +20645,8 @@ static IrInstruction *ir_analyze_ptr_cast(IrAnalyze *ira, IrInstruction *source_
2060620645
}
2060720646

2060820647
if (instr_is_comptime(ptr)) {
20648+
// Undefined is OK here; @ptrCast is defined to reinterpret the bit pattern
20649+
// of the pointer as the new pointer type.
2060920650
ConstExprValue *val = ir_resolve_const(ira, ptr, UndefOk);
2061020651
if (!val)
2061120652
return ira->codegen->invalid_instruction;

0 commit comments

Comments
 (0)