Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Arvind Sankar <nivedita@alum.mit.edu>
Date:   Tue, 18 Aug 2020 19:51:18 -0400
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     Arvind Sankar <nivedita@alum.mit.edu>,
        Eli Friedman <efriedma@quicinc.com>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Michal Marek <michal.lkml@markovi.net>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Tony Luck <tony.luck@intel.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Joe Perches <joe@perches.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Daniel Axtens <dja@axtens.net>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Alexandru Ardelean <alexandru.ardelean@analog.com>,
        Yury Norov <yury.norov@gmail.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        Daniel Kiper <daniel.kiper@oracle.com>,
        Bruce Ashfield <bruce.ashfield@gmail.com>,
        Marco Elver <elver@google.com>,
        Vamshi K Sthambamkadi <vamshi.k.sthambamkadi@gmail.com>,
        Andi Kleen <ak@suse.de>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        =?utf-8?B?RMOhdmlkIEJvbHZhbnNrw70=?= <david.bolvansky@gmail.com>
Subject: Re: [PATCH 0/4] -ffreestanding/-fno-builtin-* patches
Message-ID: <20200818235118.GA3380006@rani.riverdale.lan>
References: <20200817220212.338670-1-ndesaulniers@google.com>
 <20200818222542.GA3254379@rani.riverdale.lan>
 <CAKwvOdmfiD1TNqRvFuX07BqonYzh1eKFE9mFmOpaSyrbR0d5Lw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAKwvOdmfiD1TNqRvFuX07BqonYzh1eKFE9mFmOpaSyrbR0d5Lw@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Aug 18, 2020 at 03:59:45PM -0700, Nick Desaulniers wrote:
> On Tue, Aug 18, 2020 at 3:25 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > Another thing that needs to be fixed is that at least lib/string.c needs
> > to be compiled with -ffreestanding.
> >
> > gcc-10 optimizes the generic memset implementation in there into a call
> > to memset. Now that's on x86 which doesn't use the generic
> > implementation, but this is just waiting to bite us.
> >
> > https://godbolt.org/z/6EhG15
> 
> I'll let you send the patch for that this time.  (It's too bad godbolt
> doesn't have newer versions of GCC for cross compilation...cant test
> aarch64 gcc-10, for example.)  It would be interesting for sure to see
> resulting differences in disassembly observed in lib/string.o with
> -ffreestanding.

https://lore.kernel.org/lkml/20200818234307.3382306-1-nivedita@alum.mit.edu/

> 
> But, oof, that's not good.  Certainly impressive and powerful loop
> idiom recognition, but wouldn't you consider it a bug that this
> optimization should probably first check that it's not replacing part
> of a loop with a potentially recursive call to itself?

Difficult to check that in general, but I would have thought they'd at
least add a check for memset directly calling memset. It looks like they
considered that but then decided to go with -ffreestanding disabling the
optimization. Even gcc 4.x does it :)

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56888

> 
> Admittedly, we've had the same shenanigans with memcpy implemented in
> terms of calls to __builtin_memcpy being lowered to infinitely
> recursive calls...which feels like the same kind of bug.  ("You wanted
> infinite recursion in the kexec purgatory image, right?" "No,
> compiler, I did not.")  example: https://godbolt.org/z/MzrTaM
> (probably should fix this in both implementations; at the least I feel
> like Clang's -Winfinite-recursion should try to help us out here).
> 
> Feels almost like it may be difficult to provide an implementation of
> memset without stepping on a landmine.  One thing I'd be curious about
> is whether all of lib/string.c would need -ffreestanding, or if you
> could move just memset to its own TU then use -ffreestanding on that.
> A free standing environment must always provide a core set of
> functions like memset, memcpy, memcmp, memmove, according to
> https://gcc.gnu.org/onlinedocs/gcc/Standards.html.  Maybe those four
> should be in a separate TU compiled as -ffreestanding, so that they
> can never be lowered to calls to themselves (potentially infinitely
> recursive)?
> -- 
> Thanks,
> ~Nick Desaulniers

I think all of it should be freestanding. Since eg the compiler could
recognize strcpy and turn it into a call to strcpy.

It turns out that at least memcpy can also be recognized, but gcc only
does that if the arguments have the restrict qualifier, so the version
in the kernel doesn't get broken.