582593 - JS crash due to invalid alignment assumptions on JSString::unitStringTable

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Ulrich Weigand]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.8) Gecko/20100723 Ubuntu/9.04 (CK-IBM) (CK-IBM) Firefox/3.6.8
Build Identifier: Ubuntu firefox-3.6.8+build1+nobinonly on armel-linux

The Ubuntu firefox-3.6.8+build1+nobinonly package fails to build from source on armv7l-unknown-linux-gnueabi using the GCC 4.4.4-7ubuntu3 compiler, due to a crash (segmentation fault) of the "js" interpreter used during the build.

Analysis seems to point to a bug in the underlying Mozilla sources that just happen to trigger due to random compiler and/or code changes.

The immediate cause of the crash is that a global array is only 4-byte aligned, but the code silently assumes 8-byte alignment.

The problem triggers in jsatom.cpp:js_AtomizeString:

            return (JSAtom *) STRING_TO_JSVAL(JSString::unitString(c));

jsapi.h:STRING_TO_JSVAL uses the low 3 bits of the pointer value to encode the type of the atom. This works only if all pointers passed to STRING_TO_JSVAL are guaranteed to be 8-byte aligned.

This is usually not a problem if the value is dynamically allocated. But in this particular instance, JSString::unitString returns the address of a global variable (jsstrinlines.h):

inline JSString *
JSString::unitString(jschar c)
{
    JS_ASSERT(c < UNIT_STRING_LIMIT);
    return &unitStringTable[c];
}

The array unitString Table is a static member of the JSString class (jsstr.h):

    static JSString unitStringTable[];

The compiler assumes that the alignment requirement of that static variable derive from the alignment requirement of the JSString type. Since this type has only two members, a size_t and a union of two pointer types, the total alignment requirement on a platform with 32-bit pointers like ARM is 4 bytes.

As it so happens, in the build of the "js" executable with this compiler and options, the variable does actually turn out to reside at an address that is only 4-byte aligned, but not 8-byte aligned:

000cc034 d JSString::unitStringTable

It seems to me that this is not a compiler bug, just something that can be triggered by random changes in the compiler ...

Interestingly enough, there is code in jsstr.h to ensure 8-byte alignment for unitStringTable, but only on Solaris:

#ifdef __SUNPRO_CC
#pragma align 8 (__1cIJSStringPunitStringTable_, __1cIJSStringOintStringTable_)
#endif

    static JSString unitStringTable[];
    static JSString intStringTable[];
    static const char *deflatedIntStringTable[];

I guess it looks like a more generic fix for this problem is required. I'm wondering why this never triggered elsewhere ...  Is there anything I'm missing that would make this work reliably?

Reproducible: Always

Steps to Reproduce:
1. Install Ubuntu maverick on armel-linux
2. Build the firefox-3.6.8+build1+nobinonly package from source
3. Build aborts with a JS crash:
gcc -c -x c -E -P -I. imacro_asm.js.in > imacro_asm.js
./../../dist/bin/run-mozilla.sh ./../../dist/bin/js imacro_asm.js ./imacros.jsasm > imacros.c.tmp
Segmentation fault
make[4]: *** [libs] Error 139
make[4]: Leaving directory `/build/buildd/firefox-3.6.6+nobinonly/build-tree/mozilla/js/src'

Actual Results:  
http://launchpadlibrarian.net/51627119/buildlog_ubuntu-maverick-armel.firefox_3.6.6%2Bnobinonly-0ubuntu1_FAILEDTOBUILD.txt.gz

Expected Results:  
Build succeeds.

See also Ubuntu bug report at:
https://bugs.launchpad.net/gcc-linaro/+bug/604874

Comment 1

[Alexander Sack]

https://bugzilla.mozilla.org/show_bug.cgi?id=560464 might be related

Comment 2

[Jason Orendorff [:jorendorff]]

In tracemonkey tip, we have this in jsstr.cpp:

  JSString JSString::unitStringTable[]
  #ifdef __GNUC__
  __attribute__ ((aligned (8)))
  #endif
  = {
      U(0x00), U(0x01), U(0x02), U(0x03), U(0x04), U(0x05), U(0x06), U(0x07),
      U(0x08), U(0x09), U(0x0a), U(0x0b), U(0x0c), U(0x0d), U(0x0e), U(0x0f),
      ...

Is that __attribute__ in your jsstr.cpp?

If not, try updating to tip and building that.

Comment 3

[Alexander Sack]

Thanks Jason.

Ulrich, if this isn't in your current code, I can probably make a tip build available for you more or less soonish. Let me know.

Comment 4

[Ulrich Weigand]

Thanks for the hint, Jason!  I do have the aligned attribute in the code, I just missed it since I was looking only at the declaration site ...

Unfortunately, it seems we have a GCC bug here: starting with GCC 4.5, the C++ front-end appears to simply ignore the aligned attribute in code like this.  (We have apparently back-ported the patch that introduced this regression to the Ubuntu/Linaro compiler, which is why we're seeing it on a 4.4-based compiler too.)

I've now opened a GCC bugzilla to track the bug there:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45112

Comment 5

[Ulrich Weigand]

Setting this bug to INVALID since it's not a Mozilla problem after all.