Bug 12382 - Crash on AMD64 in dtoa.cpp
Summary: Crash on AMD64 in dtoa.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-23 09:19 PST by Jan Krämer
Modified: 2007-01-26 06:16 PST (History)
1 user (show)

See Also:


Attachments
a small naive patch (270 bytes, patch)
2007-01-23 09:26 PST, Jan Krämer
no flags Details | Formatted Diff | Diff
a simple naive patch (798 bytes, patch)
2007-01-25 10:03 PST, Jan Krämer
no flags Details | Formatted Diff | Diff
same patch...better changelog (909 bytes, patch)
2007-01-25 10:34 PST, Jan Krämer
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Krämer 2007-01-23 09:19:22 PST
When trying out WebKit/Qt ( on my AMD64 machine I got various crashes in
kjs_dtoa at dtoa.cpp:3015
*s++ = '0' + (int)L;

The problem seems to that the containing loop does not terminate correctly, as at the time of the crash the loop was in its 36021 iteration.

The kjs version in the KDE repository had a change applied:
http://websvn.kde.org/trunk/KDE/kdelibs/kjs/dtoa.cpp?rev=569434&r1=564344&r2=569434

Following the change in the WebKit "dtoa.cpp" version seems to fix the crash.

See also the comment in dtoa.cpp:
//#define Long int on machines with 32-bit ints and 64-bit longs.
Comment 1 Jan Krämer 2007-01-23 09:26:53 PST
Created attachment 12627 [details]
a small naive patch
Comment 2 Lars Knoll 2007-01-25 05:34:08 PST
Makes absolutely sense. We're using the same code in Qt and have applied the same fix there. 

I'd r+ it if I had the right to do so ;-)
Comment 3 Jan Krämer 2007-01-25 10:03:14 PST
Created attachment 12666 [details]
a simple naive patch

Added change log entry and request review :-)

I could not test for regressions, since it just did not run on my computer before this change.

If there is a supported platform that has int < 32bit, stdint.h would need to be included, and the patch changed to:
#define Long int32_t
Comment 4 Jan Krämer 2007-01-25 10:34:11 PST
Created attachment 12668 [details]
same patch...better changelog

Added the reason for the change to the changelog.
(I had it in there before a i did a revert on the changelog...and then forgot to add it again)
Comment 5 Maciej Stachowiak 2007-01-25 22:10:00 PST
Comment on attachment 12668 [details]
same patch...better changelog

r=me
Comment 6 David Kilzer (:ddkilzer) 2007-01-26 06:16:51 PST
Committed by lars in r19136.