[PATCH] D90483: [GWP-ASan] Fuchsia specific mapping & utilities functions

Kostya Kortchinsky via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 30 21:25:50 PDT 2020


cryptoad added inline comments.


================
Comment at: compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_fuchsia.cpp:36
+                        0, Vmo, 0, Size, &Addr);
+  Check(Status == ZX_OK, "Vmo mapping failed");
+  _zx_handle_close(Vmo);
----------------
mcgrathr wrote:
> Is it possible for the failure message to include `Status` or `zx_status_get_string(Status)`?
I'd probably take that as an improvement for later as current check & die only take a string and there is no string formatting function in GWP-ASan.


================
Comment at: compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_fuchsia.h:9
+
+#if defined(__Fuchsia__)
+#ifndef GWP_ASAN_GUARDED_POOL_ALLOCATOR_FUCHSIA_H_
----------------
mcgrathr wrote:
> It seems weird to have any `#if` outside the header guard.  Obviously inverting the order makes no practical difference but I think it makes the code more readable to have it inside and separated by a blank line.
Right, that was initially the way I went, but in the previous review, Mitch suggested the other way.
I can go one way or the other if we have a consensus.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90483



More information about the llvm-commits mailing list