From 05774895395f89aad89fabcb8fc445445ed5ac0f Mon Sep 17 00:00:00 2001 From: LLeaves Date: Tue, 4 Aug 2026 16:28:52 +0800 Subject: [PATCH 01/10] fix(exceptions): recover shared synchronized cleanup --- dexdec/src/ir/exception/cleanup.rs | 55 ++++++- dexdec/src/ir/passes/split_monitor_entries.rs | 135 +++++++++++++++++- 2 files changed, 179 insertions(+), 11 deletions(-) diff --git a/dexdec/src/ir/exception/cleanup.rs b/dexdec/src/ir/exception/cleanup.rs index 72508cc..60289cd 100644 --- a/dexdec/src/ir/exception/cleanup.rs +++ b/dexdec/src/ir/exception/cleanup.rs @@ -1707,15 +1707,17 @@ impl<'a> CleanupProof<'a> { if mapped == normal { return true; } + if self.empty_path_reaches(mapped, normal) { + self.blocks.insert(handler, normal); + return true; + } + if self.empty_path_reaches(normal, mapped) { + return true; + } if !self.rethrow_blocks.contains(&handler) { return false; } - if self.empty_path_reaches(mapped, normal) { - self.blocks.insert(handler, normal); - true - } else if self.empty_path_reaches(normal, mapped) { - true - } else if let Some(join) = self.empty_path_join(mapped, normal) { + if let Some(join) = self.empty_path_join(mapped, normal) { self.blocks.insert(handler, join); true } else { @@ -2330,3 +2332,44 @@ enum EquivalenceTask<'a> { Option<&'a crate::ir::RegisterArg>, ), } + +#[cfg(test)] +mod tests { + use super::*; + use crate::ir::{ArgType, Block, RegisterArg}; + + #[test] + fn remaps_non_rethrow_handler_across_empty_normal_trampoline() { + let handler = BlockId::new(0); + let trampoline = BlockId::new(1); + let target = BlockId::new(2); + let observable = BlockId::new(3); + let mut cfg = CFG::new("cleanup_empty_trampoline"); + cfg.add_block(Block::new(handler)); + cfg.add_block(Block::new(trampoline)); + cfg.add_block(Block::new(target)); + let mut observable_block = Block::new(observable); + observable_block.push(InsnNode::monitor_exit(InsnArg::Reg(RegisterArg::new( + 0, + ArgType::object("java/lang/Object"), + )))); + cfg.add_block(observable_block); + cfg.add_edge(trampoline, target, EdgeKind::Normal); + cfg.add_edge(observable, target, EdgeKind::Normal); + + let values = SsaValueGraph::default(); + let origins = SsaOrigins::analyze(&values); + let handler_blocks = BTreeSet::from([handler]); + let rethrow_blocks = BTreeSet::new(); + let mut proof = + CleanupProof::new(&cfg, &values, &origins, &handler_blocks, &rethrow_blocks); + assert!(proof.bind_block(handler, trampoline)); + assert!(proof.bind_block(handler, target)); + assert_eq!(proof.blocks.get(&handler), Some(&target)); + + let mut proof = + CleanupProof::new(&cfg, &values, &origins, &handler_blocks, &rethrow_blocks); + assert!(proof.bind_block(handler, observable)); + assert!(!proof.bind_block(handler, target)); + } +} diff --git a/dexdec/src/ir/passes/split_monitor_entries.rs b/dexdec/src/ir/passes/split_monitor_entries.rs index 587103b..fe90378 100644 --- a/dexdec/src/ir/passes/split_monitor_entries.rs +++ b/dexdec/src/ir/passes/split_monitor_entries.rs @@ -45,10 +45,6 @@ impl Pass for SplitMonitorEntries { .is_some_and(|block| !Self::partition_boundaries(&block.insns).is_empty()) }) .collect::>(); - if candidates.is_empty() { - return Ok(PassResult::Unchanged); - } - let mut next = cfg .block_ids() .into_iter() @@ -56,6 +52,7 @@ impl Pass for SplitMonitorEntries { .max() .unwrap_or(0) + 1; + let split = !candidates.is_empty(); for original_id in candidates { let outgoing = cfg.successors_with_kind(original_id).to_vec(); let instructions = std::mem::take( @@ -121,7 +118,8 @@ impl Pass for SplitMonitorEntries { } } } - Ok(PassResult::Changed) + let distributed = Self::distribute_shared_releases(cfg, &mut next); + Ok((split || distributed).into()) } } @@ -151,6 +149,74 @@ impl SplitMonitorEntries { boundaries.dedup(); boundaries } + + /// A compiler may merge normal exits from duplicated synchronized cleanup + /// bodies into one `monitor-exit` block. SSA then needs a Phi for the lock + /// register, and no individual `monitor-enter` dominates that shared + /// release. Put the release on each incoming edge while leaving its + /// continuation shared, so monitor ownership remains path-local. + fn distribute_shared_releases(cfg: &mut CFG, next: &mut u32) -> bool { + let candidates = cfg + .block_ids() + .into_iter() + .filter(|block| *block != cfg.entry) + .filter(|block| { + cfg.block(*block) + .is_some_and(|block| Self::is_pure_release(&block.insns)) + }) + .filter_map(|block| { + let incoming = cfg.incoming_edges(block); + let sources = incoming + .iter() + .map(|(source, _)| *source) + .collect::>(); + (incoming.len() > 1 + && sources.len() == incoming.len() + && incoming + .iter() + .all(|(_, kind)| *kind != EdgeKind::Exception)) + .then_some((block, incoming)) + }) + .collect::>(); + let mut changed = false; + for (release, incoming) in candidates { + let Some(template) = cfg.block(release).cloned() else { + continue; + }; + let outgoing = cfg.successors_with_kind(release).to_vec(); + for (predecessor, kind) in incoming.into_iter().skip(1) { + let clone_id = BlockId::new(*next); + *next += 1; + let mut clone = template.clone(); + clone.id = clone_id; + cfg.add_block(clone); + cfg.set_exception_coverage(clone_id, cfg.exception_coverage_for(&template.insns)); + cfg.remove_edge(predecessor, release); + cfg.add_edge(predecessor, clone_id, kind); + for &(target, kind) in &outgoing { + cfg.add_edge(clone_id, target, kind); + } + changed = true; + } + } + changed + } + + fn is_pure_release(instructions: &[InsnNode]) -> bool { + instructions + .iter() + .filter(|instruction| instruction.insn_type != InsnType::Nop) + .try_fold(0usize, |releases, instruction| { + match instruction.insn_type { + InsnType::MonitorExit => Some(releases + 1), + _ if InstructionEffects::is_kotlin_finally_marker(instruction) => { + Some(releases) + } + _ => None, + } + }) + == Some(1) + } } #[cfg(test)] @@ -232,4 +298,63 @@ mod tests { vec![BlockId::new(2)] ); } + + #[test] + fn distributes_a_shared_release_across_incoming_edges() { + let lock = InsnArg::Reg(RegisterArg::new(0, ArgType::object("java/lang/Object"))); + let mut first = Block::new(0); + first.push(InsnNode::monitor_enter(lock.clone())); + let mut second = Block::new(1); + second.push(InsnNode::monitor_enter(lock.clone())); + let mut release = Block::new(2); + release.push(InsnNode::monitor_exit(lock)); + release.push(InsnNode::nop()); + let handler = Block::new(3); + let continuation = Block::new(4); + let mut cfg = CFG::new("shared_monitor_release"); + cfg.add_block(first); + cfg.add_block(second); + cfg.add_block(release); + cfg.add_block(handler); + cfg.add_block(continuation); + cfg.add_edge(BlockId::new(0), BlockId::new(2), EdgeKind::Normal); + cfg.add_edge(BlockId::new(1), BlockId::new(2), EdgeKind::Normal); + cfg.add_edge(BlockId::new(2), BlockId::new(3), EdgeKind::Exception); + cfg.add_edge(BlockId::new(2), BlockId::new(4), EdgeKind::Normal); + + assert_eq!( + SplitMonitorEntries.run(&mut cfg).unwrap(), + PassResult::Changed + ); + let release_blocks = cfg + .block_ids() + .into_iter() + .filter(|block| { + cfg.block(*block).is_some_and(|block| { + block + .insns + .iter() + .any(|instruction| instruction.insn_type == InsnType::MonitorExit) + }) + }) + .collect::>(); + assert_eq!(release_blocks.len(), 2); + for release in release_blocks { + assert_eq!(cfg.incoming_edges(release).len(), 1); + assert!(cfg.has_edge(release, BlockId::new(3))); + let continuation = cfg + .normal_successors(release) + .next() + .expect("shared continuation"); + assert_eq!( + cfg.block(continuation) + .unwrap() + .insns + .iter() + .map(|instruction| instruction.insn_type) + .collect::>(), + vec![InsnType::Nop] + ); + } + } } From 4df3a87d99af78f7f3b467dc25e2037b14287344 Mon Sep 17 00:00:00 2001 From: LLeaves Date: Tue, 4 Aug 2026 16:51:39 +0800 Subject: [PATCH 02/10] fix(regions): match adapted monitor release handlers --- dexdec/src/ir/region/tree.rs | 111 +++++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 5 deletions(-) diff --git a/dexdec/src/ir/region/tree.rs b/dexdec/src/ir/region/tree.rs index 16b0322..dd8c627 100644 --- a/dexdec/src/ir/region/tree.rs +++ b/dexdec/src/ir/region/tree.rs @@ -1,6 +1,6 @@ use std::collections::{BTreeMap, BTreeSet}; -use crate::ir::analysis::LexicalBoundaryAnalysis; +use crate::ir::analysis::{InstructionEffects, LexicalBoundaryAnalysis}; use crate::ir::{BlockId, InsnArg, CFG}; use super::{ @@ -68,6 +68,7 @@ impl RegionTree { } SynchronizationPlacement::analyze( self, + cfg, owner, handlers, lock, @@ -890,6 +891,7 @@ pub(super) struct SynchronizationRewrite { impl SynchronizationPlacement { fn analyze( tree: &RegionTree, + cfg: &CFG, owner: RegionId, handlers: &[RegionId], lock: InsnArg, @@ -905,7 +907,7 @@ impl SynchronizationPlacement { let release_handlers = handlers .iter() .copied() - .filter(|handler| Self::is_release_entry(tree, *handler, release_entries)) + .filter(|handler| Self::is_release_entry(cfg, tree, *handler, release_entries)) .collect::>(); if release_handlers.is_empty() { return Err(RegionInvariantError::MissingSynchronizationHandler { @@ -958,7 +960,7 @@ impl SynchronizationPlacement { .values() .filter(|region| !release_handlers.contains(®ion.id)) .filter(|region| !nested_release_handlers.contains(®ion.id)) - .filter(|region| Self::is_release_entry(tree, region.id, release_entries)) + .filter(|region| Self::is_release_entry(cfg, tree, region.id, release_entries)) .filter(|region| { region .parent @@ -1540,7 +1542,12 @@ impl SynchronizationPlacement { Ok(()) } - fn is_release_entry(tree: &RegionTree, region: RegionId, entries: &BTreeSet) -> bool { + fn is_release_entry( + cfg: &CFG, + tree: &RegionTree, + region: RegionId, + entries: &BTreeSet, + ) -> bool { tree.region(region).is_some_and(|region| { let semantic_entry = region.entry.or_else(|| match ®ion.kind { RegionKind::Catch(catch) | RegionKind::Cleanup(catch) => catch.continuation, @@ -1549,10 +1556,48 @@ impl SynchronizationPlacement { matches!( ®ion.kind, RegionKind::Catch(_) | RegionKind::Finally | RegionKind::Cleanup(_) - ) && semantic_entry.is_some_and(|entry| entries.contains(&entry)) + ) && semantic_entry.is_some_and(|target| { + entries.iter().copied().any(|entry| { + entry == target || Self::transparent_release_adapter(cfg, entry, target) + }) + }) }) } + /// Exception analysis can retain a `move-exception` adapter as the + /// release entry while the lexical handler starts at its successor. + /// Relate those representations only across a deterministic path of SSA + /// bookkeeping with no exceptional side exit. + fn transparent_release_adapter(cfg: &CFG, entry: BlockId, target: BlockId) -> bool { + let mut current = entry; + let mut visited = BTreeSet::new(); + while visited.insert(current) { + if current == target { + return true; + } + let Some(block) = cfg.block(current) else { + return false; + }; + if block + .insns + .iter() + .any(|instruction| !InstructionEffects::is_ssa_bookkeeping(instruction)) + || cfg + .successors_with_kind(current) + .iter() + .any(|(_, kind)| kind.is_exception()) + { + return false; + } + let successors = cfg.normal_successors(current).collect::>(); + let [next] = successors.as_slice() else { + return false; + }; + current = *next; + } + false + } + fn mark_release_handlers(&self, tree: &mut RegionTree) -> Result<(), RegionInvariantError> { for handler in &self.release_handlers { let kind = &mut tree @@ -1705,6 +1750,7 @@ impl SynchronizationPlacement { mod tests { use super::super::{CatchRegion, LoopRegion}; use super::*; + use crate::ir::{ArgType, Block, EdgeKind, InsnNode, RegisterArg}; fn blocks(ids: impl IntoIterator) -> BTreeSet { ids.into_iter().map(BlockId::new).collect() @@ -1817,4 +1863,59 @@ mod tests { assert_eq!(tree.region(protected).unwrap().parent, Some(handler)); } + + #[test] + fn release_handler_matches_a_transparent_move_exception_adapter() { + let mut tree = RegionTree::new(Some(BlockId::new(0))); + let root = tree.root(); + let handler = add_region(&mut tree, root, RegionKind::Finally, 1, [0, 1, 2, 3, 4]); + + let mut adapter = Block::new(0); + adapter.push(InsnNode::move_exception(RegisterArg::new( + 0, + ArgType::throwable(), + ))); + let target = Block::new(1); + let exceptional_target = Block::new(2); + let mut observable = Block::new(3); + observable.push(InsnNode::monitor_exit(InsnArg::Reg(RegisterArg::new( + 1, + ArgType::object("java/lang/Object"), + )))); + let mut branching_adapter = Block::new(4); + branching_adapter.push(InsnNode::move_exception(RegisterArg::new( + 2, + ArgType::throwable(), + ))); + + let mut cfg = CFG::new("release_handler_adapter"); + cfg.add_block(adapter); + cfg.add_block(target); + cfg.add_block(exceptional_target); + cfg.add_block(observable); + cfg.add_block(branching_adapter); + cfg.add_edge(BlockId::new(0), BlockId::new(1), EdgeKind::Normal); + cfg.add_edge(BlockId::new(3), BlockId::new(1), EdgeKind::Normal); + cfg.add_edge(BlockId::new(4), BlockId::new(1), EdgeKind::Normal); + cfg.add_edge(BlockId::new(4), BlockId::new(2), EdgeKind::Exception); + + assert!(SynchronizationPlacement::is_release_entry( + &cfg, + &tree, + handler, + &blocks([0]), + )); + assert!(!SynchronizationPlacement::is_release_entry( + &cfg, + &tree, + handler, + &blocks([3]), + )); + assert!(!SynchronizationPlacement::is_release_entry( + &cfg, + &tree, + handler, + &blocks([4]), + )); + } } From 1d3b87f883449ae5877cf1cbc26181c57b233bdd Mon Sep 17 00:00:00 2001 From: LLeaves Date: Tue, 4 Aug 2026 19:03:54 +0800 Subject: [PATCH 03/10] fix(phi): preserve copies across repeated block occurrences --- .../src/ir/analysis/source_variables/phi.rs | 139 ++++++++++++++++-- 1 file changed, 125 insertions(+), 14 deletions(-) diff --git a/dexdec/src/ir/analysis/source_variables/phi.rs b/dexdec/src/ir/analysis/source_variables/phi.rs index d2647b5..70a2eab 100644 --- a/dexdec/src/ir/analysis/source_variables/phi.rs +++ b/dexdec/src/ir/analysis/source_variables/phi.rs @@ -4,8 +4,8 @@ use crate::ir::{ analysis::{types::SourceTypeLattice, TypeHierarchy}, ArgType, BlockId, InsnArg, InsnNode, InsnType, InstructionId, InstructionTree, InstructionVisitor, RegionExit, RegionGraph, RegionId, RegisterArg, SemanticBlock, - SemanticFolder, SemanticNode, SemanticStatement, SemanticVisitor, StatementOrigin, Utf16String, - CFG, + SemanticExpression, SemanticFolder, SemanticNode, SemanticStatement, SemanticVisitor, + StatementOrigin, Utf16String, CFG, }; use super::{ @@ -684,6 +684,44 @@ impl NormalCopies { self.by_site.get(&site).cloned() } + fn place_block_occurrence( + &mut self, + site: NormalCopySite, + repeats_semantically: bool, + ) -> Option> { + if repeats_semantically && self.is_idempotent(site) { + self.place_at_occurrence(site) + } else { + self.place_once(site) + } + } + + fn is_idempotent(&self, site: NormalCopySite) -> bool { + let site = self.canonical(site); + let Some(statements) = self.by_site.get(&site) else { + return false; + }; + let destinations = statements + .iter() + .filter_map(SemanticStatement::result) + .filter_map(|result| result.code_var) + .collect::>(); + !destinations.is_empty() + && statements.iter().all(|statement| { + let Some(instruction) = statement.instruction_ref() else { + return false; + }; + instruction.insn_type == InsnType::Move + && instruction.payload.edge_copy + && instruction + .operands() + .first() + .and_then(SemanticExpression::as_register) + .and_then(|source| source.code_var) + .is_none_or(|source| !destinations.contains(&source)) + }) + } + fn first_unplaced(&self) -> Option { self.by_site .keys() @@ -2092,6 +2130,7 @@ pub(super) struct PhiLowering { handler_regions: BTreeMap, handler_blocks: BTreeSet, semantic_blocks: BTreeSet, + repeated_semantic_blocks: BTreeSet, statement_blocks: BTreeSet, materialized_instructions: BTreeSet, placed_exceptional: BTreeSet, @@ -2223,6 +2262,7 @@ impl PhiLowering { handler_regions, handler_blocks, semantic_blocks: BTreeSet::new(), + repeated_semantic_blocks: BTreeSet::new(), statement_blocks: BTreeSet::new(), materialized_instructions: BTreeSet::new(), placed_exceptional: BTreeSet::new(), @@ -2237,6 +2277,7 @@ impl PhiLowering { pub(super) fn apply(mut self, root: &mut SemanticNode) -> Result<(), SourceVariableError> { self.semantic_blocks = SemanticBlocks::collect(root); + self.repeated_semantic_blocks = SemanticBlockOccurrences::repeated(root); self.statement_blocks = StatementBlocks::collect(root); self.materialized_instructions = EvaluationInstructions::of_node(root).into_iter().collect(); @@ -2428,11 +2469,11 @@ impl PhiLowering { block.statements = statements; if owns_normal_copies { let site = NormalCopySite::Block(block.id); - let copies = if block.statements.is_empty() { - self.normal.place_once(site) - } else { - self.normal.place_at_occurrence(site) - }; + let repeats_semantically = + !block.statements.is_empty() || self.repeated_semantic_blocks.contains(&block.id); + let copies = self + .normal + .place_block_occurrence(site, repeats_semantically); if let Some(statements) = copies { block.statements.extend(statements); } @@ -2581,6 +2622,31 @@ impl SemanticVisitor for SemanticBlocks { } } +#[derive(Default)] +struct SemanticBlockOccurrences { + counts: BTreeMap, +} + +impl SemanticBlockOccurrences { + fn repeated(root: &SemanticNode) -> BTreeSet { + let mut collector = Self::default(); + collector.visit_node(root); + collector + .counts + .into_iter() + .filter_map(|(block, count)| (count > 1).then_some(block)) + .collect() + } +} + +impl SemanticVisitor for SemanticBlockOccurrences { + fn enter_node(&mut self, node: &SemanticNode) { + if let SemanticNode::BasicBlock(block) = node { + *self.counts.entry(block.id).or_default() += 1; + } + } +} + #[derive(Default)] struct StatementBlocks { blocks: BTreeSet, @@ -2897,11 +2963,11 @@ mod tests { } fn normal_copies() -> NormalCopies { - let statement = SemanticStatement::definition( - InstructionId::new(1), - RegisterArg::new(0, ArgType::INT), - SemanticExpression::Literal(LiteralArg::int(1)), - ); + let mut destination = RegisterArg::new(0, ArgType::INT); + destination.code_var = Some(7); + let mut copy = InsnNode::mov(destination, InsnArg::Lit(LiteralArg::int(1))); + copy.payload.edge_copy = true; + let statement = SemanticStatement::instruction(copy).expect("edge copy statement"); NormalCopies::new( BTreeMap::from([(NormalCopySite::Block(BlockId::new(7)), vec![statement])]), BTreeMap::new(), @@ -2923,8 +2989,53 @@ mod tests { let site = NormalCopySite::Block(BlockId::new(7)); let mut copies = normal_copies(); - assert_eq!(copies.place_once(site).unwrap().len(), 1); - assert!(copies.place_once(site).is_none()); + assert_eq!(copies.place_block_occurrence(site, false).unwrap().len(), 1); + assert!(copies.place_block_occurrence(site, false).is_none()); + } + + #[test] + fn repeated_semantic_block_copies_follow_each_occurrence() { + let site = NormalCopySite::Block(BlockId::new(7)); + let mut copies = normal_copies(); + + assert_eq!(copies.place_block_occurrence(site, true).unwrap().len(), 1); + assert_eq!(copies.place_block_occurrence(site, true).unwrap().len(), 1); + assert!(copies.first_unplaced().is_none()); + } + + #[test] + fn repeated_semantic_blocks_are_detected() { + let block = || { + SemanticNode::BasicBlock(SemanticBlock { + id: BlockId::new(7), + statements: Vec::new(), + }) + }; + let root = SemanticNode::sequence(vec![block(), block()]); + + assert_eq!( + SemanticBlockOccurrences::repeated(&root), + BTreeSet::from([BlockId::new(7)]) + ); + } + + #[test] + fn dependent_block_copies_are_not_repeated() { + let site = NormalCopySite::Block(BlockId::new(7)); + let left = source_variable(0, 0, 1); + let right = source_variable(1, 0, 2); + let mut left_copy = InsnNode::mov(left.clone(), InsnArg::Reg(right.clone())); + left_copy.payload.edge_copy = true; + let mut right_copy = InsnNode::mov(right, InsnArg::Reg(left)); + right_copy.payload.edge_copy = true; + let statements = vec![ + SemanticStatement::instruction(left_copy).expect("left edge copy"), + SemanticStatement::instruction(right_copy).expect("right edge copy"), + ]; + let mut copies = NormalCopies::new(BTreeMap::from([(site, statements)]), BTreeMap::new()); + + assert_eq!(copies.place_block_occurrence(site, true).unwrap().len(), 2); + assert!(copies.place_block_occurrence(site, true).is_none()); } #[test] From ab0b8850f294defdfbe38580bfb704061b24de70 Mon Sep 17 00:00:00 2001 From: LLeaves Date: Tue, 4 Aug 2026 19:26:18 +0800 Subject: [PATCH 04/10] fix(regions): preserve phi-copy boundary identities --- dexdec/src/ir/structure/region_reducer.rs | 34 +++++++---- .../ir/structure/region_reducer/region_cfg.rs | 59 +++++++++++++++---- 2 files changed, 70 insertions(+), 23 deletions(-) diff --git a/dexdec/src/ir/structure/region_reducer.rs b/dexdec/src/ir/structure/region_reducer.rs index 7b2278b..7834aae 100644 --- a/dexdec/src/ir/structure/region_reducer.rs +++ b/dexdec/src/ir/structure/region_reducer.rs @@ -410,24 +410,32 @@ impl<'a> RegionReducer<'a> { } for (block, leave) in ®ion_cfg.boundaries { let mut node = semantic.leave(leave)?; - if let Some(source) = leave + let mut anchor_blocks = leave .leave .source_block + .into_iter() .filter(|source| self.anchors.phi_copy_blocks().contains(source)) + .collect::>(); + if let Some(target) = leave + .leave + .edge + .map(|edge| edge.target) + .filter(|target| self.anchors.phi_copy_blocks().contains(target)) + .filter(|target| !region_cfg.representatives.contains(target)) { - node = Self::anchor_block(source, node); + anchor_blocks.push(target); } - seeded.insert( - *block, - match &leave.leave.exit { - crate::ir::RegionExit::FallThrough(target) - if self.anchors.phi_copy_blocks().contains(target) => - { - Self::anchor_block(*target, node) - } - _ => node, - }, - ); + if let crate::ir::RegionExit::FallThrough(target) = &leave.leave.exit { + if self.anchors.phi_copy_blocks().contains(target) { + anchor_blocks.push(*target); + } + } + let mut seen_anchors = BTreeSet::new(); + anchor_blocks.retain(|anchor| seen_anchors.insert(*anchor)); + for anchor in anchor_blocks.into_iter().rev() { + node = Self::anchor_block(anchor, node); + } + seeded.insert(*block, node); } for (block, target) in ®ion_cfg.entry_boundaries { let leave = SemanticNode::Leave(SemanticLeave { diff --git a/dexdec/src/ir/structure/region_reducer/region_cfg.rs b/dexdec/src/ir/structure/region_reducer/region_cfg.rs index 416d017..5d67143 100644 --- a/dexdec/src/ir/structure/region_reducer/region_cfg.rs +++ b/dexdec/src/ir/structure/region_reducer/region_cfg.rs @@ -873,17 +873,28 @@ impl RegionCfgState { } fn intern_boundary(&mut self, leave: &ResolvedRegionExit) -> Result { - let preserve_origin = leave + let copy_anchors = leave .leave .source_block - .is_some_and(|block| self.origin_sensitive_boundaries.contains(&block)); + .into_iter() + .chain( + leave + .leave + .edge + .into_iter() + .flat_map(|edge| [edge.source, edge.target]), + ) + .filter(|block| self.origin_sensitive_boundaries.contains(block)) + .collect::>() + .into_iter() + .collect(); let canonical_fallthrough = match &leave.leave.exit { RegionExit::FallThrough(target) => { Some(self.canonical_continuations.destination(*target)) } _ => None, }; - let key = BoundaryKey::of(leave, preserve_origin, canonical_fallthrough); + let key = BoundaryKey::of(leave, copy_anchors, canonical_fallthrough); if let Some(boundary) = self.boundary_ids.get(&key) { return Ok(*boundary); } @@ -1101,7 +1112,7 @@ impl RegionCfg { #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] struct BoundaryKey { exit: BoundaryExitKey, - origin: Option, + copy_anchors: Vec, region_target: RegionId, target: RegionId, cleanup: Vec, @@ -1110,12 +1121,12 @@ struct BoundaryKey { impl BoundaryKey { fn of( exit: &ResolvedRegionExit, - preserve_origin: bool, + copy_anchors: Vec, canonical_fallthrough: Option, ) -> Self { Self { exit: BoundaryExitKey::of(&exit.leave.exit, canonical_fallthrough), - origin: preserve_origin.then_some(exit.leave.source_block).flatten(), + copy_anchors, region_target: exit.leave.target, target: exit.leave.control_target.unwrap_or(exit.leave.target), cleanup: exit.cleanup_regions.clone(), @@ -1186,12 +1197,12 @@ mod tests { use std::collections::{BTreeMap, BTreeSet}; use super::{ - BoundaryExitKey, BoundaryValueKey, RegionCfg, RegionCfgBuilder, RegionEntryPorts, - RegionLayout, + BoundaryExitKey, BoundaryKey, BoundaryValueKey, RegionCfg, RegionCfgBuilder, + RegionEntryPorts, RegionLayout, }; use crate::ir::{ - ArgType, Block, BlockId, CatchRegion, EdgeKind, InsnArg, RegionExit, RegionKind, - RegionTree, CFG, + ArgType, Block, BlockId, CatchRegion, EdgeKind, InsnArg, RegionEdge, RegionExit, RegionId, + RegionKind, RegionLeave, RegionTree, ResolvedRegionExit, CFG, }; #[test] @@ -1273,6 +1284,34 @@ mod tests { ); } + #[test] + fn phi_copy_targets_keep_equivalent_break_boundaries_distinct() { + let region = RegionId::new(3); + let resolved = |source, target| ResolvedRegionExit { + leave: RegionLeave::new(region, region, RegionExit::Break) + .with_source_block(source) + .with_edge(RegionEdge { + source, + target, + kind: EdgeKind::False, + }), + cleanup_regions: Vec::new(), + }; + let left_target = BlockId::new(33); + let right_target = BlockId::new(34); + let left = resolved(BlockId::new(16), left_target); + let right = resolved(BlockId::new(20), right_target); + + assert_eq!( + BoundaryKey::of(&left, Vec::new(), None), + BoundaryKey::of(&right, Vec::new(), None) + ); + assert_ne!( + BoundaryKey::of(&left, vec![left_target], None), + BoundaryKey::of(&right, vec![right_target], None) + ); + } + #[test] fn nested_entry_is_visible_through_every_crossed_region() { let mut tree = RegionTree::new(Some(BlockId::new(0))); From 0cd950d95b47013c1840fab8bce792aae25fd3bf Mon Sep 17 00:00:00 2001 From: LLeaves Date: Tue, 4 Aug 2026 20:19:56 +0800 Subject: [PATCH 05/10] fix(phi): emit equivalent copies at each concrete site --- .../src/ir/analysis/source_variables/phi.rs | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/dexdec/src/ir/analysis/source_variables/phi.rs b/dexdec/src/ir/analysis/source_variables/phi.rs index 70a2eab..b12f726 100644 --- a/dexdec/src/ir/analysis/source_variables/phi.rs +++ b/dexdec/src/ir/analysis/source_variables/phi.rs @@ -640,7 +640,10 @@ pub(super) struct PhiCopySet { struct NormalCopies { by_site: BTreeMap>, canonical_site: BTreeMap, - placed: BTreeSet, + // Equivalent sites satisfy one structural coverage obligation, but each + // concrete CFG site that survives semantically must still execute copies. + placed_sites: BTreeSet, + covered_classes: BTreeSet, } impl NormalCopies { @@ -651,7 +654,8 @@ impl NormalCopies { Self { by_site, canonical_site, - placed: BTreeSet::new(), + placed_sites: BTreeSet::new(), + covered_classes: BTreeSet::new(), } } @@ -660,11 +664,15 @@ impl NormalCopies { } fn place_once(&mut self, site: NormalCopySite) -> Option> { - let site = self.canonical(site); - if !self.placed.insert(site) { + if !self.placed_sites.insert(site) { return None; } - let statements = self.by_site.get(&site)?; + let canonical = self.canonical(site); + self.covered_classes.insert(canonical); + let statements = self + .by_site + .get(&site) + .or_else(|| self.by_site.get(&canonical))?; Some(statements.clone()) } @@ -679,9 +687,13 @@ impl NormalCopies { } fn place_at_occurrence(&mut self, site: NormalCopySite) -> Option> { - let site = self.canonical(site); - self.placed.insert(site); - self.by_site.get(&site).cloned() + let canonical = self.canonical(site); + self.placed_sites.insert(site); + self.covered_classes.insert(canonical); + self.by_site + .get(&site) + .or_else(|| self.by_site.get(&canonical)) + .cloned() } fn place_block_occurrence( @@ -726,7 +738,7 @@ impl NormalCopies { self.by_site .keys() .map(|site| self.canonical(*site)) - .find(|site| !self.placed.contains(site)) + .find(|site| !self.covered_classes.contains(site)) } fn get(&self, site: NormalCopySite) -> Option<&Vec> { @@ -2993,6 +3005,27 @@ mod tests { assert!(copies.place_block_occurrence(site, false).is_none()); } + #[test] + fn equivalent_concrete_copy_sites_are_each_placed() { + let first = NormalCopySite::Block(BlockId::new(7)); + let second = NormalCopySite::Block(BlockId::new(8)); + let mut copies = normal_copies(); + let statements = copies.by_site[&first].clone(); + copies.by_site.insert(second, statements); + copies.canonical_site = BTreeMap::from([(first, first), (second, first)]); + + assert_eq!( + copies.place_block_occurrence(second, false).unwrap().len(), + 1 + ); + assert!(copies.first_unplaced().is_none()); + assert_eq!( + copies.place_block_occurrence(first, false).unwrap().len(), + 1 + ); + assert!(copies.place_block_occurrence(first, false).is_none()); + } + #[test] fn repeated_semantic_block_copies_follow_each_occurrence() { let site = NormalCopySite::Block(BlockId::new(7)); From aed819855492659abb55c6bbaf400357e30e0af1 Mon Sep 17 00:00:00 2001 From: LLeaves Date: Tue, 4 Aug 2026 20:37:03 +0800 Subject: [PATCH 06/10] fix(regions): partition handlers at standalone monitor exits --- dexdec/src/ir/region/tree.rs | 134 +++++++++++++++++++++++++++++++++-- 1 file changed, 127 insertions(+), 7 deletions(-) diff --git a/dexdec/src/ir/region/tree.rs b/dexdec/src/ir/region/tree.rs index dd8c627..2ffc0e4 100644 --- a/dexdec/src/ir/region/tree.rs +++ b/dexdec/src/ir/region/tree.rs @@ -64,7 +64,7 @@ impl RegionTree { method: bool, ) -> Result { if release_entries.is_empty() { - return self.insert_standalone_synchronization(lock, enter, entry, blocks, method); + return self.insert_standalone_synchronization(cfg, lock, enter, entry, blocks, method); } SynchronizationPlacement::analyze( self, @@ -84,6 +84,7 @@ impl RegionTree { fn insert_standalone_synchronization( &mut self, + cfg: &CFG, lock: InsnArg, enter: BlockId, entry: BlockId, @@ -91,6 +92,9 @@ impl RegionTree { method: bool, ) -> Result { self.close_lexical_scope(enter, entry, blocks)?; + // A handler can cross the runtime monitor-exit boundary only through + // one proven continuation, such as a return lowered after monitor-exit. + SynchronizationPlacement::partition_handler_regions_for(self, cfg, None, blocks)?; let kind = RegionKind::Synchronized(SynchronizedRegion { lock, method, @@ -1327,16 +1331,25 @@ impl SynchronizationPlacement { &self, tree: &mut RegionTree, cfg: &CFG, + ) -> Result<(), RegionInvariantError> { + Self::partition_handler_regions_for(tree, cfg, Some(self.owner), &self.blocks) + } + + fn partition_handler_regions_for( + tree: &mut RegionTree, + cfg: &CFG, + owner: Option, + blocks: &BTreeSet, ) -> Result<(), RegionInvariantError> { let mut candidates = tree .regions .values() - .filter(|region| region.id != tree.root && region.id != self.owner) + .filter(|region| region.id != tree.root && Some(region.id) != owner) .filter(|region| matches!(region.kind, RegionKind::Catch(_) | RegionKind::Cleanup(_))) .filter(|region| { - !region.blocks.is_disjoint(&self.blocks) - && !region.blocks.is_subset(&self.blocks) - && !self.blocks.is_subset(®ion.blocks) + !region.blocks.is_disjoint(blocks) + && !region.blocks.is_subset(blocks) + && !blocks.is_subset(®ion.blocks) }) .map(|region| { tree.parent_chain(region.id) @@ -1356,7 +1369,7 @@ impl SynchronizationPlacement { continue; }; let boundary = - LexicalBoundaryAnalysis::new(cfg).partition(entry, ®ion.blocks, &self.blocks); + LexicalBoundaryAnalysis::new(cfg).partition(entry, ®ion.blocks, blocks); let Some(boundary) = boundary else { continue; }; @@ -1370,7 +1383,7 @@ impl SynchronizationPlacement { let mut promote = Vec::new(); let mut crossing = None; for child in children { - if child == self.owner { + if Some(child) == owner { continue; } let child_blocks = &tree @@ -1864,6 +1877,113 @@ mod tests { assert_eq!(tree.region(protected).unwrap().parent, Some(handler)); } + #[test] + fn standalone_synchronization_partitions_a_handler_return_continuation() { + let mut cfg = CFG::new("synchronized_handler_return"); + for id in 0..=8 { + cfg.add_block(Block::new(id)); + } + for (source, target) in [(0, 1), (1, 2), (2, 3), (3, 4), (4, 5), (5, 7), (6, 8)] { + cfg.add_edge(BlockId::new(source), BlockId::new(target), EdgeKind::Normal); + } + for source in 1..=4 { + cfg.add_edge(BlockId::new(source), BlockId::new(6), EdgeKind::Exception); + } + + let mut tree = RegionTree::new(Some(BlockId::new(0))); + tree.cover_method(&cfg).unwrap(); + let root = tree.root(); + let handler = add_region( + &mut tree, + root, + RegionKind::Catch(CatchRegion { + exception_types: vec![ArgType::throwable()], + exception_value: None, + continuation: None, + }), + 6, + [6, 8], + ); + let scope = blocks([1, 2, 3, 4, 5, 6]); + let lock = InsnArg::Reg(RegisterArg::new(0, ArgType::object("java/lang/Object"))); + + tree.synchronize( + &cfg, + &BTreeMap::new(), + root, + &[], + lock, + BlockId::new(0), + BlockId::new(1), + &scope, + &BTreeSet::new(), + &BTreeSet::new(), + true, + ) + .unwrap(); + + let handler_region = tree.region(handler).unwrap(); + assert_eq!(handler_region.blocks, blocks([6])); + let RegionKind::Catch(catch) = &handler_region.kind else { + panic!("handler changed kind"); + }; + assert_eq!(catch.continuation, Some(BlockId::new(8))); + let synchronization = handler_region.parent.unwrap(); + assert!(matches!( + tree.region(synchronization).map(|region| ®ion.kind), + Some(RegionKind::Synchronized(_)) + )); + assert_eq!(tree.region(synchronization).unwrap().blocks, scope); + } + + #[test] + fn standalone_synchronization_rejects_ambiguous_handler_continuations() { + let mut cfg = CFG::new("ambiguous_synchronized_handler"); + for id in 0..=9 { + cfg.add_block(Block::new(id)); + } + for (source, target) in [(0, 1), (1, 5), (5, 7), (6, 8), (6, 9)] { + cfg.add_edge(BlockId::new(source), BlockId::new(target), EdgeKind::Normal); + } + cfg.add_edge(BlockId::new(1), BlockId::new(6), EdgeKind::Exception); + + let mut tree = RegionTree::new(Some(BlockId::new(0))); + tree.cover_method(&cfg).unwrap(); + let root = tree.root(); + add_region( + &mut tree, + root, + RegionKind::Catch(CatchRegion { + exception_types: vec![ArgType::throwable()], + exception_value: None, + continuation: None, + }), + 6, + [6, 8, 9], + ); + let scope = blocks([1, 5, 6]); + let lock = InsnArg::Reg(RegisterArg::new(0, ArgType::object("java/lang/Object"))); + + let result = tree.synchronize( + &cfg, + &BTreeMap::new(), + root, + &[], + lock, + BlockId::new(0), + BlockId::new(1), + &scope, + &BTreeSet::new(), + &BTreeSet::new(), + true, + ); + + assert!(matches!( + result, + Err(RegionInvariantError::SynchronizationRegionOverlap { .. }) + )); + } + #[test] fn release_handler_matches_a_transparent_move_exception_adapter() { let mut tree = RegionTree::new(Some(BlockId::new(0))); From d719a89be9afbbc68b98eb4bec2f31f9996a71ca Mon Sep 17 00:00:00 2001 From: LLeaves Date: Tue, 4 Aug 2026 21:08:05 +0800 Subject: [PATCH 07/10] fix(java): terminate anonymous switch dispatch cases --- dexdec/src/language/java/lower.rs | 38 ++++++++++++++++++++++++++- dexdec/src/language/java/normalize.rs | 4 +++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/dexdec/src/language/java/lower.rs b/dexdec/src/language/java/lower.rs index 297bee3..33282fe 100644 --- a/dexdec/src/language/java/lower.rs +++ b/dexdec/src/language/java/lower.rs @@ -533,7 +533,7 @@ where .iter() .map(|value| JavaExpr::Literal(JavaLiteral::Integer(*value))) .collect(), - body: Self::block_statements(body), + body: switch_case_body(region, body), is_default: case.is_default, }) .collect(), @@ -621,6 +621,18 @@ where } } +fn switch_case_body(region: Option, body: JavaStmt) -> Vec { + let mut statements = match body { + JavaStmt::Block(statements) => statements, + JavaStmt::Empty => Vec::new(), + other => vec![other], + }; + if region.is_none() && super::normalize::statements_can_complete_normally(&statements) { + statements.push(JavaStmt::Break(None)); + } + statements +} + enum BoundLoopCondition { Expression(JavaExpr), Separate { setup: Vec }, @@ -778,3 +790,27 @@ impl LowerFrame<'_> { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn anonymous_dispatch_switch_cases_end_before_the_next_case() { + assert_eq!( + switch_case_body(None, JavaStmt::Empty), + vec![JavaStmt::Break(None)] + ); + } + + #[test] + fn structured_switch_cases_keep_explicit_fallthrough() { + assert!(switch_case_body(Some(crate::ir::RegionId::new(1)), JavaStmt::Empty).is_empty()); + } + + #[test] + fn anonymous_dispatch_switch_does_not_append_an_unreachable_break() { + let body = JavaStmt::Return(None); + assert_eq!(switch_case_body(None, body.clone()), vec![body]); + } +} diff --git a/dexdec/src/language/java/normalize.rs b/dexdec/src/language/java/normalize.rs index 148c2fc..21e820b 100644 --- a/dexdec/src/language/java/normalize.rs +++ b/dexdec/src/language/java/normalize.rs @@ -536,6 +536,10 @@ impl JavaAstNormalizer { struct TerminalBranchLinearizer; +pub(super) fn statements_can_complete_normally(statements: &[JavaStmt]) -> bool { + TerminalBranchLinearizer::sequence_can_complete_normally(statements) +} + impl TerminalBranchLinearizer { const NESTING_PENALTY: usize = 12; From 2981fb68c9f4ee5f2b23c6b263f815332920b1ff Mon Sep 17 00:00:00 2001 From: LLeaves Date: Wed, 5 Aug 2026 02:06:27 +0800 Subject: [PATCH 08/10] fix(values): preserve identity across alias fanout --- .../analysis/value_recovery/flow/planner.rs | 179 +++++++++++++++++- .../expected/ComplexControlFlow.kt.expected | 2 +- .../expected/HardcoreControlFlow.kt.expected | 29 ++- .../expected/NestedLoopBranches.kt.expected | 12 +- .../expected/SynchronizedAdvanced.kt.expected | 23 ++- 5 files changed, 212 insertions(+), 33 deletions(-) diff --git a/dexdec/src/analysis/value_recovery/flow/planner.rs b/dexdec/src/analysis/value_recovery/flow/planner.rs index 25d9ec2..e3cddab 100644 --- a/dexdec/src/analysis/value_recovery/flow/planner.rs +++ b/dexdec/src/analysis/value_recovery/flow/planner.rs @@ -336,17 +336,57 @@ impl<'a> ValuePlanner<'a> { if blocking_keys.is_empty() { return Ok(()); } + let ssa_identity = self.graph().identity() == ValueIdentity::Ssa; + let mut deferred_keys = BTreeSet::new(); + for action in actions.iter() { + let Some(key) = Self::replacement_key(action) else { + continue; + }; + let mut dependencies = self.replacement_dependencies(action)?; + dependencies.remove(&key); + if (!ssa_identity || self.facts.uses_of(key).len() > 1) + && !dependencies.is_disjoint(&blocking_keys) + { + deferred_keys.insert(key); + } + } + if ssa_identity { + // A canonical replacement may jump over register moves. If an + // intermediate alias fans out, composing every replacement in one + // round would clone the effectful producer at those uses. + // Carry that boundary through the actual move chain, including + // aliases whose canonical replacement points straight at the + // producer. Do not cross arithmetic or Phi definitions: those + // are distinct state transitions whose old/new ordering must stay + // available to the current schedule. + loop { + let discovered = actions + .iter() + .filter_map(Self::replacement_key) + .filter(|key| !deferred_keys.contains(key)) + .filter(|key| { + self.graph() + .definitions + .get(key) + .into_iter() + .flatten() + .filter_map(|definition| self.register_move_source(definition)) + .any(|dependency| deferred_keys.contains(&dependency)) + }) + .collect::>(); + if discovered.is_empty() { + break; + } + deferred_keys.extend(discovered); + } + } let mut retained = Vec::with_capacity(actions.len()); for action in std::mem::take(actions) { let Some(key) = Self::replacement_key(&action) else { retained.push(action); continue; }; - let mut dependencies = self.replacement_dependencies(&action)?; - dependencies.remove(&key); - let duplicates_effects = self.graph().identity() == ValueIdentity::Source - || self.facts.uses_of(key).len() > 1; - if !duplicates_effects || dependencies.is_disjoint(&blocking_keys) { + if !deferred_keys.contains(&key) { retained.push(action); } } @@ -354,6 +394,16 @@ impl<'a> ValuePlanner<'a> { Ok(()) } + fn register_move_source(&self, definition: &DefinitionFact) -> Option { + let operation = definition.operation()?; + let [SemanticExpression::Register(register)] = operation.operands() else { + return None; + }; + (operation.insn_type == InsnType::Move) + .then(|| self.graph().key(register)) + .flatten() + } + fn replacement_key(action: &ValueAction) -> Option { match action { ValueAction::Propagate { key, .. } @@ -1805,3 +1855,122 @@ impl ValueAction { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::ir::{ + analysis::SsaValueGraph, block::Block, ArgType, BlockId, InsnNode, InvokeType, RegisterArg, + SemanticBlock, SemanticNode, SemanticStatement, CFG, + }; + + fn register(value: SsaVar, ty: &ArgType) -> RegisterArg { + RegisterArg::new_ssa(value.reg_num, value.version, ty.clone()) + } + + fn argument(value: SsaVar, ty: &ArgType) -> InsnArg { + InsnArg::Reg(register(value, ty)) + } + + fn scheduled_replacements(name: &str, block: Block) -> BTreeSet { + let mut cfg = CFG::new(name); + cfg.add_block(block); + cfg.identify_instructions(); + let values = SsaValueGraph::build(&cfg).expect("SSA graph"); + let statements = cfg + .block(BlockId::new(0)) + .expect("entry block") + .insns + .iter() + .cloned() + .map(|instruction| SemanticStatement::instruction(instruction).expect("semantic op")) + .collect(); + let mut root = SemanticNode::BasicBlock(SemanticBlock { + id: BlockId::new(0), + statements, + }); + crate::ir::SemanticSiteNumbering::assign(&mut root).expect("semantic sites"); + + let graph = ValueFlowGraph::build(&root, &values, &BTreeMap::new()).expect("value graph"); + graph + .schedule(RecoveryMode::Full) + .expect("value plan") + .actions + .iter() + .filter_map(ValuePlanner::replacement_key) + .collect() + } + + #[test] + fn defers_transitive_alias_across_effectful_producer() { + let array_type = ArgType::object_array(); + let allocation = SsaVar::new(10, 0); + let first_alias = SsaVar::new(14, 0); + let second_alias = SsaVar::new(9, 0); + + let mut block = Block::new(0u32); + block.push(InsnNode::new_array( + register(allocation, &array_type), + InsnArg::lit(1, ArgType::INT), + 0, + )); + block.push(InsnNode::mov( + register(first_alias, &array_type), + argument(allocation, &array_type), + )); + block.push(InsnNode::aput( + InsnArg::lit(0, ArgType::object("java/lang/Object")), + argument(first_alias, &array_type), + InsnArg::lit(0, ArgType::INT), + )); + block.push(InsnNode::mov( + register(second_alias, &array_type), + argument(first_alias, &array_type), + )); + block.push(InsnNode::invoke( + InvokeType::Static, + 0, + vec![argument(second_alias, &array_type)], + )); + + let scheduled = scheduled_replacements("allocation_alias_fork", block); + + assert!(scheduled.contains(&allocation)); + assert!(!scheduled.contains(&first_alias)); + assert!(!scheduled.contains(&second_alias)); + } + + #[test] + fn keeps_linear_alias_chain_with_effectful_producer() { + let array_type = ArgType::object_array(); + let allocation = SsaVar::new(10, 0); + let first_alias = SsaVar::new(14, 0); + let second_alias = SsaVar::new(9, 0); + + let mut block = Block::new(0u32); + block.push(InsnNode::new_array( + register(allocation, &array_type), + InsnArg::lit(1, ArgType::INT), + 0, + )); + block.push(InsnNode::mov( + register(first_alias, &array_type), + argument(allocation, &array_type), + )); + block.push(InsnNode::mov( + register(second_alias, &array_type), + argument(first_alias, &array_type), + )); + block.push(InsnNode::invoke( + InvokeType::Static, + 0, + vec![argument(second_alias, &array_type)], + )); + + let scheduled = scheduled_replacements("allocation_alias_chain", block); + + assert!(scheduled.contains(&allocation)); + assert!(scheduled.contains(&first_alias)); + assert!(scheduled.contains(&second_alias)); + } +} diff --git a/dexdec/tests/testcases/expected/ComplexControlFlow.kt.expected b/dexdec/tests/testcases/expected/ComplexControlFlow.kt.expected index 01eccc8..dce19ac 100644 --- a/dexdec/tests/testcases/expected/ComplexControlFlow.kt.expected +++ b/dexdec/tests/testcases/expected/ComplexControlFlow.kt.expected @@ -10,10 +10,10 @@ public open class ComplexControlFlow { return 10 } val v2: Int = p0 + 1 - p0 = v2 if (v2 >= 5) { return v2 } + p0 = v2 } } diff --git a/dexdec/tests/testcases/expected/HardcoreControlFlow.kt.expected b/dexdec/tests/testcases/expected/HardcoreControlFlow.kt.expected index cb1d93e..6208495 100644 --- a/dexdec/tests/testcases/expected/HardcoreControlFlow.kt.expected +++ b/dexdec/tests/testcases/expected/HardcoreControlFlow.kt.expected @@ -99,29 +99,28 @@ public open class HardcoreControlFlow { if (element == -1) { return total } - if (element == -2 || element == -3) { + if (element == -2) { break } - if (element <= 100) { - total += element - index2++ - continue + if (element == -3) { + break } - var step: Int = 0 - while (step < element) { - if (step * index2 > 1000) { - total += step - condition = true + if (element > 100) { + var step: Int = 0 + while (step < element) { + if (step * index2 > 1000) { + total += step + condition = true + break + } + step++ + } + if (condition) { break } - step++ - } - if (condition) { - break } total += element index2++ - continue } } index++ diff --git a/dexdec/tests/testcases/expected/NestedLoopBranches.kt.expected b/dexdec/tests/testcases/expected/NestedLoopBranches.kt.expected index 4359343..e3e3fb6 100644 --- a/dexdec/tests/testcases/expected/NestedLoopBranches.kt.expected +++ b/dexdec/tests/testcases/expected/NestedLoopBranches.kt.expected @@ -12,15 +12,13 @@ public open class NestedLoopBranches { if (step == p2) { return total + step } - val v16: Int = step % 2 - if (v16 != 0) { + if (step % 2 != 0) { total += step + if (total > 1000) { + break + } } - if (v16 == 0 || total <= 1000) { - index2++ - continue - } - break + index2++ } if (total > 1000) { return total diff --git a/dexdec/tests/testcases/expected/SynchronizedAdvanced.kt.expected b/dexdec/tests/testcases/expected/SynchronizedAdvanced.kt.expected index 729dc6e..1a68c70 100644 --- a/dexdec/tests/testcases/expected/SynchronizedAdvanced.kt.expected +++ b/dexdec/tests/testcases/expected/SynchronizedAdvanced.kt.expected @@ -17,11 +17,18 @@ public open class SynchronizedAdvanced { var index: Int = 0 var remaining: Int = 0 while (index < values!!.size) { - synchronized(SynchronizedAdvanced.lock1!!) { + val lock1: Any? = SynchronizedAdvanced.lock1 + var condition: Boolean = false + synchronized(lock1!!) { + var step: Int = 0 try { - remaining += 10 / values[index] + step = 10 / values[index] } catch (exception: ArithmeticException) { remaining-- + condition = true + } + if (!condition) { + remaining += step } } index++ @@ -77,6 +84,8 @@ public open class SynchronizedAdvanced { var result: Int = 0 while (p0 > 0) { val v5: Int = p0 / 2 + p0 = v5 + p0 = v5 result++ p0 = v5 } @@ -88,7 +97,10 @@ public open class SynchronizedAdvanced { synchronized(SynchronizedAdvanced.lock1!!) { var index: Int = 0 var result: Int = 0 - while (index < values!!.size && values[index] >= 0) { + while (index < values!!.size) { + if (values[index] < 0) { + return result + } val v6: Int = result + values[index] index++ result = v6 @@ -124,6 +136,7 @@ public open class SynchronizedAdvanced { result = 100 / p0 } catch (exception: ArithmeticException) { result = -1 + return result } finally { SynchronizedAdvanced.value = 0 } @@ -132,8 +145,8 @@ public open class SynchronizedAdvanced { } public fun syncWithSwitch(p0: Int): Int { + var result: Int = 0 synchronized(SynchronizedAdvanced.lock1!!) { - var result: Int = 0 when (p0) { 1 -> { result = 10 @@ -147,8 +160,8 @@ public open class SynchronizedAdvanced { else -> { } } - return result } + return result } } } From 3677b3b550f997fab285a7bebc1164529513be25 Mon Sep 17 00:00:00 2001 From: LLeaves Date: Wed, 5 Aug 2026 02:19:10 +0800 Subject: [PATCH 09/10] fix(values): keep one-time effects outside loops --- .../analysis/value_recovery/flow/planner.rs | 126 +++++++++++++++++- 1 file changed, 123 insertions(+), 3 deletions(-) diff --git a/dexdec/src/analysis/value_recovery/flow/planner.rs b/dexdec/src/analysis/value_recovery/flow/planner.rs index e3cddab..56affbc 100644 --- a/dexdec/src/analysis/value_recovery/flow/planner.rs +++ b/dexdec/src/analysis/value_recovery/flow/planner.rs @@ -288,6 +288,8 @@ impl<'a> ValuePlanner<'a> { /// read, or call after both actions are composed. Applying producer leaves /// first preserves use cardinality and lets the next recovery stage rebuild /// domains, effects, and use-def facts before moving the expanded consumer. + /// A single lexical use in a loop is also a repeated dynamic use and may + /// not acquire an effect evaluated before that loop. fn retain_replacement_frontier( &self, actions: &mut Vec, @@ -344,7 +346,9 @@ impl<'a> ValuePlanner<'a> { }; let mut dependencies = self.replacement_dependencies(action)?; dependencies.remove(&key); - if (!ssa_identity || self.facts.uses_of(key).len() > 1) + let crosses_repetition = + self.replacement_enters_repetition(key, &dependencies, &blocking_keys); + if (!ssa_identity || self.facts.uses_of(key).len() > 1 || crosses_repetition) && !dependencies.is_disjoint(&blocking_keys) { deferred_keys.insert(key); @@ -394,6 +398,27 @@ impl<'a> ValuePlanner<'a> { Ok(()) } + fn replacement_enters_repetition( + &self, + key: SsaVar, + dependencies: &BTreeSet, + blocking_keys: &BTreeSet, + ) -> bool { + self.facts.uses_of(key).iter().any(|usage| { + usage.repetitive + && dependencies.iter().any(|dependency| { + blocking_keys.contains(dependency) + && self + .graph() + .definitions + .get(dependency) + .is_some_and(|definitions| { + definitions.iter().any(|definition| !definition.repetitive) + }) + }) + }) + } + fn register_move_source(&self, definition: &DefinitionFact) -> Option { let operation = definition.operation()?; let [SemanticExpression::Register(register)] = operation.operands() else { @@ -1860,8 +1885,9 @@ impl ValueAction { mod tests { use super::*; use crate::ir::{ - analysis::SsaValueGraph, block::Block, ArgType, BlockId, InsnNode, InvokeType, RegisterArg, - SemanticBlock, SemanticNode, SemanticStatement, CFG, + analysis::SsaValueGraph, block::Block, ArgType, BlockId, EdgeKind, InsnNode, InvokeType, + RegionId, RegisterArg, SemanticBlock, SemanticLoopControl, SemanticLoopKind, + SemanticLoopTest, SemanticNode, SemanticPredicate, SemanticStatement, CFG, }; fn register(value: SsaVar, ty: &ArgType) -> RegisterArg { @@ -1973,4 +1999,98 @@ mod tests { assert!(scheduled.contains(&first_alias)); assert!(scheduled.contains(&second_alias)); } + + #[test] + fn defers_effectful_phi_replacement_entering_loop() { + let array_type = ArgType::object_array(); + let allocation = SsaVar::new(10, 0); + let initial_alias = SsaVar::new(14, 0); + let loop_value = SsaVar::new(14, 1); + let body_alias = SsaVar::new(9, 0); + let backedge_alias = SsaVar::new(14, 2); + + let mut preheader = Block::new(0u32); + preheader.push(InsnNode::new_array( + register(allocation, &array_type), + InsnArg::lit(1, ArgType::INT), + 0, + )); + preheader.push(InsnNode::mov( + register(initial_alias, &array_type), + argument(allocation, &array_type), + )); + + let mut header = Block::new(1u32); + header.push(InsnNode::phi( + register(loop_value, &array_type), + vec![ + (0, argument(initial_alias, &array_type)), + (2, argument(backedge_alias, &array_type)), + ], + )); + + let mut body = Block::new(2u32); + body.push(InsnNode::mov( + register(body_alias, &array_type), + argument(loop_value, &array_type), + )); + body.push(InsnNode::invoke( + InvokeType::Static, + 0, + vec![argument(body_alias, &array_type)], + )); + body.push(InsnNode::mov( + register(backedge_alias, &array_type), + argument(body_alias, &array_type), + )); + + let mut cfg = CFG::new("effectful_loop_invariant"); + cfg.add_block(preheader); + cfg.add_block(header); + cfg.add_block(body); + cfg.add_edge(BlockId::new(0), BlockId::new(1), EdgeKind::Normal); + cfg.add_edge(BlockId::new(1), BlockId::new(2), EdgeKind::Normal); + cfg.add_edge(BlockId::new(2), BlockId::new(1), EdgeKind::Normal); + cfg.identify_instructions(); + let values = SsaValueGraph::build(&cfg).expect("SSA graph"); + let semantic_block = |id| { + SemanticNode::BasicBlock(SemanticBlock { + id, + statements: cfg + .block(id) + .expect("semantic block") + .insns + .iter() + .filter(|instruction| instruction.insn_type != InsnType::Phi) + .cloned() + .map(|instruction| { + SemanticStatement::instruction(instruction).expect("semantic op") + }) + .collect(), + }) + }; + let mut root = SemanticNode::sequence([ + semantic_block(BlockId::new(0)), + SemanticNode::Loop { + control: SemanticLoopControl::Region(RegionId::new(1)), + header: Some(BlockId::new(1)), + kind: SemanticLoopKind::Endless, + test: SemanticLoopTest::pure(SemanticPredicate::True), + body: Box::new(semantic_block(BlockId::new(2))), + }, + ]); + crate::ir::SemanticSiteNumbering::assign(&mut root).expect("semantic sites"); + + let graph = ValueFlowGraph::build(&root, &values, &BTreeMap::new()).expect("value graph"); + let scheduled = graph + .schedule(RecoveryMode::Full) + .expect("value plan") + .actions + .iter() + .filter_map(ValuePlanner::replacement_key) + .collect::>(); + + assert!(scheduled.contains(&allocation)); + assert!(!scheduled.contains(&loop_value)); + } } From d52c4f380297a2b2544d723a7087b837e466e689 Mon Sep 17 00:00:00 2001 From: LLeaves Date: Wed, 5 Aug 2026 02:49:16 +0800 Subject: [PATCH 10/10] fix(java): reserve static owner qualifiers --- .../java_backend/declaration_lowering.rs | 53 ++++++++++++++++--- .../java_backend/java_model/method.rs | 18 +++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/dexdec/src/analysis/java_backend/declaration_lowering.rs b/dexdec/src/analysis/java_backend/declaration_lowering.rs index c0c2a17..1bf1561 100644 --- a/dexdec/src/analysis/java_backend/declaration_lowering.rs +++ b/dexdec/src/analysis/java_backend/declaration_lowering.rs @@ -1021,6 +1021,19 @@ impl<'a> JavaTypeLowering<'a> { let signature = declaration.signature.as_ref(); let annotations = self.method_annotations(declaration)?; let mut name_scope = crate::language::java::JavaNameScope::default(); + let reserved_type_qualifiers = method + .body + .as_ref() + .into_iter() + .flat_map(super::java_model::method::JavaMethodBody::static_owner_types) + .map(|owner| self.names.resolve_type(&owner)) + .collect::, _>>()? + .iter() + .filter_map(source_type_qualifier) + .collect::>(); + for qualifier in &reserved_type_qualifiers { + name_scope.reserve(qualifier.clone()); + } let parameter_naming = super::semantic_naming::ParameterNameRecovery::new(self.names); let mut visible_parameter = 0usize; let parameter_names = declaration @@ -1166,12 +1179,17 @@ impl<'a> JavaTypeLowering<'a> { source_type_bounds.clone(), generic_throw_types.clone(), outer_instances, - declaration - .kind - .is_class_initializer() - .then(|| owner.map(|owner| self.members.field_names(owner))) - .flatten() - .unwrap_or_default(), + { + let mut reserved = reserved_type_qualifiers.clone(); + if declaration.kind.is_class_initializer() { + reserved.extend( + owner + .map(|owner| self.members.field_names(owner)) + .unwrap_or_default(), + ); + } + reserved + }, declaration.kind.is_class_initializer(), self.observer.clone(), ) @@ -1692,6 +1710,16 @@ fn type_kind(kind: JavaClassKind) -> JavaTypeDeclarationKind { } } +/// The first source component is the expression qualifier that a static +/// member access must keep visible. Reserving it prevents a local binding from +/// turning `Owner.field` into an access through an unrelated local variable. +fn source_type_qualifier(ty: &JavaType) -> Option { + let JavaType::Class(class) = ty else { + return None; + }; + class.segments.first().map(|segment| segment.name.clone()) +} + fn method_kind(kind: MethodModelKind) -> JavaMethodDeclarationKind { match kind { MethodModelKind::Method => JavaMethodDeclarationKind::Method, @@ -1706,6 +1734,19 @@ mod tests { use crate::ir::generic_types::GenericSignatures; use crate::language::java::GenericTypeProjection; + #[test] + fn static_type_qualifier_is_reserved_from_local_names() { + let owner = JavaType::source_class("a"); + let owner = source_type_qualifier(&owner).expect("owner qualifier"); + let mut names = crate::language::java::JavaNameScope::default(); + names.reserve(owner); + + assert_eq!( + names.claim(JavaIdentifier::from_dex("a")), + JavaIdentifier::from_dex("a2") + ); + } + #[test] fn merged_throwable_rethrow_uses_generic_cast_but_catch_parameter_does_not() { let throwable = JavaType::source_class("Throwable"); diff --git a/dexdec/src/analysis/java_backend/java_model/method.rs b/dexdec/src/analysis/java_backend/java_model/method.rs index 816f5f4..3b8f32a 100644 --- a/dexdec/src/analysis/java_backend/java_model/method.rs +++ b/dexdec/src/analysis/java_backend/java_model/method.rs @@ -150,6 +150,14 @@ impl JavaMethodBody { collector.fields } + pub(in crate::analysis::java_backend) fn static_owner_types( + &self, + ) -> std::collections::BTreeSet { + let mut collector = MemberReferenceCollector::default(); + crate::ir::SemanticVisitor::visit_node(&mut collector, self.semantic.body()); + collector.static_owners + } + pub(in crate::analysis::java_backend) fn outer_instance_field( &self, ) -> Option<(&crate::ir::FieldReference, &ArgType)> { @@ -356,15 +364,25 @@ impl JavaMethodBody { struct MemberReferenceCollector { methods: std::collections::BTreeSet, fields: std::collections::BTreeSet, + static_owners: std::collections::BTreeSet, } impl crate::ir::SemanticVisitor for MemberReferenceCollector { fn enter_operation(&mut self, operation: &crate::ir::SemanticOperation) { match operation.payload.reference.as_ref() { Some(crate::ir::MemberReference::Method(method)) => { + if operation.payload.invoke_type == Some(crate::ir::InvokeType::Static) { + self.static_owners.insert(method.owner.clone()); + } self.methods.insert(method.clone()); } Some(crate::ir::MemberReference::Field(field)) => { + if matches!( + operation.insn_type, + crate::ir::InsnType::Sget | crate::ir::InsnType::Sput + ) { + self.static_owners.insert(field.owner.clone()); + } self.fields.insert(field.clone()); } None => {}