Upgrade ASM to 7.1

[Godin]

Fixes #839

[marchof]

Normally we have an change log entry for this. Should we also have one for a minor version upgrade?

[Godin]

@marchof this is because you picked PR which is still in "implementation" 😆 btw hard to predict url for changelog before creation of PR 😉

[marchof]

Magic number? The intend was more clear with Opcodes.V12 + 1.

[Godin]

@marchof inlining of former implementation of getVersionMajor into this method results in

final int majorVersion = (short) (((b[MAJOR_VERSION_INDEX] & 0xFF) << 8)
		| (b[MAJOR_VERSION_INDEX + 1] & 0xFF));

if (majorVersion == Opcodes.V12 + 1)

which IMO contains far more magic and far less readable than

if (b[6] == 0x00 && b[7] == 0x39)

Can also add comment.

Readable and non-magical alternative

final int majorVersion = java.nio.ByteBuffer.wrap(b)
		.order(java.nio.ByteOrder.BIG_ENDIAN).getShort(6);

looks like a bit of overkill.

[Godin]

BTW numbers 6, 7 and 0x39 are even shown in https://en.wikipedia.org/wiki/Java_class_file 😉

[marchof]

So why inlining? If we keep the previous method (in addition to the new Reader version) we have testable and readable code ;)

[marchof]

@Godin If you like we can merge as is and check in a separate PR how we can streamline version reading/writing. Ok?

[marchof]

@Godin What about requesting a new public API ClassReader.getVersion() from the ASM project?

[marchof]

@Godin With explicit methods to read versions the code becomes way more understandable:

public static ClassReader classReaderFor(final byte[] b) {
	final int originalVersion = getVersion(b);
	if (getVersionMajor(b) == Opcodes.V12 + 1) {
		// temporary downgrade version for ASM
		setVersion(Opcodes.V12, b);
	}
	final ClassReader classReader = new ClassReader(b);
	setVersion(originalVersion, b);
	return classReader;
}

[Godin]

@marchof

What about requesting a new public API ClassReader.getVersion() from the ASM project?

Feel free to ask them 😉 😇

---

Speaking about understandability, testability and magic 🔮 - actually implementation of getVersionMajor(byte[]) in 0.8.3

jacoco/org.jacoco.core/src/org/jacoco/core/internal/instr/InstrSupport.java

Lines 170 to 173 in 6cd3f0b

	public static int getVersionMajor(final byte[] b) {
	return (short) (((b[MAJOR_VERSION_INDEX] & 0xFF) << 8)
	| (b[MAJOR_VERSION_INDEX + 1] & 0xFF));
	}

is incorrect, as well as

final int majorVersion = java.nio.ByteBuffer.wrap(b)
		.order(java.nio.ByteOrder.BIG_ENDIAN).getShort(6);

because according to JVMS major_version is unsigned 😉

For example let's consider major_version that with current temps (2 per year) will be approximately in 16393 years from now 😈 - 32786 (0x8012) and thus following test

final byte[] bytes = new byte[] { //
		(byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE, // magic
		0x00, 0x00, // minor_version
		(byte) 0x80, (byte) 0x12 // major_version
};
assertEquals(0x8000, InstrSupport.getVersionMajor(bytes));

which fails

Expected :32786
Actual   :-32750

IMO such mistake is impossible in case of proposed exact comparison of bytes

if (b[6] == 0x80 && b[7] == 0x12)

😉

However proposed implementation of getVersionMajor(ClassReader)

jacoco/org.jacoco.core/src/org/jacoco/core/internal/instr/InstrSupport.java

Lines 168 to 171 in c7e2176

	public static int getVersionMajor(final ClassReader reader) {
	final int firstConstantPoolEntryOffset = reader.getItem(1) - 1;
	return reader.readShort(firstConstantPoolEntryOffset - 4);
	}

has the same mistake because of readShort which reads signed value.

Both come from the fact that ASM itself uses same signed readShort to read major_version 😆 🤣 This also means that instead of "downgrade" trick, we can actually do "upgrade" trick 🤣 🤣

---

Moreover major_version is 2 bytes and so implementation of needsFrames(int)

jacoco/org.jacoco.core/src/org/jacoco/core/internal/instr/InstrSupport.java

Lines 182 to 185 in 6cd3f0b

	public static boolean needsFrames(final int version) {
	// consider major version only (due to 1.1 anomaly)
	return (version & 0xff) >= Opcodes.V1_6;
	}

is also incorrect and will fail / require change earlier - in about 122 years 😈

assertEquals(true, InstrSupport.needsFrames(0x0100));

Morever similarly to #851 (comment) about getVersionMajor(ClassReader), needsFrames currently always receives major_version and so doesn't need to "consider 1.1 anomaly" ? 😉

---

I added correct implementations of getMajorVersion(byte[]) and setMajorVersion(int, byte[]),
fixed, renamed and commented getMajorVersion(ClassReader). And let's fix needsFrames separately.

[marchof]

@Godin Thanks for fixing this right away!

will fail / require change earlier - in about 122 years

Ok, we document this as know limitation -- along with > 32 dimensional arrays 🤣