Closed Bug 145267 Opened 23 years ago Closed 23 years ago

gcc 3.1 optimization problems in nsFrame::GetBidiProperty

Categories

(SeaMonkey :: Build Config, defect)

Product:
SeaMonkey
Component:
Build Config
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED WONTFIX

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

(Keywords: relnote)

Attachments

(1 file)

simplified testcase for optimization problem
368 bytes, text/plain
Details
My gcc 3.1 -O2 build always draws the caret in the wrong place. printf debugging led to the problem being at the end of nsTextFrame::GetPointFromOffset. When outPoint->x gets filled in from width, it just doesn't do anything.
Well, I can certainly say that the basic block reordering makes this function fun to read. Since I had a printf near where the correct value should have been filled in, I'll start at that point. gcc 3.0.4 does (in the |else| near the end that ought to have braces around it): 70b1: 8b 4d 08 mov 0x8(%ebp),%ecx 70b4: 8b 95 6c fb ff ff mov 0xfffffb6c(%ebp),%edx 70ba: 8b 41 0c mov 0xc(%ecx),%eax 70bd: 39 c2 cmp %eax,%edx 70bf: 7e 07 jle 70c8 <nsTextFrame::GetPoi 70c1: 8b 55 18 mov 0x18(%ebp),%edx 70c4: 89 02 mov %eax,(%edx) 70c6: eb 86 jmp 704e <nsTextFrame::GetPoi 70c8: 8b 4d 18 mov 0x18(%ebp),%ecx 70cb: 89 11 mov %edx,(%ecx) 70cd: e9 7c ff ff ff jmp 704e <nsTextFrame::GetPoi which jumps to a list of destructor calls (presumably): 704e: 8b 45 18 mov 0x18(%ebp),%eax 7051: 83 ec 0c sub $0xc,%esp 7054: c7 40 04 00 00 00 00 movl $0x0,0x4(%eax) 705b: 8d 85 98 fb ff ff lea 0xfffffb98(%ebp),%eax 7061: 50 push %eax 7062: e8 fc ff ff ff call 7063 <nsTextFrame::GetPoi 7067: 8d 85 c8 fc ff ff lea 0xfffffcc8(%ebp),%eax 706d: 89 04 24 mov %eax,(%esp,1) 7070: e8 fc ff ff ff call 7071 <nsTextFrame::GetPoi 7075: 8d 85 d8 fc ff ff lea 0xfffffcd8(%ebp),%eax 707b: 89 04 24 mov %eax,(%esp,1) 707e: e8 fc ff ff ff call 707f <nsTextFrame::GetPoi 7083: 8d 85 e8 fc ff ff lea 0xfffffce8(%ebp),%eax 7089: 89 04 24 mov %eax,(%esp,1) 708c: e8 fc ff ff ff call 708d <nsTextFrame::GetPoi 7091: 8d 85 88 fe ff ff lea 0xfffffe88(%ebp),%eax 7097: 89 04 24 mov %eax,(%esp,1) 709a: e8 fc ff ff ff call 709b <nsTextFrame::GetPoi 709f: 8d 45 98 lea 0xffffff98(%ebp),%eax 70a2: 89 04 24 mov %eax,(%esp,1) 70a5: e8 fc ff ff ff call 70a6 <nsTextFrame::GetPoi 70aa: 31 c0 xor %eax,%eax 70ac: e9 eb fd ff ff jmp 6e9c <nsTextFrame::GetPoi which then goes to: 6e9c: 83 c4 10 add $0x10,%esp 6e9f: 8d 65 f4 lea 0xfffffff4(%ebp),%esp 6ea2: 5b pop %ebx 6ea3: 5e pop %esi 6ea4: 5f pop %edi 6ea5: 5d pop %ebp 6ea6: c3 ret gcc 3.1 does this: 77d0: 8b 4d 08 mov 0x8(%ebp),%ecx 77d3: 8b 95 6c fb ff ff mov 0xfffffb6c(%ebp),%edx 77d9: 8b 41 0c mov 0xc(%ecx),%eax 77dc: 39 c2 cmp %eax,%edx 77de: 7e 07 jle 77e7 <nsTextFrame::GetPoi 77e0: 8b 55 18 mov 0x18(%ebp),%edx 77e3: 89 02 mov %eax,(%edx) 77e5: eb 80 jmp 7767 <nsTextFrame::GetPoi which then brings us to: 7767: 8b 45 18 mov 0x18(%ebp),%eax 776a: c7 40 04 00 00 00 00 movl $0x0,0x4(%eax) 7771: 8b 95 54 fb ff ff mov 0xfffffb54(%ebp),%edx 7777: 89 14 24 mov %edx,(%esp,1) 777a: e8 fc ff ff ff call 777b <nsTextFrame::GetPoi 777f: 8b 8d 58 fb ff ff mov 0xfffffb58(%ebp),%ecx 7785: 89 0c 24 mov %ecx,(%esp,1) 7788: e8 fc ff ff ff call 7789 <nsTextFrame::GetPoi 778d: 8d 85 d8 fc ff ff lea 0xfffffcd8(%ebp),%eax 7793: 89 04 24 mov %eax,(%esp,1) 7796: e8 fc ff ff ff call 7797 <nsTextFrame::GetPoi 779b: 8b 85 5c fb ff ff mov 0xfffffb5c(%ebp),%eax 77a1: 89 04 24 mov %eax,(%esp,1) 77a4: e8 fc ff ff ff call 77a5 <nsTextFrame::GetPoi 77a9: 8b 95 60 fb ff ff mov 0xfffffb60(%ebp),%edx 77af: 89 14 24 mov %edx,(%esp,1) 77b2: e8 fc ff ff ff call 77b3 <nsTextFrame::GetPoi 77b7: 8b 8d 64 fb ff ff mov 0xfffffb64(%ebp),%ecx 77bd: 89 0c 24 mov %ecx,(%esp,1) 77c0: e8 fc ff ff ff call 77c1 <nsTextFrame::GetPoi 77c5: 31 c0 xor %eax,%eax 77c7: e9 8f fd ff ff jmp 755b <nsTextFrame::GetPoi and thus: 755b: 81 c4 cc 04 00 00 add $0x4cc,%esp 7561: 5b pop %ebx 7562: 5e pop %esi 7563: 5f pop %edi 7564: 5d pop %ebp 7565: c3 ret
In the first of the blocks from gcc 3.1 I skipped the end: 77e7: 8b 4d 18 mov 0x18(%ebp),%ecx 77ea: 89 11 mov %edx,(%ecx) 77ec: e9 76 ff ff ff jmp 7767 <nsTextFrame::GetPoi
So, after all that, I still don't see anything wrong.
OK, I should have put more printfs in sooner. The problem seems to be that |width| is being clobbered (set to zero) around the call to GetBidiProperty.
OK, the real problem is in nsFrame::GetBidiProperty, where the |memset| is clobbering more than the 1 bytes that it's asked to clobber.
Summary: gcc 3.1 optimization problems in nsTextFrame::GetPointFromOffset → gcc 3.1 optimization problems in nsFrame::GetBidiProperty
This seems to be as simple as I could get it: * I had to use offset 3. * I had to cast to (void**) so the function parameter would be void**, not void*. * I had to pass the 1 as a parameter.
dbaron@dbaron Linux (0) ~/gccbug/145267 > /usr/local/gcc-3.1/bin/gcc -O test.cpp ; ./a.out BABABABA BABABABA 00BABABA BABA0000 dbaron@dbaron Linux (0) ~/gccbug/145267 > /usr/local/gcc-3.1/bin/gcc test.cpp ; ./a.out BABABABA BABABABA 00BABABA BABABABA Note that gcc 3.0.4 gives an internal compiler error on this particular testcase. So, brendan, bbaetz -- is this our bug or gcc's? I'm guessing it's gcc's, since memset should do what it's supposed to, but could it be considered ours since we're dealing with a variable in a void** and we only want 1 byte of what it points to to be modified?
FWIW, a cvs gcc build from last night prints: BABABABA BABABABA 00BABABA BABABABA I don't have 3.1 arround, but does -fno-builtins fix it, perhaps?
-fno-builtins doesn't fix it.
What about -fno-strict-aliasing? I think the test case you have is valid, only because going from a char to anything shouldn't break the aliasing rules, but I'm not certain. If you put do_memset into a separate file than main, and/or use -fno-inline, does that fix it?
Neither -fno-inline nor -fno-strict-aliasing fixes the problem. When I created the testcase, I originally tried to make the memset inline, but that didn't work.
This is a better link to the gcc PR: http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=6703 Glenn Nakamura added a comment to the PR pointing out a patch that fixes the problem that is on the gcc trunk.
The gcc patch provided fixes the caret positioning for me.
*** Bug 145213 has been marked as a duplicate of this bug. ***
*** Bug 151857 has been marked as a duplicate of this bug. ***
*** Bug 147774 has been marked as a duplicate of this bug. ***
*** Bug 153717 has been marked as a duplicate of this bug. ***
*** Bug 156041 has been marked as a duplicate of this bug. ***
GCC 3.1.1 is out now and includes the fix.
Is the right thing to do WONTFIX this, and add something to the release notes documenting gcc 3.1 as an unsuitable compiler?
INVALID rather than WONTFIX probably, but I don't see an issue with that. I don't thik we need a configure check or anything like that, though.
OK, wontfix, and adding |relnote| keyword. Is that enough to get noticed, or should I email someone as well? SUGGESTED RELEASE NOTE TEXT: gcc 3.1 should not be used to compile Mozilla on x86 platforms, since it has an optimization bug that prevents the caret from being drawn in the right place (and may cause other bugs). See bug 145267. Other versions of gcc, such as 3.0.4, 3.1.1, and 3.2, do not have this problem.
Status: NEW → RESOLVED
Closed: 23 years ago
Keywords: relnote
Resolution: --- → WONTFIX
v wontfix
Status: RESOLVED → VERIFIED
*** Bug 148723 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: