Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProbeInserter should update indexes of local variables in annotations #894

Merged
merged 4 commits into from Jun 11, 2019

Conversation

Godin
Copy link
Member

@Godin Godin commented Jun 7, 2019

Using

$ javac -version
javac 11.0.3

compilation (javac -g Example.java) of

@java.lang.annotation.Documented
@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.CLASS)
@java.lang.annotation.Target(java.lang.annotation.ElementType.TYPE_USE)
@interface NonNull {
}

class Example {
    Object legacy() {
        return null;
    }

    Object example() {
        @NonNull Object o = legacy();
        return o;
    }
}

results (javap -v -p Example.class) in

  java.lang.Object example();
    descriptor: ()Ljava/lang/Object;
    flags: (0x0000)
    Code:
      stack=1, locals=2, args_size=1
         0: aload_0
         1: invokevirtual #2                  // Method legacy:()Ljava/lang/Object;
         4: astore_1
         5: aload_1
         6: areturn
      LineNumberTable:
        line 13: 0
        line 14: 5
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       7     0  this   LExample;
            5       2     1     o   Ljava/lang/Object;
      RuntimeInvisibleTypeAnnotations:
        0: #18(): LOCAL_VARIABLE, {start_pc=5, length=2, index=1}
          NonNull

after instrumentation (java -jar jacoco-0.8.4/lib/jacococli.jar instrument Example.class --dest instrumented && javap -v -p instrumented/Example.class)

  java.lang.Object example();
    descriptor: ()Ljava/lang/Object;
    flags: (0x0000)
    Code:
      stack=4, locals=3, args_size=1
         0: ldc           #32                 // Dynamic #0:$jacocoData:Ljava/lang/Object;
         2: checkcast     #34                 // class "[Z"
         5: astore_1
         6: aload_0
         7: invokevirtual #2                  // Method legacy:()Ljava/lang/Object;
        10: astore_2
        11: aload_2
        12: aload_1
        13: iconst_2
        14: iconst_1
        15: bastore
        16: areturn
      LineNumberTable:
        line 13: 6
        line 14: 11
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            6      11     0  this   LExample;
           11       6     2     o   Ljava/lang/Object;
      RuntimeInvisibleTypeAnnotations:
        0: #18(): LOCAL_VARIABLE, {start_pc=11, length=6, index=1}
          NonNull

where index in RuntimeInvisibleTypeAnnotations is still 1, whereas should be 2.

This was found during work on #893

@Godin Godin added type: bug 🐛 Something isn't working component: core labels Jun 7, 2019
@Godin Godin self-assigned this Jun 7, 2019
@Godin Godin added this to Implementation in Current work items via automation Jun 7, 2019
@Godin Godin marked this pull request as ready for review June 7, 2019 05:24
@Godin Godin moved this from Implementation to Review in Current work items Jun 7, 2019
@Godin Godin requested a review from marchof June 7, 2019 05:35
Copy link
Member

@marchof marchof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good beside two formal issues.

@marchof marchof merged commit 6c62355 into master Jun 11, 2019
Current work items automation moved this from Review to Done Jun 11, 2019
@marchof marchof deleted the remap_LocalVariableAnnotation branch June 11, 2019 16:16
@Godin Godin added this to the 0.8.5 milestone Jun 11, 2019
@jacoco jacoco locked as resolved and limited conversation to collaborators Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: core type: bug 🐛 Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants