[clang] [llvm] [Inliner] Propagate more attributes to params when inlining (PR #91101)

[via cfe-commits]

goldsteinn wrote:

> Reproducer:
> 
> ```
> ; bin/opt -passes=inline reduced.ll -S
> target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
> target triple = "x86_64-unknown-linux-gnu"
> 
> define i64 @caller(ptr %p1, ptr %p2) {
>   %1 = call i64 %p1(ptr %p2)
>   %2 = call i64 @callee(i64 %1)
>   ret i64 %2
> }
> 
> define i64 @callee(i64 range(i64 0, 7) %0) {
>   %i = tail call i64 @llvm.umin.i64(i64 %0, i64 %0)
>   ret i64 %i
> }
> ```
> 
> ```
> Attribute after last parameter!
>   %1 = call i64 %p1(ptr range(i64 0, 7) %p2)
> LLVM ERROR: Broken module found, compilation aborted!
> PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
> Stack dump:
> 0.      Program arguments: bin/opt -passes=inline reduced.ll -S
> 1.      Running pass "verify" on module "reduced.ll"
>  #0 0x00007fdd0dc14782 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMSupport.so.20.0git+0x214782)
>  #1 0x00007fdd0dc1164f llvm::sys::RunSignalHandlers() (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMSupport.so.20.0git+0x21164f)
>  #2 0x00007fdd0dc11795 SignalHandler(int) Signals.cpp:0:0
>  #3 0x00007fdd0d642520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
>  #4 0x00007fdd0d6969fc __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
>  #5 0x00007fdd0d6969fc __pthread_kill_internal ./nptl/pthread_kill.c:78:10
>  #6 0x00007fdd0d6969fc pthread_kill ./nptl/pthread_kill.c:89:10
>  #7 0x00007fdd0d642476 gsignal ./signal/../sysdeps/posix/raise.c:27:6
>  #8 0x00007fdd0d6287f3 abort ./stdlib/abort.c:81:7
>  #9 0x00007fdd0da6d70f llvm::report_fatal_error(llvm::Twine const&, bool) (.cold) ErrorHandling.cpp:0:0
> #10 0x00007fdd0db1cc90 unsigned long std::uniform_int_distribution<unsigned long>::operator()<std::random_device>(std::random_device&, std::uniform_int_distribution<unsigned long>::param_type const&) (.isra.0) ExponentialBackoff.cpp:0:0
> #11 0x00007fdd06596958 (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/../lib/libLLVMCore.so.20.0git+0x396958)
> #12 0x00007fdd0de84645 llvm::detail::PassModel<llvm::Module, llvm::VerifierPass, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMOptDriver.so.20.0git+0x1f645)
> #13 0x00007fdd06553d9a llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/../lib/libLLVMCore.so.20.0git+0x353d9a)
> #14 0x00007fdd0de921b1 llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool) (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMOptDriver.so.20.0git+0x2d1b1)
> #15 0x00007fdd0de9d934 optMain (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMOptDriver.so.20.0git+0x38934)
> #16 0x00007fdd0d629d90 __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
> #17 0x00007fdd0d629e40 call_init ./csu/../csu/libc-start.c:128:20
> #18 0x00007fdd0d629e40 __libc_start_main ./csu/../csu/libc-start.c:379:5
> #19 0x00005d8d8cac3095 _start (bin/opt+0x1095)
> Aborted (core dumped)
> ```

Okay, the bug is from the fact that we simplify the values in `VMap` which can mean that even if `VMap.lookup(InnerCB)` exists, it doesn't imply that `NewInnerCB` is actually the same function. This is really only an issue for intrins which can simplify but is an active bug: https://godbolt.org/z/r873jTdvM

introduced in: https://github.com/llvm/llvm-project/pull/95888 :(

I think this bug shows up in other places we propagate i.e:
https://godbolt.org/z/zzbMvW5cq

Here we may create UB by propagating the `nonnull` back to `%p1()`.

I think the necessary fix is something along the lines of checking that the new callbase calls the same function.
I will have a patch up shortly. 

https://github.com/llvm/llvm-project/pull/91101