Fix printf issue with printing pointers from 32bit kernel on 64bit system.

Change-Id: I77771b4ebe6c4335d51dc1834f0b8f9df2a069a4

Author: PiotrZdunowski

runtime/kernel/kernel.h

@@ -358,6 +358,10 @@ class Kernel : public BaseObject<_cl_kernel> {
         return isParentKernel && getProgram()->getBlockKernelManager()->getIfBlockUsesPrintf();
     }
 
+    bool is32Bit() const {
+        return kernelInfo.gpuPointerSize == 4;
+    }
+
     int32_t getDebugSurfaceBti() const {
         if (kernelInfo.patchInfo.pAllocateSystemThreadSurface) {
             return kernelInfo.patchInfo.pAllocateSystemThreadSurface->BTI;

runtime/os_interface/linux/print.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Intel Corporation
+ * Copyright (c) 2017 - 2018, Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -39,10 +39,6 @@ size_t simple_sprintf(char *output, size_t outputSize, const char *format, const
     return snprintf(output, outputSize, format, value);
 }
 
-size_t simple_sprintf(char *output, size_t outputSize, const char *format, void *value) {
-    return snprintf(output, outputSize, format, value);
-}
-
 template size_t simple_sprintf<float>(char *output, size_t output_size, const char *format, float value);
 template size_t simple_sprintf<double>(char *output, size_t output_size, const char *format, double value);
 template size_t simple_sprintf<char>(char *output, size_t output_size, const char *format, char value);

runtime/os_interface/windows/print.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Intel Corporation
+ * Copyright (c) 2017 - 2018, Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -80,10 +80,6 @@ size_t simple_sprintf(char *output, size_t outputSize, const char *format, const
     return sprintf_s(output, outputSize, format, value);
 }
 
-size_t simple_sprintf(char *output, size_t outputSize, const char *format, void *value) {
-    return sprintf_s(output, outputSize, format, value);
-}
-
 template size_t simple_sprintf<float>(char *output, size_t output_size, const char *format, float value);
 template size_t simple_sprintf<double>(char *output, size_t output_size, const char *format, double value);
 template size_t simple_sprintf<char>(char *output, size_t output_size, const char *format, char value);

runtime/program/print_formatter.cpp

@@ -119,7 +119,6 @@ void PrintFormatter::stripVectorTypeConversion(char *format) {
 
 size_t PrintFormatter::printToken(char *output, size_t size, const char *formatString) {
     PRINTF_DATA_TYPE type(PRINTF_DATA_TYPE::INVALID);
-    size_t ret = 0;
     read(&type);
 
     switch (type) {
@@ -134,10 +133,7 @@ size_t PrintFormatter::printToken(char *output, size_t size, const char *formatS
     case PRINTF_DATA_TYPE::LONG:
         return typedPrintToken<int64_t>(output, size, formatString);
     case PRINTF_DATA_TYPE::POINTER:
-        ret = typedPrintToken<void *>(output, size, formatString);
-        // always pad read data to 8 bytes when handling pointers
-        offset += 8 - sizeof(void *);
-        return ret;
+        return printPointerToken(output, size, formatString);
     case PRINTF_DATA_TYPE::DOUBLE:
         return typedPrintToken<double>(output, size, formatString);
     case PRINTF_DATA_TYPE::VECTOR_BYTE:
@@ -157,6 +153,30 @@ size_t PrintFormatter::printToken(char *output, size_t size, const char *formatS
     }
 }
 
+size_t PrintFormatter::printStringToken(char *output, size_t size, const char *formatString) {
+    int index = 0;
+    int type = 0;
+    // additional read to discard the token
+    read(&type);
+    read(&index);
+    if (type == static_cast<int>(PRINTF_DATA_TYPE::STRING)) {
+        return simple_sprintf(output, size, formatString, kernel.getKernelInfo().queryPrintfString(index));
+    } else {
+        return simple_sprintf(output, size, formatString, 0);
+    }
+}
+
+size_t PrintFormatter::printPointerToken(char *output, size_t size, const char *formatString) {
+    uint64_t value = {0};
+    read(&value);
+
+    if (kernel.is32Bit()) {
+        value &= 0x00000000FFFFFFFF;
+    }
+
+    return simple_sprintf(output, size, formatString, value);
+}
+
 char PrintFormatter::escapeChar(char escape) {
     switch (escape) {
     case 'n':

runtime/program/print_formatter.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Intel Corporation
+ * Copyright (c) 2017 - 2018, Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -63,6 +63,8 @@ class PrintFormatter {
   protected:
     void printString(const char *formatString, const std::function<void(char *)> &print);
     size_t printToken(char *output, size_t size, const char *formatString);
+    size_t printStringToken(char *output, size_t size, const char *formatString);
+    size_t printPointerToken(char *output, size_t size, const char *formatString);
 
     char escapeChar(char escape);
     bool isConversionSpecifier(char c);
@@ -108,8 +110,9 @@ class PrintFormatter {
         for (int i = 0; i < valueCount; i++) {
             read(&value);
             charactersPrinted += simple_sprintf(output + charactersPrinted, size - charactersPrinted, strippedFormat, value);
-            if (i < valueCount - 1)
+            if (i < valueCount - 1) {
                 charactersPrinted += simple_sprintf(output + charactersPrinted, size - charactersPrinted, "%c", ',');
+            }
         }
 
         if (sizeof(T) < 4) {
@@ -119,23 +122,11 @@ class PrintFormatter {
         return charactersPrinted;
     }
 
-    size_t printStringToken(char *output, size_t size, const char *formatString) {
-        int index = 0;
-        int type = 0;
-        // additional read to discard the data type
-        read(&type);
-        read(&index);
-        if (type == static_cast<int>(PRINTF_DATA_TYPE::STRING))
-            return simple_sprintf(output, size, formatString, kernel.getKernelInfo().queryPrintfString(index));
-        else
-            return simple_sprintf(output, size, formatString, 0);
-    }
-
     Kernel &kernel;
     GraphicsAllocation &data;
 
     uint8_t *buffer;     // buffer extracted from the kernel, contains values to be printed
     uint32_t bufferSize; // size of the data contained in the buffer
     uint32_t offset;     // current position in currently parsed buffer
 };
-};
+}; // namespace OCLRT

unit_tests/kernel/kernel_tests.cpp

@@ -2128,3 +2128,27 @@ TEST(KernelTest, givenKernelWhenDebugFlagToUseMaxSimdForCalculationsIsUsedThenMa
     kernel.mockKernel->getWorkGroupInfo(device.get(), CL_KERNEL_WORK_GROUP_SIZE, sizeof(size_t), &maxKernelWkgSize, nullptr);
     EXPECT_EQ(256u, maxKernelWkgSize);
 }
+
+TEST(KernelTest, givenKernelWithKernelInfoWith32bitPointerSizeThenReport32bit) {
+    KernelInfo info;
+    info.gpuPointerSize = 4;
+
+    MockContext context;
+    MockProgram program(&context, false);
+    std::unique_ptr<MockDevice> device(Device::create<OCLRT::MockDevice>(nullptr));
+    std::unique_ptr<MockKernel> kernel(new MockKernel(&program, info, *device.get()));
+
+    EXPECT_TRUE(kernel->is32Bit());
+}
+
+TEST(KernelTest, givenKernelWithKernelInfoWith64bitPointerSizeThenReport64bit) {
+    KernelInfo info;
+    info.gpuPointerSize = 8;
+
+    MockContext context;
+    MockProgram program(&context, false);
+    std::unique_ptr<MockDevice> device(Device::create<OCLRT::MockDevice>(nullptr));
+    std::unique_ptr<MockKernel> kernel(new MockKernel(&program, info, *device.get()));
+
+    EXPECT_FALSE(kernel->is32Bit());
+}

unit_tests/program/printf_helper_tests.cpp

@@ -749,7 +749,7 @@ TEST_F(PrintFormatterTest, GivenPrintfFormatWhenPointerThenInsertAddress) {
     storeData(reinterpret_cast<void *>(&temp));
 
     // on 32bit configurations add extra 4 bytes when storing pointers, IGC always stores pointers on 8 bytes
-    if (!is64bit) {
+    if (is32bit) {
         uint32_t padding = 0;
         storeData(padding);
     }
@@ -764,6 +764,33 @@ TEST_F(PrintFormatterTest, GivenPrintfFormatWhenPointerThenInsertAddress) {
     EXPECT_STREQ(referenceOutput, actualOutput);
 }
 
+TEST_F(PrintFormatterTest, GivenPrintfFormatWhenPointerWith32BitKernelThenPrint32BitPointer) {
+    auto stringIndex = injectFormatString("%p");
+    storeData(stringIndex);
+    kernelInfo->gpuPointerSize = 4;
+
+    storeData(PRINTF_DATA_TYPE::POINTER);
+
+    // store pointer
+    uint32_t addressValue = 0;
+    storeData(addressValue);
+
+    void *pointer = nullptr;
+
+    // store non zero padding
+    uint32_t padding = 0xdeadbeef;
+    storeData(padding);
+
+    char actualOutput[PrintFormatter::maxPrintfOutputLength];
+    char referenceOutput[PrintFormatter::maxPrintfOutputLength];
+
+    snprintf(referenceOutput, sizeof(referenceOutput), "%p", pointer);
+
+    printFormatter->printKernelOutput([&actualOutput](char *str) { strncpy_s(actualOutput, PrintFormatter::maxPrintfOutputLength, str, PrintFormatter::maxPrintfOutputLength); });
+
+    EXPECT_STREQ(referenceOutput, actualOutput);
+}
+
 TEST_F(PrintFormatterTest, GivenPrintfFormatWhen2ByteVectorsThenParseDataBufferProperly) {
     int channelCount = 4;