[PATCH] Implement variable-sized alloca instrumentation.

[Kostya Serebryany]

================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:547
@@ +546,3 @@
+
+  void handleDynamicAllocaCall(AllocaInst *AI) {
+    IRBuilder<> IRBefore(AI);
----------------
m.ostepenko wrote:
> m.ostepenko wrote:
> > ygribov wrote:
> > > kcc wrote:
> > > > ygribov wrote:
> > > > > kcc wrote:
> > > > > > This will not create a left red zone, right? 
> > > > > > And even if it will (due to alignment) it will not poison it. 
> > > > > > 
> > > > > > I would prefer to create both left and right redzones and [un]poison them inline with one 4-byte store for the left rz and one or two 4-byte stores for the right one. 
> > > > > > Make sure to make the new size 0 mod 32
> > > > > > 
> > > > > > 
> > > > > I think Max's idea was to create thricely left, right and partial. As for inlining, it would be a mess for partial redzone - it's size is unknown until runtime so we won't be able to use 4-byte stores and will have to use an ugly loop instead.
> > > > A loop? Come on, I am sure you can construct the appropriate 32-bit constant to poison the partial 32-byte zon just using arithmetic (masks and shifts)
> > > I wonder if this can be less ugly? We don't want to emit this mess in codegen, do we?
> > > 
> > > Tail = OldSize & 5;  // Get length of 32-byte unaligned part
> > > if (Tail >= 24) {
> > >   sh = 24;
> > >   shadow_word = 0;
> > > } else if (Tail >= 16) {
> > >   sh = 16;
> > >   shadow_word = 0xcb000000;  // 0xcb is right magic
> > > } else if (Tail >= 8) {
> > >   sh = 8;
> > >   shadow_word = 0xcbcb0000;  // 0xcb is right magic
> > > } else {
> > >   sh = 0;
> > >   shadow_word = 0xcbcbcb00;  // 0xcb is right magic
> > > }
> > > Tail8 = Tail - sh;
> > > shadow_byte = Tail8 == 8 ? 0 : Tail8 ? Tail8 : 0xcb;
> > > shadow_word |= shadow_byte << sh;
> > > 
> > Hm, perhaps something like this would be preferable:
> > 
> > 
> > ```
> > padding = OldSize & (Align - 1) // get padding
> > if (padding) {
> >   shift = padding & ~7; // the number of bits we need to shift to access first chunk in shadow memory, containing nonzero bytes
> >   // Example:
> >   // padding = 21                       padding = 16
> >   // Shadow:  |00|00|05|cb|          Shadow:  |00|00|cb|cb|         
> >   //                ^                               ^
> >   //                |                               |
> >   // shift = 21 & ~7 = 16            shift = 16 & ~7 = 16
> >   val1 = 0xcbcbcbcb << (shift + 8);
> >   partialBits = padding & 7;
> >   if (!partialBits) partialBits = 0xcb;
> >   val2 = partialBits << shift;
> >   result = val1 | val2;
> > }
> > ```
> > 
> > if (!partialBits) partialBits = 0xcb; looks ugly, but right now I don't see any convenient way to avoid it.
> Oh, Align is 32, of course.
Yep, something along these lines. Cool. 

Please make sure to have a test for all 32 values of padding. 
You may use __asan_region_is_poisoned call in such test

Don't forget about little- vs big- endian 

>>   if (!partialBits) partialBits = 0xcb;
This should be fine actually, you can use SelectInst to avoid creating new BB

http://reviews.llvm.org/D6055