[PATCH] D53789: [hwasan] optionally right-align heap allocations

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/hwasan/hwasan_allocator.cc:136
 
+  if (orig_size % kShadowAlignment) {
+    if (int malloc_align_right = flags()->malloc_align_right) {
----------------
kcc wrote:
> dvyukov wrote:
> > eugenis wrote:
> > > dvyukov wrote:
> > > > Shouldn't we also look at user-requested alignment here? It looks like we only satisfy natural alignment.
> > > Yeah, at least for calls like posix_memalign.
> > > 
> > > This mode will break some things anyway, ex.:
> > > struct S {
> > >   int count;
> > >   char name[0];
> > > };
> > > 
> > > When allocated with malloc(sizeof(S) + count) the size will not be a multiple of alignment, so "count" will be misaligned. That's why this mode can not be on by default. Let's hope it is not a common case.
> > > 
> > Why will not respecting alignment argument help? That's the alignment that we ensure in normal mode.
> I've changed the code so that it respects alignment > 16. 
> Respecting smaller alignment is harder because the implecit alignment requirement for malloc is 16
> (it's a bit more complicated than that, actually, I don't think the standard actually tells what the alignment is, 
> it just tells that it should be sufficient for any builtin type)
> 
> This mode is unlikely to be enabled by default in its current form. 
> For we can only tell what modifications will be needed once we try. 
Strictly saying this can be handled in standard-conforming way for C++.
We need to enable -faligned-allocation to get C++17 alignment-aware operator new:
https://en.cppreference.com/w/cpp/memory/new/operator_new
And then pass -fnew-alignment=1 which will force compiler to call operator new with explicit alignment always.
This results in:
```
        int *volatile p1 = new int;
  400614:       bf 04 00 00 00          mov    $0x4,%edi
  400619:       be 04 00 00 00          mov    $0x4,%esi
  40061e:       e8 ed fe ff ff          callq  400510 <operator new(unsigned long, std::align_val_t)@plt>
  400623:       48 89 44 24 08          mov    %rax,0x8(%rsp)
        short *volatile p2 = new short[2];
  400628:       bf 04 00 00 00          mov    $0x4,%edi
  40062d:       be 02 00 00 00          mov    $0x2,%esi
  400632:       e8 c9 fe ff ff          callq  400500 <operator new[](unsigned long, std::align_val_t)@plt>
  400637:       48 89 44 24 10          mov    %rax,0x10(%rsp)
```


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D53789