Date
1 - 4 of 4
[PATCH, bpf-llvm] Fix bug on silently truncating 64-bit immediate
|
Yonghong Song
Hi, Jiong,
The patch looks good. Thanks for fixing the issue. Will merge soon. Yonghong On Thu, Oct 12, 2017 at 5:08 AM, Jiong Wang via iovisor-dev <iovisor-dev@...> wrote: On 12/10/2017 12:04, Jesper Dangaard Brouer wrote: |
|
|
|
Jiong Wang
On 12/10/2017 12:04, Jesper Dangaard Brouer wrote:
On Thu, 12 Oct 2017 06:21:30 -0400Hi Jesper, I am thinking the users & developers of eBPF llvm are quite focused in xdp-newbies and iovisor-dev, so if the code is related with user visible effect, for example bug fix, and if the code change is straightforward and small, I tend to just send it to these two so related user could be aware of it in case they are encountering the same problem, and then this patch could be posted to llvm review site (https://reviews.llvm.org). This has not been made into any LLVM official release.This bug looks to be introduced by r308080.This looks like a very recent change: The bug may show up if the user is trying to use distributor snapshop release. -- Regards, Jiong |
|
|
|
On Thu, 12 Oct 2017 06:21:30 -0400
Jiong Wang <jiong.wang@...> wrote: We came across an llvm bug when compiling some testcases that 64-bitI think you send this to the wrong mailing list... this looks like a patch against the LLVM source code. Shouldn't you send to: llvm-dev@... ? This bug looks to be introduced by r308080.This looks like a very recent change: https://llvm.org/viewvc/llvm-project?view=revision&revision=308080 Sat Jul 15 05:41:42 2017 UTC (2 months, 4 weeks ago) (As you are sending this to a user mailing list: xdp-newbies@...) I want to know if this have made it into a LLVM release? and which release? As the git-repo[1] replica of LLVM SVN-repo does not git-tag the releases, I could not answer this question myself via the command: $ git describe --contains 7c423e0690 [1] http://llvm.org/git/llvm.git The Select_Ri pattern is-- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer |
|
|
|
Jiong Wang
We came across an llvm bug when compiling some testcases that 64-bit
immediates are silently truncated into 32-bit and then packed into BPF_JMP | BPF_K encoding. This caused comparison with wrong value. This bug looks to be introduced by r308080. The Select_Ri pattern is supposed to be lowered into J*_Ri while the latter only support 32-bit immediate encoding, therefore Select_Ri should have similar immediate predicate check as what J*_Ri are doing. Reported-by: Jakub Kicinski <jakub.kicinski@...> Signed-off-by: Jiong Wang <jiong.wang@...> --- lib/Target/BPF/BPFISelLowering.cpp | 8 ++++++-- lib/Target/BPF/BPFInstrInfo.td | 2 +- test/CodeGen/BPF/select_ri.ll | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/lib/Target/BPF/BPFISelLowering.cpp b/lib/Target/BPF/BPFISelLowering.cpp index d4e06dd..995f206 100644 --- a/lib/Target/BPF/BPFISelLowering.cpp +++ b/lib/Target/BPF/BPFISelLowering.cpp @@ -611,11 +611,15 @@ BPFTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI, .addReg(LHS) .addReg(MI.getOperand(2).getReg()) .addMBB(Copy1MBB); - else + else { + int64_t imm32 = MI.getOperand(2).getImm(); + // sanity check before we build J*_ri instruction. + assert (isInt<32>(imm32)); BuildMI(BB, DL, TII.get(NewCC)) .addReg(LHS) - .addImm(MI.getOperand(2).getImm()) + .addImm(imm32) .addMBB(Copy1MBB); + } // Copy0MBB: // %FalseValue = ... diff --git a/lib/Target/BPF/BPFInstrInfo.td b/lib/Target/BPF/BPFInstrInfo.td index fcd6a60..a3ad2ee 100644 --- a/lib/Target/BPF/BPFInstrInfo.td +++ b/lib/Target/BPF/BPFInstrInfo.td @@ -460,7 +460,7 @@ let usesCustomInserter = 1 in { (ins GPR:$lhs, i64imm:$rhs, i64imm:$imm, GPR:$src, GPR:$src2), "# Select PSEUDO $dst = $lhs $imm $rhs ? $src : $src2", [(set i64:$dst, - (BPFselectcc i64:$lhs, (i64 imm:$rhs), (i64 imm:$imm), i64:$src, i64:$src2))]>; + (BPFselectcc i64:$lhs, (i64immSExt32:$rhs), (i64 imm:$imm), i64:$src, i64:$src2))]>; } // load 64-bit global addr into register diff --git a/test/CodeGen/BPF/select_ri.ll b/test/CodeGen/BPF/select_ri.ll index b802b64..7b1f852 100644 --- a/test/CodeGen/BPF/select_ri.ll +++ b/test/CodeGen/BPF/select_ri.ll @@ -25,3 +25,38 @@ entry: } attributes #0 = { norecurse nounwind readonly } + +; test immediate out of 32-bit range +; Source file: + +; unsigned long long +; load_word(void *buf, unsigned long long off) +; asm("llvm.bpf.load.word"); +; +; int +; foo(void *buf) +; { +; unsigned long long sum = 0; +; +; sum += load_word(buf, 100); +; sum += load_word(buf, 104); +; +; if (sum != 0x1ffffffffULL) +; return ~0U; +; +; return 0; +;} + +; Function Attrs: nounwind readonly +define i32 @foo(i8*) local_unnamed_addr #0 { + %2 = tail call i64 @llvm.bpf.load.word(i8* %0, i64 100) + %3 = tail call i64 @llvm.bpf.load.word(i8* %0, i64 104) + %4 = add i64 %3, %2 + %5 = icmp ne i64 %4, 8589934591 +; CHECK: r{{[0-9]+}} = 8589934591 ll + %6 = sext i1 %5 to i32 + ret i32 %6 +} + +; Function Attrs: nounwind readonly +declare i64 @llvm.bpf.load.word(i8*, i64) #1 -- 2.7.4 |
|
|