[Webkit-unassigned] [Bug 209236] REGRESSION(r249808): [GTK] Crash in JSC Config::permanentlyFreeze() on architecture ppc64el

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=209236

--- Comment #35 from Mark Lam <mark.lam at apple.com> ---
(In reply to Carlos Alberto Lopez Perez from comment #34)
> (In reply to Michael Catanzaro from comment #28)
> > The best solution is to get page size at runtime using
> > sysconf(_SC_PAGESIZE), but it looks like the code really wants a
> > compile-time solution. So maybe just hardcode 64 KB for these CPUs and for
> > CPU(UNKNOWN)? Ideally we would share the value with MarkedBlock.h? clopez,
> > what do you think?
> > 

Hardcoding to 64K is a good approach in addition to the check below.

> (In reply to Tomas Popela from comment #24)
> > 
> > s390x, ppc64(le) in RHEL and Fedora use 64 KB page size
> > aarch64 on RHEL uses 64 KB and on Fedora 4 KB page size
> 
> 
> I wonder about doing something like the patch below to avoid future problems
> in cases where we can't predict the page size.
> 
> 
> diff --git a/Source/JavaScriptCore/runtime/JSCConfig.cpp
> b/Source/JavaScriptCore/runtime/JSCConfig.cpp
> index 79cc2b67ba9..b85393e1def 100644
> --- a/Source/JavaScriptCore/runtime/JSCConfig.cpp
> +++ b/Source/JavaScriptCore/runtime/JSCConfig.cpp
> @@ -33,6 +33,7 @@
>  #include <mach/mach.h>
>  #elif OS(LINUX)
>  #include <sys/mman.h>
> +#include <unistd.h>
>  #endif
>  
>  namespace JSC {
> @@ -70,7 +71,10 @@ void Config::permanentlyFreeze()
>      // There's no going back now!
>      result = vm_protect(mach_task_self(),
> reinterpret_cast<vm_address_t>(&g_jscConfig), ConfigSizeToProtect,
> DisallowPermissionChangesAfterThis, VM_PROT_READ);
>  #elif OS(LINUX)
> -    result = mprotect(&g_jscConfig, ConfigSizeToProtect, PROT_READ);
> +    // Some architectures on Linux may have non-default page size.
> +    // In that cases, avoid the crash in the RELEASE_ASSERT below due to
> using mprotect with a wrong page size.
> +    if (sysconf(_SC_PAGESIZE) == ConfigSizeToProtect)
> +        result = mprotect(&g_jscConfig, ConfigSizeToProtect, PROT_READ);

This check is not correct.  ConfigSizeToProtect does not need to be strictly equal to sysconf(_SC_PAGESIZE).  It needs to be a multiple of sysconf(_SC_PAGESIZE), where the multiple can be 1 or higher.  You can use WTF::roundUpToMultipleOf() to compute that.


> 
> 
> 
> Does this looks like a good idea?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200319/c4544a99/attachment.htm>