pcc added a comment.

In https://reviews.llvm.org/D46403#1291697, @filcab wrote:

> Sorry to ressurect this review, but I have a few questions:
>
> - What kind of functions fail?
> - Are there bugzillas to track these?
> - How can a compiler expect to have blacklists for "all" its CFI clients? 
> (I'm ok with having a default for "most-used, well-known, problematic 
> functions", e.g: functions in libstdc++ or libc++)


The default blacklist should contain problematic functions in the stdlibs that 
we don't control (i.e. libstdc++ and the MSVC stdlib). In libc++, we blacklist 
functions directly in the source code using the `no_sanitize` attribute. In 
general the blacklist entries for the stdlib are necessary because the standard 
requires that functions have a particular signature which requires the 
implementation to make what is considered to be a bad cast by CFI, so there 
isn't a bug as such.

> - clang/compiler-rt don't seem to have a default blacklist, what should the 
> contents be?

There is a default blacklist, see 
http://llvm-cs.pcc.me.uk/projects/compiler-rt/lib/cfi/cfi_blacklist.txt
Its contents are as mentioned above.

It should be installed by default, is that not happening for you?

> This patch seems to be saying "There are some well-known functions that 
> should never be instrumented with CFI, I'll provide a list of names", which 
> doesn't really seem possible to do in general, for "most" CFI-toolchain 
> clients. As I see it, each client will need to have their own blacklist, and 
> provide it as an argument if needed.

Right, if the client needs a blacklist it should be passed on the command line. 
More importantly, it should be applied in addition to the standard one so that 
the user doesn't have to repeat the stdlib blacklist in their blacklist (which 
was the situation before we added support for multiple blacklists).

> Which brings us to:
> 
> - If I pass `-fsanitize-blacklist=my_blacklist.txt` I still get an error. 
> This is not ideal, as I might be working on the blacklist to include in my 
> toolchain/program.
> 
>   I don't think we should have an error that is default enabled for this 
> issue. At most a warning (+ `-Werror`) if there was no blacklist passed in 
> nor found in the toolchain resource directory.

Clang prints this error if the blacklist is missing from the resource 
directory, which means that Clang was not installed properly. This is 
intentional as it prevents us from silently not applying the standard blacklist 
in that case.

If you are working on the toolchain blacklist, that is a very special case and 
there are plenty of ways around this issue. For example you can edit 
`lib/cfi/cfi_blacklist.txt` in place and use `ninja` to install it.


Repository:
  rC Clang

https://reviews.llvm.org/D46403



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D46403: [CFI] ... Filipe Cabecinhas via Phabricator via cfe-commits
    • [PATCH] D46403: [... Peter Collingbourne via Phabricator via cfe-commits

Reply via email to