linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arvind Sankar <nivedita@alum.mit.edu>
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>,
	"Dávid Bolvanský" <david.bolvansky@gmail.com>
Subject: Re: [PATCH 0/4] -ffreestanding/-fno-builtin-* patches
Date: Tue, 18 Aug 2020 19:51:18 -0400	[thread overview]
Message-ID: <20200818235118.GA3380006@rani.riverdale.lan> (raw)
In-Reply-To: <CAKwvOdmfiD1TNqRvFuX07BqonYzh1eKFE9mFmOpaSyrbR0d5Lw@mail.gmail.com>

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.

  reply	other threads:[~2020-08-18 23:51 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-17 22:02 [PATCH 0/4] -ffreestanding/-fno-builtin-* patches Nick Desaulniers
2020-08-17 22:02 ` [PATCH 1/4] Makefile: add -fno-builtin-stpcpy Nick Desaulniers
2020-08-17 22:31   ` H. Peter Anvin
2020-08-17 23:36     ` Nick Desaulniers
2020-08-18 19:21     ` Kees Cook
2020-08-18  7:10   ` Ard Biesheuvel
2020-08-18  7:25     ` Greg KH
2020-08-18  7:29       ` Ard Biesheuvel
2020-08-18  7:34         ` Greg KH
2020-08-18 19:23   ` Kees Cook
2020-08-17 22:02 ` [PATCH 2/4] Revert "lib/string.c: implement a basic bcmp" Nick Desaulniers
2020-08-18  5:44   ` Nathan Chancellor
2020-08-18 18:00     ` Nick Desaulniers
2020-08-18 19:24       ` Kees Cook
2020-08-17 22:02 ` [PATCH 3/4] x86/boot: use -fno-builtin-bcmp Nick Desaulniers
2020-08-18 19:24   ` Kees Cook
2020-08-17 22:02 ` [PATCH 4/4] x86: don't build CONFIG_X86_32 as -ffreestanding Nick Desaulniers
2020-08-18 19:24   ` Kees Cook
2021-01-07  0:27   ` Fangrui Song
2020-08-17 22:44 ` [PATCH 0/4] -ffreestanding/-fno-builtin-* patches H. Peter Anvin
2020-08-18 17:56   ` Nick Desaulniers
2020-08-18 19:02     ` H. Peter Anvin
2020-08-18 19:13       ` Linus Torvalds
2020-08-18 19:25         ` Nick Desaulniers
2020-08-18 19:58           ` Nick Desaulniers
2020-08-19 12:19             ` Clement Courbet
2020-08-18 20:24         ` Arvind Sankar
2020-08-18 20:27           ` Nick Desaulniers
2020-08-18 20:58             ` Nick Desaulniers
2020-08-18 21:41               ` Arvind Sankar
2020-08-18 21:51                 ` Dávid Bolvanský
2020-08-18 21:59                 ` Nick Desaulniers
2020-08-18 22:05                   ` Dávid Bolvanský
2020-08-18 23:22                     ` Nick Desaulniers
2020-08-20 14:56                 ` Rasmus Villemoes
2020-08-20 17:56                   ` Arvind Sankar
2020-08-20 18:05                     ` Dávid Bolvanský
2020-08-20 23:33                     ` Linus Torvalds
2020-08-21 17:29                       ` Arvind Sankar
2020-08-21 17:54                         ` Linus Torvalds
2020-08-21 18:02                           ` Linus Torvalds
2020-08-21 19:14                             ` Arvind Sankar
2020-08-21 19:23                               ` Linus Torvalds
2020-08-21 19:57                           ` Arvind Sankar
2020-08-21 20:03                             ` Peter Zijlstra
2020-08-21 21:39                             ` Linus Torvalds
2020-08-22  0:12                               ` Nick Desaulniers
2020-08-22 12:20                                 ` David Laight
2020-08-21  6:45                     ` Rasmus Villemoes
2020-08-24 15:57                 ` Masahiro Yamada
2020-08-24 17:34                   ` Arvind Sankar
2020-08-25  7:10                     ` Nick Desaulniers
2020-08-25  7:31                       ` Nick Desaulniers
2020-08-25 12:28                       ` Masahiro Yamada
2020-08-25 14:02                         ` Nick Desaulniers
2020-08-26 13:28                           ` Masahiro Yamada
2020-08-18 21:53               ` David Laight
2020-08-20 22:41               ` H. Peter Anvin
2020-08-20 23:17                 ` Arvind Sankar
2020-08-18 19:35       ` Nick Desaulniers
2020-08-18 22:25 ` Arvind Sankar
2020-08-18 22:59   ` Nick Desaulniers
2020-08-18 23:51     ` Arvind Sankar [this message]
2020-08-19  0:20     ` Arvind Sankar
2020-08-19  8:26   ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200818235118.GA3380006@rani.riverdale.lan \
    --to=nivedita@alum.mit.edu \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alexandru.ardelean@analog.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=bruce.ashfield@gmail.com \
    --cc=daniel.kiper@oracle.com \
    --cc=david.bolvansky@gmail.com \
    --cc=dja@axtens.net \
    --cc=dvyukov@google.com \
    --cc=efriedma@quicinc.com \
    --cc=elver@google.com \
    --cc=hpa@zytor.com \
    --cc=joe@perches.com \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=ndesaulniers@google.com \
    --cc=paulmck@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vamshi.k.sthambamkadi@gmail.com \
    --cc=x86@kernel.org \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).