Skip to content

JustIce: represent the attributes of a module descriptor in StringRepresentation - #543

Closed
simpleqt wants to merge 1 commit into
apache:masterfrom
simpleqt:fix/stringrepresentation-module
Closed

simpleqt wants to merge 1 commit into
apache:masterfrom
simpleqt:fix/stringrepresentation-module

Conversation

@simpleqt

@simpleqt simpleqt commented Nov 4, 2026

Copy link
Copy Markdown

StringRepresentation has no representation for the attributes of a module descriptor, so JustIce fails with an internal error instead of reporting them:

VerifierFactory.getVerifier("module-info").doPass2();
// org.apache.bcel.verifier.exc.AssertionViolatedException: INTERNAL ERROR:
// Please adapt 'class org.apache.bcel.verifier.statics.StringRepresentation'
// to deal with objects of class 'class org.apache.bcel.classfile.Module'.

Pass2Verifier reports every class attribute it does not know through StringRepresentation (Pass2Verifier.java, visitJavaClass), and the contract of StringRepresentation is that toString() never throws for a Node ("A (new StringRepresentation(Node n)).toString() never throws any exception", StringRepresentation.java). The Module attribute, the ModulePackages and ModuleMainClass attributes, and the ModuleExports, ModuleOpens, ModuleProvides and ModuleRequires entries of a module descriptor were the remaining class file nodes without a visit method, so any module descriptor ended in that internal error.

How

The missing visit methods are added, following the pattern of the attribute types that are already handled (tostring = toString(obj);):

  • visitModule, visitModuleExports, visitModuleMainClass, visitModuleOpens, visitModulePackages, visitModuleProvides, visitModuleRequires.

With this change doPass2() on a module descriptor completes: the Module attribute is reported as an attribute the verifier does not know and the class file is then rejected with the existing, accurate message "Superclass of 'module-info' missing but not java.lang.Object itself!" (a module descriptor has no superclass), instead of an internal error.

Tests

src/test/java/org/apache/bcel/verifier/statics/StringRepresentationTest.java uses the existing src/test/resources/jpms/java17/commons-io/module-info.class fixture:

  • the Module attribute has a string representation that equals its toString();
  • all of its requires, exports, opens and provides entries do too;
  • ModulePackages and ModuleMainClass do too (built on the fixture's constant pool);
  • doPass2() on the module descriptor does not throw.

Result with the fix: Tests run: 4, Failures: 0, Errors: 0, Skipped: 0. Without the fix (only the main source change reverted) the same four tests fail with 1 failure and 3 errors:

AssertionViolatedException: INTERNAL ERROR: Please adapt 'class org.apache.bcel.verifier.statics.StringRepresentation'
  to deal with objects of class 'class org.apache.bcel.classfile.Module'.
AssertionViolatedException: INTERNAL ERROR: Please adapt ... 'class org.apache.bcel.classfile.ModulePackages'.
AssertionViolatedException: INTERNAL ERROR: Please adapt ... 'class org.apache.bcel.classfile.ModuleRequires'.
AssertionFailedError: Unexpected exception thrown: ... class 'class org.apache.bcel.classfile.Module'.

Full build: mvn test -> Tests run: 3441, Failures: 0, Errors: 0, Skipped: 17, mvn checkstyle:check -> 0 violations.

AI disclosure

  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?

The change was produced with the DeepSeek Harness (dsv41) coding agent: it found the failing path by running the verifier over a corpus of javac compiled class files and module descriptors, wrote the fix and the test, and ran the build. I reviewed the diff and the test results, and I am responsible for the contribution. The commit carries the Assisted-By: trailer required by commons-parent/AGENTS.md.

StringRepresentation reports every class file attribute that the verifier does
not know, and its contract is that toString() never throws for a Node. It had
no representation for the Module attribute of a module descriptor, for the
ModulePackages and ModuleMainClass attributes, or for the ModuleExports,
ModuleOpens, ModuleProvides and ModuleRequires entries, so verifying a module
descriptor failed with an internal error:

  org.apache.bcel.verifier.exc.AssertionViolatedException: INTERNAL ERROR:
  Please adapt 'class org.apache.bcel.verifier.statics.StringRepresentation'
  to deal with objects of class 'class org.apache.bcel.classfile.Module'.

Add the missing visit methods, following the pattern of the attribute types
that are already handled. Pass 2 of the verifier now reports the Module
attribute and rejects the module descriptor with the existing message about
its missing superclass instead of throwing an internal error.

Assisted-By: DeepSeek Harness (dsv41)
Copilot AI balanced review requested due to automatic review settings November 4, 2026 20:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The tests leave module opens/provides visitors unexercised and accept a rendering fallback caused by an incorrect constant type, allowing representation defects to go undetected.

2 open issues

Review effort: Balanced

Comment on lines +96 to +98
entries.addAll(Arrays.asList(module.getOpensTable()));
entries.addAll(Arrays.asList(module.getProvidesTable()));
assertFalse(entries.isEmpty(), "The module descriptor is expected to have entries");
Comment on lines +108 to +115
final ConstantPool constantPool = constantPoolGen.getConstantPool();
final ModulePackages modulePackages = new ModulePackages(constantPoolGen.addUtf8("ModulePackages"), 0,
new int[] {constantPoolGen.addClass("org.apache.commons.io.IOUtils")}, constantPool);
final ModuleMainClass moduleMainClass = new ModuleMainClass(constantPoolGen.addUtf8("ModuleMainClass"), 0, moduleInfo.getClassNameIndex(),
constantPool);
for (final Node node : Arrays.asList(modulePackages, moduleMainClass)) {
assertNotNull(new StringRepresentation(node).toString(), node.getClass().getName());
}
@garydgregory

Copy link
Copy Markdown
Member

@simpleqt
Pleas review the Copilot comments.

garydgregory added a commit that referenced this pull request Nov 7, 2026
- Add visitors for module attributes and entries so verifier diagnostics
no
longer throw AssertionViolatedException for these nodes.
- See PR #543
@garydgregory

Copy link
Copy Markdown
Member

Closing: Done with better tests in git master. Thank you for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants