tmsriram updated this revision to Diff 251868.
tmsriram marked 11 inline comments as done.
tmsriram added a comment.

Address @rnk comments:

- Moved implementation to getMangledNameImpl to catch multi-versioned symbols 
and internal linkage globals
- Moved hash computation to consrtuctor
- Renamed option to -funique-internal-linkage-names
- Add documentation to UsersManual.rst and verified with html conversion
- Added tests for MV symbols and static globals, is there anything else? I am 
not too familiar with internal/private symbol possibilites


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

https://reviews.llvm.org/D73307

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/unique-internal-linkage-names.cpp
  clang/test/Driver/funique-internal-linkage-names.c

Index: clang/test/Driver/funique-internal-linkage-names.c
===================================================================
--- /dev/null
+++ clang/test/Driver/funique-internal-linkage-names.c
@@ -0,0 +1,4 @@
+// RUN: %clang -### -funique-internal-linkage-names %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
+// RUN: %clang -### -funique-internal-linkage-names -fno-unique-internal-linkage-names %s -c 2>&1 | FileCheck -check-prefix=CHECK-NOOPT %s
+// CHECK-OPT: "-funique-internal-linkage-names"
+// CHECK-NOOPT-NOT: "-funique-internal-linkage-names"
Index: clang/test/CodeGen/unique-internal-linkage-names.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGen/unique-internal-linkage-names.cpp
@@ -0,0 +1,46 @@
+// This test checks if internal linkage symbols get unique names with
+// -funique-internal-linkage-names option.
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int glob;
+static int foo() {
+  return 0;
+}
+
+int (*bar())() {
+  return foo;
+}
+
+int getGlob() {
+  return glob;
+}
+
+// Multiversioning symbols
+__attribute__((target("default"))) static int mver() {
+  return 0;
+}
+
+__attribute__((target("sse4.2"))) static int mver() {
+  return 1;
+}
+
+int mver_call() {
+  return mver();
+}
+
+// PLAIN: @_ZL4glob = internal global
+// PLAIN: @_ZL3foov()
+// PLAIN: @_ZL4mverv.resolver()
+// PLAIN: @_ZL4mverv()
+// PLAIN: @_ZL4mverv.sse4.2()
+// UNIQUE-NOT: @_ZL4glob = internal global
+// UNIQUE-NOT: @_ZL3foov()
+// UNIQUE-NOT: @_ZL4mverv.resolver()
+// UNIQUE-NOT: @_ZL4mverv()
+// UNIQUE-NOT: @_ZL4mverv.sse4.2()
+// UNIQUE: @_ZL4glob.{{[0-9a-f]+}} = internal global
+// UNIQUE: @_ZL3foov.{{[0-9a-f]+}}()
+// UNIQUE: @_ZL4mverv.{{[0-9a-f]+}}.resolver()
+// UNIQUE: @_ZL4mverv.{{[0-9a-f]+}}()
+// UNIQUE: @_ZL4mverv.{{[0-9a-f]+}}.sse4.2()
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -958,6 +958,8 @@
   Opts.StackSizeSection = Args.hasArg(OPT_fstack_size_section);
   Opts.UniqueSectionNames = !Args.hasArg(OPT_fno_unique_section_names);
 
+  Opts.UniqueInternalLinkageNames =
+      Args.hasArg(OPT_funique_internal_linkage_names);
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
Index: clang/docs/UsersManual.rst
===================================================================
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -1677,6 +1677,23 @@
    on ELF targets when using the integrated assembler. This flag currently
    only has an effect on ELF targets.
 
+**-f[no]-unique-internal-linkage-names**
+
+   Controls whether Clang emits a unique (best-effort) symbol name for internal
+   linkage symbols. The unique name is obtained by appending the MD5 hash of the
+   full module name to the original symbol. This option is particularly useful
+   in attributing profile information to the correct function when multiple
+   functions with the same private linkage name exist in the binary.
+
+   It should be noted that this option cannot guarantee uniqueness and the
+   following is an example where it is not unique:
+
+   .. code-block:: console
+
+     $ cd $P/foo && clang -c -funique-internal-linkage-names name_conflict.c
+     $ cd $P/bar && clang -c -funique-internal-linkage-names name_conflict.c
+     $ cd $P && clang foo/name_conflict.o && bar/name_conflict.o
+
 Profile Guided Optimization
 ---------------------------
 
Index: clang/lib/CodeGen/CodeGenModule.h
===================================================================
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -307,6 +307,7 @@
   const TargetInfo &Target;
   std::unique_ptr<CGCXXABI> ABI;
   llvm::LLVMContext &VMContext;
+  std::string ModuleNameHash = "";
 
   std::unique_ptr<CodeGenTBAA> TBAA;
 
