Klocwork check null before dereference in acl_profiler.cpp by haoxian2 · Pull Request #203 · intel/fpga-runtime-for-opencl (original) (raw)

This repository was archived by the owner on Mar 2, 2026. It is now read-only.

Merged

pcolberg

merged 1 commit into

Nov 23, 2022

Conversation

Fixed the following Klocwork issue:

1. Pointer 'accel_def' returned from call to function 'acl_find_accel_def' at line 605 may be NULL and may be dereferenced at line 620.

acl_find_accl_def bails out if the return value would be NULL, therefore ensuring that accel_def can never be NULL if status == CL_SUCCESS. So an assert statement is included instead after the status check, which would rule out NULL return values.

Choose a reason for hiding this comment

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

Thanks @haoxian2! This one needs a bit more investigation as detailed below.

accel_def receives the return value of acl_find_accl_def, but acl_find_accl_def also sets the status variable. There's a line in acl_find_accl_def that specifically says if the return value would be NULL, then status would be set to CL_INVALID_KERNEL_NAME and it would return 0. Therefore, the subsequent status check would not pass if acl_find_accl_def would return a NULL. The assert inserted would only be reached if status == CL_SUCCESS.

acl_find_accel_def ensures that the returned value is not NULL.

accel_def receives the return value of acl_find_accl_def, but acl_find_accl_def also sets the status variable. There's a line in acl_find_accl_def that specifically says if the return value would be NULL, then status would be set to CL_INVALID_KERNEL_NAME and it would return 0. Therefore, the subsequent status check would not pass if acl_find_accl_def would return a NULL. The assert inserted would only be reached if status == CL_SUCCESS.

Thanks @haoxian2 for the detailed explanation and my apologies for missing this the first time, which you had already confirmed in the commit message.

Choose a reason for hiding this comment

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

Thanks @haoxian2. I pushed a minor revision of the commit message, adding a tag in the subject and moving the reasoning to the body to keep the subject brief.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Labels

bug

Something isn't working

2 participants