On 4/22/20 9:00 PM, Dean Long wrote:
It looks like calling the JVMCI getSourceFileName() on an array would
have accessed random memory because it was expecting an
InstanceKlass. Instead of returning null we might want to throw an
exception like in HotSpotResolvedPrimitiveType.
It was never called, except when I tried to call it on an array in the
test. It caused an assert in the hotspot code. How about this?
Something else in that file throws JVMCIError with a similar message.
public String getSourceFileName() {
if (isArray()) {
throw new JVMCIError("Cannot call getSourceFileName() on an
array klass type: %s", this);
}
return getConstantPool().getSourceFileName();
}
dl
On 4/22/20 5:39 PM, Dean Long wrote:
Can you compare the result to some string, like "Object.java"? That
seems to be what HotSpotResolvedObjectTypeTest.java is doing.
Also, did getSourceFileName() return null for arrays before your change?
Fixed:
public void getSourceFileNameTest() {
Class<?> c = Object.class;
ResolvedJavaType type = metaAccess.lookupJavaType(c);
assertEquals(type.getSourceFileName(), "Object.java");
}
thanks,
Coleen
dl
On 4/22/20 8:18 AM, coleen.phillim...@oracle.com wrote:
Hi Dean, Thank you for looking at the JVMCI changes and the
suggestion to add the test. I did this and found a bug. The new
test is quite limited because there's no good test to see if a
source file name can assertNotNull(type.getSourceFileName()), so I
couldn't iterate through the list of loaded classes like the other
tests in that file.
http://cr.openjdk.java.net/~coleenp/2020/8238048.02.incr/webrev/index.html
Thanks,
Coleen
On 4/21/20 9:51 PM, Dean Long wrote:
Hi Coleen. The JVMCI changes look OK. It looks like there is a
Graal unittest that covers getSourceFileName, but those tests don't
always get run. If it's not too much trouble, could you look into
enabling getSourceFileName() testing in
test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java
? It's currently on the "untested" list.
thanks,
dl
On 4/21/20 1:12 PM, coleen.phillim...@oracle.com wrote:
Summary: moved fields around and some constant fields into
ConstantPool
This is a simple change except that I moved some constant fields
from InstanceKlass into the constant pool so they can be shared
read-only in the CDS archive. There are associated repercussions
in SA and JVMCI, so please look at these changes. Also moved
similarly sized fields together in the class so there's less
likelihood of introducing gaps in future InstanceKlass changes.
InstanceKlass is reduced from 544 to 520 bytes in a simple Hello
World class.
open webrev at
http://cr.openjdk.java.net/~coleenp/2020/8238048.01/webrev
bug link https://bugs.openjdk.java.net/browse/JDK-8238048
Tested with tier1-6.
Thanks,
Coleen
