214210 - crash in JS_strdup ()

Status: VERIFIED DUPLICATE of bug 214761

(Core :: JavaScript Engine, defect, P1)

Description

[R.K.Aa.]

With a day old trunk CVS, Linux, i crash every time i maneuvre around in a story
at http://www.dinside.no/php/art.php?id=91176

It was much harder to crash when i ran moz in gdb, but managed to get the
attached backtrace after some back and forth.

To reproduce:

Load http://www.dinside.no/php/art.php?id=91176
Click the link "Motor og kjøreegenskaper"
If that loads ok, click the next link; "Plass og komfort"
And if that too loads ok, click the next link again; "Konklusjon og konkurrenter"
...and the next... etc.. untill crash.

I crash on the second or third link when i run mozilla normally, and can
reproduce the crash each time. The crash MAY be related to wheelscrolling the
page while it loads.

I am not able to reproduce the crash with 1.4

Comment 1

[R.K.Aa.]

Attachment: backtrace, non-debug

Comment 2

[Phil Schwartau]

Probably more fallout from the bug 208030 checkin on 2003-07-26.
Reassigning and adding this bug as a dependency.

Let's see what happens here when bug 208030 is fixed.

Comment 3

[Brendan Eich [:brendan]]

I'm gonna assert that this is a dup of the reopened bug 208030.  I backed out
the offending small part of the (followup) checkin for that bug, so this
shouldn't be reproducing off today's official builds.

/be

*** This bug has been marked as a duplicate of 208030 ***

Comment 4

[Phil Schwartau]

[e-mail]: could you try the latest build and report back?
If the crash is gone, please mark this bug Verified. If the crash
is still there, you can reopen this; thanks -

Comment 5

[R.K.Aa.]

Latest official build 2003072810 still crashes.
But was checkin you mention done at 07/28/2003 11:30 ?
If so, perhaps it's not yet in the official linux build.

I submitted a talkback of the crash some minutes ago, FWIW.
(With reference to this bug #)

Comment 6

[Phil Schwartau]

Attachment: Latest Talkback incident

Comment 7

[Phil Schwartau]

> But was checkin you mention done at 07/28/2003 11:30 ?
> If so, perhaps it's not yet in the official linux build.

[e-mail]: you are right; sorry I spoke too soon.
Could you try tomorrow's build and see how it goes? Thanks -

Comment 8

[R.K.Aa.]

Sorry. I clobbered, pulled and rebuilt: It still crashes.
Reopening.

Comment 9

[Brendan Eich [:brendan]]

Taking.

/be

Comment 10

[Brendan Eich [:brendan]]

Attachment: proposed fix

The real fix is in jsemit.h, CG_COUNT_FINAL_SRCNOTES.  The rest is symmetry,
consistent casting from (void*), and reindentation.  And of course the back-out
of today's earlier partial back-out in jsscript.c.

/be

Comment 11

[Brendan Eich [:brendan]]

I checked in after testing this quite a bit (with some added sanity checks that
I am not checking in).  It fixes the impurity I checked in on Sunday, and backed
out yesterday from jsscript.c only partly.  The bug was that the extra
SRC_XDELTA notes needed to count the distance in bytecodes from the last prolog
note to the SRC_SETLINE one emitted by js_FinishTakingSrcNotes were not counted
by CG_COUNT_FINAL_SRCNOTES.

But with the big Saturday checkin, 3 bytes of slop were added to the end of the
allocation containing all srcnotes.  This was usually enough to absorb the
extra, unaccounted-for SRC_XDELTA notes.  But it still wasn't in all cases. 
Depending on malloc heap state and how it evolved, different impurities would
result.

The 3 bytes are needed for alignment of script->trynotes, only if the script has
try/catch statements.  Obviously, nothing should depend on alignment padding for
correct operation!

The patch in this bug correctly accounts for all the needed space, so it can be
preallocated in one big hunk (the entire JSScript, its bytecode, srcnotes, and
any trynotes).

I still don't have active reviewers available for approving JS changes.

/be

Comment 12

[Phil Schwartau]

Brendan: is this ready to be marked fixed, then?

Comment 13

[R.K.Aa.]

Built with the fix: No more crash.

Comment 14

[Brendan Eich [:brendan]]

Yup, fixed.  The other bugs blocked by bug 208030 should be closed too if people
can verify they're fixed in today's builds.

/be

Comment 15

[Phil Schwartau]

Marking Verified per rkaa's Comment #13 -

Comment 16

[R.K.Aa.]

well that's strange. I just crashed 5 times in a row with the same stack and a
new build. At http://www.dinside.no/php/art.php?id=91436
Then i could suddenly NOT make it crash there. The page is dynamic, several adds
change over time. Well. Can't reproduce now, but there may be some cruft left.

Comment 17

[R.K.Aa.]

and again. 2003080305 crashes with the stack at http://www.digi.no
Talkback doesn't trigger, but i've crashed 7 times in a row now.
Deleting cache has no effect, nor XUL.mfl for that matter.

Comment 18

[R.K.Aa.]

i discovered that this is profile dependant. A new profile doesn't crash there.
My old profile does. (who broke profile manager btw...  it starts with
"previously used profile" without displaying options anymore)

Comment 19

[R.K.Aa.]

more nagging :)

Forgot: Allthough the crash is profile dependant with a fresh build, mozilla 1.4
does NOT crash when using same profile that *will* crash a fresh 1.5.
So something has changed.

Comment 20

[Phil Schwartau]

rkaa: can you do a diff of your new profile vs. the old profile
and attach or paste the results here? Thanks -

Comment 21

[Phil Schwartau]

