You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Andrew Cornwall (JIRA)" <ji...@apache.org> on 2008/03/26 18:59:38 UTC

[jira] Created: (HARMONY-5654) [classlib][pack200] Init methods cannot be synchronized

[classlib][pack200] Init methods cannot be synchronized
-------------------------------------------------------

                 Key: HARMONY-5654
                 URL: https://issues.apache.org/jira/browse/HARMONY-5654
             Project: Harmony
          Issue Type: Bug
          Components: Classlib
         Environment: All pack200
            Reporter: Andrew Cornwall


In the current code, it's possible for a CPMethod  <init> to be emitted as synchronized. This will not pass Sun's verification. For instance, if you run pack200 on itself, NewAttributeBands$LayoutElement in the following will be emitted with a synchronized <init>.

I've temporarily hacked this with the following code in CPMethod:

	public CPMethod(CPUTF8 name, CPUTF8 descriptor, long flags, List attributes) {
		// TODO Check that we only pass these on, or remap
        super(name, descriptor, 0x7FFF & flags, attributes);
	    if(name.underlyingString().equals("<init>")) {
	        // hack hack - init should never be synchronized
	        SegmentUtils.debug("Hacking off synchronized on an init method");
	        this.flags = (short)(this.flags & 0x7FDF);
	    }
	}

but I really need to look into it more deeply and find out why the 0x20 flag is being passed in. (This may end up being an encoding issue, but I need to look more first).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HARMONY-5654) [classlib][pack200] Init methods cannot be synchronized

Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HARMONY-5654?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12583895#action_12583895 ] 

Andrew Cornwall commented on HARMONY-5654:
------------------------------------------

