418456 - wrong assert in js_PutBlockObject

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

From IRC session:

crowder: jsobj.c:1956 asserts than a uintN is >= 0
...
crowder:         JS_ASSERT(depth >= 0);
...
crowder: in js_PutBlockObject

The assert became useless after changes from bug 416439. It would be nice to replace it by a check that verifies that depth < script->depth.

Comment 1

[Brendan Eich [:brendan]]

Indeed, we know uint16 fits in jsint and we can assert on the "store side" that depth is not negative in ptrdiff_t terms, and what's more that it fits in uint16. Hence my claim on IRC that the not-negative assert was vacuous assuming depth was jsint in js_PutBlockObjects.

I still like unsigned types for ruling out "what if" tests and code costs, even assertions which consume source complexity budget and brainprint. Java's lack of unsigned for this, and for strength-reducible multiply and divide/modulus, was a loss.

/be

Comment 2

[Igor Bukanov]

(In reply to comment #1)
> Java's lack
> of unsigned for this, and for strength-reducible multiply and divide/modulus,
> was a loss.

I second that. I made my share of bugs in Java with code like:

byte b;
...
int i = b; // should be i = b & 0xFF

If universal unsigned types were too much for Java, then at least in the language the byte type should be unsigned, not signed.   

Comment 3

[Igor Bukanov]

Attachment: v1

Comment 4

[Igor Bukanov]

Comment on attachment 304301 [details] [diff] [review]
v1

The fix strengthen the asserts both for depth and slot.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 304301 [details] [diff] [review]
v1

r/a=me, thanks!

/be

Comment 7

[Igor Bukanov]

I checked in the patch from comment 3 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1203499405&maxdate=1203499470&who=igor%25mir2.org