Skip to content

Commit 7eff323

Browse files
aratajewigcbot
authored andcommitted
Fix handling of printf with excess arguments
Printf can have more arguments than conversion specifiers in format string, example: ```c printf("text with exhausted format\n", i++, evaluated = modify_char()); ``` According to printf specification: "If the format is exhausted while arguments remain, the excess arguments are evaluated..." The issue is that IGC writes excess arguments into printf buffer even if they don't have corresponding conversion specifiers in format string. This change introduces counting mechanism for conversion specifiers in format string and allows only for "non-excess" arguments to be written into printf buffer. This change also fixes the issue of writing tagged format string address into printf buffer - it happens when a kernel addrspacecasts format string pointer to generic pointer.
1 parent cfc1fe0 commit 7eff323

File tree

2 files changed

+57
-5
lines changed

2 files changed

+57
-5
lines changed

IGC/Compiler/Optimizer/OpenCLPasses/OpenCLPrintf/OpenCLPrintfResolution.cpp

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,6 @@ std::string OpenCLPrintfResolution::getEscapedString(const ConstantDataSequentia
226226

227227
Value* OpenCLPrintfResolution::processPrintfString(Value* arg, Function& F)
228228
{
229-
if (m_CGContext->enableZEBinary())
230-
{
231-
return arg;
232-
}
233-
234229
GlobalVariable* formatString = nullptr;
235230

236231
if (isa<GlobalVariable>(arg))
@@ -241,6 +236,12 @@ Value* OpenCLPrintfResolution::processPrintfString(Value* arg, Function& F)
241236
IGC_ASSERT_MESSAGE(0, "Unexpected printf argument (expected string literal)");
242237
return ConstantInt::get(m_int32Type, -1);
243238
}
239+
240+
if (m_CGContext->enableZEBinary())
241+
{
242+
return arg;
243+
}
244+
244245
ConstantDataArray* formatStringConst = dyn_cast<ConstantDataArray>(formatString->getInitializer());
245246
std::string escaped_string = getEscapedString(formatStringConst);
246247

@@ -373,6 +374,24 @@ static StoreInst* genStoreInternal(Value* Val, Value* Ptr, BasicBlock* InsertAtE
373374
return SI;
374375
}
375376

377+
void OpenCLPrintfResolution::removeExcessArgs()
378+
{
379+
SPrintfArgDescriptor* formatStringArgDesc = &m_argDescriptors[0];
380+
381+
Value* formatString = formatStringArgDesc->value;
382+
IGC::SHADER_PRINTF_TYPE dataType = formatStringArgDesc->argType;
383+
IGC_ASSERT(dataType == SHADER_PRINTF_STRING_LITERAL);
384+
385+
if (auto GV = dyn_cast<GlobalVariable>(formatString))
386+
{
387+
IGC_ASSERT(GV->hasInitializer());
388+
389+
unsigned int numFormatSpecifiers = getNumFormatSpecifiers(cast<ConstantDataArray>(GV->getInitializer()));
390+
if (m_argDescriptors.size() > numFormatSpecifiers + 1)
391+
m_argDescriptors.erase(m_argDescriptors.begin() + numFormatSpecifiers + 1, m_argDescriptors.end());
392+
}
393+
}
394+
376395
void OpenCLPrintfResolution::expandPrintfCall(CallInst& printfCall, Function& F)
377396
{
378397
/* Replace a printf call with IR instructions that fill the printf
@@ -449,6 +468,8 @@ void OpenCLPrintfResolution::expandPrintfCall(CallInst& printfCall, Function& F)
449468
// Scalarize vector arguments and substitute string arguments by their indices.
450469
preprocessPrintfArgs(printfCall);
451470

471+
removeExcessArgs();
472+
452473
// writeOffset = atomic_add(bufferPtr, dataSize)
453474
Value *basebufferPtr = isPrintfBuiltin
454475
? printfCall.getArgOperand(0)
@@ -705,6 +726,29 @@ Value* OpenCLPrintfResolution::fixupPrintfArg(CallInst& printfCall, Value* arg,
705726
return arg;
706727
}
707728

729+
unsigned OpenCLPrintfResolution::getNumFormatSpecifiers(const ConstantDataArray* dataArray)
730+
{
731+
unsigned int count = 0;
732+
StringRef formatString = dataArray->getRawDataValues();
733+
734+
const size_t length = formatString.size();
735+
for (size_t i = 0; i < length; i++)
736+
{
737+
if (formatString[i] == '%')
738+
{
739+
if (i + 1 < length && formatString[i + 1] == '%')
740+
{
741+
i++;
742+
continue;
743+
}
744+
745+
count++;
746+
}
747+
}
748+
749+
return count;
750+
}
751+
708752
void OpenCLPrintfResolution::preprocessPrintfArgs(CallInst& printfCall)
709753
{
710754
int i = 0;

IGC/Compiler/Optimizer/OpenCLPasses/OpenCLPrintf/OpenCLPrintfResolution.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,14 @@ namespace IGC
8888
// Replaces a printf call with a sequence of IR instrictions.
8989
void expandPrintfCall(llvm::CallInst& printfCall, llvm::Function& F);
9090

91+
// Removes excess arguments from m_argDescriptors vector. Excess arguments
92+
// are arguments that do not have a corresponding format specifier in a
93+
// format string.
94+
void removeExcessArgs();
95+
96+
// Returns number of format specifiers in a format string.
97+
unsigned getNumFormatSpecifiers(const llvm::ConstantDataArray* formatString);
98+
9199
// Walkes over printf arguments (including the format string) and adds them to printfArgs.
92100
// If an argument has vector type, adds the vector elements instead.
93101
void preprocessPrintfArgs(llvm::CallInst& printfCall);

0 commit comments

Comments
 (0)