Bug 129992 – JS engine completely broken on ia64

• Component: JavaScriptCore
• Status: NEW

[Émeric MASCHINO]

Hi,

Starting up a WebKit-based application, e.g. GNOME Epiphany or Yelp, immediately segfaults.

Back in the WebKit 1.x days, patches for WebKit 1.8.1 on Debian were provided to fix JS engine on ia64 [1][2]. Sadly, it seems to me that these were never reported/merged upstream. They address three problems in WebKit JS engine.

[1st issue] On ia64, USE(JSVALUE64) is true. From the source code comment, JSVALUE64 path uses some sophisticated tricks in order to get everything in a 64-bits wide data type. As outlined there, this will work as long the high 16-bits of the addresses of any allocated memory are cleared. This is not always the case on ia64. A similar issue was fixed on Mozilla JS engine [3]. Original 01-ia64-wide-ptr patch against WebKit 1.8.1 [4] fixed this issue by defining a third JSVALUE64W option that was used on ia64 only. This path uses an encapsulated union without any trick for the variant data. As outlined by original author, this a portable but the data type is 128-bits wide and enabling JIT compiler isn't possible (note that ia64 doesn't have a JIT compiler).

[2nd issue] On ia64, and probably other arches too, caches are not coherent automatically. As such fast malloc implementation in Source/JavaScriptCore/wtf/FastMalloc.cpp is totally buggy. This was work-around by using system malloc instead in ia64-use-system-malloc patch [5].

[3rd issue] On Linux ia64, and probably other arches too, memory pages are not necessarily 4K. ia64 can have 4K, 8K, 16K or 64K memory pages. Most (all?) Linux ia64 distributions come with 16K memory pages. This was fixed in large-mem-page patch [6]. It's my understanding that this issue has also been fixed in bug #80615, but I may be wrong.

So, we're left with first two issues, unless second issue is no more valid nowadays.

Since original patch author no more wants to invest time on debugging WebKit, I've tried to port his work to WebKit 2.2.5, as it's the version we're trying to stabilize with GNOME 3.10 on Gentoo. It's not an easy task for me as (i) I'm not the original patch author so don't know what he's got in mind and (ii) I'm not a WebKit developer at all, so don't know how code has evolved or has been refactored since the Webkit 1.x days.

So, if a kind soul could have a look at the patches I'll post in this bugreport and give me advice to get WebKit JS engine fixed on ia64 or commonalize this work with other arches (is ia64 the only one using full 64-bit data path?), I would be really happy ;-)

     Émeric

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=642750#96
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=694971#5
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=589735
[4] https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=69;filename=01-ia64-wide-ptr.patch;att=1;bug=642750
[5] https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=69;filename=02-ia64-use-system-malloc.patch;att=2;bug=642750
[6] https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=large-mem-page.patch;att=1;bug=694971

[Émeric MASCHINO]

Created attachment 226259 [details]
First attempt at porting WebKit 1.8.1 01-ia64-wide-ptr.patch to WebKit 2.2.5

This is a port to WebKit 2.2.5 of 01-ia64-wide-ptr.patch originally created against WebKit 1.8.1 [1]. With this, all is not done, as I'm still getting compilation errors in files that either didn't exist in the WebKit 1.8.1 era or weren't #defined USE(VALUE32_64) or USE(VALUE64) as now. It seems to me that some of the problematic files could be:
- Source/JavaScriptCore/runtime/CommonSlowPaths.h
- Source/JavaScriptCore/runtime/JSWrapperObject.h
- Source/JavaScriptCore/interpreter/CallFrame.h
- Source/JavaScriptCore/interpreter/CallFrameInlines.h

So, reviewing at the attached patch, if you can advise me for the above files (or other files too) to what I should do/fix, I'm your man.

     Émeric