@@ -578,6 +579,8 @@
   /// Return true iff an Objective-C runtime has been configured.
   bool hasObjCRuntime() { return !!ObjCRuntime; }
 
+  const std::string &getModuleNameHash() const { return ModuleNameHash; }
+
   /// Return a reference to the configured OpenCL runtime.
   CGOpenCLRuntime &getOpenCLRuntime() {
     assert(OpenCLRuntime != nullptr);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -176,6 +176,18 @@
   // CoverageMappingModuleGen object.
   if (CodeGenOpts.CoverageMapping)
     CoverageMapping.reset(new CoverageMappingModuleGen(*this, *CoverageInfo));
+
+  // Generate the module name hash here if needed.
+  if (CodeGenOpts.UniqueInternalLinkageNames &&
+      !getModule().getSourceFileName().empty()) {
+    llvm::MD5 Md5;
+    Md5.update(getModule().getSourceFileName());
+    llvm::MD5::MD5Result R;
+    Md5.final(R);
+    SmallString<32> Str;
+    llvm::MD5::stringifyResult(R, Str);
+    ModuleNameHash = ("." + Str).str();
+  }
 }
 
 CodeGenModule::~CodeGenModule() {}
@@ -1018,7 +1030,7 @@
   }
 }
 
-static std::string getMangledNameImpl(const CodeGenModule &CGM, GlobalDecl GD,
+static std::string getMangledNameImpl(CodeGenModule &CGM, GlobalDecl GD,
                                       const NamedDecl *ND,
                                       bool OmitMultiVersionMangling = false) {
   SmallString<256> Buffer;
@@ -1042,6 +1054,17 @@
     }
   }
 
+  // Check if the module name hash should be appended for internal linkage
+  // symbols.
+  const Decl *D = GD.getDecl();
+  if (CGM.getCodeGenOpts().UniqueInternalLinkageNames &&
+      !CGM.getModuleNameHash().empty() &&
+      ((isa<FunctionDecl>(D) &&
+        CGM.getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage) ||
+       (isa<VarDecl>(D) && CGM.getContext().GetGVALinkageForVariable(
+                               cast<VarDecl>(D)) == GVA_Internal)))
+    Out << CGM.getModuleNameHash();
+
   if (const auto *FD = dyn_cast<FunctionDecl>(ND))
     if (FD->isMultiVersion() && !OmitMultiVersionMangling) {
       switch (FD->getMultiVersionKind()) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4222,6 +4222,8 @@
         options::OPT_fno_function_sections,
         options::OPT_fdata_sections,
         options::OPT_fno_data_sections,
+        options::OPT_funique_internal_linkage_names,
+        options::OPT_fno_unique_internal_linkage_names,
         options::OPT_funique_section_names,
         options::OPT_fno_unique_section_names,
         options::OPT_mrestrict_it,
@@ -4819,6 +4821,13 @@
                     options::OPT_fno_unique_section_names, true))
     CmdArgs.push_back("-fno-unique-section-names");
 
+  if (Args.hasFlag(options::OPT_funique_internal_linkage_names,
+                   options::OPT_fno_unique_internal_linkage_names, false))
+    CmdArgs.push_back("-funique-internal-linkage-names");
+
+  if (Args.hasArg(options::OPT_funique_bb_section_names))
+    CmdArgs.push_back("-funique-bb-section-names");
+
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
                   options::OPT_finstrument_functions_after_inlining,
                   options::OPT_finstrument_function_entry_bare);
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1971,6 +1971,12 @@
 def fno_unique_section_names : Flag <["-"], "fno-unique-section-names">,
   Group<f_Group>, Flags<[CC1Option]>;
 
+def funique_internal_linkage_names : Flag <["-"], "funique-internal-linkage-names">,
+  Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Uniqueify Internal Linkage Symbol Names by appending the MD5 hash of the module path">;
+def fno_unique_internal_linkage_names : Flag <["-"], "fno-unique-internal-linkage-names">,
+  Group<f_Group>;
+
 def fstrict_return : Flag<["-"], "fstrict-return">, Group<f_Group>,
   Flags<[CC1Option]>,
   HelpText<"Always treat control flow paths that fall off the end of a "
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -162,6 +162,7 @@
 CODEGENOPT(NoTrappingMath    , 1, 0) ///< Set when -fno-trapping-math is enabled.
 CODEGENOPT(NoNaNsFPMath      , 1, 0) ///< Assume FP arguments, results not NaN.
 CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///< -cl-fp32-correctly-rounded-divide-sqrt
+CODEGENOPT(UniqueInternalLinkageNames, 1, 0) ///< Internal Linkage symbols get unique names.
 
 /// When false, this attempts to generate code as if the result of an
 /// overflowing conversion matches the overflowing behavior of a target's native
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to