Skip to content

Commit

Permalink
printf: handle empty string, and string of only null
Browse files Browse the repository at this point in the history
Add tests:
- verify the new case
- verify we don't mess up pointer-to-zero-char4

Fixed: google#1385
  • Loading branch information
dneto0 committed Jul 19, 2024
1 parent bdc5c4e commit cb787a4
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 10 deletions.
26 changes: 18 additions & 8 deletions lib/PrintfPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

using namespace llvm;

std::string clspv::PrintfPass::GetStringLiteral(Value *Val) {
std::optional<std::string> clspv::PrintfPass::GetStringLiteral(Value *Val) {
GlobalVariable *GlobalVal = nullptr;
Instruction *ConstExprEquivInstr = nullptr;
// If opaque pointers aren't enabled, the global may be hidden behind a
Expand Down Expand Up @@ -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<ConstantAggregateZero>(Initializer)) {
if (auto *arrTy = dyn_cast<ArrayType>(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) {
Expand Down Expand Up @@ -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;
}

Expand All @@ -292,15 +302,15 @@ 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);
ArgMD.push_back(ConstantAsMetadata::get(ArgSizeConst));
}

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
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions lib/PrintfPass.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "llvm/IR/Module.h"
#include "llvm/IR/PassManager.h"
#include <optional>

#ifndef _CLSPV_LIB_PRINTF_PASS_H
#define _CLSPV_LIB_PRINTF_PASS_H
Expand All @@ -24,8 +25,8 @@ struct PrintfPass : llvm::PassInfoMixin<PrintfPass> {

private:
// Find the underlying compile-time string literal, if any, for the given
// value
std::string GetStringLiteral(llvm::Value *);
// value.
std::optional<std::string> GetStringLiteral(llvm::Value *);

// Get the printf buffer storage size for the given type
unsigned GetPrintfStoreSize(const llvm::DataLayout &DL, llvm::Type *Ty);
Expand Down
49 changes: 49 additions & 0 deletions test/Printf/printf_ptr_to_char4_zero.ll
Original file line number Diff line number Diff line change
@@ -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 = !{!""}
46 changes: 46 additions & 0 deletions test/Printf/printf_string_only_null.ll
Original file line number Diff line number Diff line change
@@ -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 = !{!""}

0 comments on commit cb787a4

Please sign in to comment.