[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=69;filename=01-ia64-wide-ptr.patch;att=1;bug=642750

[Émeric MASCHINO]

Created attachment 228977 [details]
Second attempt at porting WebKit 1.8.1 01-ia64-wide-ptr.patch to WebKit 2.2.5

This new patch fixes the compilation problems of the initial patch port. As a quick test, MiniBrowser now starts up flawlessly, some URLs can even be browsed, others simply don't work (nothing happens when hitting the Enter key). As I'm not a WebKit developer, it's entirely plausible that I'm doing a lot of things wrong, but without more help from the WebKit community, it would be hard for me to go further. Indeed, in some places of my patch, it's really tries/guesses...

Currently, the impacted files are:
- API/APICast.h
- heap/CopiedBlock.h
- heap/Region.h
- interpreter/CallFrame.h
- interpreter/CallFrameInlines.h
- interpreter/Register.h
- llint/LLIntData.cpp
- llint/LLIntOfflineAsmConfig.h
- llint/LowLevelInterpreter.asm
- llint/LowLevelInterpreter.cpp
- runtime/JSCJSValue.h
- runtime/JSCJSValueInlines.h
- runtime/JSGlobalObjectFunctions.cpp
- runtime/JSWrapperObject.h
- wtf/Platform.h

Since the basic idea of this patch is to introduce another JSVALUE64W value type alongside JSVALUE32_64 and JSVALUE64, does this filelist sound OK or are they obvious missing participants?

One big difference between WebKit 1.8 and 2.x is the low-level interpreter that is used for C Loops when JIT is disabled (this is the case here for ia64). So I'm wondering e.g. if I have to mimic the JSVALUE64W changes at the llint level as I seem to decipher a close pattern between the structures in JSCJSValues.h and LLIntData.cpp for the JSVALUE32_64 and JSVALUE64 cases. Or should I adapt the JSVALUE64W-defined structures in JSCJSValues.h to use the JSVALUE64 path in LLIntData.cpp?

[Michael Catanzaro]

If you want a review on this, please submit a rebased patch with updated changelog files (using Tools/Scripts/prepare-ChangeLogs) and set the review? and commit-queue? Bugzilla flag on the attachment. See http://www.webkit.org/coding/contributing.html

[Filip Pizlo]

(In reply to comment #0)
> Hi,
> 
> Starting up a WebKit-based application, e.g. GNOME Epiphany or Yelp,
> immediately segfaults.
> 
> Back in the WebKit 1.x days, patches for WebKit 1.8.1 on Debian were
> provided to fix JS engine on ia64 [1][2]. Sadly, it seems to me that these
> were never reported/merged upstream. They address three problems in WebKit
> JS engine.
> 
> [1st issue] On ia64, USE(JSVALUE64) is true. From the source code comment,
> JSVALUE64 path uses some sophisticated tricks in order to get everything in
> a 64-bits wide data type. As outlined there, this will work as long the high
> 16-bits of the addresses of any allocated memory are cleared. This is not
> always the case on ia64. A similar issue was fixed on Mozilla JS engine [3].
> Original 01-ia64-wide-ptr patch against WebKit 1.8.1 [4] fixed this issue by
> defining a third JSVALUE64W option that was used on ia64 only. This path
> uses an encapsulated union without any trick for the variant data. As
> outlined by original author, this a portable but the data type is 128-bits
> wide and enabling JIT compiler isn't possible (note that ia64 doesn't have a
> JIT compiler).

You're going to have a really bad time with JSVALUE64W, particularly since it will always be an obscure configuration that nobody uses.  This is especially true since ia64 is effectively dead - it's hard to get help implementing and maintaining a port for unused architectures. I recommend trying to address this problem with a less invasive approach that is less work to implement and less work to maintain.

I suggest teaching our GC to allocate pages in a low memory range where the top 16 address bits are zero. I think that's what Mozilla did. 

You could also have a global variable that contains the global heap offset and turn each access into:

PtrBits = Value & 0xffffffffffff
Ptr = HeapBase + PtrBits

This would still be a big change, so it's preferable to just find the right mmap trick to pin the heap in low memory. 

> 
> [2nd issue] On ia64, and probably other arches too, caches are not coherent
> automatically. As such fast malloc implementation in
> Source/JavaScriptCore/wtf/FastMalloc.cpp is totally buggy. This was
> work-around by using system malloc instead in ia64-use-system-malloc patch
> [5].

If this is true then you'll need to make lots of other changes in WebKit. WebKit assumes that the memory model is never weaker than what ARM would do. 

> 
> [3rd issue] On Linux ia64, and probably other arches too, memory pages are
> not necessarily 4K. ia64 can have 4K, 8K, 16K or 64K memory pages. Most
> (all?) Linux ia64 distributions come with 16K memory pages. This was fixed
> in large-mem-page patch [6]. It's my understanding that this issue has also
> been fixed in bug #80615, but I may be wrong.

I doubt that this is an issue anymore. I know we run with different page sizes on different platforms now. 

> 
> So, we're left with first two issues, unless second issue is no more valid
> nowadays.
> 
> Since original patch author no more wants to invest time on debugging
> WebKit, I've tried to port his work to WebKit 2.2.5, as it's the version
> we're trying to stabilize with GNOME 3.10 on Gentoo. It's not an easy task
> for me as (i) I'm not the original patch author so don't know what he's got
> in mind and (ii) I'm not a WebKit developer at all, so don't know how code
> has evolved or has been refactored since the Webkit 1.x days.
> 
> So, if a kind soul could have a look at the patches I'll post in this
> bugreport and give me advice to get WebKit JS engine fixed on ia64 or
> commonalize this work with other arches (is ia64 the only one using full
> 64-bit data path?), I would be really happy ;-)
> 
>      Émeric
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=642750#96
> [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=694971#5
> [3] https://bugzilla.mozilla.org/show_bug.cgi?id=589735
> [4]
> https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=69;filename=01-ia64-wide-
> ptr.patch;att=1;bug=642750
> [5]
> https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=69;filename=02-ia64-use-
> system-malloc.patch;att=2;bug=642750
> [6]
> https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=large-mem-page.
> patch;att=1;bug=694971

[Émeric MASCHINO]

(In reply to comment #3)
> If you want a review on this, please submit a rebased patch with updated
> changelog files (using Tools/Scripts/prepare-ChangeLogs) and set the review?
> and commit-queue? Bugzilla flag on the attachment. See
> http://www.webkit.org/coding/contributing.html

Well, idea fast rather a quick informal review, or more appropriately implementation guidances, just as Filip Pizlo did in comment #4. From there, porting old WebKit 1.x Debian patches to WebKit 2.x doesn't seem the right way to go. Exactly the kind of orientation I was after.

Formal review will follow later, much later... when I actually rewrote something more suitable with current WebKit architecture/codebase.

Thanks anyway,

     Émeric

[Émeric MASCHINO]

(In reply to comment #4)

Thank you for taking the time to look at this.

> You're going to have a really bad time with JSVALUE64W, particularly since
> it will always be an obscure configuration that nobody uses.  This is
> especially true since ia64 is effectively dead - it's hard to get help
> implementing and maintaining a port for unused architectures. I recommend
> trying to address this problem with a less invasive approach that is less
> work to implement and less work to maintain.

Thank you for guidance here.

> I suggest teaching our GC to allocate pages in a low memory range where the
> top 16 address bits are zero. I think that's what Mozilla did. 

You're probably referring to [1], aren't you?

Mozilla's approach is thus to pass mmap an "addr" parameter that equals 0x0000070000000000 to ensure that all allocated pointers have their high 17 bits clear.

Would the same mmap trick work for WebKit's GC, adapting it to have the high 16 bits clear (rather than 17)?

> If this is true then you'll need to make lots of other changes in WebKit.
> WebKit assumes that the memory model is never weaker than what ARM would do. 

I can't be sure there, I'm not a kernel developer.

Isn't there no other architecture in WebKit's codebase with a memory model as weak as ia64 that I can have a look at to discover how they deal with cache/memory coherency? I can see a lot of arches in CMakeLists.txt and Source/JavaScriptCore/CMakeLists.txt, but are they actually working properly or are in the same bad shape than ia64 currently?

> I doubt that this is an issue anymore. I know we run with different page
> sizes on different platforms now.

Are you referring to [2] there? From what I understand, choice of 4K and 16K page sizes are supported, but what about other values, such as 8K and 64K for Linux ia64?

Fortunately enough, most (all?) ia64 Linux distributions came configured by default with CONFIG_IA64_PAGE_SIZE_16KB=y back at the time, so are on a supported case w.r.t. bug #115502.

     Émeric


[1] http://hg.mozilla.org/mozilla-central/rev/9c15d0fb3e25
[2] http://trac.webkit.org/changeset/149472