[PATCH] Add -finstruments-functions-exclude-{file, function}-list options

Gonzalo BG gonzalobg88 at gmail.com
Tue Nov 19 02:24:43 PST 2013


gnzlbg added you to the CC list for the revision "Add -finstruments-functions-exclude-{file, function}-list options".

This options allow to exclude functions from instrumentation either by
providing a list of user-visible function names or a list of file paths. These
allows to exclude e.g. the standard library from instrumentation and thus to use
standard library functions in the instrumentation call-backs.

This is a patch by Matthew Iselin, original thread is here:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130311/075910.html

- Bugfix: "Inline functions that are not externally visible mustn't
be instrumented. They create a named reference to the inlined function, as the
first parameter to __cyg_profile_* functions, which a linker will never be able
to resolve." (see patch from Matthew Iselin here 
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130311/075917.html)

Some issues were raised and the patch died: 

"std::set is pretty heavyweight, and isn't buying us anything. I think just
having one large string to search would be best. Comma probably isn't the best
separator character; embedded NULLs or something non-ASCII would be better."

I replaced std::set<std::string> with a std::vector<std::string>. Rationale: 1)
we get a std::vector<string> as input anyways, 2) using a string with a
delimiter is more complicated, 3) in particular NULLs, because some string
algorithms like find stop at the first NULL. 

"if Fn->isExternC(), there's no point in demangling. Let's save ourselves the work
in that case."

How can I call isExternC()? Fn is of type llvm::Function but the member function
is in clang::FunctionDecl::isExternC().

"Our correctness did not depend on host's cxa_demangle before... There's also a
fixme in compiler_rt about shipping our own cxa_demangle in order not to leak
memory (it is used inside sanitizers, so it has special requirements about
allocating memory)."

What should I use instead?

"I suggest that we look through macro expansions to find the FileID where the
expansion occurred, then check that. Also, can we cache the result of this
lookup based on the FileID? There are a lot of string searches we're potentially
doing here, many of which will be redundant."

I've added a std::map for caching the result of the lookup. An unordered_map
would be a better fit.

http://llvm-reviews.chandlerc.com/D2219

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp

Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -535,6 +535,12 @@
 def findirect_virtual_calls : Flag<["-"], "findirect-virtual-calls">, Alias<fapple_kext>;
 def finline_functions : Flag<["-"], "finline-functions">, Group<clang_ignored_f_Group>;
 def finline : Flag<["-"], "finline">, Group<clang_ignored_f_Group>;
+def finstrument_functions_exclude_function_list : CommaJoined<["-"],
+  "finstrument-functions-exclude-function-list=">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Exclude given (mangled, if C++) functions from function instrumentation.">;
+def finstrument_functions_exclude_file_list : CommaJoined<["-"],
+  "finstrument-functions-exclude-file-list=">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Exclude given path segments from function instrumentation.">;
 def finstrument_functions : Flag<["-"], "finstrument-functions">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Generate calls to instrument function entry and exit">;
 def fkeep_inline_functions : Flag<["-"], "fkeep-inline-functions">, Group<clang_ignored_f_Group>;
Index: include/clang/Frontend/CodeGenOptions.h
===================================================================
--- include/clang/Frontend/CodeGenOptions.h
+++ include/clang/Frontend/CodeGenOptions.h
@@ -131,6 +131,12 @@
   /// A list of dependent libraries.
   std::vector<std::string> DependentLibraries;
 
+  /// Functions to exclude from function instrumentation.
+  std::vector<std::string> InstrumentFunctionExclusionsFunctions;
+
+  /// Path segments to exclude from function instrumentation, eg, '/bits'
+  std::vector<std::string> InstrumentFunctionExclusionsPathSegments;
+
 public:
   // Define accessors/mutators for code generation options of enumeration type.
 #define CODEGENOPT(Name, Bits, Default)
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -29,6 +29,7 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Operator.h"
+#include <map>
 using namespace clang;
 using namespace CodeGen;
 
@@ -232,7 +233,7 @@
   // Emit function epilog (to return).
   EmitReturnBlock();
 
-  if (ShouldInstrumentFunction())
+  if (ShouldInstrumentFunction(CurFn))
     EmitFunctionInstrumentation("__cyg_profile_func_exit");
 
   // Emit debug descriptor for function end.
@@ -277,13 +278,77 @@
     EmitDeclMetadata();
 }
 
+#if !defined(_MSC_VER)
+// Assume that __cxa_demangle is provided by libcxxabi (except for Windows).
+extern "C" char *__cxa_demangle(const char *mangled_name, char *output_buffer,
+                                size_t *length, int *status);
+#endif
+
 /// ShouldInstrumentFunction - Return true if the current function should be
 /// instrumented with __cyg_profile_func_* calls
