Opened 11 years ago

Closed 11 years ago

#6535 closed patch (fixed)

Soundtouch crashes in sse_gcc.cpp due to unaligned variable

Reported by: Jeff Lu <jll544@…> Owned by: danielk
Priority: minor Milestone: 0.22
Component: MythTV - General Version: head
Severity: medium Keywords: soundtouch sse
Cc: Ticket locked: no

Description

calcCrossCorrMulti and calcCrossCorrStereo use movdqa, which requires 16-byte alignment, but variable "out" is declared as dynamic unaligned. This patch redefines "out" as global static aligned(16). Global because gcc versions <4.4 reportedly fail to align dynamic variables properly. Tested on x86_32 with gcc 4.2.4

Attachments (2)

sse_gcc.patch (861 bytes) - added by Jeff Lu <jll544@…> 11 years ago.
sse_gcc_2.patch (1.2 KB) - added by Jeff Lu <jll544@…> 11 years ago.
Revised patch

Download all attachments as: .zip

Change History (9)

Changed 11 years ago by Jeff Lu <jll544@…>

Attachment: sse_gcc.patch added

comment:1 Changed 11 years ago by danielk

Status: newinfoneeded_new

Jeff, a global is no good. Please create a larger variable on the stack yourself and align a pointer into it and call it out. Something like "int x[16+4]; int *out=((&x+15)&~0xf);" but of course tested. Also a comment should reference the gcc bug report # for the fix, or at least the version it was fixed in.

PS I believe all versions of gcc actually do successfully give you 8 byte alignment; you could use that to make x smaller...

comment:2 Changed 11 years ago by Jeff Lu <jll544@…>

GCC bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16660 Comment 14 indicates gcc 4.4 revision 138335 fixes dynamic alignment > required alignment. I'll try your suggested workaround.

Changed 11 years ago by Jeff Lu <jll544@…>

Attachment: sse_gcc_2.patch added

Revised patch

comment:3 Changed 11 years ago by Jeff Lu <jll544@…>

Revised the patch using Daniel's workaround. I tried attribute((aligned(8))) to make x smaller, but gcc then incorrectly calculated (int*)out. I believe this is the behavior described in comment 5 of the gcc bugzilla.

comment:4 in reply to:  3 Changed 11 years ago by Jeff Lu <jll544@…>

To clarify my last comment, the revised patch (which doesn't use the aligned attribute) works and was tested against gcc 4.2.4. Debugging showed proper data alignment and pointer calculation.

comment:5 Changed 11 years ago by danielk

Status: infoneeded_newnew

comment:6 Changed 11 years ago by danielk

Milestone: unknown0.22
Owner: changed from Isaac Richards to danielk
Status: newassigned

comment:7 Changed 11 years ago by danielk

Resolution: fixed
Status: assignedclosed

(In [20599]) Fixes #6535. Hack to avoid segfault with older gcc which failed to enforce 16 byte alignment for stack variable when requested via alignment attribute. Bug fixed in gcc 4.4.0+.

Note: See TracTickets for help on using tickets.