RFR 8168663: Nashorn: ant testng tests doesn't support external java options

[Srinivas Dama]

Hi,

Please review http://cr.openjdk.java.net/~sdama/8168663/webrev.00/ 
for https://bugs.openjdk.java.net/browse/JDK-8168663 

Added run.test.jvmargs.external property so that we can pass jvm options for 
complete test run as below.
ant -Drun.test.jvmargs.external="-XX:+UseSerialGC -XX:-TieredCompilation" test

Regards,
Srinivas

Re: Review request for JDK-8170594: >>>=0 generates invalid bytecode for BaseNode LHS

[Sundararajan Athijegannathan]

+1

On 01/12/16, 6:49 PM, Attila Szegedi wrote:

Please review JDK-8170594 ">>>=0 generates invalid bytecode for BaseNode LHS" 
at  
for

Thanks,
   Attila.

Re: Review request for JDK-8170594: >>>=0 generates invalid bytecode for BaseNode LHS

[Jim Laskey (Oracle)]

+1

> On Dec 1, 2016, at 9:19 AM, Attila Szegedi  wrote:
> 
> Please review JDK-8170594 ">>>=0 generates invalid bytecode for BaseNode LHS" 
> at  for 
> 
> 
> Thanks,
>  Attila.

Review request for JDK-8170594: >>>=0 generates invalid bytecode for BaseNode LHS

[Attila Szegedi]

Please review JDK-8170594 ">>>=0 generates invalid bytecode for BaseNode LHS" 
at  for 


Thanks,
  Attila.

Re: java.lang.VerifyError: Inconsistent stackmap frames at branch target

[Attila Szegedi]

