Simplify some functionality and address review feedback.

- don't reject weakref base pointers (allowed in quals now anyway)
- fold buildConstantAddress into getConstantSignedPointer (originally
  to provide a caching layer for global wrappers but not needed anymore)
- remove stripPointerCasts from it (originally for typed pointers)
- use ConstantInt for the integer/other component of discriminators
- remove CodeGen:: helper (originally for swift but not required)
- rejigger tests; try for parity in fn body and global init
- add a couple missing tests
- tweak doc/header comment
- adopt 'can be used in cst exprs' phrasing from LangExts doc
- various nits, naming, and format

Author: ahmedbougacha

clang/docs/PointerAuthentication.rst

@@ -368,10 +368,14 @@ a non-attackable sequence.
 
 ``pointer`` must be a constant expression of pointer type which evaluates to
 a non-null pointer.  The result will have the same type as ``discriminator``.
+``pointer`` must be a constant expression of pointer type which evaluates to
+a non-null pointer.
+``key``  must be a constant expression of type ``ptrauth_key``.
+``discriminator`` must be a constant expression of pointer or integer type;
+if an integer, it will be coerced to ``ptrauth_extra_data_t``.
+The result will have the same type as ``pointer``.
 
-Calls to this are constant expressions if the discriminator is a null-pointer
-constant expression or an integer constant expression. Implementations may
-allow other pointer expressions as well.
+This can be used in constant expressions.
 
 ``ptrauth_sign_unauthenticated``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/CodeGen/CodeGenABITypes.h

@@ -104,12 +104,6 @@ llvm::Type *convertTypeForMemory(CodeGenModule &CGM, QualType T);
 unsigned getLLVMFieldNumber(CodeGenModule &CGM,
                             const RecordDecl *RD, const FieldDecl *FD);
 
-/// Return a signed constant pointer.
-llvm::Constant *getConstantSignedPointer(CodeGenModule &CGM,
-                                         llvm::Constant *Pointer, unsigned Key,
-                                         llvm::Constant *StorageAddress,
-                                         llvm::Constant *OtherDiscriminator);
-
 /// Given the language and code-generation options that Clang was configured
 /// with, set the default LLVM IR attributes for a function definition.
 /// The attributes set here are mostly global target-configuration and

clang/lib/CodeGen/CGExprConstant.cpp

@@ -1859,7 +1859,7 @@ class ConstantLValueEmitter : public ConstStmtVisitor<ConstantLValueEmitter,
   ConstantLValue emitPointerAuthSignConstant(const CallExpr *E);
   llvm::Constant *emitPointerAuthPointer(const Expr *E);
   unsigned emitPointerAuthKey(const Expr *E);
