[llvm] [Inliner] Fix bug when propagating poison generating return attributes (PR #66036)

[Nikita Popov via llvm-commits]

================
@@ -1407,6 +1416,58 @@ static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
     // dereferenceable_or_null etc). See AttrBuilder::merge for more details.
     AttributeList AL = NewRetVal->getAttributes();
     AttributeList NewAL = AL.addRetAttributes(Context, Valid);
+    // Attributes that may generate poison returns are a bit tricky. If we
+    // propagate them, other uses of the callsite might have there behavior
+    // change/or cause UB (if they have noundef) b.c of the new potential
+    // poison.
+    // Take the following three cases:
+    //
+    // 1)
+    // define nonnull ptr @foo() {
+    //   %p = call ptr @bar()
+    //   call void @use(ptr %p) willreturn nounwind
+    //   ret ptr %p
+    // }
+    //
+    // 2)
+    // define noundef nonnull ptr @foo() {
+    //   %p = call ptr @bar()
+    //   call void @use(ptr %p) willreturn nounwind
+    //   ret ptr %p
+    // }
+    //
+    // 3)
+    // define nonnull ptr @foo() {
+    //   %p = call noundef ptr @bar()
+    //   ret ptr %p
+    // }
+    //
+    // In case 1, we can't propagate nonnull because poison value in @use may
+    // change behavior or trigger UB.
+    // In case 2, we don't need to be concerned about propagating nonnull, as
+    // any new poison at @use will trigger UB anyways.
+    // In case 3, we can never propagate nonnull because it may create UB due to
+    // the noundef on @bar.
+    if (ValidPG.hasAttributes()) {
+      bool Okay = true;
+      if (!CB.hasRetAttr(Attribute::NoUndef)) {
+        Okay = !RetVal->hasRetAttr(Attribute::NoUndef);
+        // See if we use this outside of return context. In that case since we
+        // don't have noundef on the to-be-inlined callsite, we won't
+        // necessarily have UB so don't propagate. TODO: This is overly
+        // conservative, we could check the uses to see if poison would change
+        // behavior / trigger UB.
+        for (const llvm::User *U : RetVal->users()) {
----------------
nikic wrote:

IMHO we should only introduce this loop once it actually does something useful. For now this is just confusing dead code.

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