On Mon, 13 May 2024 17:24:39 GMT, Maurizio Cimadamore <[email protected]>
wrote:
>> src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java
>> line 205:
>>
>>> 203: private void verifyAttribute(AttributedElement ae, Attribute<?> a,
>>> List<VerifyError> errors) {
>>> 204: int payLoad = ((BoundAttribute)a).payloadLen();
>>> 205: if (payLoad != switch (a) {
>>
>> Please break this up - e.g. with an intermediate `foundPayload` variable -
>> having a multi-line switch nested as an if condition looks very jarring!
>
> Is this method only supposed to check the attribute size? It would be nice
> perhaps to enhance this to enforce more structural constraints - I added a
> couple of comments in that direction, but there's many more (e.g. for
> instance make sure that any entry that morally points to a class/method is of
> the right kind)
Some of the checks don't verify the attributes point to valid cp entries; since
CF API is lazy, those entries much be expanded by calling the accessors on
Bound attributes.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16809#discussion_r1598822622
