From: Qing Zhao <qing.zhao@oracle.com>
To: Richard Biener <richard.guenther@gmail.com>,
Siddhesh Poyarekar <siddhesh@gotplt.org>
Cc: Joseph Myers <josmyers@redhat.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
"isanbard@gmail.com" <isanbard@gmail.com>,
Kees Cook <keescook@chromium.org>,
"uecker@tugraz.at" <uecker@tugraz.at>
Subject: Re: [PATCH v9 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)
Date: Tue, 23 Apr 2024 19:56:26 +0000 [thread overview]
Message-ID: <489A3D87-437E-494E-9200-8A0B28EB6AE2@oracle.com> (raw)
In-Reply-To: <20240412135430.4122328-1-qing.zhao@oracle.com>
Ping for the middle-end change approval.
And an update on the status of the patch set:
******Approval status:
All C FE changes have been approved.
******Review status:
All Middle-end changes have been reviewed by Sid, no remaining issue.
Okay for GCC15?
thanks.
Qing
> On Apr 12, 2024, at 09:54, Qing Zhao <qing.zhao@oracle.com> wrote:
>
> Hi,
>
> This is the 9th version of the patch.
>
> Compare with the 8th version, the difference are:
>
> updates per Joseph's comments:
>
> 1. in C FE, add checking for counted_by attribute for the new multiple definitions of the same tag for C23 in the routine "tagged_types_tu_compatible_p".
> Add a new testing case flex-array-counted-by-8.c for this.
> This is for Patch 1;
>
> 2. two minor typo fixes in c-typeck.cc.
> This is for Patch 2;
>
> Approval status:
>
> Patch 2's C FE change has been approved with minor typo fixes (the above 2);
> Patch 4 has been approved;
> Patch 5's C FE change has been approved;
>
> Review status:
>
> Patch 3, Patch 2 and Patch 5's Middle-end change have been review by Sid, No issue.
>
> More review needed:
>
> Patch 1's new change to C FE (the above 1);
> Patch 2, 3 and 5's middle-end change need to be approved
>
> The 8th version is here:
> https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648559.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648560.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648561.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648562.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648563.html
>
> It based on the following original 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, TYPE_OF_REF)
>
> 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: the number of bytes;
> 1: the number of the elements of the object type;
> 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
> 6th argument "TYPE_OF_REF": A constant 0 with the pointer TYPE to
> to the original flexible array type.
>
> ****** 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.
>
> 5. Add the 6th argument to .ACCESS_WITH_SIZE to carry the TYPE of the flexible array.
> which includes:
> * Add the 6th argument to .ACCESS_WITH_SIZE.
> * use the type of the 6th argument of the routine in tree-object-size.cc
> * testing case.
>
> ******Remaining works:
>
> 6 Improve __bdos to use the counted_by info in whole-object size for the structure with FAM.
> 7 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
next prev parent reply other threads:[~2024-04-23 19:56 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 13:54 Qing Zhao
2024-04-12 13:54 ` [PATCH v9 1/5] Provide counted_by attribute to flexible array member field (PR108896) Qing Zhao
2024-04-22 20:38 ` Joseph Myers
2024-04-22 21:52 ` Qing Zhao
2024-04-12 13:54 ` [PATCH v9 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE Qing Zhao
2024-04-12 13:54 ` [PATCH v9 3/5] Use the .ACCESS_WITH_SIZE in builtin object size Qing Zhao
2024-04-12 13:54 ` [PATCH v9 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer Qing Zhao
2024-04-12 13:54 ` [PATCH v9 5/5] Add the 6th argument to .ACCESS_WITH_SIZE Qing Zhao
2024-04-23 19:56 ` Qing Zhao [this message]
2024-05-07 14:02 ` Ping * 2 [PATCH v9 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
2024-05-07 14:14 ` Qing Zhao
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=489A3D87-437E-494E-9200-8A0B28EB6AE2@oracle.com \
--to=qing.zhao@oracle.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=isanbard@gmail.com \
--cc=josmyers@redhat.com \
--cc=keescook@chromium.org \
--cc=richard.guenther@gmail.com \
--cc=siddhesh@gotplt.org \
--cc=uecker@tugraz.at \
/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 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 read-only IMAP folder(s) and NNTP newsgroup(s).