From 4f9ee0a967ef2d7ab56edd23b0f0469e6fae9646 Mon Sep 17 00:00:00 2001
From: foreman <dl.constructicon@amd.com>
Date: Tue, 10 Nov 2015 14:33:05 -0500
Subject: [PATCH] P4 to Git Change 1209566 by smekhano@stas-nova-hsa on
 2015/11/10 14:25:56

	SWDEV-80874 - fixed out of bound access to the printf format string
	We do not really need two separate induction variables, pos and i, and we had a bug of not incrementing i as needed.
	The only reason it used to work is because all strings we used for testing ended with '\n'.
	The bug resulted in ignoring this '\n', but the code unconditionally adds '\n', so nobody noticed.
	If you try to print anything having any other escape, '\n' not at the end, or a colon, there will be assertion.

	That is fixed, and newline now is only added if last symbol in user's format was not newline, because otherwise
	we would now print 2 new lines. NB, I prefer to use bool variable rather then addressing last symbol of the string
	which could be empty.

	A side node, why do we run flex scanner past the last colon? If we do not we would not need this double encoding at all.

	Testing: smoke, precheckin, conformance printf with HSAIL forced, custom test
	Reviewed by German Andreev

Affected files ...

... //depot/stg/opencl/drivers/opencl/runtime/device/gpu/gpukernel.cpp#309 edit


[ROCm/clr commit: eea9bc6733d3cf0e60cff0a44c3f3ccc6b18e9c3]
---
 .../rocclr/runtime/device/gpu/gpukernel.cpp    | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/projects/clr/rocclr/runtime/device/gpu/gpukernel.cpp b/projects/clr/rocclr/runtime/device/gpu/gpukernel.cpp
index 7218f67371..20d4a66ab4 100644
--- a/projects/clr/rocclr/runtime/device/gpu/gpukernel.cpp
+++ b/projects/clr/rocclr/runtime/device/gpu/gpukernel.cpp
@@ -3304,12 +3304,14 @@ HSAILKernel::initPrintf(const aclPrintfFmt* aclPrintf)
         std::string pfmt = aclPrintf->fmtStr;
         info.fmtString_.clear();
         size_t  pos = 0;
-        for (size_t i = 0; i < pfmt.size(); ++i) {
-          char symbol = pfmt[pos++];
+        bool need_nl = true;
+        for (size_t pos = 0; pos < pfmt.size(); ++pos) {
+          char symbol = pfmt[pos];
+          need_nl = true;
           if (symbol == '\\') {
             // Rest of the C escape sequences (e.g. \') are handled correctly
             // by the MDParser, we are not sure exactly how!
-            switch (pfmt[pos]) {
+            switch (pfmt[pos+1]) {
             case 'a':
               pos++;
               symbol = '\a';
@@ -3325,6 +3327,7 @@ HSAILKernel::initPrintf(const aclPrintfFmt* aclPrintf)
             case 'n':
               pos++;
               symbol = '\n';
+              need_nl = false;
               break;
             case 'r':
               pos++;
@@ -3335,9 +3338,8 @@ HSAILKernel::initPrintf(const aclPrintfFmt* aclPrintf)
               symbol = '\v';
               break;
             case '7':
-              if (pfmt[++pos] == '2') {
-                pos++;
-                i++;
+              if (pfmt[pos+2] == '2') {
+                pos += 2;
                 symbol = '\72';
               }
               break;
@@ -3347,7 +3349,9 @@ HSAILKernel::initPrintf(const aclPrintfFmt* aclPrintf)
           }
           info.fmtString_.push_back(symbol);
         }
-        info.fmtString_ += "\n";
+        if (need_nl) {
+          info.fmtString_ += "\n";
+        }
         uint32_t *tmp_ptr = const_cast<uint32_t*>(aclPrintf->argSizes);
         for (uint i = 0; i < aclPrintf->numSizes; i++ , tmp_ptr++) {
             info.arguments_.push_back(*tmp_ptr);