Re: [PATCH v2 1/2] bpf: don't rely on GCC __attribute__((optimize)) to disable GCSE

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:BPF JIT for MIPS (32-BIT AND 64-BIT)" 
	<netdev@vger.kernel.org>,
	"open list:BPF JIT for MIPS (32-BIT AND 64-BIT)" 
	<bpf@vger.kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	Randy Dunlap <rdunlap@infradead.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [PATCH v2 1/2] bpf: don't rely on GCC __attribute__((optimize)) to disable GCSE
Date: Thu, 29 Oct 2020 20:22:47 -0700	[thread overview]
Message-ID: <20201030032247.twch6rvnk6ql3zjb@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAKwvOd=Zrza=i54_=H3n2HkmMhg9EJ3Wy0kR5AXTSqBowsQV5g@mail.gmail.com>

On Thu, Oct 29, 2020 at 05:28:11PM -0700, Nick Desaulniers wrote:
> 
> We already know that -fno-asynchronous-unwind-tables get dropped,
> hence this patch.  

On arm64 only. Not on x86

> And we know -fomit-frame-pointer or
> -fno-omit-frame-pointer I guess gets dropped, hence your ask.  

yep. that one is bugged.

> We might not know the full extent which other flags get dropped with the
> optimize attribute, but I'd argue that my list above can all result in
> pretty bad bugs when accidentally omitted (ok, maybe not -fshort-wchar
> or -fmacro-prefix-map, idk what those do) or when mixed with code that

true.
Few month back I've checked that strict-aliasing and no-common flags
from your list are not dropped by this attr in gcc [6789].
I've also checked that no-red-zone and model=kernel preserved as well.

> has different values those flags control.  Searching GCC's bug tracker
> for `__attribute__((optimize` turns up plenty of reports to make me
> think this attribute maybe doesn't work the way folks suspect or
> intend: https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__attribute__%28%28optimize&list_id=283390.

There is a risk.
Is it a footgun? Sure.
Yet. gcc testsuite is using __attribute__((optimize)).
And some of these tests were added _after_ offical gcc doc said that this
attribute is broken.
imo it's like 'beware of the dog' sign.

> There's plenty of folks arguing against the use of the optimize
> attribute in favor of the command line flag.  I urge you to please
> reconsider the request.

ok. Applied this first patch to bpf tree and will get it to Linus soon.
Second patch that is splitting interpreter out because of this mess
is dropped. The effect of gcse on performance is questionable.
iirc some interpreters used to do -fno-gcse to gain performance.