It appears the right thing to do is not to add the synthetic flag. (In other words, I think the code is correct.) The Sun-decompiled version of the .pack.gz archive has:

  method {
    access_flag = x00  // 
    name_index = #3  // "<init>"
    descriptor_index = #28  // "(Lorg/apache/harmony/pack200/NewAttributeBands;Lorg/apache/harmony/pack200/NewAttributeBands$LayoutElement;)V"
    attributes_count = 2
      attributes {
        attribute Synthetic {
          attribute_name_index = #11  // "Synthetic"
          attribute_length = 0
        }
        attribute Code {
          attribute_name_index = #5  // "Code"
          attribute_length = 38
          max_stack = 2
          max_locals = 3
          code_length = 6
          code asm {
          0:	aload_0
...

and we have:
  method {
    access_flag = x00  // 
    name_index = #2  // "<init>"
    descriptor_index = #21  // "(Lorg/apache/harmony/pack200/NewAttributeBands;Lorg/apache/harmony/pack200/NewAttributeBands$LayoutElement;)V"
    attributes_count = 2
      attributes {
        attribute Synthetic {
          attribute_name_index = #37  // "Synthetic"
          attribute_length = 0
        }
        attribute Code {
          attribute_name_index = #30  // "Code"
          attribute_length = 38
          max_stack = 2
          max_locals = 3
          code_length = 6
          code asm {
          0:	aload_0
...

which looks right to me. I'm wondering now if a similar situation exists for the BRIDGE flag?


> [classlib][pack200] Init methods cannot be synchronized
> -------------------------------------------------------
>
>                 Key: HARMONY-5654
>                 URL: https://issues.apache.org/jira/browse/HARMONY-5654
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>         Environment: All pack200
>            Reporter: Andrew Cornwall
>            Assignee: Sian January
>         Attachments: pack200.pack.gz
>
>
> In the current code, it's possible for a CPMethod  <init> to be emitted as synchronized. This will not pass Sun's verification. For instance, if you run pack200 on itself, NewAttributeBands$LayoutElement in the following will be emitted with a synchronized <init>.
> I've temporarily hacked this with the following code in CPMethod:
> 	public CPMethod(CPUTF8 name, CPUTF8 descriptor, long flags, List attributes) {
> 		// TODO Check that we only pass these on, or remap
>         super(name, descriptor, 0x7FFF & flags, attributes);
> 	    if(name.underlyingString().equals("<init>")) {
> 	        // hack hack - init should never be synchronized
> 	        SegmentUtils.debug("Hacking off synchronized on an init method");
> 	        this.flags = (short)(this.flags & 0x7FDF);
> 	    }
> 	}
> but I really need to look into it more deeply and find out why the 0x20 flag is being passed in. (This may end up being an encoding issue, but I need to look more first).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HARMONY-5654) [classlib][pack200] Init methods cannot be synchronized

Posted by "Sian January (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HARMONY-5654?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12583652#action_12583652 ] 

Sian January commented on HARMONY-5654:
---------------------------------------

I've checked in a fix at r642960, but I'm not sure whether I should add the synthetic flag back in or not. Andrew would you be able to check this for me against Sun's if you've already got something set up for looking at the class files?  Thanks very much.

> [classlib][pack200] Init methods cannot be synchronized
> -------------------------------------------------------
>
>                 Key: HARMONY-5654
>                 URL: https://issues.apache.org/jira/browse/HARMONY-5654
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>         Environment: All pack200
>            Reporter: Andrew Cornwall
>            Assignee: Sian January
>         Attachments: pack200.pack.gz
>
>
> In the current code, it's possible for a CPMethod  <init> to be emitted as synchronized. This will not pass Sun's verification. For instance, if you run pack200 on itself, NewAttributeBands$LayoutElement in the following will be emitted with a synchronized <init>.
> I've temporarily hacked this with the following code in CPMethod:
> 	public CPMethod(CPUTF8 name, CPUTF8 descriptor, long flags, List attributes) {
> 		// TODO Check that we only pass these on, or remap
>         super(name, descriptor, 0x7FFF & flags, attributes);
> 	    if(name.underlyingString().equals("<init>")) {
> 	        // hack hack - init should never be synchronized
> 	        SegmentUtils.debug("Hacking off synchronized on an init method");
> 	        this.flags = (short)(this.flags & 0x7FDF);
> 	    }
> 	}
> but I really need to look into it more deeply and find out why the 0x20 flag is being passed in. (This may end up being an encoding issue, but I need to look more first).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (HARMONY-5654) [classlib][pack200] Init methods cannot be synchronized

Posted by "Sian January (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HARMONY-5654?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sian January reassigned HARMONY-5654:
-------------------------------------

    Assignee: Sian January

> [classlib][pack200] Init methods cannot be synchronized
> -------------------------------------------------------
>
>                 Key: HARMONY-5654
>                 URL: https://issues.apache.org/jira/browse/HARMONY-5654
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>         Environment: All pack200
>            Reporter: Andrew Cornwall
>            Assignee: Sian January
>         Attachments: pack200.pack.gz
>
>
> In the current code, it's possible for a CPMethod  <init> to be emitted as synchronized. This will not pass Sun's verification. For instance, if you run pack200 on itself, NewAttributeBands$LayoutElement in the following will be emitted with a synchronized <init>.
> I've temporarily hacked this with the following code in CPMethod:
> 	public CPMethod(CPUTF8 name, CPUTF8 descriptor, long flags, List attributes) {
> 		// TODO Check that we only pass these on, or remap
>         super(name, descriptor, 0x7FFF & flags, attributes);
> 	    if(name.underlyingString().equals("<init>")) {
> 	        // hack hack - init should never be synchronized
> 	        SegmentUtils.debug("Hacking off synchronized on an init method");
> 	        this.flags = (short)(this.flags & 0x7FDF);
> 	    }
> 	}
> but I really need to look into it more deeply and find out why the 0x20 flag is being passed in. (This may end up being an encoding issue, but I need to look more first).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (HARMONY-5654) [classlib][pack200] Init methods cannot be synchronized

Posted by "Sian January (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HARMONY-5654?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sian January resolved HARMONY-5654.
-----------------------------------

       Resolution: Fixed
    Fix Version/s: 5.0M6

I guess we can assume that if something has been redefined it's an attribute rather than a flag.  I'm not sure about the case when we have BRIDGE or SYNTHETIC flags that haven't been redefined though (or even ANNOTATION or ENUM which are Java 5 specific), but it's probably safe to assume that it's ok to have them as flags I think.  

I'm going to resolve this issue for now, but if you think there's further work to do please re-open it.

> [classlib][pack200] Init methods cannot be synchronized
> -------------------------------------------------------
>
>                 Key: HARMONY-5654
>                 URL: https://issues.apache.org/jira/browse/HARMONY-5654
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>         Environment: All pack200
>            Reporter: Andrew Cornwall
>            Assignee: Sian January
>             Fix For: 5.0M6
>
>         Attachments: pack200.pack.gz
>
>
> In the current code, it's possible for a CPMethod  <init> to be emitted as synchronized. This will not pass Sun's verification. For instance, if you run pack200 on itself, NewAttributeBands$LayoutElement in the following will be emitted with a synchronized <init>.
> I've temporarily hacked this with the following code in CPMethod:
> 	public CPMethod(CPUTF8 name, CPUTF8 descriptor, long flags, List attributes) {
> 		// TODO Check that we only pass these on, or remap
>         super(name, descriptor, 0x7FFF & flags, attributes);
> 	    if(name.underlyingString().equals("<init>")) {
> 	        // hack hack - init should never be synchronized
> 	        SegmentUtils.debug("Hacking off synchronized on an init method");
> 	        this.flags = (short)(this.flags & 0x7FDF);
> 	    }
> 	}
> but I really need to look into it more deeply and find out why the 0x20 flag is being passed in. (This may end up being an encoding issue, but I need to look more first).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HARMONY-5654) [classlib][pack200] Init methods cannot be synchronized

Posted by "Sian January (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HARMONY-5654?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12583086#action_12583086 ] 

Sian January commented on HARMONY-5654:
---------------------------------------

Andrew - just for your curiosity this is happening because Synthetic has been redefined as bit 5 in the pack200 archive instead of Synchronized.  I'm working on a fix - should be able to check something in on Monday.

> [classlib][pack200] Init methods cannot be synchronized
> -------------------------------------------------------
>
>                 Key: HARMONY-5654
>                 URL: https://issues.apache.org/jira/browse/HARMONY-5654
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>         Environment: All pack200
>            Reporter: Andrew Cornwall
>            Assignee: Sian January
>         Attachments: pack200.pack.gz
>
>
> In the current code, it's possible for a CPMethod  <init> to be emitted as synchronized. This will not pass Sun's verification. For instance, if you run pack200 on itself, NewAttributeBands$LayoutElement in the following will be emitted with a synchronized <init>.
> I've temporarily hacked this with the following code in CPMethod:
> 	public CPMethod(CPUTF8 name, CPUTF8 descriptor, long flags, List attributes) {
> 		// TODO Check that we only pass these on, or remap
>         super(name, descriptor, 0x7FFF & flags, attributes);
> 	    if(name.underlyingString().equals("<init>")) {
> 	        // hack hack - init should never be synchronized
> 	        SegmentUtils.debug("Hacking off synchronized on an init method");
> 	        this.flags = (short)(this.flags & 0x7FDF);
> 	    }
> 	}
> but I really need to look into it more deeply and find out why the 0x20 flag is being passed in. (This may end up being an encoding issue, but I need to look more first).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HARMONY-5654) [classlib][pack200] Init methods cannot be synchronized

Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HARMONY-5654?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrew Cornwall updated HARMONY-5654:
-------------------------------------

    Attachment: pack200.pack.gz

pack200.pack.gz data which leads to synchronized <init> method

> [classlib][pack200] Init methods cannot be synchronized
> -------------------------------------------------------
>
>                 Key: HARMONY-5654
>                 URL: https://issues.apache.org/jira/browse/HARMONY-5654
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>         Environment: All pack200
>            Reporter: Andrew Cornwall
>         Attachments: pack200.pack.gz
>
>
> In the current code, it's possible for a CPMethod  <init> to be emitted as synchronized. This will not pass Sun's verification. For instance, if you run pack200 on itself, NewAttributeBands$LayoutElement in the following will be emitted with a synchronized <init>.
> I've temporarily hacked this with the following code in CPMethod:
> 	public CPMethod(CPUTF8 name, CPUTF8 descriptor, long flags, List attributes) {
> 		// TODO Check that we only pass these on, or remap
>         super(name, descriptor, 0x7FFF & flags, attributes);
> 	    if(name.underlyingString().equals("<init>")) {
> 	        // hack hack - init should never be synchronized
> 	        SegmentUtils.debug("Hacking off synchronized on an init method");
> 	        this.flags = (short)(this.flags & 0x7FDF);
> 	    }
> 	}
> but I really need to look into it more deeply and find out why the 0x20 flag is being passed in. (This may end up being an encoding issue, but I need to look more first).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HARMONY-5654) [classlib][pack200] Init methods cannot be synchronized

Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HARMONY-5654?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582458#action_12582458 ] 

Andrew Cornwall commented on HARMONY-5654:
------------------------------------------

This does look like an encoding issue to me. Using the attached pack200.pack.gz and unpacking, I find that parseFlags("method_flags"...) is returning 131104 (0x20020) for methodFlags[19][2], which corresponds to NewAttributeBands$LayoutElement.<init>(NewAttributeBands, NewAttributeBands$LayoutElement). In the original JAR (which I exported from my pack200 project in Eclipse), this method has an access_flags of 0x0.

Some useful code snippets for debugging:

>From ClassBands.parseMethodAttrBands:
    private void parseMethodAttrBands(InputStream in) throws IOException, Pack200Exception {
        methodFlags = parseFlags("method_flags", in, classMethodCount,
                Codec.UNSIGNED5, options.hasMethodFlagsHi());
        try {
            if(((methodFlags[19][2]) & 0x20) == 0x20) {
                System.out.println("Halt here: methodFlags[19][2] is "+ methodFlags[19][2]);
            }
        } catch (ArrayIndexOutOfBoundsException ex) {
            // do nothing
        }
...


>From Segment.buildClassFile:
		// add methods
		ClassFileEntry cfMethods[] = new ClassFileEntry[classBands.getClassMethodCount()[classNum]];
		// fieldDescr and fieldFlags used to create this
		for (i = 0; i < cfMethods.length; i++) {
            String descriptorStr = classBands.getMethodDescr()[classNum][i];
            int colon = descriptorStr.indexOf(':');
            CPUTF8 name = cpBands.cpUTF8Value(descriptorStr.substring(0,colon), ClassConstantPool.DOMAIN_NORMALASCIIZ);
            CPUTF8 descriptor = cpBands.cpUTF8Value(descriptorStr.substring(colon+1), ClassConstantPool.DOMAIN_SIGNATUREASCIIZ);
            if((classNum == 19) && (i==2)) {
                System.out.println("self halt");
            }
			cfMethods[i] = cp.add(new CPMethod(name, descriptor, classBands
                    .getMethodFlags()[classNum][i], classBands
                    .getMethodAttributes()[classNum][i]));
		}

(Also incidentally, shouldn't the comment be:
 		// methodDescr and methodFlags used to create this
instead of fieldFlags? Looks like a cut & paste error from the code above.)

> [classlib][pack200] Init methods cannot be synchronized
> -------------------------------------------------------
>
>                 Key: HARMONY-5654
>                 URL: https://issues.apache.org/jira/browse/HARMONY-5654
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>         Environment: All pack200
>            Reporter: Andrew Cornwall
>         Attachments: pack200.pack.gz
>
>
> In the current code, it's possible for a CPMethod  <init> to be emitted as synchronized. This will not pass Sun's verification. For instance, if you run pack200 on itself, NewAttributeBands$LayoutElement in the following will be emitted with a synchronized <init>.
> I've temporarily hacked this with the following code in CPMethod:
> 	public CPMethod(CPUTF8 name, CPUTF8 descriptor, long flags, List attributes) {
> 		// TODO Check that we only pass these on, or remap
>         super(name, descriptor, 0x7FFF & flags, attributes);
> 	    if(name.underlyingString().equals("<init>")) {
> 	        // hack hack - init should never be synchronized
> 	        SegmentUtils.debug("Hacking off synchronized on an init method");
> 	        this.flags = (short)(this.flags & 0x7FDF);
> 	    }
> 	}
> but I really need to look into it more deeply and find out why the 0x20 flag is being passed in. (This may end up being an encoding issue, but I need to look more first).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.