[PATCH] D74364: [GWP-ASan] Update alignment on Android.

Matt Morehouse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 12 11:10:33 PST 2020


morehouse added inline comments.


================
Comment at: compiler-rt/lib/gwp_asan/options.inc:19
     "the page boundary? By default (false), we round up allocation size to the "
-    "nearest power of two (1, 2, 4, 8, 16) up to a maximum of 16-byte "
-    "alignment for performance reasons. Setting this to true can find single "
-    "byte buffer-overflows for multibyte allocations at the cost of "
-    "performance, and may be incompatible with some architectures.")
+    "nearest power of two (1, 2, 4, 8) up to a maximum of 8-byte alignment for "
+    "performance reasons. On Android, we use 8-byte alignment by "
----------------
morehouse wrote:
> I'm not positive if its safe to go down to 8 bytes here.  Until [recently](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2293.htm), there was an ambiguity in the standard that meant most mallocs used 16-byte alignment.  It's gone now, but there may still be Android use cases that expect `malloc(16+)` to have the higher alignment (pretty sure this exists in Google3).
> 
> In TCMalloc, all `operator new`s get 8-byte alignment by default while `malloc` and friends follow the `memalign(16, size)` path.  But  I don't know how Bionic does this...
Regardless, I think we'll need 16-byte alignment for malloc() with size>=16.  Unless `malloc()` doesn't follow the path with GWP-ASan sampling.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74364/new/

https://reviews.llvm.org/D74364





More information about the llvm-commits mailing list