345350 - crash in js_FreeRuntimeScriptState - jsscript.c

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Mike Moening]

js_FreeRuntimeScriptState in jsscript.c crashes when doing GC.
Whenever the rt->scriptFilenameTable member is NULL this function attempts to iterate using the NULL pointer.

Comment 1

[Mike Moening]

Attachment: Patch to handle null value.

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #0)
> js_FreeRuntimeScriptState in jsscript.c crashes when doing GC.

When shutting down, or really when calling JS_DestroyRuntime, rather.

> Whenever the rt->scriptFilenameTable member is NULL this function attempts to
> iterate using the NULL pointer.

Thanks for filing this.  Nominating for 1.8.1.

/be

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 229983 [details] [diff] [review]
Patch to handle null value.

This was my patch, so mrbkap should review.

/be

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 229983 [details] [diff] [review]
Patch to handle null value.

Null defense, important for embeddings and general correctness, zero risk.

/be

Comment 5

[Brendan Eich [:brendan]]

Fixed on trunk.

/be

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 229983 [details] [diff] [review]
Patch to handle null value.

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and mark fixed1.8.1 once you have.

Comment 7

[Bob Clary [:bc] (inactive)]

See Bug 345530 Comment 8

Comment 8

[Brendan Eich [:brendan]]

Sorry, I don't see how this null check could have caused a leak.  If the runtime's scriptFilenameTable is null, no script filename prefixes can be saved.  If it's not null, then prefixes may or may not be in the prefix list.  Someone please show cause and effect, or more leak traces, or something more than just guilty hook.

Do we need a separate bug on file?  Don't reopen this bug, in any event.

/be