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

[Matthew Iselin]

Hi,

I have attached a new patch that only fixes the inline issue.

On 14/03/2013, at 9:06 AM, Douglas Gregor <dgregor at apple.com> wrote:

> ...
> 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.

Sure. This seems a lot more sensible. Where would be the best place to cache the lookup (and might it also be worth caching the 'exclude state' of specific functions to reduce checks?) so that it can be used?

> 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.

This also makes sense.

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

Missed isExternC in the docs (wasn't looking for the right thing) - definitely saves a lot of trouble in that code path.

> I didn't see anything to handle escaped commas in the input, which GCC's documentation mentions.

I need to add a test case for this particular case - wasn't certain if a CommaJoined option handled this automatically.

On 14/03/2013, at 9:30 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> ...
> Our correctness did not depend on host's cxa_demangle before…
> ...

Correctness according to the GCC definition of the -finstrument-functions-exclude-function-list option would be to always operate on a demangled name where the name was mangled in the first place. I'm not fully sure how to go about doing this as there doesn't seem to be anything available to demangle a name, other than __cxa_demangle. It could be possible to use the mangled name for comparisons, and document the difference in behaviour? It seems that in GCC, according to the documentation, it should be possible to exclude a function with the substring "blah(const vec".

I will look at these issues with the patch and work on resolving them.

Cheers,

- Matthew


-------------- next part --------------
A non-text attachment was scrubbed...
Name: finstrument-functions-inline-fix.patch
Type: application/octet-stream
Size: 844 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130314/9e3357a6/attachment.obj>
-------------- next part --------------



On 14/03/2013, at 9:30 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> On Tue, Mar 12, 2013 at 6:31 AM, Matthew Iselin <matthew at theiselins.net> wrote:
>> This patch adds two options, -finstrument-functions-exclude-file-list= and -finstrument-functions-exclude-function=list=, which offer a means through which headers in various locations and individual functions can be excluded from instrumentation via -finstrument-functions. These options can be found in GCC alongside -finstrument-functions.
> 
> +  char *result = __cxa_demangle(Fn->getName().str().c_str(), 0, 0, &status);
> 
> 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).
> 
> Dmitri
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/