From: Mark Gilbert (markgilbert@hotpop.com)
Date: Sun Feb 16 2003 - 15:56:36 EST
Thank you very much for bringing this to our attention. Hub will prolly
commit before I am back online.
You are a gift
-MG
On Sun, 2003-02-16 at 14:46, Johnny Lee wrote:
> HEAD has the fix, but STABLE seems to have only fixed the code for the DEBUG
> build, which isn't very helpful for 99% of the userbase.
>
> Patch is also attached as 'patch2715.txt' in case Hotmail severely munges
> the unified diff patch below.
>
> For abi\src\wp\ap\xp\ap_TopRuler.cpp:
>
> --- ap_TopRuler.cpp.old Sun Feb 16 13:59:27 2003
> +++ ap_TopRuler.cpp Sun Feb 16 13:52:45 2003
> @@ -1398,11 +1398,9 @@
>
> fl_BlockLayout * pBL = (static_cast<FV_View
> *>(m_pView))->getCurrentBlock();
> UT_ASSERT (pBL);
> -#if DEBUG
> if (pBL == NULL) {
> return false;
> }
> -#endif
> bool bRTLpara = pBL->getDominantDirection() == FRIBIDI_TYPE_RTL;
>
> if(bRTLpara)
>
>
> =================================================================
>
> Long description:
> ------------------------------------------------------------
>
> I read the description for bug 2715.
>
> I was able to repro the bug in the released AbiWord 1.0.4.
>
> I tried to repro in my debug build of the 1.0.4 source code, but I never got
> a crash.
>
> I was able to come up with reproable steps for the bug in the release build:
>
> Repro Steps:
>
> 1. Open the attachment from comment # 65 of bug 2715,
> <http://bugzilla.abisource.com/show_bug.cgi?id=2715#c65>.
>
> 2. From the Zoom submenu under the View menu, select the "Zoom to 50%" menu
> item.
>
> 3. In the Zoom combobox on the toolbar, select the 75% zoom level with the
> mouse.
>
> Result:
> Crash.
>
> But the debug build never crashed.
>
> I was going to have to do this the hard way...
>
> I fired up the debugger on the release build of AbiWord 1.0.4 and set the
> debugger to always stop for access violation exceptions.
>
> I reproed the bug and looked at the crash under the debugger.
>
>
> Here's the disassembly, crash is at 0x004343BE, eax == 0:
>
> ------------------------------------------------------------------
> 00434383 89 44 24 34 mov dword ptr [esp+34h],eax
> 00434387 51 push ecx
> 00434388 57 push edi
> 00434389 8B CE mov ecx,esi
> 0043438B E8 F0 22 00 00 call 00436680
> 00434390 8B 8E 88 00 00 00 mov ecx,dword ptr [esi+88h]
> 00434396 8D 54 24 23 lea edx,[esp+23h]
> 0043439A 03 C8 add ecx,eax
> 0043439C 52 push edx
> 0043439D 68 5C 79 5B 00 push 5B795Ch
> 004343A2 89 44 24 24 mov dword ptr [esp+24h],eax
> 004343A6 89 4C 24 20 mov dword ptr [esp+20h],ecx
> 004343AA E8 41 70 FD FF call 0040B3F0
> 004343AF 8B C8 mov ecx,eax
> 004343B1 E8 1A 79 FD FF call 0040BCD0
> 004343B6 8B 4E 24 mov ecx,dword ptr [esi+24h]
> 004343B9 E8 12 24 0B 00 call 004E67D0
>
> vvvvvvvvvvvvvvvvvvvvvvvvv
>
> 004343BE 81 B8 E0 00 00 00 11 cmp dword ptr [eax+0E0h],111h
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 004343C8 0F 94 C0 sete al
> 004343CB 84 C0 test al,al
> 004343CD 88 44 24 13 mov byte ptr [esp+13h],al
> 004343D1 74 10 je 004343E3
> 004343D3 8B 44 24 18 mov eax,dword ptr [esp+18h]
> 004343D7 8B 4C 24 24 mov ecx,dword ptr [esp+24h]
> 004343DB 2B C1 sub eax,ecx
> 004343DD 89 44 24 14 mov dword ptr [esp+14h],eax
> 004343E1 EB 0E jmp 004343F1
> 004343E3 8B 4C 24 24 mov ecx,dword ptr [esp+24h]
> 004343E7 8B 44 24 1C mov eax,dword ptr [esp+1Ch]
> 004343EB 2B C8 sub ecx,eax
> ------------------------------------------------------------------
>
> After chasing down several goose chases, I noticed a unique piece of
> assembly language:
>
> 0043439D 68 5C 79 5B 00 push 5B795Ch
>
> This isn't common. So it's either a constant or a data reference.
>
> In the debugger, I looked to see what was at memory location 0x005B795C.
>
>
> 005B795C 44 65 66 61 75 6C 74 44 69 72 65 63 74 69 6F 6E 52
> DefaultDirectionR
> 005B796D 74 6C 00 2C 00 00 00 74 65 78 74 2D 69 6E 64 65 6E
> tl.,...text-inden
>
> Hmm. "DefaultDirectionRtl"?! That's unique and should be easy to track down.
>
> Searching the code for this string revealed that it was in the BIDI builds.
> But I was only building the debug non-BIDI code.
>
> Looking at all the code that used the string revealed one bit of code which
> looked like the culprit.
>
>
> In abi\src\wp\ap\xp\ap_TopRuler.cpp, ~line 1400:
>
> #ifdef BIDI_ENABLED
> UT_sint32 xAbsRight = xAbsLeft + m_infoCache.u.c.m_xColumnWidth;
> bool bRTLglobal;
> XAP_App::getApp()->getPrefsValueBool((XML_Char*)AP_PREF_KEY_DefaultDirectionRtl,
> &bRTLglobal);
>
> fl_BlockLayout * pBL = (static_cast<FV_View
> *>(m_pView))->getCurrentBlock();
> UT_ASSERT (pBL);
> #if DEBUG
> if (pBL == NULL) {
> return false;
> }
> #endif
> bool bRTLpara = pBL->getDominantDirection() == FRIBIDI_TYPE_RTL;
>
>
> The "#if DEBUG" got my attention.
>
> That would explain why my debug build never crashed even when I build the
> BIDI version.
>
> Looking at the CVS HEAD branch of ap_TopRuler.cpp, there was a checkin by
> tomas_f back in July 2002 which had a comment about fixing bug 2715. But the
> description for bug 2715 said that the fix had been backported.
>
> I took a look at the source code for the nightly build of the STABLE branch
> and ap_TopRuler.cpp was the same as mine. So it looks like tomas_f's fix was
> not in the STABLE branch.
>
> The patch above is equivalent to tomas_f's patch in the release build.
>
>
> _________________________________________________________________
> Protect your PC - get McAfee.com VirusScan Online
> http://clinic.mcafee.com/clinic/ibuy/campaign.asp?cid=3963
This archive was generated by hypermail 2.1.4 : Sun Feb 16 2003 - 15:50:20 EST