[PATCH] D35865: [asan] Fuchsia port

Roland McGrath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 6 17:44:00 PDT 2017


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


================
Comment at: lib/asan/asan_errors.cc:74
+#if SANITIZER_FUCHSIA
+  UNIMPLEMENTED();
+#else
----------------
vitalybuka wrote:
> Why you can't leave the current implementation?
It's dead code already because Fuchsia doesn't have anything like signal handling (it would take a bunch of changes in generic code to get rid of the references to it in other dead code paths).  Defining it would require me to define more functions of dead code just to satisfy its references (__sanitizer::DescribeSignalOrException, __sanitizer::SignalContext::DumpAllRegisters).  But there is plenty of dead code cruft already and plenty of stub functions I've defined just because of all the dead code already.  And reducing #if complexity is good.  So I'll do that.


================
Comment at: lib/asan/asan_rtl.cc:370
 
+#if !SANITIZER_FUCHSIA
+AsanThread *CreateMainThread() {
----------------
vitalybuka wrote:
> Please move into CL with thread API refactoring as well
Done in D36385.


================
Comment at: lib/asan/asan_shadow_setup.cc:17
+
+// asan_fuchsia.cc has its own InitializeShadowMemory implementation.
+#if !SANITIZER_FUCHSIA
----------------
vitalybuka wrote:
> Can I ask you to try later to move such full file "#if !SANITIZER_FUCHSIA ..." into cmake rules?
That would be inconsistent with the rest of the code base.
I don't have an opinion on how this is done, but it should be consistent.
If you also want to change all the *_win, *_mac, *_posix etc. files to a new style where they don't have #if around their whole contents are are excluded at the cmake level instead, that's fine with me and I'm happy to help make the Fuchsia-specific code consistent with the rest of the code base.



================
Comment at: lib/asan/asan_thread.cc:273
+  if (SANITIZER_FUCHSIA) {
+    // On Fuchsia, Init() is called (and calls here) in the creator thread
+    // before the new thread is actually started, but its stack address range
----------------
vitalybuka wrote:
> Please move API change and other thread code movements, without fucshia, into separate patch
> 
Done in D36385.


https://reviews.llvm.org/D35865





More information about the llvm-commits mailing list