Page MenuHomePhabricator

Add -finstruments-functions-exclude-{file, function}-list options
Needs ReviewPublic

Authored by gnzlbg on Nov 19 2013, 2:11 AM.
This revision needs review, but there are no reviewers specified.



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:

  • 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

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.

Diff Detail