Merge pull request #21 from LLeavesG/fix/java-control-flow-output

Recover Java switch emission and preserve colliding source outputs

Author: asLody

dexdec/src/analysis/java_backend/anonymous_lowering.rs

@@ -2168,8 +2168,9 @@ impl AnonymousInstance {
             values: captures,
             identity: candidate.identity.as_ref(),
             value_types,
+            substitute_this: true,
         }
-        .rewrite_anonymous_body(&mut body);
+        .rewrite_root_anonymous_body(&mut body);
         if let Some(identity) = candidate.identity.as_ref() {
             AnonymousIdentitySubstitution { identity }.rewrite_anonymous_body(&mut body);
         }
@@ -2655,9 +2656,19 @@ struct CaptureSubstitution<'a> {
     values: BTreeMap<JavaIdentifier, JavaExpr>,
     identity: Option<&'a JavaType>,
     value_types: &'a BTreeMap<JavaIdentifier, LexicalValueType>,
+    // A nested anonymous body's `this` owns a separate field namespace. Its
+    // explicit QualifiedThis references can still target this capture owner.
+    substitute_this: bool,
 }
 
 impl JavaAstRewriter for CaptureSubstitution<'_> {
+    fn rewrite_anonymous_body(&mut self, body: &mut JavaAnonymousClassBody) {
+        let substitute_this = self.substitute_this;
+        self.substitute_this = false;
+        self.rewrite_anonymous_members(body);
+        self.substitute_this = substitute_this;
+    }
+
     fn finish_expression(&mut self, expression: JavaExpr) -> JavaExpr {
         match expression {
             JavaExpr::Cast { ty, value }
@@ -2668,7 +2679,7 @@ impl JavaAstRewriter for CaptureSubstitution<'_> {
                 *value
             }
             JavaExpr::Field { owner, name }
-                if matches!(owner.as_ref(), JavaExpr::This)
+                if self.substitute_this && matches!(owner.as_ref(), JavaExpr::This)
                     || matches!(
                         (owner.as_ref(), self.identity),
                         (JavaExpr::QualifiedThis(owner), Some(identity)) if owner == identity
@@ -2685,6 +2696,36 @@ impl JavaAstRewriter for CaptureSubstitution<'_> {
 }
 
 impl CaptureSubstitution<'_> {
+    fn rewrite_root_anonymous_body(&mut self, body: &mut JavaAnonymousClassBody) {
+        self.rewrite_anonymous_members(body);
+    }
+
+    fn rewrite_anonymous_members(&mut self, body: &mut JavaAnonymousClassBody) {
+        for field in &mut body.fields {
+            self.rewrite_annotations(&mut field.annotations);
+            field.initializer = field
+                .initializer
+                .take()
+                .map(|value| self.rewrite_expression(value));
+        }
+        for method in &mut body.methods {
+            self.rewrite_annotations(&mut method.annotations);
+            for parameter in &mut method.parameters {
+                self.rewrite_annotations(&mut parameter.annotations);
+            }
+            if let Some(body) = &mut method.body {
+                self.rewrite_body(body);
+            }
+        }
+        let substitute_this = self.substitute_this;
+        self.substitute_this = false;
+        for nested in &mut body.nested {
+            self.rewrite_type_declaration(nested);
+        }
+        self.substitute_this = substitute_this;
+        self.finish_anonymous_body(body);
+    }
+
     fn capture_type(&self, expression: &JavaExpr) -> Option<&LexicalValueType> {
         match expression {
             JavaExpr::Name(name) if self.values.values().any(|captured| captured == expression) => {
@@ -2705,3 +2746,80 @@ impl CaptureSubstitution<'_> {
         }
     }
 }
+
+#[cfg(test)]
+mod capture_substitution_tests {
+    use super::*;
+    use crate::language::java::JavaFieldDeclaration;
+
+    fn field(name: &str, initializer: JavaExpr) -> JavaFieldDeclaration {
+        JavaFieldDeclaration {
+            annotations: Vec::new(),
+            modifiers: Vec::new(),
+            ty: JavaType::Class(JavaClassType::from_source("java.lang.Object")),
+            name: JavaIdentifier::from_dex(name),
+            initializer: Some(initializer),
+        }
+    }
+
+    #[test]
+    fn captured_field_substitution_stops_at_nested_anonymous_body() {
+        let captured = JavaIdentifier::from_dex("captured");
+        let direct_reference = JavaExpr::Field {
+            owner: Box::new(JavaExpr::This),
+            name: captured.clone(),
+        };
+        let nested_reference = direct_reference.clone();
+        let identity = JavaType::Class(JavaClassType::from_source("example.ParentAnonymous"));
+        let enclosing_reference = JavaExpr::Field {
+            owner: Box::new(JavaExpr::QualifiedThis(identity.clone())),
+            name: captured.clone(),
+        };
+        let nested_body = JavaAnonymousClassBody {
+            fields: vec![
+                field("nestedUse", nested_reference.clone()),
+                field("enclosingUse", enclosing_reference),
+            ],
+            methods: Vec::new(),
+            nested: Vec::new(),
+        };
+        let mut body = JavaAnonymousClassBody {
+            fields: vec![
+                field("directUse", direct_reference),
+                field(
+                    "nested",
+                    JavaExpr::New {
+                        enclosing: None,
+                        ty: JavaType::Class(JavaClassType::from_source("example.Listener")),
+                        target_type: None,
+                        args: Vec::new(),
+                        anonymous_body: Some(Box::new(nested_body)),
+                    },
+                ),
+            ],
+            methods: Vec::new(),
+            nested: Vec::new(),
+        };
+        let replacement = JavaExpr::Name(JavaIdentifier::from_dex("value"));
+        let value_types = BTreeMap::new();
+
+        CaptureSubstitution {
+            values: BTreeMap::from([(captured, replacement.clone())]),
+            identity: Some(&identity),
+            value_types: &value_types,
+            substitute_this: true,
+        }
+        .rewrite_root_anonymous_body(&mut body);
+
+        assert_eq!(body.fields[0].initializer, Some(replacement.clone()));
+        let Some(JavaExpr::New {
+            anonymous_body: Some(nested),
+            ..
+        }) = body.fields[1].initializer.as_ref()
+        else {
+            panic!("expected nested anonymous body");
+        };
+        assert_eq!(nested.fields[0].initializer, Some(nested_reference));
+        assert_eq!(nested.fields[1].initializer, Some(replacement));
+    }
+}

dexdec/src/analysis/java_backend/declaration_lowering.rs

@@ -582,9 +582,10 @@ impl<'a> JavaTypeLowering<'a> {
             .ok_or(JavaDecompilerError::MalformedDeclarationStack)?;
         let mut declaration = lowered.declaration;
         let recovered_functions = lowered.liveness.apply(&mut declaration);
-        let outer_aliases = self
-            .outer_instances
-            .iter()
+        // Resolved simple owner names can collide across unrelated classes;
+        // only aliases owned by this lexical model may rewrite its fields.
+        let outer_aliases = class
+            .outer_instances()
             .map(|(field, outer)| {
                 Ok((
                     self.names.resolve_type(&field.owner)?,

dexdec/src/analysis/java_backend/java_model/method.rs

@@ -3,7 +3,7 @@ use crate::ir::{ty::ArgType, CFG};
 use crate::language::java::{
     AggregateInitializer, DefiniteAssignment, JavaAstNormalizer, JavaAstTransform, JavaIdentifier,
     JavaInitializerExitLowering, JavaLowerer, JavaMethodCompletion, JavaModifier, JavaType,
-    LexicalDeclarationPlacement,
+    JavaVoidTailLinearizer, LexicalDeclarationPlacement,
 };
 
 use super::super::method_pipeline::MethodBodyAnalysis;
@@ -324,6 +324,15 @@ impl JavaMethodBody {
                 .apply(&mut ast)
                 .map_err(crate::language::java::JavaLoweringError::from)
         })?;
+        if !class_initializer && self.return_type.as_ref() == Some(&ArgType::VOID) {
+            let mut tail = JavaVoidTailLinearizer;
+            crate::profile_scope!("java_backend.method_lower.void_tail", {
+                match tail.apply(&mut ast) {
+                    Ok(_) => {}
+                    Err(never) => match never {},
+                }
+            });
+        }
         if semantically_terminal {
             if let Some(return_type) = completion_type {
                 let mut completion = JavaMethodCompletion::new(return_type);
@@ -498,7 +507,9 @@ impl JavaMethodDeclaration {
             && parsed_signature.as_ref().is_some_and(|signature| {
                 signature.has_unbound_type_variables(lexical_type_variables)
             });
-        let explicit_signature = parsed_signature.filter(|_| !has_unbound_variables);
+        let explicit_signature = parsed_signature.filter(|signature| {
+            !has_unbound_variables && method_signature_is_java_denotable(signature)
+        });
         let (signature, source_return_type) = match explicit_signature {
             Some(signature) => (Some(signature), None),
             None => {
@@ -660,6 +671,24 @@ impl JavaMethodDeclaration {
     }
 }
 
+fn method_signature_is_java_denotable(
+    signature: &crate::ir::generic_types::MethodSignature,
+) -> bool {
+    signature.type_parameters.iter().all(|parameter| {
+        parameter
+            .class_bound
+            .iter()
+            .chain(&parameter.interface_bounds)
+            .all(|bound| {
+                !matches!(
+                    bound,
+                    crate::ir::generic_types::JvmTypeSignature::Array(_)
+                        | crate::ir::generic_types::JvmTypeSignature::BaseType(_)
+                )
+            })
+    })
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(in crate::analysis::java_backend) enum JavaMethodDeclarationKind {
     Method,
@@ -801,3 +830,30 @@ fn is_meaningful_param_name(name: &str) -> bool {
     }
     chars.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '$')
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::ir::generic_types::GenericSignatures;
+
+    use super::method_signature_is_java_denotable;
+
+    #[test]
+    fn rejects_an_array_type_parameter_bound_in_a_method_signature() {
+        let signature = GenericSignatures::method(
+            "<C:[Ljava/lang/Object;:TR;R:Ljava/lang/Object;>\
+             (TC;Lkotlin/jvm/functions/Function0<+TR;>;)TR;",
+        )
+        .expect("Kotlin array intersection signature");
+
+        assert!(!method_signature_is_java_denotable(&signature));
+    }
+
+    #[test]
+    fn accepts_a_java_denotable_method_signature() {
+        let signature =
+            GenericSignatures::method("<T:Ljava/lang/Object;>(Ljava/util/List<+TT;>;)TT;")
+                .expect("ordinary generic method signature");
+
+        assert!(method_signature_is_java_denotable(&signature));
+    }
+}

dexdec/src/analysis/java_backend/java_model/source_abi.rs

@@ -63,6 +63,10 @@ pub(in crate::analysis::java_backend) struct OuterInstanceField {
 }
 
 impl OuterInstanceField {
+    pub(in crate::analysis::java_backend) fn reference(&self) -> &FieldReference {
+        &self.reference
+    }
+
     /// Proves the enclosing-instance field from constructor def-use rather
     /// than field names or type uniqueness. This is required for local and
     /// anonymous classes, where another captured value can have the same type

dexdec/src/analysis/java_backend/member_names.rs

@@ -2,7 +2,7 @@
 
 use crate::ir::{ArgType, MethodDescriptor};
 use crate::language::java::{
-    JavaConstructorLayout, JavaFieldSymbol, JavaMemberNames, JavaMethodSymbol,
+    JavaConstructorLayout, JavaFieldSymbol, JavaIdentifier, JavaMemberNames, JavaMethodSymbol,
 };
 
 use super::java_model::method::{JavaMethodDeclarationKind, JavaMethodModel};
@@ -13,6 +13,7 @@ pub(super) struct ClassMemberNames;
 impl ClassMemberNames {
     pub(super) fn collect(root: &JavaClassModel) -> JavaMemberNames {
         let mut fields = Vec::new();
+        let mut hidden_fields = Vec::new();
         let mut methods = Vec::new();
         let mut constructors = Vec::new();
         let mut pending = vec![root];
@@ -24,6 +25,14 @@ impl ClassMemberNames {
             fields.extend(class.fields.iter().map(|field| {
                 JavaFieldSymbol::new(owner.clone(), field.name.clone(), field.field_type.clone())
             }));
+            hidden_fields.extend(class.outer_instance.iter().map(|outer| {
+                let field = outer.reference();
+                JavaFieldSymbol::new(
+                    field.owner.clone(),
+                    JavaIdentifier::from_dex(&field.name),
+                    field.field_type.clone(),
+                )
+            }));
             methods.extend(
                 class
                     .methods
@@ -37,7 +46,9 @@ impl ClassMemberNames {
                     .filter_map(|method| Self::constructor(owner.clone(), method)),
             );
         }
-        JavaMemberNames::allocate(fields, methods).with_constructor_layouts(constructors)
+        JavaMemberNames::allocate(fields, methods)
+            .with_hidden_fields(hidden_fields)
+            .with_constructor_layouts(constructors)
     }
 
     pub(super) fn method_only(

dexdec/src/analysis/value_recovery/flow.rs

@@ -73,6 +73,7 @@ struct UseFact {
     repetitive: bool,
     evaluation_prefix: EffectSummary,
     context: UseContext,
+    consumer: Option<InsnType>,
     site: Option<UseSite>,
 }