Skip to content

Commit

Permalink
Call heuristics: defuse 'recent lr update' after a branch.
Browse files Browse the repository at this point in the history
This protects against misidentifying an internal branch within a
function as a potential call on the strength of a sequence such as

  callee pops LR
  callee returns to LR
  caller tests return value
  caller conditionally branches

in which the final conditional branch would previously have been a
PendingCall candidate on the grounds that LR had been written <8
instructions previously. But I think that because another branch
(the RET) happened _between_ the LR write and the conditional branch,
that disqualifies the latter from being a call with that LR as its
return value.

The same thing can also happen to a conditional branch just after a
call, because that too is a moment where LR has just been written. But
that version of the problem only triggers if the call instruction and
the callee are very close together, because of the 'within 64 bytes'
clause in the heuristic analysis. However, that can happen when a
function recursively calls itself - which caused 7 of our existing
regression tests to fail, because the call tree analysis of
tests/quicksort.tarmac had changed, because that error was happening
in the recursive calls to quicksort!

(An easy-to-observe reason why the new expected output for those tests
is better: the tarmac-calltree output with symbols listed some calls
to quicksort as having address 0x8038 and some as 0x8054, which is
suspicious, because surely a function should have only one address. In
fact 0x8038 is the real address of quicksort() in that test trace, and
0x8054 is an internal branch label. Now all the calls to quicksort
list the former.)
  • Loading branch information
statham-arm committed Jun 14, 2024
1 parent 0888d26 commit 2d514a4
Show file tree
Hide file tree
Showing 16 changed files with 478 additions and 332 deletions.
4 changes: 4 additions & 0 deletions lib/index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ void Index::update_pc(unsigned long long pc, unsigned long long next_pc,

pending_calls.insert(PendingCall(sp, lr, prev_lineno));
}

// After a transfer of control, reset insns_since_lr_update to
// pretend lr hasn't been updated recently.
insns_since_lr_update = BRANCH_LR_WRITE_THRESHOLD;
}

curr_pc = pc;
Expand Down
28 changes: 28 additions & 0 deletions samples/source/calltests/calltest32.S
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ loopend:
// of call, so these ones can be done the standard way.
bl internal_bl
bl internal_bx_lr
bl branch_after_call_wrapper
// After returning from this function, immediately take a
// conditional branch, which should not be identified as even a
// _candidate_ for being a call.
cmp r0, #122
bne branch_target_after_return
mov r0, #121
branch_target_after_return:

// We're done. Exit.
mov r0, #0x18
Expand Down Expand Up @@ -139,3 +147,23 @@ internal_bx_lr:
b done
done: mul r0, r0, r0
pop {r4, pc}

// Wrapper function that calls branch_after_call, while being
// very near it, so that the internal branch might trip the
// call detector.
branch_after_call_wrapper:
push {r4, lr}
bl branch_after_call
pop {r4, lr}
bx lr
// A function that does a conditional branch immediately after
// being called.
branch_after_call:
push {r4, lr}
cmp r0, #123
bne branch_after_call_internal_label
mov r0, #122
branch_after_call_internal_label:
add r0, r0, #10
pop {r4, lr}
bx lr
31 changes: 31 additions & 0 deletions samples/source/calltests/calltest64.S
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ loopstart: mov x1, #0x100000000
b leaf_ret
loopend:

// More calls to test functions. I've run out of unusual types
// of call, so these ones can be done the standard way.
bl branch_after_call_wrapper
// After returning from this function, immediately take a
// conditional branch, which should not be identified as even a
// _candidate_ for being a call.
cmp x0, #122
bne branch_target_after_return
mov x0, #121
branch_target_after_return:

// We're done. Exit.
mov x0, #0x18
ldr x1, =0x20026
Expand All @@ -71,3 +82,23 @@ nonleaf_pop_lr:

// Helper function for the indirect call above.
call_via_x1: br x1

