Refine patch a bit.

- IR: rewrite isCompatibleWith, rename to isKnownCompatibleWith
- IR: comments, authenticated -> signed/ptrauth
- IR: hasAddressDiversity -> hasAddressDiscriminator
- LangRef.rst: typo
- PointerAuth.md: whitespace
- Bitcode: format

Author: ahmedbougacha

llvm/docs/LangRef.rst

@@ -4769,7 +4769,7 @@ Its type is the same as the first argument.  An integer constant discriminator
 and an address discriminator may be optionally specified.  Otherwise, they have
 values ``i64 0`` and ``ptr null``.
 
-If the address disciminator is ``null`` then the expression is equivalent to
+If the address discriminator is ``null`` then the expression is equivalent to
 
 .. code-block:: llvm
 

llvm/docs/PointerAuth.md

@@ -237,7 +237,7 @@ The latter are represented using a
 which describes an authenticated relocation producing a signed pointer.
 
 ```llvm
-  ptrauth (ptr CST, i32 KEY, i64 DISC, ptr ADDRDISC)
+ptrauth (ptr CST, i32 KEY, i64 DISC, ptr ADDRDISC)
 ```
 
 is equivalent to:

llvm/include/llvm/IR/Constants.h

@@ -1007,7 +1007,7 @@ struct OperandTraits<NoCFIValue> : public FixedNumOperandTraits<NoCFIValue, 1> {
 
 DEFINE_TRANSPARENT_OPERAND_ACCESSORS(NoCFIValue, Value)
 
-/// A signed pointer
+/// A signed pointer, in the ptrauth sense.
 ///
 class ConstantPtrAuth final : public Constant {
   friend struct ConstantPtrAuthKeyType;
@@ -1022,7 +1022,7 @@ class ConstantPtrAuth final : public Constant {
   Value *handleOperandChangeImpl(Value *From, Value *To);
 
 public:
-  /// Return a pointer authenticated with the specified parameters.
+  /// Return a pointer signed with the specified parameters.
   static ConstantPtrAuth *get(Constant *Ptr, ConstantInt *Key,
                               ConstantInt *Disc, Constant *AddrDisc);
 
@@ -1033,34 +1033,34 @@ class ConstantPtrAuth final : public Constant {
   /// Transparently provide more efficient getOperand methods.
   DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Constant);
 
-  /// The pointer that is authenticated in this authenticated global reference.
+  /// The pointer that is signed in this ptrauth signed pointer.
   Constant *getPointer() const { return cast<Constant>(Op<0>().get()); }
 
   /// The Key ID, an i32 constant.
   ConstantInt *getKey() const { return cast<ConstantInt>(Op<1>().get()); }
 
-  /// The discriminator.
+  /// The integer discriminator, an i64 constant, or 0.
   ConstantInt *getDiscriminator() const {
     return cast<ConstantInt>(Op<2>().get());
   }
 
   /// The address discriminator if any, or the null constant.
   /// If present, this must be a value equivalent to the storage location of
-  /// the only user of the authenticated ptrauth global.
+  /// the only global-initializer user of the ptrauth signed pointer.
   Constant *getAddrDiscriminator() const {
     return cast<Constant>(Op<3>().get());
   }
 
   /// Whether there is any non-null address discriminator.
-  bool hasAddressDiversity() const {
+  bool hasAddressDiscriminator() const {
     return !getAddrDiscriminator()->isNullValue();
   }
 
-  /// Check whether an authentication operation with key \p KeyV and (possibly
-  /// blended) discriminator \p DiscriminatorV is compatible with this
-  /// authenticated global reference.
-  bool isCompatibleWith(const Value *Key, const Value *Discriminator,
-                        const DataLayout &DL) const;
+  /// Check whether an authentication operation with key \p Key and (possibly
+  /// blended) discriminator \p Discriminator is known to be compatible with
+  /// this ptrauth signed pointer.
+  bool isKnownCompatibleWith(const Value *Key, const Value *Discriminator,
+                             const DataLayout &DL) const;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Value *V) {

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

@@ -3646,10 +3646,10 @@ Error BitcodeReader::parseConstants() {
       if (Record.size() < 4)
         return error("Invalid ptrauth record");
       // Ptr, Key, Disc, AddrDisc
-      V = BitcodeConstant::create(
-        Alloc, CurTy, BitcodeConstant::ConstantPtrAuthOpcode,
-        {(unsigned)Record[0], (unsigned)Record[1], (unsigned)Record[2],
-         (unsigned)Record[3]});
+      V = BitcodeConstant::create(Alloc, CurTy,
+                                  BitcodeConstant::ConstantPtrAuthOpcode,
+                                  {(unsigned)Record[0], (unsigned)Record[1],
+                                   (unsigned)Record[2], (unsigned)Record[3]});
       break;
     }
     }

llvm/lib/IR/Constants.cpp

@@ -2021,59 +2021,6 @@ Value *NoCFIValue::handleOperandChangeImpl(Value *From, Value *To) {
 //---- ConstantPtrAuth::get() implementations.
 //
 
-static bool areEquivalentAddrDiscriminators(const Value *V1, const Value *V2,
-                                            const DataLayout &DL) {
-  APInt Off1(DL.getTypeSizeInBits(V1->getType()), 0);
-  APInt Off2(DL.getTypeSizeInBits(V2->getType()), 0);
-
-  if (auto *V1Cast = dyn_cast<PtrToIntOperator>(V1))
-    V1 = V1Cast->getPointerOperand();
-  if (auto *V2Cast = dyn_cast<PtrToIntOperator>(V2))
-    V2 = V2Cast->getPointerOperand();
-  auto *V1Base = V1->stripAndAccumulateConstantOffsets(
-      DL, Off1, /*AllowNonInbounds=*/true);
-  auto *V2Base = V2->stripAndAccumulateConstantOffsets(
-      DL, Off2, /*AllowNonInbounds=*/true);
-  return V1Base == V2Base && Off1 == Off2;
-}
-
-bool ConstantPtrAuth::isCompatibleWith(const Value *Key,
-                                       const Value *Discriminator,
-                                       const DataLayout &DL) const {
-  // If the keys are different, there's no chance for this to be compatible.
-  if (Key != getKey())
-    return false;
-
-  // If the discriminators are the same, this is compatible iff there is no
-  // address discriminator.
-  if (Discriminator == getDiscriminator())
-    return getAddrDiscriminator()->isNullValue();
-
-  // If we dynamically blend the discriminator with the address discriminator,
-  // this is compatible.
-  if (auto *DiscBlend = dyn_cast<IntrinsicInst>(Discriminator)) {
-    if (DiscBlend->getIntrinsicID() == Intrinsic::ptrauth_blend &&
-        DiscBlend->getOperand(1) == getDiscriminator() &&
-        areEquivalentAddrDiscriminators(DiscBlend->getOperand(0),
-                                        getAddrDiscriminator(), DL))
-      return true;
-  }
-
-  // If we don't have a non-address discriminator, we don't need a blend in
-  // the first place:  accept the address discriminator as the discriminator.
-  if (getDiscriminator()->isNullValue() &&
-      areEquivalentAddrDiscriminators(getAddrDiscriminator(), Discriminator,
-                                      DL))
-    return true;
-
-  // Otherwise, we don't know.
-  return false;
-}
-
-ConstantPtrAuth *ConstantPtrAuth::getWithSameSchema(Constant *Pointer) const {
-  return get(Pointer, getKey(), getDiscriminator(), getAddrDiscriminator());
-}
-
 ConstantPtrAuth *ConstantPtrAuth::get(Constant *Ptr, ConstantInt *Key,
                                       ConstantInt *Disc, Constant *AddrDisc) {
   Constant *ArgVec[] = {Ptr, Key, Disc, AddrDisc};
@@ -2082,6 +2029,10 @@ ConstantPtrAuth *ConstantPtrAuth::get(Constant *Ptr, ConstantInt *Key,
   return pImpl->ConstantPtrAuths.getOrCreate(Ptr->getType(), MapKey);
 }
 
+ConstantPtrAuth *ConstantPtrAuth::getWithSameSchema(Constant *Pointer) const {
+  return get(Pointer, getKey(), getDiscriminator(), getAddrDiscriminator());
+}
+
 ConstantPtrAuth::ConstantPtrAuth(Constant *Ptr, ConstantInt *Key,
                                  ConstantInt *Disc, Constant *AddrDisc)
     : Constant(Ptr->getType(), Value::ConstantPtrAuthVal, &Op<0>(), 4) {
@@ -2125,6 +2076,66 @@ Value *ConstantPtrAuth::handleOperandChangeImpl(Value *From, Value *ToV) {
       Values, this, From, To, NumUpdated, OperandNo);
 }
 
+bool ConstantPtrAuth::isKnownCompatibleWith(const Value *Key,
+                                            const Value *Discriminator,
+                                            const DataLayout &DL) const {
+  // If the keys are different, there's no chance for this to be compatible.
+  if (getKey() != Key)
+    return false;
+
+  // We can have 3 kinds of discriminators:
+  // - simple, integer-only:    `i64 x, ptr null` vs. `i64 x`
+  // - address-only:            `i64 0, ptr p` vs. `ptr p`
+  // - blended address/integer: `i64 x, ptr p` vs. `@llvm.ptrauth.blend(p, x)`
+
+  // If this constant has a simple discriminator (integer, no address), easy:
+  // it's compatible iff the provided full discriminator is also a simple
+  // discriminator, identical to our integer discriminator.
+  if (!hasAddressDiscriminator())
+    return getDiscriminator() == Discriminator;
+
+  // Otherwise, we can isolate address and integer discriminator components.
+  const Value *AddrDiscriminator = nullptr;
+
+  // This constant may or may not have an integer discriminator (instead of 0).
+  if (!getDiscriminator()->isNullValue()) {
+    // If it does, there's an implicit blend.  We need to have a matching blend
+    // intrinsic in the provided full discriminator.
+    if (!match(Discriminator,
+               m_Intrinsic<Intrinsic::ptrauth_blend>(
+                   m_Value(AddrDiscriminator), m_Specific(getDiscriminator()))))
+      return false;
+  } else {
+    // Otherwise, interpret the provided full discriminator as address-only.
+    AddrDiscriminator = Discriminator;
+  }
+
+  // Either way, we can now focus on comparing the address discriminators.
+
+  // Discriminators are i64, so the provided addr disc may be a ptrtoint.
+  if (auto *Cast = dyn_cast<PtrToIntOperator>(AddrDiscriminator))
+    AddrDiscriminator = Cast->getPointerOperand();
+
+  // Beyond that, we're only interested in compatible pointers.
+  if (getAddrDiscriminator()->getType() != AddrDiscriminator->getType())
+    return false;
+
+  // These are often the same constant GEP, making them trivially equivalent.
+  if (getAddrDiscriminator() == AddrDiscriminator)
+    return true;
+
+  // Finally, they may be equivalent base+offset expressions.
+  APInt Off1(DL.getIndexTypeSizeInBits(getAddrDiscriminator()->getType()), 0);
+  auto *Base1 = getAddrDiscriminator()->stripAndAccumulateConstantOffsets(
+      DL, Off1, /*AllowNonInbounds=*/true);
+
+  APInt Off2(DL.getIndexTypeSizeInBits(AddrDiscriminator->getType()), 0);
+  auto *Base2 = AddrDiscriminator->stripAndAccumulateConstantOffsets(
+      DL, Off2, /*AllowNonInbounds=*/true);
+
+  return Base1 == Base2 && Off1 == Off2;
+}
+
 //---- ConstantExpr::get() implementations.
 //
 

llvm/test/Assembler/ptrauth-const.ll

@@ -11,16 +11,14 @@
 ; CHECK: @intdisc = global ptr ptrauth (ptr @var, i32 0, i64 -1)
 @intdisc = global ptr ptrauth (ptr @var, i32 0, i64 -1)
 
-@addrdisc_storage = global ptr null
-; CHECK: @addrdisc = global ptr ptrauth (ptr @var, i32 2, i64 1234, ptr @addrdisc_storage)
-@addrdisc = global ptr ptrauth (ptr @var, i32 2, i64 1234, ptr @addrdisc_storage)
+; CHECK: @addrdisc = global ptr ptrauth (ptr @var, i32 2, i64 1234, ptr @addrdisc)
+@addrdisc = global ptr ptrauth (ptr @var, i32 2, i64 1234, ptr @addrdisc)
 
 
 @var1 = addrspace(1) global i32 0
 
 ; CHECK: @addrspace = global ptr addrspace(1) ptrauth (ptr addrspace(1) @var1, i32 0)
 @addrspace = global ptr addrspace(1) ptrauth (ptr addrspace(1) @var1, i32 0)
 
-@addrspace_addrdisc_storage = addrspace(2) global ptr addrspace(1) null
-; CHECK: @addrspace_addrdisc = global ptr addrspace(1) ptrauth (ptr addrspace(1) @var1, i32 2, i64 1234, ptr addrspace(2) @addrspace_addrdisc_storage)
-@addrspace_addrdisc = global ptr addrspace(1) ptrauth (ptr addrspace(1) @var1, i32 2, i64 1234, ptr addrspace(2) @addrspace_addrdisc_storage)
+; CHECK: @addrspace_addrdisc = addrspace(2) global ptr addrspace(1) ptrauth (ptr addrspace(1) @var1, i32 2, i64 1234, ptr addrspace(2) @addrspace_addrdisc)
+@addrspace_addrdisc = addrspace(2) global ptr addrspace(1) ptrauth (ptr addrspace(1) @var1, i32 2, i64 1234, ptr addrspace(2) @addrspace_addrdisc)