Compare bug 214761 comment 4, where this pref is found to cause a crash:

     Edit > Preferences > Debug > Show strict JavaScript warnings 


It can be set by the Edit > Preferences GUI. Behind the scenes, here is
how the pref appears in the prefs file (user.js, all.js, or prefs.js):

------- Additional Comment_ #4 From Olivier Cahagne  2003-08-04 00:22 -------
I found the culprit, this particular pref crashes on URL above:

           user_pref("javascript.options.strict", true);

Comment 22

[R.K.Aa.]

Uploading my whole profile here doesn't sound so tempting.  Will try figure out
which pref is relevant. 

Re comment 21: Debug prefs are not the issue in this case.

Comment 23

[Phil Schwartau]

> Uploading my whole profile here doesn't sound so tempting.
> Will try figure out which pref is relevant. 

Right, I meant using diff at the command line, e.g.: 

     diff NEW_prefs.js OLD_prefs.js > DIFFS.txt

and attaching DIFFS.txt here (or pasting if small). 

Comment 24

[R.K.Aa.]

Attachment: diff

account-names and folders anonymized

Comment 25

[Phil Schwartau]

These differences stand out to me. If I'm reading the diff file correctly,
they are all in the old prefs file, but missing from the new one. I wonder
if adding any of them to the new prefs file makes the crash happen:

< user_pref("capability.policy.default.Window.onunload", "noAccess");
< user_pref("javascript.options.showInConsole", false);
< user_pref("layout.reflow.dumpframebyframecounts", false);
< user_pref("layout.reflow.dumpframecounts", false);
< user_pref("layout.reflow.showframecounts", false);
< user_pref("nglayout.debug.disable_xul_cache", false);
< user_pref("nglayout.debug.enable_xpcom_refcnt_log", false);
< user_pref("nglayout.debug.invalidate_dumping", false);
< user_pref("nglayout.debug.motion_event_dumping", false);
< user_pref("nglayout.debug.paint_dumping", false);
< user_pref("nglayout.debug.paint_flashing", false);
< user_pref("nglayout.widget.debugWindow", false);
< user_pref("plugin.soname.list", "libXt.so:libXext.so");
< user_pref("pref.browser.smartbrowsing.disable_textbox.add", false);
< user_pref("privacy.popups.first_popup", false);
< user_pref("privacy.popups.remove_blacklist", false);
< user_pref("privacy.popups.sound_enabled", true);


This is just a guess - it may be one of the other differences in the list.

Comment 26

[Phil Schwartau]

Another two that stand out to me. In the old prefs, but not in the new:

< user_pref("browser.enable_automatic_image_resizing", true);
< user_pref("browser.tabs.loadInBackground", true);

Comment 27

[R.K.Aa.]

to comment 25/26: I deleted all those prefs from prefs.js - started mozilla -
deleted cache - loaded page - same crash. So that wasn't it.

Comment 28

[R.K.Aa.]

actually.. i deleted prefs.js as such while moz wasn't running - then restarted:
It *still* crashes. It's profile dependand, but has nothing to do with prefs.js.
I also deleted user.js ;) And localstore.rdf. And XUL.mfasl. No go. Strange.

Comment 29

[Phil Schwartau]

Wow, it looks like there have been several issues with the dinside site,
including several crashers:

http://bugzilla.mozilla.org/buglist.cgi?bugidtype=include&order=bugs.bug_id&bug_id=,76049,87015,103477,144567,147247,148547,184029,204165

That makes me suspect one of those issues is continuing or resurfacing,
rather than a JS Engine problem -

Comment 30

[R.K.Aa.]

Both digi.no and dinside.no are popular sites. I visit them several times a day
myself. And I think comment 29 is a digression. The stack when i crash at
http://www.dinside.no and http://www.digi.no are identical to the one i first
submitted this bugreport.

None of the bugs you mention look anything like that. Matter of fact.. most are
quote clear instances of other and now fixed bugs. And none of them indicate
profile related misery.

Comment 31

[Phil Schwartau]

Reopening bug, since rkaa is crashing with the same stack trace
as originally reported above -

Comment 32

[Brendan Eich [:brendan]]

R.K.Aa, is wheel-scrolling while loading stlil necessary?  It's clear there is
an impurity, but I think it had nothign to do with the patch here for the
follow-on fix to bug 208030.  Oh well, it was worth a try.

I'm trying to reproduce now.

/be

Comment 33

[Brendan Eich [:brendan]]

Argh, no line numbers.

R.K.Aa, can you reproduce with a debug build (debug and optimized should be good
enough for getting an almost-correct line number)?  Also, if you get that stack
again, I am very interested in what you see in *script when you go to frame #5:

#5  0x4004bbe4 in js_Interpret ()

especially script->filename and script->lineno; also, the value in the local pc
variable in that js_Interpret frame.  Thanks,

/be

Comment 34

[R.K.Aa.]

wheel-scrolling was a red herring - it doesn't affect the crash.
I'll make a debug build tomorrow to get a better stack.

Comment 35

[R.K.Aa.]

Attachment: backtrace

optimized debug build, 2 crashes:  dinside.no and digi.no

Comment 36

[Phil Schwartau]

The stacktraces suggest to me that this is a duplicate of bug 214761.
I think we should dupe this one forward, since bug 214761 has a testcase
and more analysis. It is a bit subtle, since the other report seems
to require that JavaScript strict warnings be turned on. But the stacktraces
seem the same.

Comment 37

[R.K.Aa.]

*** This bug has been marked as a duplicate of 214761 ***

Comment 38

[Phil Schwartau]

Verified Duplicate.

R.K.Aa: thank you for all your work on clarifying this issue !!!