-ffreestanding typically inhibits "libcall optimizations" where calls to
certain library functions can be replaced by the compiler in certain
cases to calls to other library functions that may be more efficient.
This can be problematic for embedded targets that don't provide full
libc implementations.
-ffreestanding inhibits all such optimizations, which is the safe
choice, but generally we want the optimizations that are performed. The
Linux kernel does implement a fair amount of libc routines. Instead of
-ffreestanding (which makes more sense in smaller images like kexec's
purgatory image), prefer -fno-builtin-* flags to disable the compiler
from emitting calls to functions which may not be defined.
If you see a linkage failure due to a missing symbol that's typically
defined in a libc, and not explicitly called from the source code, then
the compiler may have done such a transform. You can either implement
such a function (ie. in lib/string.c) or disable the transform outright
via -fno-builtin-* flag (where * is the name of the library routine, ie.
-fno-builtin-bcmp).
Patch 1 unbreaks the build with ToT clang, which has been red all
weekend, by adding -fno-builtin-stpcpy.
Patch 2 is a revert but adds -fno-builtin-bcmp.
Patch 3 does the same for x86 purgatory.
Patch 4 removes -ffreestanding from i386.
The first patch makes sense for Kbuild, the second maybe akpm@, the
third and forth for x86. Not sure who should pick up the series (they
can be merged out of order, technically) but I really need the first
patch soon. The 3 latter patches are cleanups.
Nick Desaulniers (4):
Makefile: add -fno-builtin-stpcpy
Revert "lib/string.c: implement a basic bcmp"
x86/boot: use -fno-builtin-bcmp
x86: don't build CONFIG_X86_32 as -ffreestanding
Makefile | 7 +++++++
arch/x86/Makefile | 3 ---
arch/x86/boot/Makefile | 1 +
arch/x86/boot/string.c | 8 --------
include/linux/string.h | 3 ---
lib/string.c | 20 --------------------
6 files changed, 8 insertions(+), 34 deletions(-)
--
2.28.0.220.ged08abb693-goog
We're reverting
commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
in favor of -fno-builtin-bcmp. Remove the additional definition here,
too.
arch/x86/purgatory/Makefile uses -ffreestanding, so there's no risk of
this libcall optimization occurring for arch/x86/boot/purgatory.ro.
arch/x86/boot/Makefile resets KBUILD_CFLAGS, so make sure to reset this
flag that was set for the top level Makefile.
Fixes: 4ce97317f41d ("x86/purgatory: Do not use __builtin_memcpy and __builtin_memset")
Signed-off-by: Nick Desaulniers <[email protected]>
---
arch/x86/boot/Makefile | 1 +
arch/x86/boot/string.c | 8 --------
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index fe605205b4ce..ef7f15bfceab 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -70,6 +70,7 @@ KBUILD_CFLAGS := $(REALMODE_CFLAGS) -D_SETUP
KBUILD_AFLAGS := $(KBUILD_CFLAGS) -D__ASSEMBLY__
KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
+KBUILD_CFLAGS += -fno-builtin-bcmp
GCOV_PROFILE := n
UBSAN_SANITIZE := n
diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index 8a3fff9128bb..23d91aa7691e 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -37,14 +37,6 @@ int memcmp(const void *s1, const void *s2, size_t len)
return diff;
}
-/*
- * Clang may lower `memcmp == 0` to `bcmp == 0`.
- */
-int bcmp(const void *s1, const void *s2, size_t len)
-{
- return memcmp(s1, s2, len);
-}
-
int strcmp(const char *str1, const char *str2)
{
const unsigned char *s1 = (const unsigned char *)str1;
--
2.28.0.220.ged08abb693-goog
LLVM implemented a recent "libcall optimization" that lowers calls to
`sprintf(dest, "%s", str)` where the return value is used to
`stpcpy(dest, str) - dest`. This generally avoids the machinery involved
in parsing format strings. This optimization was introduced into
clang-12. Because the kernel does not provide an implementation of
stpcpy, we observe linkage failures for almost all targets when building
with ToT clang.
The interface is unsafe as it does not perform any bounds checking.
Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
Unlike
commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
which cited failures with `-fno-builtin-*` flags being retained in LLVM
LTO, that bug seems to have been fixed by
https://reviews.llvm.org/D71193, so the above sha can now be reverted in
favor of `-fno-builtin-bcmp`.
Cc: [email protected] # 4.4
Link: https://bugs.llvm.org/show_bug.cgi?id=47162
Link: https://github.com/ClangBuiltLinux/linux/issues/1126
Link: https://reviews.llvm.org/D85963
Reported-by: Sami Tolvanen <[email protected]>
Suggested-by: Dávid Bolvanský <[email protected]>
Suggested-by: Kees Cook <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
Makefile | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Makefile b/Makefile
index 9cac6fde3479..211a1b6f6478 100644
--- a/Makefile
+++ b/Makefile
@@ -959,6 +959,12 @@ ifdef CONFIG_RETPOLINE
KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
endif
+# The compiler may "libcall optimize" certain function calls into the below
+# functions, for architectures that don't use -ffreestanding. If we don't plan
+# to provide implementations of these routines, then prevent the compiler from
+# emitting calls to what will be undefined symbols.
+KBUILD_CFLAGS += -fno-builtin-stpcpy
+
# include additional Makefiles when needed
include-y := scripts/Makefile.extrawarn
include-$(CONFIG_KASAN) += scripts/Makefile.kasan
--
2.28.0.220.ged08abb693-goog
On 2020-08-17 15:02, Nick Desaulniers wrote:
> LLVM implemented a recent "libcall optimization" that lowers calls to
> `sprintf(dest, "%s", str)` where the return value is used to
> `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> in parsing format strings. This optimization was introduced into
> clang-12. Because the kernel does not provide an implementation of
> stpcpy, we observe linkage failures for almost all targets when building
> with ToT clang.
>
> The interface is unsafe as it does not perform any bounds checking.
> Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
>
> Unlike
> commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> which cited failures with `-fno-builtin-*` flags being retained in LLVM
> LTO, that bug seems to have been fixed by
> https://reviews.llvm.org/D71193, so the above sha can now be reverted in
> favor of `-fno-builtin-bcmp`.
>
stpcpy() and (to a lesser degree) mempcpy() are fairly useful routines
in general. Perhaps we *should* provide them?
-hpa
On 2020-08-17 15:02, Nick Desaulniers wrote:
> -ffreestanding typically inhibits "libcall optimizations" where calls to
> certain library functions can be replaced by the compiler in certain
> cases to calls to other library functions that may be more efficient.
> This can be problematic for embedded targets that don't provide full
> libc implementations.
>
> -ffreestanding inhibits all such optimizations, which is the safe
> choice, but generally we want the optimizations that are performed. The
> Linux kernel does implement a fair amount of libc routines. Instead of
> -ffreestanding (which makes more sense in smaller images like kexec's
> purgatory image), prefer -fno-builtin-* flags to disable the compiler
> from emitting calls to functions which may not be defined.
>
> If you see a linkage failure due to a missing symbol that's typically
> defined in a libc, and not explicitly called from the source code, then
> the compiler may have done such a transform. You can either implement
> such a function (ie. in lib/string.c) or disable the transform outright
> via -fno-builtin-* flag (where * is the name of the library routine, ie.
> -fno-builtin-bcmp).
>
This is arguably exactly the wrong way around.
The way this *should* be done is by opt-in, not opt-out, which by almost
definition ends up being a game of whack-a-mole, like in this case
stpcpy(). Furthermore, it is unlikely that people will remember what
options to flip when and if stpcpy() happens to be implemented in the
kernel.
The problem here is twofold:
1. The user would be expected to know what kind of the optimizations the
compiler can do on what function, which is private knowledge to the
compiler.
2. The only way to override -fno-builtin is by a header file with macros
overriding the function names with __builtin, but that doesn't tell the
compiler proper anything about the execution environment.
So the Right Thing is for the compiler authors to change the way
-ffreestanding works. -ffreestanding means, by definition, that there
are no library calls (other than libgcc or whatever else is supplied
with the compiler) that the compiler can call. That is currently an
all-or-nothing choice, or at least one choice per C standard implemented.
Instead, a compile job with -ffreestanding should be able to provide a
list of standard C functions that the compiler may call, and thus the
compiler actually can do the right thing depending on which exact
functions it would consider calling. This list is probably most easily
supplied in the form of a header file with #pragma directives.
-hpa
On Mon, Aug 17, 2020 at 3:31 PM H. Peter Anvin <[email protected]> wrote:
>
> On 2020-08-17 15:02, Nick Desaulniers wrote:
> > LLVM implemented a recent "libcall optimization" that lowers calls to
> > `sprintf(dest, "%s", str)` where the return value is used to
> > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> > in parsing format strings. This optimization was introduced into
> > clang-12. Because the kernel does not provide an implementation of
> > stpcpy, we observe linkage failures for almost all targets when building
> > with ToT clang.
> >
> > The interface is unsafe as it does not perform any bounds checking.
> > Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
> >
> > Unlike
> > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> > which cited failures with `-fno-builtin-*` flags being retained in LLVM
> > LTO, that bug seems to have been fixed by
> > https://reviews.llvm.org/D71193, so the above sha can now be reverted in
> > favor of `-fno-builtin-bcmp`.
> >
>
> stpcpy() and (to a lesser degree) mempcpy() are fairly useful routines
> in general. Perhaps we *should* provide them?
Sorry, I forgot to provide context of the previous thread, which is
worth a read. To answer this question specifically (or at least for
stpcpy), the answer from the previous thread was (via Kees): "No;
please no more unbounded string.h routines":
https://lore.kernel.org/lkml/202008150921.B70721A359@keescook/
--
Thanks,
~Nick Desaulniers
On Tue, 18 Aug 2020 at 00:02, Nick Desaulniers <[email protected]> wrote:
>
> LLVM implemented a recent "libcall optimization" that lowers calls to
> `sprintf(dest, "%s", str)` where the return value is used to
> `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> in parsing format strings. This optimization was introduced into
> clang-12. Because the kernel does not provide an implementation of
> stpcpy, we observe linkage failures for almost all targets when building
> with ToT clang.
>
> The interface is unsafe as it does not perform any bounds checking.
> Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
>
> Unlike
> commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> which cited failures with `-fno-builtin-*` flags being retained in LLVM
> LTO, that bug seems to have been fixed by
> https://reviews.llvm.org/D71193, so the above sha can now be reverted in
> favor of `-fno-builtin-bcmp`.
>
> Cc: [email protected] # 4.4
Why does a fix for Clang-12 have to be backported all the way to v4.4?
How does that meet the requirements for stable patches?
> Link: https://bugs.llvm.org/show_bug.cgi?id=47162
> Link: https://github.com/ClangBuiltLinux/linux/issues/1126
> Link: https://reviews.llvm.org/D85963
> Reported-by: Sami Tolvanen <[email protected]>
> Suggested-by: Dávid Bolvanský <[email protected]>
> Suggested-by: Kees Cook <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> Makefile | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 9cac6fde3479..211a1b6f6478 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -959,6 +959,12 @@ ifdef CONFIG_RETPOLINE
> KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
> endif
>
> +# The compiler may "libcall optimize" certain function calls into the below
> +# functions, for architectures that don't use -ffreestanding. If we don't plan
> +# to provide implementations of these routines, then prevent the compiler from
> +# emitting calls to what will be undefined symbols.
> +KBUILD_CFLAGS += -fno-builtin-stpcpy
> +
> # include additional Makefiles when needed
> include-y := scripts/Makefile.extrawarn
> include-$(CONFIG_KASAN) += scripts/Makefile.kasan
> --
> 2.28.0.220.ged08abb693-goog
>
On Tue, Aug 18, 2020 at 09:10:01AM +0200, Ard Biesheuvel wrote:
> On Tue, 18 Aug 2020 at 00:02, Nick Desaulniers <[email protected]> wrote:
> >
> > LLVM implemented a recent "libcall optimization" that lowers calls to
> > `sprintf(dest, "%s", str)` where the return value is used to
> > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> > in parsing format strings. This optimization was introduced into
> > clang-12. Because the kernel does not provide an implementation of
> > stpcpy, we observe linkage failures for almost all targets when building
> > with ToT clang.
> >
> > The interface is unsafe as it does not perform any bounds checking.
> > Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
> >
> > Unlike
> > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> > which cited failures with `-fno-builtin-*` flags being retained in LLVM
> > LTO, that bug seems to have been fixed by
> > https://reviews.llvm.org/D71193, so the above sha can now be reverted in
> > favor of `-fno-builtin-bcmp`.
> >
> > Cc: [email protected] # 4.4
>
> Why does a fix for Clang-12 have to be backported all the way to v4.4?
> How does that meet the requirements for stable patches?
Because people like to build older kernels with new compliler versions.
And those "people" include me, who doesn't want to keep around old
compilers just because my distro moved to the latest one...
We've been doing this for the past 4+ years, for new versions of gcc,
keeping 4.4.y building properly with the bleeding edge of that compiler,
why is clang any different here?
thanks,
greg k-h
On Tue, 18 Aug 2020 at 09:25, Greg KH <[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 09:10:01AM +0200, Ard Biesheuvel wrote:
> > On Tue, 18 Aug 2020 at 00:02, Nick Desaulniers <[email protected]> wrote:
> > >
> > > LLVM implemented a recent "libcall optimization" that lowers calls to
> > > `sprintf(dest, "%s", str)` where the return value is used to
> > > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> > > in parsing format strings. This optimization was introduced into
> > > clang-12. Because the kernel does not provide an implementation of
> > > stpcpy, we observe linkage failures for almost all targets when building
> > > with ToT clang.
> > >
> > > The interface is unsafe as it does not perform any bounds checking.
> > > Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
> > >
> > > Unlike
> > > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> > > which cited failures with `-fno-builtin-*` flags being retained in LLVM
> > > LTO, that bug seems to have been fixed by
> > > https://reviews.llvm.org/D71193, so the above sha can now be reverted in
> > > favor of `-fno-builtin-bcmp`.
> > >
> > > Cc: [email protected] # 4.4
> >
> > Why does a fix for Clang-12 have to be backported all the way to v4.4?
> > How does that meet the requirements for stable patches?
>
> Because people like to build older kernels with new compliler versions.
>
> And those "people" include me, who doesn't want to keep around old
> compilers just because my distro moved to the latest one...
>
> We've been doing this for the past 4+ years, for new versions of gcc,
> keeping 4.4.y building properly with the bleeding edge of that compiler,
> why is clang any different here?
>
Fair enough. I am just struggling to match stable-kernel-rules.rst
with the actual practices - perhaps it is time to update that
document?
On Tue, Aug 18, 2020 at 09:29:39AM +0200, Ard Biesheuvel wrote:
> On Tue, 18 Aug 2020 at 09:25, Greg KH <[email protected]> wrote:
> >
> > On Tue, Aug 18, 2020 at 09:10:01AM +0200, Ard Biesheuvel wrote:
> > > On Tue, 18 Aug 2020 at 00:02, Nick Desaulniers <[email protected]> wrote:
> > > >
> > > > LLVM implemented a recent "libcall optimization" that lowers calls to
> > > > `sprintf(dest, "%s", str)` where the return value is used to
> > > > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> > > > in parsing format strings. This optimization was introduced into
> > > > clang-12. Because the kernel does not provide an implementation of
> > > > stpcpy, we observe linkage failures for almost all targets when building
> > > > with ToT clang.
> > > >
> > > > The interface is unsafe as it does not perform any bounds checking.
> > > > Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
> > > >
> > > > Unlike
> > > > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> > > > which cited failures with `-fno-builtin-*` flags being retained in LLVM
> > > > LTO, that bug seems to have been fixed by
> > > > https://reviews.llvm.org/D71193, so the above sha can now be reverted in
> > > > favor of `-fno-builtin-bcmp`.
> > > >
> > > > Cc: [email protected] # 4.4
> > >
> > > Why does a fix for Clang-12 have to be backported all the way to v4.4?
> > > How does that meet the requirements for stable patches?
> >
> > Because people like to build older kernels with new compliler versions.
> >
> > And those "people" include me, who doesn't want to keep around old
> > compilers just because my distro moved to the latest one...
> >
> > We've been doing this for the past 4+ years, for new versions of gcc,
> > keeping 4.4.y building properly with the bleeding edge of that compiler,
> > why is clang any different here?
> >
>
> Fair enough. I am just struggling to match stable-kernel-rules.rst
> with the actual practices - perhaps it is time to update that
> document?
The rules are tiny and simple for 99% of the issues involved. Stuff
like "add patches to fix build failures and warnings for newer compiler
versions" are so rare (they only happen every 2 years or so), it's not
worth it.
thanks,
greg k-h
On Mon, Aug 17, 2020 at 3:44 PM H. Peter Anvin <[email protected]> wrote:
>
> On 2020-08-17 15:02, Nick Desaulniers wrote:
> > -ffreestanding typically inhibits "libcall optimizations" where calls to
> > certain library functions can be replaced by the compiler in certain
> > cases to calls to other library functions that may be more efficient.
> > This can be problematic for embedded targets that don't provide full
> > libc implementations.
> >
> > -ffreestanding inhibits all such optimizations, which is the safe
> > choice, but generally we want the optimizations that are performed. The
> > Linux kernel does implement a fair amount of libc routines. Instead of
> > -ffreestanding (which makes more sense in smaller images like kexec's
> > purgatory image), prefer -fno-builtin-* flags to disable the compiler
> > from emitting calls to functions which may not be defined.
> >
> > If you see a linkage failure due to a missing symbol that's typically
> > defined in a libc, and not explicitly called from the source code, then
> > the compiler may have done such a transform. You can either implement
> > such a function (ie. in lib/string.c) or disable the transform outright
> > via -fno-builtin-* flag (where * is the name of the library routine, ie.
> > -fno-builtin-bcmp).
> >
>
> This is arguably exactly the wrong way around.
>
> The way this *should* be done is by opt-in, not opt-out, which by almost
> definition ends up being a game of whack-a-mole, like in this case
> stpcpy(). Furthermore, it is unlikely that people will remember what
> options to flip when and if stpcpy() happens to be implemented in the
> kernel.
>
> The problem here is twofold:
>
> 1. The user would be expected to know what kind of the optimizations the
> compiler can do on what function, which is private knowledge to the
> compiler.
>
> 2. The only way to override -fno-builtin is by a header file with macros
> overriding the function names with __builtin, but that doesn't tell the
> compiler proper anything about the execution environment.
>
> So the Right Thing is for the compiler authors to change the way
> -ffreestanding works.
Sir, this is an Arby's
There are things all across the compilation landscape that make we
want to pontificate or even throw a tantrum in an Arby's. Would I?
Well, no, I'm just trying to flip burgers or shovel the elephant
sh...or w/e they do at Arby's (I've never actually been; I detest
roast beef).
Would it be interesting to have a way of opting in, as you describe,
such that your compiler knew exactly what kind of embedded environment
it was targeting? Maybe, but I'd argue that opting out is just the
other side of the same coin. Heads, I win; tails, you lose. That the
opt in or opt out list is shorter for a given project is not
particularly interesting. Should we change the semantics of a fairly
commonly used compiler flag that multiple toolchains are in agreement
of, then fix all of the breakage in all of the code that relied on
those semantics? I'm afraid that ship may have already
sailed...probably 20 or 30 years ago.
> -ffreestanding means, by definition, that there
> are no library calls (other than libgcc or whatever else is supplied
> with the compiler) that the compiler can call. That is currently an
> all-or-nothing choice, or at least one choice per C standard implemented.
Yes?
>
> Instead, a compile job with -ffreestanding should be able to provide a
> list of standard C functions that the compiler may call, and thus the
> compiler actually can do the right thing depending on which exact
> functions it would consider calling. This list is probably most easily
> supplied in the form of a header file with #pragma directives.
>
> -hpa
>
>
Here we have a one line patch for keeping the build green. If there's
some compiler feature you'd like implemented, let's sit down sometime
and work out the details. I'll even buy you a beer. But right now,
sir, the Arby's is on fire. Please take your soapbox outside.
--
Thanks,
~Nick Desaulniers
On 2020-08-18 10:56, Nick Desaulniers wrote:
>>
>> The problem here is twofold:
>>
>> 1. The user would be expected to know what kind of the optimizations the
>> compiler can do on what function, which is private knowledge to the
>> compiler.
>>
>> 2. The only way to override -fno-builtin is by a header file with macros
>> overriding the function names with __builtin, but that doesn't tell the
>> compiler proper anything about the execution environment.
>>
>> So the Right Thing is for the compiler authors to change the way
>> -ffreestanding works.
>
> Sir, this is an Arby's
>
> There are things all across the compilation landscape that make we
> want to pontificate or even throw a tantrum in an Arby's. Would I?
> Well, no, I'm just trying to flip burgers or shovel the elephant
> sh...or w/e they do at Arby's (I've never actually been; I detest
> roast beef).
>
> Would it be interesting to have a way of opting in, as you describe,
> such that your compiler knew exactly what kind of embedded environment
> it was targeting? Maybe, but I'd argue that opting out is just the
> other side of the same coin. Heads, I win; tails, you lose. That the
> opt in or opt out list is shorter for a given project is not
> particularly interesting. Should we change the semantics of a fairly
> commonly used compiler flag that multiple toolchains are in agreement
> of, then fix all of the breakage in all of the code that relied on
> those semantics? I'm afraid that ship may have already
> sailed...probably 20 or 30 years ago.
>
>> -ffreestanding means, by definition, that there
>> are no library calls (other than libgcc or whatever else is supplied
>> with the compiler) that the compiler can call. That is currently an
>> all-or-nothing choice, or at least one choice per C standard implemented.
>
> Yes?
>
I'm not saying "change the semantics", nor am I saying that playing
whack-a-mole *for a limited time* is unreasonable. But I would like to go back
to the compiler authors and get them to implement such a #pragma: "this
freestanding implementation *does* support *this specific library function*,
and you are free to call it." The only way we can get what we really need from
the compilers is by speaking up and requesting it, and we have done so very
successfully recently; further back we tended to get a lot of
language-lawyering, but these days both the gcc and the clang teams have been
wonderfully responsive.
-hpa
On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
>
> I'm not saying "change the semantics", nor am I saying that playing
> whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> to the compiler authors and get them to implement such a #pragma: "this
> freestanding implementation *does* support *this specific library function*,
> and you are free to call it."
I'd much rather just see the library functions as builtins that always
do the right thing (with the fallback being "just call the standard
function").
IOW, there's nothing wrong with -ffreestanding if you then also have
__builtin_memcpy() etc, and they do the sane compiler optimizations
for memcpy().
What we want to avoid is the compiler making *assumptions* based on
standard names, because we may implement some of those things
differently.
And honestly, a compiler that uses 'bcmp' is just broken. WTH? It's
the year 2020, we don't use bcmp. It's that simple. Fix your damn
broken compiler and use memcmp. The argument that memcmp is more
expensive than bcmp is garbage legacy thinking from four decades ago.
It's likely the other way around, where people have actually spent
time on memcmp, but not on bcmp.
If somebody really *wants* to use bcmp, give them the "Get off my
lawn" flag, and leave them alone. But never ever should "use bcmp" be
any kind of default behavior. That's some batshit crazy stuff.
Linus
On Mon, Aug 17, 2020 at 03:31:26PM -0700, H. Peter Anvin wrote:
> On 2020-08-17 15:02, Nick Desaulniers wrote:
> > LLVM implemented a recent "libcall optimization" that lowers calls to
> > `sprintf(dest, "%s", str)` where the return value is used to
> > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> > in parsing format strings. This optimization was introduced into
> > clang-12. Because the kernel does not provide an implementation of
> > stpcpy, we observe linkage failures for almost all targets when building
> > with ToT clang.
> >
> > The interface is unsafe as it does not perform any bounds checking.
> > Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
> >
> > Unlike
> > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> > which cited failures with `-fno-builtin-*` flags being retained in LLVM
> > LTO, that bug seems to have been fixed by
> > https://reviews.llvm.org/D71193, so the above sha can now be reverted in
> > favor of `-fno-builtin-bcmp`.
> >
>
> stpcpy() and (to a lesser degree) mempcpy() are fairly useful routines
> in general. Perhaps we *should* provide them?
As Nick mentioned, I really don't want to expand the already bad
interfaces from libc. We have enough messes to clean up already, and I
don't want to add more. The kernel already uses a subset of C, we have
(several) separate non-libc memory allocators, we're using strscpy() and
scnprintf() widely in favor of their buggy libc counterparts, etc. We
don't need to match the libc string interfaces especially when they're
arguably bug-prone foot-guns. :)
--
Kees Cook
On Mon, Aug 17, 2020 at 03:02:11PM -0700, Nick Desaulniers wrote:
> We're reverting
> commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> in favor of -fno-builtin-bcmp. Remove the additional definition here,
> too.
>
> arch/x86/purgatory/Makefile uses -ffreestanding, so there's no risk of
> this libcall optimization occurring for arch/x86/boot/purgatory.ro.
>
> arch/x86/boot/Makefile resets KBUILD_CFLAGS, so make sure to reset this
> flag that was set for the top level Makefile.
>
> Fixes: 4ce97317f41d ("x86/purgatory: Do not use __builtin_memcpy and __builtin_memset")
> Signed-off-by: Nick Desaulniers <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Mon, Aug 17, 2020 at 03:02:09PM -0700, Nick Desaulniers wrote:
> LLVM implemented a recent "libcall optimization" that lowers calls to
> `sprintf(dest, "%s", str)` where the return value is used to
> `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> in parsing format strings. This optimization was introduced into
> clang-12. Because the kernel does not provide an implementation of
> stpcpy, we observe linkage failures for almost all targets when building
> with ToT clang.
>
> The interface is unsafe as it does not perform any bounds checking.
> Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
>
> Unlike
> commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
> which cited failures with `-fno-builtin-*` flags being retained in LLVM
> LTO, that bug seems to have been fixed by
> https://reviews.llvm.org/D71193, so the above sha can now be reverted in
> favor of `-fno-builtin-bcmp`.
>
> Cc: [email protected] # 4.4
> Link: https://bugs.llvm.org/show_bug.cgi?id=47162
> Link: https://github.com/ClangBuiltLinux/linux/issues/1126
> Link: https://reviews.llvm.org/D85963
> Reported-by: Sami Tolvanen <[email protected]>
> Suggested-by: D?vid Bolvansk? <[email protected]>
> Suggested-by: Kees Cook <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Tue, Aug 18, 2020 at 12:19 PM Linus Torvalds
<[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> >
> > I'm not saying "change the semantics", nor am I saying that playing
> > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > to the compiler authors and get them to implement such a #pragma: "this
> > freestanding implementation *does* support *this specific library function*,
> > and you are free to call it."
>
> I'd much rather just see the library functions as builtins that always
> do the right thing (with the fallback being "just call the standard
> function").
>
> IOW, there's nothing wrong with -ffreestanding if you then also have
> __builtin_memcpy() etc, and they do the sane compiler optimizations
> for memcpy().
>
> What we want to avoid is the compiler making *assumptions* based on
> standard names, because we may implement some of those things
> differently.
>
> And honestly, a compiler that uses 'bcmp' is just broken. WTH? It's
> the year 2020, we don't use bcmp. It's that simple. Fix your damn
> broken compiler and use memcmp. The argument that memcmp is more
> expensive than bcmp is garbage legacy thinking from four decades ago.
>
> It's likely the other way around, where people have actually spent
> time on memcmp, but not on bcmp.
>
> If somebody really *wants* to use bcmp, give them the "Get off my
> lawn" flag, and leave them alone. But never ever should "use bcmp" be
> any kind of default behavior. That's some batshit crazy stuff.
>
> Linus
You'll have to ask Clement about that. I'm not sure I ever saw the
"faster bcmp than memcmp" implementation, but I was told "it exists"
when I asked for a revert when all of our kernel builds went red.
--
Thanks,
~Nick Desaulniers
On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
>
> I'm not saying "change the semantics", nor am I saying that playing
> whack-a-mole *for a limited time* is unreasonable.
Ah, ok then. Sorry I mischaracterized your position.
> But I would like to go back
> to the compiler authors and get them to implement such a #pragma: "this
> freestanding implementation *does* support *this specific library function*,
> and you are free to call it."
I think the first thing that would be helpful is some more detailed
write up of the problem statement, and analysis of why the current
provided tools are close but not enough. Maybe filing a Clang bug
would be helpful to get more feedback from additional toolchain folks
than just me (https://bugs.llvm.org/enter_bug.cgi?product=clang "new
bug" component). Alternatively, if you're planning on attending
plumbers next week, I plan to propose a "kernel toolchain" mailing
list for folks with whatever background to discuss future GNU C
extensions and how they might be used in kernel development. That
might be more appropriate than a Clang bug, but it doesn't exist yet,
and feedback might be that it's a terrible idea for some reason.
> The only way we can get what we really need from
> the compilers is by speaking up and requesting it, and we have done so very
> successfully recently; further back we tended to get a lot of
> language-lawyering, but these days both the gcc and the clang teams have been
> wonderfully responsive.
Just trying to avoid the shoe, again. I'd really like to get to the
point where we can untangle putting out fires from implementing
feature requests though.
--
Thanks,
~Nick Desaulniers
On Tue, Aug 18, 2020 at 12:25 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 12:19 PM Linus Torvalds
> <[email protected]> wrote:
> >
> > On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> > >
> > > I'm not saying "change the semantics", nor am I saying that playing
> > > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > > to the compiler authors and get them to implement such a #pragma: "this
> > > freestanding implementation *does* support *this specific library function*,
> > > and you are free to call it."
> >
> > I'd much rather just see the library functions as builtins that always
> > do the right thing (with the fallback being "just call the standard
> > function").
> >
> > IOW, there's nothing wrong with -ffreestanding if you then also have
> > __builtin_memcpy() etc, and they do the sane compiler optimizations
> > for memcpy().
> >
> > What we want to avoid is the compiler making *assumptions* based on
> > standard names, because we may implement some of those things
> > differently.
That's asking for trouble; please don't implement routines with
identifiers from libc but with differing function signatures, and then
proceed to *not* use -ffreestanding. You can't have it both ways
(optimizations from *not* using -ffreestanding, then breaking all
kinds of assumptions based on conventions used across userspace), at
least not with the tools you currently have.
> >
> > And honestly, a compiler that uses 'bcmp' is just broken. WTH? It's
> > the year 2020, we don't use bcmp. It's that simple. Fix your damn
> > broken compiler and use memcmp. The argument that memcmp is more
> > expensive than bcmp is garbage legacy thinking from four decades ago.
> >
> > It's likely the other way around, where people have actually spent
> > time on memcmp, but not on bcmp.
> >
> > If somebody really *wants* to use bcmp, give them the "Get off my
> > lawn" flag,
I wrote a paper in college on the philosophy and symbolism in "Gran
Torino." Would recommend (the movie, not the paper).
> > and leave them alone. But never ever should "use bcmp" be
> > any kind of default behavior. That's some batshit crazy stuff.
> >
> > Linus
>
> You'll have to ask Clement about that. I'm not sure I ever saw the
> "faster bcmp than memcmp" implementation, but I was told "it exists"
> when I asked for a revert when all of our kernel builds went red.
Also, to Clement's credit, every patch I've ever seen from Clement is
backed up by data; typically fleetwide profiles at Google. "we spend
a lot of time in memcmp, particularly comparing the result against
zero and no other value; hmm...how do we spend less time in
memcmp...oh, well there's another library function with slightly
different semantics we can call instead." I don't think anyone would
consider the optimization batshit crazy given the number of cycles
saved across the fleet. That an embedded project didn't provide an
implementation, is a footnote that can be fixed in the embedded
project, either by using -ffreestanding or -fno-builtin-bcmp, which is
what this series proposes to do.
--
Thanks,
~Nick Desaulniers
On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
> On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> >
> > I'm not saying "change the semantics", nor am I saying that playing
> > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > to the compiler authors and get them to implement such a #pragma: "this
> > freestanding implementation *does* support *this specific library function*,
> > and you are free to call it."
>
> I'd much rather just see the library functions as builtins that always
> do the right thing (with the fallback being "just call the standard
> function").
>
> IOW, there's nothing wrong with -ffreestanding if you then also have
> __builtin_memcpy() etc, and they do the sane compiler optimizations
> for memcpy().
>
> What we want to avoid is the compiler making *assumptions* based on
> standard names, because we may implement some of those things
> differently.
>
-ffreestanding as it stands today does have __builtin_memcpy and
friends. But you need to then use #define memcpy __builtin_memcpy etc,
which is messy and also doesn't fully express what you want. #pragma, or
even just allowing -fbuiltin-foo options would be useful.
The two compilers have some peculiarities, which means you really can't
have functions with the same name that do something else if you want to
use builtins at all, and can also lead to missed optimizations.
For eg, __builtin_strchr(s,'\0') can be optimized to strlen. gcc will
optimize it that way even if -ffreestanding is used (so strlen has to
mean strlen), while clang won't, so it misses a potential optimization.
This is admittedly a silly example, but you could imagine something like
strncpy being optimized to memcpy+memset if the source length was
previously computed.
PS: clang optimizes sprintf, but doesn't provide __builtin_sprintf?
On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
> > On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> > >
> > > I'm not saying "change the semantics", nor am I saying that playing
> > > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > > to the compiler authors and get them to implement such a #pragma: "this
> > > freestanding implementation *does* support *this specific library function*,
> > > and you are free to call it."
> >
> > I'd much rather just see the library functions as builtins that always
> > do the right thing (with the fallback being "just call the standard
> > function").
> >
> > IOW, there's nothing wrong with -ffreestanding if you then also have
> > __builtin_memcpy() etc, and they do the sane compiler optimizations
> > for memcpy().
> >
> > What we want to avoid is the compiler making *assumptions* based on
> > standard names, because we may implement some of those things
> > differently.
> >
>
> -ffreestanding as it stands today does have __builtin_memcpy and
> friends. But you need to then use #define memcpy __builtin_memcpy etc,
> which is messy and also doesn't fully express what you want. #pragma, or
> even just allowing -fbuiltin-foo options would be useful.
>
> The two compilers have some peculiarities, which means you really can't
> have functions with the same name that do something else if you want to
> use builtins at all, and can also lead to missed optimizations.
>
> For eg, __builtin_strchr(s,'\0') can be optimized to strlen. gcc will
> optimize it that way even if -ffreestanding is used (so strlen has to
> mean strlen), while clang won't, so it misses a potential optimization.
> This is admittedly a silly example, but you could imagine something like
> strncpy being optimized to memcpy+memset if the source length was
> previously computed.
>
> PS: clang optimizes sprintf, but doesn't provide __builtin_sprintf?
https://bugs.llvm.org/show_bug.cgi?id=47224
--
Thanks,
~Nick Desaulniers
On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
> >
> > On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
> > > On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> > > >
> > > > I'm not saying "change the semantics", nor am I saying that playing
> > > > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > > > to the compiler authors and get them to implement such a #pragma: "this
> > > > freestanding implementation *does* support *this specific library function*,
> > > > and you are free to call it."
> > >
> > > I'd much rather just see the library functions as builtins that always
> > > do the right thing (with the fallback being "just call the standard
> > > function").
> > >
> > > IOW, there's nothing wrong with -ffreestanding if you then also have
> > > __builtin_memcpy() etc, and they do the sane compiler optimizations
> > > for memcpy().
> > >
> > > What we want to avoid is the compiler making *assumptions* based on
> > > standard names, because we may implement some of those things
> > > differently.
> > >
> >
> > -ffreestanding as it stands today does have __builtin_memcpy and
> > friends. But you need to then use #define memcpy __builtin_memcpy etc,
> > which is messy and also doesn't fully express what you want. #pragma, or
> > even just allowing -fbuiltin-foo options would be useful.
I do really like the idea of -fbuiltin-foo. For example, you'd specify:
-ffreestanding -fbuiltin-bcmp
as an example. `-ffreestanding` would opt you out of ALL libcall
optimizations, `-fbuiltin-bcmp` would then opt you back in to
transforms that produce bcmp. That way you're informing the compiler
more precisely about the environment you'd be targeting. It feels
symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
easy when there is such symmetry). And it's already convention that
if you specify multiple conflicting compiler flags, then the latter
one specified "wins." In that sense, turning back on specific
libcalls after disabling the rest looks more ergonomic to me.
Maybe Eli or David have thoughts on why that may or may not be as
ergonomic or possible to implement as I imagine?
> >
> > The two compilers have some peculiarities, which means you really can't
> > have functions with the same name that do something else if you want to
> > use builtins at all, and can also lead to missed optimizations.
> >
> > For eg, __builtin_strchr(s,'\0') can be optimized to strlen. gcc will
> > optimize it that way even if -ffreestanding is used (so strlen has to
> > mean strlen), while clang won't, so it misses a potential optimization.
> > This is admittedly a silly example, but you could imagine something like
> > strncpy being optimized to memcpy+memset if the source length was
> > previously computed.
> >
> > PS: clang optimizes sprintf, but doesn't provide __builtin_sprintf?
>
> https://bugs.llvm.org/show_bug.cgi?id=47224
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Tue, Aug 18, 2020 at 01:58:51PM -0700, Nick Desaulniers wrote:
> On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
> <[email protected]> wrote:
> >
> > On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
> > >
> > > On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
> > > > On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> > > > >
> > > > > I'm not saying "change the semantics", nor am I saying that playing
> > > > > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > > > > to the compiler authors and get them to implement such a #pragma: "this
> > > > > freestanding implementation *does* support *this specific library function*,
> > > > > and you are free to call it."
> > > >
> > > > I'd much rather just see the library functions as builtins that always
> > > > do the right thing (with the fallback being "just call the standard
> > > > function").
> > > >
> > > > IOW, there's nothing wrong with -ffreestanding if you then also have
> > > > __builtin_memcpy() etc, and they do the sane compiler optimizations
> > > > for memcpy().
> > > >
> > > > What we want to avoid is the compiler making *assumptions* based on
> > > > standard names, because we may implement some of those things
> > > > differently.
> > > >
> > >
> > > -ffreestanding as it stands today does have __builtin_memcpy and
> > > friends. But you need to then use #define memcpy __builtin_memcpy etc,
> > > which is messy and also doesn't fully express what you want. #pragma, or
> > > even just allowing -fbuiltin-foo options would be useful.
>
> I do really like the idea of -fbuiltin-foo. For example, you'd specify:
>
> -ffreestanding -fbuiltin-bcmp
>
> as an example. `-ffreestanding` would opt you out of ALL libcall
> optimizations, `-fbuiltin-bcmp` would then opt you back in to
> transforms that produce bcmp. That way you're informing the compiler
> more precisely about the environment you'd be targeting. It feels
> symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
> easy when there is such symmetry). And it's already convention that
> if you specify multiple conflicting compiler flags, then the latter
> one specified "wins." In that sense, turning back on specific
> libcalls after disabling the rest looks more ergonomic to me.
>
> Maybe Eli or David have thoughts on why that may or may not be as
> ergonomic or possible to implement as I imagine?
>
Note that -fno-builtin-foo seems to mean slightly different things in
clang and gcc. From experimentation, clang will neither optimize a call
to foo, nor perform an optimization that introduces a call to foo. gcc
will avoid optimizing calls to foo, but it can still generate new calls
to foo while optimizing something else. Which means that
-fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
just that gcc doesn't seem to have implemented those optimizations.
So -fno-builtin-stpcpy is not ideal solution then.. gcc may implement this opt too and here we go again, red builds.
We should either provide stpcpy implementation or fix few places and do not use sprintf there.
> Dňa 18. 8. 2020 o 23:41 užívateľ Arvind Sankar <[email protected]> napísal:
>
> On Tue, Aug 18, 2020 at 01:58:51PM -0700, Nick Desaulniers wrote:
>>> On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
>>> <[email protected]> wrote:
>>>
>>> On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
>>>>
>>>> On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
>>>>> On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
>>>>>>
>>>>>> I'm not saying "change the semantics", nor am I saying that playing
>>>>>> whack-a-mole *for a limited time* is unreasonable. But I would like to go back
>>>>>> to the compiler authors and get them to implement such a #pragma: "this
>>>>>> freestanding implementation *does* support *this specific library function*,
>>>>>> and you are free to call it."
>>>>>
>>>>> I'd much rather just see the library functions as builtins that always
>>>>> do the right thing (with the fallback being "just call the standard
>>>>> function").
>>>>>
>>>>> IOW, there's nothing wrong with -ffreestanding if you then also have
>>>>> __builtin_memcpy() etc, and they do the sane compiler optimizations
>>>>> for memcpy().
>>>>>
>>>>> What we want to avoid is the compiler making *assumptions* based on
>>>>> standard names, because we may implement some of those things
>>>>> differently.
>>>>>
>>>>
>>>> -ffreestanding as it stands today does have __builtin_memcpy and
>>>> friends. But you need to then use #define memcpy __builtin_memcpy etc,
>>>> which is messy and also doesn't fully express what you want. #pragma, or
>>>> even just allowing -fbuiltin-foo options would be useful.
>>
>> I do really like the idea of -fbuiltin-foo. For example, you'd specify:
>>
>> -ffreestanding -fbuiltin-bcmp
>>
>> as an example. `-ffreestanding` would opt you out of ALL libcall
>> optimizations, `-fbuiltin-bcmp` would then opt you back in to
>> transforms that produce bcmp. That way you're informing the compiler
>> more precisely about the environment you'd be targeting. It feels
>> symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
>> easy when there is such symmetry). And it's already convention that
>> if you specify multiple conflicting compiler flags, then the latter
>> one specified "wins." In that sense, turning back on specific
>> libcalls after disabling the rest looks more ergonomic to me.
>>
>> Maybe Eli or David have thoughts on why that may or may not be as
>> ergonomic or possible to implement as I imagine?
>>
>
> Note that -fno-builtin-foo seems to mean slightly different things in
> clang and gcc. From experimentation, clang will neither optimize a call
> to foo, nor perform an optimization that introduces a call to foo. gcc
> will avoid optimizing calls to foo, but it can still generate new calls
> to foo while optimizing something else. Which means that
> -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
> just that gcc doesn't seem to have implemented those optimizations.
From: Nick Desaulniers
> Sent: 18 August 2020 21:59
>
> On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
> <[email protected]> wrote:
...
> > > -ffreestanding as it stands today does have __builtin_memcpy and
> > > friends. But you need to then use #define memcpy __builtin_memcpy etc,
> > > which is messy and also doesn't fully express what you want. #pragma, or
> > > even just allowing -fbuiltin-foo options would be useful.
>
> I do really like the idea of -fbuiltin-foo. For example, you'd specify:
>
> -ffreestanding -fbuiltin-bcmp
>
> as an example. `-ffreestanding` would opt you out of ALL libcall
> optimizations, `-fbuiltin-bcmp` would then opt you back in to
> transforms that produce bcmp. That way you're informing the compiler
> more precisely about the environment you'd be targeting. It feels
> symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
> easy when there is such symmetry). And it's already convention that
> if you specify multiple conflicting compiler flags, then the latter
> one specified "wins." In that sense, turning back on specific
> libcalls after disabling the rest looks more ergonomic to me.
>
> Maybe Eli or David have thoughts on why that may or may not be as
> ergonomic or possible to implement as I imagine?
You might want -fbuiltin-bcmp=my_bcmp_function so that you can specify
the actual elf symbol name to be used.
I was recently trying to compile an application so that it would
run on a system with an old libc.
Avoiding explicit calls to new functions wasn't a problem, but I
couldn't do anything about the memcpy() calls generated by gcc
for structure copies.
Due to the silly glibc fiasco with memcpy going backwards I'd
either need to force out a reference to the old version of memcpy
or a reference to memove - but neither is possible.
I then tried a C++ program and gave up because 'char traits'
was all different. I then realised I need to remove all references
to std::string to get any kind of efficient object code :-)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Tue, Aug 18, 2020 at 2:41 PM Arvind Sankar <[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 01:58:51PM -0700, Nick Desaulniers wrote:
> > On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
> > <[email protected]> wrote:
> > >
> > > On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
> > > >
> > > > On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
> > > > > On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> > > > > >
> > > > > > I'm not saying "change the semantics", nor am I saying that playing
> > > > > > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > > > > > to the compiler authors and get them to implement such a #pragma: "this
> > > > > > freestanding implementation *does* support *this specific library function*,
> > > > > > and you are free to call it."
> > > > >
> > > > > I'd much rather just see the library functions as builtins that always
> > > > > do the right thing (with the fallback being "just call the standard
> > > > > function").
> > > > >
> > > > > IOW, there's nothing wrong with -ffreestanding if you then also have
> > > > > __builtin_memcpy() etc, and they do the sane compiler optimizations
> > > > > for memcpy().
> > > > >
> > > > > What we want to avoid is the compiler making *assumptions* based on
> > > > > standard names, because we may implement some of those things
> > > > > differently.
> > > > >
> > > >
> > > > -ffreestanding as it stands today does have __builtin_memcpy and
> > > > friends. But you need to then use #define memcpy __builtin_memcpy etc,
> > > > which is messy and also doesn't fully express what you want. #pragma, or
> > > > even just allowing -fbuiltin-foo options would be useful.
> >
> > I do really like the idea of -fbuiltin-foo. For example, you'd specify:
> >
> > -ffreestanding -fbuiltin-bcmp
> >
> > as an example. `-ffreestanding` would opt you out of ALL libcall
> > optimizations, `-fbuiltin-bcmp` would then opt you back in to
> > transforms that produce bcmp. That way you're informing the compiler
> > more precisely about the environment you'd be targeting. It feels
> > symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
> > easy when there is such symmetry). And it's already convention that
> > if you specify multiple conflicting compiler flags, then the latter
> > one specified "wins." In that sense, turning back on specific
> > libcalls after disabling the rest looks more ergonomic to me.
> >
> > Maybe Eli or David have thoughts on why that may or may not be as
> > ergonomic or possible to implement as I imagine?
> >
>
> Note that -fno-builtin-foo seems to mean slightly different things in
> clang and gcc. From experimentation, clang will neither optimize a call
> to foo, nor perform an optimization that introduces a call to foo. gcc
> will avoid optimizing calls to foo, but it can still generate new calls
> to foo while optimizing something else. Which means that
> -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
> just that gcc doesn't seem to have implemented those optimizations.
Can you please share some godbolt links that demonstrate these observations?
--
Thanks,
~Nick Desaulniers
On Mon, Aug 17, 2020 at 03:02:08PM -0700, Nick Desaulniers wrote:
> -ffreestanding typically inhibits "libcall optimizations" where calls to
> certain library functions can be replaced by the compiler in certain
> cases to calls to other library functions that may be more efficient.
> This can be problematic for embedded targets that don't provide full
> libc implementations.
>
> -ffreestanding inhibits all such optimizations, which is the safe
> choice, but generally we want the optimizations that are performed. The
> Linux kernel does implement a fair amount of libc routines. Instead of
> -ffreestanding (which makes more sense in smaller images like kexec's
> purgatory image), prefer -fno-builtin-* flags to disable the compiler
> from emitting calls to functions which may not be defined.
>
> If you see a linkage failure due to a missing symbol that's typically
> defined in a libc, and not explicitly called from the source code, then
> the compiler may have done such a transform. You can either implement
> such a function (ie. in lib/string.c) or disable the transform outright
> via -fno-builtin-* flag (where * is the name of the library routine, ie.
> -fno-builtin-bcmp).
>
> Patch 1 unbreaks the build with ToT clang, which has been red all
> weekend, by adding -fno-builtin-stpcpy.
> Patch 2 is a revert but adds -fno-builtin-bcmp.
> Patch 3 does the same for x86 purgatory.
> Patch 4 removes -ffreestanding from i386.
>
> The first patch makes sense for Kbuild, the second maybe akpm@, the
> third and forth for x86. Not sure who should pick up the series (they
> can be merged out of order, technically) but I really need the first
> patch soon. The 3 latter patches are cleanups.
>
> Nick Desaulniers (4):
> Makefile: add -fno-builtin-stpcpy
> Revert "lib/string.c: implement a basic bcmp"
> x86/boot: use -fno-builtin-bcmp
> x86: don't build CONFIG_X86_32 as -ffreestanding
>
> Makefile | 7 +++++++
> arch/x86/Makefile | 3 ---
> arch/x86/boot/Makefile | 1 +
> arch/x86/boot/string.c | 8 --------
> include/linux/string.h | 3 ---
> lib/string.c | 20 --------------------
> 6 files changed, 8 insertions(+), 34 deletions(-)
>
> --
> 2.28.0.220.ged08abb693-goog
>
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
On Tue, Aug 18, 2020 at 3:25 PM Arvind Sankar <[email protected]> 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.
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?
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
Here:
https://godbolt.org/z/qjo5P6
st 19. 8. 2020 o 0:00 Nick Desaulniers <[email protected]> napísal(a):
>
> On Tue, Aug 18, 2020 at 2:41 PM Arvind Sankar <[email protected]> wrote:
> >
> > On Tue, Aug 18, 2020 at 01:58:51PM -0700, Nick Desaulniers wrote:
> > > On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
> > > <[email protected]> wrote:
> > > >
> > > > On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
> > > > >
> > > > > On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
> > > > > > On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> > > > > > >
> > > > > > > I'm not saying "change the semantics", nor am I saying that playing
> > > > > > > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > > > > > > to the compiler authors and get them to implement such a #pragma: "this
> > > > > > > freestanding implementation *does* support *this specific library function*,
> > > > > > > and you are free to call it."
> > > > > >
> > > > > > I'd much rather just see the library functions as builtins that always
> > > > > > do the right thing (with the fallback being "just call the standard
> > > > > > function").
> > > > > >
> > > > > > IOW, there's nothing wrong with -ffreestanding if you then also have
> > > > > > __builtin_memcpy() etc, and they do the sane compiler optimizations
> > > > > > for memcpy().
> > > > > >
> > > > > > What we want to avoid is the compiler making *assumptions* based on
> > > > > > standard names, because we may implement some of those things
> > > > > > differently.
> > > > > >
> > > > >
> > > > > -ffreestanding as it stands today does have __builtin_memcpy and
> > > > > friends. But you need to then use #define memcpy __builtin_memcpy etc,
> > > > > which is messy and also doesn't fully express what you want. #pragma, or
> > > > > even just allowing -fbuiltin-foo options would be useful.
> > >
> > > I do really like the idea of -fbuiltin-foo. For example, you'd specify:
> > >
> > > -ffreestanding -fbuiltin-bcmp
> > >
> > > as an example. `-ffreestanding` would opt you out of ALL libcall
> > > optimizations, `-fbuiltin-bcmp` would then opt you back in to
> > > transforms that produce bcmp. That way you're informing the compiler
> > > more precisely about the environment you'd be targeting. It feels
> > > symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
> > > easy when there is such symmetry). And it's already convention that
> > > if you specify multiple conflicting compiler flags, then the latter
> > > one specified "wins." In that sense, turning back on specific
> > > libcalls after disabling the rest looks more ergonomic to me.
> > >
> > > Maybe Eli or David have thoughts on why that may or may not be as
> > > ergonomic or possible to implement as I imagine?
> > >
> >
> > Note that -fno-builtin-foo seems to mean slightly different things in
> > clang and gcc. From experimentation, clang will neither optimize a call
> > to foo, nor perform an optimization that introduces a call to foo. gcc
> > will avoid optimizing calls to foo, but it can still generate new calls
> > to foo while optimizing something else. Which means that
> > -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
> > just that gcc doesn't seem to have implemented those optimizations.
>
> Can you please share some godbolt links that demonstrate these observations?
> --
> Thanks,
> ~Nick Desaulniers
On Tue, Aug 18, 2020 at 3:05 PM Dávid Bolvanský
<[email protected]> wrote:
>
> st 19. 8. 2020 o 0:00 Nick Desaulniers <[email protected]> napísal(a):
> >
> > On Tue, Aug 18, 2020 at 2:41 PM Arvind Sankar <[email protected]> wrote:
> > >
> > > Note that -fno-builtin-foo seems to mean slightly different things in
> > > clang and gcc. From experimentation, clang will neither optimize a call
> > > to foo, nor perform an optimization that introduces a call to foo. gcc
> > > will avoid optimizing calls to foo, but it can still generate new calls
> > > to foo while optimizing something else. Which means that
> > > -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
> > > just that gcc doesn't seem to have implemented those optimizations.
> >
> > Can you please share some godbolt links that demonstrate these observations?
> Here:
> https://godbolt.org/z/qjo5P6
Ok, when I implemented this version that used -fno-builtin-stpcpy, I
initially+locally had it added to CLANG_FLAGS rather than
KBUILD_CFLAGS, but changed it to KBUILD_CFLAGS because I believed that
BOTH compilers would not lower calls to foo given -fno-builtin-foo.
Since we have evidence that's not the case, maybe that's the final
solution and my final proposal (v3). A summary:
1. v1 "implement stpcpy"
https://lore.kernel.org/lkml/[email protected]/T/#u
"Please don't provide more unsafe string functions to the kernel"
2. v2 "KBUILD_CFLAGS += -fno-builtin-stpcpy"
https://lore.kernel.org/lkml/[email protected]/T/#t
"-fno-builtin-* doesn't work like that on GCC"
3. v3 "CLANG_FLAGS += -fno-builtin-stpcpy" TODO
I'll argue that providing an implementation of stpcpy while hiding the
declaration from include/lib/string.h "for the possibility that GCC
may one day perform the same libcall optimization" as YAGNI, that we
may cross that bridge by resurrecting v1 (with the removal of the hunk
against include/lib/string.h). This also defers adding more unsafe
string functions in the kernel.
Thoughts before I send the patch and write that up?
--
Thanks,
~Nick Desaulniers
On Tue, Aug 18, 2020 at 03:59:45PM -0700, Nick Desaulniers wrote:
> On Tue, Aug 18, 2020 at 3:25 PM Arvind Sankar <[email protected]> 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
>
> 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).
>
What's the other implementation we need to worry about? purgatory (at
least on x86) has freestanding already.
On Tue, Aug 18, 2020 at 03:59:45PM -0700, Nick Desaulniers wrote:
> On Tue, Aug 18, 2020 at 3:25 PM Arvind Sankar <[email protected]> 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/[email protected]/
>
> 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.
From: Arvind Sankar
> Sent: 18 August 2020 23:26
...
> 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
Gah, if I want a call to memset() I'll write a call to memset().
If I'm trying to zero a small number of bytes I'll write a loop
because it'll be faster.
I wish compilers wouldn't even consider these sorts of 'optimisations'.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Tue, Aug 18, 2020 at 9:58 PM Nick Desaulniers <[email protected]> wrote:
On Tue, Aug 18, 2020 at 12:25 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 12:19 PM Linus Torvalds
> <[email protected]> wrote:
> >
> > And honestly, a compiler that uses 'bcmp' is just broken. WTH? It's
> > the year 2020, we don't use bcmp. It's that simple. Fix your damn
> > broken compiler and use memcmp. The argument that memcmp is more
> > expensive than bcmp is garbage legacy thinking from four decades ago.
> >
> > It's likely the other way around, where people have actually spent
> > time on memcmp, but not on bcmp.
> >
> > Linus
>
> You'll have to ask Clement about that. I'm not sure I ever saw the
> "faster bcmp than memcmp" implementation, but I was told "it exists"
> when I asked for a revert when all of our kernel builds went red.
If **is** possible to make bcmp much faster then memcmp. We have one
such implementation internally (it's scheduled to be released as part of
llvm-libc some time this year), but most libc implementations just alias to
memcmp.
Below is a graph showing the impact of releasing this compiler optimization
with our optimized bcmp on the google fleet (the cumulative memcmp+bcmp usage
of all programs running on google datacenters, including the kernel). Scale and
dates have been redacted for obvious reasons, but note that the graph starts at
y=0, so you can compare the values relative to each other. Note how as memcmp
is progressively being replaced by bcmp (more and more programs being
recompiled with the compiler patch), the cumulative usage of memory
comparison drops significantly.
https://drive.google.com/file/d/1p8z1ilw2xaAJEnx_5eu-vflp3tEOv0qY/view?usp=sharing
The reasons why bcmp can be faster are:
- typical libc implementations use the hardware to its full capacity, e.g. for
bcmp we can use vector loads and compares, which can process up to 64 bytes
(avx512) in one instruction. It's harder to implement memcmp with these for
little-endian architectures as there is no vector bswap. Because the kernel
only uses GPRs I can see how that might not perfectly fit the kernel use case.
But the kernel really is a special case, the compiler is written for most
programs, not specifically for the kernel, and most programs should benefit from
this optimization.
- bcmp() does not have to look at the bytes in order, e.g. it can look at the
first and last . This is useful when comparing buffers that have common
prefixes (as happens in mostly sorted containers, and we have data that shows
that this is a quite common instance).
> Also, to Clement's credit, every patch I've ever seen from Clement is
> backed up by data; typically fleetwide profiles at Google. "we spend
> a lot of time in memcmp, particularly comparing the result against
> zero and no other value; hmm...how do we spend less time in
> memcmp...oh, well there's another library function with slightly
> different semantics we can call instead." I don't think anyone would
> consider the optimization batshit crazy given the number of cycles
> saved across the fleet. That an embedded project didn't provide an
> implementation, is a footnote that can be fixed in the embedded
> project, either by using -ffreestanding or -fno-builtin-bcmp, which is
> what this series proposes to do.
On 18/08/2020 23.41, Arvind Sankar wrote:
> On Tue, Aug 18, 2020 at 01:58:51PM -0700, Nick Desaulniers wrote:
>> On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
>> <[email protected]> wrote:
>>>
>>> On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
>>>>
>>>> On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
>>>>> On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
>>>>>>
>>>>>> I'm not saying "change the semantics", nor am I saying that playing
>>>>>> whack-a-mole *for a limited time* is unreasonable. But I would like to go back
>>>>>> to the compiler authors and get them to implement such a #pragma: "this
>>>>>> freestanding implementation *does* support *this specific library function*,
>>>>>> and you are free to call it."
>>>>>
>>>>> I'd much rather just see the library functions as builtins that always
>>>>> do the right thing (with the fallback being "just call the standard
>>>>> function").
>>>>>
>>>>> IOW, there's nothing wrong with -ffreestanding if you then also have
>>>>> __builtin_memcpy() etc, and they do the sane compiler optimizations
>>>>> for memcpy().
>>>>>
>>>>> What we want to avoid is the compiler making *assumptions* based on
>>>>> standard names, because we may implement some of those things
>>>>> differently.
>>>>>
>>>>
>>>> -ffreestanding as it stands today does have __builtin_memcpy and
>>>> friends. But you need to then use #define memcpy __builtin_memcpy etc,
>>>> which is messy and also doesn't fully express what you want. #pragma, or
>>>> even just allowing -fbuiltin-foo options would be useful.
>>
>> I do really like the idea of -fbuiltin-foo. For example, you'd specify:
>>
>> -ffreestanding -fbuiltin-bcmp
>>
>> as an example. `-ffreestanding` would opt you out of ALL libcall
>> optimizations, `-fbuiltin-bcmp` would then opt you back in to
>> transforms that produce bcmp. That way you're informing the compiler
>> more precisely about the environment you'd be targeting. It feels
>> symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
>> easy when there is such symmetry). And it's already convention that
>> if you specify multiple conflicting compiler flags, then the latter
>> one specified "wins." In that sense, turning back on specific
>> libcalls after disabling the rest looks more ergonomic to me.
>>
>> Maybe Eli or David have thoughts on why that may or may not be as
>> ergonomic or possible to implement as I imagine?
>>
>
> Note that -fno-builtin-foo seems to mean slightly different things in
> clang and gcc. From experimentation, clang will neither optimize a call
> to foo, nor perform an optimization that introduces a call to foo. gcc
> will avoid optimizing calls to foo, but it can still generate new calls
> to foo while optimizing something else. Which means that
> -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
> just that gcc doesn't seem to have implemented those optimizations.
>
I think it's more than that. I've always read gcc's documentation
'-fno-builtin'
'-fno-builtin-FUNCTION'
Don't recognize built-in functions that do not begin with
'__builtin_' as prefix. ...
GCC normally generates special code to handle certain built-in
functions more efficiently; for instance, calls to 'alloca' may
become single instructions which adjust the stack directly, and
calls to 'memcpy' may become inline copy loops.
...
to mean exactly that observed above and nothing more, i.e. that
-fno-builtin-foo merely means that gcc stops treating a call of a
function named foo to mean a call to a function implementing the
standard function by that name (and hence allows it to e.g. replace a
memcpy(d, s, 1) by byte load+store). It does not mean to prevent
emitting calls to foo, and I don't think it ever will - it's a bit sad
that clang has chosen to interpret these options differently.
Thinking out load, it would be useful if both compilers grew
-fassume-provided-std-foo
and
-fno-assume-provided-std-foo
options to tell the compiler that a function named foo with standard
semantics can be assumed (or not) to be provided by the execution
environment; i.e. one half of what -f(no-)builtin-foo apparently does
for clang currently.
And yes, the positive -fbuiltin-foo would also be quite useful in order
to get the compiler to recognize a few important functions (memcpy,
memcmp) while using -ffreestanding (or just plain -fno-builtin) to tell
it to avoid assuming anything about most std functions - I've worked on
a VxWorks target where snprintf() didn't have the correct "return what
would be written" semantics but rather behaved like the kernel's
non-standard scnprintf(), and who knows what other odd quirks that libc had.
Rasmus
On Thu, Aug 20, 2020 at 04:56:02PM +0200, Rasmus Villemoes wrote:
> On 18/08/2020 23.41, Arvind Sankar wrote:
> >
> > Note that -fno-builtin-foo seems to mean slightly different things in
> > clang and gcc. From experimentation, clang will neither optimize a call
> > to foo, nor perform an optimization that introduces a call to foo. gcc
> > will avoid optimizing calls to foo, but it can still generate new calls
> > to foo while optimizing something else. Which means that
> > -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
> > just that gcc doesn't seem to have implemented those optimizations.
> >
>
> I think it's more than that. I've always read gcc's documentation
>
> '-fno-builtin'
> '-fno-builtin-FUNCTION'
> Don't recognize built-in functions that do not begin with
> '__builtin_' as prefix. ...
>
> GCC normally generates special code to handle certain built-in
> functions more efficiently; for instance, calls to 'alloca' may
> become single instructions which adjust the stack directly, and
> calls to 'memcpy' may become inline copy loops.
> ...
>
> to mean exactly that observed above and nothing more, i.e. that
> -fno-builtin-foo merely means that gcc stops treating a call of a
> function named foo to mean a call to a function implementing the
> standard function by that name (and hence allows it to e.g. replace a
> memcpy(d, s, 1) by byte load+store). It does not mean to prevent
> emitting calls to foo, and I don't think it ever will - it's a bit sad
> that clang has chosen to interpret these options differently.
That documentation is misleading, as it also goes on to say:
"...nor can you change the behavior of the functions by linking with a
different library"
which implies that you _can_ change the behavior if you use the option,
and which is what your "i.e." is saying as well.
My point is that this is not completely true: in gcc, foo by default is
defined to be __builtin_foo, and -fno-builtin-foo simply removes this
definition. So the effect is just that calls to foo in the original
source will be left alone.
But in order for an optimization that introduces a new call to foo to be
valid, foo _must_ have standard semantics: strchr(s,'\0') is not s +
strlen(s) unless strlen has standard semantics. This is an oversight in
gcc's optimizations: it converts to s + __builtin_strlen(s), which then
(normally) becomes s + strlen(s).
Check out this horror: https://godbolt.org/z/a1r9fK
Clang will disable this optimization if -fno-builtin-strlen is
specified.
Clang's interpretation is more useful for embedded, since you can use
-fno-builtin-foo and avoid calling __builtin_foo directly, and be
guaranteed that there will be no calls to foo that you didn't write
explicitly (outside of memcpy/memset/memcmp). In this case you are free
to implement foo with non-standard semantics, or avoid implementing it
altogether, and be reasonably confident that it will all work.
>
> Thinking out load, it would be useful if both compilers grew
>
> -fassume-provided-std-foo
>
> and
>
> -fno-assume-provided-std-foo
>
> options to tell the compiler that a function named foo with standard
> semantics can be assumed (or not) to be provided by the execution
> environment; i.e. one half of what -f(no-)builtin-foo apparently does
> for clang currently.
Not following: -fno-assume-provided-std-foo sounds like it would have
exactly the same semantics as Clang's -fno-builtin-foo, except maybe in
addition it should cause the compiler to error on seeing __builtin_foo
if it can't implement that without calling foo.
>
> And yes, the positive -fbuiltin-foo would also be quite useful in order
> to get the compiler to recognize a few important functions (memcpy,
> memcmp) while using -ffreestanding (or just plain -fno-builtin) to tell
> it to avoid assuming anything about most std functions - I've worked on
> a VxWorks target where snprintf() didn't have the correct "return what
> would be written" semantics but rather behaved like the kernel's
> non-standard scnprintf(), and who knows what other odd quirks that libc had.
>
> Rasmus
Yeah, gcc is doing weird things here : (
It is kinda sad that same flag does different things with gcc and clang.
> Dňa 20. 8. 2020 o 19:56 užívateľ Arvind Sankar <[email protected]> napísal:
>
> On Thu, Aug 20, 2020 at 04:56:02PM +0200, Rasmus Villemoes wrote:
>>> On 18/08/2020 23.41, Arvind Sankar wrote:
>>>
>>> Note that -fno-builtin-foo seems to mean slightly different things in
>>> clang and gcc. From experimentation, clang will neither optimize a call
>>> to foo, nor perform an optimization that introduces a call to foo. gcc
>>> will avoid optimizing calls to foo, but it can still generate new calls
>>> to foo while optimizing something else. Which means that
>>> -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
>>> just that gcc doesn't seem to have implemented those optimizations.
>>>
>>
>> I think it's more than that. I've always read gcc's documentation
>>
>> '-fno-builtin'
>> '-fno-builtin-FUNCTION'
>> Don't recognize built-in functions that do not begin with
>> '__builtin_' as prefix. ...
>>
>> GCC normally generates special code to handle certain built-in
>> functions more efficiently; for instance, calls to 'alloca' may
>> become single instructions which adjust the stack directly, and
>> calls to 'memcpy' may become inline copy loops.
>> ...
>>
>> to mean exactly that observed above and nothing more, i.e. that
>> -fno-builtin-foo merely means that gcc stops treating a call of a
>> function named foo to mean a call to a function implementing the
>> standard function by that name (and hence allows it to e.g. replace a
>> memcpy(d, s, 1) by byte load+store). It does not mean to prevent
>> emitting calls to foo, and I don't think it ever will - it's a bit sad
>> that clang has chosen to interpret these options differently.
>
> That documentation is misleading, as it also goes on to say:
> "...nor can you change the behavior of the functions by linking with a
> different library"
> which implies that you _can_ change the behavior if you use the option,
> and which is what your "i.e." is saying as well.
>
> My point is that this is not completely true: in gcc, foo by default is
> defined to be __builtin_foo, and -fno-builtin-foo simply removes this
> definition. So the effect is just that calls to foo in the original
> source will be left alone.
>
> But in order for an optimization that introduces a new call to foo to be
> valid, foo _must_ have standard semantics: strchr(s,'\0') is not s +
> strlen(s) unless strlen has standard semantics. This is an oversight in
> gcc's optimizations: it converts to s + __builtin_strlen(s), which then
> (normally) becomes s + strlen(s).
>
> Check out this horror: https://godbolt.org/z/a1r9fK
>
> Clang will disable this optimization if -fno-builtin-strlen is
> specified.
>
> Clang's interpretation is more useful for embedded, since you can use
> -fno-builtin-foo and avoid calling __builtin_foo directly, and be
> guaranteed that there will be no calls to foo that you didn't write
> explicitly (outside of memcpy/memset/memcmp). In this case you are free
> to implement foo with non-standard semantics, or avoid implementing it
> altogether, and be reasonably confident that it will all work.
>
>>
>> Thinking out load, it would be useful if both compilers grew
>>
>> -fassume-provided-std-foo
>>
>> and
>>
>> -fno-assume-provided-std-foo
>>
>> options to tell the compiler that a function named foo with standard
>> semantics can be assumed (or not) to be provided by the execution
>> environment; i.e. one half of what -f(no-)builtin-foo apparently does
>> for clang currently.
>
> Not following: -fno-assume-provided-std-foo sounds like it would have
> exactly the same semantics as Clang's -fno-builtin-foo, except maybe in
> addition it should cause the compiler to error on seeing __builtin_foo
> if it can't implement that without calling foo.
>
>>
>> And yes, the positive -fbuiltin-foo would also be quite useful in order
>> to get the compiler to recognize a few important functions (memcpy,
>> memcmp) while using -ffreestanding (or just plain -fno-builtin) to tell
>> it to avoid assuming anything about most std functions - I've worked on
>> a VxWorks target where snprintf() didn't have the correct "return what
>> would be written" semantics but rather behaved like the kernel's
>> non-standard scnprintf(), and who knows what other odd quirks that libc had.
>>
>> Rasmus
On 2020-08-18 13:58, Nick Desaulniers wrote:
> On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
> <[email protected]> wrote:
>>
>> On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
>>>
>>> On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
>>>> On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
>>>>>
>>>>> I'm not saying "change the semantics", nor am I saying that playing
>>>>> whack-a-mole *for a limited time* is unreasonable. But I would like to go back
>>>>> to the compiler authors and get them to implement such a #pragma: "this
>>>>> freestanding implementation *does* support *this specific library function*,
>>>>> and you are free to call it."
>>>>
>>>> I'd much rather just see the library functions as builtins that always
>>>> do the right thing (with the fallback being "just call the standard
>>>> function").
>>>>
>>>> IOW, there's nothing wrong with -ffreestanding if you then also have
>>>> __builtin_memcpy() etc, and they do the sane compiler optimizations
>>>> for memcpy().
>>>>
>>>> What we want to avoid is the compiler making *assumptions* based on
>>>> standard names, because we may implement some of those things
>>>> differently.
>>>>
>>>
>>> -ffreestanding as it stands today does have __builtin_memcpy and
>>> friends. But you need to then use #define memcpy __builtin_memcpy etc,
>>> which is messy and also doesn't fully express what you want. #pragma, or
>>> even just allowing -fbuiltin-foo options would be useful.
>
> I do really like the idea of -fbuiltin-foo. For example, you'd specify:
>
> -ffreestanding -fbuiltin-bcmp
>
> as an example. `-ffreestanding` would opt you out of ALL libcall
> optimizations, `-fbuiltin-bcmp` would then opt you back in to
> transforms that produce bcmp. That way you're informing the compiler
> more precisely about the environment you'd be targeting. It feels
> symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
> easy when there is such symmetry). And it's already convention that
> if you specify multiple conflicting compiler flags, then the latter
> one specified "wins." In that sense, turning back on specific
> libcalls after disabling the rest looks more ergonomic to me.
>
> Maybe Eli or David have thoughts on why that may or may not be as
> ergonomic or possible to implement as I imagine?
>
I would prefer this to be a #pragma for a header file, rather than
having a very long command line for everything...
-hpa
On Thu, Aug 20, 2020 at 03:41:33PM -0700, H. Peter Anvin wrote:
>
> I would prefer this to be a #pragma for a header file, rather than
> having a very long command line for everything...
>
> -hpa
>
There is @option_file, though.
On Thu, Aug 20, 2020 at 10:56 AM Arvind Sankar <[email protected]> wrote:
>
> Clang's interpretation is more useful for embedded, since you can use
> -fno-builtin-foo and avoid calling __builtin_foo directly, and be
> guaranteed that there will be no calls to foo that you didn't write
> explicitly (outside of memcpy/memset/memcmp). In this case you are free
> to implement foo with non-standard semantics, or avoid implementing it
> altogether, and be reasonably confident that it will all work.
Honestly, I think concentrating on whether __builtin_foo() works or
not misses the point entirely.
That has never _ever_ been a problem for us, and I doubt it has been a
problem for anybody else either.
If you use __builtin_memcpy() in your source tree, then why would you
possibly ever want to disable it? And if you _don't_ use it, then
again - why would you ever want to disable it?
No, the real (and only) problem has always been about the compilers
magically and silently "recognizing" certain source patterns, and
turning them into function calls behind the users back.
And that has nothing to do with __builtin_foo(). At least it
_shouldn't_ have anything to do with it.
So this is things like the compiler silently seeing "oh, you called
your function 'free()', so we know that the stores you did to it are
dead and we'll remove them".
Or this is the compiler doing "Oh, you did four stores of zero in a
row, and and you asked for size optimizations, so we'll turn those
into a 'bzero()' call".
Or the compiler doing completely broken things, and turning a
"!memcmp()" expression into a "!bcmp()" because the compilier
incorrectly assumes it's faster.
Notice? Not a single one of those had any __builtin_xyz() pattern in
them. Quite the reverse. The compiler took something completely
different, and assumed builtin semantics without us having told it to.
So I think "-fno-builtin-xyz" is barking *completely* up the wrong
tree. It's missing the problem. The problem is not "I have some
builtin patterns, here, you can use them".
It's the same as all the vector intrinsics. Those don't hurt anybody -
as long as they only get used when people use the intrinsics. If the
compiler starts to suddenly use vector intrinsics without being told
to, *THAT* can be a problem. But there is never any reson to turn off
any particular intrinsic otherwise.
If you don't want it used, don't use it. And if you do use it, the
compiler generates the vector code sequence. It's that simple.
So to me, a compiler flag like "-fno-builtin-memcpy" is completely
insane. The flag adds absolutely no value.
The real value would be "-fno-magic-bcmp" which turns off stupid
optimizations that magically turn non-bcmp things into bcmp. But it
should not turn off *actual* __builtin_bcmp() if such a thing exists
and people want to explicitly use it.
Linus
On 20/08/2020 19.56, Arvind Sankar wrote:
> On Thu, Aug 20, 2020 at 04:56:02PM +0200, Rasmus Villemoes wrote:
>> On 18/08/2020 23.41, Arvind Sankar wrote:
>>>
>>> Note that -fno-builtin-foo seems to mean slightly different things in
>>> clang and gcc. From experimentation, clang will neither optimize a call
>>> to foo, nor perform an optimization that introduces a call to foo. gcc
>>> will avoid optimizing calls to foo, but it can still generate new calls
>>> to foo while optimizing something else. Which means that
>>> -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
>>> just that gcc doesn't seem to have implemented those optimizations.
>>>
>>
>> I think it's more than that. I've always read gcc's documentation
>>
>> '-fno-builtin'
>> '-fno-builtin-FUNCTION'
>> Don't recognize built-in functions that do not begin with
>> '__builtin_' as prefix. ...
>>
>> GCC normally generates special code to handle certain built-in
>> functions more efficiently; for instance, calls to 'alloca' may
>> become single instructions which adjust the stack directly, and
>> calls to 'memcpy' may become inline copy loops.
>> ...
>>
>> to mean exactly that observed above and nothing more, i.e. that
>> -fno-builtin-foo merely means that gcc stops treating a call of a
>> function named foo to mean a call to a function implementing the
>> standard function by that name (and hence allows it to e.g. replace a
>> memcpy(d, s, 1) by byte load+store). It does not mean to prevent
>> emitting calls to foo, and I don't think it ever will - it's a bit sad
>> that clang has chosen to interpret these options differently.
>
> That documentation is misleading, as it also goes on to say:
> "...nor can you change the behavior of the functions by linking with a
> different library"
> which implies that you _can_ change the behavior if you use the option,
> and which is what your "i.e." is saying as well.
>
> My point is that this is not completely true: in gcc, foo by default is
> defined to be __builtin_foo, and -fno-builtin-foo simply removes this
> definition. So the effect is just that calls to foo in the original
> source will be left alone.
Yes, this is a much better way of putting it. And with -fbuiltin-foo in
effect, the compiler just needs to transform the code in some way as-if
the standard function by that name was called, which it can of course
decide to implement by emitting such a call, but it can also open-code
it - or synthesize it using other std functions.
> But in order for an optimization that introduces a new call to foo to be
> valid, foo _must_ have standard semantics: strchr(s,'\0') is not s +
> strlen(s) unless strlen has standard semantics.
Correct. So I agree that -fno-builtin-strlen should prevent the compiler
from generating calls to strlen() that don't appear in the code.
This is an oversight in
> gcc's optimizations: it converts to s + __builtin_strlen(s), which then
> (normally) becomes s + strlen(s).
>
> Check out this horror: https://godbolt.org/z/a1r9fK
>
> Clang will disable this optimization if -fno-builtin-strlen is
> specified.
>
> Clang's interpretation is more useful for embedded, since you can use
> -fno-builtin-foo and avoid calling __builtin_foo directly, and be
> guaranteed that there will be no calls to foo that you didn't write
> explicitly (outside of memcpy/memset/memcmp). In this case you are free
> to implement foo with non-standard semantics, or avoid implementing it
> altogether, and be reasonably confident that it will all work.
Yeah, except that the list of -fno-builtin-foo one would have to pass is
enourmous, so for targets with a somewhat wonky libc, I'd much rather be
able to do a blanket -fno-builtin, and then manually check their memcpy,
memset and memcmp implementations and opt back in with
-fbuiltin-mem{cpy,set,cmp} so that small constant-size memcpys do get
properly open-coded.
The advice in gcc's documentation of just #definining memcpy() to
__builtin_memcpy() doesn't work in the real world (for example it breaks
C++ code that uses std::memcpy(...)).
>> Thinking out load, it would be useful if both compilers grew
>>
>> -fassume-provided-std-foo
>>
>> and
>>
>> -fno-assume-provided-std-foo
>>
>> options to tell the compiler that a function named foo with standard
>> semantics can be assumed (or not) to be provided by the execution
>> environment; i.e. one half of what -f(no-)builtin-foo apparently does
>> for clang currently.
>
> Not following: -fno-assume-provided-std-foo sounds like it would have
> exactly the same semantics as Clang's -fno-builtin-foo, except maybe in
> addition it should cause the compiler to error on seeing __builtin_foo
> if it can't implement that without calling foo.
Yeah, I think you've convinced me there's no use for a separate option
to prevent inventing calls to foo() - I was mostly thinking of it as a
way to avoid having to provide each and every libc function that may
have been half-way standardized at some point. But if one doesn't
provide, say, bcmp, the code base certainly doesn't use bcmp itself, so
one doesn't lose anything by just using -fno-builtin-bcmp; there are no
explicit bcmp() uses that fail to get optimized.
Rasmus
On Thu, Aug 20, 2020 at 04:33:03PM -0700, Linus Torvalds wrote:
> On Thu, Aug 20, 2020 at 10:56 AM Arvind Sankar <[email protected]> wrote:
> >
> > Clang's interpretation is more useful for embedded, since you can use
> > -fno-builtin-foo and avoid calling __builtin_foo directly, and be
> > guaranteed that there will be no calls to foo that you didn't write
> > explicitly (outside of memcpy/memset/memcmp). In this case you are free
> > to implement foo with non-standard semantics, or avoid implementing it
> > altogether, and be reasonably confident that it will all work.
>
> Honestly, I think concentrating on whether __builtin_foo() works or
> not misses the point entirely.
>
> That has never _ever_ been a problem for us, and I doubt it has been a
> problem for anybody else either.
>
> If you use __builtin_memcpy() in your source tree, then why would you
> possibly ever want to disable it? And if you _don't_ use it, then
> again - why would you ever want to disable it?
>
> No, the real (and only) problem has always been about the compilers
> magically and silently "recognizing" certain source patterns, and
> turning them into function calls behind the users back.
>
> And that has nothing to do with __builtin_foo(). At least it
> _shouldn't_ have anything to do with it.
>
There seems to be some confusion here. The recognition and
__builtin_foo() go hand-in-hand: memcpy() is special _because_ the
compiler defines it to be __builtin_memcpy(); and the compiler turns the
patterns into __builtin_foo() calls, which just end up as a call to
foo() if they can't be inlined. The no-builtin- options _don't_ disable
__builtin_ functions. They remove the default definition of foo() as
__builtin_foo().
Take the problem that instigated this thread. __builtin_stpcpy() doesn't
work in the kernel because the fallback, stpcpy(), isn't implemented.
The optimization is doing:
sprintf(buf,"%s",s)
-> __builtin_sprintf(buf,"%s",s)
-> __builtin_stpcpy(buf,s)-buf
-> stpcpy(buf,s)-buf
Now, further below, you basically say this is an example of the compiler
taking something non-stpcpy() and turning it into stpcpy(), and you ask
for a no-magic-stpcpy that would stop this optimization. That's what
clang's no-builtin-stpcpy already does. The only extra thing it does is
that the compiler will also not touch an explicit call to stpcpy(), but
you can still call __builtin_stpcpy() if you really want it.
This is what was going on in that LZ4 memcpy() issue: the compiler was
faithfully compiling code like memcpy(d,s,8) into a call to memcpy()
because we told it not to define memcpy() as __builtin_memcpy(), by
compiling for a freestanding environment.
This is why I'm saying clang's no-builtin-foo option is useful for
embedded: it doesn't prevent the programmer using __builtin_foo(), it
prevents the _compiler_ using __builtin_foo() on its own.
> So this is things like the compiler silently seeing "oh, you called
> your function 'free()', so we know that the stores you did to it are
> dead and we'll remove them".
>
> Or this is the compiler doing "Oh, you did four stores of zero in a
> row, and and you asked for size optimizations, so we'll turn those
> into a 'bzero()' call".
This one is slightly different from the previous one. The first case is
really a call to __builtin_free().
This one is turning something that wasn't a function call into
__builtin_bzero(), and I would hope that no-builtin-bzero would stop it
as well. OTOH, the compiler is free to turn it into memset(), just like
it could for structure/array initializers.
The memcpy/memset/memcmp family is a bit of an edge case: the compiler
requires them to be defined even for freestanding environments, so you
can't in general stop the compiler from turning something into memset().
(That -ffreestanding stops gcc from turning loops into memset() is a
pragmatic recognition that some people are going to try to implement
memset() in C.)
>
> Or the compiler doing completely broken things, and turning a
> "!memcmp()" expression into a "!bcmp()" because the compilier
> incorrectly assumes it's faster.
Stop it with the bcmp-shaming already. bcmp _can_ be implemented faster
than memcmp, and it's no great loss if it isn't, since then it'll just
be an alias to memcmp in any sensible libc.
>
> Notice? Not a single one of those had any __builtin_xyz() pattern in
> them. Quite the reverse. The compiler took something completely
> different, and assumed builtin semantics without us having told it to.
>
> So I think "-fno-builtin-xyz" is barking *completely* up the wrong
> tree. It's missing the problem. The problem is not "I have some
> builtin patterns, here, you can use them".
Nope: in a hosted environment, xyz() _is_ __builtin_xyz(), and that is
almost always a good thing for 99% of the code out there: you tell it to
use builtin semantics by choosing to compile for a hosted environment.
If you want something in between freestanding and hosted, you absolutely
need some way to tell the compiler exactly which xyz()'s can be treated
as __builtin_xyz() and which ones shouldn't. The no-builtin- flags allow
you to start from a hosted environment and turn off the specialness of
the functions that you don't want to be special. The proposed builtin-
flags would allow you to start from freestanding and turn on the
specialness of the functions that you do want to be special.
>
> It's the same as all the vector intrinsics. Those don't hurt anybody -
> as long as they only get used when people use the intrinsics. If the
> compiler starts to suddenly use vector intrinsics without being told
> to, *THAT* can be a problem. But there is never any reson to turn off
> any particular intrinsic otherwise.
>
> If you don't want it used, don't use it. And if you do use it, the
> compiler generates the vector code sequence. It's that simple.
>
> So to me, a compiler flag like "-fno-builtin-memcpy" is completely
> insane. The flag adds absolutely no value.
>
> The real value would be "-fno-magic-bcmp" which turns off stupid
> optimizations that magically turn non-bcmp things into bcmp. But it
> should not turn off *actual* __builtin_bcmp() if such a thing exists
> and people want to explicitly use it.
>
> Linus
On Fri, Aug 21, 2020 at 10:29 AM Arvind Sankar <[email protected]> wrote:
>
> The no-builtin- options _don't_ disable
> __builtin_ functions. They remove the default definition of foo() as
> __builtin_foo().
Oh, ok, then it's fine.
> Take the problem that instigated this thread. __builtin_stpcpy() doesn't
> work in the kernel because the fallback, stpcpy(), isn't implemented.
Right.
The only problem is the - bogus - recognition of (or rewriting of)
code patterns that the compiler recohnmizes.
__builtin_stpcpy() itself is fine if somebody wants to use it - and
has the fallback.
But implicitly recognizing some code sequence and turning it into
something else is the problem.
> This is why I'm saying clang's no-builtin-foo option is useful for
> embedded: it doesn't prevent the programmer using __builtin_foo(), it
> prevents the _compiler_ using __builtin_foo() on its own.
And that's fine. But it's apparently not what gcc does.
> > So this is things like the compiler silently seeing "oh, you called
> > your function 'free()', so we know that the stores you did to it are
> > dead and we'll remove them".
> >
> > Or this is the compiler doing "Oh, you did four stores of zero in a
> > row, and and you asked for size optimizations, so we'll turn those
> > into a 'bzero()' call".
>
> This one is slightly different from the previous one. The first case is
> really a call to __builtin_free().
No, the first case is a disgrace and a compiler bug.
We've had a situation where gcc complained about a static function
called "free()", without any header file inclusion, and then
complained about it not matching its idea of what "free()" is.
Which is pure and utter garbage.
It's like you have a local variable "int free", and the compiler says
"hey, this doesn't match the prototype that I know this name should
have". It's BS. You just saw the user not just *use* that name, but
*define* it, and do it in a scope where the complaint is irrelevant
and stupid, and when we hadn't even included the header that would
have resulted in conflicts.
IOW, it's an example of a compiler that thinks "it knows better".
It's a broken compiler. And it's an example of the kind of breakage
that compilers absolutely shouldn't do.
The second example is from clang doesn't something horribly horribly stupid.
> This one is turning something that wasn't a function call into
> __builtin_bzero(), and I would hope that no-builtin-bzero would stop it
> as well. OTOH, the compiler is free to turn it into memset(), just like
> it could for structure/array initializers.
The whole "the compiler is free to do X" argument is pure BS. Stop
repeating that bogus argument.
Of COURSE a compiler can do whatever the hell it wants.
That doesn't change the fact that certain things are broken beyond
words and utterly stupid, and a compiler that does them is a *BAD*
compiler.
Turning four stores into a memset() is garbage. Just admit it, instead
of trying to say that it's allowed.
Technically a compiler can decode to simply not compile the kernel at
all, because we do things that are outside a lot of standards.
So the "technically allowed" is not an argument.
Linus
On Fri, Aug 21, 2020 at 10:54 AM Linus Torvalds
<[email protected]> wrote:
>
> On Fri, Aug 21, 2020 at 10:29 AM Arvind Sankar <[email protected]> wrote:
> >
> > This is why I'm saying clang's no-builtin-foo option is useful for
> > embedded: it doesn't prevent the programmer using __builtin_foo(), it
> > prevents the _compiler_ using __builtin_foo() on its own.
>
> And that's fine. But it's apparently not what gcc does.
Oh, testing it seems to say that that is exactly what gcc does too. I
must have misunderstood some comment in this thread to mean otherwise.
Linus
On Fri, Aug 21, 2020 at 11:02:08AM -0700, Linus Torvalds wrote:
> On Fri, Aug 21, 2020 at 10:54 AM Linus Torvalds
> <[email protected]> wrote:
> >
> > On Fri, Aug 21, 2020 at 10:29 AM Arvind Sankar <[email protected]> wrote:
> > >
> > > This is why I'm saying clang's no-builtin-foo option is useful for
> > > embedded: it doesn't prevent the programmer using __builtin_foo(), it
> > > prevents the _compiler_ using __builtin_foo() on its own.
> >
> > And that's fine. But it's apparently not what gcc does.
>
> Oh, testing it seems to say that that is exactly what gcc does too. I
> must have misunderstood some comment in this thread to mean otherwise.
>
> Linus
How are you testing it?
https://godbolt.org/z/eahdGn
On Fri, Aug 21, 2020 at 12:15 PM Arvind Sankar <[email protected]> wrote:
>
> How are you testing it?
>
> https://godbolt.org/z/eahdGn
Ugh. I tested the reverse thing - that the builtin is still available
for manual use despite the -fno-builtin.
Because I - stupidly - assumed that fno-builtin would do *something*.
But if it's available for manual use _and_ it's used implicitly by the
compiler, then that was clearly very naive of me.
Linus
On Fri, Aug 21, 2020 at 10:54:57AM -0700, Linus Torvalds wrote:
> On Fri, Aug 21, 2020 at 10:29 AM Arvind Sankar <[email protected]> wrote:
> >
> > This one is slightly different from the previous one. The first case is
> > really a call to __builtin_free().
>
> No, the first case is a disgrace and a compiler bug.
>
> We've had a situation where gcc complained about a static function
> called "free()", without any header file inclusion, and then
> complained about it not matching its idea of what "free()" is.
>
> Which is pure and utter garbage.
>
> It's like you have a local variable "int free", and the compiler says
> "hey, this doesn't match the prototype that I know this name should
> have". It's BS. You just saw the user not just *use* that name, but
> *define* it, and do it in a scope where the complaint is irrelevant
> and stupid, and when we hadn't even included the header that would
> have resulted in conflicts.
>
> IOW, it's an example of a compiler that thinks "it knows better".
>
> It's a broken compiler. And it's an example of the kind of breakage
> that compilers absolutely shouldn't do.
That's -Wbuiltin-declaration-mismatch, and can be turned off, and it
won't warn if you have -fno-builtin-free. I don't completely agree with
you, though warning for static functions is a bit overzealous. For an
external function, especially something more obscure like stpcpy(), I
appreciate the warning.
>
> The second example is from clang doesn't something horribly horribly stupid.
Calm down man :)
>
> > This one is turning something that wasn't a function call into
> > __builtin_bzero(), and I would hope that no-builtin-bzero would stop it
> > as well. OTOH, the compiler is free to turn it into memset(), just like
> > it could for structure/array initializers.
>
> The whole "the compiler is free to do X" argument is pure BS. Stop
> repeating that bogus argument.
>
> Of COURSE a compiler can do whatever the hell it wants.
>
> That doesn't change the fact that certain things are broken beyond
> words and utterly stupid, and a compiler that does them is a *BAD*
> compiler.
>
> Turning four stores into a memset() is garbage. Just admit it, instead
> of trying to say that it's allowed.
>
Look, four stores into memset(), yeah that's a bit weird. I didn't think
you meant "four" literally. But in any case, that has nothing to do with
the topic at hand. It would be just as bad if it was a 16-byte structure
being initialized with an out-of-line memset() call.
But coming back to the actual topic: it is fine if the compiler turns
four stores into __builtin_memset(). A size-16 or -32 __builtin_memset()
will get inlined anyway. There's a lot of garbage here if you look
closely: check out what gcc does to initialize a 7-character array with
zeros at -Os.
On Fri, Aug 21, 2020 at 03:57:12PM -0400, Arvind Sankar wrote:
> closely: check out what gcc does to initialize a 7-character array with
> zeros at -Os.
You do know that 's' stands for 'stupid', right?
-Os is infamous for generating atrocious crap.
On Fri, Aug 21, 2020 at 12:57 PM Arvind Sankar <[email protected]> wrote:
>
> Look, four stores into memset(), yeah that's a bit weird. I didn't think
> you meant "four" literally. But in any case, that has nothing to do with
> the topic at hand. It would be just as bad if it was a 16-byte structure
> being initialized with an out-of-line memset() call.
Actually, I mis-remembered. It wasn't four stores.
It was two.
We have this lovely "sas_ss_reset()" function that initializes three
fields in a structure (two to zero, one to '2').
And we used it in a critical place that didn't allow function calls
(because we have magic rules with the SMAP instructions).
And clang turned the initalization into a memset(). Which then
triggered our "you can't do that here" check on the generated code.
This is the kind of special rules we sometimes can have for code
generation, where the compiler really doesn't understand that no, you
can't just replace this code sequence with a function call, because
there are things going on around it that really mean that the code
should be generated the way we wrote it.
> But coming back to the actual topic: it is fine if the compiler turns
> four stores into __builtin_memset(). A size-16 or -32 __builtin_memset()
> will get inlined anyway. There's a lot of garbage here if you look
> closely: check out what gcc does to initialize a 7-character array with
> zeros at -Os.
Yeah. The reason we had to make -Os be a non-preferred thing is
because gcc has some really sad things it does when optimizing for
size.
I happen to believe that I$ matters, but -Os made _such_ a mess of
things that it was untenable to use ;(
Linus
On Fri, Aug 21, 2020 at 2:39 PM Linus Torvalds
<[email protected]> wrote:
>
> On Fri, Aug 21, 2020 at 12:57 PM Arvind Sankar <[email protected]> wrote:
> >
> > Look, four stores into memset(), yeah that's a bit weird. I didn't think
> > you meant "four" literally. But in any case, that has nothing to do with
> > the topic at hand. It would be just as bad if it was a 16-byte structure
> > being initialized with an out-of-line memset() call.
>
> Actually, I mis-remembered. It wasn't four stores.
>
> It was two.
>
> We have this lovely "sas_ss_reset()" function that initializes three
> fields in a structure (two to zero, one to '2').
>
> And we used it in a critical place that didn't allow function calls
> (because we have magic rules with the SMAP instructions).
>
> And clang turned the initalization into a memset(). Which then
> triggered our "you can't do that here" check on the generated code.
>
> This is the kind of special rules we sometimes can have for code
> generation, where the compiler really doesn't understand that no, you
> can't just replace this code sequence with a function call, because
> there are things going on around it that really mean that the code
> should be generated the way we wrote it.
For more context for folks at home eating popcorn and enjoying the
show: https://github.com/ClangBuiltLinux/linux/issues/876#issuecomment-613049480.
And that was specifically with KASAN enabled and doesn't appear to be
common behavior in clang otherwise (higher threshold). Why the
heuristics change for when it seems to be more profitable to roll
assignment of contiguous members of the same struct to the same value
into a memset, and 2 longs seems to be the threshold for KASAN, I
don't know. But I agree that should be fixed on the compiler side,
which is why I haven't been pushing the kernel workaround.
Everyone's just too busy right now; folks working on kernel sanitizers
have their hands full with KCSAN or MTE (on armv8.5) or default
initialization, and I'm trying to keep the build green (ie. this
series, well the v2 below, and
https://lore.kernel.org/lkml/[email protected]/
would be much appreciated), and get ready for plumbers, and wrap up my
intern's project, and do yearly performance reviews at my day job, and
million other things.
I've filed https://bugs.llvm.org/show_bug.cgi?id=47280 to discuss more
the ability to opt into builtins from a freestanding environment.
Now that Arvind has provided an excellent analysis of how the builtin
functionality works (bookmarked:
https://lore.kernel.org/lkml/[email protected]/),
were there still objections to add the -fno-builtin-stpcpy flags for
clang to the Makefile? I would like to get
https://lore.kernel.org/lkml/[email protected]/T/#m76c445f9645f62bc6ffc48ca26949725235688a0
landed so the build is not red for another week.
--
Thanks,
~Nick Desaulniers
> For more context for folks at home eating popcorn and enjoying the
> show: https://github.com/ClangBuiltLinux/linux/issues/876#issuecomment-613049480.
> And that was specifically with KASAN enabled and doesn't appear to be
> common behavior in clang otherwise (higher threshold). Why the
> heuristics change for when it seems to be more profitable to roll
> assignment of contiguous members of the same struct to the same value
> into a memset, and 2 longs seems to be the threshold for KASAN, I
> don't know. But I agree that should be fixed on the compiler side,
> which is why I haven't been pushing the kernel workaround.
Given x86 has is a simple 3-instruction loop for memset
that will do 1 write/clock (the max on current cpu) I
doubt it is ever worth not inlining memset().
The only real special case is lengths < 8.
For KASAN I wonder if something is stopping it inlining memset()?
So what usually happens is the two stores get converted to memset()
and then the memset() gets inlined back to two stores?
OTOH all this faffing for memset and memcpy is probably a
waste of time.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Wed, Aug 19, 2020 at 6:41 AM Arvind Sankar <[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 01:58:51PM -0700, Nick Desaulniers wrote:
> > On Tue, Aug 18, 2020 at 1:27 PM Nick Desaulniers
> > <[email protected]> wrote:
> > >
> > > On Tue, Aug 18, 2020 at 1:24 PM Arvind Sankar <[email protected]> wrote:
> > > >
> > > > On Tue, Aug 18, 2020 at 12:13:22PM -0700, Linus Torvalds wrote:
> > > > > On Tue, Aug 18, 2020 at 12:03 PM H. Peter Anvin <[email protected]> wrote:
> > > > > >
> > > > > > I'm not saying "change the semantics", nor am I saying that playing
> > > > > > whack-a-mole *for a limited time* is unreasonable. But I would like to go back
> > > > > > to the compiler authors and get them to implement such a #pragma: "this
> > > > > > freestanding implementation *does* support *this specific library function*,
> > > > > > and you are free to call it."
> > > > >
> > > > > I'd much rather just see the library functions as builtins that always
> > > > > do the right thing (with the fallback being "just call the standard
> > > > > function").
> > > > >
> > > > > IOW, there's nothing wrong with -ffreestanding if you then also have
> > > > > __builtin_memcpy() etc, and they do the sane compiler optimizations
> > > > > for memcpy().
> > > > >
> > > > > What we want to avoid is the compiler making *assumptions* based on
> > > > > standard names, because we may implement some of those things
> > > > > differently.
> > > > >
> > > >
> > > > -ffreestanding as it stands today does have __builtin_memcpy and
> > > > friends. But you need to then use #define memcpy __builtin_memcpy etc,
> > > > which is messy and also doesn't fully express what you want. #pragma, or
> > > > even just allowing -fbuiltin-foo options would be useful.
> >
> > I do really like the idea of -fbuiltin-foo. For example, you'd specify:
> >
> > -ffreestanding -fbuiltin-bcmp
> >
> > as an example. `-ffreestanding` would opt you out of ALL libcall
> > optimizations, `-fbuiltin-bcmp` would then opt you back in to
> > transforms that produce bcmp. That way you're informing the compiler
> > more precisely about the environment you'd be targeting. It feels
> > symmetric to existing `-fno-` flags (clang makes -f vs -fno- pretty
> > easy when there is such symmetry). And it's already convention that
> > if you specify multiple conflicting compiler flags, then the latter
> > one specified "wins." In that sense, turning back on specific
> > libcalls after disabling the rest looks more ergonomic to me.
> >
> > Maybe Eli or David have thoughts on why that may or may not be as
> > ergonomic or possible to implement as I imagine?
> >
>
> Note that -fno-builtin-foo seems to mean slightly different things in
> clang and gcc. From experimentation, clang will neither optimize a call
> to foo, nor perform an optimization that introduces a call to foo. gcc
> will avoid optimizing calls to foo, but it can still generate new calls
> to foo while optimizing something else. Which means that
> -fno-builtin-{bcmp,stpcpy} only solves things for clang, not gcc. It's
> just that gcc doesn't seem to have implemented those optimizations.
To prevent transformation from foo() into bar(),
there are two ways in Clang to do that;
-fno-builtin-foo, and -fno-builtin-bar.
There is only one in GCC; -fno-buitin-foo.
Is this correct?
I just played the optimization
from printf("helloworld\n") to puts("helloworld").
https://godbolt.org/z/5s4ded
-fno-builtin-puts cannot prevent clang
from emitting puts.
Is it because clang does not support
-fno-builtin-puts?
It is not clear to find out
which -fno-builtin-* is supported
because compilation succeeds anyway...
--
Best Regards
Masahiro Yamada
On Tue, Aug 25, 2020 at 12:57:22AM +0900, Masahiro Yamada wrote:
>
>
> To prevent transformation from foo() into bar(),
> there are two ways in Clang to do that;
> -fno-builtin-foo, and -fno-builtin-bar.
> There is only one in GCC; -fno-buitin-foo.
>
> Is this correct?
>
It looked that way from previous experimentation, but...
>
>
> I just played the optimization
> from printf("helloworld\n") to puts("helloworld").
>
> https://godbolt.org/z/5s4ded
>
>
> -fno-builtin-puts cannot prevent clang
> from emitting puts.
> Is it because clang does not support
> -fno-builtin-puts?
Ugh. clang doesn't have __builtin_puts() but it optimizes printf() into
puts(). It doesn't have __builtin_putchar() but will optimize
printf("c") into putchar('c').
And it has .. half of __builtin_fwrite()? fwrite() of a single byte gets
optimized into fputc(), fprintf() of a string literal gets optimized
into fwrite(), -fno-builtin-fwrite prevents both optimizations, but
__builtin_fwrite() gives a compile error.
I give up.
On Mon, Aug 24, 2020 at 10:34 AM Arvind Sankar <[email protected]> wrote:
>
> On Tue, Aug 25, 2020 at 12:57:22AM +0900, Masahiro Yamada wrote:
> >
> >
> > To prevent transformation from foo() into bar(),
> > there are two ways in Clang to do that;
> > -fno-builtin-foo, and -fno-builtin-bar.
> > There is only one in GCC; -fno-buitin-foo.
> >
> > Is this correct?
> >
>
> It looked that way from previous experimentation, but...
>
> >
> >
> > I just played the optimization
> > from printf("helloworld\n") to puts("helloworld").
> >
> > https://godbolt.org/z/5s4ded
> >
> >
> > -fno-builtin-puts cannot prevent clang
> > from emitting puts.
> > Is it because clang does not support
> > -fno-builtin-puts?
>
> Ugh. clang doesn't have __builtin_puts() but it optimizes printf() into
> puts(). It doesn't have __builtin_putchar() but will optimize
> printf("c") into putchar('c').
Bah, merely a <strikethrough>flesh
wound</strikethrough><strikethrough>compiler bug</strikethrough>rather
long TODO in the compiler.
https://github.com/llvm/llvm-project/blob/be2bc7d4cef2edd66c7fb74b70adf62fc68754db/clang/include/clang/Basic/Builtins.def#L943
Anyways, give me a week and I'll hack through the rest of them
https://reviews.llvm.org/D86508. Certainly made HPA's point hit home,
that's a lot of functionality to implement or disable in an
environment.
Masahiro, are you implying that we shouldn't take the
-fno-builtin-stpcpy patch, because Clang is inconsistent? (That can be
fixed.) Even though -fno-builtin-stpcpy works here as intended?
https://lore.kernel.org/lkml/[email protected]/
Otherwise we need to provide an implementation of this symbol in the kernel.
https://lore.kernel.org/lkml/[email protected]/
Please, pick your poison.
--
Thanks,
~Nick Desaulniers
On Tue, Aug 25, 2020 at 12:10 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Mon, Aug 24, 2020 at 10:34 AM Arvind Sankar <[email protected]> wrote:
> >
> > On Tue, Aug 25, 2020 at 12:57:22AM +0900, Masahiro Yamada wrote:
> > >
> > >
> > > To prevent transformation from foo() into bar(),
> > > there are two ways in Clang to do that;
> > > -fno-builtin-foo, and -fno-builtin-bar.
> > > There is only one in GCC; -fno-buitin-foo.
> > >
> > > Is this correct?
> > >
> >
> > It looked that way from previous experimentation, but...
> >
> > >
> > >
> > > I just played the optimization
> > > from printf("helloworld\n") to puts("helloworld").
> > >
> > > https://godbolt.org/z/5s4ded
> > >
> > >
> > > -fno-builtin-puts cannot prevent clang
> > > from emitting puts.
> > > Is it because clang does not support
> > > -fno-builtin-puts?
> >
> > Ugh. clang doesn't have __builtin_puts() but it optimizes printf() into
> > puts(). It doesn't have __builtin_putchar() but will optimize
> > printf("c") into putchar('c').
>
> Bah, merely a <strikethrough>flesh
> wound</strikethrough><strikethrough>compiler bug</strikethrough>rather
> long TODO in the compiler.
> https://github.com/llvm/llvm-project/blob/be2bc7d4cef2edd66c7fb74b70adf62fc68754db/clang/include/clang/Basic/Builtins.def#L943
>
> Anyways, give me a week and I'll hack through the rest of them
> https://reviews.llvm.org/D86508. Certainly made HPA's point hit home,
> that's a lot of functionality to implement or disable in an
> environment.
>
> Masahiro, are you implying that we shouldn't take the
> -fno-builtin-stpcpy patch, because Clang is inconsistent? (That can be
> fixed.) Even though -fno-builtin-stpcpy works here as intended?
> https://lore.kernel.org/lkml/[email protected]/
Sorry, the above link ^ should be this hunk (beyond tired, getting up
in 4.5hrs for plumbers):
diff --git a/Makefile b/Makefile
index c4470a4e131f..6a08cdfa58ae 100644
--- a/Makefile
+++ b/Makefile
@@ -577,6 +577,7 @@ ifneq ($(LLVM_IAS),1)
CLANG_FLAGS += -no-integrated-as
endif
CLANG_FLAGS += -Werror=unknown-warning-option
+CLANG_FLAGS += -fno-builtin-stpcpy
KBUILD_CFLAGS += $(CLANG_FLAGS)
KBUILD_AFLAGS += $(CLANG_FLAGS)
export CLANG_FLAGS
>
> Otherwise we need to provide an implementation of this symbol in the kernel.
> https://lore.kernel.org/lkml/[email protected]/
>
> Please, pick your poison.
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Tue, Aug 25, 2020 at 4:10 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Mon, Aug 24, 2020 at 10:34 AM Arvind Sankar <[email protected]> wrote:
> >
> > On Tue, Aug 25, 2020 at 12:57:22AM +0900, Masahiro Yamada wrote:
> > >
> > >
> > > To prevent transformation from foo() into bar(),
> > > there are two ways in Clang to do that;
> > > -fno-builtin-foo, and -fno-builtin-bar.
> > > There is only one in GCC; -fno-buitin-foo.
> > >
> > > Is this correct?
> > >
> >
> > It looked that way from previous experimentation, but...
> >
> > >
> > >
> > > I just played the optimization
> > > from printf("helloworld\n") to puts("helloworld").
> > >
> > > https://godbolt.org/z/5s4ded
> > >
> > >
> > > -fno-builtin-puts cannot prevent clang
> > > from emitting puts.
> > > Is it because clang does not support
> > > -fno-builtin-puts?
> >
> > Ugh. clang doesn't have __builtin_puts() but it optimizes printf() into
> > puts(). It doesn't have __builtin_putchar() but will optimize
> > printf("c") into putchar('c').
>
> Bah, merely a <strikethrough>flesh
> wound</strikethrough><strikethrough>compiler bug</strikethrough>rather
> long TODO in the compiler.
> https://github.com/llvm/llvm-project/blob/be2bc7d4cef2edd66c7fb74b70adf62fc68754db/clang/include/clang/Basic/Builtins.def#L943
>
> Anyways, give me a week and I'll hack through the rest of them
> https://reviews.llvm.org/D86508. Certainly made HPA's point hit home,
> that's a lot of functionality to implement or disable in an
> environment.
>
> Masahiro, are you implying that we shouldn't take the
> -fno-builtin-stpcpy patch, because Clang is inconsistent? (That can be
> fixed.) Even though -fno-builtin-stpcpy works here as intended?
> https://lore.kernel.org/lkml/[email protected]/
>
> Otherwise we need to provide an implementation of this symbol in the kernel.
> https://lore.kernel.org/lkml/[email protected]/
>
> Please, pick your poison.
I am not a compiler expert.
Nor am I sure if I am the right person who makes this decision.
But, if so, I would choose the latter.
(implement stpcpy() in the kernel)
I was addressed last night, so
I should write up my thoughts.
I do not think -fno-builtin-stpcpy is a
general solution.
-fno-builtin-stpcpy will work for now
because only Clang implements the transformation
from 'sprintf(dest, "%s", str)' into
'stpcpy(dest, str) - dest'.
If GCC implements it some day,
we would run into a problem because
in GCC, it is not -fno-builtin-stpcpy, but
-fno-builtin-sprintf that disables that optimization.
In this regard, 'KBUILD_CFLAGS += -fno-builtin-sprintf'
would be more future-proof, but it is potentially
an overkill.
We want to disable optimization from sprintf() to stpcpy(),
but we may still benefit from the optimization from
sprintf() into something else.
Linus is uncomfortable with this kind of compiler magic.
If we take compiler's freedom away,
-ffreestanding is a big hammer to solve this problem.
If we welcome the compiler's optimization,
we should implement stpcpy(), bcmp(), and whatever
until we solve all link errors.
--
Best Regards
Masahiro Yamada
On Tue, Aug 25, 2020 at 5:30 AM Masahiro Yamada <[email protected]> wrote:
>
> On Tue, Aug 25, 2020 at 4:10 PM Nick Desaulniers
> <[email protected]> wrote:
> >
> > Masahiro, are you implying that we shouldn't take the
> > -fno-builtin-stpcpy patch, because Clang is inconsistent? (That can be
> > fixed.) Even though -fno-builtin-stpcpy works here as intended?
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Otherwise we need to provide an implementation of this symbol in the kernel.
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Please, pick your poison.
>
>
>
> I am not a compiler expert.
>
> Nor am I sure if I am the right person who makes this decision.
> But, if so, I would choose the latter.
> (implement stpcpy() in the kernel)
>
> I was addressed last night, so
> I should write up my thoughts.
>
> I do not think -fno-builtin-stpcpy is a
> general solution.
>
> -fno-builtin-stpcpy will work for now
> because only Clang implements the transformation
> from 'sprintf(dest, "%s", str)' into
> 'stpcpy(dest, str) - dest'.
>
> If GCC implements it some day,
> we would run into a problem because
> in GCC, it is not -fno-builtin-stpcpy, but
> -fno-builtin-sprintf that disables that optimization.
>
> In this regard, 'KBUILD_CFLAGS += -fno-builtin-sprintf'
> would be more future-proof, but it is potentially
> an overkill.
> We want to disable optimization from sprintf() to stpcpy(),
> but we may still benefit from the optimization from
> sprintf() into something else.
>
>
> Linus is uncomfortable with this kind of compiler magic.
> If we take compiler's freedom away,
> -ffreestanding is a big hammer to solve this problem.
>
> If we welcome the compiler's optimization,
> we should implement stpcpy(), bcmp(), and whatever
> until we solve all link errors.
Speculating that -ffreestanding is untenable, submitted v3:
https://lore.kernel.org/lkml/[email protected]/T/#u
--
Thanks,
~Nick Desaulniers
On Tue, Aug 25, 2020 at 11:02 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Tue, Aug 25, 2020 at 5:30 AM Masahiro Yamada <[email protected]> wrote:
> >
> > On Tue, Aug 25, 2020 at 4:10 PM Nick Desaulniers
> > <[email protected]> wrote:
> > >
> > > Masahiro, are you implying that we shouldn't take the
> > > -fno-builtin-stpcpy patch, because Clang is inconsistent? (That can be
> > > fixed.) Even though -fno-builtin-stpcpy works here as intended?
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > Otherwise we need to provide an implementation of this symbol in the kernel.
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > Please, pick your poison.
> >
> >
> >
> > I am not a compiler expert.
> >
> > Nor am I sure if I am the right person who makes this decision.
> > But, if so, I would choose the latter.
> > (implement stpcpy() in the kernel)
> >
> > I was addressed last night, so
> > I should write up my thoughts.
> >
> > I do not think -fno-builtin-stpcpy is a
> > general solution.
> >
> > -fno-builtin-stpcpy will work for now
> > because only Clang implements the transformation
> > from 'sprintf(dest, "%s", str)' into
> > 'stpcpy(dest, str) - dest'.
> >
> > If GCC implements it some day,
> > we would run into a problem because
> > in GCC, it is not -fno-builtin-stpcpy, but
> > -fno-builtin-sprintf that disables that optimization.
> >
> > In this regard, 'KBUILD_CFLAGS += -fno-builtin-sprintf'
> > would be more future-proof, but it is potentially
> > an overkill.
> > We want to disable optimization from sprintf() to stpcpy(),
> > but we may still benefit from the optimization from
> > sprintf() into something else.
> >
> >
> > Linus is uncomfortable with this kind of compiler magic.
> > If we take compiler's freedom away,
> > -ffreestanding is a big hammer to solve this problem.
> >
> > If we welcome the compiler's optimization,
> > we should implement stpcpy(), bcmp(), and whatever
> > until we solve all link errors.
>
> Speculating that -ffreestanding is untenable, submitted v3:
> https://lore.kernel.org/lkml/[email protected]/T/#u
> --
> Thanks,
> ~Nick Desaulniers
FYI:
Andrew Morton picked up this patch.
--
Best Regards
Masahiro Yamada