https://github.com/delcypher updated https://github.com/llvm/llvm-project/pull/92623
>From 4af270878cb243832f5aeb47c785efa263ba8c78 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Fri, 17 May 2024 17:15:48 -0700 Subject: [PATCH] [Bounds Safety] Add `-fexperimental-bounds-safety` CC1 and language option and use it to tweak `counted_by`'s semantics This adds the `-fexperimental-bounds-safety` CC1 and corresponding language option. This language option enables "-fbounds-safety" which is a bounds-safety extension for C that is being incrementally upstreamed. This CC1 flag is not exposed as a driver flag yet because most of the implementation isn't upstream yet. The language option is used to make a small semantic change to how the `counted_by` attribute is treated. Without `-fexperimental-bounds-safety` the attribute is allowed (but emits a warning) on a flexible array member where the element type is a struct with a flexible array member. With the flag this situation is an error. E.g. ``` struct has_unannotated_FAM { int count; char buffer[]; }; struct buffer_of_structs_with_unnannotated_FAM { int count; // Forbidden with `-fexperimental-bounds-safety` struct has_unannotated_FAM Arr[] __counted_by(count); }; ``` The above code **should always** be an error. However, when #90786 was originally landed (which allowed `counted_by` to be used on pointers in structs) it exposed an issue in code in the Linux kernel that was using the `counted_by` attribute incorrectly (see https://github.com/llvm/llvm-project/pull/90786#issuecomment-2118416515) which was now caught by a new error diagnostic in the PR. To unbreak the build of the Linux kernel the error diagnostic was temporarily downgraded to be a warning to give the kernel authors time to fix their code. This downgrading of the error diagnostic to a warning is a departure from the intended semantics of `-fbounds-safety` so in order to have both behaviors (error and warning) it is necessary for Clang to actually have a notion of `-fbounds-safety` being on vs off. rdar://125400392 --- clang/include/clang/Basic/LangOptions.def | 3 ++ clang/include/clang/Driver/Options.td | 8 ++++ clang/lib/Sema/SemaDeclAttr.cpp | 2 +- .../Sema/attr-counted-by-bounds-safety-vlas.c | 37 +++++++++++++++++++ 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 clang/test/Sema/attr-counted-by-bounds-safety-vlas.c diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 5ad9c1f24b7c5..a6f36b23f07dc 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -520,6 +520,9 @@ BENIGN_LANGOPT(CheckNew, 1, 0, "Do not assume C++ operator new may not return NU BENIGN_LANGOPT(CheckConstexprFunctionBodies, 1, 1, "Emit diagnostics for a constexpr function body that can never " "be used in a constant expression.") + +LANGOPT(BoundsSafety, 1, 0, "Bounds safety extension for C") + #undef LANGOPT #undef COMPATIBLE_LANGOPT #undef BENIGN_LANGOPT diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index fecfa2f3c7dba..5229940ec8258 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1911,6 +1911,14 @@ def fapinotes_swift_version : Joined<["-"], "fapinotes-swift-version=">, MetaVarName<"<version>">, HelpText<"Specify the Swift version to use when filtering API notes">; +defm bounds_safety : BoolFOption< + "experimental-bounds-safety", + LangOpts<"BoundsSafety">, DefaultFalse, + PosFlag<SetTrue, [], [CC1Option], "Enable">, + NegFlag<SetFalse, [], [CC1Option], "Disable">, + BothFlags<[], [CC1Option], + " experimental bounds safety extension for C">>; + defm addrsig : BoolFOption<"addrsig", CodeGenOpts<"Addrsig">, DefaultFalse, PosFlag<SetTrue, [], [ClangOption, CC1Option], "Emit">, diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 41295bfb3b94f..f2f3bfcd09035 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5948,7 +5948,7 @@ CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E, } else if (PointeeTy->isFunctionType()) { InvalidTypeKind = CountedByInvalidPointeeTypeKind::FUNCTION; } else if (PointeeTy->isStructureTypeWithFlexibleArrayMember()) { - if (FieldTy->isArrayType()) { + if (FieldTy->isArrayType() && !S.getLangOpts().BoundsSafety) { // This is a workaround for the Linux kernel that has already adopted // `counted_by` on a FAM where the pointee is a struct with a FAM. This // should be an error because computing the bounds of the array cannot be diff --git a/clang/test/Sema/attr-counted-by-bounds-safety-vlas.c b/clang/test/Sema/attr-counted-by-bounds-safety-vlas.c new file mode 100644 index 0000000000000..7d9c9a90880ff --- /dev/null +++ b/clang/test/Sema/attr-counted-by-bounds-safety-vlas.c @@ -0,0 +1,37 @@ +// RUN: %clang_cc1 -fsyntax-only -fexperimental-bounds-safety -verify %s +// +// This is a portion of the `attr-counted-by-vla.c` test but is checked +// under the semantics of `-fexperimental-bounds-safety` which has different +// behavior. + +#define __counted_by(f) __attribute__((counted_by(f))) + +struct has_unannotated_VLA { + int count; + char buffer[]; +}; + +struct has_annotated_VLA { + int count; + char buffer[] __counted_by(count); +}; + +struct buffer_of_structs_with_unnannotated_vla { + int count; + // expected-error@+1{{'counted_by' cannot be applied to an array with element of unknown size because 'struct has_unannotated_VLA' is a struct type with a flexible array member}} + struct has_unannotated_VLA Arr[] __counted_by(count); +}; + + +struct buffer_of_structs_with_annotated_vla { + int count; + // expected-error@+1{{'counted_by' cannot be applied to an array with element of unknown size because 'struct has_annotated_VLA' is a struct type with a flexible array member}} + struct has_annotated_VLA Arr[] __counted_by(count); +}; + +struct buffer_of_const_structs_with_annotated_vla { + int count; + // Make sure the `const` qualifier is printed when printing the element type. + // expected-error@+1{{'counted_by' cannot be applied to an array with element of unknown size because 'const struct has_annotated_VLA' is a struct type with a flexible array member}} + const struct has_annotated_VLA Arr[] __counted_by(count); +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits