Skip to content

Commit

Permalink
fix(opcode/Ifacmp): compare objects by data pointer only
Browse files Browse the repository at this point in the history
We store class instances as Rc<dyn ClassInstanc> values.
The pointers to the inner values consist of
1. a pointer to the vtable of the trait
2. a pointer to the data itself
Due to rust internals, the vtable pointer can be different
for Rc<dyn ClassInstance> values that point to the same *data*,
effectively making any comparison involing the vtable pointer moot.
This uses a function rust just introduced to compare such pointers.
(cf. rust-lang/rust#106447,
rust-lang/rust#117717,
rust-lang/rust#116325)
  • Loading branch information
liketechnik committed Dec 19, 2023
1 parent fa5d323 commit 37a43ea
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/executor/op_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ use crate::{
heap::Heap,
};

/// Explicitly compare only the data part of fat/trait/dyn Trait pointers.
#[allow(clippy::ptr_eq)]
fn trait_pointer_eq<T: ?Sized, U: ?Sized>(p: *const T, q: *const U) -> bool {
(p as *const ()) == (q as *const ())
}

#[derive(Clone, Debug)]
pub enum ArrayReferenceKinds {
Boolean,
Expand Down Expand Up @@ -1268,7 +1274,7 @@ got: {:?}",
},
(None, None) => Update::None,
(Some(op1), Some(op2)) => {
if !Rc::<dyn ClassInstance>::ptr_eq(&op1, &op2) {
if !trait_pointer_eq(op1.as_ref(), op2.as_ref()) {
Update::GoTo(*size, *direction)
} else {
Update::None
Expand All @@ -1286,7 +1292,7 @@ got: {:?}",
(Some(_), None) | (None, Some(_)) => Update::None,
(None, None) => Update::GoTo(*size, *direction),
(Some(op1), Some(op2)) => {
if Rc::<dyn ClassInstance>::ptr_eq(&op1, &op2) {
if trait_pointer_eq(op1.as_ref(), op2.as_ref()) {
Update::GoTo(*size, *direction)
} else {
Update::None
Expand Down
4 changes: 3 additions & 1 deletion tests/control_flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ fn control_flow() -> Result<(), Box<dyn std::error::Error>> {
.stdout(predicate::str::contains("a / 2 == 5\n"))
.stdout(predicate::str::contains("l / 2 != 5\n"))
.stdout(predicate::str::contains("d:\n15\n"))
.stdout(predicate::str::contains("f > 10"));
.stdout(predicate::str::contains("f > 10"))
.stdout(predicate::str::contains("s1 == s2"))
.stdout(predicate::str::contains("s1 != s3"));

Ok(())
}
Binary file modified tests/data/control_flow/Main.class
Binary file not shown.
14 changes: 14 additions & 0 deletions tests/data/control_flow/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,19 @@ public static void main(String[] args) {
} else {
System.out.println("f == 10");
}

String s1 = "Hello World";
String s2 = s1;
String s3 = "Goodbye";
if (s1 == s2) {
System.out.println("s1 == s2");
} else {
System.out.println("s1 != s2");
}
if (s1 != s3) {
System.out.println("s1 != s3");
} else {
System.out.println("s1 == s3");
}
}
}

0 comments on commit 37a43ea

Please sign in to comment.