| Message ID | 20220117153348.2513798-1-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] Makefile: Tell compiler to generate bare-metal code | expand |
Hi Andre, On Mon, Jan 17, 2022 at 03:33:48PM +0000, Andre Przywara wrote: > Our GCC invocation does not provide many parameters, which lets the > toolchain fill in its own default setup. > In case of a native build or when using a full-featured cross-compiler, > this probably means Linux userland, which is not what we want for a > bare-metal application like boot-wrapper. > > Tell the compiler to forget about those standard settings, and only use > what we explicitly ask for. In particular that means to not use toolchain > provided libraries, since they might pull in more code than we want, and > might not run well in the boot-wrapper environment. Thanks for splitting this out; I'd like to apply this with two minor fixups (which I'll apply locally if you agree). > Disable the stack protector, as this adds code that relies on userland: > "If a guard check fails, an error message is printed and the program > exits." (from the gcc manpage). I think we should say: | Disable the stack protector, as this relies on support code, e.g. | a __stack_chk_guard variable and __stack_chk_fail function, which the | boot-wrapper does not implement. > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Hi, > > compared to [PATCH v2 3/9] this drops the more contentious options (for > now, at least), and focuses on what's really needed. Including > -fno-stack-protector, as the need for this showed up in the cleanup series > already. > > Cheers, > Andre > > Makefile.am | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Makefile.am b/Makefile.am > index f941b07..581840e 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -125,6 +125,8 @@ CHOSEN_NODE := chosen { \ > CPPFLAGS += $(INITRD_FLAGS) > CFLAGS += -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/ > CFLAGS += -Wall -fomit-frame-pointer > +CFLAGS += -ffreestanding -nostdlib # this is not for userland > +CFLAGS += -fno-stack-protector # no terminal to print For now, I'd like to drop these comments, since I think the "no terminal to print" comment is a bit confusing, and the "this is not for userland" comment is arguably the most obvious. Then as a future follow-up we can reoganise this for clarity: | # Build code suitable for a bare-metal environment which does not | # depend on any support libraries | CFLAGS += ffreestanding -nostdlib | | # Disable unnecessary features which require runtime support | CFLAGS += -fno-stack-protector | CFLAGS += ... | | # Disable unnecessary features which impact code-size | CFLAGS += -fomit-frame-pointer | CFLAGS += ... | | # Allow the linker to remove unused code and data to shrink the | # resulting binary | CFLAGS += -ffunction-sections -fdata-sections | LDFLAGS += --gc-sections > CFLAGS += -ffunction-sections -fdata-sections > CFLAGS += -fno-pic -fno-pie > LDFLAGS += --gc-sections > -- > 2.25.1 >
On Mon, 17 Jan 2022 16:47:20 +0000 Mark Rutland <mark.rutland@arm.com> wrote: > Hi Andre, > > On Mon, Jan 17, 2022 at 03:33:48PM +0000, Andre Przywara wrote: > > Our GCC invocation does not provide many parameters, which lets the > > toolchain fill in its own default setup. > > In case of a native build or when using a full-featured cross-compiler, > > this probably means Linux userland, which is not what we want for a > > bare-metal application like boot-wrapper. > > > > Tell the compiler to forget about those standard settings, and only use > > what we explicitly ask for. In particular that means to not use toolchain > > provided libraries, since they might pull in more code than we want, and > > might not run well in the boot-wrapper environment. > > Thanks for splitting this out; I'd like to apply this with two minor > fixups (which I'll apply locally if you agree). Yes, I am fine with those changes, please go ahead! > > Disable the stack protector, as this adds code that relies on userland: > > "If a guard check fails, an error message is printed and the program > > exits." (from the gcc manpage). > > I think we should say: > > | Disable the stack protector, as this relies on support code, e.g. > | a __stack_chk_guard variable and __stack_chk_fail function, which the > | boot-wrapper does not implement. > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > Hi, > > > > compared to [PATCH v2 3/9] this drops the more contentious options (for > > now, at least), and focuses on what's really needed. Including > > -fno-stack-protector, as the need for this showed up in the cleanup series > > already. > > > > Cheers, > > Andre > > > > Makefile.am | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/Makefile.am b/Makefile.am > > index f941b07..581840e 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -125,6 +125,8 @@ CHOSEN_NODE := chosen { \ > > CPPFLAGS += $(INITRD_FLAGS) > > CFLAGS += -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/ > > CFLAGS += -Wall -fomit-frame-pointer > > +CFLAGS += -ffreestanding -nostdlib # this is not for userland > > +CFLAGS += -fno-stack-protector # no terminal to print > > For now, I'd like to drop these comments, since I think the "no terminal > to print" comment is a bit confusing, and the "this is not for userland" > comment is arguably the most obvious. > > Then as a future follow-up we can reoganise this for clarity: > > | # Build code suitable for a bare-metal environment which does not > | # depend on any support libraries > | CFLAGS += ffreestanding -nostdlib > | > | # Disable unnecessary features which require runtime support > | CFLAGS += -fno-stack-protector > | CFLAGS += ... > | > | # Disable unnecessary features which impact code-size > | CFLAGS += -fomit-frame-pointer > | CFLAGS += ... > | > | # Allow the linker to remove unused code and data to shrink the > | # resulting binary > | CFLAGS += -ffunction-sections -fdata-sections > | LDFLAGS += --gc-sections Yes, that looks good to me as a plan. I agree that trying to fit meaningful comment to the rest of the line was not a great idea. Cheers, Andre > > > > CFLAGS += -ffunction-sections -fdata-sections > > CFLAGS += -fno-pic -fno-pie > > LDFLAGS += --gc-sections > > -- > > 2.25.1 > >
On Mon, Jan 17, 2022 at 04:53:10PM +0000, Andre Przywara wrote: > On Mon, 17 Jan 2022 16:47:20 +0000 > Mark Rutland <mark.rutland@arm.com> wrote: > > > Hi Andre, > > > > On Mon, Jan 17, 2022 at 03:33:48PM +0000, Andre Przywara wrote: > > > Our GCC invocation does not provide many parameters, which lets the > > > toolchain fill in its own default setup. > > > In case of a native build or when using a full-featured cross-compiler, > > > this probably means Linux userland, which is not what we want for a > > > bare-metal application like boot-wrapper. > > > > > > Tell the compiler to forget about those standard settings, and only use > > > what we explicitly ask for. In particular that means to not use toolchain > > > provided libraries, since they might pull in more code than we want, and > > > might not run well in the boot-wrapper environment. > > > > Thanks for splitting this out; I'd like to apply this with two minor > > fixups (which I'll apply locally if you agree). > > Yes, I am fine with those changes, please go ahead! Thanks; I've applied those and pushed that out. Mark.
On Mon, Jan 17, 2022 at 03:33:48PM +0000, Andre Przywara wrote: > Our GCC invocation does not provide many parameters, which lets the > toolchain fill in its own default setup. > In case of a native build or when using a full-featured cross-compiler, > this probably means Linux userland, which is not what we want for a > bare-metal application like boot-wrapper. > > Tell the compiler to forget about those standard settings, and only use > what we explicitly ask for. In particular that means to not use toolchain > provided libraries, since they might pull in more code than we want, and > might not run well in the boot-wrapper environment. > > Disable the stack protector, as this adds code that relies on userland: > "If a guard check fails, an error message is printed and the program > exits." (from the gcc manpage). > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Hi, > > compared to [PATCH v2 3/9] this drops the more contentious options (for > now, at least), and focuses on what's really needed. Including > -fno-stack-protector, as the need for this showed up in the cleanup series > already. There is history here. -ffreestanding has been tried before. See: https://patchwork.kernel.org/project/linux-kbuild/patch/20200817220212.338670-5-ndesaulniers@google.com/ Have the issues in that thread been addressed? If not, -ffreestanding does not belong in the top-level makefile.
On Mon, Jan 17, 2022 at 05:49:43PM +0000, Russell King (Oracle) wrote: > On Mon, Jan 17, 2022 at 03:33:48PM +0000, Andre Przywara wrote: > > Our GCC invocation does not provide many parameters, which lets the > > toolchain fill in its own default setup. > > In case of a native build or when using a full-featured cross-compiler, > > this probably means Linux userland, which is not what we want for a > > bare-metal application like boot-wrapper. > > > > Tell the compiler to forget about those standard settings, and only use > > what we explicitly ask for. In particular that means to not use toolchain > > provided libraries, since they might pull in more code than we want, and > > might not run well in the boot-wrapper environment. > > > > Disable the stack protector, as this adds code that relies on userland: > > "If a guard check fails, an error message is printed and the program > > exits." (from the gcc manpage). > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > Hi, > > > > compared to [PATCH v2 3/9] this drops the more contentious options (for > > now, at least), and focuses on what's really needed. Including > > -fno-stack-protector, as the need for this showed up in the cleanup series > > already. > > There is history here. -ffreestanding has been tried before. See: > > https://patchwork.kernel.org/project/linux-kbuild/patch/20200817220212.338670-5-ndesaulniers@google.com/ > > Have the issues in that thread been addressed? > > If not, -ffreestanding does not belong in the top-level makefile. Oh, this is not the kernel. Please make it clear as you're posting to the linux-arm-kernel mailing list...
On Mon, 17 Jan 2022 17:49:43 +0000 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: Hi Russell, > On Mon, Jan 17, 2022 at 03:33:48PM +0000, Andre Przywara wrote: > > Our GCC invocation does not provide many parameters, which lets the > > toolchain fill in its own default setup. > > In case of a native build or when using a full-featured cross-compiler, > > this probably means Linux userland, which is not what we want for a > > bare-metal application like boot-wrapper. > > > > Tell the compiler to forget about those standard settings, and only use > > what we explicitly ask for. In particular that means to not use toolchain > > provided libraries, since they might pull in more code than we want, and > > might not run well in the boot-wrapper environment. > > > > Disable the stack protector, as this adds code that relies on userland: > > "If a guard check fails, an error message is printed and the program > > exits." (from the gcc manpage). > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > Hi, > > > > compared to [PATCH v2 3/9] this drops the more contentious options (for > > now, at least), and focuses on what's really needed. Including > > -fno-stack-protector, as the need for this showed up in the cleanup series > > already. > > There is history here. -ffreestanding has been tried before. See: > > https://patchwork.kernel.org/project/linux-kbuild/patch/20200817220212.338670-5-ndesaulniers@google.com/ Thanks for the link, interesting insight. > Have the issues in that thread been addressed? My apologies, I just see that I accidentally dropped the "boot-wrapper" part from the subject line. So this is not a Linux patch, we just use the mailing list for boot-wrapper discussion as well. As the boot-wrapper is a really minimal and small code base, I don't think we bother too much about lost optimisation opportunities, instead want to become more robust. At the moment we really don't use any options telling the compiler to not use the standard library (which caused issues already), so we'd like to play safe here. Thanks, Andre > If not, -ffreestanding does not belong in the top-level makefile.
diff --git a/Makefile.am b/Makefile.am index f941b07..581840e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -125,6 +125,8 @@ CHOSEN_NODE := chosen { \ CPPFLAGS += $(INITRD_FLAGS) CFLAGS += -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/ CFLAGS += -Wall -fomit-frame-pointer +CFLAGS += -ffreestanding -nostdlib # this is not for userland +CFLAGS += -fno-stack-protector # no terminal to print CFLAGS += -ffunction-sections -fdata-sections CFLAGS += -fno-pic -fno-pie LDFLAGS += --gc-sections
Our GCC invocation does not provide many parameters, which lets the toolchain fill in its own default setup. In case of a native build or when using a full-featured cross-compiler, this probably means Linux userland, which is not what we want for a bare-metal application like boot-wrapper. Tell the compiler to forget about those standard settings, and only use what we explicitly ask for. In particular that means to not use toolchain provided libraries, since they might pull in more code than we want, and might not run well in the boot-wrapper environment. Disable the stack protector, as this adds code that relies on userland: "If a guard check fails, an error message is printed and the program exits." (from the gcc manpage). Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- Hi, compared to [PATCH v2 3/9] this drops the more contentious options (for now, at least), and focuses on what's really needed. Including -fno-stack-protector, as the need for this showed up in the cleanup series already. Cheers, Andre Makefile.am | 2 ++ 1 file changed, 2 insertions(+)