-  std::pair<llvm::Constant*, llvm::Constant*>
+  std::pair<llvm::Constant *, llvm::ConstantInt *>
   emitPointerAuthDiscriminator(const Expr *E);
 
   bool hasNonZeroOffset() const {
@@ -2075,9 +2075,7 @@ ConstantLValue
 ConstantLValueEmitter::emitPointerAuthSignConstant(const CallExpr *E) {
   llvm::Constant *UnsignedPointer = emitPointerAuthPointer(E->getArg(0));
   unsigned Key = emitPointerAuthKey(E->getArg(1));
-  llvm::Constant *StorageAddress;
-  llvm::Constant *OtherDiscriminator;
-  std::tie(StorageAddress, OtherDiscriminator) =
+  auto [StorageAddress, OtherDiscriminator] =
       emitPointerAuthDiscriminator(E->getArg(2));
 
   llvm::Constant *SignedPointer = CGM.getConstantSignedPointer(
@@ -2101,26 +2099,25 @@ unsigned ConstantLValueEmitter::emitPointerAuthKey(const Expr *E) {
   return E->EvaluateKnownConstInt(CGM.getContext()).getZExtValue();
 }
 
-std::pair<llvm::Constant *, llvm::Constant *>
+std::pair<llvm::Constant *, llvm::ConstantInt *>
 ConstantLValueEmitter::emitPointerAuthDiscriminator(const Expr *E) {
   E = E->IgnoreParens();
 
-  if (auto *Call = dyn_cast<CallExpr>(E)) {
+  if (const auto *Call = dyn_cast<CallExpr>(E)) {
     if (Call->getBuiltinCallee() ==
         Builtin::BI__builtin_ptrauth_blend_discriminator) {
       llvm::Constant *Pointer = ConstantEmitter(CGM).emitAbstract(
           Call->getArg(0), Call->getArg(0)->getType());
-      llvm::Constant *Extra = ConstantEmitter(CGM).emitAbstract(
-          Call->getArg(1), Call->getArg(1)->getType());
+      auto *Extra = cast<llvm::ConstantInt>(ConstantEmitter(CGM).emitAbstract(
+          Call->getArg(1), Call->getArg(1)->getType()));
       return {Pointer, Extra};
     }
   }
 
   llvm::Constant *Result = ConstantEmitter(CGM).emitAbstract(E, E->getType());
   if (Result->getType()->isPointerTy())
     return {Result, nullptr};
-  else
-    return {nullptr, Result};
+  return {nullptr, cast<llvm::ConstantInt>(Result)};
 }
 
 ConstantLValue

clang/lib/CodeGen/CGPointerAuth.cpp

@@ -17,47 +17,27 @@
 using namespace clang;
 using namespace CodeGen;
 
-/// Build a signed-pointer "ptrauth" constant.
-static llvm::ConstantPtrAuth *
-buildConstantAddress(CodeGenModule &CGM, llvm::Constant *Pointer, unsigned Key,
-                     llvm::Constant *StorageAddress,
-                     llvm::Constant *OtherDiscriminator) {
-  llvm::Constant *AddressDiscriminator = nullptr;
+llvm::Constant *
+CodeGenModule::getConstantSignedPointer(llvm::Constant *Pointer, unsigned Key,
+                                        llvm::Constant *StorageAddress,
+                                        llvm::ConstantInt *OtherDiscriminator) {
+  llvm::Constant *AddressDiscriminator;
   if (StorageAddress) {
+    assert(StorageAddress->getType() == UnqualPtrTy);
     AddressDiscriminator = StorageAddress;
-    assert(StorageAddress->getType() == CGM.UnqualPtrTy);
   } else {
-    AddressDiscriminator = llvm::Constant::getNullValue(CGM.UnqualPtrTy);
+    AddressDiscriminator = llvm::Constant::getNullValue(UnqualPtrTy);
   }
 
-  llvm::ConstantInt *IntegerDiscriminator = nullptr;
+  llvm::ConstantInt *IntegerDiscriminator;
   if (OtherDiscriminator) {
-    assert(OtherDiscriminator->getType() == CGM.Int64Ty);
-    IntegerDiscriminator = cast<llvm::ConstantInt>(OtherDiscriminator);
+    assert(OtherDiscriminator->getType() == Int64Ty);
+    IntegerDiscriminator = OtherDiscriminator;
   } else {
-    IntegerDiscriminator = llvm::ConstantInt::get(CGM.Int64Ty, 0);
+    IntegerDiscriminator = llvm::ConstantInt::get(Int64Ty, 0);
   }
 
   return llvm::ConstantPtrAuth::get(Pointer,
-                                    llvm::ConstantInt::get(CGM.Int32Ty, Key),
+                                    llvm::ConstantInt::get(Int32Ty, Key),
                                     IntegerDiscriminator, AddressDiscriminator);
 }
-
-llvm::Constant *
-CodeGenModule::getConstantSignedPointer(llvm::Constant *Pointer, unsigned Key,
-                                        llvm::Constant *StorageAddress,
-                                        llvm::Constant *OtherDiscriminator) {
-  llvm::Constant *Stripped = Pointer->stripPointerCasts();
-
-  // Build the constant.
-  return buildConstantAddress(*this, Stripped, Key, StorageAddress,
-                              OtherDiscriminator);
-}
-
-llvm::Constant *
-CodeGen::getConstantSignedPointer(CodeGenModule &CGM, llvm::Constant *Pointer,
-                                  unsigned Key, llvm::Constant *StorageAddress,
-                                  llvm::Constant *OtherDiscriminator) {
-  return CGM.getConstantSignedPointer(Pointer, Key, StorageAddress,
-                                      OtherDiscriminator);
-}

clang/lib/CodeGen/CodeGenModule.h

@@ -937,10 +937,10 @@ class CodeGenModule : public CodeGenTypeCache {
   // Return the function body address of the given function.
   llvm::Constant *GetFunctionStart(const ValueDecl *Decl);
 
-  llvm::Constant *getConstantSignedPointer(llvm::Constant *Pointer,
-                                           unsigned Key,
-                                           llvm::Constant *StorageAddress,
-                                           llvm::Constant *ExtraDiscrim);
+  llvm::Constant *
+  getConstantSignedPointer(llvm::Constant *Pointer, unsigned Key,
+                           llvm::Constant *StorageAddress,
+                           llvm::ConstantInt *OtherDiscriminator);
 
   // Return whether RTTI information should be emitted for this target.
   bool shouldEmitRTTI(bool ForEH = false) {

clang/lib/Headers/ptrauth.h

@@ -78,17 +78,17 @@ typedef __UINTPTR_TYPE__ ptrauth_generic_signature_t;
 #define ptrauth_blend_discriminator(__pointer, __integer)                      \
   __builtin_ptrauth_blend_discriminator(__pointer, __integer)
 
-/* Add a signature to the given pointer value using a specific key,
-   using the given extra data as a salt to the signing process.
+/* Return a signed pointer for a constant address in a manner which guarantees
+   a non-attackable sequence.
 
-   The value must be a constant expression of pointer type.
+   The value must be a constant expression of pointer type which evaluates to
+   a non-null pointer.
    The key must be a constant expression of type ptrauth_key.
    The extra data must be a constant expression of pointer or integer type;
    if an integer, it will be coerced to ptrauth_extra_data_t.
    The result will have the same type as the original value.
 
-   This is a constant expression if the extra data is an integer or
-   null pointer constant. */
+   This can be used in constant expressions.  */
 #define ptrauth_sign_constant(__value, __key, __data)                          \
   __builtin_ptrauth_sign_constant(__value, __key, __data)
 

clang/lib/Sema/SemaChecking.cpp

@@ -2045,19 +2045,17 @@ findConstantBaseAndOffset(Sema &S, Expr *E) {
   // Must evaluate as a pointer.
   Expr::EvalResult Result;
   if (!E->EvaluateAsRValue(Result, S.Context) || !Result.Val.isLValue())
-    return std::make_pair(nullptr, CharUnits());
+    return {nullptr, CharUnits()};
 
-  // Base must be a declaration and can't be weakly imported.
   const auto *BaseDecl =
       Result.Val.getLValueBase().dyn_cast<const ValueDecl *>();
-  if (!BaseDecl || BaseDecl->hasAttr<WeakRefAttr>())
-    return std::make_pair(nullptr, CharUnits());
+  if (!BaseDecl)
+    return {nullptr, CharUnits()};
 
-  return std::make_pair(BaseDecl, Result.Val.getLValueOffset());
+  return {BaseDecl, Result.Val.getLValueOffset()};
 }
 
-static bool checkPointerAuthValue(Sema &S, Expr *&Arg,
-                                  PointerAuthOpKind OpKind,
+static bool checkPointerAuthValue(Sema &S, Expr *&Arg, PointerAuthOpKind OpKind,
                                   bool RequireConstant = false) {
   if (Arg->hasPlaceholderType()) {
     ExprResult R = S.CheckPlaceholderExpr(Arg);
@@ -2119,25 +2117,23 @@ static bool checkPointerAuthValue(Sema &S, Expr *&Arg,
   // The main argument.
   if (OpKind == PAO_Sign) {
     // Require the value we're signing to have a special form.
-    auto BaseOffsetPair = findConstantBaseAndOffset(S, Arg);
+    auto [BaseDecl, Offset] = findConstantBaseAndOffset(S, Arg);
     bool Invalid;
 
     // Must be rooted in a declaration reference.
-    if (!BaseOffsetPair.first) {
+    if (!BaseDecl)
       Invalid = true;
 
-      // If it's a function declaration, we can't have an offset.
-    } else if (isa<FunctionDecl>(BaseOffsetPair.first)) {
-      Invalid = !BaseOffsetPair.second.isZero();
+    // If it's a function declaration, we can't have an offset.
+    else if (isa<FunctionDecl>(BaseDecl))
+      Invalid = !Offset.isZero();
 
-      // Otherwise we're fine.
-    } else {
+    // Otherwise we're fine.
+    else
       Invalid = false;
-    }
 
-    if (Invalid) {
+    if (Invalid)
       S.Diag(Arg->getExprLoc(), diag::err_ptrauth_bad_constant_pointer);
-    }
     return Invalid;
   }
 
@@ -2169,10 +2165,9 @@ static bool checkPointerAuthValue(Sema &S, Expr *&Arg,
     // TODO: if we're initializing a global, check that the address is
     // somehow related to what we're initializing.  This probably will
     // never really be feasible and we'll have to catch it at link-time.
-    auto BaseOffsetPair = findConstantBaseAndOffset(S, Pointer);
-    if (!BaseOffsetPair.first || !isa<VarDecl>(BaseOffsetPair.first)) {
+    auto [BaseDecl, Offset] = findConstantBaseAndOffset(S, Pointer);
+    if (!BaseDecl || !isa<VarDecl>(BaseDecl))
       Invalid = true;
-    }
   }
 
   // Check the integer.
@@ -2182,9 +2177,8 @@ static bool checkPointerAuthValue(Sema &S, Expr *&Arg,
       Invalid = true;
   }
 
-  if (Invalid) {
+  if (Invalid)
     S.Diag(Arg->getExprLoc(), diag::err_ptrauth_bad_constant_discriminator);
-  }
   return Invalid;
 }