Skip to content

Commit 9c64763

Browse files
committed
Fix opcode operand parsing, interface instanceof, and runtime class bugs
- invokeinterface/invokedynamic now consume all four operand bytes - ClassDefinition exposes interface names; is_inherited_from checks interfaces - putstatic narrows int stack values to the field type like putfield - JavaLangString::to_rust_string no longer panics on unpaired surrogates - ByteArrayInputStream.mark saves position instead of readlimit - InputStream.skip returns actual skipped bytes with bounded buffer - Integer.parseInt throws NumberFormatException on invalid input - String.substring validates range, ISO-8859-1 encoding maps unmappable chars to '?' - Vector.firstElement throws NoSuchElementException on empty vector
1 parent 8ff00a9 commit 9c64763

37 files changed

Lines changed: 527 additions & 31 deletions

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ jvm_rust = { workspace = true }
4949
java_class_proto = { workspace = true }
5050
java_runtime = { workspace = true }
5151

52+
[dev-dependencies]
53+
test_utils = { workspace = true }
54+
5255
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
5356
tokio = { workspace = true, features = ["rt-multi-thread"] }
5457

classfile/src/opcode.rs

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,12 @@ impl Opcode {
317317
Opcode::Instanceof(ConstantPoolReference::from_constant_pool(constant_pool, x as _))
318318
})
319319
.parse(data),
320-
0xba => map(be_u16, |x| {
320+
0xba => map((be_u16, be_u16), |(x, _)| {
321321
Opcode::Invokedynamic(ConstantPoolReference::from_constant_pool(constant_pool, x as _))
322322
})
323323
.parse(data),
324-
0xb9 => map(be_u16, |x| {
325-
Opcode::Invokeinterface(ConstantPoolReference::from_constant_pool(constant_pool, x as _), 0, 0)
324+
0xb9 => map((be_u16, u8, u8), |(x, count, zero)| {
325+
Opcode::Invokeinterface(ConstantPoolReference::from_constant_pool(constant_pool, x as _), count, zero)
326326
})
327327
.parse(data),
328328
0xb7 => map(be_u16, |x| {
@@ -432,3 +432,59 @@ impl Opcode {
432432
}
433433
}
434434
}
435+
436+
#[cfg(test)]
437+
mod test {
438+
use alloc::{collections::BTreeMap, string::ToString, sync::Arc};
439+
440+
use super::Opcode;
441+
use crate::constant_pool::ConstantPoolItem;
442+
443+
fn constant_pool() -> BTreeMap<u16, ConstantPoolItem> {
444+
[
445+
(1, ConstantPoolItem::Utf8(Arc::new("Foo".to_string()))),
446+
(2, ConstantPoolItem::Class { name_index: 1 }),
447+
(3, ConstantPoolItem::Utf8(Arc::new("bar".to_string()))),
448+
(4, ConstantPoolItem::Utf8(Arc::new("()V".to_string()))),
449+
(
450+
5,
451+
ConstantPoolItem::NameAndType {
452+
name_index: 3,
453+
descriptor_index: 4,
454+
},
455+
),
456+
(
457+
6,
458+
ConstantPoolItem::InterfaceMethodref {
459+
class_index: 2,
460+
name_and_type_index: 5,
461+
},
462+
),
463+
(
464+
7,
465+
ConstantPoolItem::Methodref {
466+
class_index: 2,
467+
name_and_type_index: 5,
468+
},
469+
),
470+
]
471+
.into_iter()
472+
.collect()
473+
}
474+
475+
#[test]
476+
fn test_invokeinterface_consumes_count_and_zero() {
477+
let (remaining, opcode) = Opcode::parse(&[0xb9, 0x00, 0x06, 0x01, 0x00], 0, &constant_pool()).unwrap();
478+
479+
assert!(remaining.is_empty());
480+
assert!(matches!(opcode, Opcode::Invokeinterface(_, 1, 0)));
481+
}
482+
483+
#[test]
484+
fn test_invokedynamic_consumes_reserved_bytes() {
485+
let (remaining, opcode) = Opcode::parse(&[0xba, 0x00, 0x07, 0x00, 0x00], 0, &constant_pool()).unwrap();
486+
487+
assert!(remaining.is_empty());
488+
assert!(matches!(opcode, Opcode::Invokedynamic(_)));
489+
}
490+
}

classfile/tests/test.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,22 @@ fn test_switch() {
113113
Opcode::Lookupswitch(default, pairs) if *default == 82 && *pairs == vec![(1, 41), (10, 52), (100, 63), (1000, 74)]));
114114
}
115115
}
116+
117+
#[test]
118+
fn test_invokeinterface() {
119+
let interface = include_bytes!("../../test_data/Interface.class");
120+
121+
let class = ClassInfo::parse(interface).unwrap();
122+
123+
assert_eq!(class.methods[1].name, "main".to_string().into());
124+
if let AttributeInfo::Code(x) = &class.methods[1].attributes[0] {
125+
assert_eq!(x.code.len(), 7);
126+
assert!(matches!(x.code.get(&9).unwrap(),
127+
Opcode::Invokeinterface(ConstantPoolReference::InterfaceMethodref(m), 1, 0) if m.class == "Interface$IInterface".to_string().into() && m.name == "test".to_string().into()));
128+
assert!(!x.code.contains_key(&12));
129+
assert!(!x.code.contains_key(&13));
130+
assert!(matches!(x.code.get(&14).unwrap(), Opcode::Return));
131+
} else {
132+
panic!("Expected code attribute");
133+
}
134+
}

java_runtime/src/classes/java/io/byte_array_input_stream.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ impl ByteArrayInputStream {
151151
async fn mark(jvm: &Jvm, _: &mut RuntimeContext, mut this: ClassInstanceRef<Self>, readlimit: i32) -> Result<()> {
152152
tracing::debug!("java.io.ByteArrayInputStream::mark({:?}, {:?})", &this, readlimit);
153153

154-
jvm.put_field(&mut this, "mark", "I", readlimit).await?;
154+
let pos: i32 = jvm.get_field(&this, "pos", "I").await?;
155+
jvm.put_field(&mut this, "mark", "I", pos).await?;
155156

156157
Ok(())
157158
}

java_runtime/src/classes/java/io/input_stream.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,25 @@ impl InputStream {
5050
async fn skip(jvm: &Jvm, _: &mut RuntimeContext, this: ClassInstanceRef<Self>, n: i64) -> Result<i64> {
5151
tracing::debug!("java.io.InputStream::skip({:?}, {:?})", &this, n);
5252

53-
let scratch = jvm.instantiate_array("B", n as _).await?;
54-
let _: i32 = jvm.invoke_virtual(&this, "read", "([BII)I", (scratch.clone(), 0, n as i32)).await?;
53+
if n <= 0 {
54+
return Ok(0);
55+
}
56+
57+
let scratch_size = n.min(4096);
58+
let scratch = jvm.instantiate_array("B", scratch_size as _).await?;
59+
60+
let mut remaining = n;
61+
while remaining > 0 {
62+
let len_to_read = remaining.min(scratch_size) as i32;
63+
let read: i32 = jvm.invoke_virtual(&this, "read", "([BII)I", (scratch.clone(), 0, len_to_read)).await?;
64+
if read <= 0 {
65+
break;
66+
}
67+
68+
remaining -= read as i64;
69+
}
5570

56-
Ok(n)
71+
Ok(n - remaining)
5772
}
5873

5974
async fn mark(_jvm: &Jvm, _: &mut RuntimeContext, this: ClassInstanceRef<Self>, readlimit: i32) -> Result<()> {

java_runtime/src/classes/java/lang.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ mod linkage_error;
1919
mod math;
2020
mod negative_array_size_exception;
2121
mod no_class_def_found_error;
22+
mod number_format_exception;
2223
mod no_such_field_error;
2324
mod no_such_method_error;
2425
mod null_pointer_exception;
@@ -29,6 +30,7 @@ mod runtime_exception;
2930
mod security_exception;
3031
mod string;
3132
mod string_buffer;
33+
mod string_index_out_of_bounds_exception;
3234
mod system;
3335
mod thread;
3436
mod throwable;
@@ -41,8 +43,9 @@ pub use self::{
4143
exception::Exception, illegal_argument_exception::IllegalArgumentException, incompatible_class_change_error::IncompatibleClassChangeError,
4244
index_out_of_bounds_exception::IndexOutOfBoundsException, instantiation_error::InstantiationError, integer::Integer,
4345
interrupted_exception::InterruptedException, linkage_error::LinkageError, math::Math, negative_array_size_exception::NegativeArraySizeException,
44-
no_class_def_found_error::NoClassDefFoundError, no_such_field_error::NoSuchFieldError, no_such_method_error::NoSuchMethodError,
46+
no_class_def_found_error::NoClassDefFoundError, number_format_exception::NumberFormatException, no_such_field_error::NoSuchFieldError, no_such_method_error::NoSuchMethodError,
4547
null_pointer_exception::NullPointerException, object::Object, runnable::Runnable, runtime::Runtime, runtime_exception::RuntimeException,
46-
security_exception::SecurityException, string::String, string_buffer::StringBuffer, system::System, thread::Thread, throwable::Throwable,
48+
security_exception::SecurityException, string::String, string_buffer::StringBuffer,
49+
string_index_out_of_bounds_exception::StringIndexOutOfBoundsException, system::System, thread::Thread, throwable::Throwable,
4750
unsupported_operation_exception::UnsupportedOperationException,
4851
};

java_runtime/src/classes/java/lang/integer.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,10 @@ impl Integer {
7676

7777
let s = JavaLangString::to_rust_string(jvm, &s).await?;
7878

79-
Ok(s.parse().unwrap())
79+
match s.parse() {
80+
Ok(x) => Ok(x),
81+
Err(_) => Err(jvm.exception("java/lang/NumberFormatException", &format!("For input string: \"{s}\"")).await),
82+
}
8083
}
8184

8285
async fn to_hex_string(jvm: &Jvm, _: &mut RuntimeContext, value: i32) -> Result<ClassInstanceRef<String>> {
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
use alloc::vec;
2+
3+
use java_class_proto::JavaMethodProto;
4+
use jvm::{ClassInstanceRef, Jvm, Result};
5+
6+
use crate::{RuntimeClassProto, RuntimeContext, classes::java::lang::String};
7+
8+
// class java/lang/NumberFormatException
9+
pub struct NumberFormatException;
10+
11+
impl NumberFormatException {
12+
pub fn as_proto() -> RuntimeClassProto {
13+
RuntimeClassProto {
14+
name: "java/lang/NumberFormatException",
15+
parent_class: Some("java/lang/IllegalArgumentException"),
16+
interfaces: vec![],
17+
methods: vec![
18+
JavaMethodProto::new("<init>", "()V", Self::init, Default::default()),
19+
JavaMethodProto::new("<init>", "(Ljava/lang/String;)V", Self::init_with_message, Default::default()),
20+
],
21+
fields: vec![],
22+
access_flags: Default::default(),
23+
}
24+
}
25+
26+
async fn init(jvm: &Jvm, _: &mut RuntimeContext, this: ClassInstanceRef<Self>) -> Result<()> {
27+
tracing::debug!("java.lang.NumberFormatException::<init>({:?})", &this);
28+
29+
let _: () = jvm.invoke_special(&this, "java/lang/IllegalArgumentException", "<init>", "()V", ()).await?;
30+
31+
Ok(())
32+
}
33+
34+
async fn init_with_message(jvm: &Jvm, _: &mut RuntimeContext, this: ClassInstanceRef<Self>, message: ClassInstanceRef<String>) -> Result<()> {
35+
tracing::debug!("java.lang.NumberFormatException::<init>({:?}, {:?})", &this, &message);
36+
37+
let _: () = jvm
38+
.invoke_special(&this, "java/lang/IllegalArgumentException", "<init>", "(Ljava/lang/String;)V", (message,))
39+
.await?;
40+
41+
Ok(())
42+
}
43+
}

java_runtime/src/classes/java/lang/string.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use core::cmp::Ordering;
22

33
use alloc::{
4+
format,
45
string::{String as RustString, ToString},
56
vec,
67
vec::Vec,
@@ -350,10 +351,20 @@ impl String {
350351

351352
let string = JavaLangString::to_rust_string(jvm, &this.clone()).await?;
352353

354+
let length = string.chars().count() as i32;
355+
if begin_index < 0 || end_index > length || begin_index > end_index {
356+
return Err(jvm
357+
.exception(
358+
"java/lang/StringIndexOutOfBoundsException",
359+
&format!("begin {begin_index}, end {end_index}, length {length}"),
360+
)
361+
.await);
362+
}
363+
353364
let substr = string
354365
.chars()
355366
.skip(begin_index as usize)
356-
.take(end_index as usize - begin_index as usize)
367+
.take((end_index - begin_index) as usize)
357368
.collect::<RustString>(); // TODO buffer sharing
358369

359370
Ok(JavaLangString::from_rust_string(jvm, &substr).await?.into())
@@ -778,7 +789,7 @@ impl String {
778789
match charset.to_ascii_uppercase().replace('_', "-").as_str() {
779790
"UTF-8" | "UTF8" => string.as_bytes().to_vec(),
780791
"EUC-KR" | "EUCKR" | "KS-C-5601-1987" | "MS949" | "CP949" => encoding_rs::EUC_KR.encode(string).0.to_vec(),
781-
"ISO-8859-1" | "LATIN1" | "US-ASCII" | "ASCII" => string.chars().map(|c| c as u8).collect(),
792+
"ISO-8859-1" | "LATIN1" | "US-ASCII" | "ASCII" => string.chars().map(|c| if (c as u32) <= 0xff { c as u8 } else { b'?' }).collect(),
782793
_ => unimplemented!("unsupported charset: {}", charset),
783794
}
784795
}

0 commit comments

Comments
 (0)