Bug 1738300 - Apply clang upstream patch fixing miscompilation. r=firefox-build-system-reviewers,mhentges

Differential Revision: https://phabricator.services.mozilla.com/D130428

build/build-clang/clang-13-linux64.json

@@ -18,6 +18,7 @@
       "revert-llvmorg-13-init-7827-g2a078c307204.patch",
       "loosen-msvc-detection.patch",
       "fuzzing_ccov_build_clang_12.patch",
+      "llvmorg-14-init-6706-g6404f4b5af39.patch",
       "revert-ga478b0a199f4.patch"
     ]
 }

build/build-clang/clang-13-macosx64.json

@@ -17,6 +17,7 @@
       "static-llvm-symbolizer_clang_12.patch",
       "compiler-rt-cross-compile.patch",
       "revert-llvmorg-13-init-7827-g2a078c307204.patch",
+      "llvmorg-14-init-6706-g6404f4b5af39.patch",
       "compiler-rt-13-no-codesign.patch",
       "revert-ga478b0a199f4.patch"
     ]

build/build-clang/clang-13-win64-2stage.json

@@ -8,6 +8,7 @@
     "ml": "ml64.exe",
     "patches": [
       "llvmorg-14-init-4465-g22ea0cea595e-v2.patch",
+      "llvmorg-14-init-6706-g6404f4b5af39.patch",
       "unpoison-thread-stacks_clang_10.patch",
       "bug47258-extract-symbols-mbcs.patch"
     ]

build/build-clang/clang-13-win64.json

@@ -15,6 +15,7 @@
       "bug47258-extract-symbols-mbcs.patch",
       "Remove-FlushViewOfFile-when-unmaping-gcda-files.patch",
       "revert-llvmorg-13-init-7827-g2a078c307204.patch",
+      "llvmorg-14-init-6706-g6404f4b5af39.patch",
       "loosen-msvc-detection.patch",
       "win64-no-symlink.patch",
       "revert-ga478b0a199f4.patch"

build/build-clang/llvmorg-14-init-6706-g6404f4b5af39.patch

@@ -0,0 +1,48 @@
+From 6404f4b5af39840a2dad27abc3924eb3846ae8a4 Mon Sep 17 00:00:00 2001
+From: Shoaib Meenai <smeenai@fb.com>
+Date: Sun, 10 Oct 2021 14:06:49 -0700
+Subject: [PATCH] [InstCombine] Remove attributes after hoisting free above
+ null check
+
+If the parameter had been annotated as nonnull because of the null
+check, we want to remove the attribute, since it may no longer apply and
+could result in miscompiles if left. Similarly, we also want to remove
+undef-implying attributes, since they may not apply anymore either.
+
+Fixes PR52110.
+
+Reviewed By: nikic
+
+Differential Revision: https://reviews.llvm.org/D111515
+
+diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+index 4e3b18e805ee..71b3a411cc18 100644
+--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
++++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+@@ -2843,6 +2843,26 @@ static Instruction *tryToMoveFreeBeforeNullTest(CallInst &FI,
+   }
+   assert(FreeInstrBB->size() == 1 &&
+          "Only the branch instruction should remain");
++
++  // Now that we've moved the call to free before the NULL check, we have to
++  // remove any attributes on its parameter that imply it's non-null, because
++  // those attributes might have only been valid because of the NULL check, and
++  // we can get miscompiles if we keep them. This is conservative if non-null is
++  // also implied by something other than the NULL check, but it's guaranteed to
++  // be correct, and the conservativeness won't matter in practice, since the
++  // attributes are irrelevant for the call to free itself and the pointer
++  // shouldn't be used after the call.
++  AttributeList Attrs = FI.getAttributes();
++  Attrs = Attrs.removeParamAttribute(FI.getContext(), 0, Attribute::NonNull);
++  Attribute Dereferenceable = Attrs.getParamAttr(0, Attribute::Dereferenceable);
++  if (Dereferenceable.isValid()) {
++    uint64_t Bytes = Dereferenceable.getDereferenceableBytes();
++    Attrs = Attrs.removeParamAttribute(FI.getContext(), 0,
++                                       Attribute::Dereferenceable);
++    Attrs = Attrs.addDereferenceableOrNullParamAttr(FI.getContext(), 0, Bytes);
++  }
++  FI.setAttributes(Attrs);
++
+   return &FI;
+ }
+