Closed Bug 552627 Opened 13 years ago Closed 13 years ago
Crash with SSE2 enabled build on Linux
(Core :: XPConnect, defect)
(Reporter: yong.wu, Assigned: yong.wu)
(1 file, 3 obsolete files)
1.18 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/532.5 (KHTML, like Gecko) Chrome/22.214.171.124 Safari/532.5 Build Identifier: Build Firefox or Fennec with SSE2 enabled (Linux/x86) and browse websites www.cnn.com or www.sina.com.cn can cause Firefox/Fennec to crash. Reproducible: Always Steps to Reproduce: 1. Build Linux Firefox or Fennec with SSE enabled. This is the mozconfig option: ac_add_options --enable-optimize="-g -fno-inline -march=core2 -mssse3 -msse2 -mtune=generic -mfpmath=sse " 2. Run Firefox/Fennec on any Xeon/Core2/Atom platform platform. 3. Launch www.cnn.com or www.sina.com.cn 3. Actual Results: Firefox/Fennec crashes.
you didn't specify your version. bug 410509 http://hg.mozilla.org/mozilla-central/rev/c4c01d41e11a supposedly fixed this. bug 218629 has what i believe is a better fix for this, but it requires someone familiar with something to review. if you can vouch for it being better, we can see about pushing it.
I am using the Firefox 3.6 and Fennec 1.0rc3 with xulrunner 1.9.2 which has already those patches. But it still crash. I have attached a simple patch to fix.
Comment on attachment 432763 [details] [diff] [review] Fix the sse misalignment issue. local style uses an asterisk for each line of a comment: * is assumed and required, e.g. by GCC4's -ftree-vectorize option. */ indentation should be to this column [using tabs :(]: "andl $0xfffffff0, %ecx\n\t" /* drop(?) stack ptr to 128-bit align */ - "subl $8, %ecx\n\t" /* lower again; push/call below will re-align */ - + /* The $esp should be aligned to 16-bytes boundary here (note there + is additional 4 bytes in push instrucitons below). instructions But, probably "note we include an additional 4 bytes in a push later instruction" + This will ensure the $ebp in the function called below aligned to below _is_ aligned to _a_ 0x8 boundary. ? + 8x boundary. The SSE instructions like movapd/movdqa expects the omit 'The' + memory operand to be aligned to 16-bytes boundary. GCC compiler will aligned on/at ? a 16-byte boundary. ? "The GCC compiler" or "GCC" + generate the memory operand using $ebp with 8 bytes offset. with an 8 byte offset. + */ + "subl $0xc, %ecx\n\t" /* lower again; push/call below will re-align */ "movl %ecx, %esp\n\t" /* make stack space */
Thank Timeless's comments. I've corrected indention and syntax errors in comments. This is the second version of that patch.
Attachment #432763 - Attachment is obsolete: true
Attachment #432781 - Attachment is obsolete: true
13 years ago
Seems like this bug can be confirmed. Wu: Do you know about the review process, etc.? Maybe you should request official review from timeless so that the patch can be marked r+ or r- :).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #432782 - Flags: review+
Attachment #432782 - Flags: review+ → review?(timeless)
Attachment #432782 - Flags: review?(timeless) → review+
Comment on attachment 432782 [details] [diff] [review] Fix sse misalignment issue (v3) >+/* The $esp should be aligned to 16-bytes boundary here (note we include an drop "The" before $esp/$ebp ... to _a_ 16-byte__ boundary ... >+ * additional 4 bytes in a later push instruction). This will ensure the $ebp >+ * in the function called below is aligned to a 0x8 boundary. SSE instructions >+ * like movapd/movdqa expects memory operand to be aligned on a 16-byte ... expect ... >+ * boundary. The GCC compiler will generate the memory operand using $ebp with >+ * an 8-byte offset.
13 years ago
Thanks again for timeless's correction. Should I submit a new patch with the changes.. timeless?
13 years ago
it'll be easier if you do
13 years ago
Updated patch according to timeless's corrections.
Attachment #432782 - Attachment is obsolete: true
Attachment #447024 - Flags: review?
Attachment #447024 - Flags: review? → review?(timeless)
Attachment #447024 - Flags: review?(timeless) → review+
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.