[PATCH] D35864: [sanitizer_common] Fuchsia port

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr marked 6 inline comments as done.
mcgrathr added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_common_libcdep.cc:43
+
+static INLINE bool ReportSupportsColors() {
+  return true;
----------------
filcab wrote:
> In Fuchsia the output will never go into a file or somewhere where colors aren't supported?
Correct.  Fuchsia's logging will always go through a presentation filter that can massage the colorization as appropriate for its final output format.
I'll add a comment.


================
Comment at: lib/sanitizer_common/sanitizer_platform.h:279
 // that this allocation happens in dynamic linker and should be ignored.
-#if SANITIZER_PPC || defined(__thumb__)
+#if (SANITIZER_LINUX && !SANITIZER_ANDROID) && \
+  (SANITIZER_PPC || defined(__thumb__))
----------------
filcab wrote:
> Doesn't seem Fuchsia related.
It's related in the sense that it's cleaning up an OS-specific assumption.
This setting is specifically for dealing with glibc implementation issues and hence applies only on non-Android Linux.
It so happens today that non-Android Linux is the only PPC configuration, but this setting has nothing intrinsically to do with PPC.


================
Comment at: lib/sanitizer_common/sanitizer_platform_interceptors.h:25
+
+// TODO(mcgrathr): Should be renamed SI_UNIX or something.
+#if SANITIZER_WINDOWS || SANITIZER_FUCHSIA
----------------
filcab wrote:
> `NOT_WINDOWS` doesn't mean UNIX, though. We do have `SI_POSIX`, which might be close to what you need?
It looks like SANITIZER_POSIX does in fact so far just mean !SANITIZER_WINDOWS.
So I'm fine with changing all the not-windows tests to be is-posix instead.



================
Comment at: lib/sanitizer_common/sanitizer_platform_interceptors.h:120
-#define SANITIZER_INTERCEPT_MEMCHR 1
-#define SANITIZER_INTERCEPT_MEMRCHR (SI_FREEBSD || SI_LINUX || SI_NETBSD)
 
----------------
filcab wrote:
> Where did this go?
Oops.  Restored.


================
Comment at: lib/sanitizer_common/sanitizer_stacktrace_printer.cc:36
+  // Fuchsia uses its own bespoke format to enable offline symbolization.
+  RenderStackFrame(buffer, frame_no, info.address);
+#else
----------------
filcab wrote:
> Maybe have a different default? Or are you planning on simply not having this type of functionality (allow users to set their stacktrace format)?
We won't have that functionality at all.
I'll add a comment.



Repository:
  rL LLVM

https://reviews.llvm.org/D35864