[inliner][bug] AddReturnAttributes crash with non existing operands

[erthalion]

Hi,

I think we've hit the Inliner bug when using LLVM in PostgreSQL [1]. Here is
what happens:

• To process a query PostgreSQL creates a deform function
• The module with this function is fed into LLVMRunPasses
• The Inliner pass crashes inside AddReturnAttributes with an assert:

llvm/include/llvm/IR/Instructions.h:3420: llvm::Value* llvm::ReturnInst::getOperand(unsigned int) const:
Assertion `i_nocapture < OperandTraits<ReturnInst>::operands(this) && "getOperand() out of range!"' failed.

It seems CallBase was identified to have some attributes, but one
ReturnInst doesn't have any operands to work with [2]. The instruction
we're talking about is the return instruction from the outblock in the deform
function, which was generated via LLVMBuildRetVoid and is literally
ret void (I'll add the full function body at the end).

The unfortunate part is that this started to crash only recently. My
understanding that the reason is this change [3], which has enabled noundef
attributes propagation. It looks like ret void is considered to be noundef
(this is exactly the kind of an attribute that causes the crash), thus
ValidUB.hasAttributes() now returns true and everything proceed further to
work with operands. If I comment out if (CB.hasRetAttr(Attribute::NoUndef))
condition everything works fine.

Any ideas how to fix that? Any other information necessary? I've tried to
experiment with excluding problematic ReturnInst from the loop, but it turns
out not only getOperand, getNumOperands is also crashing. I'll try to come
up with an isolated test case, in the meantime if you folks are brave enough I
can show how to reproduce it directly on PostgreSQL.

---

Here is the full body of the deform function, the outblock is a cleanup code
that does ret void.

; Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: write) uwtable
define internal void @deform_0_1(ptr nocapture noundef writeonly align 8 %0) #0 {
entry:
  %v_offp = alloca i64, align 8
  %1 = getelementptr inbounds %struct.TupleTableSlot, ptr %0, i32 0, i32 5
  %tts_values = load ptr, ptr %1, align 8
  %2 = getelementptr inbounds %struct.TupleTableSlot, ptr %0, i32 0, i32 6
  %tts_ISNULL = load ptr, ptr %2, align 8
  %3 = getelementptr inbounds %struct.TupleTableSlot, ptr %0, i32 0, i32 1
  %4 = getelementptr inbounds %struct.TupleTableSlot, ptr %0, i32 0, i32 2
  %5 = getelementptr inbounds %struct.MinimalTupleTableSlot, ptr %0, i32 0, i32 4
  %6 = getelementptr inbounds %struct.MinimalTupleTableSlot, ptr %0, i32 0, i32 1
  %tupleheader = load ptr, ptr %6, align 8
  %7 = getelementptr inbounds %struct.HeapTupleData, ptr %tupleheader, i32 0, i32 3
  %tuple = load ptr, ptr %7, align 8
  %8 = getelementptr inbounds %struct.HeapTupleHeaderData, ptr %tuple, i32 0, i32 5
  %9 = getelementptr inbounds %struct.HeapTupleHeaderData, ptr %tuple, i32 0, i32 3
  %infomask1 = load i16, ptr %9, align 2
  %10 = getelementptr inbounds %struct.HeapTupleHeaderData, ptr %tuple, i32 0, i32 2
  %infomask2 = load i16, ptr %10, align 2
  %11 = and i16 1, %infomask1
  %hasnulls = icmp ne i16 %11, 0
  %maxatt = and i16 2047, %infomask2
  %12 = getelementptr inbounds %struct.HeapTupleHeaderData, ptr %tuple, i32 0, i32 4
  %13 = load i8, ptr %12, align 1
  %t_hoff = zext i8 %13 to i32
  %v_tupdata_base = getelementptr i8, ptr %tuple, i32 %t_hoff
  %v_slot_off = load i32, ptr %5, align 4
  %14 = zext i32 %v_slot_off to i64
  store i64 %14, ptr %v_offp, align 8
  %15 = icmp ult i16 %maxatt, 1
  br i1 %15, label %adjust_unavail_cols, label %find_startblock

adjust_unavail_cols:                              ; preds = %entry
  %16 = zext i16 %maxatt to i32
  call void @slot_getmissingattrs(ptr %0, i32 %16, i32 1)
  br label %find_startblock

find_startblock:                                  ; preds = %adjust_unavail_cols, %entry
  %17 = load i16, ptr %4, align 2
  switch i16 %17, label %deadblock [
    i16 0, label %block.attr.0.attcheckattno
  ]

outblock:                                         ; preds = %block.attr.0.store, %block.attr.0.attisnull, %block.attr.0.attcheckattno
  %18 = load i64, ptr %v_offp, align 8
  store i16 1, ptr %4, align 2
  %19 = trunc i64 %18 to i32
  store i32 %19, ptr %5, align 4
  %tts_flags = load i16, ptr %3, align 2
  %20 = or i16 %tts_flags, 8
  store i16 %20, ptr %3, align 2
  ret void

deadblock:                                        ; preds = %find_startblock
  unreachable

block.attr.0.attcheckattno:                       ; preds = %find_startblock
  store i64 0, ptr %v_offp, align 8
  %heap_natts = icmp uge i16 0, %maxatt
  br i1 %heap_natts, label %outblock, label %block.attr.0.start

block.attr.0.start:                               ; preds = %block.attr.0.attcheckattno
  %21 = getelementptr i8, ptr %8, i32 0
  %attnullbyte = load i8, ptr %21, align 1
  %22 = and i8 %attnullbyte, 1
  %attisnull = icmp eq i8 %22, 0
  %23 = and i1 %hasnulls, %attisnull
  br i1 %23, label %block.attr.0.attisnull, label %block.attr.0.attcheckalign

block.attr.0.attisnull:                           ; preds = %block.attr.0.start
  %24 = getelementptr i8, ptr %tts_ISNULL, i16 0
  store i8 1, ptr %24, align 1
  %25 = getelementptr i64, ptr %tts_values, i16 0
  store i64 0, ptr %25, align 8
  br label %outblock

block.attr.0.attcheckalign:                       ; preds = %block.attr.0.start
  br label %block.attr.0.align

block.attr.0.align:                               ; preds = %block.attr.0.attcheckalign
  br label %block.attr.0.store

block.attr.0.store:                               ; preds = %block.attr.0.align
  %26 = load i64, ptr %v_offp, align 8
  %27 = getelementptr i8, ptr %v_tupdata_base, i64 %26
  %28 = getelementptr i64, ptr %tts_values, i16 0
  %29 = getelementptr i8, ptr %tts_ISNULL, i16 0
  store i8 0, ptr %29, align 1
  %attr_byval = load i8, ptr %27, align 1
  %30 = zext i8 %attr_byval to i64
  store i64 %30, ptr %28, align 8
  %31 = load i64, ptr %v_offp, align 8
  %increment_offset = add i64 %31, 1
  store i64 %increment_offset, ptr %v_offp, align 8
  br label %outblock
}

[erthalion]

/cc @goldsteinn as an author of the commit referenced in [3], and @chandlerc as mentioned in code_owners regarding inlining pass.

[erthalion]

It turns out PostgreSQL was copying function attributes when generating IR, and noundef was incorrectly copied as well.