Skip to content

Commit b18bfdf

Browse files
committed
[class-parse] support Module AttributeInfo
Fixes: dotnet#1096 Context: https://stackoverflow.com/questions/57358750/module-info-class-file-is-different-in-the-module-jar-file-and-compiled-module-i Context: 678c4bd 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, the update `XmlClassDeclarationBuilder` so that it updates all `public` types *outside* of the "exported" packages to have a visibility of `kotlin-internal`. Why a `//*/@visibility` value of `kotlin-internal`? From a [suggestion][2] for the commit message of 678c4bd, which was sadly overlooked in the final merge: > 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*. If we use `internal`, *those types are still bound*, it's just that the bound types have C# `internal` visibility, while we *want* them to be *skipped entirely*. A visibility value of `kotlin-internal` allows us to skip them, which is desired. `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" … visibility="kotlin-internal" /> Note that `com.xamarin.internal.PublicClassNotInModuleExports` is now shown as `kotlin-internal` instead of `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]: dotnet#793 (comment)
1 parent f0e3300 commit b18bfdf

13 files changed

+410
-10
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: 35 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

@@ -360,6 +357,7 @@ public XElement ToXElement ()
360357
FixUpParametersFromClasses ();
361358

362359
KotlinFixups.Fixup (classFiles);
360+
FixupModuleVisibility ();
363361

364362
var packagesDictionary = GetPackages ();
365363
var api = new XElement ("api",
@@ -375,6 +373,40 @@ packagesDictionary [p].OrderBy (c => c.ThisClass.Name.Value, StringComparer.Ordi
375373
return api;
376374
}
377375

376+
void FixupModuleVisibility ()
377+
{
378+
var publicPackages = new HashSet<string> ();
379+
380+
var moduleFiles = classFiles.Where (c => c.AccessFlags == ClassAccessFlags.Module)
381+
.ToList ();
382+
if (moduleFiles.Count == 0) {
383+
return;
384+
}
385+
foreach (var moduleFile in moduleFiles) {
386+
classFiles.Remove (moduleFile);
387+
foreach (var moduleAttr in moduleFile.Attributes.OfType<ModuleAttribute> ()) {
388+
foreach (var export in moduleAttr.Exports) {
389+
publicPackages.Add (export.Exports);
390+
}
391+
}
392+
}
393+
394+
foreach (var c in classFiles) {
395+
if (!c.AccessFlags.HasFlag (ClassAccessFlags.Public)) {
396+
continue;
397+
}
398+
var jniName = c.ThisClass.Name.Value;
399+
var packageEnd = jniName.LastIndexOf ('/');
400+
if (packageEnd < 0) {
401+
continue;
402+
}
403+
var package = jniName.Substring (0, packageEnd);
404+
if (!publicPackages.Contains (package)) {
405+
c.AccessFlags = KotlinFixups.SetVisibility (c.AccessFlags, ClassAccessFlags.Internal);
406+
}
407+
}
408+
}
409+
378410
public void SaveXmlDescription (string fileName)
379411
{
380412
var encoding = new UTF8Encoding (encoderShouldEmitUTF8Identifier: false);

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)