This is an archive of the discontinued LLVM Phabricator instance.

Differential D32055

[bpf] Fix memory offset check for loads and stores
ClosedPublic

Authored by ast on Apr 13 2017, 2:46 PM.

Details

Reviewers
namit
ast
Commits
rG56db14516450: [bpf] Fix memory offset check for loads and stores
rL300269: [bpf] Fix memory offset check for loads and stores
Summary

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.

Diff Detail

Repository
rL LLVM

Event Timeline

namit created this revision.Apr 13 2017, 2:46 PM
Harbormaster completed remote builds in B5552: Diff 95218.Apr 13 2017, 2:46 PM
ast accepted this revision.Apr 13 2017, 3:07 PM
This revision is now accepted and ready to land.Apr 13 2017, 3:07 PM
ast commandeered this revision.Apr 13 2017, 3:10 PM
ast edited reviewers, added: namit; removed: ast.
This revision now requires review to proceed.Apr 13 2017, 3:10 PM
ast accepted this revision.Apr 13 2017, 3:10 PM
This revision is now accepted and ready to land.Apr 13 2017, 3:10 PM
Closed by commit rL300269: [bpf] Fix memory offset check for loads and stores (authored by ast). · Explain WhyApr 13 2017, 3:37 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
trunk/
lib/
Target/
BPF/
4 lines
test/
CodeGen/
BPF/
17 lines

Diff 95230

llvm/trunk/lib/Target/BPF/BPFISelDAGToDAG.cpp

Show First 20 Lines • Show All 65 Lines • ▼ Show 20 Linesbool BPFDAGToDAGISel::SelectAddr(SDValue Addr, SDValue &Base, SDValue &Offset) {
if (Addr.getOpcode() == ISD::TargetExternalSymbol || if (Addr.getOpcode() == ISD::TargetExternalSymbol ||
Addr.getOpcode() == ISD::TargetGlobalAddress) Addr.getOpcode() == ISD::TargetGlobalAddress)
return false; return false;
// Addresses of the form Addr+const or Addr|const // Addresses of the form Addr+const or Addr|const
if (CurDAG->isBaseWithConstantOffset(Addr)) { if (CurDAG->isBaseWithConstantOffset(Addr)) {
ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Addr.getOperand(1)); 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 the first operand is a FI, get the TargetFI Node
if (FrameIndexSDNode *FIN = if (FrameIndexSDNode *FIN =
dyn_cast<FrameIndexSDNode>(Addr.getOperand(0))) dyn_cast<FrameIndexSDNode>(Addr.getOperand(0)))
Base = CurDAG->getTargetFrameIndex(FIN->getIndex(), MVT::i64); Base = CurDAG->getTargetFrameIndex(FIN->getIndex(), MVT::i64);
else else
Base = Addr.getOperand(0); Base = Addr.getOperand(0);
Show All 11 Lines
bool BPFDAGToDAGISel::SelectFIAddr(SDValue Addr, SDValue &Base, SDValue &Offset) { bool BPFDAGToDAGISel::SelectFIAddr(SDValue Addr, SDValue &Base, SDValue &Offset) {
SDLoc DL(Addr); SDLoc DL(Addr);
if (!CurDAG->isBaseWithConstantOffset(Addr)) if (!CurDAG->isBaseWithConstantOffset(Addr))
return false; return false;
// Addresses of the form Addr+const or Addr|const // Addresses of the form Addr+const or Addr|const
ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Addr.getOperand(1)); 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 the first operand is a FI, get the TargetFI Node
if (FrameIndexSDNode *FIN = if (FrameIndexSDNode *FIN =
dyn_cast<FrameIndexSDNode>(Addr.getOperand(0))) dyn_cast<FrameIndexSDNode>(Addr.getOperand(0)))
Base = CurDAG->getTargetFrameIndex(FIN->getIndex(), MVT::i64); Base = CurDAG->getTargetFrameIndex(FIN->getIndex(), MVT::i64);
else else
return false; return false;
▲ Show 20 Lines • Show All 76 Lines • Show Last 20 Lines

llvm/trunk/test/CodeGen/BPF/mem_offset.ll

; 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
}