galoisinc / jvm-parser Goto Github PK

Trivial Scala code which compiles to only simple Java works. Slightly more interesting Scala which depends on the scala-library for collections fails with a parse error:

saw: Index 587 is not a method reference.
CallStack (from HasCallStack):
  error, called at src/Language/JVM/Parser.hs:387:12 in jvm-parser-0.3.0.0-8vikjDDT9aZHOxrZiDNeeN:Language.JVM.Parser

This example was produced with the nightly build of SAW: saw-0.2-2019-01-29-MacOSX-64

In the Instruction datatype, constructors Invokeinterface and Invokestatic both take ClassName and MethodKey arguments, while the related Invokespecial and Invokevirtual constructors take Type instead of ClassName.

According to the JVM spec, all invoke instruction variants require a reference to a class or interface; they do not say that they work with a "class, array, or interface type" like some other instructions do (e.g. new, checkcast, anewarray, etc).

The crucible-jvm package fails during translation if either Invokespecial or Invokevirtual uses a Type that is not a class. The jvm-verifier package handles invoke instructions on array types by substituting in class java/lang/Object for the array type; however, I can't find any such behavior documented in any version of the JVM spec, so I expect that it's just wrong.

We should replace Type arguments with ClassName in the Invokespecial and Invokevirtual constructors so that all four invoke instructions are treated uniformly in accordance with the JVM spec.

As the docstring for Newarray points out, the Type argument is the type of the array itself, and not the element type.

This is both confusing and inconvenient. No code in the library ever produces a Newarray constructor with anything other than an ArrayType argument, so its presence conveys no information. And every function that consumes Instruction values must deal with the possibility of Newarray with a non-ArrayType argument, and implement some error handling for that case (which actually never arises in practice).

It would be much cleaner to change the convention so that Newarray has the element type as its argument.

This change should probably go along with bumping the package version number, as it breaks backward compatibility in a subtle way that is not reflected in the type signatures. We will have to make sure that downstream packages like crucible-jvm are adapted appropriately.

Functions parseTypeDescriptor and parseMethodDescriptor both call the haskell error function in the case of a parse error. This makes them not useful for parsing user-provided strings.

These functions should be changed to return Maybe types, so that a parse error can be indicated with Nothing.

The version on Hackage is outdated: http://hackage.haskell.org/package/jvm-parser