archive mirror
 help / color / mirror / Atom feed
From: "Thomas Weißschuh" <>
To: Willy Tarreau <>
Cc: Zhangjin Wu <>,,,,,,,
Subject: Re: [PATCH] selftests/nolibc: Fix up compile error for rv32
Date: Sat, 20 May 2023 16:07:34 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

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:
> 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.


  reply	other threads:[~2023-05-20 14:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-20  9:53 [PATCH] tools/nolibc: riscv: add stackprotector support Thomas Weißschuh
2023-05-20 12:02 ` [PATCH] selftests/nolibc: Fix up compile error for rv32 Zhangjin Wu
2023-05-20 13:32   ` Willy Tarreau
2023-05-20 14:07     ` Thomas Weißschuh [this message]
2023-05-20 14:13       ` Willy Tarreau
2023-05-20 14:00   ` Thomas Weißschuh
2023-05-20 14:09     ` Willy Tarreau
2023-05-20 18:30       ` Zhangjin Wu
2023-05-21  3:58         ` Willy Tarreau
2023-05-21 18:08           ` Zhangjin Wu
2023-05-23 18:03             ` Zhangjin Wu
2023-05-23 18:56               ` Willy Tarreau
2023-05-20 13:52 ` Zhangjin Wu

Reply instructions:

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

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

  Avoid top-posting and favor interleaved quoting:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \ \

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