You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Daniel Stuber (JIRA)" <ji...@apache.org> on 2015/04/04 13:14:33 UTC

[jira] [Comment Edited] (BCEL-202) StackMapTableEntry.copy() needs to be deep; Improved support for StackMaps

    [ https://issues.apache.org/jira/browse/BCEL-202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14395240#comment-14395240 ] 

Daniel Stuber edited comment on BCEL-202 at 4/4/15 11:14 AM:
-------------------------------------------------------------

Some more work needs to be done for allowing users to adjust a method's stack-map-table.

Some entry-types have their offset-delta stored in the frame-type. Thus altering the offset-delta may need altering the frame-type, too. It can therefore not be final any more. 

I also suggest to rename the getEntryByteSize() method reduce its visibility to package-private as follows:
StackMapTableEntry.java:
    int calculateLength() {
      if (frame_type >= Constants.SAME_FRAME && frame_type <= Constants.SAME_FRAME_MAX) {
        return 1;
      } else if (frame_type >= Constants.SAME_LOCALS_1_STACK_ITEM_FRAME && frame_type <= Constants.SAME_LOCALS_1_STACK_ITEM_FRAME_MAX) {
          return 1 + (types_of_stack_items[0].hasIndex() ? 3 : 1);
      } else if (frame_type == Constants.SAME_LOCALS_1_STACK_ITEM_FRAME_EXTENDED) {
          return 3 + (types_of_stack_items[0].hasIndex() ? 3 : 1);
      } else if (frame_type >= Constants.CHOP_FRAME && frame_type <= Constants.CHOP_FRAME_MAX) {
        return 3;
      } else if (frame_type == Constants.SAME_FRAME_EXTENDED) {
        return 3;
      } else if (frame_type >= Constants.APPEND_FRAME && frame_type <= Constants.APPEND_FRAME_MAX) {
        int len = 3;
          for (int i = 0; i < types_of_locals.length; i++) {
              len += types_of_locals[i].hasIndex() ? 3 : 1;
          }
          return len;
      } else if (frame_type == Constants.FULL_FRAME) {        
        int len = 7;
          for (int i = 0; i < types_of_locals.length; i++) {
              len += types_of_locals[i].hasIndex() ? 3 : 1;
          }
          for (int i = 0; i < types_of_stack_items.length; i++) {
              len += types_of_stack_items[i].hasIndex() ? 3 : 1;
          }
          return len;
      } else {
          /* Can't happen */
          throw new ClassFormatException ("Invalid Stack map table tag: " + frame_type);
      }
    }


Furthermore it might be necessary to completely replace a stack-map-entry with another element resulting in a different length of the whole stack-map-table. I do this as follows:
StackMapTable.java
    public final void setStackMapTable( StackMapTableEntry[] map ) {
        this.map = map;

        int len = 2;
        for (int i = 0; i < map.length; i++) {
          len += map[i].calculateLength();
        }
        setLength(len);
    }

This enforces a slight change in one constructor by directly setting the map variable rather than calling setStackMapTable(...):
    public StackMapTable(int name_index, int length, StackMapTableEntry[] map, ConstantPool constant_pool) {
        super(Constants.ATTR_STACK_MAP_TABLE, name_index, length, constant_pool);
        this.map = map;
    }



was (Author: daniel.stuber@corix.ch):
Some more work needs to be done for allowing users to adjust a method's stack-map-table.

Some entry-types have their offset-delta stored in the frame-type. Thus altering the offset-delta may need altering the frame-type, too. It can therefore not be final any more. I also suggest to reduce the visibility of the method to package-private.
StackMapTableEntry.java:
    int calculateLength() {
      if (frame_type >= Constants.SAME_FRAME && frame_type <= Constants.SAME_FRAME_MAX) {
        return 1;
      } else if (frame_type >= Constants.SAME_LOCALS_1_STACK_ITEM_FRAME && frame_type <= Constants.SAME_LOCALS_1_STACK_ITEM_FRAME_MAX) {
          return 1 + (types_of_stack_items[0].hasIndex() ? 3 : 1);
      } else if (frame_type == Constants.SAME_LOCALS_1_STACK_ITEM_FRAME_EXTENDED) {
          return 3 + (types_of_stack_items[0].hasIndex() ? 3 : 1);
      } else if (frame_type >= Constants.CHOP_FRAME && frame_type <= Constants.CHOP_FRAME_MAX) {
        return 3;
      } else if (frame_type == Constants.SAME_FRAME_EXTENDED) {
        return 3;
      } else if (frame_type >= Constants.APPEND_FRAME && frame_type <= Constants.APPEND_FRAME_MAX) {
        int len = 3;
          for (int i = 0; i < types_of_locals.length; i++) {
              len += types_of_locals[i].hasIndex() ? 3 : 1;
          }
          return len;
      } else if (frame_type == Constants.FULL_FRAME) {        
        int len = 7;
          for (int i = 0; i < types_of_locals.length; i++) {
              len += types_of_locals[i].hasIndex() ? 3 : 1;
          }
          for (int i = 0; i < types_of_stack_items.length; i++) {
              len += types_of_stack_items[i].hasIndex() ? 3 : 1;
          }
          return len;
      } else {
          /* Can't happen */
          throw new ClassFormatException ("Invalid Stack map table tag: " + frame_type);
      }
    }


Furthermore it might be necessary to completely replace a stack-map-entry with another element resulting in a different length of the whole stack-map-table. I do this as follows:
StackMapTable.java
    public final void setStackMapTable( StackMapTableEntry[] map ) {
        this.map = map;

        int len = 2;
        for (int i = 0; i < map.length; i++) {
          len += map[i].calculateLength();
        }
        setLength(len);
    }

This enforces a slight change in one constructor by directly setting the map variable rather than calling setStackMapTable():
    public StackMapTable(int name_index, int length, StackMapTableEntry[] map, ConstantPool constant_pool) {
        super(Constants.ATTR_STACK_MAP_TABLE, name_index, length, constant_pool);
        this.map = map;
    }


> StackMapTableEntry.copy() needs to be deep; Improved support for StackMaps
> --------------------------------------------------------------------------
>
>                 Key: BCEL-202
>                 URL: https://issues.apache.org/jira/browse/BCEL-202
>             Project: Commons BCEL
>          Issue Type: Bug
>            Reporter: Mark Roberts
>         Attachments: stackmap.diff
>
>
> There are several ways a user can modify a Java class file that should cause BCEL to update the StackMaps automatically.  Unfortunately, it does not.  These additional methods at least allow users to take care of these issues for themselves.
> The patch also fixes a bug - StackMapTableEntry.copy() needs to be a deep copy to prevent StackMapTypes from being reused.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)