// Wrapper function that calls branch_after_call, while being
// very near it, so that the internal branch might trip the
// call detector.
branch_after_call_wrapper:
stp x19, lr, [sp, #-16]!
bl branch_after_call
ldp x19, lr, [sp], #16
ret
// A function that does a conditional branch immediately after
// being called.
branch_after_call:
stp x19, lr, [sp, #-16]!
cmp x0, #123
bne branch_after_call_internal_label
mov x0, #122
branch_after_call_internal_label:
add x0, x0, #10
ldp x19, lr, [sp], #16
ret
Binary file modified tests/calltests/calltest32.elf
Binary file not shown.
102 changes: 58 additions & 44 deletions tests/calltests/calltest32.ref
Original file line number Diff line number Diff line change
@@ -1,86 +1,100 @@
transfer of control @ 52, sp=ffffffffffffffff, pc=200b4
transfer of control @ 60, sp=100000, pc=20108
transfer of control @ 60, sp=100000, pc=20118
inserting as pending call with sp=100000 lr=200c4
transfer of control @ 67, sp=100000, pc=200c4
looks like return for call @ 60
transfer of control @ 72, sp=100000, pc=20128
transfer of control @ 72, sp=100000, pc=20138
inserting as pending call with sp=100000 lr=200cc
transfer of control @ 78, sp=ffff8, pc=20108
inserting as pending call with sp=ffff8 lr=20130
transfer of control @ 85, sp=ffff8, pc=20130
transfer of control @ 78, sp=ffff8, pc=20118
inserting as pending call with sp=ffff8 lr=20140
transfer of control @ 85, sp=ffff8, pc=20140
looks like return for call @ 78
transfer of control @ 87, sp=ffff8, pc=20118
inserting as pending call with sp=ffff8 lr=20134
transfer of control @ 94, sp=ffff8, pc=20134
transfer of control @ 87, sp=ffff8, pc=20128
inserting as pending call with sp=ffff8 lr=20144
transfer of control @ 94, sp=ffff8, pc=20144
looks like return for call @ 87
transfer of control @ 99, sp=100000, pc=200cc
looks like return for call @ 72
transfer of control @ 105, sp=100000, pc=20138
transfer of control @ 105, sp=100000, pc=20148
inserting as pending call with sp=100000 lr=200d8
transfer of control @ 111, sp=ffff8, pc=20118
inserting as pending call with sp=ffff8 lr=20140
transfer of control @ 118, sp=ffff8, pc=20140
transfer of control @ 111, sp=ffff8, pc=20128
inserting as pending call with sp=ffff8 lr=20150
transfer of control @ 118, sp=ffff8, pc=20150
looks like return for call @ 111
transfer of control @ 120, sp=ffff8, pc=20108
inserting as pending call with sp=ffff8 lr=20144
transfer of control @ 127, sp=ffff8, pc=20144
transfer of control @ 120, sp=ffff8, pc=20118
inserting as pending call with sp=ffff8 lr=20154
transfer of control @ 127, sp=ffff8, pc=20154
looks like return for call @ 120
transfer of control @ 134, sp=100000, pc=200d8
looks like return for call @ 105
transfer of control @ 139, sp=100000, pc=2014c
transfer of control @ 139, sp=100000, pc=2015c
inserting as pending call with sp=100000 lr=200e0
transfer of control @ 140, sp=100000, pc=20118
transfer of control @ 140, sp=100000, pc=20128
transfer of control @ 147, sp=100000, pc=200e0
looks like return for call @ 139
transfer of control @ 156, sp=100000, pc=20108
transfer of control @ 156, sp=100000, pc=20118
inserting as pending call with sp=100000 lr=200e4
transfer of control @ 163, sp=100000, pc=200e4
looks like return for call @ 156
transfer of control @ 170, sp=100000, pc=20108
transfer of control @ 170, sp=100000, pc=20118
inserting as pending call with sp=100000 lr=200e4
transfer of control @ 177, sp=100000, pc=200e4
looks like return for call @ 170
transfer of control @ 184, sp=100000, pc=20108
transfer of control @ 184, sp=100000, pc=20118
inserting as pending call with sp=100000 lr=200e4
transfer of control @ 191, sp=100000, pc=200e4
looks like return for call @ 184
transfer of control @ 194, sp=100000, pc=200f4
transfer of control @ 196, sp=100000, pc=20150
transfer of control @ 196, sp=100000, pc=20160
inserting as pending call with sp=100000 lr=200f8
transfer of control @ 208, sp=ffff8, pc=20168
inserting as pending call with sp=ffff8 lr=20164
transfer of control @ 208, sp=ffff8, pc=20178
inserting as pending call with sp=ffff8 lr=20174
transfer of control @ 219, sp=100000, pc=200f8
looks like return for call @ 196
transfer of control @ 221, sp=100000, pc=20178
transfer of control @ 221, sp=100000, pc=20188
inserting as pending call with sp=100000 lr=200fc
transfer of control @ 230, sp=ffff8, pc=201a0
inserting as pending call with sp=ffff8 lr=201a0
transfer of control @ 230, sp=ffff8, pc=201b0
inserting as pending call with sp=ffff8 lr=201b0
transfer of control @ 240, sp=100000, pc=200fc
looks like return for call @ 221
o t:1000000 l:52 pc:0x200b4 - t:94000000 l:245 pc:0x20104 :
transfer of control @ 242, sp=100000, pc=201c0
inserting as pending call with sp=100000 lr=20100
transfer of control @ 248, sp=ffff8, pc=201d0
inserting as pending call with sp=ffff8 lr=201c8
transfer of control @ 255, sp=ffff0, pc=201e0
transfer of control @ 264, sp=ffff8, pc=201c8
looks like return for call @ 248
transfer of control @ 271, sp=100000, pc=20100
looks like return for call @ 242
transfer of control @ 274, sp=100000, pc=2010c
o t:1000000 l:52 pc:0x200b4 - t:107000000 l:279 pc:0x20114 :
- t:4000000 l:58 pc:0x200c0 - t:9000000 l:67 pc:0x200c4
o t:5000000 l:60 pc:0x20108 - t:8000000 l:66 pc:0x20114 : leaf_bx_lr
o t:5000000 l:60 pc:0x20118 - t:8000000 l:66 pc:0x20124 : leaf_bx_lr
- t:10000000 l:70 pc:0x200c8 - t:23000000 l:99 pc:0x200cc
o t:11000000 l:72 pc:0x20128 - t:22000000 l:94 pc:0x20134 : nonleaf_pop_pc
- t:12000000 l:76 pc:0x2012c - t:17000000 l:85 pc:0x20130
o t:13000000 l:78 pc:0x20108 - t:16000000 l:84 pc:0x20114 : leaf_bx_lr
- t:17000000 l:85 pc:0x20130 - t:22000000 l:94 pc:0x20134
o t:18000000 l:87 pc:0x20118 - t:21000000 l:93 pc:0x20124 : leaf_mov_pc_lr
o t:11000000 l:72 pc:0x20138 - t:22000000 l:94 pc:0x20144 : nonleaf_pop_pc
- t:12000000 l:76 pc:0x2013c - t:17000000 l:85 pc:0x20140
o t:13000000 l:78 pc:0x20118 - t:16000000 l:84 pc:0x20124 : leaf_bx_lr
- t:17000000 l:85 pc:0x20140 - t:22000000 l:94 pc:0x20144
o t:18000000 l:87 pc:0x20128 - t:21000000 l:93 pc:0x20134 : leaf_mov_pc_lr
- t:25000000 l:104 pc:0x200d4 - t:39000000 l:134 pc:0x200d8
o t:26000000 l:105 pc:0x20138 - t:38000000 l:133 pc:0x20148 : nonleaf_pop_lr
- t:27000000 l:109 pc:0x2013c - t:32000000 l:118 pc:0x20140
o t:28000000 l:111 pc:0x20118 - t:31000000 l:117 pc:0x20124 : leaf_mov_pc_lr
- t:32000000 l:118 pc:0x20140 - t:37000000 l:127 pc:0x20144
o t:33000000 l:120 pc:0x20108 - t:36000000 l:126 pc:0x20114 : leaf_bx_lr
o t:26000000 l:105 pc:0x20148 - t:38000000 l:133 pc:0x20158 : nonleaf_pop_lr
- t:27000000 l:109 pc:0x2014c - t:32000000 l:118 pc:0x20150
o t:28000000 l:111 pc:0x20128 - t:31000000 l:117 pc:0x20134 : leaf_mov_pc_lr
- t:32000000 l:118 pc:0x20150 - t:37000000 l:127 pc:0x20154
o t:33000000 l:120 pc:0x20118 - t:36000000 l:126 pc:0x20124 : leaf_bx_lr
- t:40000000 l:137 pc:0x200dc - t:46000000 l:147 pc:0x200e0
o t:41000000 l:139 pc:0x2014c - t:45000000 l:146 pc:0x20124 : call_via_r1
o t:41000000 l:139 pc:0x2015c - t:45000000 l:146 pc:0x20134 : call_via_r1
- t:50000000 l:155 pc:0x200f0 - t:55000000 l:163 pc:0x200e4
o t:51000000 l:156 pc:0x20108 - t:54000000 l:162 pc:0x20114 : leaf_bx_lr
o t:51000000 l:156 pc:0x20118 - t:54000000 l:162 pc:0x20124 : leaf_bx_lr
- t:58000000 l:169 pc:0x200f0 - t:63000000 l:177 pc:0x200e4
o t:59000000 l:170 pc:0x20108 - t:62000000 l:176 pc:0x20114 : leaf_bx_lr
o t:59000000 l:170 pc:0x20118 - t:62000000 l:176 pc:0x20124 : leaf_bx_lr
- t:66000000 l:183 pc:0x200f0 - t:71000000 l:191 pc:0x200e4
o t:67000000 l:184 pc:0x20108 - t:70000000 l:190 pc:0x20114 : leaf_bx_lr
o t:67000000 l:184 pc:0x20118 - t:70000000 l:190 pc:0x20124 : leaf_bx_lr
- t:73000000 l:194 pc:0x200f4 - t:83000000 l:219 pc:0x200f8
o t:74000000 l:196 pc:0x20150 - t:82000000 l:214 pc:0x20174 : internal_bl
o t:74000000 l:196 pc:0x20160 - t:82000000 l:214 pc:0x20184 : internal_bl
- t:83000000 l:219 pc:0x200f8 - t:92000000 l:240 pc:0x200fc
o t:84000000 l:221 pc:0x20178 - t:91000000 l:235 pc:0x201ac : internal_bx_lr
o t:84000000 l:221 pc:0x20188 - t:91000000 l:235 pc:0x201bc : internal_bx_lr
- t:92000000 l:240 pc:0x200fc - t:103000000 l:271 pc:0x20100
o t:93000000 l:242 pc:0x201c0 - t:102000000 l:270 pc:0x201cc : branch_after_call_wrapper
- t:94000000 l:246 pc:0x201c4 - t:101000000 l:264 pc:0x201c8
o t:95000000 l:248 pc:0x201d0 - t:100000000 l:263 pc:0x201e8 : branch_after_call
Loading

0 comments on commit 2d514a4

Please sign in to comment.