On 2023-05-23 22:12:38+0200, Willy Tarreau wrote:
> Hi Thomas, Zhangjin,
>
> I merged and pushed this series on top of the previous series, it's in
> branch 20230523-nolibc-rv32+stkp3.
>
> However, Thomas, I found an issue with this last patch that I partially
> reverted in a last patch I pushed as well so that we can discuss it:
>
> On Sun, May 21, 2023 at 11:36:35AM +0200, Thomas Weißschuh wrote:
> >
> > -CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \
> > - $(call cc-option,-mstack-protector-guard=global) \
> > - $(call cc-option,-fstack-protector-all)
> > -CFLAGS_STKP_i386 = $(CFLAGS_STACKPROTECTOR)
> > -CFLAGS_STKP_x86_64 = $(CFLAGS_STACKPROTECTOR)
> > -CFLAGS_STKP_x86 = $(CFLAGS_STACKPROTECTOR)
> > -CFLAGS_STKP_arm64 = $(CFLAGS_STACKPROTECTOR)
> > -CFLAGS_STKP_arm = $(CFLAGS_STACKPROTECTOR)
> > -CFLAGS_STKP_mips = $(CFLAGS_STACKPROTECTOR)
> > -CFLAGS_STKP_riscv = $(CFLAGS_STACKPROTECTOR)
> > -CFLAGS_STKP_loongarch = $(CFLAGS_STACKPROTECTOR)
> > +CFLAGS_STACKPROTECTOR = $(call cc-option,-mstack-protector-guard=global -fstack-protector-all)
> > CFLAGS_s390 = -m64
> > CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \
> > $(call cc-option,-fno-stack-protector) \
> > + $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) \
> > $(CFLAGS_STKP_$(ARCH)) $(CFLAGS_$(ARCH))
>
> By doing so, we reintroduce the forced stack-protector mechanism
> that cannot be disabled anymore. The ability to disable it was the
> main point of the options above. In fact checking __SSP__* was a
> solution to save the user from having to set -DNOLIBC_STACKPROTECTOR
> to match their compiler's flags, but here in the nolibc-test we still
> want to be able to forcefully disable stkp for a run (at least to
> unbreak gcc-9, and possibly to confirm that non-stkp builds still
> continue to work when your local toolchain has it by default).
Wouldn't this work?
make CFLAGS_x86=-fno-stack-protector nolibc-test
Or we do
CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all))
CFLAGS ?= ... $(CFLAGS_STACKPROTECTOR)
And then it is always:
make CFLAGS_STACKPROTECTOR= nolibc-test
An alternative would also be to use a GCC 9 compatible mechanism:
#if __has_attribute(no_stack_protector)
#define __no_stack_protector __attribute__((no_stack_protector))
#else
#define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector")))
#endif
Or we combine CFLAGS_STACKPROTECTOR and __no_stack_protector.
> So I reverted that part and only dropped -DNOLIBC_STACKPROTECTOR.
> This way I could run the test on all archs with gcc-9 by forcing
> CFLAGS_STACKPROTECTOR= and verified it was still possible to
> disable it for a specific arch only by setting CFLAGS_STKP_$ARCH.
>
> If you're fine with this we can squash this one into your cleanup
> patch.
To be honest I was happy to get rid of all these architecture specific
variables.
> Zhangjin I think this branch should work well enough for you to
> serve as a base for your upcoming series. We'll clean it up later
> once we all agree on the stat() replacement for syscall() and on
> the various remaining points including your time32 alternatives.
>
> Thanks to you both,
> Willy
>
> PS: we're still carrying a long CC list of individuals who are likely
> not that much interested in our discussion, I propose that we trim
> it on next exchanges and only keep us 3 and the lists, in order to
> remove some of their e-mail pollution.
I trimmed the list a bit.
Thomas
