Skip to content
This repository has been archived by the owner on Feb 23, 2018. It is now read-only.

Varming/5.1 #11

Merged
merged 17 commits into from
Jul 20, 2016
Merged

Varming/5.1 #11

merged 17 commits into from
Jul 20, 2016

Conversation

varming
Copy link
Contributor

@varming varming commented Jul 18, 2016

I tried to run scala with a JDK9 early access release 127 and ran into this:

Exception in thread "main" java.lang.BootstrapMethodError: java.lang.IncompatibleClassChangeError: Inconsistent constant data for scala/collection/generic/Growable.$anonfun$$plus$plus$eq$1(Lscala/collection/generic/Growable;Ljava/lang/Object;)Lscala/collection/generic/Growable; at index 67
at scala.collection.generic.Growable.$plus$plus$eq$(Growable.scala:50)
at scala.collection.mutable.ListBuffer.$plus$plus$eq(ListBuffer.scala:182)
at scala.collection.mutable.ListBuffer.$plus$plus$eq(ListBuffer.scala:44)
at scala.collection.TraversableLike.to$(TraversableLike.scala:587)
at scala.collection.TraversableLike.to(TraversableLike.scala:587)
at scala.collection.TraversableOnce.toList$(TraversableOnce.scala:294)
at scala.collection.TraversableOnce.toList(TraversableOnce.scala:294)
at scala.tools.nsc.MainGenericRunner.process(MainGenericRunner.scala:41)
at scala.tools.nsc.MainGenericRunner$.main(MainGenericRunner.scala:103)
at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)
Caused by: java.lang.IncompatibleClassChangeError: Inconsistent constant data for scala/collection/generic/Growable.$anonfun$$plus$plus$eq$1(Lscala/collection/generic/Growable;Ljava/lang/Object;)Lscala/collection/generic/Growable; at index 67
... 10 more

A similar error has been reported to OpenJDK: https://bugs.openjdk.java.net/browse/JDK-8159375. The solution there was to upgrade to ASM 5.1 which handle method references on interfaces correctly (see revision r1791 and r1792 in the ASM svn repo).

Carsten Varming and others added 16 commits July 18, 2016 12:26
Allows creating an asm.Attribute and directly providing the content
as a byte array.

Cherry-pick of
lrytz/scala@9a7e518
Fail early when adding an instruction that already belongs to a
different instruction list.

Cherry-pick of
lrytz/scala@eccd0dc
Make class MethodWriter public and add accessors for maxLocals and
maxStack. This is the simplest way to compute the maxs using the
ASM framework. It is used in the optimizer, see
scala/scala@90781e8

Cherry-pick of
lrytz/scala@91810c9
Fix typos. This change was initially part of
scala/scala@549dc88

Cherry-pick of
lrytz/scala@e1fbc7f
Adds a method `setStack` to analysis.Frame which allows setting the
dataflow value of a stack slot.

Used in nullness analysis. After, for example, an instance call,
aliases of the receiver can be set to NotNull, both in locals and
on the stack.
Introduces a number of methods that are called when initializing or
updating abstract values in an analyzer frame.

Before this commit, Analyzer.analyze and Frame.execute would always
call Interpreter.newValue for initializing or updating frame values.

Having multiple methods allows users to return more precise values
for the individual cases. For example, in a nullness analysis, the
initial value for the `this` parameter of an instance method can be
set to not-null.
…ll copied values

Before this change, `Frame.execute` did not invoke the interpreter's
`copyInstruction` method for all values that are pushed on the frame's
when executing some copying instructions.

For example, in the case of SWAP, copyInstruction is invoked:

    value2 = pop();
    value1 = pop();
    push(interpreter.copyOperation(insn, value2));
    push(interpreter.copyOperation(insn, value1));

For DUP on the other hand, the original value is pushed onto the
stack without notifying the interpreter:

    value1 = pop();
    push(value1);
    push(interpreter.copyOperation(insn, value1));

This leads to a problem for the `SourceInterpreter`, which collects
for every value a set of potential producer instructions. Given the
bytecode sequence

    NEW java/lang/Object
    DUP
    INVOKESPECIAL java/lang/Object.<init> ()V

In the frame of the INVOKESPECIAL instruction, the value on the stack
lists as its producer the `NEW` operation instead of the `DUP`, which
not expected.
The method `findItemByIndex` only looked at the first item in each
bucket of the `items` hash table.

Also, in case the item was not found, the counter `i` would be at
`items.length`, therefore the access `items[i]` crashed with an
ArrayIndexOutOfBoundsException.

Fixes the issue scala/scala-asm#8.
@retronym
Copy link
Member

@varming FYI, here's the umbrella issue I'm using to scope out JDK9 support: scala/scala-dev#139

@varming
Copy link
Contributor Author

varming commented Jul 19, 2016

FYI: The change to ASM was discussed here: https://bugs.openjdk.java.net/browse/JDK-8147755

@@ -19,10 +19,10 @@ This makes it easy to see how our fork differs from the official release.

## Current Version

The current sources are based on the following version of ASM ([browse tags here](http://websvn.ow2.org/listing.php?repname=asm&path=%2Ftags%2F&peg=1748)):
The current sources are based on the following version of ASM ([browse tags here](http://websvn.ow2.org/listing.php?repname=asm&path=%2Ftags%2F&peg=1827)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://websvn.ow2.org/listing.php?repname=asm&path=%2Ftags%2F works and doesn't have a revision in the url

@lrytz
Copy link
Member

lrytz commented Jul 20, 2016

LGTM otherwise, thanks a lot!

@varming
Copy link
Contributor Author

varming commented Jul 20, 2016

Dear @lrytz,

I addressed your comments from README.md.

@lrytz
Copy link
Member

lrytz commented Jul 20, 2016

Thanks!

@lrytz lrytz merged commit e5c39e5 into scala:master Jul 20, 2016
@varming varming deleted the varming/5.1 branch July 20, 2016 15:15
@lrytz
Copy link
Member

lrytz commented Jul 20, 2016

pushed a new release (http://repo2.maven.org/maven2/org/scala-lang/modules/scala-asm/5.1.0-scala-1/) and started using it (scala/scala#5293).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants