Skip to content

Commit

Permalink
[class-parse] .jmod file support (#891)
Browse files Browse the repository at this point in the history
Context: #858
Context: https://stackoverflow.com/questions/44732915/why-did-java-9-introduce-the-jmod-file-format/64202720#64202720
Context: https://openjdk.java.net/projects/jigsaw/
Context: xamarin/monodroid@c9e5cbd

JDK 9 replaced the "venerable" (and huge, ~63MB) `jre/lib/rt.jar`
with a set of `.jmod` files.

Thus, as of JDK 9, there is no `.jar` file to try to parse with
`class-parse`, only `.jmod` files!

A `.jmod` file, in turn, is still a ZIP container, much like `.jar`
files, but:

 1. With a different internal directory structure, and
 2. With a custom file header.

The result of (2) is that while `unzip -l` can show and extract the
contents of a `.jmod` file -- with a warning --
`System.IO.Compression.ZipArchive` cannot process the file:

	% mono …/class-parse.exe $HOME/android-toolchain/jdk-11/jmods/java.base.jmod
	class-parse: Unable to read file 'java.base.jmod': Number of entries expected in End Of Central Directory does not correspond to number of entries in Central Directory.
	<api
	  api-source="class-parse" />

Update `Xamarin.Android.Tools.Bytecode.ClassPath` to support `.jmod`
files by using `PartialStream` (73096d9) to skip the first 4 bytes.

Once able to read a `.jmod` file, lots of debug messages appeared
while parsing `java.base.jmod`, a'la:

	class-parse: method com/xamarin/JavaType$1MyStringList.<init>(Lcom/xamarin/JavaType;Ljava/lang/String;ILjava/lang/StringBuilder;)V:
	  Local variables array has 2 entries ('LocalVariableTableAttribute(
	      LocalVariableTableEntry(Name='this', Descriptor='Lcom/xamarin/JavaType$1MyStringList;', StartPC=0, Index=0),
	      LocalVariableTableEntry(Name='this$0', Descriptor='Lcom/xamarin/JavaType;', StartPC=0, Index=1),
	      LocalVariableTableEntry(Name='a', Descriptor='Ljava/lang/String;', StartPC=0, Index=2),
	      LocalVariableTableEntry(Name='b', Descriptor='I', StartPC=0, Index=3))'
	  ); descriptor has 3 entries!
	class-parse: method com/xamarin/JavaType$1MyStringList.<init>(Lcom/xamarin/JavaType;Ljava/lang/String;ILjava/lang/StringBuilder;)V:
	Signature ('Signature((Ljava/lang/String;I)V)') has 2 entries; Descriptor '(Lcom/xamarin/JavaType;Ljava/lang/String;ILjava/lang/StringBuilder;)V' has 3 entries!

This was a variation on the "JDK 8?" block that previously didn't
have much detail, in part because it didn't have a repro.  Now we
have a repro, based on [JDK code][0] which contains a class
declaration within a method declaration

	// Java
	public List<String> staticActionWithGenerics(…) {
	    class MyStringList extends ArrayList<String> {
	        public MyStringList(String a, int b) {
	        }
	        public String get(int index) {
	            return unboundedList.toString() + value1.toString();
	        }
	    }
	}

The deal is that `staticActionWithGenerics()` contains a `MyStringList`
class, which in turn contains a constructor with two parameters.
*However*, as far as Java bytecode is concerned, the constructor
contains *3* local variables with StartPC==0, which is what we use to
infer parameter names.

Refactor, cleanup, and otherwise modify huge swaths of `Methods.cs`
to get to a "happy medium" of:

  * No warnings from our unit tests, ensured by updating
    `ClassFileFixture` to have a `[SetUp]` method which sets the
    `Log.OnLog` field to a delegate which may call `Assert.Fail()`
    when invoked.  This asserts for all messages starting with
    `class-parse: methods`, which are produced by `Methods.cs`.

  * No warnings when processing `java.base.jmod`:

        % mono bin/Debug/class-parse.exe $HOME/android-toolchain/jdk-11/jmods/java.base.jmod >/dev/null
        # no error messages

  * No warnings when processing Android API-31:

        % mono bin/Debug/class-parse.exe $HOME/android-toolchain/sdk/platforms/android-31/android.jar >/dev/null
        # no error messages

Additionally, improve `Log.cs` so that there are `M(string)`
overloads for the existing `M(string, params object[])` methods.
This is a "sanity-preserving" change, as "innocuous-looking" code
such as `Log.Debug("{foo}")` will throw `FormatException` when the
`(string, params object[])` overload is used.

Aside: closures are *weird* and finicky.
Consider the following Java code:

	class ClosureDemo {
	    public void m(String a) {
	        class Example {
	            public Example(int x) {
	                System.out.println (a);
	            }
	        }
	    }
	}

It looks like the JNI signature for the `Example` constructor might
be `(I)V`, but isn't.  It is instead:

	(LClosureDemo;ILjava/lang/String;)V

Breaking that down:

  * `LClosureDemo;`: `Example` is an inner class, and thus has an
    implicit reference to the containing type.  OK, easy to forget.

  * `I`: the `int x` parameter.  Expected.

  * `Ljava/lang/String`: the `String a` parameter from the enclosing
    scope!  This is the closure parameter.

