Summary: Add                 JVM_CONSTANT_PseudoString in place of                 JVM_CONSTANT_Object and use this tag to distinguish                 patched pseudo strings. The original string is retained                 if it was present.             

            
            
This is reasonable; it is a good cleanup. �If you can               propose a name better than "PseudoString" I'm all ears.
                            
        If the string is really meaningless, maybe it can be deleted and         we don't need this JVM_CONSTANT_PseudoString.� The only reason I         kept "String" in the name is because I thought the string would         have some meaning to be preserved.
        
        
          
            

            
            
Consider getting rid of�set_has_pseudo_string. �That               flag was present (IIRC) only to tell the GC that there               might be non-perm oops in the constant pool. �Do we still               need that?
          
        
        
        I'd be happy to.� I noticed it wasn't being used.�� Neither is         _has_invokedynamic for that matter.�� _has_preresolution does do         something.
        
          

            I'm not sure how class file                 reconstitution for pseudo-strings is going to work, but                 I thought it was prudent to leave the Symbol* in the                 slot for the patched string.
                     
          
If you really wanted to reconstitute a class file for an             anonymous class, and if that class has oop patching             (pseudo-strings), you would need either to (a) reconstitute             the patches array handed to Unsafe.defineAnonymousClass, or             (b) accept whatever odd strings were there first, as an             approximation. �The "odd strings" are totally insignificant,             and are typically something like "CONSTANT_PLACEHOLDER_42"             (see InvokerBytecodeGenerator::constantPlaceholder).
          

          
                 
        Maybe there isn't a way or API to reconstitute an anonymous         class.�� I don't know if there is.� I'm not sure how to         reconstitute a normal class in the first place.�� Maybe Serguei         can comment.�� If this class cannot be reconsitituted, I'll         change this to remove the string in the patched case and won't         need JVM_CONSTANT_PseudoString (and the constant for Object can         be removed too).
             
      Reconstitution of the ConstantPool is needed for both the JVMTI       GetConstantPool() and RetransformClasses().
      Finally, it goes to the ConstantPool::copy_cpool_bytes().
      
      This is the place that has to be fixed:
      
      ConstantPool.cpp:
      int ConstantPool::copy_cpool_bytes(int cpool_size,
      ����� . . .
      ����� case JVM_CONSTANT_String: {
      ������� *bytes = JVM_CONSTANT_String;
      ������� Symbol* sym = unresolved_string_at(idx);
      ������� idx1 = tbl->symbol_to_value(sym);
      ������� assert(idx1 != 0, "Have not found a hashtable entry");
      ������� Bytes::put_Java_u2((address) (bytes+1), idx1);
      ������� DBG(char *str = sym->as_utf8());
      ������� DBG(printf("JVM_CONSTANT_String: idx=#%03hd, %s", idx1,       str));
      ������� break;
      ����� }
      
      I think, the John's suggestion for odd strings should work here.
      Something like:
             � if (sym == NULL) {
      ��� str = "CONSTANT_PLACEHOLDER_42";
      � }
      
      I will file a bug on this.
      
      
      Thanks,
      Serguei
      
       
        Thanks!
        Coleen
        
        
          
� John
        
        
             
         
   ">

(original) (raw)


Thank you for filing the new bug on reconstitution.�� I think it needs more than this patch#42 symbol because the oop has to be carried forth from the resolved\_references array in the old constant pool, and repatched into the new constant pool once the new resolved\_references array is created.�� And I didn't follow what this this symbol hashtable was trying to do.�� It would be good to start with a test for this.

Thanks,
Coleen

On 2/21/2013 2:16 PM, serguei.spitsyn@oracle.com wrote:
On 2/20/13 11:59 AM, Coleen Phillimore wrote:
On 2/20/2013 2:51 PM, John Rose wrote:
On Feb 20, 2013, at 10:20 AM, Coleen Phillimore <coleen.phillimore@oracle.com>

wrote:




Summary: Add
JVM_CONSTANT_PseudoString in place of
JVM_CONSTANT_Object and use this tag to distinguish
patched pseudo strings. The original string is retained
if it was present.




This is reasonable; it is a good cleanup. �If you can
propose a name better than "PseudoString" I'm all ears.





If the string is really meaningless, maybe it can be deleted and
we don't need this JVM_CONSTANT_PseudoString.� The only reason I
kept "String" in the name is because I thought the string would
have some meaning to be preserved.








Consider getting rid of�set_has_pseudo_string. �That
flag was present (IIRC) only to tell the GC that there
might be non-perm oops in the constant pool. �Do we still
need that?





I'd be happy to.� I noticed it wasn't being used.�� Neither is
_has_invokedynamic for that matter.�� _has_preresolution does do
something.




I'm not sure how class file
reconstitution for pseudo-strings is going to work, but
I thought it was prudent to leave the Symbol* in the
slot for the patched string.




If you really wanted to reconstitute a class file for an
anonymous class, and if that class has oop patching
(pseudo-strings), you would need either to (a) reconstitute
the patches array handed to Unsafe.defineAnonymousClass, or
(b) accept whatever odd strings were there first, as an
approximation. �The "odd strings" are totally insignificant,
and are typically something like "CONSTANT_PLACEHOLDER_42"
(see InvokerBytecodeGenerator::constantPlaceholder).







Maybe there isn't a way or API to reconstitute an anonymous
class.�� I don't know if there is.� I'm not sure how to
reconstitute a normal class in the first place.�� Maybe Serguei
can comment.�� If this class cannot be reconsitituted, I'll
change this to remove the string in the patched case and won't
need JVM_CONSTANT_PseudoString (and the constant for Object can
be removed too).




Reconstitution of the ConstantPool is needed for both the JVMTI
GetConstantPool() and RetransformClasses().

Finally, it goes to the ConstantPool::copy_cpool_bytes().



This is the place that has to be fixed:



ConstantPool.cpp:

int ConstantPool::copy_cpool_bytes(int cpool_size,

����� . . .

����� case JVM_CONSTANT_String: {

������� *bytes = JVM_CONSTANT_String;

������� Symbol* sym = unresolved_string_at(idx);

������� idx1 = tbl->symbol_to_value(sym);

������� assert(idx1 != 0, "Have not found a hashtable entry");

������� Bytes::put_Java_u2((address) (bytes+1), idx1);

������� DBG(char *str = sym->as_utf8());

������� DBG(printf("JVM_CONSTANT_String: idx=#%03hd, %s", idx1,
str));

������� break;

����� }



I think, the John's suggestion for odd strings should work here.

Something like:


� if (sym == NULL) {

��� str = "CONSTANT_PLACEHOLDER_42";

� }



I will file a bug on this.





Thanks,

Serguei





Thanks!

Coleen




� John