This is an archive of the discontinued LLVM Phabricator instance.

[BPF] Emit fatal error if out of range for FK_PCRel_2 branch target
ClosedPublic

Authored by yonghong-song on Apr 15 2022, 3:11 PM.

Details

Summary

Currently for the branch insn like

"if $dst "#OpcodeStr#" $imm goto $BrDst"

The $BrDst range needs to be in the range of [INT16_MIN, INT16_MAX].

When running bpf selftest with latest llvm, I found
pyperf600.o generated insn with range outside
of [INT16_MIN, INT16_MAX], which caused verifier failure.
See below insn #12.

0000000000000000 <on_event>:
; { 
       0:       7b 1a 00 ff 00 00 00 00 *(u64 *)(r10 - 256) = r1
;       uint64_t pid_tgid = bpf_get_current_pid_tgid();
       1:       85 00 00 00 0e 00 00 00 call 14
       2:       bf 06 00 00 00 00 00 00 r6 = r0
;       pid_t pid = (pid_t)(pid_tgid >> 32);
       3:       bf 61 00 00 00 00 00 00 r1 = r6
       4:       77 01 00 00 20 00 00 00 r1 >>= 32
       5:       63 1a fc ff 00 00 00 00 *(u32 *)(r10 - 4) = r1
       6:       bf a2 00 00 00 00 00 00 r2 = r10 
       7:       07 02 00 00 fc ff ff ff r2 += -4
;       PidData* pidData = bpf_map_lookup_elem(&pidmap, &pid);
       8:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
      10:       85 00 00 00 01 00 00 00 call 1
      11:       bf 08 00 00 00 00 00 00 r8 = r0
;       if (!pidData)
      12:       15 08 15 e8 00 00 00 00 if r8 == 0 goto -6123 <LBB0_27588+0xffffffffffdae100>
      13:       b4 01 00 00 00 00 00 00 w1 = 0

We may need to add new insn to extend the range of $BrDst.
This patch added fatal error if out of range so compiler can warn
the otherwise incorrect code generation.

Diff Detail

Event Timeline

yonghong-song created this revision.Apr 15 2022, 3:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2022, 3:11 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
yonghong-song requested review of this revision.Apr 15 2022, 3:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2022, 3:11 PM
ast accepted this revision.Apr 19 2022, 10:29 AM

assert is a heavy hammer.
can we exit with report_fatal_error instead?

This revision is now accepted and ready to land.Apr 19 2022, 10:29 AM

assert is a heavy hammer.
can we exit with report_fatal_error instead?

Okay. Let me implement with a report_fatal_error.

yonghong-song retitled this revision from [BPF] Add assert for the range of FK_PCRel_2 branch target to [BPF] Emit fatal error if out of range for FK_PCRel_2 branch target.
yonghong-song edited the summary of this revision. (Show Details)
  • use report_fatal_error instead of assert.
ast accepted this revision.Apr 19 2022, 1:30 PM

Thanks!

This revision was landed with ongoing or failed builds.Apr 19 2022, 2:42 PM
This revision was automatically updated to reflect the committed changes.