Skip to content

Resolve address sanitizer issues in runtime unit test #154

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.

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

Merged
merged 7 commits into from
Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion src/acl_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -895,9 +895,11 @@ static cl_int l_init_context_with_devices(cl_context context,
} else {
num_absent++;
}
if (num_present && num_absent)
if (num_present && num_absent) {
acl_free(context->command_queue);
ERR_RET(CL_INVALID_DEVICE, context,
"Can't create a context with both offline and online devices");
}

usable = usable || acl_platform.offline_device ==
devices[i]->def.autodiscovery_def.name;
Expand Down
7 changes: 7 additions & 0 deletions src/acl_event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,13 @@ static void l_release_command_resources(acl_command_info_t &cmd) {

case CL_COMMAND_TASK:
case CL_COMMAND_NDRANGE_KERNEL:
if (cmd.info.ndrange_kernel.memory_migration.num_mem_objects != 0 &&
cmd.info.ndrange_kernel.memory_migration.src_mem_list) {
// src_mem should be user-provided buffers, users are responsible for
// releasing them Just free the src memory list here
acl_free(cmd.info.ndrange_kernel.memory_migration.src_mem_list);
cmd.info.ndrange_kernel.memory_migration.src_mem_list = nullptr;
}
// Cleanup is handled via the completion callback.
break;

Expand Down
5 changes: 4 additions & 1 deletion src/acl_kernel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,11 +345,14 @@ CL_API_ENTRY cl_int CL_API_CALL clSetKernelArgIntelFPGA(cl_kernel kernel,
"Non-memory object passed in as memory object argument");

} else if (arg_info->category == ACL_ARG_SAMPLER) {
if (arg_value && !acl_sampler_is_valid(*(cl_sampler *)arg_value))
if (arg_value && (arg_size != sizeof(cl_sampler) ||
!acl_sampler_is_valid(*(cl_sampler *)arg_value))) {
UNLOCK_ERR_RET(CL_INVALID_SAMPLER, context,
"Non-sampler object passed in as sampler object argument");
}
is_sampler = CL_TRUE;
} else if (arg_size != arg_info->size && arg_value &&
arg_size == sizeof(cl_sampler) &&
acl_sampler_is_valid(*(cl_sampler *)arg_value)) {
is_sampler = CL_TRUE;
}
Expand Down
21 changes: 21 additions & 0 deletions src/acl_mem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ static int acl_allocate_block(acl_block_allocation_t *block_allocation,
static int copy_image_metadata(cl_mem mem);
static void remove_mem_block_linked_list(acl_block_allocation_t *block);
static cl_bool is_image(cl_mem mem);
static void l_free_image_members(cl_mem mem);

cl_int acl_convert_image_format(const void *input_element, void *output_element,
cl_image_format format_from,
Expand Down Expand Up @@ -198,6 +199,9 @@ CL_API_ENTRY cl_int CL_API_CALL clReleaseMemObjectIntelFPGA(cl_mem mem) {
--mem->fields.buffer_objs.parent->fields.buffer_objs.num_subbuffers;
clReleaseMemObject(mem->fields.buffer_objs.parent);
} else {
if (is_image(mem)) {
l_free_image_members(mem);
}
// The only case wehre mem->region->is_user_provided && mem->host_mem.raw
// != NULL is when user creates a buffer with CL_MEM_USE_HOST_PTR set and
// the pointer is allocated with clSVMAlloc.
Expand Down Expand Up @@ -6247,6 +6251,23 @@ static cl_bool is_image(cl_mem mem) {
mem->mem_object_type == CL_MEM_OBJECT_IMAGE1D_BUFFER);
}

static void l_free_image_members(cl_mem mem) {
if (mem->fields.image_objs.image_format != NULL) {
acl_free(mem->fields.image_objs.image_format);
}
if (mem->fields.image_objs.image_desc != NULL) {
if (mem->fields.image_objs.image_desc->buffer != NULL) {
clReleaseMemObject(mem->fields.image_objs.image_desc->buffer);
mem->fields.image_objs.image_desc->buffer = NULL;
}
if (mem->fields.image_objs.image_desc->mem_object != NULL) {
clReleaseMemObject(mem->fields.image_objs.image_desc->mem_object);
mem->fields.image_objs.image_desc->mem_object = NULL;
}
acl_free(mem->fields.image_objs.image_desc);
}
}

void acl_copy_device_buffers_to_host_before_programming(
cl_context _context, unsigned int physical_device_id,
void(CL_CALLBACK *read_callback)(cl_mem, int)) {
Expand Down
18 changes: 16 additions & 2 deletions test/acl_command_queue_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,15 @@ MT_TEST(acl_command_queue, create) {
3 + sizeof(unsupported_props) / sizeof(cl_command_queue_properties);
CHECK_EQUAL(callback_num, m_callback_count);

// check if acl_command_queue_is_valid is working properly
ACL_LOCKED(CHECK(!acl_command_queue_is_valid(0)));
ACL_LOCKED(CHECK(!acl_command_queue_is_valid((cl_command_queue)&status)));
{
cl_command_queue fake_cq = acl_alloc_cl_command_queue();
assert(fake_cq);
fake_cq->magic = 0xDEADBEEFDEADBEEF;
ACL_LOCKED(CHECK(!acl_command_queue_is_valid(fake_cq)));
acl_free_cl_command_queue(fake_cq);
}

CHECK_EQUAL(CL_INVALID_COMMAND_QUEUE, clRetainCommandQueue(0));
CHECK_EQUAL(CL_INVALID_COMMAND_QUEUE, clReleaseCommandQueue(0));
Expand Down Expand Up @@ -356,8 +363,15 @@ MT_TEST(acl_command_queue, create_with_properties) {
sizeof(invalid_props) / sizeof(cl_command_queue_properties);
CHECK_EQUAL(callback_num, m_callback_count);

// check if acl_command_queue_is_valid is working properly
ACL_LOCKED(CHECK(!acl_command_queue_is_valid(0)));
ACL_LOCKED(CHECK(!acl_command_queue_is_valid((cl_command_queue)&status)));
{
cl_command_queue fake_cq = acl_alloc_cl_command_queue();
assert(fake_cq);
fake_cq->magic = 0xDEADBEEFDEADBEEF;
ACL_LOCKED(CHECK(!acl_command_queue_is_valid(fake_cq)));
acl_free_cl_command_queue(fake_cq);
}

CHECK_EQUAL(CL_INVALID_COMMAND_QUEUE, clRetainCommandQueue(0));
CHECK_EQUAL(CL_INVALID_COMMAND_QUEUE, clReleaseCommandQueue(0));
Expand Down
3 changes: 3 additions & 0 deletions test/acl_mem_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3394,6 +3394,8 @@ MT_TEST(acl_mem, map_svm_pointer) {
sizeof(uses_svm_pointer),
&uses_svm_pointer, NULL));
CHECK_EQUAL(CL_TRUE, uses_svm_pointer);

acl_free(host_pointer);
clReleaseMemObject(svm_mem);
}

Expand Down Expand Up @@ -3502,6 +3504,7 @@ MT_TEST(acl_mem, map_unmap_image) {
CHECK_EQUAL(acl_ref_count(image), refcnt);

clReleaseMemObject(image);
acl_free(input_ptr);

ACL_LOCKED(acl_print_debug_msg("end map_unmap_image\n"));
}
Expand Down
15 changes: 8 additions & 7 deletions test/acl_program_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1143,19 +1143,20 @@ MT_TEST(from_source, offline_mode_build_failure) {
CHECK_EQUAL(0, size_ret);

// Try to get the binaries.
unsigned char bins[1] = {'h'};
unsigned char bin[1] = {'h'};
unsigned char *bins[1] = {bin};
size_ret = 99;
CHECK_EQUAL(CL_SUCCESS, clGetProgramInfo(m_program, CL_PROGRAM_BINARIES, 0, 0,
&size_ret));
CHECK_EQUAL(sizeof(char *), size_ret);
CHECK_EQUAL(sizeof(bins), size_ret);
CHECK_EQUAL(CL_SUCCESS, clGetProgramInfo(m_program, CL_PROGRAM_BINARIES,
sizeof(char *), bins, 0));
sizeof(bins), bins, 0));
// Should not have overwritten!
CHECK_EQUAL('h', bins[0]);
bins[0] = 'i';
CHECK_EQUAL('h', bins[0][0]);
bins[0][0] = 'i';
CHECK_EQUAL(CL_SUCCESS, clGetProgramInfo(m_program, CL_PROGRAM_BINARIES,
sizeof(char *), bins, 0));
CHECK_EQUAL('i', bins[0]);
sizeof(bins), bins, 0));
CHECK_EQUAL('i', bins[0][0]);
}

MT_TEST(from_source, compile_program) {
Expand Down