From mboxrd@z Thu Jan 1 00:00:00 1970 From: Edward Cree Subject: Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF_END instructions Date: Fri, 22 Sep 2017 14:46:42 +0100 Message-ID: References: <7013ee9d-a8e6-13fd-cc5f-86cf3d8bf4e0@solarflare.com> <20170921155215.jta52sesbiq54vri@ast-mbp> <4cfac985-4f99-cf85-fc15-c3ad1f8ff123@solarflare.com> <207ecd4c-b1b4-3dcd-62a6-30824c19dbf7@solarflare.com> <59C4131D.8050003@iogearbox.net> <20170921194426.tnd5xos5irm3gred@ast-mbp> <46aa4442-b8ed-e4c1-4897-8f650e23d448@solarflare.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: Alexei Starovoitov , Daniel Borkmann , David Miller , netdev To: Y Song Return-path: Received: from dispatch1-us1.ppe-hosted.com ([67.231.154.164]:39684 "EHLO dispatch1-us1.ppe-hosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752061AbdIVNqt (ORCPT ); Fri, 22 Sep 2017 09:46:49 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On 22/09/17 00:11, Y Song wrote: > On Thu, Sep 21, 2017 at 12:58 PM, Edward Cree wrote: >> On 21/09/17 20:44, Alexei Starovoitov wrote: >>> On Thu, Sep 21, 2017 at 09:29:33PM +0200, Daniel Borkmann wrote: >>>> More intuitive, but agree on the from_be/le. Maybe we should >>>> just drop the "to_" prefix altogether, and leave the rest as is since >>>> it's not surrounded by braces, it's also not a cast but rather an op. >> That works for me. >>> 'be16 r4' is ambiguous regarding upper bits. >>> >>> what about my earlier suggestion: >>> r4 = (be16) (u16) r4 >>> r4 = (le64) (u64) r4 >>> >>> It will be pretty clear what instruction is doing (that upper bits become zero). >> Trouble with that is that's very *not* what C will do with those casts >> and it doesn't really capture the bidirectional/symmetry thing. The >> closest I could see with that is something like `r4 = (be16/u16) r4`, >> but that's quite an ugly mongrel. >> I think Daniel's idea of `be16`, `le32` etc one-arg opcodes is the >> cleanest and clearest. Should it be >> r4 = be16 r4 >> or just >> be16 r4 >> ? Personally I incline towards the latter, but admit it doesn't really >> match the syntax of other opcodes. > I did some quick prototyping in llvm to make sure we have a syntax > llvm is happy. Apparently, llvm does not like the syntax > r4 = be16 r4 or r4 = (be16) (u16) r4. > > In llvm:utils/TableGen/AsmMatcherEmitter.cpp: > > // Verify that any operand is only mentioned once. Wait, how do you deal with (totally legal) r4 += r4? Or r4 = *(r4 +0)? Even jumps can have src_reg == dst_reg, though it doesn't seem useful.