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

[leonardchan]

Summary

This contains all the definitions required by hwasan for the fuchsia implementation and can be landed independently from the remaining parts of D91466.

[leonardchan]

Let me know if these functions should be split up or can land together.

[mcgrathr]

compiler-rt/lib/hwasan/hwasan_fuchsia.cpp

30

These need comments about what they are and why they need to exist as runtime variables at all.

35

What actually refers to this?
The optimal implementation for Fuchsia would just know everywhere at compile time that it's a fized value.
If there's reason for the runtime variable to exist at all, it should have comments.

38

Isn't this __sanitizer::GetMaxUserVirtualAddress() ?

60

This comment isn't very meaningful, since it only really applies to the two functions after these three.
This is also a weird comment syntax not used elsewhere in this file.

63

As we discussed before, this is insufficient plumbing for the thread tracking.
It probably makes sense to either do all the necessary refactoring for the thread tracking plumbing first, or else start this file simpler without anything related to thread tracking, and then add more stuff later.

Also, as a matter of style it's best to define C++ functions in the __hwasan namespace or a local anonymous namespace, and then have an extern "C" block at the end where the libc hook API implementation functions are just simple wrapper calls with no more than a line or two in them.

See asan_fuchsia.cpp for a good example of how to arrange the hook functions.

75

This is not the name of the hook. __sanitizer_process_exit_hook is the name of the hook. This is a good example of a simple wrapper.

However, it's unclear whether this should use the exit hook or not. We don't use that hook in asan, we just use its normal atexit method. In hwasan, the atexit hook doesn't really do much. ReportStats is a no-op in hwasan, and DumpProcessMap is always a no-op on Fuchsia anyway. So all it's really doing in practice is changing the exit code, which is what's a perfect fit for the sanitizer hook.

90

Why isn't this just in hwasan.cpp?

96

I'd put blank lines between these.
Most or all of them merit an individual comment about how Fuchsia handles things differently and thus doesn't need that particular thing.

102

I'd put this comment inside the function. Seems like a good idea to rename this function to something not so Linuxy, e.g. InitializeOsSupport.

[leonardchan]

compiler-rt/lib/hwasan/hwasan_fuchsia.cpp

30

kHighMemEnd and kHighMemBeg are used only by MemIsApp which is only used in hwasan_thread.cpp for some debugging checks:

if (stack_bottom_) {
  int local;
  CHECK(AddrIsInStack((uptr)&local));
  CHECK(MemIsApp(stack_bottom_));
  CHECK(MemIsApp(stack_top_ - 1));
}

Rather than having these, we could just use their values (__sanitizer::ShadowBounds.memory_limit - 1 and __sanitizer::ShadowBounds.shadow_limit) directly in MemIsApp to avoid these globals.

kAliasRegionStart is used in HwasanAllocatorInit for setting up the allocator, but is only relevant for an experimental x86 implementation that uses page aliasing for placing tags in heap allocations (see D98875). I didn't look too much into the machinery for this since I didn't think we would be using hwasan for x86 anytime soon, but we can explore this now if we want to plan ahead. We could also make it such that this is just defined as 0 on x86 platforms, similar to SHADOW_OFFSET in asan.

35

It's only used for converting between application memory and shadow memory:

// hwasan_mapping.h
inline uptr MemToShadow(uptr untagged_addr) {
  return (untagged_addr >> kShadowScale) +
         __hwasan_shadow_memory_dynamic_address;
}
inline uptr ShadowToMem(uptr shadow_addr) {
  return (shadow_addr - __hwasan_shadow_memory_dynamic_address) << kShadowScale;
}

Perhaps we could have something similar to the SHADOW_OFFSET macro in asan where it can be defined to either a constant or __hwasan_shadow_memory_dynamic_address on different platforms and these functions can just use the macro.

38

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.

60

Removed.

63

Moved these in D104085 and left the remaining functions here.

75

Ah, if the functions in HwasanAtExit are no-ops and it only sets the exit code to 1 with internal__exit on an error, then it might not be necessary to have this here.

90

Removed from this and will move to hwasan.cpp in a separate patch. I imagine other platforms would've had their own versions of this we didn't want to centralize, but it looks like they all share the linux one.

