Address review comments

- Remove unneeded declarations. - Fix includes. - Use `llvm::to_underlying`. - Fix variable names. - Fix size and type of bit-fields. - Add test case for `err_drv_ptrauth_not_supported`.
llvm · Jun 20, 2024 · 6d578df · 6d578df
1 parent 4cebec2
commit 6d578df
Show file tree

Hide file tree

Showing 10 changed files with 52 additions and 64 deletions.
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
@@ -346,8 +346,6 @@ class LangOptionsBase {
     BKey
   };
 
-  using PointerAuthenticationMode = ::clang::PointerAuthenticationMode;
-
   enum class ThreadModelKind {
     /// POSIX Threads.
     POSIX,

diff --git a/clang/include/clang/Basic/PointerAuthOptions.h b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -16,12 +16,10 @@
 
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Target/TargetOptions.h"
-#include <map>
-#include <memory>
-#include <string>
-#include <vector>
+#include <optional>
 
 namespace clang {
 
@@ -76,7 +74,7 @@ class PointerAuthSchema {
         IsIsaPointer(IsIsaPointer),
         AuthenticatesNullValues(AuthenticatesNullValues),
         SelectedAuthenticationMode(AuthenticationMode),
-        DiscriminationKind(OtherDiscrimination), Key(unsigned(Key)) {
+        DiscriminationKind(OtherDiscrimination), Key(llvm::to_underlying(Key)) {
     assert((getOtherDiscrimination() != Discrimination::Constant ||
             ConstantDiscriminatorOrNone) &&
            "constant discrimination requires a constant!");
@@ -126,15 +124,15 @@ class PointerAuthSchema {
 
   uint16_t getConstantDiscrimination() const {
     assert(getOtherDiscrimination() == Discrimination::Constant);
-    return (uint16_t)ConstantDiscriminator;
+    return ConstantDiscriminator;
   }
 
   unsigned getKey() const {
     switch (getKind()) {
     case Kind::None:
       llvm_unreachable("calling getKey() on disabled schema");
     case Kind::ARM8_3:
-      return unsigned(getARM8_3Key());
+      return llvm::to_underlying(getARM8_3Key());
     }
     llvm_unreachable("bad key kind");
   }

diff --git a/clang/lib/CodeGen/CGCall.h b/clang/lib/CodeGen/CGCall.h
@@ -196,9 +196,9 @@ class CGCallee {
     KindOrFunctionPointer =
         SpecialKind(reinterpret_cast<uintptr_t>(functionPtr));
   }
-  void setPointerAuthInfo(CGPointerAuthInfo pointerAuth) {
+  void setPointerAuthInfo(CGPointerAuthInfo PointerAuth) {
     assert(isOrdinary());
-    OrdinaryInfo.PointerAuthInfo = pointerAuth;
+    OrdinaryInfo.PointerAuthInfo = PointerAuth;
   }
 
   bool isVirtual() const {

diff --git a/clang/lib/CodeGen/CGPointerAuth.cpp b/clang/lib/CodeGen/CGPointerAuth.cpp
@@ -20,7 +20,7 @@ using namespace CodeGen;
 /// Return the abstract pointer authentication schema for a pointer to the given
 /// function type.
 CGPointerAuthInfo CodeGenModule::getFunctionPointerAuthInfo(QualType T) {
-  auto &Schema = getCodeGenOpts().PointerAuth.FunctionPointers;
+  const auto &Schema = getCodeGenOpts().PointerAuth.FunctionPointers;
   if (!Schema)
     return CGPointerAuthInfo();
 
@@ -62,33 +62,25 @@ CodeGenModule::getConstantSignedPointer(llvm::Constant *Pointer, unsigned Key,
 
 /// If applicable, sign a given constant function pointer with the ABI rules for
 /// functionType.
-llvm::Constant *CodeGenModule::getFunctionPointer(llvm::Constant *pointer,
-                                                  QualType functionType,
+llvm::Constant *CodeGenModule::getFunctionPointer(llvm::Constant *Pointer,
+                                                  QualType FunctionType,
                                                   GlobalDecl GD) {
-  assert(functionType->isFunctionType() ||
-         functionType->isFunctionReferenceType() ||
-         functionType->isFunctionPointerType());
+  assert(FunctionType->isFunctionType() ||
+         FunctionType->isFunctionReferenceType() ||
+         FunctionType->isFunctionPointerType());
 
-  if (auto pointerAuth = getFunctionPointerAuthInfo(functionType)) {
+  if (auto PointerAuth = getFunctionPointerAuthInfo(FunctionType)) {
     return getConstantSignedPointer(
-      pointer, pointerAuth.getKey(), nullptr,
-      cast_or_null<llvm::Constant>(pointerAuth.getDiscriminator()));
+      Pointer, PointerAuth.getKey(), nullptr,
+      cast_or_null<llvm::Constant>(PointerAuth.getDiscriminator()));
   }
 
-  return pointer;
+  return Pointer;
 }
 
 llvm::Constant *CodeGenModule::getFunctionPointer(GlobalDecl GD,
                                                   llvm::Type *Ty) {
-  const FunctionDecl *FD = cast<FunctionDecl>(GD.getDecl());
-
-  // Annoyingly, K&R functions have prototypes in the clang AST, but
-  // expressions referring to them are unprototyped.
+  const auto *FD = cast<FunctionDecl>(GD.getDecl());
   QualType FuncType = FD->getType();
-  if (!FD->hasPrototype())
-    if (const auto *Proto = FuncType->getAs<FunctionProtoType>())
-      FuncType = Context.getFunctionNoProtoType(Proto->getReturnType(),
-                                                Proto->getExtInfo());
-
   return getFunctionPointer(getRawFunctionPointer(GD, Ty), FuncType, GD);
 }
diff --git a/clang/lib/CodeGen/CGPointerAuthInfo.h b/clang/lib/CodeGen/CGPointerAuthInfo.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_LIB_CODEGEN_CGPOINTERAUTHINFO_H
 
 #include "clang/AST/Type.h"
+#include "clang/Basic/LangOptions.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
 
@@ -23,9 +24,9 @@ namespace CodeGen {
 class CGPointerAuthInfo {
 private:
   PointerAuthenticationMode AuthenticationMode : 2;
-  bool IsIsaPointer : 1;
-  bool AuthenticatesNullValues : 1;
-  unsigned Key : 28;
+  unsigned IsIsaPointer : 1;
+  unsigned AuthenticatesNullValues : 1;
+  unsigned Key : 4;
   llvm::Value *Discriminator;
 
 public:

diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -3049,18 +3049,18 @@ llvm::Value *CodeGenFunction::emitBoolVecConversion(llvm::Value *SrcVec,
 }
 
 void CodeGenFunction::EmitPointerAuthOperandBundle(
-    const CGPointerAuthInfo &pointerAuth,
-    SmallVectorImpl<llvm::OperandBundleDef> &bundles) {
-  if (!pointerAuth.isSigned())
+    const CGPointerAuthInfo &PointerAuth,
+    SmallVectorImpl<llvm::OperandBundleDef> &Bundles) {
+  if (!PointerAuth.isSigned())
     return;
 
-  auto key = Builder.getInt32(pointerAuth.getKey());
+  auto Key = Builder.getInt32(PointerAuth.getKey());
 
-  llvm::Value *discriminator = pointerAuth.getDiscriminator();
-  if (!discriminator) {
-    discriminator = Builder.getSize(0);
+  llvm::Value *Discriminator = PointerAuth.getDiscriminator();
+  if (!Discriminator) {
+    Discriminator = Builder.getSize(0);
   }
 
-  llvm::Value *args[] = {key, discriminator};
-  bundles.emplace_back("ptrauth", args);
+  llvm::Value *Args[] = {Key, Discriminator};
+  Bundles.emplace_back("ptrauth", Args);
 }
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4416,13 +4416,10 @@ class CodeGenFunction : public CodeGenTypeCache {
   }
 
   bool isPointerKnownNonNull(const Expr *E);
-  CGPointerAuthInfo EmitPointerAuthInfo(const PointerAuthSchema &schema,
-                                        llvm::Value *storageAddress,
-                                        GlobalDecl calleeDecl,
-                                        QualType calleeType);
+
   void EmitPointerAuthOperandBundle(
-      const CGPointerAuthInfo &info,
-      SmallVectorImpl<llvm::OperandBundleDef> &bundles);
+      const CGPointerAuthInfo &Info,
+      SmallVectorImpl<llvm::OperandBundleDef> &Bundles);
 
   // Return the copy constructor name with the prefix "__copy_constructor_"
   // removed.

diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
@@ -25,7 +25,6 @@
 #include "clang/Basic/ABI.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/NoSanitizeList.h"
-#include "clang/Basic/PointerAuthOptions.h"
 #include "clang/Basic/ProfileList.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/XRayLists.h"
@@ -953,28 +952,23 @@ class CodeGenModule : public CodeGenTypeCache {
   /// Return the ABI-correct function pointer value for a reference
   /// to the given function.  This will apply a pointer signature if
   /// necessary, but will only cache the result if \p FD is passed.
-  llvm::Constant *getFunctionPointer(llvm::Constant *pointer,
-                                     QualType functionType,
+  llvm::Constant *getFunctionPointer(llvm::Constant *Pointer,
+                                     QualType FunctionType,
                                      GlobalDecl GD = GlobalDecl());
 
-  CGPointerAuthInfo getFunctionPointerAuthInfo(QualType functionType);
+  CGPointerAuthInfo getFunctionPointerAuthInfo(QualType T);
 
-  CGPointerAuthInfo getPointerAuthInfoForPointeeType(QualType type);
+  llvm::Constant *getConstantSignedPointer(llvm::Constant *Pointer,
+                                           const PointerAuthSchema &Schema,
+                                           llvm::Constant *StorageAddress,
+                                           GlobalDecl SchemaDecl,
+                                           QualType SchemaType);
 
-  CGPointerAuthInfo getPointerAuthInfoForType(QualType type);
-
-  llvm::Constant *getConstantSignedPointer(llvm::Constant *pointer,
-                                           const PointerAuthSchema &schema,
-                                           llvm::Constant *storageAddress,
-                                           GlobalDecl schemaDecl,
-                                           QualType schemaType);
   llvm::Constant *
   getConstantSignedPointer(llvm::Constant *Pointer, unsigned Key,
                            llvm::Constant *StorageAddress,
                            llvm::ConstantInt *OtherDiscriminator);
 
-  CGPointerAuthInfo EmitPointerAuthInfo(const RecordDecl *RD);
-
   // Return whether RTTI information should be emitted for this target.
   bool shouldEmitRTTI(bool ForEH = false) {
     return (ForEH || getLangOpts().RTTI) && !getLangOpts().CUDAIsDevice &&

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1476,7 +1476,6 @@ bool CompilerInvocation::setDefaultPointerAuthOptions(
 }
 
 static bool parsePointerAuthOptions(PointerAuthOptions &Opts,
-                                    ArgList &Args,
                                     const LangOptions &LangOpts,
                                     const llvm::Triple &Triple,
                                     DiagnosticsEngine &Diags) {
@@ -2187,7 +2186,7 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
   if (!LangOpts->CUDAIsDevice)
-    parsePointerAuthOptions(Opts.PointerAuth, Args, *LangOpts, T, Diags);
+    parsePointerAuthOptions(Opts.PointerAuth, *LangOpts, T, Diags);
 
   if (Args.hasArg(options::OPT_ffinite_loops))
     Opts.FiniteLoops = CodeGenOptions::FiniteLoopsKind::Always;

diff --git a/clang/test/Driver/ptrauth.c b/clang/test/Driver/ptrauth.c
@@ -0,0 +1,9 @@
+// RUN: %clang -target arm64-apple-macosx -fptrauth-calls -c %s -### 2>&1 | FileCheck %s --check-prefix PTRAUTH_CALLS
+// RUN: %clang -target aarch64-linux-gnu -fptrauth-calls -c %s -### 2>&1 | FileCheck %s --check-prefix PTRAUTH_CALLS
+// RUN: %clang -target aarch64-windows-msvc -fptrauth-calls -c %s -### 2>&1 | FileCheck %s --check-prefix PTRAUTH_CALLS
+// RUN: not %clang -target x86_64-apple-macosx -fptrauth-calls -c %s -### 2>&1 | FileCheck %s --check-prefix INVALID
+// RUN: not %clang -target x86_64-linux-gnu -fptrauth-calls -c %s -### 2>&1 | FileCheck %s --check-prefix INVALID
+// RUN: not %clang -target x86_64-windows-msvc -fptrauth-calls -c %s -### 2>&1 | FileCheck %s --check-prefix INVALID
+
+// PTRAUTH_CALLS: "-fptrauth-calls"
+// INVALID: unsupported option '-fptrauth-calls'