Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 20 May 2023 16:07:34 +0200
From:   Thomas =?utf-8?Q?Wei=C3=9Fschuh?= <thomas@t-8ch.de>
To:     Willy Tarreau <w@1wt.eu>
Cc:     Zhangjin Wu <falcon@tinylab.org>, aou@eecs.berkeley.edu,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-riscv@lists.infradead.org, palmer@dabbelt.com,
        paul.walmsley@sifive.com, shuah@kernel.org
Subject: Re: [PATCH] selftests/nolibc: Fix up compile error for rv32
Message-ID: <d98dfd41-8938-4388-90f0-38199d2f0574@t-8ch.de>
References: <20230520-nolibc-stackprotector-riscv-v1-1-d8912012a034@weissschuh.net>
 <20230520120254.66315-1-falcon@tinylab.org>
 <20230520133237.GA27501@1wt.eu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230520133237.GA27501@1wt.eu>
Precedence: bulk

Hi Willy!

On 2023-05-20 15:32:37+0200, Willy Tarreau wrote:
> Thomas, Zhangjin,
> 
> I've merged your latest patches in my branch 20230520-nolibc-rv32+stkp2,
> which was rebased to integrate the updated commit messages and a few
> missing s-o-b from mine. Please have a look:
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git
> 
> However, Thomas, I noticed something puzzling me. While I tested with
> gcc-9.5 (that I have here along my toolchains) I found that it would
> systematically fail:
> 
>   sysroot/x86/include/stackprotector.h:46:1: warning: 'no_stack_protector' attribute directive ignored [-Wattributes]
>      46 | {
>         | ^
>   !!Stack smashing detected!!
>   qemu: uncaught target signal 6 (Aborted) - core dumped
>   0 test(s) passed.
> 
> The reason is that it doesn't support the attribute "no_stack_protector".
> Upon closer investigation, I noticed that _start() on x86_64 doens't have
> it, yet it works on more recent compilers! So I don't understand why it
> works with more recent compilers.

_start() not having the attribute is indeed an oversight.
No idea how it worked before.

> I managed to avoid the crash by enclosing the __stack_chk_init() function
> in a #pragma GCC optimize("-fno-stack-protector") while removing the
> attribute (though Clang and more recent gcc use this attribute so we
> shouldn't completely drop it either).

I would like to first align x86 to __attribute__((no_stack_protector))
for uniformity and then figure out on how to make it nicer.

> I consider this non-critical as we can expect that regtests are run with
> a reasonably recent compiler version, but if in the long term we can find
> a more reliable detection for this, it would be nice.
> 
> For example I found that gcc defines __SSP_ALL__ to 1 when
> -fstack-protector is used, and 2 when -fstack-protector-all is used.
> With clang, it's 1 and 3 respectively. Maybe we should use that and
> drop NOLIBC_STACKPROTECTOR, that would be one less variable to deal
> with: the code would automatically adapt to whatever cflags the user
> sets on the compiler, which is generally better.

That sounds great!

I explicitly looked for something like this before, dumping preprocessor
directives and comparing.
It seems the fact that my compilers enable this feature by default made
me miss it.

I'll send patches.

Thomas