From 37a43ea01a706f6350a9529438fce0b3549435c3 Mon Sep 17 00:00:00 2001
From: Florian Warzecha <florian_filip.warzecha@hsbi.de>
Date: Tue, 19 Dec 2023 18:25:24 +0100
Subject: [PATCH] fix(opcode/Ifacmp): compare objects by data pointer only

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. https://github.com/rust-lang/rust/issues/106447,
https://github.com/rust-lang/rust/issues/117717,
https://github.com/rust-lang/rust/pull/116325)
---
 src/executor/op_code.rs            |  10 ++++++++--
 tests/control_flow.rs              |   4 +++-
 tests/data/control_flow/Main.class | Bin 1127 -> 1416 bytes
 tests/data/control_flow/Main.java  |  14 ++++++++++++++
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/executor/op_code.rs b/src/executor/op_code.rs
index 7982e95..8868c02 100644
--- a/src/executor/op_code.rs
+++ b/src/executor/op_code.rs
@@ -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,
@@ -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
@@ -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
diff --git a/tests/control_flow.rs b/tests/control_flow.rs
index 5c465e0..21a77a6 100644
--- a/tests/control_flow.rs
+++ b/tests/control_flow.rs
@@ -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(())
 }
diff --git a/tests/data/control_flow/Main.class b/tests/data/control_flow/Main.class
index aec2d07d89aff686534750a811e68ad964ddc852..d5be999872caba67c66eeb8d6925fc55a82c6486 100644
GIT binary patch
delta 720
zcmY*X&rcIU6#izq?aa2-U~d(RTeTtx{V5{F;uaMUslSS%SYx)dA=ORSv`FHqH^aq)
zS^X2#vmP)u5fg5DGchqH-aL5oC^zDp*2Xy5y!rNh?|tu^$^4WXf!3$p_7;E>xZ_6@
zQ##JTl*s!rjk6NxB+h$@R;$tjya&7zGZM2t^GLVCXU4lgVonKM(DEjr<qbe$UgDy}
zf<!^$lEh_+D-u_AiE9EyfyJlmA6mSbVhAnVv(~MIZPiy3#kEGQzB<Xkn`sp?410=e
z*2?{Y^<dE|+f{+<4F00?u(48|so7Nqy<pYqaU~JNAsiOC5yTR1G6ZTy%t$4TT+T=e
z+zMhDx2e}M5=L4j$C&$sje3?*a^JIo>A+mowjE>1Y1kD8G3z*$@*~oh&6KKEeiV%U
z56NJ!Ame^x<KE5zT+W*%gYH)?4yKGzRM(cEozd<cehc=xL&qrkI!O<X5)O6?Fo<LH
zr(+1il%ZxGg9bn4cHGZ=vk=;!6=9K;q5iBM7Ge1kd*6VE4s1eug@A$)g2d=Ay3D{o
z<{&{e-2*lKF{17>&)wD#+B9nm$FYqOe8ecekm2u0;Rn+AMX%;JPIf$&$QMUHRgQGr
z?oh3I3<kbZhssUi9g@_^QO0xX&XD&FIHWN~r)?ugO5+eTZ33qVNo@BKH9=5EmZak7
Xb4@K0-bF-U^6$4#ko0L{IdXphbP{zk

delta 440
zcmXv}O-lk%6g}_F*Hg!!a1mNMmYP|aR`#V%+NTw*DrgbJm@cG<7H)!|ty`I(ZGWMS
zMvHzxf24oVB8KSBh!@_y=bn4+dGB=s-{R@(@8c7|3Xc7VVA)5gAh7Dk8rB6i1U3~K
zyU`c|J_L$DQJ|z&5(9-=$qs-(*)=fbY6Xz11|YB{P!XtV*w(Papq5&9t+v_i_C|uI
zZR`5x#JWGT>bFh%kwxMYnAEVVV-NcbftHap@;ReiHgXycbkuNYzq6uL!;$^NqV@$Z
z*sr{K8Kxh&!NEWdl%v!wQe{+?Odssc8^<WC9x1^ftPcc;V~RQ-rja0qmf168XvFnx
zOX>yPZ3*sn9piRGzG|nveUsccU7keBbLwLTX;K+@!K}xbct8eoq^jc_^H?C?4m~YP
kM@5dbG3X9B3;jZI5Mr8xuuMsoMjfR1m`28Vl5s5k15VF5V*mgE

diff --git a/tests/data/control_flow/Main.java b/tests/data/control_flow/Main.java
index 967fab4..d3cef95 100644
--- a/tests/data/control_flow/Main.java
+++ b/tests/data/control_flow/Main.java
@@ -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");
+        }
     }
 }