From 1c82bd7e5c660ed21219bf0f6e8c44581e4b91c0 Mon Sep 17 00:00:00 2001 From: Ilan Truanovsky Date: Wed, 25 Jan 2023 13:43:00 -0800 Subject: [PATCH] Fix DEADCODE Coverity issue A typo was causing much of the code to be seen as deadcode. A loop was not looping the correct number of times. This commit makes changes so that the correct number of loop iterations are done. However, with just this change, the test will fail since clCreateContextFromType returns CL_INVALID_DEVICE on the iteration with ienv = 1, apitype = 1, and i = 0. The reasoning for why this happens in the test is as follows: 1. First, we set the environment to contain L_CONTEXT_OFFLINE_DEVICE_INTELFPGA=+a10_ref_small 2. Then, we initialize our 5 accelerators that we use in the test. Since we have L_CONTEXT_OFFLINE_DEVICE_INTELFPGA=+a10_ref_small in our environment, this is equivalent to adding a 6th offline accelerator to our system. 3. We then try to get our context by calling clCreateContextFromType. Note that we use CL_DEVICE_TYPE_ALL meaning we want all device types included in the context (i.e. doesn't matter if the devices are GPUs, CPUs, etc). 4. Inside the clCreateContextFromType call, we get our desired set of devices by calling clGetDeviceIDs and pass them to l_finalize_context. The devices that we get from clGetDeviceIDs include the offline device we added through the environment variable. 5. l_finalize_context calls l_init_context_with_devices with the same devices. 6. Finally, l_init_context_with_devices correctly errors out since we have a combination of present and absent (offline) devices. So, this commit also changes the code within the for loop to account for the differences between each iteration. These Coverity issues are fixed with this commit: test/acl_context_test.cpp:1116:9: Type: Logically dead code (DEADCODE) test/acl_context_test.cpp:1111:9: cond_const: Condition "apitype", taking false branch. Now the value of "apitype" is equal to 0. test/acl_context_test.cpp:1117:13: const: At condition "apitype", the value of "apitype" must be equal to 0. test/acl_context_test.cpp:1116:9: dead_error_condition: The condition "apitype" cannot be true. test/acl_context_test.cpp:1116:9: dead_error_line: Execution cannot reach the expression "clCreateContextFromType(props, 4294967295UL, notify_me, this, &status)" inside this statement: "context = (apitype ? clCrea...". test/acl_context_test.cpp:1125:11: Type: Logically dead code (DEADCODE) test/acl_context_test.cpp:1103:9: assignment: Assigning: "fixed_val" = "0L". test/acl_context_test.cpp:1124:13: const: At condition "fixed_val != 0L", the value of "fixed_val" must be equal to 0. test/acl_context_test.cpp:1124:9: dead_error_condition: The condition "fixed_val != 0L" cannot be true. test/acl_context_test.cpp:1125:11: dead_error_line: Execution cannot reach this statement: "if (!envvals[ienv].valid) {...". --- test/acl_context_test.cpp | 59 +++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/test/acl_context_test.cpp b/test/acl_context_test.cpp index c72fd70e..5b090a18 100644 --- a/test/acl_context_test.cpp +++ b/test/acl_context_test.cpp @@ -1096,11 +1096,9 @@ MT_TEST(Context, offline_device) { } syncThreads(); - - for (int apitype = 0; apitype < 1; apitype++) { // just do test by type. + for (int apitype = 0; apitype < 2; apitype++) { // just do test by type. for (int i = 0; i < 2; i++) { // without/with platform. int idx = 0; - cl_context_properties fixed_val = 0; if (i) { props[idx++] = CL_CONTEXT_PLATFORM; props[idx++] = (cl_context_properties)m_platform; @@ -1118,42 +1116,41 @@ MT_TEST(Context, offline_device) { ? clCreateContextFromType(props, CL_DEVICE_TYPE_ALL, notify_me, this, &status) : clCreateContext(props, 1, m_device, notify_me, this, &status); - ACL_LOCKED(acl_print_debug_msg(" fixed_val %d valid %d\n", - (int)fixed_val, - (int)envvals[ienv].valid)); - if (fixed_val != 0) { - if (!envvals[ienv].valid) { - CHECK_EQUAL(CL_INVALID_VALUE, status); - continue; - } - if (envvals[ienv].str != 0) { - ACL_LOCKED(acl_print_debug_msg(" %s vs. %s \n", (char *)fixed_val, - envvals[ienv].str)); - if (0 != strcmp((char *)fixed_val, envvals[ienv].str)) { - CHECK_EQUAL(CL_DEVICE_NOT_AVAILABLE, status); - continue; - } - } + ACL_LOCKED(acl_print_debug_msg("valid %d\n", (int)envvals[ienv].valid)); + + if (apitype && envvals[ienv].str) { + CHECK_EQUAL(CL_INVALID_DEVICE, status); + } else { + CHECK_EQUAL(CL_SUCCESS, status); } - CHECK_EQUAL(CL_SUCCESS, status); ACL_LOCKED(acl_print_debug_msg(" Got context %p\n", context)); cl_context_properties props_ret[20]; size_t size_ret = 0; - CHECK_EQUAL(CL_SUCCESS, - clGetContextInfo(context, CL_CONTEXT_PROPERTIES, - sizeof(props_ret), props_ret, &size_ret)); - CHECK_EQUAL(sizeof(cl_context_properties) * idx, size_ret); - int idx_ret = 0; - if (i) { - CHECK_EQUAL(CL_CONTEXT_PLATFORM, props_ret[idx_ret++]); - CHECK_EQUAL((cl_context_properties)m_platform, props_ret[idx_ret++]); - } + if (apitype && envvals[ienv].str) { + CHECK_EQUAL( + CL_INVALID_CONTEXT, + clGetContextInfo(context, CL_CONTEXT_PROPERTIES, + sizeof(props_ret), props_ret, + &size_ret)); // props_ret and size_ret is not + // guaranteed to contain anything + } else { + CHECK_EQUAL(CL_SUCCESS, clGetContextInfo( + context, CL_CONTEXT_PROPERTIES, + sizeof(props_ret), props_ret, &size_ret)); + CHECK_EQUAL(sizeof(cl_context_properties) * idx, size_ret); + int idx_ret = 0; + if (i) { + CHECK_EQUAL(CL_CONTEXT_PLATFORM, props_ret[idx_ret++]); + CHECK_EQUAL((cl_context_properties)m_platform, + props_ret[idx_ret++]); + } - CHECK_EQUAL(0, props_ret[idx_ret++]); - CHECK_EQUAL(idx, idx_ret); + CHECK_EQUAL(0, props_ret[idx_ret++]); + CHECK_EQUAL(idx, idx_ret); + } syncThreads(); ACL_LOCKED(acl_print_debug_msg(" Releaseing context\n"));