On Thu, 16 Feb 2023 13:40:49 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> added 4-byte Unicode text to Utf8EntryTest
>
> src/java.base/share/classes/jdk/internal/classfile/constantpool/ConstantPoolBuilder.java
> line 75:
>
>> 73: static ConstantPoolBuilder of(ClassModel classModel) {
>> 74: ClassReader reader = (ClassReader) classModel.constantPool();
>> 75: return reader.optionValue(Classfile.Option.Key.CP_SHARING)
>
> Question: the fact that, when transforming, the new constant pool is the same
> as the old one, plus some "appended" entries at the end, is a pretty crucial
> property to ensure that existing code that is not transformed (e.g. unknown
> attributes) remains legal after transformation. But it seems that, if the
> `CP_SHARING` option is not passed, we build a brand new constant pool, in
> which case there's no guarantee that old indices will still point to the same
> position. Doesn't that lead to issue with unknown attributes?
>
> I guess my question is: shouldn't the semantics properties/guarantees of a
> classfile transform be specified regardless of the options being passed in?
> E.g. it's ok if using different options ends up with different performance
> model, but I'm a bit worried if options can affect correctness of the
> transform?
If you propose `constantPoolSharing(false)` should automatically imply
`processUnknownAttributes(false)` - it would avoid potential invalid CP entries
in the unknown attributes and we may consider that dependency between the two
options.
However on the other side there might be unknown attributes completely
independent of CP and for such the transformation is safe. Strong options
implication would force users to create custom attribute mappers for such
attributes.
Current options independence gives user power to configure transformations
flexibly, however on the other side it allows to produce classes with invalid
CP entries in unknown attributes.
-------------
PR: https://git.openjdk.org/jdk/pull/10982
