Skip to content

Commit cbc7862

Browse files
committed
runtime/classpath: fix array class loading
The initiating loader was always handling the loading of component classes. It now defers to the *component's* defining loader, per the spec.
1 parent 7c31810 commit cbc7862

3 files changed

Lines changed: 65 additions & 41 deletions

File tree

classfile/src/fieldinfo.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,11 @@ impl FieldType {
165165
}
166166
}
167167

168-
pub fn as_java_type(&self) -> Cow<'static, str> {
168+
/// Get the `FieldType` as a Java type name
169+
///
170+
/// If `external_class_names` is true, classes will be in their "external name" forms (with path
171+
/// segments separated with '.' rather than '/')
172+
pub fn as_java_type(&self, external_class_names: bool) -> Cow<'static, str> {
169173
match self {
170174
Self::Byte => "byte".into(),
171175
Self::Character => "char".into(),
@@ -176,8 +180,17 @@ impl FieldType {
176180
Self::Short => "short".into(),
177181
Self::Boolean => "boolean".into(),
178182
Self::Void => "void".into(),
179-
Self::Object(name) => String::from_utf8_lossy(name).replace('/', ".").into(),
180-
Self::Array(component) => format!("[{}", component.as_java_type()).into(),
183+
Self::Object(name) => {
184+
let ret = String::from_utf8_lossy(name);
185+
if external_class_names {
186+
ret.replace('/', ".").into()
187+
} else {
188+
ret.into_owned().into()
189+
}
190+
},
191+
Self::Array(component) => {
192+
format!("[{}", component.as_java_type(external_class_names)).into()
193+
},
181194
}
182195
}
183196

runtime/src/classpath/loader/mod.rs

Lines changed: 46 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,9 @@ impl ClassLoader {
385385

386386
// https://docs.oracle.com/javase/specs/jvms/se23/html/jvms-5.html#jvms-5.3.2
387387
fn load_user_defined(&'static self, name: Symbol) -> Throws<ClassPtr> {
388-
// First, the Java Virtual Machine determines whether the bootstrap class loader has
389-
// already been recorded as an initiating loader of a class or interface denoted by N.
390-
// If so, this class or interface is C, and no class loading or creation is necessary.
388+
// First, the Java Virtual Machine determines whether L has already been recorded as an initiating loader
389+
// of a class or interface denoted by N. If so, this class or interface is C, and no class loading or
390+
// creation is necessary.
391391
if let Some(ret) = self.lookup_class(name) {
392392
return Throws::Ok(ret);
393393
}
@@ -630,53 +630,64 @@ impl ClassLoader {
630630
// Otherwise, the following steps are performed to create C:
631631
//
632632
// If the component type is a reference type, the algorithm of this section (§5.3) is applied recursively using L in order to load and thereby create the component type of C.
633-
let mut descriptor_str = descriptor.as_str();
634-
// TODO: Could also just skip the parsing entirely and count the number of '[' and strip, since we only need the number of dimensions
635-
let array = FieldType::parse(&mut descriptor_str.as_bytes()).unwrap(); // TODO: Error handling
636-
let FieldType::Array(mut component) = array else {
637-
unreachable!("The descriptor was validated as an array prior");
638-
};
633+
let mut descriptor_bytes = descriptor.as_bytes();
639634

640-
loop {
641-
if let FieldType::Object(obj) = &*component {
642-
self.load(Symbol::intern(obj))?;
643-
break;
644-
}
645-
646-
if let FieldType::Array(array_component) = *component {
647-
// Just strip '[' until we finally reach the component type.
648-
//
649-
// Note that for multidimensional arrays, the component classes are **not** preemptively
650-
// loaded.
651-
//
652-
// So, in the case of `[[Ljava/lang/String;`, only *that* class will be loaded. Not
653-
// `[Ljava/lang/String;`, unless it is explicitly needed later on.
654-
component = array_component;
655-
continue;
656-
}
657-
658-
break;
659-
}
635+
// Just strip '[' until we finally reach the component type.
636+
//
637+
// Note that for multidimensional arrays, the component classes are **not** preemptively
638+
// loaded.
639+
//
640+
// So, in the case of `[[Ljava/lang/String;`, only *that* class will be loaded. Not
641+
// `[Ljava/lang/String;`, unless it is explicitly loaded later on.
642+
let dimensions = descriptor_bytes
643+
.iter()
644+
.copied()
645+
.take_while(|c| *c == b'[')
646+
.count();
647+
assert!(
648+
dimensions > 0,
649+
"The descriptor was validated as an array prior"
650+
);
660651

661652
// The Java Virtual Machine creates a new array class with the indicated component type and number of dimensions.
662-
let array_class = unsafe { Class::new_array(descriptor, *component, self)? };
653+
654+
// (Handled below)
663655

664656
// If the component type is a reference type, the Java Virtual Machine marks C to have the defining loader of the component type as its defining loader.
665657
// Otherwise, the Java Virtual Machine marks C to have the bootstrap class loader as its defining loader.
666658

667-
// (Already handled)
659+
let component_type = FieldType::parse(&mut &descriptor_bytes[dimensions..]).unwrap(); // TODO: Error handling
660+
let mut array_class = None; // Might need to defer to other loaders
661+
if component_type.is_primitive() {
662+
// Only the bootstrap loader can load primitive array classes, defer.
663+
if !self.is_bootstrap() {
664+
array_class = Some(Self::bootstrap().load(descriptor)?);
665+
}
666+
} else {
667+
let component_class_name = Symbol::intern(component_type.as_java_type(false));
668+
let component_class = self.load(component_class_name)?;
669+
if component_class.loader() != self {
670+
array_class = Some(component_class.loader().load(descriptor)?);
671+
}
672+
}
673+
674+
let array_class = match array_class {
675+
Some(class) => class,
676+
None => {
677+
let array_class = unsafe { Class::new_array(descriptor, component_type, self)? };
678+
init_mirror(array_class);
679+
array_class
680+
},
681+
};
668682

669683
// In any case, the Java Virtual Machine then records that L is an initiating loader for C (§5.3.4).
670684

671-
// (Already handled)
685+
self.add_class(array_class)?;
672686

673687
// TODO:
674688
// If the component type is a reference type, the accessibility of the array class is determined by the accessibility of its component type (§5.4.4).
675689
// Otherwise, the array class is accessible to all classes and interfaces.
676690

677-
init_mirror(array_class);
678-
679-
self.add_class(array_class)?;
680691
Throws::Ok(array_class)
681692
}
682693

runtime/src/objects/method/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,12 +291,12 @@ impl Method {
291291
pub fn external_name(&self) -> String {
292292
let mut external_name = format!(
293293
"{} {}.{}(",
294-
self.descriptor.return_type.as_java_type(),
294+
self.descriptor.return_type.as_java_type(true),
295295
self.class.external_name(),
296296
self.name
297297
);
298298
for param in &self.descriptor.parameters {
299-
external_name.push_str(&param.as_java_type());
299+
external_name.push_str(&param.as_java_type(true));
300300
}
301301
external_name.push(')');
302302

@@ -306,7 +306,7 @@ impl Method {
306306
pub fn external_signature(&self, pretty: bool) -> String {
307307
let mut external_signature = String::new();
308308
for param in &self.descriptor.parameters {
309-
external_signature.push_str(&param.as_java_type());
309+
external_signature.push_str(&param.as_java_type(true));
310310
}
311311

312312
if pretty {

0 commit comments

Comments
 (0)