Skip to content

Commit

Permalink
Fix DEADCODE Coverity issue
Browse files Browse the repository at this point in the history
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) {...".
  • Loading branch information
IlanTruanovsky authored and pcolberg committed Jan 30, 2023
1 parent 67e23e1 commit 8dee731
Showing 1 changed file with 28 additions and 31 deletions.
59 changes: 28 additions & 31 deletions test/acl_context_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1097,11 +1097,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;
Expand All @@ -1119,42 +1117,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"));
Expand Down

0 comments on commit 8dee731

Please sign in to comment.