On Thu, Mar 29, 2018 at 7:30 AM Marc Hoffmann <[email protected]> wrote:
> Hi Evgeny, > > I was as bit confused at the beginning as we have ClassFileVersionsTest > which tests for existence of frames. > > But after a closer look I realized that this test only checks whether our > generated runtime access methods insert frames. Probe insertion may also > cause frames. The test case contains no code which causes such additional > frames. That's why the test case is probably missing it. > Yep and looking at https://github.com/jacoco/jacoco/commit/4cfef884b41ffca31f15bb6e53e7ef62366c5602 this was overlooked since then and not a regression. > But still confused as we run our integration test suite on Java 5... > StackMapTable attribute was introduced in Java 6, from https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7-110 : The StackMapTable attribute must be recognized and correctly read by a class file reader if the class file's version number is 50.0 or above and the *Java Virtual Machine implementation recognizes class files whose version number is 50.0 or above*. Our validation tests do not perform explicit verification of absence of attribute when compiling into 49 bytecode, nor reading of instrumented classes back, only their loading into JVM and JVM 5 simply ignores this attribute. Also you might remember that loading of class into JVM doesn't guarantee good bytecode verification - https://github.com/jacoco/jacoco/commit/cdd0ec17b976892ad8c8805e90831d0baa470b1a , https://github.com/jacoco/jacoco/issues/626 , etc However this might bother other tools that read classes after our instrumentation and use attribute even if class file version is lower than 50. This AIOBE is kind of example of this: we shoot into own foot - in case of resizing of wide jumps ASM for writing of class performs reading (which is not the case in absence of such resizing) and during this operates with frames regardless of class file version solely based on presence of frames. BTW presence of wide jumps (big methods) is quite rare - just quite recently we fixed https://github.com/jacoco/jacoco/pull/177 and https://github.com/jacoco/jacoco/pull/462 connected with this resizing. On Thu, Mar 29, 2018 at 11:17 AM Marc Hoffmann <[email protected]> wrote: > I'll now try to improve ClassFileVersionsTest to demonstrate the problem. > Maybe better / simpler to add explicit check of absence of frames into all validation tests - e.g. into org.jacoco.core.test.InstrumentingLoader ? <http://twitter.com/_godin_> -- You received this message because you are subscribed to the Google Groups "JaCoCo and EclEmma Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jacoco/CAEPFu696MvK0qA4rqXmOUjTccX4PhkOXNFR3uktgyur5-7%3Dqmg%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
