Fix JSC DebugHeap Crash

[webghost009]

Summary

Fix a crash in the JSC DebugHeap when using tools like ASAN by adding an implementation of posix_memalign() and not patching the DebugHeap::memalign() method.

Previously JSC's debug heap would crash when activated by using tools like ASAN. This was due to a call to posix_memalign() being patched out during the build process which caused DebugHeap::memalign() to unconditionally call BCRASH(). The call to posix_memalign() was originally patched out because it is not offered in Android API <= 16 and jsc-android targets API >= 16. This change replaces the original patch with one that adds an implementation of posix_memalign() borrowed from the Android Support Library source.

Because Android API version 16 doesn't include posix_memalign(), the call to it gets patched out to work around the compilation error it causes. This is done by jsc_fix_build_error_memalign.patch. DebugHeap::memalign() becomes:

void* DebugHeap::memalign(size_t alignment, size_t size, bool crashOnFailure)
{
    BCRASH();
    return nullptr;
}

The DebugHeap isn't typically enabled. But it can be. The decision is made by Environment::computeIsDebugHeapEnabled():

bool Environment::computeIsDebugHeapEnabled()
{
    if (isMallocEnvironmentVariableSet())
        return true;
    if (isLibgmallocEnabled())
        return true;
    if (isSanitizerEnabled())
        return true;
#if BUSE(CHECK_NANO_MALLOC)
    if (!isNanoMallocEnabled() && !shouldProcessUnconditionallyUseBmalloc())
        return true;
#endif
    return false;
}

isSanitizerEnabled() is the particular check giving me problems (though the other checks may also be of concern in other scenarios). This method searches the current process for the ASAN and TSAN init methods:

static bool isSanitizerEnabled()
{
    void* handle = dlopen(nullptr, RTLD_NOW);
    if (!handle)
        return false;
    bool result = !!dlsym(handle, "__asan_init") || !!dlsym(handle, "__tsan_init");
    dlclose(handle);
    return result;
}

The decision made by Environment::computeIsDebugHeapEnabled() is cached by the Environment object and exposed outward via Environment::isDebugHeapEnabled(). This method is then referenced in several places by Webkit's custom allocator, bmalloc. Here it's used in the bmalloc Heap constructor to decide whether to allocate a DebugHeap object:

Heap::Heap(HeapKind kind, std::lock_guard<Mutex>&)
    : m_kind(kind)
    , m_vmPageSizePhysical(vmPageSizePhysical())
    , m_debugHeap(nullptr)
{
    RELEASE_BASSERT(vmPageSizePhysical() >= smallPageSize);
    RELEASE_BASSERT(vmPageSize() >= vmPageSizePhysical());

    initializeLineMetadata();
    initializePageMetadata();
    
    if (PerProcess<Environment>::get()->isDebugHeapEnabled())
        m_debugHeap = PerProcess<DebugHeap>::get();
    else {
        Gigacage::ensureGigacage();
#if GIGACAGE_ENABLED
        if (usingGigacage()) {
            RELEASE_BASSERT(gigacageBasePtr());
            m_largeFree.add(LargeRange(gigacageBasePtr(), gigacageSize(), 0, 0));
        }
#endif
    }
    
    m_scavenger = PerProcess<Scavenger>::get();
}

When the ASAN wrap.sh, included with the Android NDK, is used:

#!/system/bin/sh
HERE="$(cd "$(dirname "$0")" && pwd)"
export ASAN_OPTIONS=log_to_syslog=false,allow_user_segv_handler=1
export LD_PRELOAD=$HERE/libclang_rt.asan-arm-android.so
"$@"

libclang_rt.asan-arm-android.so is loaded before JSC and so the symbol __asan_init exists when the sanitizer check is performed. The decision to enable the debug heap is made and JSC crashes with a SIGILL when DebugHeap::memalign() is called and invokes BCRASH().

This patch avoids the crash by not patching the DebugHeap::memalign() method and instead adding an implementation for posix_memalign(), taken from the Android Support Library source. https://android.googlesource.com/platform/ndk/+/c066f37aeadeb8a8b21468ad8c82f4469fb5a70d/sources/android/support/src/posix_memalign.cpp

Test Plan

Ran the modified build of JSC on an x86 simulator and a real Pixel 2 (armeabi-v7a) with and without ASAN enabled in my own C++ code. Observed that JSC did not crash in any of these cases.

What's required for testing (prerequisites)?

A local build of JSC with the included modifications and a RN project modified to to use the wrap.sh feature to LD_PRELOAD the ASAN library.

https://developer.android.com/ndk/guides/wrap-script
https://github.com/google/sanitizers/wiki/AddressSanitizerOnAndroid

What are the steps to reproduce (after prerequisites)?

Run the application using a publicly released build of jsc-android on a device with the appropriate wrap.sh and ASAN .so included and observe the crash. Then switch to use the local JSC build with this patch and observe that it does not crash.

Compatibility

OS	Implemented
iOS	❌
Android	✅

Checklist

• I have tested this on a device and a simulator
• I added the documentation in README.md
• I mentioned this change in CHANGELOG.md
• I updated the typed files (TS and Flow)
• I added a sample use of the API in the example project (example/App.js)

[Kudo]

Awesome! Thanks for your contribution and brief description.
Let's make jsc-android better together.

[webghost009]

@Kudo - Thanks for the help with this! :-)

[webghost009]

@Kudo - When can I expect this change to be published to npm?

[Kudo]

@webghost009 We used to publish new version if there are major updates.
May I know your use case?
As from my understanding, ASAN verification only happens in internal testing.
You may able to build custom jsc-android binary to test, right?
If you need, I could update the build generated from CircleCI for you.
Hopefully this helps your use case, thank you.