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

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> 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.

Right, great! But surely that implies that a regression such as that
described here, where a new LLVM version turns a previously-valid
program into one that no longer verifies is a bug, no?

>> Could we maybe try to not *keep* making it harder for people to use BPF? :/
>
> Whom do you mean by "we" ?

I mean "we as a community who would like BPF to be as useful as possible
to as many people as possible". Usability is a big part of this.

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

I am not enough of a compiler person to get the nuances of that
discussion, but it seems that the last message[0] by Y Song seems to
imply that you guys do want to fix such issues in LLVM, just not by
disabling the optimisation, but at a later stage in the processing
pipeline?

-Toke

[0] http://lists.llvm.org/pipermail/cfe-dev/2020-June/066015.html