Skip to content

Commit

Permalink
[bpf] Fix memory offset check for loads and stores
Browse files Browse the repository at this point in the history
If the offset cannot fit into the instruction, an addition to the
pointer is emitted before the actual access. However, BPF offsets are
16-bit but LLVM considers them to be, for the matter of this check,
to be 32-bit long.

This causes the following program:

int bpf_prog1(void *ign)
{

volatile unsigned long t = 0x8983984739ull;
return *(unsigned long *)((0xffffffff8fff0002ull) + t);

}

To generate the following (wrong) code:

0: 18 01 00 00 39 47 98 83 00 00 00 00 89 00 00 00

r1 = 590618314553ll

2: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
3: 79 a1 f8 ff 00 00 00 00 r1 = *(u64 *)(r10 - 8)
4: 79 10 02 00 00 00 00 00 r0 = *(u64 *)(r1 + 2)
5: 95 00 00 00 00 00 00 00 exit

Fix it by changing the offset check to 16-bit.

Patch by Nadav Amit <nadav.amit@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Differential Revision: https://reviews.llvm.org/D32055

llvm-svn: 300269
  • Loading branch information
4ast committed Apr 13, 2017
1 parent e6185b7 commit 56db145
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
4 changes: 2 additions & 2 deletions llvm/lib/Target/BPF/BPFISelDAGToDAG.cpp
Expand Up @@ -71,7 +71,7 @@ bool BPFDAGToDAGISel::SelectAddr(SDValue Addr, SDValue &Base, SDValue &Offset) {
// Addresses of the form Addr+const or Addr|const
if (CurDAG->isBaseWithConstantOffset(Addr)) {
ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Addr.getOperand(1));
if (isInt<32>(CN->getSExtValue())) {
if (isInt<16>(CN->getSExtValue())) {

// If the first operand is a FI, get the TargetFI Node
if (FrameIndexSDNode *FIN =
Expand Down Expand Up @@ -99,7 +99,7 @@ bool BPFDAGToDAGISel::SelectFIAddr(SDValue Addr, SDValue &Base, SDValue &Offset)

// Addresses of the form Addr+const or Addr|const
ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Addr.getOperand(1));
if (isInt<32>(CN->getSExtValue())) {
if (isInt<16>(CN->getSExtValue())) {

// If the first operand is a FI, get the TargetFI Node
if (FrameIndexSDNode *FIN =
Expand Down
17 changes: 17 additions & 0 deletions llvm/test/CodeGen/BPF/mem_offset.ll
@@ -0,0 +1,17 @@
; RUN: llc -march=bpfel -show-mc-encoding < %s | FileCheck %s

; Function Attrs: nounwind
define i32 @bpf_prog1(i8* nocapture readnone) local_unnamed_addr #0 {
; CHECK: r1 += -1879113726 # encoding: [0x07,0x01,0x00,0x00,0x02,0x00,0xff,0x8f]
; CHECK: r0 = *(u64 *)(r1 + 0) # encoding: [0x79,0x10,0x00,0x00,0x00,0x00,0x00,0x00]
%2 = alloca i64, align 8
%3 = bitcast i64* %2 to i8*
store volatile i64 590618314553, i64* %2, align 8
%4 = load volatile i64, i64* %2, align 8
%5 = add i64 %4, -1879113726
%6 = inttoptr i64 %5 to i64*
%7 = load i64, i64* %6, align 8
%8 = trunc i64 %7 to i32
ret i32 %8
}

0 comments on commit 56db145

Please sign in to comment.