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

Alexei Starovoitov
2020-10-30T03:22:55Z

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.