InstCombine pass crash due to calling a function with a bad signature

[Hatsunespica]

https://gcc.godbolt.org/z/ov8Tf1PcT

opt -instcombine crashed with this IR

@percent_s = constant [3 x i8] c"%s\00"

declare i32 @sprintf(i8**, i32*, ...)

define void @PR51200(i8** %p, i32* %p2) {
  %call = call i32 (i8**, i32*, ...) @sprintf(i8** %p, i32* bitcast ([3 x i8]* @percent_s to i32*), i32* %p2)
  ret void
}

It calls a function sprintf and returns. This case is mutated from Transforms/InstCombine/stpcpy-1.ll The difference is the original file returns %call directly but here it returns void. By the way, it also crashes if it returns another i32 except %call. Example: Another example returning i32

Error message:

opt: /home/spica/GitRepo/llvm-project/llvm/lib/IR/Instructions.cpp:528: void llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, llvm::ArrayRef<llvm::Value*>, llvm::ArrayRef<llvm::OperandBundleDefT<llvm::Value*> >, const llvm::Twine&): Assertion `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: opt stpcpy-16.ll -O2
 #0 0x00007f7ddc8f80d4 PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
 #1 0x00007f7ddc8f5114 SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f7ddc2ff0c0 (/lib/x86_64-linux-gnu/libc.so.6+0x430c0)
 #3 0x00007f7ddc2ff03b raise /build/glibc-sMfBJT/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #4 0x00007f7ddc2de859 abort /build/glibc-sMfBJT/glibc-2.31/stdlib/abort.c:81:7
 #5 0x00007f7ddc2de729 get_sysdep_segment_value /build/glibc-sMfBJT/glibc-2.31/intl/loadmsgcat.c:509:8
 #6 0x00007f7ddc2de729 _nl_load_domain /build/glibc-sMfBJT/glibc-2.31/intl/loadmsgcat.c:970:34
 #7 0x00007f7ddc2f0006 (/lib/x86_64-linux-gnu/libc.so.6+0x34006)
 #8 0x00007f7ddcbdbdf1 llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, llvm::ArrayRef<llvm::Value*>, llvm::ArrayRef<llvm::OperandBundleDefT<llvm::Value*>>, llvm::Twine const&) (/usr/local/bin/../lib/libLLVMCore.so.15git+0x244df1)
 #9 0x00007f7ddd7ce9dc llvm::IRBuilderBase::CreateCall(llvm::FunctionType*, llvm::Value*, llvm::ArrayRef<llvm::Value*>, llvm::Twine const&, llvm::MDNode*) (.constprop.0) BuildLibCalls.cpp:0:0
#10 0x00007f7ddd7d2000 emitLibCall(llvm::LibFunc, llvm::Type*, llvm::ArrayRef<llvm::Type*>, llvm::ArrayRef<llvm::Value*>, llvm::IRBuilderBase&, llvm::TargetLibraryInfo const*, bool) BuildLibCalls.cpp:0:0
#11 0x00007f7ddd7d2cdd llvm::emitStrCpy(llvm::Value*, llvm::Value*, llvm::IRBuilderBase&, llvm::TargetLibraryInfo const*) (/usr/local/bin/../lib/libLLVMTransformUtils.so.15git+0xb0cdd)
#12 0x00007f7ddd9c2710 llvm::LibCallSimplifier::optimizeSPrintFString(llvm::CallInst*, llvm::IRBuilderBase&) (/usr/local/bin/../lib/libLLVMTransformUtils.so.15git+0x2a0710)
#13 0x00007f7ddd9c28e0 llvm::LibCallSimplifier::optimizeSPrintF(llvm::CallInst*, llvm::IRBuilderBase&) (/usr/local/bin/../lib/libLLVMTransformUtils.so.15git+0x2a08e0)
#14 0x00007f7ddd9ca05d llvm::LibCallSimplifier::optimizeCall(llvm::CallInst*, llvm::IRBuilderBase&) (/usr/local/bin/../lib/libLLVMTransformUtils.so.15git+0x2a805d)
#15 0x00007f7dddde0a1c llvm::InstCombinerImpl::tryOptimizeCall(llvm::CallInst*) (/usr/local/bin/../lib/libLLVMInstCombine.so.15git+0x9ca1c)
#16 0x00007f7dddde64e1 llvm::InstCombinerImpl::visitCallBase(llvm::CallBase&) (/usr/local/bin/../lib/libLLVMInstCombine.so.15git+0xa24e1)
#17 0x00007f7dddde9e24 llvm::InstCombinerImpl::visitCallInst(llvm::CallInst&) (/usr/local/bin/../lib/libLLVMInstCombine.so.15git+0xa5e24)
#18 0x00007f7dddd911e4 llvm::InstCombinerImpl::run() (/usr/local/bin/../lib/libLLVMInstCombine.so.15git+0x4d1e4)
#19 0x00007f7dddd93148 combineInstructionsOverFunction(llvm::Function&, llvm::InstructionWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::TargetTransformInfo&, llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, llvm::BlockFrequencyInfo*, llvm::ProfileSummaryInfo*, unsigned int, llvm::LoopInfo*) InstructionCombining.cpp:0:0
#20 0x00007f7dddd93a81 llvm::InstCombinePass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/bin/../lib/libLLVMInstCombine.so.15git+0x4fa81)
#21 0x00007f7ddf605516 llvm::detail::PassModel<llvm::Function, llvm::InstCombinePass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/bin/../lib/libLLVMPasses.so.15git+0xa6516)
#22 0x00007f7ddcc713a4 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/bin/../lib/libLLVMCore.so.15git+0x2da3a4)
#23 0x00007f7ddf5ff246 llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/bin/../lib/libLLVMPasses.so.15git+0xa0246)
#24 0x00007f7ddcc6fed6 llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/bin/../lib/libLLVMCore.so.15git+0x2d8ed6)
#25 0x00007f7ddf607ad6 llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/bin/../lib/libLLVMPasses.so.15git+0xa8ad6)
#26 0x00007f7ddcc6df71 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/bin/../lib/libLLVMCore.so.15git+0x2d6f71)
#27 0x00005566b3955345 llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::StringRef>, llvm::ArrayRef<llvm::PassPlugin>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool) (/usr/local/bin/opt+0x34345)
#28 0x00005566b3945a7b main (/usr/local/bin/opt+0x24a7b)
#29 0x00007f7ddc2e00b3 __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:342:3
#30 0x00005566b39464fe _start (/usr/local/bin/opt+0x254fe)

cc @regehr

[msebor]

There are other examples that boil down to the same underlying problem: isValidProtoForLibFunc being overly permissive and accepting as valid declarations of functions whose signatures the libcall simplifier is not prepared for. For instance, the following triggers an abort as well, albeit with a different stack trace:

declare i64 @strtol(i8*, i8**)

@a = constant [2 x i8] c"1\00"

define i64 @f() {
  %p = getelementptr [2 x i8], [2 x i8]* @a, i32 0, i32 0
  %n = call i64 @strtol(i8* %p, i8** null) 
  ret i64 %n
}

[msebor]

Another example is below. This is one is closer to the sprintf problem in that it's induced by transforming the strchr(p, 0) call to p + strlen(p) and assuming that p's type is i8* rather that by making bad assumptions about the number of arguments (the strtol case).

declare i32* @strchr(i32*, i32)

define void @f(i32* %p) {
  call i32* @strchr(i32* %p, i32 0)
  ret void
}

These assumptions are pervasive throughout SimplifyLibcalls.cpp and although they could be removed and the hardwired type replaced by one derived from the IR, I'm not sure that effort would be worthwhile given the expectation of replacing typed pointers with opaque pointers. That also suggests that in the absence of -fno-builtin-xxx, incompatible declarations like those in these test cases (i.e., the sprintf and the strchr one but not that of the two argument strtol) should be treated as invalid and an ICE an adequate response. @nikic, your thoughts?

[nikic]

These assumptions are pervasive throughout SimplifyLibcalls.cpp and although they could be removed and the hardwired type replaced by one derived from the IR, I'm not sure that effort would be worthwhile given the expectation of replacing typed pointers with opaque pointers.

The usual thing to do here would be to insert an explicit i8* bitcast, as done in https://github.com/llvm/llvm-project/blob/32fe1a4be95c90da9a24d63a097429ec7c3bbfba/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp#L2765 However, that one is actually incorrect, because it does not take the address space of the pointer into account.

I agree that fixing this is not worthwhile at this point. We no longer use typed pointers by default, and they are scheduled to be removed in LLVM 16, so I personally wouldn't bother fixing them for this kind of "fuzzer only" issue.

[msebor]

Okay then, let me use this bug to fix just the strtol class of bugs, i.e., those triggered by lax compatibility checking in TargetLibraryInfoImpl::isValidProtoForLibFunc unrelated to pointer types.

[mikaelholmen]

Hello,

I think there is a problem with the fix pushed in 0dcfe7aa35cd. I made a comment here: https://reviews.llvm.org/rG0dcfe7aa35cd4f6dbeb739fcd05f93aa39394f0e#1114655

I short: The commit not only tightens up the lib function lib tests, it also relaxes them! So in some cases we now identify calls as lib calls that LibCallSimplifier isn't prepared for.

[msebor]

The reported issue has been fixed in rG0dcfe7aa35cd. A patch for the problem noted above has been submitted in D129915.

[msebor]

Fixed in rG0dcfe7aa35cd.