An update to the 5th version of the patches:
Kees helped me to do more testings, and found one issue:
===
We cannot use the result type or the type of the 1st argument
of the routine .ACCESS_WITH_SIZE to decide the element type
of the original array due to possible type casting in the
source code.
The element type of the original array is needed during tree-object-size.cc
<http://tree-object-size.cc/>.
===
In order to resolve this issue, I added the 6th argument to the routine
.ACCESS_WITH_SIZE
to carry the original type of the array:
- (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
+ (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1,
+ (TYPE_OF_ARRAY *)0))
+ The 6th argument of the call is a constant 0 with the pointer TYPE
+ to the original flexible array type.
+
With this fix. The kernel (with counted-by annotation) has been built
successfully and the gcc with counted-by
Support found one kernel bug!!.
Other testings were all good.
I will send the 6th version of the patch soon. (The only change of the 6th
version compared to the 5th version
Is the above fix).
Thanks.
Qing
> On Feb 9, 2024, at 10:54 AM, Qing Zhao <[email protected]> wrote:
>
> Hi,
>
> This is the 5th version of the patch.
>
> compare with the 4th version, the major difference are:
>
> 1. Change the return type of the routine .ACCESS_WITH_SIZE
> FROM:
> Pointer to the type of the element of the flexible array;
> TO:
> Pointer to the type of the flexible array;
> And then wrap the call with an indirection reference.
>
> 2. Adjust all other parts with this change, (this will simplify the bound
> sanitizer instrument);
>
> 3. Add the fixes to the kernel building failures, which include:
> A. The operator “typeof” cannot return correct type for a->array;
> B. The operator “&” cannot return correct address for a->array;
>
> 4. Correctly handle the case when the value of “counted-by” is zero or
> negative as following
> 4.1. Update the counted-by doc with the following:
> When the counted-by field is assigned a negative integer value, the
> compiler will treat the value as zero.
> 4.2. Adjust __bdos and array bound sanitizer to handle correctly when
> “counted-by” is zero.
>
>
> It based on the following proposal:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635884.html
> Represent the missing dependence for the "counted_by" attribute and its
> consumers
>
> ******The summary of the proposal is:
>
> * Add a new internal function ".ACCESS_WITH_SIZE" to carry the size
> information for every reference to a FAM field;
> * In C FE, Replace every reference to a FAM field whose TYPE has the
> "counted_by" attribute with the new internal function ".ACCESS_WITH_SIZE";
> * In every consumer of the size information, for example, BDOS or array bound
> sanitizer, query the size information or ACCESS_MODE information from the new
> internal function;
> * When expansing to RTL, replace the internal function with the actual
> reference to the FAM field;
> * Some adjustment to ipa alias analysis, and other SSA passes to mitigate the
> impact to the optimizer and code generation.
>
>
> ******The new internal function
>
> .ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE, TYPE_OF_SIZE,
> ACCESS_MODE)
>
> INTERNAL_FN (ACCESS_WITH_SIZE, ECF_LEAF | ECF_NOTHROW, NULL)
>
> which returns the "REF_TO_OBJ" same as the 1st argument;
>
> Both the return type and the type of the first argument of this function have
> been converted from the incomplete array type to the corresponding pointer
> type.
>
> The call to .ACCESS_WITH_SIZE is wrapped with an INDIRECT_REF, whose type is
> the original imcomplete array type.
>
> Please see the following link for why:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638793.html
> https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639605.html
>
> 1st argument "REF_TO_OBJ": The reference to the object;
> 2nd argument "REF_TO_SIZE": The reference to the size of the object,
> 3rd argument "CLASS_OF_SIZE": The size referenced by the REF_TO_SIZE
> represents
> 0: unknown;
> 1: the number of the elements of the object type;
> 2: the number of bytes;
> 4th argument TYPE_OF_SIZE: A constant 0 with the TYPE of the object
> refed by REF_TO_SIZE
> 5th argument "ACCESS_MODE":
> -1: Unknown access semantics
> 0: none
> 1: read_only
> 2: write_only
> 3: read_write
>
> ****** The Patch sets included:
>
> 1. Provide counted_by attribute to flexible array member field;
> which includes:
> * "counted_by" attribute documentation;
> * C FE handling of the new attribute;
> syntax checking, error reporting;
> * testing cases;
>
> 2. Convert "counted_by" attribute to/from .ACCESS_WITH_SIZE.
> which includes:
> * The definition of the new internal function .ACCESS_WITH_SIZE in
> internal-fn.def.
> * C FE converts every reference to a FAM with "counted_by" attribute to
> a call to the internal function .ACCESS_WITH_SIZE.
> (build_component_ref in c_typeck.cc)
> This includes the case when the object is statically allocated and
> initialized.
> In order to make this working, we should update
> initializer_constant_valid_p_1 and output_constant in varasm.cc to include
> calls to .ACCESS_WITH_SIZE.
>
> However, for the reference inside "offsetof", ignore the "counted_by"
> attribute since it's not useful at all. (c_parser_postfix_expression in
> c/c-parser.cc)
> In addtion to "offsetof", for the reference inside operator "typeof" and
> "alignof", we ignore counted_by attribute too.
> When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
> replace the call with its first argument.
>
> * Convert every call to .ACCESS_WITH_SIZE to its first argument.
> (expand_ACCESS_WITH_SIZE in internal-fn.cc)
> * adjust alias analysis to exclude the new internal from clobbering
> anything.
> (ref_maybe_used_by_call_p_1 and call_may_clobber_ref_p_1 in
> tree-ssa-alias.cc)
> * adjust dead code elimination to eliminate the call to
> .ACCESS_WITH_SIZE when
> it's LHS is eliminated as dead code.
> (eliminate_unnecessary_stmts in tree-ssa-dce.cc)
> * Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
> get the reference from the call to .ACCESS_WITH_SIZE.
> (is_access_with_size_p and get_ref_from_access_with_size in tree.cc)
> * testing cases. (for offsetof, static initialization, generation of
> calls to
> .ACCESS_WITH_SIZE, code runs correctly after calls to
> .ACCESS_WITH_SIZE are
> converted back)
>
> 3. Use the .ACCESS_WITH_SIZE in builtin object size (sub-object only)
> which includes:
> * use the size info of the .ACCESS_WITH_SIZE for sub-object.
> * when the size is a negative integer, treat it as zero.
> * testing cases.
>
> 4 Use the .ACCESS_WITH_SIZE in bound sanitizer
> which includes:
> * Instrument array_ref with a call to .ACCESS_WITH_SIZE for bound
> sanitizer.
> * when the size is a negative integer, treat it as zero.
> * testing cases.
>
> ******Remaining works:
>
> 5 Improve __bdos to use the counted_by info in whole-object size for the
> structure with FAM.
> 6 Emit warnings when the user breaks the requirments for the new counted_by
> attribute
> compilation time: -Wcounted-by
> run time: -fsanitizer=counted-by
> * The initialization to the size field should be done before the first
> reference to the FAM field.
> * the array has at least # of elements specified by the size field all
> the time during the program.
>
> I have bootstrapped and regression tested on both x86 and aarch64, no issue.
>
> Let me know your comments.
>
> thanks.
>
> Qing
