Skip to content

Fixes for zero-sized allocation #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fixes for zero-sized allocation #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/analyze.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ static void resolve_union_zero_bits(CodeGen *g, TypeTableEntry *union_type);
static void analyze_fn_body(CodeGen *g, FnTableEntry *fn_table_entry);

ErrorMsg *add_node_error(CodeGen *g, AstNode *node, Buf *msg) {
assert(node != nullptr);
if (node->owner->c_import_node != nullptr) {
// if this happens, then translate_c generated code that
// failed semantic analysis, which isn't supposed to happen
Expand Down Expand Up @@ -478,7 +479,7 @@ TypeTableEntry *get_promise_frame_type(CodeGen *g, TypeTableEntry *return_type)
TypeTableEntry *awaiter_handle_type = get_maybe_type(g, g->builtin_types.entry_promise);
TypeTableEntry *result_ptr_type = get_pointer_to_type(g, return_type, false);

ZigList<const char *> field_names = {};
ZigList<const char *> field_names = {0};
field_names.append(AWAITER_HANDLE_FIELD_NAME);
field_names.append(RESULT_FIELD_NAME);
field_names.append(RESULT_PTR_FIELD_NAME);
Expand All @@ -488,7 +489,7 @@ TypeTableEntry *get_promise_frame_type(CodeGen *g, TypeTableEntry *return_type)
field_names.append(RETURN_ADDRESSES_FIELD_NAME);
}

ZigList<TypeTableEntry *> field_types = {};
ZigList<TypeTableEntry *> field_types = {0};
field_types.append(awaiter_handle_type);
field_types.append(return_type);
field_types.append(result_ptr_type);
Expand Down Expand Up @@ -1848,10 +1849,10 @@ static void resolve_struct_type(CodeGen *g, TypeTableEntry *struct_type) {
}

assert(!struct_type->data.structure.zero_bits_loop_flag);
assert(struct_type->data.structure.fields);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failed assertion.

assert(decl_node->type == NodeTypeContainerDecl);

size_t field_count = struct_type->data.structure.src_field_count;
assert(struct_type->data.structure.fields != nullptr || field_count == 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempted tweak.


size_t gen_field_count = struct_type->data.structure.gen_field_count;
LLVMTypeRef *element_types = allocate<LLVMTypeRef>(gen_field_count);
Expand Down Expand Up @@ -1964,7 +1965,7 @@ static void resolve_struct_type(CodeGen *g, TypeTableEntry *struct_type) {
struct_type->di_type = replacement_di_type;
return;
}
assert(struct_type->di_type);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failed assertion.

assert(struct_type->di_type != nullptr || field_count == 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempted tweak.



// the count may have been adjusting from packing bit fields
Expand All @@ -1976,7 +1977,8 @@ static void resolve_struct_type(CodeGen *g, TypeTableEntry *struct_type) {
// if you hit this assert then probably this type or a related type didn't
// get ensure_complete_type called on it before using it with something that
// requires a complete type
assert(LLVMStoreSizeOfType(g->target_data_ref, struct_type->type_ref) > 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failed assertion.

assert(LLVMStoreSizeOfType(g->target_data_ref, struct_type->type_ref) > 0 ||
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempted tweak.

field_count == 0);

ZigLLVMDIType **di_element_types = allocate<ZigLLVMDIType*>(debug_field_count);

Expand Down Expand Up @@ -4238,6 +4240,7 @@ TypeTableEntry **get_int_type_ptr(CodeGen *g, bool is_signed, uint32_t size_in_b
}

TypeTableEntry *get_int_type(CodeGen *g, bool is_signed, uint32_t size_in_bits) {
assert(g != nullptr);
TypeTableEntry **common_entry = get_int_type_ptr(g, is_signed, size_in_bits);
if (common_entry)
return *common_entry;
Expand Down Expand Up @@ -4326,7 +4329,7 @@ static ZigWindowsSDK *get_windows_sdk(CodeGen *g) {
Buf *get_linux_libc_lib_path(const char *o_file) {
const char *cc_exe = getenv("CC");
cc_exe = (cc_exe == nullptr) ? "cc" : cc_exe;
ZigList<const char *> args = {};
ZigList<const char *> args = {0};
args.append(buf_ptr(buf_sprintf("-print-file-name=%s", o_file)));
Termination term;
Buf *out_stderr = buf_alloc();
Expand All @@ -4352,7 +4355,7 @@ Buf *get_linux_libc_lib_path(const char *o_file) {
Buf *get_linux_libc_include_path(void) {
const char *cc_exe = getenv("CC");
cc_exe = (cc_exe == nullptr) ? "cc" : cc_exe;
ZigList<const char *> args = {};
ZigList<const char *> args = {0};
args.append("-E");
args.append("-Wp,-v");
args.append("-xc");
Expand All @@ -4368,7 +4371,7 @@ Buf *get_linux_libc_include_path(void) {
zig_panic("unable to determine libc include path: executing C compiler command failed");
}
char *prev_newline = buf_ptr(out_stderr);
ZigList<const char *> search_paths = {};
ZigList<const char *> search_paths = {0};
bool found_search_paths = false;
for (;;) {
char *newline = strchr(prev_newline, '\n');
Expand Down
1 change: 1 addition & 0 deletions src/buffer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ static inline size_t buf_len(Buf *buf) {
}

static inline char *buf_ptr(Buf *buf) {
assert(buf != nullptr);
assert(buf->list.length);
return buf->list.items;
}
Expand Down
26 changes: 13 additions & 13 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4363,7 +4363,7 @@ static LLVMValueRef get_coro_alloc_helper_fn_val(CodeGen *g, LLVMTypeRef alloc_f
LLVMTypeRef *alloc_fn_arg_types = allocate<LLVMTypeRef>(LLVMCountParamTypes(alloc_raw_fn_type_ref));
LLVMGetParamTypes(alloc_raw_fn_type_ref, alloc_fn_arg_types);

ZigList<LLVMTypeRef> arg_types = {};
ZigList<LLVMTypeRef> arg_types = {0};
arg_types.append(alloc_fn_type_ref);
if (g->have_err_ret_tracing) {
arg_types.append(alloc_fn_arg_types[1]);
Expand Down Expand Up @@ -4415,10 +4415,10 @@ static LLVMValueRef get_coro_alloc_helper_fn_val(CodeGen *g, LLVMTypeRef alloc_f
LLVMValueRef alignment_val = LLVMConstInt(g->builtin_types.entry_u29->type_ref,
get_coro_frame_align_bytes(g), false);

ZigList<LLVMValueRef> args = {};
ZigList<LLVMValueRef> args = {0};
args.append(sret_ptr);
if (g->have_err_ret_tracing) {
args.append(stack_trace_val);
args.append(stack_trace_val); // NOLINT
}
args.append(allocator_val);
args.append(coro_size);
Expand Down Expand Up @@ -4463,7 +4463,7 @@ static LLVMValueRef ir_render_coro_alloc_helper(CodeGen *g, IrExecutable *execut
size_t err_code_ptr_arg_index = get_async_err_code_arg_index(g, &g->cur_fn->type_entry->data.fn.fn_type_id);
size_t allocator_arg_index = get_async_allocator_arg_index(g, &g->cur_fn->type_entry->data.fn.fn_type_id);

ZigList<LLVMValueRef> params = {};
ZigList<LLVMValueRef> params = {0};
params.append(alloc_fn);
uint32_t err_ret_trace_arg_index = get_err_ret_trace_arg_index(g, g->cur_fn);
if (err_ret_trace_arg_index != UINT32_MAX) {
Expand Down Expand Up @@ -5922,20 +5922,20 @@ static const uint8_t int_sizes_in_bits[] = {
};

struct CIntTypeInfo {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Padding fix discussed in #993.

CIntType id;
const char *name;
CIntType id;
bool is_signed;
};

static const CIntTypeInfo c_int_type_infos[] = {
{CIntTypeShort, "c_short", true},
{CIntTypeUShort, "c_ushort", false},
{CIntTypeInt, "c_int", true},
{CIntTypeUInt, "c_uint", false},
{CIntTypeLong, "c_long", true},
{CIntTypeULong, "c_ulong", false},
{CIntTypeLongLong, "c_longlong", true},
{CIntTypeULongLong, "c_ulonglong", false},
{"c_short", CIntTypeShort, true},
{"c_ushort", CIntTypeUShort, false},
{"c_int", CIntTypeInt, true},
{"c_uint", CIntTypeUInt, false},
{"c_long", CIntTypeLong, true},
{"c_ulong", CIntTypeULong, false},
{"c_longlong", CIntTypeLongLong, true},
{"c_ulonglong", CIntTypeULongLong, false},
};

static const bool is_signed_list[] = { false, true, };
Expand Down
2 changes: 1 addition & 1 deletion src/ir.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7647,7 +7647,7 @@ static TypeTableEntry *get_error_set_intersection(IrAnalyze *ira, TypeTableEntry
assert(errors[error_entry->value] == nullptr);
errors[error_entry->value] = error_entry;
}
ZigList<ErrorTableEntry *> intersection_list = {};
ZigList<ErrorTableEntry *> intersection_list = {0};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will see this in a few places. ZigList needs to be initialized in order to guarantee the members are zeroed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you cite a source showing that {0} is necessary and {} is not sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you are right: https://stackoverflow.com/a/1069634/1930331. Maybe I'm thinking about C, but C++ will work.


TypeTableEntry *err_set_type = new_type_table_entry(TypeTableEntryIdErrorSet);
buf_resize(&err_set_type->name, 0);
Expand Down
2 changes: 1 addition & 1 deletion src/link.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ static void add_rpath(LinkJob *lj, Buf *rpath) {
static Buf *try_dynamic_linker_path(const char *ld_name) {
const char *cc_exe = getenv("CC");
cc_exe = (cc_exe == nullptr) ? "cc" : cc_exe;
ZigList<const char *> args = {};
ZigList<const char *> args = {0};
args.append(buf_ptr(buf_sprintf("-print-file-name=%s", ld_name)));
Termination term;
Buf *out_stderr = buf_alloc();
Expand Down
21 changes: 13 additions & 8 deletions src/util.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ static inline int clzll(unsigned long long mask) {

template<typename T>
ATTRIBUTE_RETURNS_NOALIAS static inline T *allocate_nonzero(size_t count) {
if (count == 0) {
return nullptr;
}
T *ptr = reinterpret_cast<T*>(malloc(count * sizeof(T)));
if (!ptr)
zig_panic("allocation failed");
Expand All @@ -73,6 +76,9 @@ ATTRIBUTE_RETURNS_NOALIAS static inline T *allocate_nonzero(size_t count) {

template<typename T>
ATTRIBUTE_RETURNS_NOALIAS static inline T *allocate(size_t count) {
if (count == 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes behavior for zero sized allocation portable.

return nullptr;
}
T *ptr = reinterpret_cast<T*>(calloc(count, sizeof(T)));
if (!ptr)
zig_panic("allocation failed");
Expand All @@ -92,21 +98,20 @@ static inline void safe_memcpy(T *dest, const T *src, size_t count) {
}

template<typename T>
static inline T *reallocate(T *old, size_t old_count, size_t new_count) {
static inline T *reallocate_nonzero(T *old, size_t old_count, size_t new_count) {
T *ptr = reinterpret_cast<T*>(realloc(old, new_count * sizeof(T)));
if (!ptr)
zig_panic("allocation failed");
if (new_count > old_count) {
memset(&ptr[old_count], 0, (new_count - old_count) * sizeof(T));
}
return ptr;
}

template<typename T>
static inline T *reallocate_nonzero(T *old, size_t old_count, size_t new_count) {
T *ptr = reinterpret_cast<T*>(realloc(old, new_count * sizeof(T)));
if (!ptr)
zig_panic("allocation failed");
static inline T *reallocate(T *old, size_t old_count, size_t new_count) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implement reallocate using reallocate_nonzero to avoid code duplication.

T* ptr = reallocate_nonzero(old, old_count, new_count);
assert(ptr != nullptr);
if (new_count > old_count) {
memset(&ptr[old_count], 0, (new_count - old_count) * sizeof(T));
}
return ptr;
}

Expand Down