All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willy Tarreau <w@1wt.eu>
To: Zhangjin Wu <falcon@tinylab.org>, linux@weissschuh.net
Cc: 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
Date: Sat, 20 May 2023 15:32:37 +0200	[thread overview]
Message-ID: <20230520133237.GA27501@1wt.eu> (raw)
In-Reply-To: <20230520120254.66315-1-falcon@tinylab.org>

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.

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

Regards,
Willy

WARNING: multiple messages have this Message-ID (diff)
From: Willy Tarreau <w@1wt.eu>
To: Zhangjin Wu <falcon@tinylab.org>, linux@weissschuh.net
Cc: 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
Date: Sat, 20 May 2023 15:32:37 +0200	[thread overview]
Message-ID: <20230520133237.GA27501@1wt.eu> (raw)
In-Reply-To: <20230520120254.66315-1-falcon@tinylab.org>

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.

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

Regards,
Willy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2023-05-20 13:32 UTC|newest]

Thread overview: 26+ 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  9:53 ` Thomas Weißschuh
2023-05-20 12:02 ` [PATCH] selftests/nolibc: Fix up compile error for rv32 Zhangjin Wu
2023-05-20 12:02   ` Zhangjin Wu
2023-05-20 13:32   ` Willy Tarreau [this message]
2023-05-20 13:32     ` Willy Tarreau
2023-05-20 14:07     ` Thomas Weißschuh
2023-05-20 14:07       ` Thomas Weißschuh
2023-05-20 14:13       ` Willy Tarreau
2023-05-20 14:13         ` Willy Tarreau
2023-05-20 14:00   ` Thomas Weißschuh
2023-05-20 14:00     ` Thomas Weißschuh
2023-05-20 14:09     ` Willy Tarreau
2023-05-20 14:09       ` Willy Tarreau
2023-05-20 18:30       ` Zhangjin Wu
2023-05-20 18:30         ` Zhangjin Wu
2023-05-21  3:58         ` Willy Tarreau
2023-05-21  3:58           ` Willy Tarreau
2023-05-21 18:08           ` Zhangjin Wu
2023-05-21 18:08             ` Zhangjin Wu
2023-05-23 18:03             ` Zhangjin Wu
2023-05-23 18:03               ` Zhangjin Wu
2023-05-23 18:56               ` Willy Tarreau
2023-05-23 18:56                 ` Willy Tarreau
2023-05-20 13:52 ` Zhangjin Wu
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20230520133237.GA27501@1wt.eu \
    --to=w@1wt.eu \
    --cc=aou@eecs.berkeley.edu \
    --cc=falcon@tinylab.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux@weissschuh.net \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.