This does make sense.  The problem is that it's *selective*: only
variables used within `Example` become extra parameters.
If the `Example` constructor is updated to remove the
`System.out.println(a)` statement, then `a` is no longer used, and
is no longer present as a constructor parameter.

The only way I found to "reasonably" determine if a constructor
parameter was a closure parameter was by checking all fields in the
class with names starting with `val$`, and then comparing the types
of those fields to types within the enclosing method's descriptor.
I can't think of a way to avoid using `val$`. :-(

Another aside: closure parameter behavior *differs* between JDK 1.8
and JDK-11: there appears to be a JDK 1.8 `javac` bug in which it
assigns the *wrong* parameter names.  Consider `MyStringList`:
The Java constructor declaration is:

	public static <T, …>
	void staticActionWithGenerics (
	        T value1, …
	        List<?> unboundedList, …)
	{
	    class MyStringList extends ArrayList<String> {
	        public MyStringList(String a, int b) {
	        }
	        // …
	    }
	}

The JNI signature for the `MyStringList` constructor is:

	(Ljava/lang/String;ILjava/util/List;Ljava/lang/Object;)V

which is:

  * `String`: parameter `a`
  * `I`: parameter `b`
  * `List`: closure parameter for `unboundedList`
  * `Object`: closure parameter for `value1`.

If we build with JDK 1.8 `javac -parameters`, the `MethodParameters`
annotation states that the closure parameters are:

	MyStringList(String a, int b, List val$value1, Object val$unboundedList);

which is *wrong*; `unboundedList` is the `List`, `value1` is `Object`!

This was fixed in JDK-11, with the `MethodParameters` annotations
specifying:

	MyStringList(String a, int b, List val$unboundedList, Object val$value1);

This means that the unit tests need to take this into consideration.

Add a new `ConfiguredJdkInfo` class, which contains code similar to
`tests/TestJVM/TestJVM.cs`: it will read `bin/Build*/JdkInfo.props`
to find the path to the JDK found in `make prepare`, and then
determine the JDK version from that directory's `release` file.

[0]: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/WhileOps.java#L334
  • Loading branch information
jonpryor authored Oct 15, 2021
1 parent 7c8d463 commit 8ccb837
Show file tree
Hide file tree
Showing 15 changed files with 912 additions and 106 deletions.
2 changes: 2 additions & 0 deletions src/Xamarin.Android.Tools.Bytecode/AttributeInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,8 @@ public string InnerName {
}
}

public string OuterClassName => OuterClass?.Name?.Value;

public override string ToString ()
{
return string.Format ("InnerClass(InnerClass='{0}', OuterClass='{1}', InnerName='{2}', InnerClassAccessFlags={3})",
Expand Down
37 changes: 31 additions & 6 deletions src/Xamarin.Android.Tools.Bytecode/ClassPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,22 @@ public ClassPath (string path = null)
Load (path);
}

public void Load (string jarFile)
public void Load (string filePath)
{
if (!IsJarFile (jarFile))
throw new ArgumentException ("'jarFile' is not a valid .jar file.", "jarFile");

using (var jarStream = File.OpenRead (jarFile)) {
Load (jarStream);
if (IsJmodFile (filePath)) {
using (var source = File.OpenRead (filePath)) {
var slice = new PartialStream (source, 4);
Load (slice);
}
return;
}
if (IsJarFile (filePath)) {
using (var jarStream = File.OpenRead (filePath)) {
Load (jarStream);
}
return;
}
throw new ArgumentException ($"`{filePath}` is not a supported file format.", nameof (filePath));
}

public void Load (Stream jarStream, bool leaveOpen = false)
Expand Down Expand Up @@ -113,6 +121,23 @@ public static bool IsJarFile (string jarFile)
}
}

public static bool IsJmodFile (string jmodFile)
{
if (jmodFile == null)
throw new ArgumentNullException (nameof (jmodFile));
try {
var f = File.OpenRead (jmodFile);
var h = new byte[4];
if (f.Read (h, 0, h.Length) != 4) {
return false;
}
return h[0] == 0x4a && h[1] == 0x4d && h[2] == 0x01 && h[3] == 0x00;
}
catch (Exception) {
return false;
}
}

XAttribute GetApiSource ()
{
if (string.IsNullOrEmpty (ApiSource))
Expand Down
8 changes: 8 additions & 0 deletions src/Xamarin.Android.Tools.Bytecode/Log.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public static void Warning (int verbosity, string format, params object[] args)
log (TraceLevel.Warning, verbosity, format, args);
}

public static void Warning (int verbosity, string message) => Warning (verbosity, "{0}", message);

public static void Error (string format, params object[] args)
{
var log = OnLog;
Expand All @@ -23,6 +25,8 @@ public static void Error (string format, params object[] args)
log (TraceLevel.Error, 0, format, args);
}

public static void Error (string message) => Error ("{0}", message);

public static void Message (string format, params object[] args)
{
var log = OnLog;
Expand All @@ -31,13 +35,17 @@ public static void Message (string format, params object[] args)
log (TraceLevel.Info, 0, format, args);
}

public static void Message (string message) => Message ("{0}", message);

public static void Debug (string format, params object[] args)
{
var log = OnLog;
if (log == null)
return;
log (TraceLevel.Verbose, 0, format, args);
}

public static void Debug (string message) => Debug ("{0}", message);
}
}

Loading

0 comments on commit 8ccb837

Please sign in to comment.