necipfazil updated this revision to Diff 358505.
necipfazil added a comment.

Several review comments are addressed

- Don't emit type metadata for Objective-C
- Don't emit metadata for function definitions, which leads to duplicates
- Improve the test case
  - Add C struct parameter test
  - Fix variable capturing
  - Make function and callsite metadata checking distinct
  - Some formatting

TODO: a C++ test will be added with class functions, templates, etc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105909/new/

https://reviews.llvm.org/D105909

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/call-graph-section.c

Index: clang/test/CodeGen/call-graph-section.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/call-graph-section.c
@@ -0,0 +1,75 @@
+// Tests that we assign appropriate identifiers to indirect calls and targets.
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,ITANIUM %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,MS %s
+
+// CHECK-DAG: define {{(dso_local)?}} void @foo({{.*}} !type [[F_TVOID:![0-9]+]]
+void foo() {
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @bar({{.*}} !type [[F_TVOID]]
+void bar() {
+  void (*fp)() = foo;
+  // CHECK: call {{.*}} !type [[CS_TVOID:![0-9]+]]
+  fp();
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32 @baz({{.*}} !type [[F_TPRIMITIVE:![0-9]+]]
+int baz(char a, float b, double c) {
+  return 1;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32* @qux({{.*}} !type [[F_TPTR:![0-9]+]]
+int *qux(char *a, float *b, double *c) {
+  return 0;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @corge({{.*}} !type [[F_TVOID]]
+void corge() {
+  int (*fp_baz)(char, float, double) = baz;
+  // CHECK: call i32 {{.*}}, !type [[CS_TPRIMITIVE:![0-9]+]]
+  fp_baz('a', .0f, .0);
+
+  int *(*fp_qux)(char *, float *, double *) = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR:![0-9]+]]
+  fp_qux(0, 0, 0);
+}
+
+struct st1 {
+  int *(*fp)(char *, float *, double *);
+};
+
+struct st2 {
+  struct st1 m;
+};
+
+// CHECK-DAG: define {{(dso_local)?}} void @stf({{.*}} !type [[F_TVOID]]
+void stf() {
+  struct st1 St1;
+  St1.fp = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR]]
+  St1.fp(0, 0, 0);
+
+  struct st2 St2;
+  St2.m.fp = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR]]
+  St2.m.fp(0, 0, 0);
+}
+
+// ITANIUM-DAG: [[F_TVOID]] = !{i64 0, !"_ZTSFvE.generalized"}
+// ITANIUM-DAG: [[CS_TVOID]] = !{!"_ZTSFvE.generalized"}
+// MS-DAG: [[F_TVOID]] = !{i64 0, !"?6AX@Z.generalized"}
+// MS-DAG: [[CS_TVOID]] = !{!"?6AX@Z.generalized"}
+
+// ITANIUM-DAG: [[F_TPRIMITIVE]] = !{i64 0, !"_ZTSFicfdE.generalized"}
+// ITANIUM-DAG: [[CS_TPRIMITIVE]] = !{!"_ZTSFicfdE.generalized"}
+// MS-DAG: [[F_TPRIMITIVE]] = !{i64 0, !"?6AHDMN@Z.generalized"}
+// MS-DAG: [[CS_TPRIMITIVE]] = !{!"?6AHDMN@Z.generalized"}
+
+// ITANIUM-DAG: [[F_TPTR]] = !{i64 0, !"_ZTSFPvS_S_S_E.generalized"}
+// ITANIUM-DAG: [[CS_TPTR]] = !{!"_ZTSFPvS_S_S_E.generalized"}
+// MS-DAG: [[F_TPTR]] = !{i64 0, !"?6APEAXPEAX00@Z.generalized"}
+// MS-DAG: [[CS_TPTR]] = !{!"?6APEAXPEAX00@Z.generalized"}
Index: clang/lib/CodeGen/CodeGenModule.h
===================================================================
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1406,6 +1406,10 @@
   void CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
                                           llvm::Function *F);
 
+  /// Create and attach type metadata to the given call.
+  void CreateFunctionTypeMetadataForIcall(const QualType &QT,
+                                          llvm::CallBase *CB);
+
   /// Whether this function's return type has no side effects, and thus may
   /// be trivially discarded if it is unused.
   bool MayDropFunctionReturn(const ASTContext &Context, QualType ReturnType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2057,7 +2057,17 @@
 
 void CodeGenModule::CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
                                                        llvm::Function *F) {
-  // Only if we are checking indirect calls.
+  bool EmittedMDIdGeneralized = false;
+  if (CodeGenOpts.CallGraphSection &&
+      (!F->hasLocalLinkage() ||
+       F->getFunction().hasAddressTaken(nullptr, /* IgnoreCallbackUses */ true,
+                                        /* IgnoreAssumeLikeCalls */ true,
+                                        /* IgnoreLLVMUsed */ false))) {
+    F->addTypeMetadata(0, CreateMetadataIdentifierGeneralized(FD->getType()));
+    EmittedMDIdGeneralized = true;
+  }
+
+  // Add additional metadata only if we are checking indirect calls with CFI.
   if (!LangOpts.Sanitize.has(SanitizerKind::CFIICall))
     return;
 
@@ -2068,7 +2078,9 @@
 
   llvm::Metadata *MD = CreateMetadataIdentifierForType(FD->getType());
   F->addTypeMetadata(0, MD);
-  F->addTypeMetadata(0, CreateMetadataIdentifierGeneralized(FD->getType()));
+  // Add the generalized identifier if not added already.
+  if (!EmittedMDIdGeneralized)
+    F->addTypeMetadata(0, CreateMetadataIdentifierGeneralized(FD->getType()));
 
   // Emit a hash-based bit set entry for cross-DSO calls.
   if (CodeGenOpts.SanitizeCfiCrossDso)
@@ -2076,6 +2088,17 @@
       F->addTypeMetadata(0, llvm::ConstantAsMetadata::get(CrossDsoTypeId));
 }
 
+void CodeGenModule::CreateFunctionTypeMetadataForIcall(const QualType &QT,
+                                                       llvm::CallBase *CB) {
+  // Only if needed for call graph section and only for indirect calls.
+  if (!(CodeGenOpts.CallGraphSection && CB && CB->isIndirectCall()))
+    return;
+
+  auto *MD = CreateMetadataIdentifierGeneralized(QT);
+  auto *MDN = llvm::MDNode::get(getLLVMContext(), MD);
+  CB->setMetadata(llvm::LLVMContext::MD_type, MDN);
+}
+
 void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
                                           bool IsIncompleteFunction,
                                           bool IsThunk) {
@@ -2150,7 +2173,8 @@
   // are non-canonical then we need type metadata in order to produce the local
   // jump table.
   if (!CodeGenOpts.SanitizeCfiCrossDso ||
-      !CodeGenOpts.SanitizeCfiCanonicalJumpTables)
+      !CodeGenOpts.SanitizeCfiCanonicalJumpTables ||
+      CodeGenOpts.CallGraphSection)
     CreateFunctionTypeMetadataForIcall(FD, F);
 
   if (getLangOpts().OpenMP && FD->hasAttr<OMPDeclareSimdDeclAttr>())
