* [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO @ 2023-02-10 21:51 Andrea Righi 2023-02-10 22:43 ` Kees Cook 0 siblings, 1 reply; 10+ messages in thread From: Andrea Righi @ 2023-02-10 21:51 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho Cc: Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes -ftrivial-auto-var-init=zero to clang, that triggers the following error: error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang' However, this additional option that is currently required by clang is going to be removed in the future (as the name of the option suggests), likely with clang-17. So, make sure bindgen is using this extra option if the major version of the libclang used by bindgen is < 17. In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST without triggering any build error. Link: https://github.com/llvm/llvm-project/issues/44842 Signed-off-by: Andrea Righi <andrea.righi@canonical.com> --- Changes in v2: - check the version of libclang used by bindgen to determine if we need to pass the additional clang option rust/Makefile | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/rust/Makefile b/rust/Makefile index ff70c4c916f8..c77d7ce96a85 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -269,6 +269,19 @@ BINDGEN_TARGET := $(BINDGEN_TARGET_$(SRCARCH)) # some configurations, with new GCC versions, etc. bindgen_extra_c_flags = -w --target=$(BINDGEN_TARGET) +# Auto variable zero-initialization requires an additional special option with +# clang that is going to be removed sometimes in the future (likely in +# clang-17), so make sure to pass this option only if clang supports it +# (libclang major version < 17). +# +# https://github.com/llvm/llvm-project/issues/44842 +ifdef CONFIG_INIT_STACK_ALL_ZERO +libclang_maj_ver=$(shell $(BINDGEN) $(srctree)/scripts/rust_is_available_bindgen_libclang.h 2>&1 | sed -ne 's/.*clang version \([0-9]*\).*/\1/p') +ifeq ($(shell expr $(libclang_maj_ver) \< 17), 1) +bindgen_extra_c_flags += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang +endif +endif + bindgen_c_flags = $(filter-out $(bindgen_skip_c_flags), $(c_flags)) \ $(bindgen_extra_c_flags) endif -- 2.38.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-02-10 21:51 [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO Andrea Righi @ 2023-02-10 22:43 ` Kees Cook 2023-02-11 9:04 ` Andrea Righi 0 siblings, 1 reply; 10+ messages in thread From: Kees Cook @ 2023-02-10 22:43 UTC (permalink / raw) To: Andrea Righi, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho Cc: Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On February 10, 2023 1:51:41 PM PST, Andrea Righi <andrea.righi@canonical.com> wrote: >With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes >-ftrivial-auto-var-init=zero to clang, that triggers the following >error: > > error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang' > >However, this additional option that is currently required by clang is >going to be removed in the future (as the name of the option suggests), >likely with clang-17. > >So, make sure bindgen is using this extra option if the major version of >the libclang used by bindgen is < 17. > >In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST >without triggering any build error. > >Link: https://github.com/llvm/llvm-project/issues/44842 >Signed-off-by: Andrea Righi <andrea.righi@canonical.com> >--- > >Changes in v2: > - check the version of libclang used by bindgen to determine if we need > to pass the additional clang option > > rust/Makefile | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > >diff --git a/rust/Makefile b/rust/Makefile >index ff70c4c916f8..c77d7ce96a85 100644 >--- a/rust/Makefile >+++ b/rust/Makefile >@@ -269,6 +269,19 @@ BINDGEN_TARGET := $(BINDGEN_TARGET_$(SRCARCH)) > # some configurations, with new GCC versions, etc. > bindgen_extra_c_flags = -w --target=$(BINDGEN_TARGET) > >+# Auto variable zero-initialization requires an additional special option with >+# clang that is going to be removed sometimes in the future (likely in >+# clang-17), so make sure to pass this option only if clang supports it >+# (libclang major version < 17). >+# >+# https://github.com/llvm/llvm-project/issues/44842 >+ifdef CONFIG_INIT_STACK_ALL_ZERO >+libclang_maj_ver=$(shell $(BINDGEN) $(srctree)/scripts/rust_is_available_bindgen_libclang.h 2>&1 | sed -ne 's/.*clang version \([0-9]*\).*/\1/p') >+ifeq ($(shell expr $(libclang_maj_ver) \< 17), 1) >+bindgen_extra_c_flags += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang >+endif >+endif This logic already exists in the top-level Makefile. How does -ftrivial-auto-var-init=zero make it into bindgen_c_flags normally? (I.e. why does the legacy -enable... option not?) >+ > bindgen_c_flags = $(filter-out $(bindgen_skip_c_flags), $(c_flags)) \ > $(bindgen_extra_c_flags) > endif -- Kees Cook ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-02-10 22:43 ` Kees Cook @ 2023-02-11 9:04 ` Andrea Righi 2023-02-11 13:44 ` Kees Cook 0 siblings, 1 reply; 10+ messages in thread From: Andrea Righi @ 2023-02-11 9:04 UTC (permalink / raw) To: Kees Cook Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On Fri, Feb 10, 2023 at 02:43:56PM -0800, Kees Cook wrote: > On February 10, 2023 1:51:41 PM PST, Andrea Righi <andrea.righi@canonical.com> wrote: > >With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes > >-ftrivial-auto-var-init=zero to clang, that triggers the following > >error: > > > > error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang' > > > >However, this additional option that is currently required by clang is > >going to be removed in the future (as the name of the option suggests), > >likely with clang-17. > > > >So, make sure bindgen is using this extra option if the major version of > >the libclang used by bindgen is < 17. > > > >In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST > >without triggering any build error. > > > >Link: https://github.com/llvm/llvm-project/issues/44842 > >Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > >--- > > > >Changes in v2: > > - check the version of libclang used by bindgen to determine if we need > > to pass the additional clang option > > > > rust/Makefile | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > >diff --git a/rust/Makefile b/rust/Makefile > >index ff70c4c916f8..c77d7ce96a85 100644 > >--- a/rust/Makefile > >+++ b/rust/Makefile > >@@ -269,6 +269,19 @@ BINDGEN_TARGET := $(BINDGEN_TARGET_$(SRCARCH)) > > # some configurations, with new GCC versions, etc. > > bindgen_extra_c_flags = -w --target=$(BINDGEN_TARGET) > > > >+# Auto variable zero-initialization requires an additional special option with > >+# clang that is going to be removed sometimes in the future (likely in > >+# clang-17), so make sure to pass this option only if clang supports it > >+# (libclang major version < 17). > >+# > >+# https://github.com/llvm/llvm-project/issues/44842 > >+ifdef CONFIG_INIT_STACK_ALL_ZERO > >+libclang_maj_ver=$(shell $(BINDGEN) $(srctree)/scripts/rust_is_available_bindgen_libclang.h 2>&1 | sed -ne 's/.*clang version \([0-9]*\).*/\1/p') > >+ifeq ($(shell expr $(libclang_maj_ver) \< 17), 1) > >+bindgen_extra_c_flags += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang > >+endif > >+endif > > This logic already exists in the top-level Makefile. How does -ftrivial-auto-var-init=zero make it into bindgen_c_flags normally? (I.e. why does the legacy -enable... option not?) If we're using gcc, the top-level Makefile doesn't set the clang options, so in this case CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO_ENABLER is undefined. But then bindgen always relies on libclang to parse C, even if gcc is used globally, therefore it needs the extra clang flag. Ideally it'd be nice if bindgen would support a gcc backend, but until then we need to do something special here, like this kind of duplicated logic... -Andrea > > >+ > > bindgen_c_flags = $(filter-out $(bindgen_skip_c_flags), $(c_flags)) \ > > $(bindgen_extra_c_flags) > > endif > > > -- > Kees Cook ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-02-11 9:04 ` Andrea Righi @ 2023-02-11 13:44 ` Kees Cook 2023-02-11 14:35 ` Andrea Righi 0 siblings, 1 reply; 10+ messages in thread From: Kees Cook @ 2023-02-11 13:44 UTC (permalink / raw) To: Andrea Righi Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On February 11, 2023 1:04:16 AM PST, Andrea Righi <andrea.righi@canonical.com> wrote: >On Fri, Feb 10, 2023 at 02:43:56PM -0800, Kees Cook wrote: >> On February 10, 2023 1:51:41 PM PST, Andrea Righi <andrea.righi@canonical.com> wrote: >> >With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes >> >-ftrivial-auto-var-init=zero to clang, that triggers the following >> >error: >> > >> > error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang' >> > >> >However, this additional option that is currently required by clang is >> >going to be removed in the future (as the name of the option suggests), >> >likely with clang-17. >> > >> >So, make sure bindgen is using this extra option if the major version of >> >the libclang used by bindgen is < 17. >> > >> >In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST >> >without triggering any build error. >> > >> >Link: https://github.com/llvm/llvm-project/issues/44842 >> >Signed-off-by: Andrea Righi <andrea.righi@canonical.com> >> >--- >> > >> >Changes in v2: >> > - check the version of libclang used by bindgen to determine if we need >> > to pass the additional clang option >> > >> > rust/Makefile | 13 +++++++++++++ >> > 1 file changed, 13 insertions(+) >> > >> >diff --git a/rust/Makefile b/rust/Makefile >> >index ff70c4c916f8..c77d7ce96a85 100644 >> >--- a/rust/Makefile >> >+++ b/rust/Makefile >> >@@ -269,6 +269,19 @@ BINDGEN_TARGET := $(BINDGEN_TARGET_$(SRCARCH)) >> > # some configurations, with new GCC versions, etc. >> > bindgen_extra_c_flags = -w --target=$(BINDGEN_TARGET) >> > >> >+# Auto variable zero-initialization requires an additional special option with >> >+# clang that is going to be removed sometimes in the future (likely in >> >+# clang-17), so make sure to pass this option only if clang supports it >> >+# (libclang major version < 17). >> >+# >> >+# https://github.com/llvm/llvm-project/issues/44842 >> >+ifdef CONFIG_INIT_STACK_ALL_ZERO >> >+libclang_maj_ver=$(shell $(BINDGEN) $(srctree)/scripts/rust_is_available_bindgen_libclang.h 2>&1 | sed -ne 's/.*clang version \([0-9]*\).*/\1/p') >> >+ifeq ($(shell expr $(libclang_maj_ver) \< 17), 1) >> >+bindgen_extra_c_flags += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang >> >+endif >> >+endif >> >> This logic already exists in the top-level Makefile. How does -ftrivial-auto-var-init=zero make it into bindgen_c_flags normally? (I.e. why does the legacy -enable... option not?) > >If we're using gcc, the top-level Makefile doesn't set the clang >options, so in this case CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO_ENABLER is >undefined. But then bindgen always relies on libclang to parse C, even >if gcc is used globally, therefore it needs the extra clang flag. Ah yes! Thank you, I keep forgetting about mixed compiler builds. > >Ideally it'd be nice if bindgen would support a gcc backend, but until >then we need to do something special here, like this kind of duplicated >logic... Right. Yeah, good fix. One nit: the -enable... flag is removed in Clang 16+: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags With that fixed: Reviewed-by: Kees Cook <keescook@chromium.org> -- Kees Cook ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-02-11 13:44 ` Kees Cook @ 2023-02-11 14:35 ` Andrea Righi 2023-04-06 22:52 ` Miguel Ojeda 0 siblings, 1 reply; 10+ messages in thread From: Andrea Righi @ 2023-02-11 14:35 UTC (permalink / raw) To: Kees Cook Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On Sat, Feb 11, 2023 at 05:44:33AM -0800, Kees Cook wrote: > On February 11, 2023 1:04:16 AM PST, Andrea Righi <andrea.righi@canonical.com> wrote: > >On Fri, Feb 10, 2023 at 02:43:56PM -0800, Kees Cook wrote: > >> On February 10, 2023 1:51:41 PM PST, Andrea Righi <andrea.righi@canonical.com> wrote: > >> >With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes > >> >-ftrivial-auto-var-init=zero to clang, that triggers the following > >> >error: > >> > > >> > error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang' > >> > > >> >However, this additional option that is currently required by clang is > >> >going to be removed in the future (as the name of the option suggests), > >> >likely with clang-17. > >> > > >> >So, make sure bindgen is using this extra option if the major version of > >> >the libclang used by bindgen is < 17. > >> > > >> >In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST > >> >without triggering any build error. > >> > > >> >Link: https://github.com/llvm/llvm-project/issues/44842 > >> >Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > >> >--- > >> > > >> >Changes in v2: > >> > - check the version of libclang used by bindgen to determine if we need > >> > to pass the additional clang option > >> > > >> > rust/Makefile | 13 +++++++++++++ > >> > 1 file changed, 13 insertions(+) > >> > > >> >diff --git a/rust/Makefile b/rust/Makefile > >> >index ff70c4c916f8..c77d7ce96a85 100644 > >> >--- a/rust/Makefile > >> >+++ b/rust/Makefile > >> >@@ -269,6 +269,19 @@ BINDGEN_TARGET := $(BINDGEN_TARGET_$(SRCARCH)) > >> > # some configurations, with new GCC versions, etc. > >> > bindgen_extra_c_flags = -w --target=$(BINDGEN_TARGET) > >> > > >> >+# Auto variable zero-initialization requires an additional special option with > >> >+# clang that is going to be removed sometimes in the future (likely in > >> >+# clang-17), so make sure to pass this option only if clang supports it > >> >+# (libclang major version < 17). > >> >+# > >> >+# https://github.com/llvm/llvm-project/issues/44842 > >> >+ifdef CONFIG_INIT_STACK_ALL_ZERO > >> >+libclang_maj_ver=$(shell $(BINDGEN) $(srctree)/scripts/rust_is_available_bindgen_libclang.h 2>&1 | sed -ne 's/.*clang version \([0-9]*\).*/\1/p') > >> >+ifeq ($(shell expr $(libclang_maj_ver) \< 17), 1) > >> >+bindgen_extra_c_flags += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang > >> >+endif > >> >+endif > >> > >> This logic already exists in the top-level Makefile. How does -ftrivial-auto-var-init=zero make it into bindgen_c_flags normally? (I.e. why does the legacy -enable... option not?) > > > >If we're using gcc, the top-level Makefile doesn't set the clang > >options, so in this case CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO_ENABLER is > >undefined. But then bindgen always relies on libclang to parse C, even > >if gcc is used globally, therefore it needs the extra clang flag. > > Ah yes! Thank you, I keep forgetting about mixed compiler builds. > > > > >Ideally it'd be nice if bindgen would support a gcc backend, but until > >then we need to do something special here, like this kind of duplicated > >logic... > > Right. Yeah, good fix. One nit: the -enable... flag is removed in Clang 16+: oic, with clang-16 we get a deprecated warning (but it doesn't fail), then starting with clang-17 the -enable.. flag is not available anymore. Then I agree that a better check would be version < 16, instead of < 17. Thanks, -Andrea > > https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags > > With that fixed: > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > -- > Kees Cook ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-02-11 14:35 ` Andrea Righi @ 2023-04-06 22:52 ` Miguel Ojeda 2023-04-07 6:02 ` Andrea Righi 0 siblings, 1 reply; 10+ messages in thread From: Miguel Ojeda @ 2023-04-06 22:52 UTC (permalink / raw) To: Andrea Righi Cc: Kees Cook, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On Sat, Feb 11, 2023 at 3:35 PM Andrea Righi <andrea.righi@canonical.com> wrote: > > oic, with clang-16 we get a deprecated warning (but it doesn't fail), > then starting with clang-17 the -enable.. flag is not available anymore. Wait, Kees' link says it is deprecated in Clang 16, and the removal will happen in 18. > Then I agree that a better check would be version < 16, instead of < 17. Applied to `rust-fixes` with that changed. Please take a look if it is what you expected (both content and commit message): https://github.com/Rust-for-Linux/linux/commit/1921ea54ab0d91d7a660c5bb980fce1efc9223f9 Thanks! Cheers, Miguel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-04-06 22:52 ` Miguel Ojeda @ 2023-04-07 6:02 ` Andrea Righi 2023-04-09 20:49 ` Miguel Ojeda 0 siblings, 1 reply; 10+ messages in thread From: Andrea Righi @ 2023-04-07 6:02 UTC (permalink / raw) To: Miguel Ojeda Cc: Kees Cook, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On Fri, Apr 07, 2023 at 12:52:18AM +0200, Miguel Ojeda wrote: > On Sat, Feb 11, 2023 at 3:35 PM Andrea Righi <andrea.righi@canonical.com> wrote: > > > > oic, with clang-16 we get a deprecated warning (but it doesn't fail), > > then starting with clang-17 the -enable.. flag is not available anymore. > > Wait, Kees' link says it is deprecated in Clang 16, and the removal > will happen in 18. > > > Then I agree that a better check would be version < 16, instead of < 17. > > Applied to `rust-fixes` with that changed. Please take a look if it is > what you expected (both content and commit message): > https://github.com/Rust-for-Linux/linux/commit/1921ea54ab0d91d7a660c5bb980fce1efc9223f9 The check (< 16) looks correct and the comment also looks correct to me, this option will be removed in clang 18, as mentioned here: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags (Maybe we could add also this link as a reference) Thanks! -Andrea ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-04-07 6:02 ` Andrea Righi @ 2023-04-09 20:49 ` Miguel Ojeda 2023-04-11 7:11 ` Andrea Righi 0 siblings, 1 reply; 10+ messages in thread From: Miguel Ojeda @ 2023-04-09 20:49 UTC (permalink / raw) To: Andrea Righi Cc: Kees Cook, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On Fri, Apr 7, 2023 at 8:02 AM Andrea Righi <andrea.righi@canonical.com> wrote: > > The check (< 16) looks correct and the comment also looks correct to me, > this option will be removed in clang 18, as mentioned here: > https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags > > (Maybe we could add also this link as a reference) Thanks for taking a look -- Kees' link added! I removed "(as the name of the option suggests)" from the commit message, by the way, since the name of the option suggests the original `-ftrivial` option was to be removed, not the `-enable` one. If that understanding is wrong, please let me know! Cheers, Miguel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-04-09 20:49 ` Miguel Ojeda @ 2023-04-11 7:11 ` Andrea Righi 2023-04-11 11:17 ` Miguel Ojeda 0 siblings, 1 reply; 10+ messages in thread From: Andrea Righi @ 2023-04-11 7:11 UTC (permalink / raw) To: Miguel Ojeda Cc: Kees Cook, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On Sun, Apr 09, 2023 at 10:49:58PM +0200, Miguel Ojeda wrote: > On Fri, Apr 7, 2023 at 8:02 AM Andrea Righi <andrea.righi@canonical.com> wrote: > > > > The check (< 16) looks correct and the comment also looks correct to me, > > this option will be removed in clang 18, as mentioned here: > > https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags > > > > (Maybe we could add also this link as a reference) > > Thanks for taking a look -- Kees' link added! > > I removed "(as the name of the option suggests)" from the commit > message, by the way, since the name of the option suggests the > original `-ftrivial` option was to be removed, not the `-enable` one. > If that understanding is wrong, please let me know! Yes, that looks more clear, the name of the option is a bit misleading. :) To be clear what is going to be removed is -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang, in this way clang will be compatible with gcc and they can both use -ftrivial-auto-var-init=zero. Thanks, -Andrea > > Cheers, > Miguel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO 2023-04-11 7:11 ` Andrea Righi @ 2023-04-11 11:17 ` Miguel Ojeda 0 siblings, 0 replies; 10+ messages in thread From: Miguel Ojeda @ 2023-04-11 11:17 UTC (permalink / raw) To: Andrea Righi Cc: Kees Cook, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, bjorn3_gh, Kees Cook, rust-for-linux, linux-kernel On Tue, Apr 11, 2023 at 9:11 AM Andrea Righi <andrea.righi@canonical.com> wrote: > > Yes, that looks more clear, the name of the option is a bit misleading. :) > > To be clear what is going to be removed is > -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang, > in this way clang will be compatible with gcc and they can both use > -ftrivial-auto-var-init=zero. Exactly, i.e. `-enable` is the one getting removed, but the wording within it (`knowing-it-will-be-removed-from-clang`) meant that the `-ftrivial...=zero` one was the one that would be removed, not `-enable` itself (even if now what will happen is the opposite than what the option suggested originally). So when I read the commit message, the "this additional option ... is going to be removed in the future (as the name of the option suggests)" sounded wrong, since the name did not suggest that, but rather that `-ftrivial...=zero` would be removed, not itself (since that was the original plan). If I understood the story properly, that is. :) Thanks for taking a look! Cheers, Miguel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-04-11 11:17 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-10 21:51 [PATCH v2] rust: allow to use INIT_STACK_ALL_ZERO Andrea Righi 2023-02-10 22:43 ` Kees Cook 2023-02-11 9:04 ` Andrea Righi 2023-02-11 13:44 ` Kees Cook 2023-02-11 14:35 ` Andrea Righi 2023-04-06 22:52 ` Miguel Ojeda 2023-04-07 6:02 ` Andrea Righi 2023-04-09 20:49 ` Miguel Ojeda 2023-04-11 7:11 ` Andrea Righi 2023-04-11 11:17 ` Miguel Ojeda
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).