The default bug view has changed. See this FAQ.

Javascript 1.8.5 js_GetScriptLineExtent does not always return the correct value

RESOLVED FIXED in mozilla8

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Robin Ehrlich, Assigned: sfink)

Tracking

unspecified
mozilla8
All
Other
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 545395 [details]
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.
Could you post that as a patch?
(Reporter)

Comment 2

6 years ago
> 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;
>
(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.
(Reporter)

Comment 4

6 years ago
Created attachment 545439 [details] [diff] [review]
diff -u 8 of changes
(Reporter)

Comment 5

6 years ago
I have added an attachment with a diff -u 8 of the changes.
(Assignee)

Updated

6 years ago
Attachment #545439 - Attachment is patch: true
Attachment #545439 - Flags: review?(sphink)
(Assignee)

Comment 6

6 years ago
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.
Attachment #545439 - Flags: review?(sphink) → review+
(Assignee)

Comment 7

6 years ago
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.
Assignee: general → sphink
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/d1746a8b20a9
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.