From cb787a40e3bdf7cf3a20845abc14a0ffea773d9c Mon Sep 17 00:00:00 2001 From: David Neto Date: Fri, 19 Jul 2024 15:34:00 -0400 Subject: [PATCH] printf: handle empty string, and string of only null Add tests: - verify the new case - verify we don't mess up pointer-to-zero-char4 Fixed: #1385 --- lib/PrintfPass.cpp | 26 +++++++++---- lib/PrintfPass.h | 5 ++- test/Printf/printf_ptr_to_char4_zero.ll | 49 +++++++++++++++++++++++++ test/Printf/printf_string_only_null.ll | 46 +++++++++++++++++++++++ 4 files changed, 116 insertions(+), 10 deletions(-) create mode 100644 test/Printf/printf_ptr_to_char4_zero.ll create mode 100644 test/Printf/printf_string_only_null.ll diff --git a/lib/PrintfPass.cpp b/lib/PrintfPass.cpp index 0164d13f9..3cd0051bf 100644 --- a/lib/PrintfPass.cpp +++ b/lib/PrintfPass.cpp @@ -25,7 +25,7 @@ using namespace llvm; -std::string clspv::PrintfPass::GetStringLiteral(Value *Val) { +std::optional clspv::PrintfPass::GetStringLiteral(Value *Val) { GlobalVariable *GlobalVal = nullptr; Instruction *ConstExprEquivInstr = nullptr; // If opaque pointers aren't enabled, the global may be hidden behind a @@ -54,9 +54,19 @@ std::string clspv::PrintfPass::GetStringLiteral(Value *Val) { return InitArray->getAsString().str(); } } + // The empty C string, and strings of null chars, are + // constant-aggregate-zero. + if (auto *InitArray = dyn_cast(Initializer)) { + if (auto *arrTy = dyn_cast(InitArray->getType())) { + if (arrTy->getElementType()->isIntegerTy(8)) { + return std::string(arrTy->getNumElements(), '\0'); + } + } + } } - return ""; + // Not a string. + return {}; } unsigned clspv::PrintfPass::GetPrintfStoreSize(const DataLayout &DL, Type *Ty) { @@ -278,7 +288,7 @@ PreservedAnalyses clspv::PrintfPass::run(Module &M, ModuleAnalysisManager &) { // Check format string is valid auto *FmtStringValue = CI->getArgOperand(0); auto FmtString = GetStringLiteral(FmtStringValue); - if (FmtString == "") { + if (!FmtString.has_value()) { continue; } @@ -292,7 +302,7 @@ PreservedAnalyses clspv::PrintfPass::run(Module &M, ModuleAnalysisManager &) { // Literal Arg are store using a 32bits identifier, which differs when // using 64bits pointers from the size returned by GetPrintfStoreSize. - bool IsStringLiteral = GetStringLiteral(Arg) != ""; + bool IsStringLiteral = GetStringLiteral(Arg).has_value(); unsigned ArgSize = IsStringLiteral ? 4 : GetPrintfStoreSize(DL, ArgType); auto *ArgSizeConst = ConstantInt::get(Int32Ty, ArgSize); @@ -300,7 +310,7 @@ PreservedAnalyses clspv::PrintfPass::run(Module &M, ModuleAnalysisManager &) { } auto *PrintfMD = - CreatePrintfArgsMetadata(M, ArgMD, FmtString, PrintfCallID); + CreatePrintfArgsMetadata(M, ArgMD, FmtString.value(), PrintfCallID); PrintfMDs->addOperand(PrintfMD); // Get string literal arguments to printf (not including format string) and @@ -309,10 +319,10 @@ PreservedAnalyses clspv::PrintfPass::run(Module &M, ModuleAnalysisManager &) { for (unsigned i = 1; i < CI->arg_size(); i++) { Value *Arg = CI->getArgOperand(i); auto StringLiteral = GetStringLiteral(Arg); - if (StringLiteral != "") { + if (StringLiteral.has_value()) { auto StringLiteralID = PrintfID++; - auto *StringLiteralMD = - CreatePrintfArgsMetadata(M, {}, StringLiteral, StringLiteralID); + auto *StringLiteralMD = CreatePrintfArgsMetadata( + M, {}, StringLiteral.value(), StringLiteralID); PrintfMDs->addOperand(StringLiteralMD); // The printf now stores the string's ID rather than its data diff --git a/lib/PrintfPass.h b/lib/PrintfPass.h index 87797469f..1830ca0df 100644 --- a/lib/PrintfPass.h +++ b/lib/PrintfPass.h @@ -14,6 +14,7 @@ #include "llvm/IR/Module.h" #include "llvm/IR/PassManager.h" +#include #ifndef _CLSPV_LIB_PRINTF_PASS_H #define _CLSPV_LIB_PRINTF_PASS_H @@ -24,8 +25,8 @@ struct PrintfPass : llvm::PassInfoMixin { private: // Find the underlying compile-time string literal, if any, for the given - // value - std::string GetStringLiteral(llvm::Value *); + // value. + std::optional GetStringLiteral(llvm::Value *); // Get the printf buffer storage size for the given type unsigned GetPrintfStoreSize(const llvm::DataLayout &DL, llvm::Type *Ty); diff --git a/test/Printf/printf_ptr_to_char4_zero.ll b/test/Printf/printf_ptr_to_char4_zero.ll new file mode 100644 index 000000000..3c04e565c --- /dev/null +++ b/test/Printf/printf_ptr_to_char4_zero.ll @@ -0,0 +1,49 @@ +; RUN: clspv-opt %s -o %t.ll --passes=printf-pass +; RUN: FileCheck %s < %t.ll + +; Test this program: +; __constant char4 vc = (char4)(0,0,0,0); +; kernel void m() { printf("%v4hhd", &vc); +; +; This the initializer for vc is a constant-aggregate-zero, but of vector type, not array type. +; This justifies the check for array type when trying to see if the argument is a string. + +; CHECK: !clspv.printf_metadata = !{![[FMT:[0-9]+]]} +; CHECK: ![[FMT]] = !{i32 0, !"%v4hhd\00", + +target datalayout = "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-G1" +target triple = "spir-unknown-unknown" + +@vc = addrspace(2) constant <4 x i8> zeroinitializer, align 4 +@.str = private unnamed_addr addrspace(2) constant [7 x i8] c"%v4hhd\00", align 1 +@__spirv_WorkgroupSize = addrspace(8) global <3 x i32> zeroinitializer + +; Function Attrs: convergent norecurse nounwind +define dso_local spir_kernel void @m() #0 !kernel_arg_addr_space !6 !kernel_arg_access_qual !6 !kernel_arg_type !6 !kernel_arg_base_type !6 !kernel_arg_type_qual !6 !kernel_arg_name !6 !clspv.pod_args_impl !7 { +entry: + %call = call spir_func i32 (ptr addrspace(2), ...) @printf(ptr addrspace(2) @.str, ptr addrspace(2) @vc) #2 + ret void +} + +; Function Attrs: convergent nounwind +declare !kernel_arg_name !8 spir_func i32 @printf(ptr addrspace(2), ...) #1 + +attributes #0 = { convergent norecurse nounwind "no-builtins" "no-trapping-math"="true" "stack-protector-buffer-size"="0" "stackrealign" "uniform-work-group-size"="true" } +attributes #1 = { convergent nounwind "no-builtins" "no-trapping-math"="true" "stack-protector-buffer-size"="0" "stackrealign" } +attributes #2 = { convergent nobuiltin nounwind "no-builtins" } + +!llvm.module.flags = !{!0, !1, !2} +!opencl.ocl.version = !{!3} +!opencl.spir.version = !{!3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3} +!llvm.ident = !{!4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4} +!_Z28clspv.entry_point_attributes = !{!5} + +!0 = !{i32 1, !"wchar_size", i32 4} +!1 = !{i32 7, !"direct-access-external-data", i32 0} +!2 = !{i32 7, !"frame-pointer", i32 2} +!3 = !{i32 1, i32 2} +!4 = !{!"clang version 19.0.0git (https://github.com/llvm/llvm-project 402eca265f7162e26b8b74d18297fd76c9f100de)"} +!5 = !{!"m", !"kernel"} +!6 = !{} +!7 = !{i32 2} +!8 = !{!""} diff --git a/test/Printf/printf_string_only_null.ll b/test/Printf/printf_string_only_null.ll new file mode 100644 index 000000000..51dbe3449 --- /dev/null +++ b/test/Printf/printf_string_only_null.ll @@ -0,0 +1,46 @@ +; RUN: clspv-opt %s -o %t.ll --passes=printf-pass +; RUN: FileCheck %s < %t.ll + +; Test printf("", "\0\0\0"); +; The value is a ConstantAggregateZero whose type is array of i8 + +; CHECK: !clspv.printf_metadata = !{![[FMT:[0-9]+]], ![[LITERAL:[0-9]+]]} +; CHECK: ![[FMT]] = !{i32 0, !"\00", +; CHECK: ![[LITERAL]] = !{i32 1, !"\00\00\00\00", + +target datalayout = "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-G1" +target triple = "spir-unknown-unknown" + +@.str = private unnamed_addr addrspace(2) constant [1 x i8] zeroinitializer, align 1 +@.str.1 = private unnamed_addr addrspace(2) constant [4 x i8] zeroinitializer, align 1 +@__spirv_WorkgroupSize = addrspace(8) global <3 x i32> zeroinitializer + +; Function Attrs: convergent norecurse nounwind +define dso_local spir_kernel void @m() #0 !kernel_arg_addr_space !6 !kernel_arg_access_qual !6 !kernel_arg_type !6 !kernel_arg_base_type !6 !kernel_arg_type_qual !6 !kernel_arg_name !6 !clspv.pod_args_impl !7 { +entry: + %call = call spir_func i32 (ptr addrspace(2), ...) @printf(ptr addrspace(2) @.str, ptr addrspace(2) @.str.1) #2 + ret void +} + +; Function Attrs: convergent nounwind +declare !kernel_arg_name !8 spir_func i32 @printf(ptr addrspace(2), ...) #1 + +attributes #0 = { convergent norecurse nounwind "no-builtins" "no-trapping-math"="true" "stack-protector-buffer-size"="0" "stackrealign" "uniform-work-group-size"="true" } +attributes #1 = { convergent nounwind "no-builtins" "no-trapping-math"="true" "stack-protector-buffer-size"="0" "stackrealign" } +attributes #2 = { convergent nobuiltin nounwind "no-builtins" } + +!llvm.module.flags = !{!0, !1, !2} +!opencl.ocl.version = !{!3} +!opencl.spir.version = !{!3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3, !3} +!llvm.ident = !{!4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4, !4} +!_Z28clspv.entry_point_attributes = !{!5} + +!0 = !{i32 1, !"wchar_size", i32 4} +!1 = !{i32 7, !"direct-access-external-data", i32 0} +!2 = !{i32 7, !"frame-pointer", i32 2} +!3 = !{i32 1, i32 2} +!4 = !{!"clang version 19.0.0git (https://github.com/llvm/llvm-project 402eca265f7162e26b8b74d18297fd76c9f100de)"} +!5 = !{!"m", !"kernel"} +!6 = !{} +!7 = !{i32 2} +!8 = !{!""}