Index: clang/lib/CodeGen/CGExprCXX.cpp
===================================================================
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -86,9 +86,17 @@
       *this, MD, This, ImplicitParam, ImplicitParamTy, CE, Args, RtlArgs);
   auto &FnInfo = CGM.getTypes().arrangeCXXMethodCall(
       Args, FPT, CallInfo.ReqArgs, CallInfo.PrefixSize);
-  return EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr,
-                  CE && CE == MustTailCall,
-                  CE ? CE->getExprLoc() : SourceLocation());
+  llvm::CallBase *CallOrInvoke = nullptr;
+  auto Call = EmitCall(FnInfo, Callee, ReturnValue, Args, &CallOrInvoke,
+                       CE && CE == MustTailCall,
+                       CE ? CE->getExprLoc() : SourceLocation());
+
+  // Set type identifier metadata of indirect calls for call graph section.
+  if (CGM.getCodeGenOpts().CallGraphSection && CallOrInvoke &&
+      CallOrInvoke->isIndirectCall())
+    CGM.CreateFunctionTypeMetadataForIcall(MD->getType(), CallOrInvoke);
+
+  return Call;
 }
 
 RValue CodeGenFunction::EmitCXXDestructorCall(
@@ -112,9 +120,18 @@
   CallArgList Args;
   commonEmitCXXMemberOrOperatorCall(*this, DtorDecl, This, ImplicitParam,
                                     ImplicitParamTy, CE, Args, nullptr);
-  return EmitCall(CGM.getTypes().arrangeCXXStructorDeclaration(Dtor), Callee,
-                  ReturnValueSlot(), Args, nullptr, CE && CE == MustTailCall,
-                  CE ? CE->getExprLoc() : SourceLocation{});
+  llvm::CallBase *CallOrInvoke = nullptr;
+  auto Call =
+      EmitCall(CGM.getTypes().arrangeCXXStructorDeclaration(Dtor), Callee,
+               ReturnValueSlot(), Args, &CallOrInvoke, CE && CE == MustTailCall,
+               CE ? CE->getExprLoc() : SourceLocation{});
+
+  // Set type identifier metadata of indirect calls for call graph section.
+  if (CGM.getCodeGenOpts().CallGraphSection && CallOrInvoke &&
+      CallOrInvoke->isIndirectCall())
+    CGM.CreateFunctionTypeMetadataForIcall(DtorDecl->getType(), CallOrInvoke);
+
+  return Call;
 }
 
 RValue CodeGenFunction::EmitCXXPseudoDestructorExpr(
@@ -471,10 +488,18 @@
 
   // And the rest of the call args
   EmitCallArgs(Args, FPT, E->arguments());
-  return EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required,
-                                                      /*PrefixSize=*/0),
-                  Callee, ReturnValue, Args, nullptr, E == MustTailCall,
-                  E->getExprLoc());
+  llvm::CallBase *CallOrInvoke = nullptr;
+  auto Call = EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required,
+                                                           /*PrefixSize=*/0),
+                       Callee, ReturnValue, Args, &CallOrInvoke,
+                       E == MustTailCall, E->getExprLoc());
+
+  // Set type identifier metadata of indirect calls for call graph section.
+  if (CGM.getCodeGenOpts().CallGraphSection && CallOrInvoke &&
+      CallOrInvoke->isIndirectCall())
+    CGM.CreateFunctionTypeMetadataForIcall(QualType(FPT, 0), CallOrInvoke);
+
+  return Call;
 }
 
 RValue
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5311,6 +5311,11 @@
                                   CalleeDecl);
   }
 