102

Removed and will rename in a separate patch.

[mcgrathr]

compiler-rt/lib/hwasan/hwasan_fuchsia.cpp

30

MemIsApp is defined in this file so don't define any globals on its account (if it needs anything, make it static in this file).

HWASAN_ALIASING_MODE is not supported for Fuchsia and we don't need to make sure it compiles. Just leave out anything related to it entirely for now.

35

That sounds good. But we can make that a separate refactoring cleanup of its own. It's fine to just define it to 0 here and leave a TODO comment about removing the runtime variable altogether on Fuchsia.

That's a refactoring you could either land separately on its own first before landing the Fuchsia port, or do afterwards as a separate cleanup change.

[leonardchan]

Rebase and remove __hwasan_shadow_memory_dynamic_address. We don't need to set it anymore.

[leonardchan]

*ping* Anything else that should be updated?

[mcgrathr]

lgtm with minor changes + clang-format.

compiler-rt/lib/hwasan/hwasan_fuchsia.cpp

30

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.

38

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).

compiler-rt/lib/hwasan/CMakeLists.txt

 include_directories(..)
 
 # Runtime library sources and build flags.
 set(HWASAN_RTL_SOURCES
   hwasan.cpp
   hwasan_allocator.cpp
   hwasan_allocation_functions.cpp
   hwasan_dynamic_shadow.cpp
   hwasan_exceptions.cpp
+  hwasan_fuchsia.cpp
   hwasan_globals.cpp
   hwasan_interceptors.cpp
   hwasan_interceptors_vfork.S
   hwasan_linux.cpp
   hwasan_memintrinsics.cpp
   hwasan_poisoning.cpp
   hwasan_report.cpp
   hwasan_setjmp.S

compiler-rt/lib/hwasan/hwasan_fuchsia.cpp

+//===-- hwasan_fuchsia.cpp --------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file is a part of HWAddressSanitizer and contains Fuchsia-specific
+/// code.
+///
+//===----------------------------------------------------------------------===//
+
+#include "sanitizer_common/sanitizer_fuchsia.h"
+#if SANITIZER_FUCHSIA
+
+#include "hwasan.h"
+#include "hwasan_interface_internal.h"
+#include "hwasan_report.h"
+#include "hwasan_thread.h"
+#include "hwasan_thread_list.h"
+
+namespace __hwasan {
+
+// This is used by the hwasan allocator when page aliasing on x86 is enabled,
+// but should always remain zero on non-x86 implementations.
+uptr kAliasRegionStart = 0;
+
+bool InitShadow() {
+  __hwasan_shadow_memory_dynamic_address = 0;
+
+  // Call this explicitly to set the ShadowBounds global so we can reference it
+  // now. Otherwise, ShadowBounds will be a zero-initialized global.
+  ShadowBounds = __sanitizer_shadow_bounds();
+  CHECK_NE(__sanitizer::ShadowBounds.shadow_limit, 0);
+
+  return true;
+}
+
+bool MemIsApp(uptr p) {
+  CHECK(GetTagFromPointer(p) == 0);
+  return __sanitizer::ShadowBounds.shadow_limit <= p &&
+         p <= __sanitizer::GetMaxUserVirtualAddress();
+}
+
+// Not implemented because Fuchsia does not use signal handlers.
+void HwasanOnDeadlySignal(int signo, void *info, void *context) {}
+
+// Not implemented because Fuchsia does not use interceptors.
+void InitializeInterceptors() {}
+
+// Not implemented because this is only relevant for Android.
+void AndroidTestTlsSlot() {}
+
+// TSD was normally used on linux as a means of calling the hwasan thread exit
+// handler passed to pthread_key_create. This is not needed on Fuchsia because
+// we will be using __sanitizer_thread_exit_hook.
+void HwasanTSDInit() {}
+void HwasanTSDThreadInit() {}
+
+// On linux, this just would call `atexit(HwasanAtExit)`. The functions in
+// HwasanAtExit are unimplemented for Fuchsia and effectively no-ops, so this
+// function is unneeded.
+void InstallAtExitHandler() {}
+
+}  // namespace __hwasan
+
+#endif  // SANITIZER_FUCHSIA