Okay, I tracked this down to an incorrect optimization in code generator for 
when you use >>=0 to coerce to an uint32 :-( 

There is a special case for exactly this usage, where the right-hand-side 
operand is a literal 0, so we avoid emitting a no-op “ICONST_0; IUSHR” bytecode 
sequence. It unfortunately wasn’t working correctly for when the left-hand-side 
operand of a compound-assignment-shift-right was either a property or element 
access expression (either “obj.foo” or “obj[x]”)

Decomposing the compound assignment into an explicit shift and assignment would 
work around the problem for now:

   this.length = this.length >>> 0;  // Coerce to uint32.

I filed > and have a fix for it.

Attila.

> On 01 Dec 2016, at 13:01, Attila Szegedi  wrote:
> 
> Running with assertions enabled shows that the error is in “this.length 
> >>>=0” expression on line 31. Reducing the testcase to just:
> 
> (function (p) {
> if (p) {
>   this.length >>>= 0;  // Coerce to uint32.
> }
> })(false)
> 
> also reproduces the problem (at least, the assertion; it will cause a 
> somewhat different issue without assertions).
> 
> Attila.
> 
>> On 01 Dec 2016, at 12:18, Frantzius, Jörg  wrote:
>> 
>> Hi Sunda,
>> 
>> you can reproduce by putting the following into a test.js file and executing 
>> it using jjs.
>> 
>>  snip =
>> 
>> function Buffer(subject, encoding) {
>> if (!util.isBuffer(this))
>>   return new Buffer(subject, encoding);
>> 
>> if (util.isNumber(subject)) {
>>   this.length = +subject;
>> 
>> } else if (util.isString(subject)) {
>>   if (!util.isString(encoding) || encoding.length === 0)
>> encoding = 'utf8';
>>   this.length = Buffer.byteLength(subject, encoding);
>> 
>> // Handle Arrays, Buffers, Uint8Arrays or JSON.
>> } else if (util.isObject(subject)) {
>>   if (subject.type === 'Buffer' && util.isArray(subject.data))
>> subject = subject.data;
>>   this.length = +subject.length;
>> 
>> } else {
>>   throw new TypeError('must start with number, buffer, array or string');
>> }
>> 
>> if (this.length > kMaxLength) {
>>   throw new RangeError('Attempt to allocate Buffer larger than maximum ' +
>>'size: 0x' + kMaxLength.toString(16) + ' bytes');
>> }
>> 
>> if (this.length < 0)
>>   this.length = 0;
>> else
>>   this.length >>>= 0;  // Coerce to uint32.
>> 
>> this.parent = undefined;
>> if (this.length <= (Buffer.poolSize >>> 1) && this.length > 0) {
>>   if (this.length > poolSize - poolOffset)
>> createPool();
>>   this.parent = sliceOnto(allocPool,
>>   this,
>>   poolOffset,
>>   poolOffset + this.length);
>>   poolOffset += this.length;
>> 
>>   // Ensure aligned slices
>>   if (poolOffset & 0x7) {
>> poolOffset |= 0x7;
>> poolOffset++;
>>   }
>> } else {
>>   alloc(this, this.length);
>> }
>> 
>> if (util.isNumber(subject)) {
>>   return;
>> }
>> 
>> if (util.isString(subject)) {
>>   // In the case of base64 it's possible that the size of the buffer
>>   // allocated was slightly too large. In this case we need to rewrite
>>   // the length to the actual length written.
>>   var len = this.write(subject, encoding);
>>   // Buffer was truncated after decode, realloc internal ExternalArray
>>   if (len !== this.length) {
>> var prevLen = this.length;
>> this.length = len;
>> truncate(this, this.length);
>> // Only need to readjust the poolOffset if the allocation is a slice.
>> if (this.parent != undefined)
>>   poolOffset -= (prevLen - len);
>>   }
>> 
>> } else if (util.isBuffer(subject)) {
>>   subject.copy(this, 0, 0, this.length);
>> 
>> } else if (util.isNumber(subject.length) || util.isArray(subject)) {
>>   // Really crappy way to handle Uint8Arrays, but V8 doesn't give a simple
>>   // way to access the data from the C++ API.
>>   for (var i = 0; i < this.length; i++)
>> this[i] = subject[i];
>> }
>> }
>> 
>> new Buffer(1024);
>> 
>> == snip ==
>> 
>> 
>> I’ll hopefully get my OCA signed and sent in soon, so I can contribute in 
>> JIRA.
>> 
>> Regards,
>> Jörg
>> 
>> 
>> ---
>> 
>> Dipl. Inf. Jörg von Frantzius, Technical Director
>> 
>> E-Mail joerg.frantz...@aperto.com
>> 
>> Phone +49 30 283921-318
>> Fax +49 30 283921-29
>> 
>> Aperto GmbH – An IBM Company
>> Chausseestraße 5, D-10115 Berlin
>> http://www.aperto.com
>> http://www.facebook.com/aperto
>> https://www.xing.com/companies/apertoag
>> 
>> HRB 77049 B, AG Berlin Charlottenburg
>> Geschäftsführer: Dirk Buddensiek, Kai Großmann, Stephan Haagen, Daniel Simon
>> 
>> Am 01.12.2016 um 07:46 schrieb Sundararajan Athijegannathan 
>> >:
>> 
>> Is there a simple reduced test case that we can use it to 

Re: Error Stack Column number

[Attila Szegedi]

Sorry, disregard the message below, it was meant for a different e-mail thread 
:-)

> On 01 Dec 2016, at 13:53, Attila Szegedi  wrote:
> 
> Okay, I tracked this down to an incorrect optimization in code generator for 
> when you use >>=0 to coerce to an uint32 :-( 
> 
> There is a special case for exactly this usage, where the right-hand-side 
> operand is a literal 0, so we avoid emitting a no-op “ICONST_0; IUSHR” 
> bytecode sequence. It unfortunately wasn’t working correctly for when the 
> left-hand-side operand of a compound-assignment-shift-right was either a 
> property or element access expression (either “obj.foo” or “obj[x]”)
> 
> Decomposing the compound assignment into an explicit shift and assignment 
> would work around the problem for now:
> 
>this.length = this.length >>> 0;  // Coerce to uint32.
> 
> I filed  > and have a fix for it.
> 
> Attila.
> 
>> On 01 Dec 2016, at 07:44, Sundararajan Athijegannathan 
>> > > wrote:
>> 
>> We do not aim to provide complete compatibility with other JS 
>> implementations on the non-standard properties such as "stack". stack tries 
>> to mimic whatever is done for Java code (no column number for eg.). But, as 
>> you've noted there are enough information on Error objects via other 
>> properties like lineNumber, columnNumber, fileName. It should be possible to 
>> write a simple utility function to format stack string as desired for 
>> specific applications.
>> 
>> Thanks,
>> -Sundar
>> 
>> On 01/12/16, 3:32 AM, Art Fiedler wrote:
>>> When making an implementation of console.count([label])  part of the
>>> Mozilla's developer docs
>>> https://developer.mozilla.org/en-US/docs/Web/API/Console/count 
>>> 
>>> mentions when label is omitted, the function will count the number of times
>>> count() was called
>>> on that line... however, if the javascript code has been minified all lines
>>> of code may be on the
>>> same line... messing up all console.count() calls...
>>> 
>>> I was providing that implementation by using new Error().stack and parsing
>>> the current file:line...
>>> however I could fix the minify issue if new Error().stack output the column
>>> also for instance...
>>> other browser seem to be doing file:line:column in the stack trace.
>>> Including nodejs see:
>>> https://nodejs.org/api/errors.html#errors_error_stack 
>>> 
>>> 
>>> Microsoft Edge's dev console outputs...
>>> new Error().stack:
>>> "Error
>>>at eval code (eval code:1:1)"
>>> Firefox since FF30:
>>> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/stack
>>>  
>>> 
>>> 
>>> Currently nashorn error stacks only include the line number...
>>> public static void main(String[] args) throws Exception {
>>> NashornScriptEngineFactory factory = new
>>> NashornScriptEngineFactory();
>>> ScriptEngine scriptEngine = factory.getScriptEngine();
>>> scriptEngine.put(ScriptEngine.FILENAME, "myfile.js");
>>> scriptEngine.eval("print('fileName: ' + new Error().fileName);");
>>> scriptEngine.eval("print('lineNumber: ' + new
>>> Error().lineNumber);");
>>> scriptEngine.eval("print('columnNumber: ' + new
>>> Error().columnNumber);");
>>> scriptEngine.eval("print('stack: ' + new Error().stack);");
>>> }
>>> /*
>>> fileName: myfile.js
>>> lineNumber: 1
>>> columnNumber: -1
>>> stack: Error
>>> at  (myfile.js:1)
>>>  */
>>> 
>>> Would be nice to add the column number to the stacks and error object in
>>> nashorn.
>>> 
>>> Thanks,
>>> Arthur Fiedler
> 

Re: RFR 8170565: JSObject call() is passed undefined for the argument 'thiz'

[Attila Szegedi]

+1

> On 01 Dec 2016, at 13:48, Sundararajan Athijegannathan 
>  wrote:
> 
> Good catch Hannes! Please review the updated webrev : 
> http://cr.openjdk.java.net/~sundar/8170565/webrev.01/
> 
> PS. Had to use Function.prototype.call.call to pass undefined this explicitly 
> (as JSObject.call can't be called from script).
> 
> -Sundar
> 
> On 01/12/16, 3:13 PM, Hannes Wallnöfer wrote:
>> Hi Sundar,
>> 
>> The problem with this approach is that it will replace any occurrence of 
>> undefined this with the global object. However, this should only occur for 
>> scope calls. For example, the following call would see undefined replaced 
>> with global:
>> 
>> func.call(undefined)
>> 
>> This is probably not a problem that will occur very often, but ideally I 
>> think we should do the check and replacement on the linking side, i.e. in 
>> JSObjectLinker.findCallMethod.
>> 
>> On the other hand we can’t check for function strictness that way. Maybe do 
>> it your way but add a boolean isScope parameter and bind that at link time?
>> 
>> Hannes
>> 
>> 
>>> Am 01.12.2016 um 07:21 schrieb Sundararajan 
>>> Athijegannathan:
>>> 
>>> Please review http://cr.openjdk.java.net/~sundar/8170565/webrev.00/ for 
>>> https://bugs.openjdk.java.net/browse/JDK-8170565
>>> 
>>> Thanks,
>>> -Sundar

Re: Error Stack Column number

[Attila Szegedi]

Okay, I tracked this down to an incorrect optimization in code generator for 
when you use >>=0 to coerce to an uint32 :-( 

There is a special case for exactly this usage, where the right-hand-side 
operand is a literal 0, so we avoid emitting a no-op “ICONST_0; IUSHR” bytecode 
sequence. It unfortunately wasn’t working correctly for when the left-hand-side 
operand of a compound-assignment-shift-right was either a property or element 
access expression (either “obj.foo” or “obj[x]”)

Decomposing the compound assignment into an explicit shift and assignment would 
work around the problem for now:

   this.length = this.length >>> 0;  // Coerce to uint32.

I filed > and have a fix for it.

Attila.

> On 01 Dec 2016, at 07:44, Sundararajan Athijegannathan 
>  wrote:
> 
> We do not aim to provide complete compatibility with other JS implementations 
> on the non-standard properties such as "stack". stack tries to mimic whatever 
> is done for Java code (no column number for eg.). But, as you've noted there 
> are enough information on Error objects via other properties like lineNumber, 
> columnNumber, fileName. It should be possible to write a simple utility 
> function to format stack string as desired for specific applications.
> 
> Thanks,
> -Sundar
> 
> On 01/12/16, 3:32 AM, Art Fiedler wrote:
>> When making an implementation of console.count([label])  part of the
>> Mozilla's developer docs
>> https://developer.mozilla.org/en-US/docs/Web/API/Console/count
>> mentions when label is omitted, the function will count the number of times
>> count() was called
>> on that line... however, if the javascript code has been minified all lines
>> of code may be on the
>> same line... messing up all console.count() calls...
>> 
>> I was providing that implementation by using new Error().stack and parsing
>> the current file:line...
>> however I could fix the minify issue if new Error().stack output the column
>> also for instance...
>> other browser seem to be doing file:line:column in the stack trace.
>> Including nodejs see:
>> https://nodejs.org/api/errors.html#errors_error_stack
>> 
>> Microsoft Edge's dev console outputs...
>> new Error().stack:
>> "Error
>>at eval code (eval code:1:1)"
>> Firefox since FF30:
>> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/stack
>> 
>> Currently nashorn error stacks only include the line number...
>> public static void main(String[] args) throws Exception {
>> NashornScriptEngineFactory factory = new
>> NashornScriptEngineFactory();
>> ScriptEngine scriptEngine = factory.getScriptEngine();
>> scriptEngine.put(ScriptEngine.FILENAME, "myfile.js");
>> scriptEngine.eval("print('fileName: ' + new Error().fileName);");
>> scriptEngine.eval("print('lineNumber: ' + new
>> Error().lineNumber);");
>> scriptEngine.eval("print('columnNumber: ' + new
>> Error().columnNumber);");
>> scriptEngine.eval("print('stack: ' + new Error().stack);");
>> }
>> /*
>> fileName: myfile.js
>> lineNumber: 1
>> columnNumber: -1
>> stack: Error
>> at  (myfile.js:1)
>>  */
>> 
>> Would be nice to add the column number to the stacks and error object in
>> nashorn.
>> 
>> Thanks,
>> Arthur Fiedler

Re: RFR 8170565: JSObject call() is passed undefined for the argument 'thiz'

[Jim Laskey (Oracle)]

+1

> On Dec 1, 2016, at 8:48 AM, Sundararajan Athijegannathan 
>  wrote:
> 
> Good catch Hannes! Please review the updated webrev : 
> http://cr.openjdk.java.net/~sundar/8170565/webrev.01/
> 
> PS. Had to use Function.prototype.call.call to pass undefined this explicitly 
> (as JSObject.call can't be called from script).
> 
> -Sundar
> 
> On 01/12/16, 3:13 PM, Hannes Wallnöfer wrote:
>> Hi Sundar,
>> 
>> The problem with this approach is that it will replace any occurrence of 
>> undefined this with the global object. However, this should only occur for 
>> scope calls. For example, the following call would see undefined replaced 
>> with global:
>> 
>> func.call(undefined)
>> 
>> This is probably not a problem that will occur very often, but ideally I 
>> think we should do the check and replacement on the linking side, i.e. in 
>> JSObjectLinker.findCallMethod.
>> 
>> On the other hand we can’t check for function strictness that way. Maybe do 
>> it your way but add a boolean isScope parameter and bind that at link time?
>> 
>> Hannes
>> 
>> 
>>> Am 01.12.2016 um 07:21 schrieb Sundararajan 
>>> Athijegannathan:
>>> 
>>> Please review http://cr.openjdk.java.net/~sundar/8170565/webrev.00/ for 
>>> https://bugs.openjdk.java.net/browse/JDK-8170565
>>> 
>>> Thanks,
>>> -Sundar

Re: RFR 8170565: JSObject call() is passed undefined for the argument 'thiz'

[Hannes Wallnöfer]

+1

Nice solution, of course much simpler than what I proposed below :)

Hannes

> Am 01.12.2016 um 13:48 schrieb Sundararajan Athijegannathan 
> :
> 
> Good catch Hannes! Please review the updated webrev : 
> http://cr.openjdk.java.net/~sundar/8170565/webrev.01/
> 
> PS. Had to use Function.prototype.call.call to pass undefined this explicitly 
> (as JSObject.call can't be called from script).
> 
> -Sundar
> 
> On 01/12/16, 3:13 PM, Hannes Wallnöfer wrote:
>> Hi Sundar,
>> 
>> The problem with this approach is that it will replace any occurrence of 
>> undefined this with the global object. However, this should only occur for 
>> scope calls. For example, the following call would see undefined replaced 
>> with global:
>> 
>> func.call(undefined)
>> 
>> This is probably not a problem that will occur very often, but ideally I 
>> think we should do the check and replacement on the linking side, i.e. in 
>> JSObjectLinker.findCallMethod.
>> 
>> On the other hand we can’t check for function strictness that way. Maybe do 
>> it your way but add a boolean isScope parameter and bind that at link time?
>> 
>> Hannes
>> 
>> 
>>> Am 01.12.2016 um 07:21 schrieb Sundararajan 
>>> Athijegannathan:
>>> 
>>> Please review http://cr.openjdk.java.net/~sundar/8170565/webrev.00/ for 
>>> https://bugs.openjdk.java.net/browse/JDK-8170565
>>> 
>>> Thanks,
>>> -Sundar

Re: java.lang.VerifyError: Inconsistent stackmap frames at branch target

[Attila Szegedi]

Running with assertions enabled shows that the error is in “this.length >>>=0” 
expression on line 31. Reducing the testcase to just:

(function (p) {
 if (p) {
   this.length >>>= 0;  // Coerce to uint32.
 }
})(false)

also reproduces the problem (at least, the assertion; it will cause a somewhat 
different issue without assertions).

Attila.

> On 01 Dec 2016, at 12:18, Frantzius, Jörg  wrote:
> 
> Hi Sunda,
> 
> you can reproduce by putting the following into a test.js file and executing 
> it using jjs.
> 
>  snip =
> 
> function Buffer(subject, encoding) {
>  if (!util.isBuffer(this))
>return new Buffer(subject, encoding);
> 
>  if (util.isNumber(subject)) {
>this.length = +subject;
> 
>  } else if (util.isString(subject)) {
>if (!util.isString(encoding) || encoding.length === 0)
>  encoding = 'utf8';
>this.length = Buffer.byteLength(subject, encoding);
> 
>  // Handle Arrays, Buffers, Uint8Arrays or JSON.
>  } else if (util.isObject(subject)) {
>if (subject.type === 'Buffer' && util.isArray(subject.data))
>  subject = subject.data;
>this.length = +subject.length;
> 
>  } else {
>throw new TypeError('must start with number, buffer, array or string');
>  }
> 
>  if (this.length > kMaxLength) {
>throw new RangeError('Attempt to allocate Buffer larger than maximum ' +
> 'size: 0x' + kMaxLength.toString(16) + ' bytes');
>  }
> 
>  if (this.length < 0)
>this.length = 0;
>  else
>this.length >>>= 0;  // Coerce to uint32.
> 
>  this.parent = undefined;
>  if (this.length <= (Buffer.poolSize >>> 1) && this.length > 0) {
>if (this.length > poolSize - poolOffset)
>  createPool();
>this.parent = sliceOnto(allocPool,
>this,
>poolOffset,
>poolOffset + this.length);
>poolOffset += this.length;
> 
>// Ensure aligned slices
>if (poolOffset & 0x7) {
>  poolOffset |= 0x7;
>  poolOffset++;
>}
>  } else {
>alloc(this, this.length);
>  }
> 
>  if (util.isNumber(subject)) {
>return;
>  }
> 
>  if (util.isString(subject)) {
>// In the case of base64 it's possible that the size of the buffer
>// allocated was slightly too large. In this case we need to rewrite
>// the length to the actual length written.
>var len = this.write(subject, encoding);
>// Buffer was truncated after decode, realloc internal ExternalArray
>if (len !== this.length) {
>  var prevLen = this.length;
>  this.length = len;
>  truncate(this, this.length);
>  // Only need to readjust the poolOffset if the allocation is a slice.
>  if (this.parent != undefined)
>poolOffset -= (prevLen - len);
>}
> 
>  } else if (util.isBuffer(subject)) {
>subject.copy(this, 0, 0, this.length);
> 
>  } else if (util.isNumber(subject.length) || util.isArray(subject)) {
>// Really crappy way to handle Uint8Arrays, but V8 doesn't give a simple
>// way to access the data from the C++ API.
>for (var i = 0; i < this.length; i++)
>  this[i] = subject[i];
>  }
> }
> 
> new Buffer(1024);
> 
> == snip ==
> 
> 
> I’ll hopefully get my OCA signed and sent in soon, so I can contribute in 
> JIRA.
> 
> Regards,
> Jörg
> 
> 
> ---
> 
> Dipl. Inf. Jörg von Frantzius, Technical Director
> 
> E-Mail joerg.frantz...@aperto.com
> 
> Phone +49 30 283921-318
> Fax +49 30 283921-29
> 
> Aperto GmbH – An IBM Company
> Chausseestraße 5, D-10115 Berlin
> http://www.aperto.com
> http://www.facebook.com/aperto
> https://www.xing.com/companies/apertoag
> 
> HRB 77049 B, AG Berlin Charlottenburg
> Geschäftsführer: Dirk Buddensiek, Kai Großmann, Stephan Haagen, Daniel Simon
> 
> Am 01.12.2016 um 07:46 schrieb Sundararajan Athijegannathan 
> >:
> 
> Is there a simple reduced test case that we can use it to reproduce the issue 
> you're seeing? Please send us the same and we'll file a bug  (or you may do 
> that as well).
> 
> Thanks,
> -Sundar
> 
> On 29/11/16, 11:11 PM, Frantzius, Jörg wrote:
> Hi,
> 
> with JDK 1.8.0_112 (on Mac OS X) I’m running into the following error. When 
> querying 
> bugs.openjdk.java.net
>   for "Current frame's stack size doesn't match stackmap“, I only found bugs 
> dating from 2013, so this may not be known yet?
> 
> Unfortunately I can’t see the Javascript file name or line number in the 
> error message. The last known source location node/lib/fs.js:374 that I can 
> step to in the Netbeans debugger is calling a constructor „Buffer(size)“, 
> which is likely this source: 
> https://github.com/nodejs/node/blob/v0.12.7-release/lib/buffer.js#L48
> 
> Following is the error message:
> 
> java.lang.VerifyError: Inconsistent stackmap frames at branch target 404
> Exception 

Re: RFR 8170565: JSObject call() is passed undefined for the argument 'thiz'

[Hannes Wallnöfer]

Hi Sundar,

The problem with this approach is that it will replace any occurrence of 
undefined this with the global object. However, this should only occur for 
scope calls. For example, the following call would see undefined replaced with 
global:

func.call(undefined)

This is probably not a problem that will occur very often, but ideally I think 
we should do the check and replacement on the linking side, i.e. in 
JSObjectLinker.findCallMethod. 

On the other hand we can’t check for function strictness that way. Maybe do it 
your way but add a boolean isScope parameter and bind that at link time?

Hannes


> Am 01.12.2016 um 07:21 schrieb Sundararajan Athijegannathan 
> :
> 
> Please review http://cr.openjdk.java.net/~sundar/8170565/webrev.00/ for 
> https://bugs.openjdk.java.net/browse/JDK-8170565
> 
> Thanks,
> -Sundar