Here is a proposed fix: 

https://github.com/jacoco/jacoco/pull/667 

I didn't yet expand it to other validation tests as they depend on the
toolchain JDK. 

Cheers,
-marc 

On 2018-03-29 12:54, Evgeny Mandrikov wrote:

> On Thu, Mar 29, 2018 at 7:30 AM Marc Hoffmann <hoffm...@mountainminds.com> 
> 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 
> [1] : 
> 
>> 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 <hoffm...@mountainminds.com> 
> 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 ? 
> 
> -- 
> 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 jacoco+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/jacoco/CAEPFu696MvK0qA4rqXmOUjTccX4PhkOXNFR3uktgyur5-7%3Dqmg%40mail.gmail.com
>  [2].
> For more options, visit https://groups.google.com/d/optout.

  

Links:
------
[1]
https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7-110
[2]
https://groups.google.com/d/msgid/jacoco/CAEPFu696MvK0qA4rqXmOUjTccX4PhkOXNFR3uktgyur5-7%3Dqmg%40mail.gmail.com?utm_medium=email&amp;utm_source=footer

-- 
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 jacoco+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jacoco/4ef0b939ea874ea3043b78826196c905%40mountainminds.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to