| Message ID | 20230504181636.never.222-kees@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Compiler Attributes: Add __counted_by macro | expand |
On Thu, May 04, 2023 at 11:16:40AM -0700, Kees Cook wrote: > In an effort to annotate all flexible array members with their run-time > size information, the "element_count" attribute is being introduced by > Clang[1] and GCC[2] in future releases. This annotation will provide > the CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE features the ability > to perform run-time bounds checking on otherwise unknown-size flexible > arrays. > > Even though the attribute is under development, we can start the > annotation process in the kernel. This requires defining a macro for And FWIW, I've done a first-pass at this annotation with Coccinelle. There are plenty more to do: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=devel/counted_by&id=adc5b3cb48a049563dc673f348eab7b6beba8a9b 155 files changed, 158 insertions, 158 deletions
On Thu, May 4, 2023 at 8:16 PM Kees Cook <keescook@chromium.org> wrote: > > + * Optional: future support coming in clang 17 and gcc 14 Should we just say: Optional: only supported since clang >= 17 Optional: only supported since gcc >= 14 even if they are in the future? That way we avoid changing it later. If somebody asks, you already say it it is in the future the commit message, so it should be clear enough... :) And if the compilers end up not supporting it on those versions for some unexpected reason, well, we will need to fix the comment either way. (I can change it on my side if you prefer) Thanks! Cheers, Miguel
On Thu, May 04, 2023 at 11:16:40AM -0700, Kees Cook wrote: > In an effort to annotate all flexible array members with their run-time > size information, the "element_count" attribute is being introduced by > Clang[1] and GCC[2] in future releases. This annotation will provide > the CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE features the ability > to perform run-time bounds checking on otherwise unknown-size flexible > arrays. It's happening! :D > > Even though the attribute is under development, we can start the > annotation process in the kernel. This requires defining a macro for > it, even if we have to change the name of the actual attribute later. > Since it is likely that this attribute may change its name to "counted_by" > in the future (to better align with a future total bytes "sized_by" > attribute), name the wrapper macro "__counted_by", which also reads more > clearly (and concisely) in structure definitions. > > [1] https://reviews.llvm.org/D148381 > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 > > Cc: Miguel Ojeda <ojeda@kernel.org> > Cc: Bill Wendling <morbo@google.com> > Cc: Qing Zhao <qing.zhao@oracle.com> > Cc: Gustavo A. R. Silva <gustavoars@kernel.org> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: Tom Rix <trix@redhat.com> > Cc: llvm@lists.linux.dev > Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Thanks! -- Gustavo > --- > include/linux/compiler_attributes.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h > index e659cb6fded3..9d63fe2024d5 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -123,6 +123,18 @@ > # define __designated_init > #endif > > +/* > + * Optional: future support coming in clang 17 and gcc 14 > + * > + * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 > + * clang: https://reviews.llvm.org/D148381 > + */ > +#if __has_attribute(__element_count__) > +# define __counted_by(member) __attribute__((__element_count__(member))) > +#else > +# define __counted_by(member) > +#endif > + > /* > * Optional: only supported since clang >= 14.0 > * > -- > 2.34.1 >
On Thu, May 04, 2023 at 11:16:40AM -0700, Kees Cook wrote: > In an effort to annotate all flexible array members with their run-time > size information, the "element_count" attribute is being introduced by > Clang[1] and GCC[2] in future releases. This annotation will provide > the CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE features the ability > to perform run-time bounds checking on otherwise unknown-size flexible > arrays. > > Even though the attribute is under development, we can start the > annotation process in the kernel. This requires defining a macro for > it, even if we have to change the name of the actual attribute later. > Since it is likely that this attribute may change its name to "counted_by" > in the future (to better align with a future total bytes "sized_by" > attribute), name the wrapper macro "__counted_by", which also reads more > clearly (and concisely) in structure definitions. > > [1] https://reviews.llvm.org/D148381 > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 > > Cc: Miguel Ojeda <ojeda@kernel.org> > Cc: Bill Wendling <morbo@google.com> > Cc: Qing Zhao <qing.zhao@oracle.com> > Cc: Gustavo A. R. Silva <gustavoars@kernel.org> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: Tom Rix <trix@redhat.com> > Cc: llvm@lists.linux.dev > Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> I agree with Miguel's comment formatting and content suggestions. Thanks for the links, they look good. If we have to update the name of the attribute later, it is not the end of the world, as getting the coversion started at this phase will make the roll out quicker. > --- > include/linux/compiler_attributes.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h > index e659cb6fded3..9d63fe2024d5 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -123,6 +123,18 @@ > # define __designated_init > #endif > > +/* > + * Optional: future support coming in clang 17 and gcc 14 > + * > + * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 > + * clang: https://reviews.llvm.org/D148381 > + */ > +#if __has_attribute(__element_count__) > +# define __counted_by(member) __attribute__((__element_count__(member))) > +#else > +# define __counted_by(member) > +#endif > + > /* > * Optional: only supported since clang >= 14.0 > * > -- > 2.34.1 >
On May 4, 2023 11:27:32 AM PDT, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: >On Thu, May 4, 2023 at 8:16 PM Kees Cook <keescook@chromium.org> wrote: >> >> + * Optional: future support coming in clang 17 and gcc 14 > >Should we just say: > > Optional: only supported since clang >= 17 > Optional: only supported since gcc >= 14 > >even if they are in the future? That way we avoid changing it later. >If somebody asks, you already say it it is in the future the commit >message, so it should be clear enough... :) > >And if the compilers end up not supporting it on those versions for >some unexpected reason, well, we will need to fix the comment either >way. Yeah, fair point! > >(I can change it on my side if you prefer) Sure, thanks! (But I'm happy to send a v2 if that's easier for you, too.) -Kees
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index e659cb6fded3..9d63fe2024d5 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -123,6 +123,18 @@ # define __designated_init #endif +/* + * Optional: future support coming in clang 17 and gcc 14 + * + * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 + * clang: https://reviews.llvm.org/D148381 + */ +#if __has_attribute(__element_count__) +# define __counted_by(member) __attribute__((__element_count__(member))) +#else +# define __counted_by(member) +#endif + /* * Optional: only supported since clang >= 14.0 *
In an effort to annotate all flexible array members with their run-time size information, the "element_count" attribute is being introduced by Clang[1] and GCC[2] in future releases. This annotation will provide the CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE features the ability to perform run-time bounds checking on otherwise unknown-size flexible arrays. Even though the attribute is under development, we can start the annotation process in the kernel. This requires defining a macro for it, even if we have to change the name of the actual attribute later. Since it is likely that this attribute may change its name to "counted_by" in the future (to better align with a future total bytes "sized_by" attribute), name the wrapper macro "__counted_by", which also reads more clearly (and concisely) in structure definitions. [1] https://reviews.llvm.org/D148381 [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Bill Wendling <morbo@google.com> Cc: Qing Zhao <qing.zhao@oracle.com> Cc: Gustavo A. R. Silva <gustavoars@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Tom Rix <trix@redhat.com> Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/compiler_attributes.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)