Closed Bug 582593 Opened 14 years ago Closed 14 years ago

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

Categories

(Core :: JavaScript Engine, defect)

Other
Linux
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: uweigand, Unassigned)

References

Details

(Keywords: crash)

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
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Keywords: crash
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.
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
Setting this bug to INVALID since it's not a Mozilla problem after all.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.