level-zero icon indicating copy to clipboard operation
level-zero copied to clipboard

zeKernelGetSourceAttributes: invalid ERROR_INVALID_NULL_POINTER with parameter validation

Open maleadt opened this issue 3 years ago • 2 comments

When invoking zeKernelGetSourceAttributes with a nullptr pString pointer to obtain the attribute size, https://github.com/intel/compute-runtime/blob/128cd8a31c16977ecc41bf13bdd35c2ac4907a5b/level_zero/core/source/kernel/kernel_imp.cpp#L443-L445, the validator erroneously throws an ERROR_INVALID_NULL_POINTER, https://github.com/oneapi-src/level-zero/blob/284ccb089184180e34864a9f1e23971d3d736bd8/source/layers/validation/ze_valddi.cpp#L3087-L3088

maleadt avatar May 04 '21 07:05 maleadt

https://spec.oneapi.com/level-zero/latest/core/api.html#zekernelgetsourceattributes

It looks like the implementation of this API in compute-runtime is not compliant with the specification.

  1. The memory for the string buffer should be owned by the driver library and have a lifetime tied to lifetime of the kernel object. The driver should be outputting a pointer to its internal string buffer, not copying memory into a user owned buffer.
  2. If implemented according to (1), then there's no need to obtain string size by setting pString to nullptr, you can just simply get the string pointer and then calculate the size in the application

Could you open an issue in compute-runtime github if you agree?

bmyates avatar May 04 '21 14:05 bmyates

@bmyates Wrong implementation is in the validation layer. Driver does this:

ze_result_t KernelImp::getSourceAttributes(uint32_t *pSize, char **pString) {
    auto &desc = kernelImmData->getDescriptor();
    if (pString == nullptr) {
        *pSize = (uint32_t)desc.kernelMetadata.kernelLanguageAttributes.length() + 1;
    } else {
        strncpy_s(*pString, desc.kernelMetadata.kernelLanguageAttributes.length() + 1,
                  desc.kernelMetadata.kernelLanguageAttributes.c_str(),
                  desc.kernelMetadata.kernelLanguageAttributes.length() + 1);
    }
    return ZE_RESULT_SUCCESS;
}

so effectively we just return size if string is null. However, the validation layer has this:

if( context.enableParameterValidation )
        {
            if( nullptr == hKernel )
                return ZE_RESULT_ERROR_INVALID_NULL_HANDLE;

            if( nullptr == pSize )
                return ZE_RESULT_ERROR_INVALID_NULL_POINTER;

            if( nullptr == pString )
                return ZE_RESULT_ERROR_INVALID_NULL_POINTER;

        }

which means we always fail if string is null.

jandres742 avatar May 04 '21 18:05 jandres742