+  // Set type identifier metadata of indirect calls for call graph section.
+  if (CGM.getCodeGenOpts().CallGraphSection && CallOrInvoke &&
+      CallOrInvoke->isIndirectCall())
+    CGM.CreateFunctionTypeMetadataForIcall(QualType(FnType, 0), CallOrInvoke);
+
   return Call;
 }
 
Index: clang/lib/CodeGen/CGClass.cpp
===================================================================
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -2182,7 +2182,13 @@
   const CGFunctionInfo &Info = CGM.getTypes().arrangeCXXConstructorCall(
       Args, D, Type, ExtraArgs.Prefix, ExtraArgs.Suffix, PassPrototypeArgs);
   CGCallee Callee = CGCallee::forDirect(CalleePtr, GlobalDecl(D, Type));
-  EmitCall(Info, Callee, ReturnValueSlot(), Args, nullptr, false, Loc);
+  llvm::CallBase *CallOrInvoke = nullptr;
+  EmitCall(Info, Callee, ReturnValueSlot(), Args, &CallOrInvoke, false, Loc);
+
+  // Set type identifier metadata of indirect calls for call graph section.
+  if (CGM.getCodeGenOpts().CallGraphSection && CallOrInvoke &&
+      CallOrInvoke->isIndirectCall())
+    CGM.CreateFunctionTypeMetadataForIcall(D->getType(), CallOrInvoke);
 
   // Generate vtable assumptions if we're constructing a complete object
   // with a vtable.  We don't do this for base subobjects for two reasons:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to