[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

Roland McGrath via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 5 13:48:40 PDT 2021


mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm with minor changes + clang-format.



================
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:29
+  // now. Otherwise, ShadowBounds will be a zero-initialized global.
+  ShadowBounds = __sanitizer_shadow_bounds();
+  CHECK_NE(__sanitizer::ShadowBounds.shadow_limit, 0);
----------------
It feels sketchy to modify a variable defined in sanitizer_common directly like that.
Let's move this call into an `InitShadowBounds()` in sanitizer_common/sanitizer_fuchsia.h factored out of GetMaxUserVirtualAddress.



================
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:37
+  // This initializes __sanitizer::ShadowBounds.
+  kHighMemEnd = GetMaxUserVirtualAddress();
+  kHighMemBeg = __sanitizer::ShadowBounds.shadow_limit;
----------------
leonardchan wrote:
> mcgrathr wrote:
> > Isn't this `__sanitizer::GetMaxUserVirtualAddress()` ?
> It is. It looks there's
> 
> ```
> namespace __hwasan {
> using namespace __sanitizer;
> }
> ```
> 
> in `sanitizer_internal_defs.h` included through `hwasan.h`. Will add the `__sanitizer::` bits.
On further reflection, it's suboptimal that GetMaxUserVirtualAddress always resets the global.  It does that because in the asan implementation it's only called once at startup anyway so that was the minimal refactoring there.

Here since we're using ShadowBounds directly in this same code, I think using it directly here is more readable (as well as more optimal).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103936



More information about the cfe-commits mailing list