670958 - Javascript 1.8.5 js_GetScriptLineExtent does not always return the correct value

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Robin Ehrlich]

Attachment: getScriptLineExtent.cpp

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0; GTB6; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.04506.30; .NET CLR 3.0.04506.648; .NET CLR 3.5.21022; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729; OfficeLiveConnector.1.3; OfficeLivePatch.0.0; .NET4.0C)

Steps to reproduce:

I ran the following script using my application that embeds Javascript:

xx = 1;

try {
	 debugger;

	 xx += 1;
}
catch (e)
{
	 xx += 1;
}        




Actual results:

When the debug hook is called, my application calls JS_GetScriptLineExtent and a small value (2?) is returned. I would expect 12. The reason is that the line numbers as processed by js_GetScriptLineExtent in jsscript.cpp are not sequential.

I have attached a version of js_GetScriptLineExtent that corrects the problem.

Comment 1

[David Mandelin [:dmandelin]]

Could you post that as a patch?

Comment 2

[Robin Ehrlich]

> Could you post that as a patch?

I am sorry, but I don't know what you mean by this. The supplied code is a replacement for the function js_GetScriptLineExtent in jsscript.cpp. The changes are quite short and simple.

Here is a diff:

1924a1925,1926
>
>     bool counting;
1925a1928
>     uintN maxLineNo;
1929a1933,1934
>     maxLineNo = 0;
>     counting = true;
1932a1938,1939
>             if (maxLineNo < lineno)
>                 maxLineNo = lineno;
1933a1941,1945
>             counting = true;
>             if (maxLineNo < lineno)
>                 maxLineNo = lineno;
>             else
>                 counting = false;
1935c1947,1948
<             lineno++;
---
>             if (counting)
>                 lineno++;
1937a1951,1954
>
>     if (maxLineNo > lineno)
>         lineno = maxLineNo;
>

Comment 3

[David Mandelin [:dmandelin]]

(In reply to comment #2)
> > Could you post that as a patch?
> 
> I am sorry, but I don't know what you mean by this.

I mean a diff in the 'unified' format with 8 lines of context. That's the standard form we use to review and take changes. I guess you are using a source release rather than hg, so the output of 'diff -u 8' should work.

Comment 4

[Robin Ehrlich]

Attachment: diff -u 8 of changes

Comment 5

[Robin Ehrlich]

I have added an attachment with a diff -u 8 of the changes.

Comment 6

[Steve Fink [:sfink] [:s:]]

Comment on attachment 545439 [details] [diff] [review]
diff -u 8 of changes

Looks good to me, other than an extra newline at the top. It's funny, I was just looking at that code a few days ago and thinking, "Really? That works? We must *really* constrain our bytecode generation in funky ways!"

I actually pushed this to try an hour ago. If it's green, I'll land it along with some other patches.

Comment 7

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/d1746a8b20a9

That's your patch plus I wrapped up your test code into jsapi-test/testScriptInfo.cpp that tests this plus a few other related APIs.

Comment 8

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/d1746a8b20a9