-bool CodeGenFunction::ShouldInstrumentFunction() {
+bool CodeGenFunction::ShouldInstrumentFunction(llvm::Function *Fn) {
+  typedef std::vector<std::string>::const_iterator CIt;
   if (!CGM.getCodeGenOpts().InstrumentFunctions)
     return false;
   if (!CurFuncDecl || CurFuncDecl->hasAttr<NoInstrumentFunctionAttr>())
     return false;
+
+  // Inline functions that are not externally visible mustn't be instrumented.
+  // They create a named reference to the inlined function, as the first
+  // parameter to __cyg_profile_* functions, which a linker will never be able
+  // to resolve.
+  if (const FunctionDecl *ActualFuncDecl =
+        dyn_cast<FunctionDecl>(CurFuncDecl)) {
+    if (ActualFuncDecl->isInlined() &&
+        !ActualFuncDecl->isInlineDefinitionExternallyVisible())
+      return false;
+  }
+
+  SourceLocation SLoc = CurFuncDecl->getLocation();
+  static std::map<SourceLocation, std::string> cache;
+
+  if (SLoc.isFileID()) {
+    if (cache.find(SLoc) == cache.end()) {
+      ASTContext &ctx = CurFuncDecl->getASTContext();
+      const SourceManager &SM = ctx.getSourceManager();
+
+      PresumedLoc PLoc = SM.getPresumedLoc(SLoc);
+      cache[SLoc] = PLoc.getFilename();
+    }
+    std::string FunctionDeclPath = cache[SLoc];
+
+    const std::vector<std::string> &PathSearch =
+      CGM.getCodeGenOpts().InstrumentFunctionExclusionsPathSegments;
+
+    for (CIt i = PathSearch.begin(), e = PathSearch.end(); i != e; ++i) {
+      if(FunctionDeclPath.find(*i) != std::string::npos) {
+        return false;
+      }
+    }
+  }
+
+  // Demangle the name if it's mangled
+  std::string FunctionName = Fn->getName();
+#if !defined(_MSC_VER)
+  int status = 0;
+  char *result = __cxa_demangle(FunctionName.c_str(), 0, 0, &status);
+
+  assert((status == 0 || status == -2) && "Couldn't demangle name.");
+
+  if (status == 0) {
+    FunctionName = result;
+    free(result);
+  }
+#endif
+
+  const std::vector<std::string> &FunctionSearch =
+    CGM.getCodeGenOpts().InstrumentFunctionExclusionsFunctions;
+  for (CIt i = FunctionSearch.begin(), e = FunctionSearch.end(); i != e; ++i) {
+    if(FunctionName.find(*i) != std::string::npos) {
+      return false;
+    }
+  }
+
   return true;
 }
 
@@ -567,7 +632,7 @@
     DI->EmitFunctionStart(GD, FnType, CurFn, Builder);
   }
 
-  if (ShouldInstrumentFunction())
+  if (ShouldInstrumentFunction(CurFn))
     EmitFunctionInstrumentation("__cyg_profile_func_enter");
 
   if (CGM.getCodeGenOpts().InstrumentForProfiling)
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1202,7 +1202,7 @@
 
   /// ShouldInstrumentFunction - Return true if the current function should be
   /// instrumented with __cyg_profile_func_* calls
-  bool ShouldInstrumentFunction();
+  bool ShouldInstrumentFunction(llvm::Function *Fn);
 
   /// EmitFunctionInstrumentation - Emit LLVM code to call the specified
   /// instrumentation function with the current function and the call site, if
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2620,6 +2620,12 @@
   Args.AddAllArgs(CmdArgs, options::OPT_fdata_sections);
 
   Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
+  if (Args.hasArg(options::OPT_finstrument_functions)) {
+    Args.AddAllArgs(CmdArgs,
+                    options::OPT_finstrument_functions_exclude_file_list);
+    Args.AddAllArgs(CmdArgs,
+                    options::OPT_finstrument_functions_exclude_function_list);
+  }
 
   if (Args.hasArg(options::OPT_ftest_coverage) ||
       Args.hasArg(options::OPT_coverage))
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -432,6 +432,13 @@
   }
 
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
+  if (Opts.InstrumentFunctions) {
+    Opts.InstrumentFunctionExclusionsFunctions
+        = Args.getAllArgValues(OPT_finstrument_functions_exclude_function_list);
+    Opts.InstrumentFunctionExclusionsPathSegments
+        = Args.getAllArgValues(OPT_finstrument_functions_exclude_file_list);
+  }
+
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
   Opts.DebugCompilationDir = Args.getLastArgValue(OPT_fdebug_compilation_dir);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2219.1.patch
Type: text/x-patch
Size: 7517 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131119/66158c8b/attachment.bin>


More information about the cfe-commits mailing list