[llvm-commits] [PATCH] Extend sanitizer blacklist functionality

[Reid Watson]

Thanks!  The reason for expanding the blacklist is somewhat unfortunate:

Deploying initialization order checking on deployed code has found a
surprising number of examples of undefined behavior.  Worse, fixing
them brings up some unfortunate coding politics problems.

The C++11 standard states that access to "any non-static member or
base class of the object before the constructor begins results in
undefined behavior" [12.7.1].  The initialization order checking tool
recently patched in catches examples of this resulting from the
unspecified order of initialization of variables in different TUs.

However, some existing code exploits knowledge of the compiler/linker
to avoid initialization order problems using code that contains
undefined behavior.  For example, some classes have an explicit, one
parameter constructor which is implicitly required not to change the
object's memory.  This, together with zero initialization offers a
means to "solve" the initialization order problem on objects that can
be used if they are zero initialized.

This would be a perfect use case for constexpr, but that's still
relatively new, so expanding the blacklist is the simplest solution
for now.  I completely agree that it would be better to just get rid
of the undefined behavior.  Currently though, expanding the blacklist
lets me:
* Skip known UB without blacklisting a function which may actually be
causing other problems when called on a different global.
* Collect a list of all variables which have this issue in currently
deployed code.

All the best,
--Reid

On Thu, Aug 23, 2012 at 3:41 AM, Kostya Serebryany <kcc at google.com> wrote:
> Looks good in general, I'll commit later unless someone objects.
>
> Just curious, what is your use case?
> We sometimes use the blacklists for asan/msan/tsan during initial deployment
> in a particular source base,
> but long term we always try to fix the code (and mark the tricky code with
> attributes) and get rid of the blacklist.
>
> Thanks!
>
> --kcc
>
>
> On Wed, Aug 22, 2012 at 11:00 PM, Reid Watson <reidw at google.com> wrote:
>>
>> Also, for ease of review, here's a version with no renaming of files
>> involved.  It preserves the FunctionBlackList name, though that should
>> probably be changed if this is committed.
>>
>> On Wed, Aug 22, 2012 at 11:08 AM, Reid Watson <reidw at google.com> wrote:
>> > Hi all,
>> >
>> > First off, sincere thanks to reviewers(kcc especially) for helping
>> > with the initial initialization order patch.
>> >
>> > I'm now looking to expand the blacklisting functionality of
>> > AddressSanitizer to include the ability to blacklist global variables.
>> >  This is currently handled in FunctionBlackList.h, which is renamed to
>> > BlackList.h to reflect the fact that it can store more than functions.
>> >
>> > I've attached the patch and uploaded it to
>> > Rietveld(http://codereview.appspot.com/6473048/).  Review is
>> > appreciated as always.
>> >
>> > All the best,
>> > Reid
>
>