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

[via llvm-commits]

================
@@ -1406,7 +1417,55 @@ static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
     // existing attribute value (i.e. attributes such as dereferenceable,
     // dereferenceable_or_null etc). See AttrBuilder::merge for more details.
     AttributeList AL = NewRetVal->getAttributes();
-    AttributeList NewAL = AL.addRetAttributes(Context, Valid);
+    AttributeList NewAL = AL.addRetAttributes(Context, ValidUB);
+    // Attributes that may generate poison returns are a bit tricky. If we
+    // propagate them, other uses of the callsite might have their 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)) {
+        // Cover case 3 where introducing a poison generating attribute can
+        // cause immediate UB.
+        Okay = !RetVal->hasRetAttr(Attribute::NoUndef);
+        // This is an overly conservative way to check that there are no other
+        // uses whose behavior may change/may introduce UB if we potentially
+        // return poison.
+        // TODO: Iterative through uses and only bail if we have a potentially
+        // dangerous use.
+        Okay &= RetVal->hasOneUse();
+      }
+      if (Okay)
----------------
goldsteinn wrote:

Done, although imo it makes the comments a bit more difficult to read.

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