Zebin: return error on parsing empty vector entries in zeInfo

This commit prevents a yaml parsing error in case a data type is passed after empty vetor type data entry with the same indendation. In this case, a parsing error will be returned. - Corrected .ze_info section in valid empty program (zebin mock) - Minor ults refactor in order to use mock zebin program with valid .ze_info Related-To: NEO-6735 Signed-off-by: Kacper Nowak <kacper.nowak@intel.com>
2026-01-08 14:02:58 +08:00 · 2022-04-07 13:20:02 +00:00
parent 002e6d5205
commit 1a33fc0dc7
11 changed files with 74 additions and 12 deletions
--- a/level_zero/core/test/unit_tests/fixtures/module_fixture.h
+++ b/level_zero/core/test/unit_tests/fixtures/module_fixture.h
@@ -305,7 +305,7 @@ struct ModuleWithZebinFixture : public DeviceFixture {
        MockImmutableData(L0::Device *device) {

            auto mockKernelDescriptor = new NEO::KernelDescriptor;
-            mockKernelDescriptor->kernelMetadata.kernelName = "kernel";
+            mockKernelDescriptor->kernelMetadata.kernelName = ZebinTestData::ValidEmptyProgram::kernelName;
            kernelDescriptor = mockKernelDescriptor;
            this->device = device;
            isaGraphicsAllocation.reset(new NEO::MockGraphicsAllocation(0,
--- a/level_zero/core/test/unit_tests/sources/debugger/test_module_with_debug.cpp
+++ b/level_zero/core/test/unit_tests/sources/debugger/test_module_with_debug.cpp
@@ -300,6 +300,7 @@ TEST_F(ModuleWithSLDTest, GivenDebugDataWithMultipleRelocationsWhenInitializingM
 using ModuleWithZebinAndSLDTest = Test<ModuleWithZebinFixture>;
 TEST_F(ModuleWithZebinAndSLDTest, GivenZebinThenCreateDebugZebinAndPassToSLD) {
    module->addEmptyZebin();
+    module->addSegments();

    auto debugger = new MockActiveSourceLevelDebugger(new MockOsLibrary);
    neoDevice->getExecutionEnvironment()->rootDeviceEnvironments[0]->debugger.reset(debugger);
@@ -560,6 +561,7 @@ HWTEST_F(ModuleWithZebinAndL0DebuggerTest, GivenZebinDebugDataWhenInitializingMo
    auto kernelInfo = std::make_unique<KernelInfo>();
    kernelInfo->heapInfo.KernelHeapSize = 1;
    kernelInfo->heapInfo.pKernelHeap = &kernelHeap;
+    kernelInfo->kernelDescriptor.kernelMetadata.kernelName = ZebinTestData::ValidEmptyProgram::kernelName;

    auto kernelImmutableData = ::std::make_unique<KernelImmutableData>(device);
    kernelImmutableData->initialize(kernelInfo.get(), device, 0, nullptr, nullptr, false);
@@ -615,6 +617,7 @@ HWTEST_F(ModuleWithZebinAndL0DebuggerTest, GivenZebinWhenModuleIsInitializedAndD
    auto kernelInfo = std::make_unique<KernelInfo>();
    kernelInfo->heapInfo.KernelHeapSize = 1;
    kernelInfo->heapInfo.pKernelHeap = &kernelHeap;
+    kernelInfo->kernelDescriptor.kernelMetadata.kernelName = ZebinTestData::ValidEmptyProgram::kernelName;

    auto kernelImmutableData = ::std::make_unique<KernelImmutableData>(device);
    kernelImmutableData->initialize(kernelInfo.get(), device, 0, nullptr, nullptr, false);
@@ -659,6 +662,7 @@ HWTEST_F(ModuleWithZebinAndL0DebuggerTest, GivenModuleDebugHandleZeroWhenInitial
    auto kernelInfo = std::make_unique<KernelInfo>();
    kernelInfo->heapInfo.KernelHeapSize = 1;
    kernelInfo->heapInfo.pKernelHeap = &kernelHeap;
+    kernelInfo->kernelDescriptor.kernelMetadata.kernelName = ZebinTestData::ValidEmptyProgram::kernelName;

    auto kernelImmutableData = ::std::make_unique<KernelImmutableData>(device);
    kernelImmutableData->initialize(kernelInfo.get(), device, 0, nullptr, nullptr, false);
--- a/level_zero/core/test/unit_tests/sources/module/test_module.cpp
+++ b/level_zero/core/test/unit_tests/sources/module/test_module.cpp
@@ -2597,7 +2597,7 @@ TEST_F(ModuleWithZebinTest, givenZebinSegmentsThenSegmentsArePopulated) {
    };
    checkGPUSeg(module->translationUnit->globalConstBuffer, segments.constData);
    checkGPUSeg(module->translationUnit->globalConstBuffer, segments.varData);
-    checkGPUSeg(module->kernelImmDatas[0]->getIsaGraphicsAllocation(), segments.nameToSegMap["kernel"]);
+    checkGPUSeg(module->kernelImmDatas[0]->getIsaGraphicsAllocation(), segments.nameToSegMap[ZebinTestData::ValidEmptyProgram::kernelName]);

    EXPECT_EQ(reinterpret_cast<uintptr_t>(module->translationUnit->programInfo.globalStrings.initData), segments.stringData.address);
    EXPECT_EQ(module->translationUnit->programInfo.globalStrings.size, segments.stringData.size);
@@ -2605,6 +2605,7 @@ TEST_F(ModuleWithZebinTest, givenZebinSegmentsThenSegmentsArePopulated) {

 TEST_F(ModuleWithZebinTest, givenValidZebinWhenGettingDebugInfoThenDebugZebinIsCreatedAndReturned) {
    module->addEmptyZebin();
+    module->addSegments();
    size_t debugDataSize;
    module->getDebugInfo(&debugDataSize, nullptr);
    auto debugData = std::make_unique<uint8_t[]>(debugDataSize);
@@ -2616,6 +2617,7 @@ TEST_F(ModuleWithZebinTest, givenValidZebinWhenGettingDebugInfoThenDebugZebinIsC

 TEST_F(ModuleWithZebinTest, givenValidZebinAndPassedDataSmallerThanDebugDataThenErrorIsReturned) {
    module->addEmptyZebin();
+    module->addSegments();
    size_t debugDataSize;
    module->getDebugInfo(&debugDataSize, nullptr);
    auto debugData = std::make_unique<uint8_t[]>(debugDataSize);
--- a/opencl/test/unit_test/offline_compiler/ocloc_validator_tests.cpp
+++ b/opencl/test/unit_test/offline_compiler/ocloc_validator_tests.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020-2021 Intel Corporation
+ * Copyright (C) 2020-2022 Intel Corporation
 *
 * SPDX-License-Identifier: MIT
 *
@@ -89,7 +89,7 @@ TEST(OclocValidate, WhenErrorsEmitedThenRedirectsThemToStdout) {
    int res = NEO::Ocloc::validate({"-file", "src.gen"}, &argHelper);
    std::string oclocStdout = argHelper.getPrinterRef().getLog().str();
    EXPECT_EQ(static_cast<int>(NEO::DecodeError::InvalidBinary), res) << oclocStdout;
-    EXPECT_NE(nullptr, strstr(oclocStdout.c_str(), "Validator detected errors :\nDeviceBinaryFormat::Zebin::.ze_info : Expected at most one kernels entry in global scope of .ze_info, got : 2")) << oclocStdout;
+    EXPECT_NE(nullptr, strstr(oclocStdout.c_str(), "Validator detected errors :\nNEO::Yaml : Could not parse line : [1] : [kernels ] <-- parser position on error. Reason : Vector data type expects to have at least one value starting with -")) << oclocStdout;
 }

 TEST(OclocValidate, givenDeviceProductTableEveryProductMatchesProperPattern) {
--- a/opencl/test/unit_test/program/program_with_zebin.cpp
+++ b/opencl/test/unit_test/program/program_with_zebin.cpp
@@ -36,7 +36,7 @@ void ProgramWithZebinFixture::addEmptyZebin(NEO::MockProgram *program) {

 void ProgramWithZebinFixture::populateProgramWithSegments(NEO::MockProgram *program) {
    kernelInfo = std::make_unique<KernelInfo>();
-    kernelInfo->kernelDescriptor.kernelMetadata.kernelName = kernelName;
+    kernelInfo->kernelDescriptor.kernelMetadata.kernelName = ZebinTestData::ValidEmptyProgram::kernelName;
    mockAlloc = std::make_unique<MockGraphicsAllocation>();
    kernelInfo->kernelAllocation = mockAlloc.get();

--- a/opencl/test/unit_test/program/program_with_zebin.h
+++ b/opencl/test/unit_test/program/program_with_zebin.h
@@ -20,7 +20,6 @@ class ProgramWithZebinFixture : public ProgramTests {
    std::unique_ptr<MockBuffer> globalSurface;
    std::unique_ptr<MockBuffer> constantSurface;
    const char strings[12] = "Hello olleH";
-    const char kernelName[8] = "kernel1";
    void SetUp() override;
    void TearDown() override;
    void addEmptyZebin(MockProgram *program);
--- a/opencl/test/unit_test/program/program_with_zebin_tests.cpp
+++ b/opencl/test/unit_test/program/program_with_zebin_tests.cpp
@@ -7,6 +7,7 @@

 #include "shared/test/common/mocks/mock_source_level_debugger.h"
 #include "shared/test/common/test_macros/test.h"
+#include "shared/test/unit_test/device_binary_format/zebin_tests.h"

 #include "opencl/test/unit_test/mocks/mock_buffer.h"
 #include "opencl/test/unit_test/program/program_with_zebin.h"
@@ -39,7 +40,7 @@ TEST_F(ProgramWithZebinFixture, givenZebinSegmentsThenSegmentsArePopulated) {
    };
    checkGPUSeg(program->buildInfos[rootDeviceIndex].constantSurface, segments.constData);
    checkGPUSeg(program->buildInfos[rootDeviceIndex].globalSurface, segments.varData);
-    checkGPUSeg(program->getKernelInfoArray(rootDeviceIndex)[0]->getGraphicsAllocation(), segments.nameToSegMap["kernel1"]);
+    checkGPUSeg(program->getKernelInfoArray(rootDeviceIndex)[0]->getGraphicsAllocation(), segments.nameToSegMap[ZebinTestData::ValidEmptyProgram::kernelName]);

    EXPECT_EQ(reinterpret_cast<uintptr_t>(program->buildInfos[rootDeviceIndex].constStringSectionData.initData), segments.stringData.address);
    EXPECT_EQ(reinterpret_cast<const char *>(program->buildInfos[rootDeviceIndex].constStringSectionData.initData), strings);
--- a/shared/source/device_binary_format/yaml/yaml_parser.cpp
+++ b/shared/source/device_binary_format/yaml/yaml_parser.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020-2021 Intel Corporation
+ * Copyright (C) 2020-2022 Intel Corporation
 *
 * SPDX-License-Identifier: MIT
 *
@@ -340,9 +340,18 @@ void finalizeNode(NodeId nodeId, const TokensCache &tokens, NodesCache &outNodes
    ++node.numChildren;
 }

+bool isEmptyVector(const Token &token, size_t lineId, std::string &outError) {
+    if (isVectorDataType(token)) {
+        outError = constructYamlError(lineId, token.pos, token.pos + token.len, "Vector data type expects to have at least one value starting with -");
+        return false;
+    }
+    return true;
+}
+
 bool buildTree(const LinesCache &lines, const TokensCache &tokens, NodesCache &outNodes, std::string &outErrReason, std::string &outWarning) {
    StackVec<NodeId, 64> nesting;
    size_t lineId = 0U;
+    size_t lastUsedLine = 0u;
    outNodes.resize(1);
    outNodes.rbegin()->id = 0U;
    outNodes.rbegin()->firstChildId = 1U;
@@ -358,6 +367,9 @@ bool buildTree(const LinesCache &lines, const TokensCache &tokens, NodesCache &o

        auto currLineIndent = lines[lineId].indent;
        if (currLineIndent == outNodes.rbegin()->indent) {
+            if (lineId > 0u && false == isEmptyVector(tokens[lines[lastUsedLine].first], lastUsedLine, outErrReason)) {
+                return false;
+            }
            outNodes.reserve(outNodes.size() + 1);
            auto &prev = *outNodes.rbegin();
            auto &parent = outNodes[*nesting.rbegin()];
@@ -426,6 +438,7 @@ bool buildTree(const LinesCache &lines, const TokensCache &tokens, NodesCache &o
                outNodes.rbegin()->value = lines[lineId].first + 1;
            }
        }
+        lastUsedLine = lineId;
        ++lineId;
    }

@@ -433,7 +446,6 @@ bool buildTree(const LinesCache &lines, const TokensCache &tokens, NodesCache &o
        finalizeNode(*nesting.rbegin(), tokens, outNodes, outErrReason, outWarning);
        nesting.pop_back();
    }
-
    if (1U == outNodes.size()) {
        outWarning.append("NEO::Yaml : Text has no data\n");
        outNodes.clear();
--- a/shared/source/device_binary_format/yaml/yaml_parser.h
+++ b/shared/source/device_binary_format/yaml/yaml_parser.h
@@ -11,6 +11,7 @@
 #include "shared/source/utilities/const_stringref.h"
 #include "shared/source/utilities/stackvec.h"

+#include <array>
 #include <iterator>
 #include <string>

@@ -205,6 +206,24 @@ constexpr bool operator!=(char matcher, Token token) {
    return token != matcher;
 }

+constexpr bool isVectorDataType(const Token &token) {
+    auto tokenString = ConstStringRef(token.pos, token.len);
+    constexpr std::array<const char *, 7> vectorAttributesNames = {
+        "kernels",
+        "functions",
+        "global_host_access_table",
+        "payload_arguments",
+        "per_thread_payload_arguments",
+        "binding_table_indices",
+        "per_thread_memory_buffers"};
+    for (const auto &type : vectorAttributesNames) {
+        if (equals(tokenString, type)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 struct Line {
    enum class LineType : uint8_t { Empty,
                                    Comment,
--- a/shared/test/unit_test/device_binary_format/zebin_decoder_tests.cpp
+++ b/shared/test/unit_test/device_binary_format/zebin_decoder_tests.cpp
@@ -2567,7 +2567,7 @@ TEST(DecodeSingleDeviceBinaryZebin, GivenEmptyInZeInfoThenEmitsWarning) {
 TEST(DecodeSingleDeviceBinaryZebin, GivenUnknownEntryInZeInfoGlobalScopeThenEmitsWarning) {
    ZebinTestData::ValidEmptyProgram zebin;
    zebin.removeSection(NEO::Elf::SHT_ZEBIN::SHT_ZEBIN_ZEINFO, NEO::Elf::SectionsNamesZebin::zeInfo);
-    auto brokenZeInfo = std::string("some_entry : a\nkernels : \nversion:\'") + toString(zeInfoDecoderVersion) + "\'\n";
+    auto brokenZeInfo = std::string("some_entry : a\nkernels : \n  - name : valid_empty_kernel\n    execution_env : \n      simd_size  : 32\n      grf_count : 128\nversion:\'") + toString(zeInfoDecoderVersion) + "\'\n";
    zebin.appendSection(NEO::Elf::SHT_ZEBIN::SHT_ZEBIN_ZEINFO, NEO::Elf::SectionsNamesZebin::zeInfo, ArrayRef<const uint8_t>::fromAny(brokenZeInfo.data(), brokenZeInfo.size()));

    NEO::ProgramInfo programInfo;
@@ -2601,7 +2601,7 @@ TEST(DecodeSingleDeviceBinaryZebin, WhenZeInfoDoesNotContainKernelsSectionThenEm
 TEST(DecodeSingleDeviceBinaryZebin, WhenZeInfoContainsMultipleKernelSectionsThenFails) {
    ZebinTestData::ValidEmptyProgram zebin;
    zebin.removeSection(NEO::Elf::SHT_ZEBIN::SHT_ZEBIN_ZEINFO, NEO::Elf::SectionsNamesZebin::zeInfo);
-    auto brokenZeInfo = std::string("version:\'") + toString(zeInfoDecoderVersion) + "\'\nkernels:\nkernels:\n";
+    auto brokenZeInfo = std::string("version:\'") + toString(zeInfoDecoderVersion) + "\'\nkernels : \n  - name : valid_empty_kernel\n    execution_env : \n      simd_size  : 32\n      grf_count : 128\n" + "\nkernels : \n  - name : valid_empty_kernel\n    execution_env : \n      simd_size  : 32\n      grf_count : 128\n...\n";
    zebin.appendSection(NEO::Elf::SHT_ZEBIN::SHT_ZEBIN_ZEINFO, NEO::Elf::SectionsNamesZebin::zeInfo, ArrayRef<const uint8_t>::fromAny(brokenZeInfo.data(), brokenZeInfo.size()));

    NEO::ProgramInfo programInfo;
@@ -5544,3 +5544,25 @@ functions:
    EXPECT_STREQ(expectedError, errors.c_str());
    EXPECT_TRUE(warnings.empty()) << warnings;
 }
+
+TEST(ParsingEmptyOptionalVectorTypesZeInfo, givenEmptyOptionalVectorDataEntryInZeInfoWithSamePreviousIndentationWhenParsingZeInfoThenFalsIsReturnedAndErrorPrinted) {
+    NEO::Yaml::YamlParser parser;
+    bool parseResult = true;
+    std::string errors;
+    std::string warnings;
+
+    NEO::ConstStringRef zeInfoMissingEntrySamePrevInd = R"===(---
+kernels:
+  - name:            some_kernel
+    execution_env:
+      simd_size: 8
+    per_thread_payload_arguments:
+    binding_table_indices:
+      - bti_value:       0
+        arg_index:       0
+...
+)===";
+    parseResult = parser.parse(zeInfoMissingEntrySamePrevInd, errors, warnings);
+    ASSERT_FALSE(parseResult);
+    EXPECT_STREQ("NEO::Yaml : Could not parse line : [5] : [per_thread_payload_arguments:] <-- parser position on error. Reason : Vector data type expects to have at least one value starting with -\n", errors.c_str());
+}
--- a/shared/test/unit_test/device_binary_format/zebin_tests.h
+++ b/shared/test/unit_test/device_binary_format/zebin_tests.h
@@ -28,12 +28,15 @@ inline std::string toString(NEO::Elf::ZebinKernelMetadata::Types::Version versio
 namespace ZebinTestData {

 struct ValidEmptyProgram {
+    static constexpr char kernelName[19] = "valid_empty_kernel";
+
    ValidEmptyProgram() {
        NEO::Elf::ElfEncoder<> enc;
        enc.getElfFileHeader().type = NEO::Elf::ET_ZEBIN_EXE;
        enc.getElfFileHeader().machine = productFamily;
-        auto zeInfo = std::string{"---\nversion : \'" + toString(NEO::zeInfoDecoderVersion) + "\'" + "\nkernels : \n...\n"};
+        auto zeInfo = std::string{"---\nversion : \'" + toString(NEO::zeInfoDecoderVersion) + "\'" + "\nkernels : \n  - name : " + kernelName + "\n    execution_env : \n      simd_size  : 32\n      grf_count : 128\n...\n"};
        enc.appendSection(NEO::Elf::SHT_ZEBIN_ZEINFO, NEO::Elf::SectionsNamesZebin::zeInfo, zeInfo);
+        enc.appendSection(NEO::Elf::SHT_PROGBITS, NEO::Elf::SectionsNamesZebin::textPrefix.str() + "valid_empty_kernel", zeInfo);
        storage = enc.encode();
        recalcPtr();
    }