Yeah!
It is the first time for me to become a GSoC mentor, ;)
I hope we can work together to make it happen.
I think the biggest problem now is Firefox's bus error when browsing certain
websites.
>
> And while we are on this topic:
> gcc & binutils have references in the code to ABI_EABI, I am wondering
> how hard it is to move from n32 to EABI - would be interesting to get
> an EABI port in the future :D
It seems there is little documentation about EABI. What's its advantages
comparing to N32/N64?
--
Zhang, Le
Gentoo/Loongson Developer
http://zhangle.is-a-geek.org
0260 C902 B8F8 6506 6586 2B90 BC51 C808 1E4E 2973
Congratulations to you as well. I think you have a Twitter (or blog)
entry related to being a SoC mentor :D
I guess you will be getting a Google T-shirt?
> I think the biggest problem now is Firefox's bus error when browsing certain
> websites.
I've read your Twitter (or blog?) entry related to that as well... but
I did not fully understand the issues around it.
Is it still that OS/architecture dependent Firefox module causing problems??
And were you able to get a stack trace when it "bus error"ed?
> It seems there is little documentation about EABI. What's its advantages
> comparing to N32/N64?
I did not read the "NUBI - A Revised ABI for the MIPS Architecture" in
detail, I have only read:
http://www.linux-mips.org/wiki/IntroducingNUBI
Besides more efficient use of registers, I think this is interesting:
* Adds a thread pointer : for efficient TLS.
Seems to me that the current ABIs available do not have a register for
TLS. So if you compile this code:
__thread int i;
func()
{
i = 12;
}
gcc will generate:
...
rdhwr $3,$29
.set pop
lui $2,%tprel_hi(i)
addu $3,$2,$3
...
And rdhwr is not supported by Loongson or any other MIPS processors.
So the processor will trigger an exception when it executes that code
sequence and the kernel needs to emulate this instruction. (also see:
http://www.linux-mips.org/archives/linux-mips/2006-06/msg00101.html)
On x64, accessing a __thread variable takes less than 100 cycles. I
think on MIPS we will need at least that number of cycles just to
enter the kernel.
I am not sure how much faster it is, or how usable is the EABI support
in the toolchain. But the important thing is, since the Loongson
platform is still young, we can define whatever is needed to make the
ABI efficient. When we have more commercial & proprietary applications
ported to the platform, then we will need to worry about backward
compatibility issues and won't be able to change the ABI easily.
Rayson
Yeah, I guess so. ;)
>
> > I think the biggest problem now is Firefox's bus error when browsing certain
> > websites.
>
> I've read your Twitter (or blog?) entry related to that as well... but
> I did not fully understand the issues around it.
>
> Is it still that OS/architecture dependent Firefox module causing problems??
No, I don't think so. There is a little test case to test that module, and it
passed.
>
> And were you able to get a stack trace when it "bus error"ed?
Yes, but it doesn't help much, at least to me.
>
>
> > It seems there is little documentation about EABI. What's its advantages
> > comparing to N32/N64?
>
> I did not read the "NUBI - A Revised ABI for the MIPS Architecture" in
> detail, I have only read:
>
> http://www.linux-mips.org/wiki/IntroducingNUBI
[snip]
I may get this wrong, but are you implying that EABI == NUBI?
Please take a look:
http://sourceware.org/ml/libc-ports/2009-03/msg00017.html
In the above link, Ralf said:
"Add the stillborn EABI and NUBI variants."
Judging from the wording, I think they may not necessarily be the same thing.
What do you think?
So is it always the same function casuing the problem?? And if so, can
you post the stack trace?
> I may get this wrong, but are you implying that EABI == NUBI?
...
> Judging from the wording, I think they may not necessarily be the same thing.
>
> What do you think?
Ah, this is ugly. If we don't clean up this mess, I think we will
continue to use the new old ABI (= n32) that was designed without
"native" TLS support.
OK, after googling a bit, I found that EABI = Embedded Application
Binary Interface (EABI), which may not be what I want as my focus is
on speed rather than space efficiency :D
Rayson
It depends on the action you are doing. The thing I don't understand is why the
source code is the same, but it behaves so differently in different ABIs. It
could be the kernel(different syscall for different ABIs), the toolchain(gcc or
gas). I have no clue.
I know there are two(I am sure that there are more than that) ways to crash it:
1. browsing www.sina.com.cn (many other sites seem to work pretty ok though)
2. view page source via context menu
The instructions triggered bus error are different in these two cases.
The stack trace for the second case is:
(gdb) bt
#0 PropertyProvider::GetSpacingInternal (this=0x7f975ce0, aStart=0, aLength=2, aSpacing=0x7f9745e4, aIgnoreTabs=0)
at nsTextFrameThebes.cpp:2232
#1 0x2bce87dc in gfxTextRun::GetAdjustedSpacingArray (this=0x11eb2b80, aStart=<value optimized out>,
aEnd=<value optimized out>, aProvider=<value optimized out>, aSpacingStart=0, aSpacingEnd=2, aSpacing=0x7f9745d8)
at gfxFont.cpp:1287
#2 0x2bce90ec in gfxTextRun::AccumulateMetricsForRun (this=0x11eb2b80, aFont=0x1211bfd0, aStart=0, aEnd=2, aTight=0,
aRefContext=0x110a2e18, aProvider=<value optimized out>, aSpacingStart=300624812, aSpacingEnd=2, aMetrics=0x7f975388)
at gfxFont.cpp:1588
#3 0x2bce94e0 in gfxTextRun::MeasureText (this=0x11eb2b80, aStart=<value optimized out>, aLength=<value optimized out>,
aTightBoundingBox=0, aRefContext=0x110a2e18, aProvider=0x7f975ce0) at gfxFont.cpp:1669
#4 0x2bce99e0 in gfxTextRun::BreakAndMeasureText (this=0x11eb2b80, aStart=0, aMaxLength=2,
aLineBreakBefore=<value optimized out>, aWidth=-115219, aProvider=0x7f975ce0,
aSuppressInitialBreak=<value optimized out>, aTrimWhitespace=<value optimized out>, aMetrics=0x7f975c70,
aTightBoundingBox=0, aRefContext=0x110a2e18, aUsedHyphenation=0x7f975b7c, aLastBreak=0x7f975b78) at gfxFont.cpp:1815
#5 0x2b4e26a8 in nsTextFrame::Reflow (this=0x12462054, aPresContext=<value optimized out>, aMetrics=@0x7f975e98,
aReflowState=@0x7f975ed8, aStatus=<value optimized out>) at nsTextFrameThebes.cpp:5512
#6 0x2b4b89cc in nsLineLayout::ReflowFrame (this=0x7f976138, aFrame=0x12462054, aReflowStatus=@0x7f976010, aMetrics=0x0,
aPushedFrame=<value optimized out>) at nsLineLayout.cpp:859
#7 0x2b47ca14 in nsBlockFrame::ReflowInlineFrame (this=0x1245bd28, aState=@0x7f9764d8, aLineLayout=@0x7f976138, aLine=
{mCurrent = 0x11e40080}, aFrame=0x12462054, aLineReflowStatus=0x7f976090) at nsBlockFrame.cpp:3568
#8 0x2b480598 in nsBlockFrame::DoReflowInlineFrames (this=0x1245bd28, aState=@0x7f9764d8, aLineLayout=@0x7f976138,
aLine=<value optimized out>, aKeepReflowGoing=0x7f976340, aLineReflowStatus=<value optimized out>,
aAllowPullUp=<value optimized out>) at nsBlockFrame.cpp:3390
#9 0x2b4808bc in nsBlockFrame::ReflowInlineFrames (this=0x1245bd28, aState=@0x7f9764d8, aLine={mCurrent = 0x11e40080},
aKeepReflowGoing=0x7f976340) at nsBlockFrame.cpp:3239
#10 0x2b480b68 in nsBlockFrame::ReflowLine (this=0x1245bd28, aState=@0x7f9764d8, aLine={mCurrent = 0x11e40080},
aKeepReflowGoing=<value optimized out>) at nsBlockFrame.cpp:2306
#11 0x2b481050 in nsBlockFrame::ReflowDirtyLines (this=0x1245bd28, aState=@0x7f9764d8) at nsBlockFrame.cpp:1886
#12 0x2b481928 in nsBlockFrame::Reflow (this=0x1245bd28, aPresContext=0x120f6350, aMetrics=@0x7f9767fc,
aReflowState=@0x7f9768f0, aStatus=@0x7f976768) at nsBlockFrame.cpp:953
---Type <return> to continue, or q <return> to quit---
#13 0x2b482558 in nsBlockReflowContext::ReflowBlock (this=0x7f9767d8, aSpace=<value optimized out>,
aApplyTopMargin=<value optimized out>, aPrevMargin=<value optimized out>, aClearance=<value optimized out>,
aIsAdjacentWithTop=<value optimized out>, aLine=<value optimized out>, aFrameRS=@0x7f9768f0,
aFrameReflowStatus=@0x7f976768, aState=@0x7f976cf8) at nsBlockReflowContext.cpp:311
#14 0x2b47d33c in nsBlockFrame::ReflowBlockFrame (this=0x12156720, aState=@0x7f976cf8, aLine={mCurrent = 0x1208ee90},
aKeepReflowGoing=<value optimized out>) at nsBlockFrame.cpp:2979
#15 0x2b480a84 in nsBlockFrame::ReflowLine (this=0x12156720, aState=@0x7f976cf8, aLine={mCurrent = 0x1208ee90},
aKeepReflowGoing=<value optimized out>) at nsBlockFrame.cpp:2251
#16 0x2b481050 in nsBlockFrame::ReflowDirtyLines (this=0x12156720, aState=@0x7f976cf8) at nsBlockFrame.cpp:1886
#17 0x2b481928 in nsBlockFrame::Reflow (this=0x12156720, aPresContext=0x120f6350, aMetrics=@0x7f97701c,
aReflowState=@0x7f977110, aStatus=@0x7f976f88) at nsBlockFrame.cpp:953
#18 0x2b482558 in nsBlockReflowContext::ReflowBlock (this=0x7f976ff8, aSpace=<value optimized out>,
aApplyTopMargin=<value optimized out>, aPrevMargin=<value optimized out>, aClearance=<value optimized out>,
aIsAdjacentWithTop=<value optimized out>, aLine=<value optimized out>, aFrameRS=@0x7f977110,
aFrameReflowStatus=@0x7f976f88, aState=@0x7f977518) at nsBlockReflowContext.cpp:311
#19 0x2b47d33c in nsBlockFrame::ReflowBlockFrame (this=0x12156424, aState=@0x7f977518, aLine={mCurrent = 0x1208ef58},
aKeepReflowGoing=<value optimized out>) at nsBlockFrame.cpp:2979
#20 0x2b480a84 in nsBlockFrame::ReflowLine (this=0x12156424, aState=@0x7f977518, aLine={mCurrent = 0x1208ef58},
aKeepReflowGoing=<value optimized out>) at nsBlockFrame.cpp:2251
#21 0x2b481050 in nsBlockFrame::ReflowDirtyLines (this=0x12156424, aState=@0x7f977518) at nsBlockFrame.cpp:1886
#22 0x2b481928 in nsBlockFrame::Reflow (this=0x12156424, aPresContext=0x120f6350, aMetrics=@0x7f9777f0,
aReflowState=@0x7f977830, aStatus=@0x7f9779c0) at nsBlockFrame.cpp:953
#23 0x2b489be0 in nsContainerFrame::ReflowChild (this=<value optimized out>, aKidFrame=0x12156424, aPresContext=0x120f6350,
aDesiredSize=@0x7f9777f0, aReflowState=@0x7f977830, aX=0, aY=0, aFlags=<value optimized out>, aStatus=@0x7f9779c0,
aTracker=0x0) at nsContainerFrame.cpp:771
#24 0x2b4a76c8 in CanvasFrame::Reflow (this=0x122ef7b0, aPresContext=0x120f6350, aDesiredSize=@0x7f977b68,
aReflowState=@0x7f9779e8, aStatus=@0x7f9779c0) at nsHTMLFrame.cpp:584
#25 0x2b489be0 in nsContainerFrame::ReflowChild (this=<value optimized out>, aKidFrame=0x122ef7b0, aPresContext=0x120f6350,
aDesiredSize=@0x7f977b68, aReflowState=@0x7f9779e8, aX=0, aY=0, aFlags=<value optimized out>, aStatus=@0x7f9779c0,
aTracker=0x0) at nsContainerFrame.cpp:771
---Type <return> to continue, or q <return> to quit---
#26 0x2b4a2964 in nsHTMLScrollFrame::ReflowScrolledFrame (this=0x122ef8f0, aState=0x7f977c28,
aAssumeHScroll=<value optimized out>, aAssumeVScroll=<value optimized out>, aMetrics=0x7f977b68,
aFirstPass=<value optimized out>) at nsGfxScrollFrame.cpp:499
#27 0x2b4a2e94 in nsHTMLScrollFrame::ReflowContents (this=0x122ef8f0, aState=0x7f977c28, aDesiredSize=<value optimized out>)
at nsGfxScrollFrame.cpp:593
#28 0x2b4a32b0 in nsHTMLScrollFrame::Reflow (this=0x122ef8f0, aPresContext=<value optimized out>, aDesiredSize=@0x7f977db0,
aReflowState=@0x7f977df0, aStatus=@0x7f977fc4) at nsGfxScrollFrame.cpp:794
#29 0x2b489be0 in nsContainerFrame::ReflowChild (this=<value optimized out>, aKidFrame=0x122ef8f0, aPresContext=0x120f6350,
aDesiredSize=@0x7f977db0, aReflowState=@0x7f977df0, aX=0, aY=0, aFlags=<value optimized out>, aStatus=@0x7f977fc4,
aTracker=0x0) at nsContainerFrame.cpp:771
#30 0x2b4e77d0 in ViewportFrame::Reflow (this=0x122ef724, aPresContext=0x120f6350, aDesiredSize=@0x7f977fd0,
aReflowState=@0x7f978010, aStatus=@0x7f977fc4) at nsViewportFrame.cpp:286
#31 0x2b463a64 in PresShell::DoReflow (this=0x117f0ca8, target=0x122ef724) at nsPresShell.cpp:6227
#32 0x2b467d70 in PresShell::ProcessReflowCommands (this=0x117f0ca8, aInterruptible=0) at nsPresShell.cpp:6333
#33 0x2b467fcc in PresShell::DoFlushPendingNotifications (this=0x117f0ca8, aType=Flush_Layout, aInterruptibleReflow=0)
at nsPresShell.cpp:4534
#34 0x2b4524cc in DocumentViewerImpl::LoadComplete (this=0x11b63e78, aStatus=0) at nsDocumentViewer.cpp:947
#35 0x2b9b1ec0 in nsDocShell::EndPageLoad (this=0x11c66200, aProgress=<value optimized out>, aChannel=0x115c2888, aStatus=0)
at nsDocShell.cpp:5096
#36 0x2b9b4238 in nsWebShell::EndPageLoad (this=0x11c66200, aProgress=0x11c66214, channel=0x115c2888, aStatus=0)
at nsWebShell.cpp:1013
#37 0x2b9ab170 in nsDocShell::OnStateChange (this=0x11c66200, aProgress=0x11c66214, aRequest=0x115c2888,
aStateFlags=<value optimized out>, aStatus=0) at nsDocShell.cpp:5001
#38 0x2b9c1344 in nsDocLoader::FireOnStateChange (this=0x11c66200, aProgress=0x11c66214, aRequest=0x115c2888,
aStateFlags=131088, aStatus=0) at nsDocLoader.cpp:1235
#39 0x2b9c1500 in nsDocLoader::doStopDocumentLoad (this=0x1, request=0x115c2888, aStatus=0) at nsDocLoader.cpp:858
#40 0x2b9c26a4 in nsDocLoader::DocLoaderIsEmpty (this=0x11c66ader.cpp:679
#42 0x2b2e8aa4 in nsLoadGroup::RemoveRequest (this=0x114641c0, request=0x11f47c18, ctxt=0x0, aStatus=0) at nsLoadGroup.cpp:688
---Type <return> to continue, or q <return> to quit---
#43 0x2b5eab7c in nsDocument::DoUnblockOnload (this=0x10ea9380) at nsDocument.cpp:5799
#44 0x2b76882c in nsBindingManager::DoProcessAttachedQueue (this=0x1191fab0) at nsBindingManager.cpp:966
#45 0x2b76a8f8 in nsRunnableMethod<nsBindingManager>::Run (this=<value optimized out>)
at ../../../dist/include/xpcom/nsThreadUtils.h:261
#46 0x2bca4488 in nsThread::ProcessNextEvent (this=0x10060c30, mayWait=1, result=<value optimized out>) at nsThread.cpp:510
#47 0x2bc63520 in NS_ProcessNextEvent_P (thread=0x1, mayWait=2) at nsThreadUtils.cpp:227
#48 0x2bbb4ec4 in nsBaseAppShell::Run (this=0x100e7d20) at nsBaseAppShell.cpp:170
#49 0x2ba31254 in nsAppStartup::Run (this=0x10129948) at nsAppStartup.cpp:181
#50 0x2b283a44 in XRE_main (argc=<value optimized out>, argv=<value optimized out>, aAppData=<value optimized out>)
at nsAppRunner.cpp:3193
#51 0x10001c18 in main ()
In the end, the ABI defines how values are passed from the caller to
the callee. So sometimes it's the toolchains, but usually it is a bug
in the source.
I worked with one of the pre-official versions of 64-bit Mach-O for
PowerPC G5 (not sure if it was like that in the official version that
got released in the end). If the function prototype and the real
function declaration are different, then the callee will read contents
from the wrong registers and thus could get some crazy errors!
In fact, in See MIPS Run, page 329: "Since the function itself and any
callers must make the same decision (or things won’t work), this
depends on having correct function prototypes."
> I know there are two(I am sure that there are more than that) ways to crash it:
> 1. browsing www.sina.com.cn (many other sites seem to work pretty ok though)
> 2. view page source via context menu
>
> The instructions triggered bus error are different in these two cases.
(I have not setup X for my Win2K machine, so I could not test my
hypothesis. Sorry I could not test it myself.)
It's good that there is a stack trace, and by looking at the bug
reports you've filed, seems to me that the version is 3.0.7.
Code in nsTextFrameThebes.cpp:2232 :
gfxFloat* tabs = GetTabWidths(aStart, aLength);
if (tabs) {
for (index = 0; index < aLength; ++index) {
aSpacing[index].mAfter += tabs[index]; <- line 2232
}
aSpacing[] is initialized in a for loop in the beginning of the
function, and the bounds are same for both loops (both are from 0 to
aLength-1). Thus it is likely that the load of tabs[index] is causing
trouble.
Did you find out what gets returned from GetTabWidths()? If the
returned pointer is not valid, then it should be easy to find out who
gives us that pointer by placing a breakpoint inside GetTabWidths()
and then stepping the code...
Rayson
Did that ever work for some ABIs?
Or don't work for any ABI?
Because the problem here is the same source code works in O32, but not in N32.
>
> In fact, in See MIPS Run, page 329: "Since the function itself and any
> callers must make the same decision (or things won’t work), this
> depends on having correct function prototypes."
O32 is not affected by this?
>
>
> > I know there are two(I am sure that there are more than that) ways to crash it:
> > 1. browsing www.sina.com.cn (many other sites seem to work pretty ok though)
> > 2. view page source via context menu
> >
> > The instructions triggered bus error are different in these two cases.
>
> (I have not setup X for my Win2K machine, so I could not test my
> hypothesis. Sorry I could not test it myself.)
>
> It's good that there is a stack trace, and by looking at the bug
> reports you've filed, seems to me that the version is 3.0.7.
>
> Code in nsTextFrameThebes.cpp:2232 :
>
> gfxFloat* tabs = GetTabWidths(aStart, aLength);
> if (tabs) {
> for (index = 0; index < aLength; ++index) {
> aSpacing[index].mAfter += tabs[index]; <- line 2232
> }
>
>
> aSpacing[] is initialized in a for loop in the beginning of the
> function, and the bounds are same for both loops (both are from 0 to
> aLength-1). Thus it is likely that the load of tabs[index] is causing
> trouble.
>
> Did you find out what gets returned from GetTabWidths()? If the
> returned pointer is not valid, then it should be easy to find out who
> gives us that pointer by placing a breakpoint inside GetTabWidths()
> and then stepping the code...
Actually, the exception happens when trying to load aSpacing[index].mAfter to f1
register in coprocessor 1.
(gdb) info registers
zero at v0 v1
R0 0000000000000000 0000000000000001 0000000011fefc20 0000000000000001
a0 a1 a2 a3
R4 0000000000000001 0000000000000002 0000000000000002 0000000000000000
a4 a5 a6 a7
R8 0000000000000000 0000000000000000 0000000000000000 0000000011eb2bac
t0 t1 t2 t3
R12 0000000000000000 4010000000000000 4030000000000000 402999999999999a
s0 s1 s2 s3
R16 000000007f975ce0 0000000000000000 000000007f9745e4 0000000000000002
s4 s5 s6 s7
R20 000000007f9745e4 0000000000000000 0000000000000000 000000007f975388
t8 t9 k0 k1
R24 0000000000000000 000000002b4dc7a0 0000000000000000 0000000000000000
gp sp s8 ra
R28 000000002c1e2dc0 000000007f974400 000000007f9752e4 000000002b4dcefc
sr lo hi bad
000000000400c4f3 0000000000000010 0000000000000000 000000007f9745ec
cause pc
0000000010000010 000000002b4dcf0c
fsr fir
00800044 00000000
Source code:
2227 // Now add tab spacing, if there is any
2228 if (!aIgnoreTabs) {
2229 gfxFloat* tabs = GetTabWidths(aStart, aLength);
2230 if (tabs) {
2231 for (index = 0; index < aLength; ++index) {
2232 aSpacing[index].mAfter += tabs[index];
2233 }
2234 }
2235 }
2236
Disassemble output:
0x2b4dcf00 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+864>: move v1,zero
0x2b4dcf04 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+868>: b 0x2b4dcf34 <_ZN16PropertyProvid
er18GetSpacingInternalEjjPN7gfxFont7SpacingEi+916>
0x2b4dcf08 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+872>: ldc1 $f0,112(s0)
0x2b4dcf0c <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+876>: ldc1 $f1,8(s4)
0x2b4dcf10 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+880>: ldc1 $f0,0(v0)
0x2b4dcf14 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+884>: addiu v0,v0,8
0x2b4dcf18 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+888>: add.d $f0,$f1,$f0
0x2b4dcf1c <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+892>: sdc1 $f0,8(s4)
0x2b4dcf20 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+896>: addiu s4,s4,16
0x2b4dcf24 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+900>: sltu a0,v1,s3
0x2b4dcf28 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+904>: bnez a0,0x2b4dcf0c <_ZN16PropertyPro
vider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+876>
0x2b4dcf2c <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+908>: addiu v1,v1,1
0x2b4dcf30 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+912>: ldc1 $f0,112(s0)
0x2b4dcf34 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+916>: dmtc1 zero,$f1
0x2b4dcf38 <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+920>: c.eq.d $f0,$f1
0x2b4dcf3c <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+924>: bc1t 0x2b4dd160 <_ZN16PropertyProvid
er18GetSpacingInternalEjjPN7gfxFont7SpacingEi+1472>
(gdb) info registers pc
pc: 0x2b4dcf0c
The instruction triggered the exception is:
ldc1 $f1,8(s4)
(gdb) p aSpacing
$14 = (gfxFont::Spacing *) 0x7f9745e4
(gdb) info registers s4
s4: 0x7f9745e4
The bad address is:
000000007f9745ec
But when I tried to access this address using:
p *0x7f9745ec
There is no error.
Any idea?
BTW, an interesting thing to note is that the crash on www.sina.com.cn is also
caused by an "ldc1" instruction.
If you pass values via the stack, then the callee can still get values
from the caller without worrying as much as the register's case.
> Actually, the exception happens when trying to load aSpacing[index].mAfter to f1
> register in coprocessor 1.
Yes, but it still doesn't make sense -- why the loop that clears the
aSpacing[] array in the beginning of the function does not generate
bus errors?
> The instruction triggered the exception is:
> ldc1 $f1,8(s4)
>
> (gdb) p aSpacing
> $14 = (gfxFont::Spacing *) 0x7f9745e4
>
> (gdb) info registers s4
> s4: 0x7f9745e4
So it is doing a load double from address 0x7f9745e4 + 8. And I just
checked, aSpacing[index].mAfter is gfxFloat (double), so the load
instruction is correct.
However, the address is 0x7f9745e4 (= 2140620260), which is not a
multiple of 8 (2140620260 / 8 = 267577532.5)!!
I think there are 2 things to check at this point:
1) find out what gets generated by the compiler (w/ -S) for the
beginning of GetSpacingInternal() -- why the for loop executes fine.
2) *aSpacing is passed via the call chain (some static functions get
inlined by the compiler, thus not showing up in the gdb stack output)
from GetAdjustedSpacingArray(). The value is computed by:
aSpacing->Elements() + aSpacingStart - aStart
Just a guess... may be the programmer means "aSpacing->Elements() +
sizeof(gfxFont::Spacing)*(aSpacingStart - aStart)"??
Rayson
Ah, seems I am not sensitive enough to this kind of problem.
This is something worth further investigating.
Since it is already late here, I will continue the investigation tomorrow.
Thank you so much for your help on this issue so far! ;)
> I think there are 2 things to check at this point:
>
> 1) find out what gets generated by the compiler (w/ -S) for the
> beginning of GetSpacingInternal() -- why the for loop executes fine.
>
> 2) *aSpacing is passed via the call chain (some static functions get
> inlined by the compiler, thus not showing up in the gdb stack output)
> from GetAdjustedSpacingArray(). The value is computed by:
> aSpacing->Elements() + aSpacingStart - aStart
>
> Just a guess... may be the programmer means "aSpacing->Elements() +
> sizeof(gfxFont::Spacing)*(aSpacingStart - aStart)"??
>
> Rayson
--
So, ldc1 from an unaligned address is a sure way to trigger bus error.
If compiler could figure out the address is unaligned before hand, it
is able to issue correct ldl/ldr instructions sequence to load the variable.
However, it seems that the compiler is not smart enough or it is not given
enough hints in this case.
Anyway, making the variable aligned is the better way to go.
>
> > I think there are 2 things to check at this point:
> >
> > 1) find out what gets generated by the compiler (w/ -S) for the
> > beginning of GetSpacingInternal() -- why the for loop executes fine.
ldc1 is not used when assigning values to aSpacing.
for (index = 0; index < aLength; ++index) {
6f4c: afc0002c sw zero,44(s8)
6f50: 1000000e b 6f8c <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+0x7c>
6f54: 00000000 nop
/var/tmp/portage/net-libs/xulrunner-1.9.0.7/work/mozilla/layout/generic/nsTextFrameThebes.cpp:2193
aSpacing[index].mBefore = 0.0;
6f58: 8fc2002c lw v0,44(s8)
6f5c: 00021100 sll v0,v0,0x4
6f60: 8fc3010c lw v1,268(s8)
6f64: 00621021 addu v0,v1,v0
6f68: fc400000 sd zero,0(v0)
/var/tmp/portage/net-libs/xulrunner-1.9.0.7/work/mozilla/layout/generic/nsTextFrameThebes.cpp:2194
aSpacing[index].mAfter = 0.0;
6f6c: 8fc2002c lw v0,44(s8)
6f70: 00021100 sll v0,v0,0x4
6f74: 8fc3010c lw v1,268(s8)
6f78: 00621021 addu v0,v1,v0
6f7c: fc400008 sd zero,8(v0)
> >
> > 2) *aSpacing is passed via the call chain (some static functions get
> > inlined by the compiler, thus not showing up in the gdb stack output)
> > from GetAdjustedSpacingArray(). The value is computed by:
> > aSpacing->Elements() + aSpacingStart - aStart
> >
> > Just a guess... may be the programmer means "aSpacing->Elements() +
> > sizeof(gfxFont::Spacing)*(aSpacingStart - aStart)"??
> >
The value aSpacing->Elements() returned is a pointer, so
sizeof(gfxFont::Spacing)* is not needed. Actually, at the time of crashing,
aSpacingStart and aStart are all zero, and the value aSpacing->Elements()
returned is already unaligned.
http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTArray.h
Please check the Elements() function in the above file.
Also buffer "mAutoBuf" at the bottom of the file also demonstrates this.
I modified the Elements() function and the layout of mAutoBuf to make the
starting address of elements aliagned. Now it is still compiling.
Hope this could make it work.
BTW, those ldl/ldr instructions and friends are costing ICT and other
MIPS licensees millions of dollars -- because those instructions are
patented by MIPS Technologies :(
Implementing those in silicon and not paying MIPS Technologies Inc. = lawsuit!
(What bugs me the most is that CISC processors supported unaligned
memory load/stores years before MIPS was born!)
> ldc1 is not used when assigning values to aSpacing.
> for (index = 0; index < aLength; ++index) {
> 6f4c: afc0002c sw zero,44(s8)
> 6f50: 1000000e b 6f8c <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+0x7c>
> 6f54: 00000000 nop
> /var/tmp/portage/net-libs/xulrunner-1.9.0.7/work/mozilla/layout/generic/nsTextFrameThebes.cpp:2193
> aSpacing[index].mBefore = 0.0;
> 6f58: 8fc2002c lw v0,44(s8)
> 6f5c: 00021100 sll v0,v0,0x4
> 6f60: 8fc3010c lw v1,268(s8)
> 6f64: 00621021 addu v0,v1,v0
> 6f68: fc400000 sd zero,0(v0)
> /var/tmp/portage/net-libs/xulrunner-1.9.0.7/work/mozilla/layout/generic/nsTextFrameThebes.cpp:2194
> aSpacing[index].mAfter = 0.0;
> 6f6c: 8fc2002c lw v0,44(s8)
> 6f70: 00021100 sll v0,v0,0x4
> 6f74: 8fc3010c lw v1,268(s8)
> 6f78: 00621021 addu v0,v1,v0
> 6f7c: fc400008 sd zero,8(v0)
Interesting... the compiler assumes the store is unaligned at the
beginning of the function, and then it decides that the access is
aligned later on -- for the same variable.
(May be we should file a bug report -- gcc is either not right from
the functional point of view, or for the case of aligned
aSpacing[].mAfter, it is not generating efficient code.)
> The value aSpacing->Elements() returned is a pointer, so
> sizeof(gfxFont::Spacing)* is not needed.
I was a bit worried that the compiler is not smart enough to convert
the type from char [] to double.
> I modified the Elements() function and the layout of mAutoBuf to make the
> starting address of elements aliagned.
I think making it a long long would work as well. Or are you going to
use a "#pragma align" for mAutoBuf?
Anyway, using reinterpret_cast to cast from char[] to double would not
play well with alignment rules. It works elsewhere because 99% of the
Firefox installations are x86/x64 :P
> Now it is still compiling. Hope this could make it work.
BTW, sometimes I use workarounds to test fixes for unalignment
problems if the *real fix* is in the header file:
- flags or directive to instruct the compiler to generate unaligned
memory operations : __unaligned (MS Visual Studio), -xmemalign=4i (Sun
Studio -- SPARC)
- use memcpy() : for this case:
for (index = 0; index < aLength; ++index)
{
double temp;
memcpy(&temp, &aSpacing[index].mAfter, sizeof(double));
temp += tabs[index];
memcpy(&aSpacing[index].mAfter, &temp, sizeof(double));
}
Then if it works, I would then apply the fix in the header... and
start compiling. May be useful on Loongson because compiling anything
takes so long!!
And thanks for looking into this, it's 4am in the morning in HK &
China... don't you need to get some sleep?? :D
Rayson
ST already got the license, actually
http://www.eetasia.com/ART_8800458955_499495_NT_921affe8.HTM
>
> (What bugs me the most is that CISC processors supported unaligned
> memory load/stores years before MIPS was born!)
>
>
> > ldc1 is not used when assigning values to aSpacing.
> > for (index = 0; index < aLength; ++index) {
> > 6f4c: afc0002c sw zero,44(s8)
> > 6f50: 1000000e b 6f8c <_ZN16PropertyProvider18GetSpacingInternalEjjPN7gfxFont7SpacingEi+0x7c>
> > 6f54: 00000000 nop
> > /var/tmp/portage/net-libs/xulrunner-1.9.0.7/work/mozilla/layout/generic/nsTextFrameThebes.cpp:2193
> > aSpacing[index].mBefore = 0.0;
> > 6f58: 8fc2002c lw v0,44(s8)
> > 6f5c: 00021100 sll v0,v0,0x4
> > 6f60: 8fc3010c lw v1,268(s8)
> > 6f64: 00621021 addu v0,v1,v0
> > 6f68: fc400000 sd zero,0(v0)
> > /var/tmp/portage/net-libs/xulrunner-1.9.0.7/work/mozilla/layout/generic/nsTextFrameThebes.cpp:2194
> > aSpacing[index].mAfter = 0.0;
> > 6f6c: 8fc2002c lw v0,44(s8)
> > 6f70: 00021100 sll v0,v0,0x4
> > 6f74: 8fc3010c lw v1,268(s8)
> > 6f78: 00621021 addu v0,v1,v0
> > 6f7c: fc400008 sd zero,8(v0)
>
> Interesting... the compiler assumes the store is unaligned at the
> beginning of the function, and then it decides that the access is
> aligned later on -- for the same variable.
Actually, I don't think compiler knows the store is unaligned at the beginning.
It is just that the value to be assigned is zero, so no need to go through c1
processor. And sd works on unaligned address in Loongson.
Here is a little demonstration:
zhangle@2f ~ (n32) $ cat sigbus.c
#include <stdio.h>
struct foo {
int i;
long long d;
} __attribute__((packed)) bar = { 1, 2 };
int main(int argc, char **argv) {
bar.d = 3;
printf("%lu\n", bar.d);
}
zhangle@2f ~ (n32) $ gcc sigbus.c -S
zhangle@2f ~ (n32) $ cat sigbus.s
...
main:
.frame $fp,32,$31 # vars= 16, regs= 2/0, args= 0, gp= 0
.mask 0xc0000000,-8
.fmask 0x00000000,0
.set noreorder
.set nomacro
addiu $sp,$sp,-32
sd $31,24($sp)
sd $fp,16($sp)
move $fp,$sp
sw $4,0($fp)
sw $5,4($fp)
lui $2,%hi(bar)
addiu $2,$2,%lo(bar)
li $3,3 # 0x3
sdl $3,11($2)
sdr $3,4($2)
...
If we substitute the last two insns with "sd $3, 4($2)", it works just the same.
If substitue with "ldc1 $f0, 4($2)", it will receive bus error.
>
> (May be we should file a bug report -- gcc is either not right from
> the functional point of view, or for the case of aligned
> aSpacing[].mAfter, it is not generating efficient code.)
>
>
> > The value aSpacing->Elements() returned is a pointer, so
> > sizeof(gfxFont::Spacing)* is not needed.
>
> I was a bit worried that the compiler is not smart enough to convert
> the type from char [] to double.
>
>
> > I modified the Elements() function and the layout of mAutoBuf to make the
> > starting address of elements aliagned.
>
> I think making it a long long would work as well. Or are you going to
> use a "#pragma align" for mAutoBuf?
>
> Anyway, using reinterpret_cast to cast from char[] to double would not
> play well with alignment rules. It works elsewhere because 99% of the
> Firefox installations are x86/x64 :P
I made a mistake. I thought it was the pointer to header which is in the
beginning of the buffer. So it didn't work.
Now I just added a Padding to Header:
struct Header {
PRUint32 mLength;
PRUint32 mCapacity : 31;
PRUint32 mIsAutoArray : 1;
PRUint32 Padding;
};
The compilation is already started. I wish I could know the following
techniques earlier, :)
>
> > Now it is still compiling. Hope this could make it work.
>
> BTW, sometimes I use workarounds to test fixes for unalignment
> problems if the *real fix* is in the header file:
>
> - flags or directive to instruct the compiler to generate unaligned
> memory operations : __unaligned (MS Visual Studio), -xmemalign=4i (Sun
> Studio -- SPARC)
>
> - use memcpy() : for this case:
>
> for (index = 0; index < aLength; ++index)
> {
> double temp;
>
> memcpy(&temp, &aSpacing[index].mAfter, sizeof(double));
> temp += tabs[index];
> memcpy(&aSpacing[index].mAfter, &temp, sizeof(double));
> }
>
> Then if it works, I would then apply the fix in the header... and
> start compiling. May be useful on Loongson because compiling anything
> takes so long!!
These are very useful techniques, thanks!
>
> And thanks for looking into this, it's 4am in the morning in HK &
> China... don't you need to get some sleep?? :D
I just watched Inter VS. Sampdoria, Italy Tim Cup semi-final. :)
Damn, another silly mistake, the original Header should be 8 bytes aligned.
So padding is not required.
But the fact that pointer to Header plus 1 is not 8 bytes aligned can't be
explained.
Anyway, I will try the memcpy method first.
>
> The compilation is already started. I wish I could know the following
> techniques earlier, :)
>
> >
> > > Now it is still compiling. Hope this could make it work.
> >
> > BTW, sometimes I use workarounds to test fixes for unalignment
> > problems if the *real fix* is in the header file:
> >
> > - flags or directive to instruct the compiler to generate unaligned
> > memory operations : __unaligned (MS Visual Studio), -xmemalign=4i (Sun
> > Studio -- SPARC)
> >
> > - use memcpy() : for this case:
> >
> > for (index = 0; index < aLength; ++index)
> > {
> > double temp;
> >
> > memcpy(&temp, &aSpacing[index].mAfter, sizeof(double));
> > temp += tabs[index];
> > memcpy(&aSpacing[index].mAfter, &temp, sizeof(double));
> > }
> >
> > Then if it works, I would then apply the fix in the header... and
> > start compiling. May be useful on Loongson because compiling anything
> > takes so long!!
>
--
I know, I added a section (
http://en.wikipedia.org/wiki/Loongson#MIPS_patent_issues ) in the
Loongson article a few weeks ago.
However, that means every chip bought from ST shares the cost of the
license because of stupid patents!
> I just watched Inter VS. Sampdoria, Italy Tim Cup semi-final. :)
Ha, I guess 6 seasons of 24 were all good :D
I watched 24 Redemption as well...
>> Now I just added a Padding to Header:
>> struct Header {
>> PRUint32 mLength;
>> PRUint32 mCapacity : 31;
>> PRUint32 mIsAutoArray : 1;
>> PRUint32 Padding;
>> };
>
> Damn, another silly mistake, the original Header should be 8 bytes aligned.
> So padding is not required.
The header might not be 8 bytes aligned:
#include <stdio.h>
int a; /* removing a will make mLength 16 bytes aligned */
struct Header {
int mLength;
int mCapacity : 31;
int mIsAutoArray : 1;
} Header; /* __attribute__ ((aligned (16))); */
main()
{
struct Header *p;
p = &Header + 1;
printf("%p\n", &a);
printf("%p\n", &Header.mLength);
printf("%p\n", p);
}
taijia rayson # ./a.out
0x440bb0
0x440bb4
0x440bbc
0x440bb4 =4459444, 4459444 / 4 = 557430.5
> But the fact that pointer to Header plus 1 is not 8 bytes aligned can't be
> explained.
If Header is not 8-byte aligned, then Header+1 (1 = 8 bytes) won't be
8-byte aligned.
In the example above, 0x440bb4+8 = 0x440bbc ...
>
> Anyway, I will try the memcpy method first.
In the example above, I use __attribute__ aligned. Enabling it:
taijia rayson # ./a.out
0x440bb0
0x440bc0
0x440bc8
0x440bc0 = 4459456, 4459456/8 = 557432.
Rayson
>
What about season 7? ;) It is nearly finished.
And I have almost finished Friends, all 10 seasons, ;)
>
>
> >> Now I just added a Padding to Header:
> >> struct Header {
> >> PRUint32 mLength;
> >> PRUint32 mCapacity : 31;
> >> PRUint32 mIsAutoArray : 1;
> >> PRUint32 Padding;
> >> };
> >
> > Damn, another silly mistake, the original Header should be 8 bytes aligned.
> > So padding is not required.
>
> The header might not be 8 bytes aligned:
>
> #include <stdio.h>
>
> int a; /* removing a will make mLength 16 bytes aligned */
This is a good point. I am thinking of this two.
I have removed #ifdef DEBUG around DebugGetHeader(), and re-compiling.
I want to see the address of Header.
BTW, I just got distcc working, again.
It didn't work because I have used -march=native before.
--
The header IS not 8 bytes aligned.
And I think that's because there is a pointer to the header immediately before
it.
However, neither adding another pointer immediately before the header nor adding
__attribute__ ((aligned (8))) to mAutoBuf would work. I got something like:
*** glibc detected *** /usr/lib/mozilla-firefox/firefox: free(): invalid pointer: 0x100c3bb4 ***
Anyway, I still haven't tried memcpy, I will try it now.
My root partition just crashed, it was an ext4.
Just restore it from a backup.
Now it is compiling.
Just realized it is because of this function:
Header* GetAutoArrayBuffer() {
NS_ASSERTION(IsAutoArray(), "Should be an auto array to call this");
return reinterpret_cast<Header*>(&mHdr + 1);
It is working now!
I just added a line:
Header *padding;
between line 170 and line 171
http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTArray.h
Good!! :D
But something I don't fully understand -- why padding "Header *mHdr"
would make a difference here. "Header *mHdr" should always be 4-byte
aligned, making it 8-byte aligned would not make a difference on
32-bit systems.
The code in GetAutoArrayBuffer() is returning &mHdr+1, but what is
actually returned?? The address of the next element of mHdr?? But mHdr
is already the last variable of class nsTArray_base.
Also, with padding or not, the value of mHdr should not make a
difference, it's just a pointer.
Lastly, GetAutoArrayBuffer() returns the "Header*" type -- if padding
makes a difference, does that mean the memory address is used to store
8 bytes values?? "Header *mHdr" is only 4 bytes in size!
Is it a bug in Firefox or there is something I still don't get??
Thanks,
Rayson
Please note that the padding pointer is added before mHdr.
>
> The code in GetAutoArrayBuffer() is returning &mHdr+1, but what is
> actually returned?? The address of the next element of mHdr?? But mHdr
> is already the last variable of class nsTArray_base.
The memory layout is like this:
| mHdr | struct Header | array of elements |
+-4 bytes-+----8 bytes----+-------------------+
so &mHdr + 1 is just the address of struct Header, which is also the starting
address of AutoArrayBuffer.
while mHdr + 1 is the starting address of the array.
>
> Also, with padding or not, the value of mHdr should not make a
> difference, it's just a pointer.
It does make a difference. Since if mHdr is at an address of a multiply of 4,
then struct Header and the array of elements are starting at an address of a
multiply of 8.
>
> Lastly, GetAutoArrayBuffer() returns the "Header*" type -- if padding
> makes a difference, does that mean the memory address is used to store
> 8 bytes values?? "Header *mHdr" is only 4 bytes in size!
The returned value points to a struct Header as shown above.
Sounds good. Thanks for looking into this!!
So now I agree that this is correct fix. However, from a software
maintenance point of view, the fix (the padding) is a bit
counterintuitive - we want to make mHdr not 8-byte aligned so that
AutoArrayBuffer is 8-byte aligned!?
Is there a way to change the code so that we can get to
AutoArrayBuffer (struct Header) without computing mHdr+1?? To me, it
sounds like if a programmer adds or removes something in the class,
then this is going to break again.
Rayson
Actually, here should be another condition: but not a multiply of 8, :)
> > then struct Header and the array of elements are starting at an address of a
> > multiply of 8.
>
> Sounds good. Thanks for looking into this!!
>
> So now I agree that this is correct fix. However, from a software
> maintenance point of view, the fix (the padding) is a bit
> counterintuitive - we want to make mHdr not 8-byte aligned so that
> AutoArrayBuffer is 8-byte aligned!?
I agree. So I think this "fix" may not will be accepted by upstream.
This is more of a proof of concept than a fix.
>
> Is there a way to change the code so that we can get to
> AutoArrayBuffer (struct Header) without computing mHdr+1?? To me, it
> sounds like if a programmer adds or removes something in the class,
> then this is going to break again.
Yeah, totally.
I will post to gcc and mozilla's mailing list, ask people there what could be
the final solution.
Cool !
That's true it's working !
Thanks a lot !!!
r1
Let us know the suggestions from the mozilla guys (IMO, gcc can't do
much for code like this). I think besides MIPS, SPARC will get bus
error as well. Another issue is that even on x86, it is slower to
access unaligned memory addresses.
Rayson
The misaligned class member is a gfxPoint, which is a struct
containing two float. Sometimes, this gfxPoint may be placed on a 4 bytes
boundary, but not evenly divisible by 8. And that's the reason of the crashing.
Immediately before this class member there is a pointer. If I add __attribute__
((aligned (8))) to this pointer, no matter where this pointer resides in the
memory, the compiler adds 4 bytes after this pointers. If I set the alignment to
16, then 12 bytes padding will be added. This is quite unexpected.
Adding __attribute__ ((aligned (alignment))) to the class seems has no effect.
Switching the gfxPoint and the pointer's location in the class won't work
either. I think that's because this class sometimes starts at an address which
is a multiply of 8, and sometimes not.
So I think maybe we still need to add a padding pointer here.
I have modified the source, and compiling now.
Just realized this should have the same effect as __attribute__ ((aligned (8))).
Another thing worth noting though, xulrunner has hard coded gcc option -Os. And
I think maybe that's the cause why __attribute__ ((aligned (alignment))) does
not work as expected..
Interesting... which source file is it, and can you provide a diff for
your code change??
Rayson
./layout/svg/base/src/nsSVGGlyphFrame.h
around line 217
I have tried switching the position of gfxPoint mPosition and gfxTextRun *mTextRun
adding __attribute__ ((aligned (8))) to them, and adding __attribute__ ((aligned (8)))
to this class definition.
None of them would work.
I am thinking maybe we should implemente a signal handler to emulate this
instruction.
Thanks for the reply.
Do you have the stack trace for the new bus error?? As long as there
are no 8-byte stores to the address of gfxPoint, then whether gfxPoint
is 4-byte but not 8-byte aligned or not should not be a problem.
Rayson
The cause is just an sdc1 instruction.
Would be great if you can also provide the stack trace...
Rayson
I *think* this crash can happen when the nsSVGGlyphFrame object is
created dynamically in NS_NewSVGGlyphFrame()
(layout/svg/base/src/nsSVGGlyphFrame.cpp). If the "new" operator (it's
programmer overloaded) is not aware of the memory alignment
requirements and does not construct the nsSVGGlyphFrame object on non
8-byte aligned addresses while sometimes it does, then not matter how
we pad or align the nsSVGGlyphFrame class members, we will get bus
errors.
Rayson
That's why I am resorting to kernel exception handler now.