Re: [PATCH bpf-next 1/2] bpf: fix a verifier failure with xor

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>
Cc: Yonghong Song <yhs@fb.com>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>
Subject: Re: [PATCH bpf-next 1/2] bpf: fix a verifier failure with xor
Date: Wed, 2 Sep 2020 07:21:58 -0700	[thread overview]
Message-ID: <20200902142158.hp26mv7dxphzyhun@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <87d034ikve.fsf@toke.dk>

On Wed, Sep 02, 2020 at 11:33:09AM +0200, Toke Høiland-Jørgensen wrote:
> Yonghong Song <yhs@fb.com> writes:
> 
> > On 9/1/20 1:07 PM, Andrii Nakryiko wrote:
> >> On Mon, Aug 24, 2020 at 11:47 PM Yonghong Song <yhs@fb.com> wrote:
> >>>
> >>> bpf selftest test_progs/test_sk_assign failed with llvm 11 and llvm 12.
> >>> Compared to llvm 10, llvm 11 and 12 generates xor instruction which
> >> 
> >> Does this mean that some perfectly working BPF programs will now fail
> >> to verify on older kernels, if compiled with llvm 11 or llvm 12? If
> >
> > Right.
> >
> >> yes, is there something that one can do to prevent Clang from using
> >> xor in such situations?
> >
> > The xor is generated by the combination of llvm simplifyCFG and 
> > instrCombine phase.
> >
> > The following is a hack to prevent compiler from generating xor's.
> 
> Wait, so this means that we can no longer tell people to just use the
> newest LLVM version - now we have to keep track of a minimum *and*
> maximum LLVM version for each kernel version?

No. The only way is forward. Everyone has to upgrade their llvm periodically.

> Could we maybe try to not *keep* making it harder for people to use BPF? :/

Whom do you mean by "we" ?

> As for the patch, sure, make the verifier smarter, but I also feel like
> LLVM should be fixed to not suddenly emit such xor instructions...

I don't think there is anything to be "fixed". It's not a bug form llvm
developers point of view. At least I suspect that's the response you
will get if you post the same sentence on llvm-dev mailing list.
If you care to help, please bisect which llvm commit introduced this change.
May be author (whoever that was) will have ideas how to pessimize it
specifically for bpf backend. But I suspect they will refuse to do so.
The discussion about partial disable of optimizations was brought up
several times. tldr optimizations cannot be disabled effectively.
Pretty much all of them may cause trouble for the verifier and
all of them are often necessary for the verifier as well.
Please read this thread:
http://clang-developers.42468.n3.nabble.com/Disable-certain-llvm-optimizations-at-clang-frontend-tp4068601.html