S RFR: Lambda: verification error in generated lambda classes

[Keith McGuigan]

I think there's an extra set of parenthesis that isn't needed, but other
than that, thanks, looks good!


On Thu, Aug 29, 2013 at 1:29 PM, Karen Kinnear <Karen.Kinnear at oracle.com>wrote:

> Keith, David,
>
> Many thanks for the suggestions. I think Keith's proposal is much simpler
> to read. I did
> a name change as well to hopefully make this more obvious.
>
> webrev.02:
>
> >> webrev: http://cr.openjdk.java.net/~acorn/8023872.02/webrev/
>> >> http://bugs.sun.com/view_bug.do?bug_id=8023872
>>
>
> I redid the manual testing, and the longer tests are in progress again.
>
> thanks,
> Karen
>
> On Aug 28, 2013, at 10:31 PM, Keith McGuigan wrote:
>
> I think it's ok as it is, if you need to check it in quick, but I agree
> that this probably should be rewritten better to make the logic more
> obvious.  Maybe separate clauses for reflection/lambda code?
>
>     ((!is_MAI && !is_MLI) ||
>     (is_MAI && VerifyReflectionBytecodes) ||
>     (is_MLI && VerifyLambdaBytecodes))
>
> Isn't this equivalent to:
>   (!is_MAI || VerifyReflectionBytecodes) &&
>   (!is_MLI || VerifyLambdaBytecodes)
>
>
>
> On Wed, Aug 28, 2013 at 8:50 PM, Karen Kinnear <Karen.Kinnear at oracle.com>wrote:
>
>> Thank you David. I agree, I was tempted to rewrite the entire method - in
>> particular, in a debugger
>> (so I didn't check the optimized code) we walk the entire && before we
>> make the initial call - in this
>> case the initial call would weed out more things faster potentially, so
>> ... But I chose not to rewrite since
>> they need this quickly and the shaking out would take quite a bit longer.
>>
>> thanks for the review,
>> Karen
>>
>> On Aug 28, 2013, at 8:48 PM, David Holmes wrote:
>>
>> > Hi Karen,
>> >
>> > I found the boolean logic very hard to follow there - not sure if the
>> refactoring helped or hindered :)
>> >
>> > But it looks okay.
>> >
>> > David
>> >
>> > On 29/08/2013 5:42 AM, Karen Kinnear wrote:
>> >> Please review - lambda team needs this change to get in by tomorrow so
>> >> they can push
>> >> the (8010433) metafactory change which is waiting on the vm. Thanks.
>> >>
>> >>
>> >> webrev: http://cr.openjdk.java.net/~acorn/8023872/webrev/
>> >> http://bugs.sun.com/view_bug.do?bug_id=8023872
>> >>
>> >> Testing:
>> >> The failure comes if you apply an early patch of 8010433 to the jdk and
>> >> do not
>> >> make the vm change. So testing included 4 binaries: master, newjdk,
>> >> newvm, newjdkvm
>> >>
>> >> jtreg for SpinedBufferTest with various flags - these are the expected
>> >> results below
>> >> The key point is newjdk -Xverify: all fails
>> >> newjdkvm -Xverify:all passes, but if you also turn on
>> >> -XX:+VerifyLambdaBytecodes you see
>> >>    the verification error
>> >>
>> >> 1. master
>> >>  no flags: timed out
>> >>  -Xverify:all: timed out
>> >>  -XX:+VerifyReflectionBytecodes: VerifyError in reflection
>> >>
>> >> 2. newjdk
>> >>  no flags: timed out
>> >>  -Xverify:all
>> >>    java.lang.BootstrapMethodError: call site initialization exception
>> >> ... cause:
>> >>      VerifyError: Bad invokespecial instruction: current class isn't
>> >> assignable to reference class
>> >>  -XX:+VerifyReflectionBytecodes: VerifyError in reflection
>> >>
>> >> 3. newvm
>> >>   no flags: timed out
>> >>   rerun.sh: -Xverify:all: passed
>> >>   -XX:+VerifyReflectionBytecodes: VerifyError in reflection
>> >>   -XX:+VerifyLambdaBytecodes: passed, didn't try verification
>> >>   rerun.sh: -Xverify:all -XX:+VerifyLambdaBytecodes: passed
>> >>
>> >> 4. newjdkvm
>> >>  rerun.sh: no flags: passed
>> >>  rerun.verify.sh: -Xverify:all: passed
>> >>  rerun.sh: -Xverify:all -XX:+VerifyLambdaBytecodes:
>> >>    Verification for java.util.streamIntPipeline$7$1$$Lambda$9 failed
>> >>
>> >> regression testing:
>> >> newvm:
>> >>   vm.quick.testlist - in progress
>> >>   jtreg java/util/stream - in progress
>> >>   jprt -stree . - in progress
>> >>
>> >> newvmjdk (Brian testing in lambda repo) - in progress
>> >>
>> >> thanks,
>> >> Karen
>> >>
>>
>>
>
>
> --
> - Keith McGuigan <keith.mcguigan at alumni.unh.edu>
>
>
>


-- 
- Keith McGuigan <keith.mcguigan at alumni.unh.edu>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130829/4458d3d9/attachment.html