Skip to content

Commit 07d5595

Browse files
jonpryorjonpryor
authored
[class-parse] support Module AttributeInfo (#1097)
Fixes: #1096 Context: https://stackoverflow.com/questions/57358750/module-info-class-file-is-different-in-the-module-jar-file-and-compiled-module-i Context: 678c4bd Context: b274a67 JDK 9 adds support for [modules][0], which are (kinda sorta) like .NET Assemblies: modules can depend upon other modules, export packages, etc. In particular: > **exports and exports…to.** An exports module directive specifies > one of the module’s packages whose `public` types (and their nested > `public` and `protected` types) should be accessible to code in all > other modules. This allows an equivalent to the [C# `internal` access modifier][1]: `public` types in a *non-`export`ed package* should be treated as "internal", while `public` types in an `export`ed package are "fully public". Update `Xamarin.Android.Tools.Bytecode.dll` to extract the module- related information, then update `ClassPath` so that it updates all `public` types *outside* of the "exported" packages to have an `//*/@annotated-visibility` attribute value of `module-info`. (See also commit b274a67, which added `//*/@@annotated-visibility`.) If there is *already* an `//*/@annotated-visibility` value, then we *append* ` module-info` to the attribute value. We use `//*/@annotated-visibility` because we are concerned about introducing an ABI break into AndroidX-related bindings because of type visibility changes. If this isn't a concern, it should be possible to use Metadata to remove those types: <attr path="//class[@annotated-visibility]" name="visibility">kotlin-internal</attr> <attr path="//interface[@annotated-visibility]" name="visibility">kotlin-internal</attr> `class-parse` command-line parsing has been altered. There is now a "global `ClassPath`", which will be used to hold `.class` files provided on the command-line. `.jar` and `.jmod` files provided on the command-line will be given their own `ClassPath` instances, and `module-info.class`-based annotated-visibility fixups are specific to each `ClassPath` instance. Global files are processed together. There is thus no way for `module-info.class` visibility changes from `a.jar` to impact `b.jar`. After visibilities are fixed up, we then merge everything into the "global" `ClassPath` instance before transforming to XML. Additionally, `class-parse --dump` can now accept `.jar` files, and will dump out *all* `.class` filers within the `.jar` file. To make this output easier, each "entry" starts with a "header" of `-- Begin {ClassFile.FullJniName}`, and a blank link will be printed between each entry. `tests/Xamarin.Android.Tools.Bytecode-Tests` has been updated to: 1. Contain a `module-info.java`, which declares a `com.xamarin` module. 2. Add a new `com.xamarin.internal.PublicClassNotInModuleExports` type which is *not* in the `com.xamarin` package, but instead a *nested* package. The type is `public`. 3. Build a `xatb.jar` artifact This makes for a simple one-off test: % dotnet build tests/Xamarin.Android.Tools.Bytecode-Tests/*.csproj % dotnet build tools/class-parse/*.csproj % dotnet bin/Debug-net7.0/class-parse.dll \ tests/Xamarin.Android.Tools.Bytecode-Tests/obj/Debug-net7.0/xatb.jar … <class name="PublicClassNotInModuleExports" … annotated-visibility="module-info" /> Note that `com.xamarin.internal.PublicClassNotInModuleExports` now has an XML attribute `annotated-visibility="module-info"`. Aside: the commit message of 678c4bd sadly overlooked this [clarification][2] for why `kotlin-internal` was introduced: > Note: we introduce and use a new `//*/@visibility` value of > `kotlin-internal` because `internal` is an *existing* value that may > be used in `Metadata.xml` files, e.g. making `public` API `internal` > so that it can still be used in the binding, but isn't *public*. Aside: a discovered oddity: `jar cf …` *modifies* `module-info.class`, adding a `ModulePackages` attribute! (Specifically, if you compare the "on-disk" `module-info.class` to the one within `tests/Xamarin.Android.Tools.Bytecode-Tests/obj/$(Configuration)/xatb.jar`, they differ in size!) [0]: https://www.oracle.com/corporate/features/understanding-java-9-modules.html [1]: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/internal [2]: #793 (comment)
1 parent f0e3300 commit 07d5595

File tree

14 files changed

+475
-46
lines changed

14 files changed

+475
-46
lines changed

‎src/Xamarin.Android.Tools.Bytecode/AttributeInfo.cs‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public class AttributeInfo {
4444
public const string InnerClasses = "InnerClasses";
4545
public const string LocalVariableTable = "LocalVariableTable";
4646
public const string MethodParameters = "MethodParameters";
47+
public const string Module = "Module";
48+
public const string ModulePackages = "ModulePackages";
4749
public const string Signature = "Signature";
4850
public const string SourceFile = "SourceFile";
4951
public const string StackMapTable = "StackMapTable";
@@ -79,6 +81,8 @@ public string Name {
7981
{ typeof (InnerClassesAttribute), InnerClasses },
8082
{ typeof (LocalVariableTableAttribute), LocalVariableTable },
8183
{ typeof (MethodParametersAttribute), MethodParameters },
84+
{ typeof (ModuleAttribute), Module },
85+
{ typeof (ModulePackagesAttribute), ModulePackages },
8286
{ typeof (RuntimeVisibleAnnotationsAttribute), RuntimeVisibleAnnotations },
8387
{ typeof (RuntimeInvisibleAnnotationsAttribute), RuntimeInvisibleAnnotations },
8488
{ typeof (SignatureAttribute), Signature },
@@ -98,6 +102,7 @@ internal static string GetAttributeName<T>()
98102
public static AttributeInfo CreateFromStream (ConstantPool constantPool, Stream stream)
99103
{
100104
var nameIndex = stream.ReadNetworkUInt16 ();
105+
var constant = constantPool [nameIndex];
101106
var name = ((ConstantPoolUtf8Item) constantPool [nameIndex]).Value;
102107
var attr = CreateAttribute (name, constantPool, nameIndex, stream);
103108
return attr;
@@ -114,6 +119,8 @@ static AttributeInfo CreateAttribute (string name, ConstantPool constantPool, us
114119
case InnerClasses: return new InnerClassesAttribute (constantPool, nameIndex, stream);
115120
case LocalVariableTable: return new LocalVariableTableAttribute (constantPool, nameIndex, stream);
116121
case MethodParameters: return new MethodParametersAttribute (constantPool, nameIndex, stream);
122+
case Module: return new ModuleAttribute (constantPool, nameIndex, stream);
123+
case ModulePackages: return new ModulePackagesAttribute (constantPool, nameIndex, stream);
117124
case RuntimeVisibleAnnotations: return new RuntimeVisibleAnnotationsAttribute (constantPool, nameIndex, stream);
118125
case RuntimeInvisibleAnnotations: return new RuntimeInvisibleAnnotationsAttribute (constantPool, nameIndex, stream);
119126
case RuntimeInvisibleParameterAnnotations: return new RuntimeInvisibleParameterAnnotationsAttribute (constantPool, nameIndex, stream);
@@ -503,6 +510,118 @@ public override string ToString ()
503510
}
504511
}
505512

513+
// https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-4.html#jvms-4.7.25
514+
public sealed class ModuleAttribute : AttributeInfo
515+
{
516+
ushort moduleNameIndex;
517+
public string ModuleName {
518+
get {return ((ConstantPoolModuleItem) ConstantPool [moduleNameIndex]).Name.Value;}
519+
}
520+
521+
public ModuleFlags ModuleFlags { get; private set; }
522+
523+
ushort moduleVersionIndex;
524+
public string? ModuleVersion {
525+
get {return ((ConstantPoolUtf8Item) ConstantPool [moduleVersionIndex])?.Value;}
526+
}
527+
528+
public Collection<ModuleRequiresInfo> Requires {get;} = new ();
529+
public Collection<ModuleExportsPackageInfo> Exports {get;} = new ();
530+
public Collection<ModuleOpensPackageInfo> Opens {get;} = new ();
531+
public Collection<ConstantPoolClassItem> Uses {get;} = new ();
532+
public Collection<ModuleProvidesInfo> Provides {get;} = new ();
533+
534+
public ModuleAttribute (ConstantPool constantPool, ushort nameIndex, Stream stream)
535+
: base (constantPool, nameIndex, stream)
536+
{
537+
var attribute_length = stream.ReadNetworkUInt32 ();
538+
539+
moduleNameIndex = stream.ReadNetworkUInt16 ();
540+
ModuleFlags = (ModuleFlags) stream.ReadNetworkUInt16 ();
541+
moduleVersionIndex = stream.ReadNetworkUInt16 ();
542+
543+
var requires_count = stream.ReadNetworkUInt16 ();
544+
for (int i = 0; i < requires_count; ++i) {
545+
Requires.Add (new ModuleRequiresInfo (constantPool, stream));
546+
}
547+
548+
var exports_count = stream.ReadNetworkUInt16 ();
549+
for (int i = 0; i < exports_count; ++i) {
550+
Exports.Add (new ModuleExportsPackageInfo (constantPool, stream));
551+
}
552+
553+
var opens_count = stream.ReadNetworkUInt16 ();
554+
for (int i = 0; i < opens_count; ++i) {
555+
Opens.Add (new ModuleOpensPackageInfo (constantPool, stream));
556+
}
557+
558+
var uses_count = stream.ReadNetworkUInt16 ();
559+
for (int i = 0; i < uses_count; ++i) {
560+
var uses_index = stream.ReadNetworkUInt16 ();
561+
Uses.Add ((ConstantPoolClassItem) constantPool [uses_index]);
562+
}
563+
564+
var provides_count = stream.ReadNetworkUInt16 ();
565+
for (int i = 0; i < provides_count; ++i) {
566+
Provides.Add (new ModuleProvidesInfo (constantPool, stream));
567+
}
568+
}
569+
570+
public override string ToString ()
571+
{
572+
var s = new StringBuilder ()
573+
.Append ("Module(").AppendLine ()
574+
.Append (" ").Append (nameof (ModuleName)).Append ("='").Append (ModuleName).AppendLine ("', ")
575+
.Append (" ").Append (nameof (ModuleVersion)).Append ("='").Append (ModuleVersion).Append ("'");
576+
AppendString (s, nameof (Requires), Requires);
577+
AppendString (s, nameof (Exports), Exports);
578+
AppendString (s, nameof (Opens), Opens);
579+
AppendString (s, nameof (Uses), Uses.Select (u => $"UsesService({u.Name})").ToList ());
580+
AppendString (s, nameof (Provides), Provides);
581+
s.Append (")");
582+
583+
return s.ToString ();
584+
}
585+
586+
static StringBuilder AppendString<T> (StringBuilder s, string collectionName, IList<T> items)
587+
{
588+
if (items.Count == 0) {
589+
return s;
590+
}
591+
s.AppendLine (",");
592+
s.Append (" ").Append (collectionName).AppendLine ("={");
593+
s.Append (" ").Append (items [0]);
594+
for (int i = 1; i < items.Count; ++i) {
595+
s.AppendLine (",");
596+
s.Append (" ");
597+
s.Append (items [i]);
598+
}
599+
return s.Append ("}");
600+
}
601+
}
602+
603+
// https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-4.html#jvms-4.7.26
604+
public sealed class ModulePackagesAttribute : AttributeInfo {
605+
public Collection<ConstantPoolPackageItem> Packages {get;} = new ();
606+
607+
public ModulePackagesAttribute (ConstantPool constantPool, ushort nameIndex, Stream stream)
608+
: base (constantPool, nameIndex, stream)
609+
{
610+
var attribute_length = stream.ReadNetworkUInt32 ();
611+
612+
var package_count = stream.ReadNetworkUInt16 ();
613+
for (int i = 0; i < package_count; ++i) {
614+
var package_index = stream.ReadNetworkUInt16 ();
615+
Packages.Add ((ConstantPoolPackageItem) constantPool [package_index]);
616+
}
617+
}
618+
619+
public override string ToString ()
620+
{
621+
return $"ModulePackages({{{string.Join (", ", Packages.Select (p => p.Name.Value))}}})";
622+
}
623+
}
624+
506625
// https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.16
507626
public sealed class RuntimeVisibleAnnotationsAttribute : AttributeInfo
508627
{

‎src/Xamarin.Android.Tools.Bytecode/ClassFile.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,15 @@ public ClassFile (Stream stream)
4848
Attributes = new AttributeCollection (ConstantPool, stream);
4949

5050
int e = stream.ReadByte ();
51-
if (e >= 0)
52-
throw new BadImageFormatException ("Stream has trailing data?!");
51+
if (e >= 0) {
52+
var trailing = new System.Text.StringBuilder ();
53+
trailing.AppendFormat ("{0:x2}", (byte) e);
54+
while ((e = stream.ReadByte ()) >= 0) {
55+
trailing.Append (" ");
56+
trailing.AppendFormat ("{0:x2}", (byte) e);
57+
}
58+
throw new BadImageFormatException ($"Stream has trailing data?! {{{trailing}}}");
59+
}
5360
}
5461

5562
public static bool IsClassFile (Stream stream)
@@ -213,6 +220,10 @@ public enum ClassAccessFlags {
213220
Synthetic = 0x1000,
214221
Annotation = 0x2000,
215222
Enum = 0x4000,
223+
Module = 0x8000,
224+
225+
// This is not a real Java ClassAccessFlags, it is used to denote non-exported public types
226+
Internal = 0x10000000,
216227
}
217228
}
218229

‎src/Xamarin.Android.Tools.Bytecode/ClassPath.cs‎

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,6 @@ static bool ShouldLoadEntry (ZipArchiveEntry entry)
8888
if (entry.Length == 0)
8989
return false;
9090

91-
if (entry.Name == "module-info.class")
92-
return false;
93-
9491
if (entry.Name.EndsWith (".jnilib", StringComparison.OrdinalIgnoreCase))
9592
return false;
9693

@@ -111,6 +108,14 @@ public void Add (ClassFile classFile)
111108
classFiles.Add (classFile);
112109
}
113110

111+
public void Add (ClassPath classPath, bool removeModules = true)
112+
{
113+
classPath.FixupModuleVisibility (removeModules);
114+
foreach (var c in classPath.classFiles) {
115+
Add (c);
116+
}
117+
}
118+
114119
public ReadOnlyDictionary<string, List<ClassFile>> GetPackages ()
115120
{
116121
return new ReadOnlyDictionary<string, List<ClassFile>> (classFiles
@@ -360,6 +365,7 @@ public XElement ToXElement ()
360365
FixUpParametersFromClasses ();
361366

362367
KotlinFixups.Fixup (classFiles);
368+
FixupModuleVisibility (removeModules: true);
363369

364370
var packagesDictionary = GetPackages ();
365371
var api = new XElement ("api",
@@ -375,6 +381,42 @@ packagesDictionary [p].OrderBy (c => c.ThisClass.Name.Value, StringComparer.Ordi
375381
return api;
376382
}
377383

384+
public void FixupModuleVisibility (bool removeModules)
385+
{
386+
var publicPackages = new HashSet<string> ();
387+
388+
var moduleFiles = classFiles.Where (c => c.AccessFlags == ClassAccessFlags.Module)
389+
.ToList ();
390+
if (moduleFiles.Count == 0) {
391+
return;
392+
}
393+
foreach (var moduleFile in moduleFiles) {
394+
if (removeModules) {
395+
classFiles.Remove (moduleFile);
396+
}
397+
foreach (var moduleAttr in moduleFile.Attributes.OfType<ModuleAttribute> ()) {
398+
foreach (var export in moduleAttr.Exports) {
399+
publicPackages.Add (export.Exports);
400+
}
401+
}
402+
}
403+
404+
foreach (var c in classFiles) {
405+
if (!c.AccessFlags.HasFlag (ClassAccessFlags.Public)) {
406+
continue;
407+
}
408+
var jniName = c.ThisClass.Name.Value;
409+
var packageEnd = jniName.LastIndexOf ('/');
410+
if (packageEnd < 0) {
411+
continue;
412+
}
413+
var package = jniName.Substring (0, packageEnd);
414+
if (!publicPackages.Contains (package)) {
415+
c.AccessFlags = KotlinFixups.SetVisibility (c.AccessFlags, ClassAccessFlags.Internal);
416+
}
417+
}
418+
}
419+
378420
public void SaveXmlDescription (string fileName)
379421
{
380422
var encoding = new UTF8Encoding (encoderShouldEmitUTF8Identifier: false);
@@ -395,5 +437,7 @@ public void SaveXmlDescription (TextWriter textWriter)
395437
contents.Save (writer);
396438
textWriter.WriteLine ();
397439
}
440+
441+
public IEnumerable<ClassFile> GetClassFiles () => classFiles;
398442
}
399443
}

‎src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ static void FixupClassVisibility (ClassFile klass, KotlinClass metadata)
102102
}
103103

104104
// Passing null for 'newVisibility' parameter means 'package-private'
105-
static ClassAccessFlags SetVisibility (ClassAccessFlags existing, ClassAccessFlags? newVisibility)
105+
internal static ClassAccessFlags SetVisibility (ClassAccessFlags existing, ClassAccessFlags? newVisibility)
106106
{
107107
// First we need to remove any existing visibility flags,
108108
// without modifying other flags like Abstract

0 commit comments

Comments
 (0)