oneapi-src / level-zero

oneAPI Level Zero Specification Headers and Loader
https://spec.oneapi.com/versions/latest/elements/l0/source/index.html
MIT License
208 stars 90 forks source link

zeKernelGetSourceAttributes: invalid ERROR_INVALID_NULL_POINTER with parameter validation #55

Open maleadt opened 3 years ago

maleadt commented 3 years ago

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

bmyates commented 3 years ago

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?

jandres742 commented 3 years ago

@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.