[llvm-dev] 5.0.1-rc2 has been tagged

[Y Song via llvm-dev]

Hi, Tom,

Considering the severity of this bug, I would like to go ahead to push
the fix into release_50 branch. The fix has been tested in the trunk and by
various people as well and I will also make sure all BPF tests passed
before the push.

Thanks!

Yonghong

On Fri, Dec 1, 2017 at 10:18 AM, Y Song <ys114321 at gmail.com> wrote:
> Hi, Tom,
>
> I have a BPF backend bug which is pretty noxious which is introduced
> in 5.0 and fixed in 6.0 trunk.
> The detailed of the backport commit is at the end of this email, which
> is slightly different from
> 6.0 fix to adjust the asm output format. Is there a way for this bug
> fix into 5.0.1 release and if yes,
> what is the process?
>
> I have applied the commit below to latest release_50 branch in my
> local repo and run through all BPF tests
> and they are fine.
>
> Thanks!
>
> Yonghong
>
> ===== Commit to backport Details =====
> commit a115edb82180f0c94d692c4abfd631984ada156b
> Author: Yonghong Song <yhs at fb.com>
> Date:   Mon Oct 16 04:14:53 2017 +0000
>
>     bpf: fix bug on silently truncating 64-bit immediate
>
>     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 (llvm 5.0). 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.
>
>     The bug is fixed by
>     git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@315889
> 91177308-0d34-0410-b5e6-96231b3b80d8
>     in llvm 6.0.
>
>     This patch is largely the same as the fix in llvm 6.0 except
>     one minor adjustment ("; CHECK:  r{{[0-9]+}} = 8589934591 ll" in
> the original commit
>     to "; CHECK:  r{{[0-9]+}} = 8589934591ll" in this patch) for the test case.
>
>     Reported-by: John Fastabend <john.fastabend at gmail.com>
>     Reported-by: Jakub Kicinski <jakub.kicinski at netronome.com>
>     Signed-off-by: Jiong Wang <jiong.wang at netronome.com>
>     Reviewed-by: Yonghong Song <yhs at fb.com>
>
> diff --git a/lib/Target/BPF/BPFISelLowering.cpp
> b/lib/Target/BPF/BPFISelLowering.cpp
> index 81b0aa7..5740b49 100644
> --- a/lib/Target/BPF/BPFISelLowering.cpp
> +++ b/lib/Target/BPF/BPFISelLowering.cpp
> @@ -578,11 +578,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 f683578..56f0f9c 100644
> --- a/lib/Target/BPF/BPFInstrInfo.td
> +++ b/lib/Target/BPF/BPFInstrInfo.td
> @@ -464,7 +464,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 c4ac376..3610d40 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]+}} = 8589934591ll
> +  %6 = sext i1 %5 to i32
> +  ret i32 %6
> +}
> +
> +; Function Attrs: nounwind readonly
> +declare i64 @llvm.bpf.load.word(i8*, i64) #1
>
> On Wed, Nov 29, 2017 at 4:19 PM, Tom Stellard via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> Hi,
>>
>> I've tagged the 5.0.1-rc2 release, go ahead and start testing and report
>> your results.
>>
>> -Tom
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev