Re: [wwwdocs][Patch] gcc-15: Fortran - mention -funsigned + PowerPC Darwin IEEE module support
On Fri, 20 Sep 2024, Tobias Burnus wrote: > Comments, remarks to, approval of the attached wwwdocs patch? + Experimental support for unsigned integers; enabled by the + -funsigned, see https://gcc.gnu.org/onlinedocs/gfortran/Experimental-features-for-Fortran-202Y.html"; + >gfortran documentation for details. This feature has been proposed + (https://j3-fortran.org/doc/year/24/24-116.txt";>J3/24-116) + for inclusion in the next Fortran standard. I think we can just say "This has been", dropping "feature", though if you strongly feel about it, keep it. Looks fine otherwise. Gerald
Re: [wwwdocs][Patch] gcc-15: Update OpenMP section for constr/destr on devices + UID routines
On Fri, 20 Sep 2024, Tobias Burnus wrote: > A minor update for a bug fix / impl.-quality feature and a proper new > feature. Looks fine to me. Gerald
[wwwdocs][Patch] gcc-15: mention wider offloading arch combination support (e.g. aarch64 + nvptx)
This is supposed to document that GCC now supports offloading, e.g., from an ARM CPU to a Nvidia GPU (i.e. Grace<->Hopper) or, e.g., x86-64 to RISC-V. → https://gcc.gnu.org/PR96265 and https://gcc.gnu.org/PR111937 for the associated PRs. I think it is important enough to get it into the release notes. However, I am not sure about the wording. Thoughts or suggestions? Tobias gcc-15: mention wider offloading arch support (e.g. aarch64 + nvptx) diff --git a/htdocs/gcc-15/changes.html b/htdocs/gcc-15/changes.html index 7c372688..e923ede4 100644 --- a/htdocs/gcc-15/changes.html +++ b/htdocs/gcc-15/changes.html @@ -36,6 +36,14 @@ a work-in-progress. General Improvements + + +For offloading, issues preventing some host-device architecture +combinations have been resolved. In particular, offloading from an +aarch64 host to a nvptx device is now supported. + + + New Languages and Language specific improvements
[wwwdocs][Patch] gcc-15: Update OpenMP section for constr/destr on devices + UID routines
A minor update for a bug fix / impl.-quality feature and a proper new feature. Any comments before I apply it? Tobias gcc-15: Update OpenMP section for constr/destr on devices + UID routines diff --git a/htdocs/gcc-15/changes.html b/htdocs/gcc-15/changes.html index 7c372688..14514131 100644 --- a/htdocs/gcc-15/changes.html +++ b/htdocs/gcc-15/changes.html @@ -55,11 +55,17 @@ a work-in-progress. GPUs, writing to the terminal from OpenMP target regions (but not from OpenACC compute regions) is now also supported in Fortran; in C/C++ and on AMD GPUs this was already supported before with both OpenMP and OpenACC. + Constructors and destructors on the device side for declare target + static aggregates are now handled. OpenMP 5.1: The unroll and tile loop-transformation constructs are now supported. + + OpenMP 6.0: The get_device_from_uid and + omp_get_uid_from_device API routines have been added. +
[wwwdocs][Patch] gcc-15: Fortran - mention -funsigned + PowerPC Darwin IEEE module support
Hi all,
I thought it makes sense to have a look at what went into GCC 15 to
update the Fortran section. However, while several bugs were fixed
(and extended some features a tiny bit) [hooray!], I did not really
see many newsworthy features.
Comments, remarks to, approval of the attached wwwdocs patch?
Tobias
PS: Anuj, the GSoC student, nearly finished his do-concurrent patch,
which will add the local/local_init/shared/default(none) of F2018
and the reduce of F2023. Still no fancy parallelization, but the first
step + useful as it will permit compiling such code and it does works
as serially run code.
gcc-15: Fortran - mention -funsigned + PowerPC Darwin IEEE module support
diff --git a/htdocs/gcc-15/changes.html b/htdocs/gcc-15/changes.html
index 7c372688..3a275d8c 100644
--- a/htdocs/gcc-15/changes.html
+++ b/htdocs/gcc-15/changes.html
@@ -111,6 +111,12 @@ a work-in-progress.
Fortran 2023: The selected_logical_kind intrinsic function
and, in the ISO_FORTRAN_ENV module, the named constants
logical{8,16,32,64} and real16 were added.
+ Experimental support for unsigned integers; enabled by the
+ -funsigned, see https://gcc.gnu.org/onlinedocs/gfortran/Experimental-features-for-Fortran-202Y.html";
+ >gfortran documentation for details. This feature has been proposed
+ (https://j3-fortran.org/doc/year/24/24-116.txt";>J3/24-116)
+ for inclusion in the next Fortran standard.
@@ -214,6 +220,11 @@ a work-in-progress.
+PowerPC Darwin
+
+ Fortran's IEEE modules are now suppored on Darwin PowerPC.
+
+
GCC 15: nvptx '-mptx=3.1' multilib variants are deprecated
Hi! Regarding ongoing maintenance efforts, and avoiding to build multilib variants that probably nobody uses apart from a few of us testing these out of routine (via building/linking with explicit '-mptx=3.1'), I propose: "GCC 15: nvptx '-mptx=3.1' multilib variants are deprecated", see attached, "[...], and will be removed in GCC 16". Any objections? If not, then I'll push this before the GCC 15 release, and timely after the GCC 15 release apply the corresponding code changes (yet to be implemented). (That is, no actual change for GCC release users for another 1.5 years.) These '-mptx=3.1' multilib variants are only useful for users of ancient CUDA/Nvidia Driver, which doesn't support GCC's default PTX ISA 6.0 multilib variants; PTX ISA 6.0 is supported as of CUDA 9, 2017-09. Grüße Thomas >From 8c099b2c4fed4f0745ef913c865868e76c061232 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Thu, 19 Sep 2024 22:04:28 +0200 Subject: [PATCH] GCC 15: nvptx '-mptx=3.1' multilib variants are deprecated --- htdocs/gcc-15/changes.html | 4 1 file changed, 4 insertions(+) diff --git a/htdocs/gcc-15/changes.html b/htdocs/gcc-15/changes.html index 7c372688..99242d2c 100644 --- a/htdocs/gcc-15/changes.html +++ b/htdocs/gcc-15/changes.html @@ -191,6 +191,10 @@ a work-in-progress. For this, a recent version of https://gcc.gnu.org/install/specific.html#nvptx-x-none"; >nvptx-tools is required. + +The -mptx=3.1 multilib variants are deprecated and will be +removed in GCC 16. + -- 2.45.2
Re: [gcc-wwwdocs PATCH] gcc-14: Mention -march=gracemont support in x86_64
On Thu, 19 Sep 2024, Haochen Jiang wrote: > When I was backporting my doc patch in gcc trunk today, I found when > adding -march=gracemont in GCC14, the corresponding wwwdoc is missing. > This patch is adding that. This looks fine, thank you. Gerald
[gcc-wwwdocs PATCH] gcc-14: Mention -march=gracemont support in x86_64
Hi all,
When I was backporting my doc patch in gcc trunk today, I found when adding
-march=gracemont in GCC14, the corresponding wwwdoc is missing. This patch
is adding that.
Ok for wwwdocs trunk?
Thx,
Haochen
---
htdocs/gcc-14/changes.html | 4
1 file changed, 4 insertions(+)
diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
index e0d856cc..ba9fc680 100644
--- a/htdocs/gcc-14/changes.html
+++ b/htdocs/gcc-14/changes.html
@@ -944,6 +944,10 @@ __asm (".global __flmap_lock" "\n\t"
Based on Sierra Forest, the switch further enables the AVX-VNNI-INT16,
PREFETCHI, SHA512, SM3, SM4 and USER_MSR ISA extensions.
+ GCC now supports the Intel CPU named Gracemont through
+-march=gracemont.
+Gracemont is based on Alder Lake.
+
GCC now supports the Intel CPU named Arrow Lake through
-march=arrowlake.
Based on Alder Lake, the switch further enables the AVX-IFMA,
--
2.31.1
Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option
On Tue, Sep 17, 2024 at 3:40 AM Dora, Sunil Kumar wrote: > > Hi Andrew, > > Initially, I thought to address long command line options (when exceeding > 128KB) without disrupting the existing GCC driver behavior. > > As you suggested, I implemented changes to use the response file format > (@file) within the set_collect_gcc_options function and ensured that this was > passed through COLLECT_GCC_OPTIONS. > However, these changes have introduced a side effect: they impact the > behavior of the -save-temps switch by generating additional .args.N files. As > a result, some existing test cases, including the one reported by the Linaro > team, are now failing. > (File: Attached) > Could you please advise on how we should proceed? Specifically, should we > adjust the test cases to accommodate the impact on the -save-temps switch, or > is there an alternative approach you would recommend? Your guidance on how to > address these issues while implementing the response file approach would be > greatly appreciated. Sounds like the testcase needs to be changed. If you were not saving around the file that was used for COLLECT_GCC_OPTIONS in the previous patches (with -save-temps), then that was broken. Or is the issue the name of the file is not based on the aux dump file but based on something else? That is what is the file that is kept around for -save-temps ? Thanks, Andrew > Thank you for your support. > > > > Thanks, > Sunil Dora > > From: Andrew Pinski > Sent: Friday, September 6, 2024 11:33 PM > To: Dora, Sunil Kumar > Cc: Hemraj, Deepthi ; GCC Patches > ; Richard Guenther ; Jeff Law > ; josmy...@redhat.com ; MacLeod, > Randy ; Gowda, Naveen > > Subject: Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option > > CAUTION: This email comes from a non Wind River email account! > > Do not click links or open attachments unless you recognize the sender and > know the content is safe. > > > On Fri, Sep 6, 2024, 9:38 AM Dora, Sunil Kumar > wrote: > > Hi Andrew, > > Thank you for your feedback. Initially, we attempted to address the issue by > utilizing GCC’s response files. However, we discovered that the > COLLECT_GCC_OPTIONS variable already contains the expanded contents of the > response files. > > As a result, using response files only mitigates the multiplication factor > but does not bypass the 128KB limit. > > > I think you missed understood me fully. What I was saying instead of creating > a string inside set_collect_gcc_options, create the response file and pass > that via COLLECT_GCC_OPTIONS with the @file format. And then inside > collect2.cc when using COLLECT_GCC_OPTIONS/extract_string instead read in the > response file options if there was an @file instead of those 2 loops. This > requires more than what you did. Oh and should be less memory hungry and > maybe slightly faster. > > Thanks, > Andrew > > > > I have included the response file usage logs and the complete history in the > Bugzilla report for your reference: Bugzilla Link. > Following your suggestion, I have updated the logic to avoid hardcoding /tmp. > Please find the revised version of patch at the following link: > > https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662519.html > > Thanks, > Sunil Dora > > From: Andrew Pinski > Sent: Friday, August 30, 2024 8:05 PM > To: Hemraj, Deepthi > Cc: gcc-patches@gcc.gnu.org ; rguent...@suse.de > ; jeffreya...@gmail.com ; > josmy...@redhat.com ; MacLeod, Randy > ; Gowda, Naveen ; > Dora, Sunil Kumar > Subject: Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option > > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and > know the content is safe. > > On Fri, Aug 30, 2024 at 12:34 AM wrote: > > > > From: Deepthi Hemraj > > > > For excessively long environment variables i.e >128KB > > Store the arguments in a temporary file and collect them back together in > > collect2. > > > > This commit patches for COLLECT_GCC_OPTIONS issue: > > GCC should not limit the length of command line passed to collect2. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527 > > > > The Linux kernel has the following limits on shell commands: > > I. Total number of bytes used to specify arguments must be under 128KB. > > II. Each environment variable passed to an executable must be under 128 KiB > > > > In order to circumvent these limitations, many build tools support > > response-files, i.e. fil
Re: [PATCH v5] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
On Tue, Sep 17, 2024 at 11:13:09AM +0200, Jakub Jelinek wrote:
> So maybe better
> tree arg = e_p->value;
> tree f;
> if ((in_typeof || in_alignof)
> && TREE_CODE (arg) == COMPONENT_REF
> && (f = TREE_OPERAND (arg, 1))
> && TREE_CODE (f) == FIELD_DECL
> && c_flexible_array_member_type_p (TREE_TYPE (f))
> && lookup_attribute ("counted_by", DECL_ATTRIBUTES (f))
> && DECL_NAME (f))
Of course with
{
auto save_in_typeof = in_typeof;
auto save_in_alignof = in_alignof;
in_typeof = 0;
in_alignof = 0;
> arg = build_component_ref (EXPR_LOCATION (arg),
>TREE_OPERAND (arg, 0),
>DECL_NAME (f), UNKNOWN_LOCATION,
>UNKNOWN_LOCATION, true);
in_typeof = save_in_typeof;
in_alignof = save_in_alignof;
}
Why don't we have the sentinel classes C++ FE has in the C FE?
Or outline the counted_by specific part of build_component_ref into
a separate function and call just that.
> if (has_counted_by_object (arg))
> expr.value = get_counted_by_ref (arg);
> else
> expr.value = null_pointer_node;
BTW, concerning just counted_by attribute, how does it play together
with _Atomic on the size member? Perhaps it should be disallowed.
The thing is that _Atomic access lowering is done in the FE, so too
early for tree-object-size, which would either need to reimplement it by
hand (sure, not that difficult, it needs to just atomically load).
Jakub
Re: [PATCH v5] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
On Sat, Sep 14, 2024 at 08:58:28PM +0200, Jakub Jelinek wrote:
> if (has_counted_by_object (e_p->value))
> expr.value = get_counted_by_ref (e_p->value);
> else if (in_typeof && TREE_CODE (e_p->value) == COMPONENT_REF)
> {
> tree counted_by_type = NULL_TREE;
> tree arg = (*cexpr_
> if (build_counted_by_ref (TREE_OPERAND (e_p->value, 0),
> TREE_OPERAND (e_p->value, 1),
> &counted_by_type))
> expr.value
> = build_zero_cst (build_pointer_type (counted_by_type));
> else
> expr.value = null_pointer_node;
> }
> else
> expr.value = null_pointer_node;
>
> (plus make build_counted_by_ref non-static and add prototype).
>
> Completely untested.
Note, the above I think ought to work with say
__typeof (__builtin_counted_by_ref (obj->fam))
or
__alignof (*__builtin_counted_by_ref (obj->fam))
but will not work with say
struct with_fam obj;
void foo () {
...
__typeof (char [__builtin_counted_by_ref (obj.fam) == &obj.size ? 1 : -1])
...
}
etc. (and similar for alignof; something like that should go into the
testsuite).
So maybe better
tree arg = e_p->value;
tree f;
if ((in_typeof || in_alignof)
&& TREE_CODE (arg) == COMPONENT_REF
&& (f = TREE_OPERAND (arg, 1))
&& TREE_CODE (f) == FIELD_DECL
&& c_flexible_array_member_type_p (TREE_TYPE (f))
&& lookup_attribute ("counted_by", DECL_ATTRIBUTES (f))
&& DECL_NAME (f))
arg = build_component_ref (EXPR_LOCATION (arg),
TREE_OPERAND (arg, 0),
DECL_NAME (f), UNKNOWN_LOCATION,
UNKNOWN_LOCATION, true);
if (has_counted_by_object (arg))
expr.value = get_counted_by_ref (arg);
else
expr.value = null_pointer_node;
Jakub
Re: [PATCH v5] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
> On Sep 14, 2024, at 8:59 PM, Jakub Jelinek wrote:
>
> On Wed, Sep 11, 2024 at 09:13:40PM +, Qing Zhao wrote:
>> @@ -11741,6 +11770,55 @@ c_parser_postfix_expression (c_parser *parser)
>>set_c_expr_source_range (&expr, loc, close_paren_loc);
>>break;
>> }
>> +case RID_BUILTIN_COUNTED_BY_REF:
>> + {
>> +vec *cexpr_list;
>> +c_expr_t *e_p;
>> +location_t close_paren_loc;
>> +
>> +in_builtin_counted_by_ref++;
>
> I think this in_builtin_counted_by_ref stuff here plus the whole c-typeck.cc
> hunk should be replaced with:
>
>> +c_parser_consume_token (parser);
>> +if (!c_parser_get_builtin_args (parser,
>> +"__builtin_counted_by_ref",
>> +&cexpr_list, false,
>> +&close_paren_loc))
>> + {
>> +expr.set_error ();
>> +in_builtin_counted_by_ref--;
>> +break;
>> + }
>> +if (vec_safe_length (cexpr_list) != 1)
>> + {
>> +error_at (loc, "wrong number of arguments to "
>> + "%<__builtin_counted_by_ref%>");
>> +expr.set_error ();
>> +in_builtin_counted_by_ref--;
>> +break;
>> + }
>> +
>> +e_p = &(*cexpr_list)[0];
>> +
>> +if (TREE_CODE (TREE_TYPE (e_p->value)) != ARRAY_TYPE)
>> + {
>> +error_at (loc, "the argument must be an array"
>> + "%<__builtin_counted_by_ref%>");
>> +expr.set_error ();
>> +in_builtin_counted_by_ref--;
>> +break;
>> + }
>> +
>
>if (has_counted_by_object (e_p->value))
> expr.value = get_counted_by_ref (e_p->value);
>else if (in_typeof && TREE_CODE (e_p->value) == COMPONENT_REF)
> {
>tree counted_by_type = NULL_TREE;
>tree arg = (*cexpr_
>if (build_counted_by_ref (TREE_OPERAND (e_p->value, 0),
> TREE_OPERAND (e_p->value, 1),
> &counted_by_type))
> expr.value
>= build_zero_cst (build_pointer_type (counted_by_type));
>else
> expr.value = null_pointer_node;
> }
>else
> expr.value = null_pointer_node;
>
> (plus make build_counted_by_ref non-static and add prototype).
>
> Completely untested.
Thanks a lot!will update as you suggested
Qing
>
>> +if (has_counted_by_object ((*cexpr_list)[0].value))
>> + expr.value
>> += get_counted_by_ref ((*cexpr_list)[0].value);
>> +else
>> + expr.value
>> += build_int_cst (build_pointer_type (void_type_node), 0);
>
>Jakub
>
New Chinese (simplified) PO file for 'gcc' (version 14.2.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Chinese (simplified) team of translators. The file is available at: https://translationproject.org/latest/gcc/zh_CN.po (This file, 'gcc-14.2.0.zh_CN.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [PATCH v5] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
On Wed, Sep 11, 2024 at 09:13:40PM +, Qing Zhao wrote:
> @@ -11741,6 +11770,55 @@ c_parser_postfix_expression (c_parser *parser)
> set_c_expr_source_range (&expr, loc, close_paren_loc);
> break;
> }
> + case RID_BUILTIN_COUNTED_BY_REF:
> + {
> + vec *cexpr_list;
> + c_expr_t *e_p;
> + location_t close_paren_loc;
> +
> + in_builtin_counted_by_ref++;
I think this in_builtin_counted_by_ref stuff here plus the whole c-typeck.cc
hunk should be replaced with:
> + c_parser_consume_token (parser);
> + if (!c_parser_get_builtin_args (parser,
> + "__builtin_counted_by_ref",
> + &cexpr_list, false,
> + &close_paren_loc))
> + {
> + expr.set_error ();
> + in_builtin_counted_by_ref--;
> + break;
> + }
> + if (vec_safe_length (cexpr_list) != 1)
> + {
> + error_at (loc, "wrong number of arguments to "
> +"%<__builtin_counted_by_ref%>");
> + expr.set_error ();
> + in_builtin_counted_by_ref--;
> + break;
> + }
> +
> + e_p = &(*cexpr_list)[0];
> +
> + if (TREE_CODE (TREE_TYPE (e_p->value)) != ARRAY_TYPE)
> + {
> + error_at (loc, "the argument must be an array"
> +"%<__builtin_counted_by_ref%>");
> + expr.set_error ();
> + in_builtin_counted_by_ref--;
> + break;
> + }
> +
if (has_counted_by_object (e_p->value))
expr.value = get_counted_by_ref (e_p->value);
else if (in_typeof && TREE_CODE (e_p->value) == COMPONENT_REF)
{
tree counted_by_type = NULL_TREE;
tree arg = (*cexpr_
if (build_counted_by_ref (TREE_OPERAND (e_p->value, 0),
TREE_OPERAND (e_p->value, 1),
&counted_by_type))
expr.value
= build_zero_cst (build_pointer_type (counted_by_type));
else
expr.value = null_pointer_node;
}
else
expr.value = null_pointer_node;
(plus make build_counted_by_ref non-static and add prototype).
Completely untested.
> + if (has_counted_by_object ((*cexpr_list)[0].value))
> + expr.value
> + = get_counted_by_ref ((*cexpr_list)[0].value);
> + else
> + expr.value
> + = build_int_cst (build_pointer_type (void_type_node), 0);
Jakub
Re: [PATCH v1][GCC] aarch64: Add GCS build attributes support.
On Wed, Sep 11, 2024 at 11:51 AM Srinath Parvathaneni wrote: > > This patch adds support for aarch64 gcs build attributes. Hi, just wondering if you could clarify what "GCS" stands for in this context? When I see it, my first thought is "GNU Coding Standards", but I don't think that's right... > This support includes generating two new assembler directives > .aeabi_subsection and .aeabi_attribute. These directives are > generated as per the syntax mentioned in spec > "Build Attributes for the Arm® 64-bit Architecture (AArch64)" > available at [1]. > > To check whether the assembler being used to build the toolchain > supports these directives, a new gcc configure check is added in > gcc/configure.ac. > > If the assembler support these directives, .aeabi_subsection and > .aeabi_attribute directives are emitted in the generated assembly, > when -mbranch-protection=gcs is passed. > > If the assembler does not support these directives, > .note.gnu.property section will emit the relevant gcs information > in the generated assembly, when -mbranch-protection=gcs is passed. > > This patch needs to be applied on top of GCC gcs patch series [2]. > > Bootstrapped on aarch64-none-linux-gnu and regression tested on > aarch64-none-elf, no issues. > > Ok for master? > > Regards, > Srinath. > > [1]: https://github.com/ARM-software/abi-aa/pull/230 > [2]: > https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs > > gcc/ChangeLog: > > 2024-09-11 Srinath Parvathaneni > > * config.in: Regenerated > * config/aarch64/aarch64.cc (aarch64_emit_aeabi_attribute): New > function declaration. > (aarch64_emit_aeabi_subsection): Likewise. > (aarch64_start_file): Emit gcs build attributes. > (aarch64_file_end_indicate_exec_stack): Update gcs bit in > note.gnu.property section. > * configure: Regenerated. > * configure.ac: Add gcc configure check. > > gcc/testsuite/ChangeLog: > > 2024-09-11 Srinath Parvathaneni > > * gcc.target/aarch64/build-attribute-gcs.c: New test. > --- > gcc/config.in | 6 +++ > gcc/config/aarch64/a.out | Bin 0 -> 656 bytes > gcc/config/aarch64/aarch64.cc | 43 ++ > gcc/configure | 35 ++ > gcc/configure.ac | 7 +++ > .../gcc.target/aarch64/build-attribute-gcs.c | 24 ++ > 6 files changed, 115 insertions(+) > create mode 100644 gcc/config/aarch64/a.out > create mode 100644 gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c >
Re: [PATCH v1][GCC] aarch64: Add GCS build attributes support.
Hi Srinath,
Not a full review, just some things that popped out to me.
> On 11 Sep 2024, at 17:50, Srinath Parvathaneni
> wrote:
>
> External email: Use caution opening links or attachments
>
>
> This patch adds support for aarch64 gcs build attributes. This support
> includes generating two new assembler directives .aeabi_subsection and
> .aeabi_attribute. These directives are generated as per the syntax
> mentioned in spec "Build Attributes for the Arm® 64-bit
> Architecture (AArch64)" available at [1].
>
> To check whether the assembler being used to build the toolchain
> supports these directives, a new gcc configure check is added in
> gcc/configure.ac.
>
> If the assembler support these directives, .aeabi_subsection and
> .aeabi_attribute directives are emitted in the generated assembly,
> when -mbranch-protection=gcs is passed.
>
> If the assembler does not support these directives,
> .note.gnu.property section will emit the relevant gcs information
> in the generated assembly, when -mbranch-protection=gcs is passed.
>
> This patch needs to be applied on top of GCC gcs patch series [2].
>
> Bootstrapped on aarch64-none-linux-gnu and regression tested on
> aarch64-none-elf, no issues.
>
> Ok for master?
>
> Regards,
> Srinath.
>
> [1]: https://github.com/ARM-software/abi-aa/pull/230
> [2]:
> https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs
>
> gcc/ChangeLog:
>
> 2024-09-11 Srinath Parvathaneni
>
>* config.in: Regenerated
>* config/aarch64/aarch64.cc (aarch64_emit_aeabi_attribute): New
>function declaration.
>(aarch64_emit_aeabi_subsection): Likewise.
>(aarch64_start_file): Emit gcs build attributes.
>(aarch64_file_end_indicate_exec_stack): Update gcs bit in
>note.gnu.property section.
>* configure: Regenerated.
>* configure.ac: Add gcc configure check.
>
> gcc/testsuite/ChangeLog:
>
> 2024-09-11 Srinath Parvathaneni
>
>* gcc.target/aarch64/build-attribute-gcs.c: New test.
> ---
> gcc/config.in | 6 +++
> gcc/config/aarch64/a.out | Bin 0 -> 656 bytes
This binary artifact shouldn’t be in the patch.
> gcc/config/aarch64/aarch64.cc | 43 ++
> gcc/configure | 35 ++
> gcc/configure.ac | 7 +++
> .../gcc.target/aarch64/build-attribute-gcs.c | 24 ++
> 6 files changed, 115 insertions(+)
> create mode 100644 gcc/config/aarch64/a.out
> create mode 100644 gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
diff --git a/gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
b/gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
new file mode 100644
index 000..eb15772757e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
@@ -0,0 +1,24 @@
+/* { dg-do compile { target aarch64*-*-linux* } } */
+
+int main()
+{
+ return 0;
+}
+
+/* { dg-options "-mbranch-protection=gcs" } */
+/* { dg-final { scan-assembler-not "\.aeabi_subsection
\.aeabi-feature-and-bits, 1, 0" } } */
+/* { dg-final { scan-assembler-not "\.aeabi_attribute 3, 1\t\/\/
Tag_Feature_GCS" } } */
+/* { dg-final { scan-assembler ".note.gnu.property" } } */
+
+/* { dg-options "-mbranch-protection=bti" } */
+/* { dg-final { scan-assembler ".note.gnu.property" } } */
+
+
+/* { dg-options "-mbranch-protection=pac-ret" } */
+/* { dg-final { scan-assembler ".note.gnu.property" } } */
+
+
+/* { dg-options "-mbranch-protection=standard" } */
+/* { dg-final { scan-assembler-not "\.aeabi_subsection
\.aeabi-feature-and-bits, 1, 0" } } */
+/* { dg-final { scan-assembler-not "\.aeabi_attribute 3, 1\t\/\/
Tag_Feature_GCS" } } */
+/* { dg-final { scan-assembler ".note.gnu.property" } } */
These scans should be in different tests compiled with different options.
You can’t have multiple dg-options directives in a single test and scanning for
“.note.gnu.property” multiple times in a single test is redundant too.
Thanks,
Kyrill
Re: [PATCH v5] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
> On Sep 11, 2024, at 18:16, Bill Wendling wrote:
>
> On Wed, Sep 11, 2024 at 2:13 PM Qing Zhao wrote:
>>
>> compared to the 4th version, the changes are (address Jacub's concerns):
>>
>> 1. change the global "in_builtin_counted_by_ref" from a boolean to an int;
>> 2. delete the label for the error handling code, and decress the global
>> "in_builtin_counted_by_ref" before each break;
>>
>> the 4th version compared to the 3rd version, the only change is the
>> size calculation in the testing case.
>>
>> The 3rd version compared to the 2nd version, the major change is:
>> the update in testing cases per Martin's suggestions.
>>
>> when the 2nd version is compared to the first version, the major changes are:
>>
>> 1. change the name of the builtin from __builtin_get_counted_by to
>> __builtin_counted_by_ref in order to reflect the fact that the returned
>> value of it is a reference to the object.
>>
>> 2. make typeof(__builtin_counted_by_ref) working.
>>
>> 3. update the testing case to use the new builtin inside _Generic.
>>
>> bootstrapped and regress tested on both X86 and aarch64. no issue.
>>
>> Okay for the trunk?
>>
>> thanks.
>>
>> Qing.
>>
>> ==
>>
>> With the addition of the 'counted_by' attribute and its wide roll-out
>> within the Linux kernel, a use case has been found that would be very
>> nice to have for object allocators: being able to set the counted_by
>> counter variable without knowing its name.
>>
>> For example, given:
>>
>> struct foo {
>>...
>>int counter;
>>...
>>struct bar array[] __attribute__((counted_by (counter)));
>> } *p;
>>
>> The existing Linux object allocators are roughly:
>>
>> #define MAX(A, B) (A > B) ? (A) : (B)
>> #define alloc(P, FAM, COUNT) ({ \
>>__auto_type __p = &(P); \
>>size_t __size = MAX (sizeof(*P),
>> __builtin_offsetof (__typeof(*P), FAM)
>> + sizeof (*(P->FAM)) * COUNT); \
>>*__p = kmalloc(__size); \
>> })
>>
>> Right now, any addition of a counted_by annotation must also
>> include an open-coded assignment of the counter variable after
>> the allocation:
>>
>> p = alloc(p, array, how_many);
>> p->counter = how_many;
>>
>> In order to avoid the tedious and error-prone work of manually adding
>> the open-coded counted-by intializations everywhere in the Linux
>> kernel, a new GCC builtin __builtin_counted_by_ref will be very useful
>> to be added to help the adoption of the counted-by attribute.
>>
>> -- Built-in Function: TYPE __builtin_counted_by_ref (PTR)
>> The built-in function '__builtin_counted_by_ref' checks whether the
>> array object pointed by the pointer PTR has another object
>> associated with it that represents the number of elements in the
>> array object through the 'counted_by' attribute (i.e. the
>> counted-by object). If so, returns a pointer to the corresponding
>> counted-by object. If such counted-by object does not exist,
>> returns a NULL pointer.
>>
>> This built-in function is only available in C for now.
>>
>> The argument PTR must be a pointer to an array. The TYPE of the
>> returned value must be a pointer type pointing to the corresponding
>> type of the counted-by object or VOID pointer type in case of a
>> NULL pointer being returned.
>>
>> With this new builtin, the central allocator could be updated to:
>>
>> #define MAX(A, B) (A > B) ? (A) : (B)
>> #define alloc(P, FAM, COUNT) ({ \
>>__auto_type __p = &(P); \
>>__auto_type __c = (COUNT); \
>>size_t __size = MAX (sizeof (*(*__p)),\
>> __builtin_offsetof (__typeof(*(*__p)),FAM) \
>> + sizeof (*((*__p)->FAM)) * __c); \
>>if ((*__p = kmalloc(__size))) { \
>> __auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
>> *_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
>>} \
>> })
>>
>> And then structs can gain the counted_by attribute without needing
>> additional open-coded counter assignments for each struct, and
>> unannotated structs could still use the same allocator.
>>
>>PR c/116016
>>
>> gcc/c-family/ChangeLog:
>>
Re: [PATCH v5] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
On Wed, Sep 11, 2024 at 2:13 PM Qing Zhao wrote:
>
> compared to the 4th version, the changes are (address Jacub's concerns):
>
> 1. change the global "in_builtin_counted_by_ref" from a boolean to an int;
> 2. delete the label for the error handling code, and decress the global
>"in_builtin_counted_by_ref" before each break;
>
> the 4th version compared to the 3rd version, the only change is the
> size calculation in the testing case.
>
> The 3rd version compared to the 2nd version, the major change is:
> the update in testing cases per Martin's suggestions.
>
> when the 2nd version is compared to the first version, the major changes are:
>
> 1. change the name of the builtin from __builtin_get_counted_by to
> __builtin_counted_by_ref in order to reflect the fact that the returned
> value of it is a reference to the object.
>
> 2. make typeof(__builtin_counted_by_ref) working.
>
> 3. update the testing case to use the new builtin inside _Generic.
>
> bootstrapped and regress tested on both X86 and aarch64. no issue.
>
> Okay for the trunk?
>
> thanks.
>
> Qing.
>
> ==
>
> With the addition of the 'counted_by' attribute and its wide roll-out
> within the Linux kernel, a use case has been found that would be very
> nice to have for object allocators: being able to set the counted_by
> counter variable without knowing its name.
>
> For example, given:
>
> struct foo {
> ...
> int counter;
> ...
> struct bar array[] __attribute__((counted_by (counter)));
> } *p;
>
> The existing Linux object allocators are roughly:
>
> #define MAX(A, B) (A > B) ? (A) : (B)
> #define alloc(P, FAM, COUNT) ({ \
> __auto_type __p = &(P); \
> size_t __size = MAX (sizeof(*P),
> __builtin_offsetof (__typeof(*P), FAM)
> + sizeof (*(P->FAM)) * COUNT); \
> *__p = kmalloc(__size); \
> })
>
> Right now, any addition of a counted_by annotation must also
> include an open-coded assignment of the counter variable after
> the allocation:
>
> p = alloc(p, array, how_many);
> p->counter = how_many;
>
> In order to avoid the tedious and error-prone work of manually adding
> the open-coded counted-by intializations everywhere in the Linux
> kernel, a new GCC builtin __builtin_counted_by_ref will be very useful
> to be added to help the adoption of the counted-by attribute.
>
> -- Built-in Function: TYPE __builtin_counted_by_ref (PTR)
> The built-in function '__builtin_counted_by_ref' checks whether the
> array object pointed by the pointer PTR has another object
> associated with it that represents the number of elements in the
> array object through the 'counted_by' attribute (i.e. the
> counted-by object). If so, returns a pointer to the corresponding
> counted-by object. If such counted-by object does not exist,
> returns a NULL pointer.
>
> This built-in function is only available in C for now.
>
> The argument PTR must be a pointer to an array. The TYPE of the
> returned value must be a pointer type pointing to the corresponding
> type of the counted-by object or VOID pointer type in case of a
> NULL pointer being returned.
>
> With this new builtin, the central allocator could be updated to:
>
> #define MAX(A, B) (A > B) ? (A) : (B)
> #define alloc(P, FAM, COUNT) ({ \
> __auto_type __p = &(P); \
> __auto_type __c = (COUNT); \
> size_t __size = MAX (sizeof (*(*__p)),\
> __builtin_offsetof (__typeof(*(*__p)),FAM) \
> + sizeof (*((*__p)->FAM)) * __c); \
> if ((*__p = kmalloc(__size))) { \
> __auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
> *_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
> } \
> })
>
> And then structs can gain the counted_by attribute without needing
> additional open-coded counter assignments for each struct, and
> unannotated structs could still use the same allocator.
>
> PR c/116016
>
> gcc/c-family/ChangeLog:
>
> * c-common.cc: Add new __builtin_counted_by_ref.
> * c-common.h (enum rid): Add RID_BUILTIN_COUNTED_BY_REF.
>
> gcc/c/ChangeLog:
>
> * c-decl.cc (names_builtin_p): Add RID_BUILTIN_COUNTED_BY_REF.
> * c-parser.cc (has_counted_by_object): New routine.
> (get_counted_by_ref): New routine.
> (c_parser_postfix_expression): Handle New RID_BUILTIN_COUNTED_BY_REF.
> * c-tree.h: New glob
[PATCH v5] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
compared to the 4th version, the changes are (address Jacub's concerns):
1. change the global "in_builtin_counted_by_ref" from a boolean to an int;
2. delete the label for the error handling code, and decress the global
"in_builtin_counted_by_ref" before each break;
the 4th version compared to the 3rd version, the only change is the
size calculation in the testing case.
The 3rd version compared to the 2nd version, the major change is:
the update in testing cases per Martin's suggestions.
when the 2nd version is compared to the first version, the major changes are:
1. change the name of the builtin from __builtin_get_counted_by to
__builtin_counted_by_ref in order to reflect the fact that the returned
value of it is a reference to the object.
2. make typeof(__builtin_counted_by_ref) working.
3. update the testing case to use the new builtin inside _Generic.
bootstrapped and regress tested on both X86 and aarch64. no issue.
Okay for the trunk?
thanks.
Qing.
==
With the addition of the 'counted_by' attribute and its wide roll-out
within the Linux kernel, a use case has been found that would be very
nice to have for object allocators: being able to set the counted_by
counter variable without knowing its name.
For example, given:
struct foo {
...
int counter;
...
struct bar array[] __attribute__((counted_by (counter)));
} *p;
The existing Linux object allocators are roughly:
#define MAX(A, B) (A > B) ? (A) : (B)
#define alloc(P, FAM, COUNT) ({ \
__auto_type __p = &(P); \
size_t __size = MAX (sizeof(*P),
__builtin_offsetof (__typeof(*P), FAM)
+ sizeof (*(P->FAM)) * COUNT); \
*__p = kmalloc(__size); \
})
Right now, any addition of a counted_by annotation must also
include an open-coded assignment of the counter variable after
the allocation:
p = alloc(p, array, how_many);
p->counter = how_many;
In order to avoid the tedious and error-prone work of manually adding
the open-coded counted-by intializations everywhere in the Linux
kernel, a new GCC builtin __builtin_counted_by_ref will be very useful
to be added to help the adoption of the counted-by attribute.
-- Built-in Function: TYPE __builtin_counted_by_ref (PTR)
The built-in function '__builtin_counted_by_ref' checks whether the
array object pointed by the pointer PTR has another object
associated with it that represents the number of elements in the
array object through the 'counted_by' attribute (i.e. the
counted-by object). If so, returns a pointer to the corresponding
counted-by object. If such counted-by object does not exist,
returns a NULL pointer.
This built-in function is only available in C for now.
The argument PTR must be a pointer to an array. The TYPE of the
returned value must be a pointer type pointing to the corresponding
type of the counted-by object or VOID pointer type in case of a
NULL pointer being returned.
With this new builtin, the central allocator could be updated to:
#define MAX(A, B) (A > B) ? (A) : (B)
#define alloc(P, FAM, COUNT) ({ \
__auto_type __p = &(P); \
__auto_type __c = (COUNT); \
size_t __size = MAX (sizeof (*(*__p)),\
__builtin_offsetof (__typeof(*(*__p)),FAM) \
+ sizeof (*((*__p)->FAM)) * __c); \
if ((*__p = kmalloc(__size))) { \
__auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
*_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
} \
})
And then structs can gain the counted_by attribute without needing
additional open-coded counter assignments for each struct, and
unannotated structs could still use the same allocator.
PR c/116016
gcc/c-family/ChangeLog:
* c-common.cc: Add new __builtin_counted_by_ref.
* c-common.h (enum rid): Add RID_BUILTIN_COUNTED_BY_REF.
gcc/c/ChangeLog:
* c-decl.cc (names_builtin_p): Add RID_BUILTIN_COUNTED_BY_REF.
* c-parser.cc (has_counted_by_object): New routine.
(get_counted_by_ref): New routine.
(c_parser_postfix_expression): Handle New RID_BUILTIN_COUNTED_BY_REF.
* c-tree.h: New global in_builtin_counted_by_ref.
* c-typeck.cc (build_component_ref): Enable generating
.ACCESS_WITH_SIZE inside typeof when inside builtin_counted_by_ref.
gcc/ChangeLog:
* doc/extend.texi: Add documentation for __builtin_counted_by_ref.
gcc/testsuite/ChangeLog:
* gcc.dg/builtin-counted-by-ref-1.c: New test.
* gcc.dg/builtin-counted-by-ref.c: New test.
---
gcc/c-family/c-common.cc | 1 +
gcc/c-family/c-common.h | 1 +
gcc/c/c-decl.cc | 1 +
gcc/c/c-parser.cc | 7
[PATCH v1][GCC] aarch64: Add GCS build attributes support.
This patch adds support for aarch64 gcs build attributes. This support
includes generating two new assembler directives .aeabi_subsection and
.aeabi_attribute. These directives are generated as per the syntax
mentioned in spec "Build Attributes for the Arm® 64-bit
Architecture (AArch64)" available at [1].
To check whether the assembler being used to build the toolchain
supports these directives, a new gcc configure check is added in
gcc/configure.ac.
If the assembler support these directives, .aeabi_subsection and
.aeabi_attribute directives are emitted in the generated assembly,
when -mbranch-protection=gcs is passed.
If the assembler does not support these directives,
.note.gnu.property section will emit the relevant gcs information
in the generated assembly, when -mbranch-protection=gcs is passed.
This patch needs to be applied on top of GCC gcs patch series [2].
Bootstrapped on aarch64-none-linux-gnu and regression tested on
aarch64-none-elf, no issues.
Ok for master?
Regards,
Srinath.
[1]: https://github.com/ARM-software/abi-aa/pull/230
[2]: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs
gcc/ChangeLog:
2024-09-11 Srinath Parvathaneni
* config.in: Regenerated
* config/aarch64/aarch64.cc (aarch64_emit_aeabi_attribute): New
function declaration.
(aarch64_emit_aeabi_subsection): Likewise.
(aarch64_start_file): Emit gcs build attributes.
(aarch64_file_end_indicate_exec_stack): Update gcs bit in
note.gnu.property section.
* configure: Regenerated.
* configure.ac: Add gcc configure check.
gcc/testsuite/ChangeLog:
2024-09-11 Srinath Parvathaneni
* gcc.target/aarch64/build-attribute-gcs.c: New test.
---
gcc/config.in | 6 +++
gcc/config/aarch64/a.out | Bin 0 -> 656 bytes
gcc/config/aarch64/aarch64.cc | 43 ++++++
gcc/configure | 35 ++++++
gcc/configure.ac | 7 +++
.../gcc.target/aarch64/build-attribute-gcs.c | 24 ++
6 files changed, 115 insertions(+)
create mode 100644 gcc/config/aarch64/a.out
create mode 100644 gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
diff --git a/gcc/config.in b/gcc/config.in
index 7fcabbe5061..eb6024dfc90 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -379,6 +379,12 @@
#endif
+/* Define if your assembler supports gcs build attributes. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_BUILD_ATTRIBUTES_GCS
+#endif
+
+
/* Define to the level of your assembler's compressed debug section support.
*/
#ifndef USED_FOR_TARGET
diff --git a/gcc/config/aarch64/a.out b/gcc/config/aarch64/a.out
new file mode 100644
index ..dd7982f2db625be166d33548dc5d00c7e7601629
GIT binary patch
literal 656
zcmb<-^>JfjWMqH=MuzPS2p&w7f#Cvz$>0EHJ20>_upx>confdefs.h
+fi
+
+# Check if we have binutils support for gcs build attributes.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for gcs build attributes support" >&5
+$as_echo_n "checking assembler for gcs build attributes support... " >&6; }
+if ${gcc_cv_as_aarch64_gcs_build_attributes+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ gcc_cv_as_aarch64_gcs_build_attributes=no
+ if test x$gcc_cv_as != x; then
+$as_echo '
+ .aeabi_subsection .aeabi-feature-and-bits, 1, 0
+ .aeabi_attribute 3, 1
+' > conftest.s
+if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; }
+then
+ gcc_cv_as_aarch64_gcs_build_attributes=yes
+else
+ echo "configure: failed program was" >&5
+ cat conftest.s >&5
+fi
+rm -f conftest.o conftest.s
+ fi
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_aarch64_gcs_build_attributes" >&5
+$as_echo "$gcc_cv_as_aarch64_gcs_build_attributes" >&6; }
+if test $gcc_cv_as_aarch64_gcs_build_attributes = yes; then
+
+$as_echo "#define HAVE_AS_BUILD_ATTRIBUTES_GCS 1" >>confdefs.h
+
fi
# Enable Branch Target Identification Mechanism and Return Address
diff --git a/gcc/configure.ac b/gcc/configure.ac
index d0b9865fc91..51b07417153 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -4388,6 +4388,13 @@ case "$target" in
ldr x0, [[x2, #:gotpage_lo15:globalsym]]
],,[AC_DEFINE(HAVE_AS_SMALL_PIC_RELOCS, 1,
[Define if your assembler supports relocs needed by -fpic.])])
+# Check if we have binutils support for gcs build attribute
Re: Is it possible to get an old posed Patch from gcc-patches archive??
Qing Zhao writes: > Hi, > > I was trying to study an old posted patch (but was not approved and > committed) in the following link: > > https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561096.html > > However, I cannot get the patch from the attachment part as following: > > -- next part -- > A non-text attachment was scrubbed... > Name: 0001-Refactor-frecord-gcc-switches.patch > Type: text/x-patch > Size: 24450 bytes > Desc: not available > URL: > <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20201204/3a03000b/attachment-0002.bin> > > I tried to click the URL link at the last line above, also failed… > Is there other way to get the old patch? I can download both of the old URLs. But also: https://inbox.sourceware.org/9159005a-ff96-3db2-65eb-7d5be3faf...@suse.cz/ (you can get the message ID from a meta tag header, it is in an In-Reply-To in the HTML file, and then give it to public-inbox) Hope that helps, have a lovely day. -- Arsen Arsenović signature.asc Description: PGP signature
Re: [PATCH v4] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
> On Sep 10, 2024, at 17:47, Jakub Jelinek wrote:
>
> On Tue, Sep 10, 2024 at 09:28:04PM +, Qing Zhao wrote:
>> @@ -11741,6 +11770,54 @@ c_parser_postfix_expression (c_parser *parser)
>>set_c_expr_source_range (&expr, loc, close_paren_loc);
>>break;
>> }
>> + case RID_BUILTIN_COUNTED_BY_REF:
>> + {
>> +vec *cexpr_list;
>> +c_expr_t *e_p;
>> +location_t close_paren_loc;
>> +
>> +in_builtin_counted_by_ref = true;
>> +
>> +c_parser_consume_token (parser);
>> +if (!c_parser_get_builtin_args (parser,
>> +"__builtin_counted_by_ref",
>> +&cexpr_list, false,
>> +&close_paren_loc))
>> + {
>> + expr.set_error ();
>> + goto error_exit;
>
> Up to Joseph or Marek as C maintainers/reviewers, but I think it is a bad
> idea to use such a generic name for a label inside of handling of one
> specific keyword.
>
> Either use RAII and just break; instead of goto error_exit;, like
>struct in_builtin_counted_by_ref_sentinel {
> ~in_builtin_counted_by_ref_sentinel ()
> { in_builtin_counted_by_ref = false; }
>} ibcbr_sentinel;
> or add those in_builtin_counted_by_ref = false; lines before each break;
Okay, I can add in_builtin_counted_by_ref = false lines before each break.
That might be the most simple and straightforward way to fix this concern. -:)
>
> Or set it just when parsing the args?
>
> Anyway, I'm not even convinced a global variable like that is a good idea.
> The argument can contain arbitrary expressions in there (e.g. comma
> expression, statement expression, ...), I strongly doubt you want to
> have that special handling in all the places in the grammar rather than just
> for the last COMPONENT_REF in there. And, there is no reason why
> you couldn't have e.g. nested call inside of the argument:
> __builtin_counted_by_ref (ptr[*__builtin_counted_by_ref
> (something->fam1)]->fam2)
> and that on the other side clears in_builtin_counted_by_ref after parsing
> the inner one.
Good point here. So, instead of of a boolean type, same as the other globals,
such as
In_typeof, in_sizeof, use an int type to record the level of the nesting might
be more proper.
What’s your suggestion here?
thanks.
Qing
>
> Jakub
>
Is it possible to get an old posed Patch from gcc-patches archive??
Hi, I was trying to study an old posted patch (but was not approved and committed) in the following link: https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561096.html However, I cannot get the patch from the attachment part as following: -- next part -- A non-text attachment was scrubbed... Name: 0001-Refactor-frecord-gcc-switches.patch Type: text/x-patch Size: 24450 bytes Desc: not available URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20201204/3a03000b/attachment-0002.bin> I tried to click the URL link at the last line above, also failed… Is there other way to get the old patch? thanks. Qing
Re: [PATCH v4] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
On Tue, Sep 10, 2024 at 09:28:04PM +, Qing Zhao wrote:
> @@ -11741,6 +11770,54 @@ c_parser_postfix_expression (c_parser *parser)
> set_c_expr_source_range (&expr, loc, close_paren_loc);
> break;
> }
> + case RID_BUILTIN_COUNTED_BY_REF:
> + {
> + vec *cexpr_list;
> + c_expr_t *e_p;
> + location_t close_paren_loc;
> +
> + in_builtin_counted_by_ref = true;
> +
> + c_parser_consume_token (parser);
> + if (!c_parser_get_builtin_args (parser,
> + "__builtin_counted_by_ref",
> + &cexpr_list, false,
> + &close_paren_loc))
> + {
> + expr.set_error ();
> + goto error_exit;
Up to Joseph or Marek as C maintainers/reviewers, but I think it is a bad
idea to use such a generic name for a label inside of handling of one
specific keyword.
Either use RAII and just break; instead of goto error_exit;, like
struct in_builtin_counted_by_ref_sentinel {
~in_builtin_counted_by_ref_sentinel ()
{ in_builtin_counted_by_ref = false; }
} ibcbr_sentinel;
or add those in_builtin_counted_by_ref = false; lines before each break;
Or set it just when parsing the args?
Anyway, I'm not even convinced a global variable like that is a good idea.
The argument can contain arbitrary expressions in there (e.g. comma
expression, statement expression, ...), I strongly doubt you want to
have that special handling in all the places in the grammar rather than just
for the last COMPONENT_REF in there. And, there is no reason why
you couldn't have e.g. nested call inside of the argument:
__builtin_counted_by_ref (ptr[*__builtin_counted_by_ref
(something->fam1)]->fam2)
and that on the other side clears in_builtin_counted_by_ref after parsing
the inner one.
Jakub
[PATCH v4] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
Hi, This is the 4th version of the patch.
Compared to the 3rd version, the only change is the size calculation in
the testing case.
The 3rd version compared to the 2nd version, the major change is:
the update in testing cases per Martin's suggestions.
when the 2nd version is compared to the first version, the major changes are:
1. change the name of the builtin from __builtin_get_counted_by to
__builtin_counted_by_ref in order to reflect the fact that the returned
value of it is a reference to the object.
2. make typeof(__builtin_counted_by_ref) working.
3. update the testing case to use the new builtin inside _Generic.
bootstrapped and regress tested on both X86 and aarch64. no issue.
Okay for the trunk?
thanks.
Qing.
With the addition of the 'counted_by' attribute and its wide roll-out
within the Linux kernel, a use case has been found that would be very
nice to have for object allocators: being able to set the counted_by
counter variable without knowing its name.
For example, given:
struct foo {
...
int counter;
...
struct bar array[] __attribute__((counted_by (counter)));
} *p;
The existing Linux object allocators are roughly:
#define MAX(A, B) (A > B) ? (A) : (B)
#define alloc(P, FAM, COUNT) ({ \
__auto_type __p = &(P); \
size_t __size = MAX (sizeof(*P),
__builtin_offsetof (__typeof(*P), FAM)
+ sizeof (*(P->FAM)) * COUNT); \
*__p = kmalloc(__size); \
})
Right now, any addition of a counted_by annotation must also
include an open-coded assignment of the counter variable after
the allocation:
p = alloc(p, array, how_many);
p->counter = how_many;
In order to avoid the tedious and error-prone work of manually adding
the open-coded counted-by intializations everywhere in the Linux
kernel, a new GCC builtin __builtin_counted_by_ref will be very useful
to be added to help the adoption of the counted-by attribute.
-- Built-in Function: TYPE __builtin_counted_by_ref (PTR)
The built-in function '__builtin_counted_by_ref' checks whether the
array object pointed by the pointer PTR has another object
associated with it that represents the number of elements in the
array object through the 'counted_by' attribute (i.e. the
counted-by object). If so, returns a pointer to the corresponding
counted-by object. If such counted-by object does not exist,
returns a NULL pointer.
This built-in function is only available in C for now.
The argument PTR must be a pointer to an array. The TYPE of the
returned value must be a pointer type pointing to the corresponding
type of the counted-by object or VOID pointer type in case of a
NULL pointer being returned.
With this new builtin, the central allocator could be updated to:
#define MAX(A, B) (A > B) ? (A) : (B)
#define alloc(P, FAM, COUNT) ({ \
__auto_type __p = &(P); \
__auto_type __c = (COUNT); \
size_t __size = MAX (sizeof (*(*__p)),\
__builtin_offsetof (__typeof(*(*__p)),FAM) \
+ sizeof (*((*__p)->FAM)) * __c); \
if ((*__p = kmalloc(__size))) { \
__auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
*_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
} \
})
And then structs can gain the counted_by attribute without needing
additional open-coded counter assignments for each struct, and
unannotated structs could still use the same allocator.
PR c/116016
gcc/c-family/ChangeLog:
* c-common.cc: Add new __builtin_counted_by_ref.
* c-common.h (enum rid): Add RID_BUILTIN_COUNTED_BY_REF.
gcc/c/ChangeLog:
* c-decl.cc (names_builtin_p): Add RID_BUILTIN_COUNTED_BY_REF.
* c-parser.cc (has_counted_by_object): New routine.
(get_counted_by_ref): New routine.
(c_parser_postfix_expression): Handle New RID_BUILTIN_COUNTED_BY_REF.
* c-tree.h: New global in_builtin_counted_by_ref.
* c-typeck.cc (build_component_ref): Enable generating
.ACCESS_WITH_SIZE inside typeof when inside builtin_counted_by_ref.
gcc/ChangeLog:
* doc/extend.texi: Add documentation for __builtin_counted_by_ref.
gcc/testsuite/ChangeLog:
* gcc.dg/builtin-counted-by-ref-1.c: New test.
* gcc.dg/builtin-counted-by-ref.c: New test.
---
gcc/c-family/c-common.cc | 1 +
gcc/c-family/c-common.h | 1 +
gcc/c/c-decl.cc | 1 +
gcc/c/c-parser.cc | 77 +++++++
gcc/c/c-tree.h | 1 +
gcc/c/c-typeck.cc | 7 +-
gcc/doc/extend.texi | 55 +++
.../gcc.dg/builtin-counted-by-ref-1.c | 97 +++
gcc/testsuite/gcc.dg/builtin-co
Re: [PATCH v2] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
> On Sep 10, 2024, at 14:48, Martin Uecker wrote:
>
> Am Dienstag, dem 10.09.2024 um 20:36 +0200 schrieb Jakub Jelinek:
>> On Tue, Sep 10, 2024 at 06:31:23PM +, Qing Zhao wrote:
>>>
>>>
>>>> On Sep 10, 2024, at 14:09, Jakub Jelinek wrote:
>>>>
>>>> On Tue, Sep 10, 2024 at 06:02:45PM +, Qing Zhao wrote:
>>>>>> #define alloc(P, FAM, COUNT) ({ \
>>>>>> __auto_type __p = &(P); \
>>>>>> __auto_type __c = (COUNT); \
>>>>>> size_t __size = sizeof(*(*__p)) + sizeof(*(*__p)->FAM) * __c; \
>>>>
>>>> Shouldn't that be
>>>> size_t __size = offsetof(__typeof(*__p), FAM) + sizeof(*(*__p)->FAM) *
>>>> __c; \
>>>> ?
>>>
>>> Yeah, I think that the correct size computation should be:
>>>
>>> #define MAX(A, B) (A > B) ? (A) : (B)
>>> size_t __size = MAX (sizeof (*(*__p)), offsetof(__typeof(*__p), FAM) +
>>> sizeof(*(*__p)->FAM) * __c); \
>>
>> No, why? sizeof (*(*__p)) should be always >= offsetof(__typeof(*__p), FAM),
>> you can't have an offset outside of a structure (ok, except doing something
>> like use fld[100] as FAM). offsetof + sizeof (elt) * count is the actually
>> needed size, say if it is
>
> (offset + sizeof * c) could be smaller than sizeof (*(*__p))).
Yes, that’s the reason.
[ ~]$ cat t.c
#include
#include
struct flex {
int b;
char other;
char c[];
} *array_annotated;
int main ()
{
printf ("the size of struct is %d \n", sizeof(struct flex));
printf ("the offset of c is %d \n", offsetof(struct flex, c));
return 0;
}
[ ~]$ gcc t.c; ./a.out
the size of struct is 8
the offset of c is 5
Then if we only allocate 2 elements for the FAM “c”, then offset + sizeof
(char) * 2 = 5 + 2 = 7, which is smaller than sizeof (struct flex), 8.
Qing
>
> Martin
>
>
>> struct S { size_t a; char b; __attribute__((counted_by (a))) char c[]; };
>> then you don't really need 2 * sizeof (size_t) + N size of N elements
>> in the flexible array, just sizeof (size_t) + 1 + N is enough.
>>
>> Or is counted_by attribute handling it in some weird way?
>
>>
>> Jakub
>>
>
Re: [PATCH v2] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
Am Dienstag, dem 10.09.2024 um 20:36 +0200 schrieb Jakub Jelinek:
> On Tue, Sep 10, 2024 at 06:31:23PM +, Qing Zhao wrote:
> >
> >
> > > On Sep 10, 2024, at 14:09, Jakub Jelinek wrote:
> > >
> > > On Tue, Sep 10, 2024 at 06:02:45PM +, Qing Zhao wrote:
> > > > > #define alloc(P, FAM, COUNT) ({ \
> > > > > __auto_type __p = &(P); \
> > > > > __auto_type __c = (COUNT); \
> > > > > size_t __size = sizeof(*(*__p)) + sizeof(*(*__p)->FAM) * __c; \
> > >
> > > Shouldn't that be
> > > size_t __size = offsetof(__typeof(*__p), FAM) + sizeof(*(*__p)->FAM) *
> > > __c; \
> > > ?
> >
> > Yeah, I think that the correct size computation should be:
> >
> > #define MAX(A, B) (A > B) ? (A) : (B)
> > size_t __size = MAX (sizeof (*(*__p)), offsetof(__typeof(*__p), FAM) +
> > sizeof(*(*__p)->FAM) * __c); \
>
> No, why? sizeof (*(*__p)) should be always >= offsetof(__typeof(*__p), FAM),
> you can't have an offset outside of a structure (ok, except doing something
> like use fld[100] as FAM). offsetof + sizeof (elt) * count is the actually
> needed size, say if it is
(offset + sizeof * c) could be smaller than sizeof (*(*__p))).
Martin
> struct S { size_t a; char b; __attribute__((counted_by (a))) char c[]; };
> then you don't really need 2 * sizeof (size_t) + N size of N elements
> in the flexible array, just sizeof (size_t) + 1 + N is enough.
>
> Or is counted_by attribute handling it in some weird way?
>
> Jakub
>
Re: [PATCH v2] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
On Tue, Sep 10, 2024 at 06:31:23PM +, Qing Zhao wrote:
>
>
> > On Sep 10, 2024, at 14:09, Jakub Jelinek wrote:
> >
> > On Tue, Sep 10, 2024 at 06:02:45PM +, Qing Zhao wrote:
> >>> #define alloc(P, FAM, COUNT) ({ \
> >>> __auto_type __p = &(P); \
> >>> __auto_type __c = (COUNT); \
> >>> size_t __size = sizeof(*(*__p)) + sizeof(*(*__p)->FAM) * __c; \
> >
> > Shouldn't that be
> > size_t __size = offsetof(__typeof(*__p), FAM) + sizeof(*(*__p)->FAM) *
> > __c; \
> > ?
>
> Yeah, I think that the correct size computation should be:
>
> #define MAX(A, B) (A > B) ? (A) : (B)
> size_t __size = MAX (sizeof (*(*__p)), offsetof(__typeof(*__p), FAM) +
> sizeof(*(*__p)->FAM) * __c); \
No, why? sizeof (*(*__p)) should be always >= offsetof(__typeof(*__p), FAM),
you can't have an offset outside of a structure (ok, except doing something
like use fld[100] as FAM). offsetof + sizeof (elt) * count is the actually
needed size, say if it is
struct S { size_t a; char b; __attribute__((counted_by (a))) char c[]; };
then you don't really need 2 * sizeof (size_t) + N size of N elements
in the flexible array, just sizeof (size_t) + 1 + N is enough.
Or is counted_by attribute handling it in some weird way?
Jakub
Re: [PATCH v2] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
> On Sep 10, 2024, at 14:09, Jakub Jelinek wrote:
>
> On Tue, Sep 10, 2024 at 06:02:45PM +, Qing Zhao wrote:
>>> #define alloc(P, FAM, COUNT) ({ \
>>> __auto_type __p = &(P); \
>>> __auto_type __c = (COUNT); \
>>> size_t __size = sizeof(*(*__p)) + sizeof(*(*__p)->FAM) * __c; \
>
> Shouldn't that be
> size_t __size = offsetof(__typeof(*__p), FAM) + sizeof(*(*__p)->FAM) * __c; \
> ?
Yeah, I think that the correct size computation should be:
#define MAX(A, B) (A > B) ? (A) : (B)
size_t __size = MAX (sizeof (*(*__p)), offsetof(__typeof(*__p), FAM) +
sizeof(*(*__p)->FAM) * __c); \
Qing
>
>>> if ((*__p = malloc(__size))) { \
>>> __auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
>>>*_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
>>> } \
>>> })
>>>
>>> to have brackets around the macro arguments to avoid accidents,
>>> to reduce compile time due to multiple evaluation of the macro
>>> arguments, and to avoid warnings for the null pointer dereference
>>> on clang.
>
> Jakub
>
[PATCH v3] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
Hi, This is the 3rd version of the patch.
compared to the 2nd version, the only change is the update in testing
cases per Martin's suggestions.
when the 2nd version is compared to the first version, the major changes are:
1. change the name of the builtin from __builtin_get_counted_by to
__builtin_counted_by_ref in order to reflect the fact that the returned
value of it is a reference to the object.
2. make typeof(__builtin_counted_by_ref) working.
3. update the testing case to use the new builtin inside _Generic.
bootstrapped and regress tested on both X86 and aarch64. no issue.
Okay for the trunk?
thanks.
Qing.
===
With the addition of the 'counted_by' attribute and its wide roll-out
within the Linux kernel, a use case has been found that would be very
nice to have for object allocators: being able to set the counted_by
counter variable without knowing its name.
For example, given:
struct foo {
...
int counter;
...
struct bar array[] __attribute__((counted_by (counter)));
} *p;
The existing Linux object allocators are roughly:
#define alloc(P, FAM, COUNT) ({ \
size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
__p = kmalloc(__size, GFP); \
})
Right now, any addition of a counted_by annotation must also
include an open-coded assignment of the counter variable after
the allocation:
p = alloc(p, array, how_many);
p->counter = how_many;
In order to avoid the tedious and error-prone work of manually adding
the open-coded counted-by intializations everywhere in the Linux
kernel, a new GCC builtin __builtin_counted_by_ref will be very useful
to be added to help the adoption of the counted-by attribute.
-- Built-in Function: TYPE __builtin_counted_by_ref (PTR)
The built-in function '__builtin_counted_by_ref' checks whether the
array object pointed by the pointer PTR has another object
associated with it that represents the number of elements in the
array object through the 'counted_by' attribute (i.e. the
counted-by object). If so, returns a pointer to the corresponding
counted-by object. If such counted-by object does not exist,
returns a NULL pointer.
This built-in function is only available in C for now.
The argument PTR must be a pointer to an array. The TYPE of the
returned value must be a pointer type pointing to the corresponding
type of the counted-by object or VOID pointer type in case of a
NULL pointer being returned.
With this new builtin, the central allocator could be updated to:
#define alloc(P, FAM, COUNT) ({ \
__auto_type __p = &(P); \
__auto_type __c = (COUNT); \
size_t __size = sizeof(*(*__p)) + sizeof(*((*__p)->FAM)) * __c; \
if ((*__p = kmalloc(__size))) { \
__auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
*_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
} \
})
And then structs can gain the counted_by attribute without needing
additional open-coded counter assignments for each struct, and
unannotated structs could still use the same allocator.
PR c/116016
gcc/c-family/ChangeLog:
* c-common.cc: Add new __builtin_counted_by_ref.
* c-common.h (enum rid): Add RID_BUILTIN_COUNTED_BY_REF.
gcc/c/ChangeLog:
* c-decl.cc (names_builtin_p): Add RID_BUILTIN_COUNTED_BY_REF.
* c-parser.cc (has_counted_by_object): New routine.
(get_counted_by_ref): New routine.
(c_parser_postfix_expression): Handle New RID_BUILTIN_COUNTED_BY_REF.
* c-tree.h: New global in_builtin_counted_by_ref.
* c-typeck.cc (build_component_ref): Enable generating
.ACCESS_WITH_SIZE inside typeof when inside builtin_counted_by_ref.
gcc/ChangeLog:
* doc/extend.texi: Add documentation for __builtin_counted_by_ref.
gcc/testsuite/ChangeLog:
* gcc.dg/builtin-counted-by-ref-1.c: New test.
* gcc.dg/builtin-counted-by-ref.c: New test.
---
gcc/c-family/c-common.cc | 1 +
gcc/c-family/c-common.h | 1 +
gcc/c/c-decl.cc | 1 +
gcc/c/c-parser.cc | 77 +++
gcc/c/c-tree.h| 1 +
gcc/c/c-typeck.cc | 7 +-
gcc/doc/extend.texi | 55 +++
.../gcc.dg/builtin-counted-by-ref-1.c | 94 +++
gcc/testsuite/gcc.dg/builtin-counted-by-ref.c | 58
9 files changed, 294 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/builtin-counted-by-ref-1.c
create mode 100644 gcc/testsuite/gcc.dg/builtin-counted-by-ref.c
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index e7e371fd26f..15b90bae8b5 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -430,6 +430,7 @@ const struct c_common_re
Re: [PATCH v2] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
On Tue, Sep 10, 2024 at 06:02:45PM +, Qing Zhao wrote:
> > #define alloc(P, FAM, COUNT) ({ \
> > __auto_type __p = &(P); \
> > __auto_type __c = (COUNT); \
> > size_t __size = sizeof(*(*__p)) + sizeof(*(*__p)->FAM) * __c; \
Shouldn't that be
size_t __size = offsetof(__typeof(*__p), FAM) + sizeof(*(*__p)->FAM) * __c; \
?
> > if ((*__p = malloc(__size))) { \
> >__auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
> > *_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
> > } \
> > })
> >
> > to have brackets around the macro arguments to avoid accidents,
> > to reduce compile time due to multiple evaluation of the macro
> > arguments, and to avoid warnings for the null pointer dereference
> > on clang.
Jakub
Re: [PATCH v2] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
Thanks a lot for the tips.
I updated the 2 testing cases per your suggestion, they work well.
I will send the 3rd version of the patch soon.
Qing
> On Sep 10, 2024, at 11:42, Martin Uecker wrote:
>
> Am Dienstag, dem 10.09.2024 um 13:51 + schrieb Qing Zhao:
>> #define alloc(P, FAM, COUNT) ({ \
>> typeof(P) __p; \
>> size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
>> __p = kmalloc(__size, GFP); \
>> typeof(_Generic(__builtin_counted_by_ref(__p->FAM), \
>> void *: (size_t *)NULL, \
>> default: __builtin_counted_by_ref(__p->FAM))) \
>> ret = __builtin_counted_by_ref(__p->FAM); \
>> if (ret) \
>> *ret = COUNT; \
>> P = __p; \
>> })
>
> Maybe not too relevant for your patch, but I would use
> something like this
>
> #define alloc(P, FAM, COUNT) ({ \
> __auto_type __p = &(P); \
> __auto_type __c = (COUNT); \
> size_t __size = sizeof(*(*__p)) + sizeof(*(*__p)->FAM) * __c; \
> if ((*__p = malloc(__size))) { \
>__auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
> *_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
> } \
> })
>
> to have brackets around the macro arguments to avoid accidents,
> to reduce compile time due to multiple evaluation of the macro
> arguments, and to avoid warnings for the null pointer dereference
> on clang.
>
> (Actually, I would also cast the sizes to a signed type
> immediately to catch overflows with UBSan, but I kept
> size_t here.)
>
> Martin
>
>
Re: [PATCH v2] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
Am Dienstag, dem 10.09.2024 um 13:51 + schrieb Qing Zhao:
> #define alloc(P, FAM, COUNT) ({ \
> typeof(P) __p; \
> size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> __p = kmalloc(__size, GFP); \
> typeof(_Generic(__builtin_counted_by_ref(__p->FAM), \
> void *: (size_t *)NULL, \
> default: __builtin_counted_by_ref(__p->FAM))) \
> ret = __builtin_counted_by_ref(__p->FAM); \
> if (ret) \
> *ret = COUNT; \
> P = __p; \
> })
Maybe not too relevant for your patch, but I would use
something like this
#define alloc(P, FAM, COUNT) ({ \
__auto_type __p = &(P); \
__auto_type __c = (COUNT); \
size_t __size = sizeof(*(*__p)) + sizeof(*(*__p)->FAM) * __c; \
if ((*__p = malloc(__size))) { \
__auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
*_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
} \
})
to have brackets around the macro arguments to avoid accidents,
to reduce compile time due to multiple evaluation of the macro
arguments, and to avoid warnings for the null pointer dereference
on clang.
(Actually, I would also cast the sizes to a signed type
immediately to catch overflows with UBSan, but I kept
size_t here.)
Martin
[PATCH v2] Provide new GCC builtin __builtin_counted_by_ref [PR116016]
Hi,
This is the 2nd version of the patch after the long discussion. We finally
decided to keep the previous design that returns a pointer to the counted_by
object.
Compared to the first version, the major changes are:
1. change the name of the builtin from __builtin_get_counted_by to
__builtin_counted_by_ref in order to reflect the fact that the returned
value of it is a reference to the object.
2. make typeof(__builtin_counted_by_ref) working.
3. update the testing case to use the new builtin inside _Generic.
bootstrapped and regress tested on both X86 and aarch64. no issue.
Okay for the trunk?
thanks.
Qing.
With the addition of the 'counted_by' attribute and its wide roll-out
within the Linux kernel, a use case has been found that would be very
nice to have for object allocators: being able to set the counted_by
counter variable without knowing its name.
For example, given:
struct foo {
...
int counter;
...
struct bar array[] __attribute__((counted_by (counter)));
} *p;
The existing Linux object allocators are roughly:
#define alloc(P, FAM, COUNT) ({ \
typeof(P) __p; \
size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
__p = kmalloc(__size, GFP); \
P = __p; \
})
Right now, any addition of a counted_by annotation must also
include an open-coded assignment of the counter variable after
the allocation:
p = alloc(p, array, how_many);
p->counter = how_many;
In order to avoid the tedious and error-prone work of manually adding
the open-coded counted-by intializations everywhere in the Linux
kernel, a new GCC builtin __builtin_counted_by_ref will be very useful
to be added to help the adoption of the counted-by attribute.
-- Built-in Function: TYPE __builtin_counted_by_ref (PTR)
The built-in function '__builtin_counted_by_ref' checks whether the
array object pointed by the pointer PTR has another object
associated with it that represents the number of elements in the
array object through the 'counted_by' attribute (i.e. the
counted-by object). If so, returns a pointer to the corresponding
counted-by object. If such counted-by object does not exist,
returns a NULL pointer.
This built-in function is only available in C for now.
The argument PTR must be a pointer to an array. The TYPE of the
returned value must be a pointer type pointing to the corresponding
type of the counted-by object or VOID pointer type in case of a
NULL pointer being returned.
With this new builtin, the central allocator could be updated to:
#define alloc(P, FAM, COUNT) ({ \
typeof(P) __p; \
size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
__p = kmalloc(__size, GFP); \
typeof(_Generic(__builtin_counted_by_ref(__p->FAM), \
void *: (size_t *)NULL, \
default: __builtin_counted_by_ref(__p->FAM))) \
ret = __builtin_counted_by_ref(__p->FAM); \
if (ret) \
*ret = COUNT; \
P = __p; \
})
And then structs can gain the counted_by attribute without needing
additional open-coded counter assignments for each struct, and
unannotated structs could still use the same allocator.
PR c/116016
gcc/c-family/ChangeLog:
* c-common.cc: Add new __builtin_counted_by_ref.
* c-common.h (enum rid): Add RID_BUILTIN_COUNTED_BY_REF.
gcc/c/ChangeLog:
* c-decl.cc (names_builtin_p): Add RID_BUILTIN_COUNTED_BY_REF.
* c-parser.cc (has_counted_by_object): New routine.
(get_counted_by_ref): New routine.
(c_parser_postfix_expression): Handle New RID_BUILTIN_COUNTED_BY_REF.
* c-tree.h: New global in_builtin_counted_by_ref.
* c-typeck.cc (build_component_ref): Enable generating
.ACCESS_WITH_SIZE inside typeof when inside builtin_counted_by_ref.
gcc/ChangeLog:
* doc/extend.texi: Add documentation for __builtin_counted_by_ref.
gcc/testsuite/ChangeLog:
* gcc.dg/builtin-counted-by-ref-1.c: New test.
* gcc.dg/builtin-counted-by-ref.c: New test.
---
gcc/c-family/c-common.cc | 1 +
gcc/c-family/c-common.h | 1 +
gcc/c/c-decl.cc | 1 +
gcc/c/c-parser.cc | 77 +++
gcc/c/c-tree.h| 1 +
gcc/c/c-typeck.cc | 7 +-
gcc/doc/extend.texi | 55 +++
.../gcc.dg/builtin-counted-by-ref-1.c | 97 +++
gcc/testsuite/gcc.dg/builtin-counted-by-ref.c | 58 +++
9 files changed, 297 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/builtin-counted-by-ref-1.c
create mode 100644 gcc/testsuite/gcc.dg/builtin-counted-by-ref.c
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index e7e371fd26f..15b90bae8b5 100644
---
Re: [PATCH] ada: Fix gcc-interface/misc.cc compilation on SPARC
Eric Botcazou writes: >> commit 72c6938f29cbeddb3220720e68add4cf09ffd794 >> Author: Eric Botcazou >> Date: Sun Aug 25 15:20:59 2024 +0200 >> >> ada: Streamline handling of low-level peculiarities of record field >> layout >> >> broke the Ada build on SPARC: >> >> In file included from ./tm_p.h:4, >> from >> /vol/gcc/src/hg/master/local/gcc/ada/gcc-interface/misc.cc:31: >> /vol/gcc/src/hg/master/local/gcc/config/sparc/sparc-protos.h:46:47: error: >> use of enum ‘memmodel’ without previous declaration 46 | extern void >> sparc_emit_membar_for_model (enum memmodel, int, int); >> | ^~~~ >> >> Fixed by including memmodel.h. > > Sorry about that, a small merge glitch, the fix is already in the pipeline. Hello, Éric's fix has been merged this morning as r15-3568-g73dc46f47be615. Marc
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Sun, Sep 8, 2024 at 3:07 AM Martin Uecker wrote:
>
> Am Sonntag, dem 08.09.2024 um 02:09 -0700 schrieb Bill Wendling:
> > On Fri, Sep 6, 2024 at 10:50 PM Martin Uecker wrote:
> > >
> > > Am Freitag, dem 06.09.2024 um 13:59 -0700 schrieb Bill Wendling:
> > > > On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker
> > > > wrote:
> > > > > > > >
>
> ...
> > > > >
> > > > > My recommendation would be not to change __builtin_choose_expr.
> > > > >
> > > > > The design where __builtin_get_counted_by returns a null
> > > > > pointer constant (void*)0 seems good. Most users will
> > > > > get an error which I think is what we want and for those
> > > > > that want it to work even if the attribute is not there, the
> > > > > following code seems perfectly acceptable to me:
> > > > >
> > > > > auto p = __builtin_get_counted_by(__p->FAM)
> > > > > *_Generic(p, void*: &(int){}, default: p) = 1;
> > > > >
> > > > >
> > > > > Kees also seemed happy with it. And if I understood it correctly,
> > > > > also Clang's bounds checking people can work with this.
> > > > >
> > > > The problem with this is explained in the Clang RFC [1]. Apple's team
> > > > rejects taking the address of the 'counter' field when using
> > > > -fbounds-safety. They suggested this as an alternative:
> > > >
> > > > __builtin_bounds_attr_arg(ptr->FAM) = COUNT;
> > > >
> > > > The __builtin_bounds_attr_arg(ptr->FAM) is replaced by an L-value to
> > > > the 'ptr->count' field during SEMA, and life goes on as normal. There
> > > > are a few reasons for this:
> > > >
> > > > 1. They want to track the relationship between the FAM and the
> > > > counter so that one isn't modified without the other being modified.
> > > > Allowing the address to be taken makes that check vastly harder.
> > >
> > > In the thread it is pointed out that returning a pointer works
> > > too, and they would simply restrict passing the pointer elsewhere.
> > >
> > It's in rapidsna's initial reply titled "Taking the address of a count
> > variable is forbidden":
> >
> >
> > https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/2
> >
> > I didn't see her retreating from that idea.
>
> The initial proposal already has this as "Alternative Design"
> and then there is this response to my comment:
>
> https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/27
>
> which seems to imply that she is open to this idea.
>
Oh! I missed that reply. Sorry about that. If they're open to
returning pointers all the better!
> > > I can't see "Apple's team rejects" and "vastly harder" at the
> > > moment.
> > >
> > > >
> > > > 2. Apple's implementation supports expressions in the '__counted_by'
> > > > attribute, thus the 'count' may be an R-value that can't have its
> > > > address taken.
> > > >
> > > > [1]
> > > > https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/
> > >
> > > Yes, this would be a limitation.
> > >
> > And not one that I'm particularly excited about (allowing for (nearly)
> > arbitrary expressions in the 'counted_by' attribute that is).
>
> And I do not see how returning an r-value can work with the
> intended use case for __builtin_get_counted_by() anyway, because
> this would break assignments.
>
> So I do not see any issue against returning a pointer and (void*)0
> when there is no attribute and if the expression is a r-value.
>
I agree.
-bw
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Sep 9, 2024, at 10:20, Jakub Jelinek wrote:
>
> On Mon, Sep 09, 2024 at 02:10:05PM +, Qing Zhao wrote:
>> Okay, now after finishing reading all the discussion so far, I realized that
>> we are back to the previous pointer approach:
>>
>> __builtin_get_counted_by (p->FAM)
>>
>> Works as:
>>
>> If (has_counted_by (p->FAM))
>> return &p->COUNT;
>> else
>> return (void *)0;
>>
>> Then the user will use it as:
>>
>> auto p = __builtin_get_counted_by(__p->FAM)
>> *_Generic(p, void*: &(int){}, default: p) = COUNT;
>>
>> The major reason for back to the previous pointer approach is (from Martin):
>>
>> "The initial proposal already has this as "Alternative Design"
>> and then there is this response to my comment:
>>
>> https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/27
>>
>> which seems to imply that she is open to this idea.
>
> I'd strongly prefer the pointer variant over some ugly hack like what is
> proposed for the returning lvalue case. Yes, returning various different
> pointer types is still not a normal builtin, but it is in line with various
> overloaded builtins where the return type depends on the argument types
> or other details, like __atomic_{load,exchange,compare_exchage} etc.
Yes. That’s reasonable.
>
> clang with -fbounds-safety can impose some restrictions like that the
> pointer shouldn't escape the current function and be just dereferenced
> and not say cast to integer etc.
> If they "return lvalue", what prevents &__builtin_whatever (p->FAM)?
> And I still don't understand how they can avoid taking the address of the
> counter, can't one bypass that by say
> (int *) ((char *)&obj + offsetof (struct something, obj))
My understainding from
https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/27
Is:
Apple agreed to the approach of returning a pointer and also impose some
restriction similar as your suggested:
"what we could also do is to design __builtin_bounds_attr_arg as returning a
pointer similar to __builtin_get_counted_by and then prevent it from being
assigned to another variable or passed as an argument, with or without
-fbounds-safety.?”
So, I think that we can keep the previous approach of retuning a pointer.
Thanks.
Qing
>
> Jakub
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Mon, Sep 09, 2024 at 02:10:05PM +, Qing Zhao wrote:
> Okay, now after finishing reading all the discussion so far, I realized that
> we are back to the previous pointer approach:
>
> __builtin_get_counted_by (p->FAM)
>
> Works as:
>
> If (has_counted_by (p->FAM))
> return &p->COUNT;
> else
> return (void *)0;
>
> Then the user will use it as:
>
> auto p = __builtin_get_counted_by(__p->FAM)
> *_Generic(p, void*: &(int){}, default: p) = COUNT;
>
> The major reason for back to the previous pointer approach is (from Martin):
>
> "The initial proposal already has this as "Alternative Design"
> and then there is this response to my comment:
>
> https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/27
>
> which seems to imply that she is open to this idea.
I'd strongly prefer the pointer variant over some ugly hack like what is
proposed for the returning lvalue case. Yes, returning various different
pointer types is still not a normal builtin, but it is in line with various
overloaded builtins where the return type depends on the argument types
or other details, like __atomic_{load,exchange,compare_exchage} etc.
clang with -fbounds-safety can impose some restrictions like that the
pointer shouldn't escape the current function and be just dereferenced
and not say cast to integer etc.
If they "return lvalue", what prevents &__builtin_whatever (p->FAM)?
And I still don't understand how they can avoid taking the address of the
counter, can't one bypass that by say
(int *) ((char *)&obj + offsetof (struct something, obj))
?
Jakub
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Okay, now after finishing reading all the discussion so far, I realized that we
are back to the previous pointer approach:
__builtin_get_counted_by (p->FAM)
Works as:
If (has_counted_by (p->FAM))
return &p->COUNT;
else
return (void *)0;
Then the user will use it as:
auto p = __builtin_get_counted_by(__p->FAM)
*_Generic(p, void*: &(int){}, default: p) = COUNT;
The major reason for back to the previous pointer approach is (from Martin):
"The initial proposal already has this as "Alternative Design"
and then there is this response to my comment:
https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/27
which seems to imply that she is open to this idea.
“
I think that this is reasonable.
Then, let’s keep the previous pointer approach.
I will make sure the above _Generic approach work as well from the user’s side
first.
Let me know if you have other opinions.
Thanks a lot!
Qing
> On Sep 8, 2024, at 06:07, Martin Uecker wrote:
>
> Am Sonntag, dem 08.09.2024 um 02:09 -0700 schrieb Bill Wendling:
>> On Fri, Sep 6, 2024 at 10:50 PM Martin Uecker wrote:
>>>
>>> Am Freitag, dem 06.09.2024 um 13:59 -0700 schrieb Bill Wendling:
On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
>
> ...
>
> My recommendation would be not to change __builtin_choose_expr.
>
> The design where __builtin_get_counted_by returns a null
> pointer constant (void*)0 seems good. Most users will
> get an error which I think is what we want and for those
> that want it to work even if the attribute is not there, the
> following code seems perfectly acceptable to me:
>
> auto p = __builtin_get_counted_by(__p->FAM)
> *_Generic(p, void*: &(int){}, default: p) = 1;
>
>
> Kees also seemed happy with it. And if I understood it correctly,
> also Clang's bounds checking people can work with this.
>
The problem with this is explained in the Clang RFC [1]. Apple's team
rejects taking the address of the 'counter' field when using
-fbounds-safety. They suggested this as an alternative:
__builtin_bounds_attr_arg(ptr->FAM) = COUNT;
The __builtin_bounds_attr_arg(ptr->FAM) is replaced by an L-value to
the 'ptr->count' field during SEMA, and life goes on as normal. There
are a few reasons for this:
1. They want to track the relationship between the FAM and the
counter so that one isn't modified without the other being modified.
Allowing the address to be taken makes that check vastly harder.
>>>
>>> In the thread it is pointed out that returning a pointer works
>>> too, and they would simply restrict passing the pointer elsewhere.
>>>
>> It's in rapidsna's initial reply titled "Taking the address of a count
>> variable is forbidden":
>>
>>
>> https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/2
>>
>> I didn't see her retreating from that idea.
>
> The initial proposal already has this as "Alternative Design"
> and then there is this response to my comment:
>
> https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/27
>
> which seems to imply that she is open to this idea.
>
>
>>> I can't see "Apple's team rejects" and "vastly harder" at the
>>> moment.
>>>
2. Apple's implementation supports expressions in the '__counted_by'
attribute, thus the 'count' may be an R-value that can't have its
address taken.
[1]
https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/
>>>
>>> Yes, this would be a limitation.
>>>
>> And not one that I'm particularly excited about (allowing for (nearly)
>> arbitrary expressions in the 'counted_by' attribute that is).
>
> And I do not see how returning an r-value can work with the
> intended use case for __builtin_get_counted_by() anyway, because
> this would break assignments.
>
> So I do not see any issue against returning a pointer and (void*)0
> when there is no attribute and if the expression is a r-value.
>
> Martin
>
>
>
>
>
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Sep 7, 2024, at 02:16, Martin Uecker wrote:
>
> Am Samstag, dem 07.09.2024 um 00:12 + schrieb Qing Zhao:
>> Now, if
>>
>> 1. __builtin_get_counted_by should return a LVALUE instead of a pointer
>> (required by CLANG’s design)
>> And
>> 2. It’s better not to change the behavior of __builtin_choose_expr.
>>
>> Then the solution left is:
>>
>> __builtin_get_counted_by (p->FAM) returns a LVALUE as p->COUNT if p->FAM has
>> a counted_by attribute, if p->FAM does not have a counted_by attribute,
>> silently do nothing. (Or just issue warning if Linux is OKEY with such
>> waning).
>
> What does silently do nothing mean?
>
> /* do nothing */ = counter;
>
> will still fail to compile. So I guess you have something
> else in mind?
Ah, you are right here -:)
In my current implementation, I am doing the following:
If (p->FAM does not have counted_by attribute)
{
error_at (loc, "the argument must have % "
"attribute %<__builtin_counted_by_ref%>”);
expr.set_error ();
goto error_exit;
}
i.e, when there is no counted_by, compilation time error.
So If I eliminate the compilation time error, this approach will still fail
during compilation time…
>
> The new _Generic selection also works if you return a
> lvalue of type void:
>
> struct foo x;
> _Generic(typeof(__counted_by(&x)), void: (int){ 0 },
>default: __counted_by(&x)) = 10;
Thanks a lot for the suggestion!
Yeah, looks like for the lvalue approach, the following will work:
1. When there is no counted_by attribute, return a LVALUE with type void;
2. Using the above _Generic expression to choose the expression based on the
type
>
> https://godbolt.org/z/41E3oj84o
>
> So why not do this then?
Thanks for the suggestion.
Qing
>
> Martin
>
>>
>> Is this acceptable?
>>
>> thanks.
>>
>> Qing
>>> On Sep 6, 2024, at 16:59, Bill Wendling wrote:
>>>
>>> On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
>>>>
>>>> Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
>>>>>
>>>>>> On Sep 5, 2024, at 18:22, Bill Wendling wrote:
>>>>>>
>>>>>> Hi Qing,
>>>>>>
>>>>>> Sorry for my late reply.
>>>>>>
>>>>>> On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Thanks for the information.
>>>>>>>
>>>>>>> Yes, providing a unary operator similar as __counted_by(PTR) as
>>>>>>> suggested by multiple people previously is a cleaner approach.
>>>>>>>
>>>>>>> Then the programmer will use the following:
>>>>>>>
>>>>>>> __builtin_choose_expr(
>>>>>>> __builtin_has_attribute (__p->FAM, "counted_by”)
>>>>>>> __builtin_get_counted_by(__p->FAM) = COUNT, 0);
>>>>>>>
>>>>>>> From the programmer’s point of view, it’s cleaner too.
>>>>>>>
>>>>>>> However, there is one issue with “__builtin_choose_expr” currently in
>>>>>>> GCC, its documentation explicitly mentions this limitation:
>>>>>>> (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
>>>>>>>
>>>>>>> "Note: This construct is only available for C. Furthermore, the unused
>>>>>>> expression (exp1 or exp2 depending on the value of const_exp) may still
>>>>>>> generate syntax errors. This may change in future revisions.”
>>>>>>>
>>>>>>> So, due to this limitation, when there is no counted_by attribute, the
>>>>>>> __builtin_get_counted_by() still is evaluated by the compiler and
>>>>>>> errors is issued and the compilation stops, this can be show from the
>>>>>>> small testing case:
>>>>>>>
>>>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
>>>>>>>
>>>>>>> struct flex {
>>>>>>> unsigned int b;
>>>>>>> int c[];
>>>>>>> } *array_flex;
>>>>>>>
>>>>>>> #define MY_ALLOC(P, FAM, COUNT) ({ \
>>>>>>> typeof(P) __p; \
>>&
Re: [PATCH 1/4] testsuite: Use dg-compile, not gcc -c
On 8/23/24 09:41, Jan Hubicka wrote:
Since this is a pure compile test it makes sense to inform dejagnu of
it.
gcc/testsuite/ChangeLog:
* gcc.misc-tests/gcov-23.c: Use dg-compile, not gcc -c
OK,
Honza
Thanks,
Pushed.
---
gcc/testsuite/gcc.misc-tests/gcov-23.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-23.c
b/gcc/testsuite/gcc.misc-tests/gcov-23.c
index 72849d80e3a..72ba0aa1389 100644
--- a/gcc/testsuite/gcc.misc-tests/gcov-23.c
+++ b/gcc/testsuite/gcc.misc-tests/gcov-23.c
@@ -1,4 +1,5 @@
-/* { dg-options "-fcondition-coverage -ftest-coverage -O2 -c" } */
+/* { dg-options "-fcondition-coverage -ftest-coverage -O2" } */
+/* { dg-do compile } */
#include
#include
--
2.39.2
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Sonntag, dem 08.09.2024 um 02:09 -0700 schrieb Bill Wendling:
> On Fri, Sep 6, 2024 at 10:50 PM Martin Uecker wrote:
> >
> > Am Freitag, dem 06.09.2024 um 13:59 -0700 schrieb Bill Wendling:
> > > On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
> > > > > > >
...
> > > >
> > > > My recommendation would be not to change __builtin_choose_expr.
> > > >
> > > > The design where __builtin_get_counted_by returns a null
> > > > pointer constant (void*)0 seems good. Most users will
> > > > get an error which I think is what we want and for those
> > > > that want it to work even if the attribute is not there, the
> > > > following code seems perfectly acceptable to me:
> > > >
> > > > auto p = __builtin_get_counted_by(__p->FAM)
> > > > *_Generic(p, void*: &(int){}, default: p) = 1;
> > > >
> > > >
> > > > Kees also seemed happy with it. And if I understood it correctly,
> > > > also Clang's bounds checking people can work with this.
> > > >
> > > The problem with this is explained in the Clang RFC [1]. Apple's team
> > > rejects taking the address of the 'counter' field when using
> > > -fbounds-safety. They suggested this as an alternative:
> > >
> > > __builtin_bounds_attr_arg(ptr->FAM) = COUNT;
> > >
> > > The __builtin_bounds_attr_arg(ptr->FAM) is replaced by an L-value to
> > > the 'ptr->count' field during SEMA, and life goes on as normal. There
> > > are a few reasons for this:
> > >
> > > 1. They want to track the relationship between the FAM and the
> > > counter so that one isn't modified without the other being modified.
> > > Allowing the address to be taken makes that check vastly harder.
> >
> > In the thread it is pointed out that returning a pointer works
> > too, and they would simply restrict passing the pointer elsewhere.
> >
> It's in rapidsna's initial reply titled "Taking the address of a count
> variable is forbidden":
>
>
> https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/2
>
> I didn't see her retreating from that idea.
The initial proposal already has this as "Alternative Design"
and then there is this response to my comment:
https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/27
which seems to imply that she is open to this idea.
> > I can't see "Apple's team rejects" and "vastly harder" at the
> > moment.
> >
> > >
> > > 2. Apple's implementation supports expressions in the '__counted_by'
> > > attribute, thus the 'count' may be an R-value that can't have its
> > > address taken.
> > >
> > > [1]
> > > https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/
> >
> > Yes, this would be a limitation.
> >
> And not one that I'm particularly excited about (allowing for (nearly)
> arbitrary expressions in the 'counted_by' attribute that is).
And I do not see how returning an r-value can work with the
intended use case for __builtin_get_counted_by() anyway, because
this would break assignments.
So I do not see any issue against returning a pointer and (void*)0
when there is no attribute and if the expression is a r-value.
Martin
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Sonntag, dem 08.09.2024 um 02:26 -0700 schrieb Bill Wendling: > On Sat, Sep 7, 2024 at 12:21 AM Jakub Jelinek wrote: > > On Sat, Sep 07, 2024 at 07:50:29AM +0200, Martin Uecker wrote: > > > > 2. Apple's implementation supports expressions in the '__counted_by' > > > > attribute, thus the 'count' may be an R-value that can't have its > > > > address taken. > > > > > > > > [1] > > > > https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/ > > > > > > Yes, this would be a limitation. > > > > But then you really can't return an lvalue either, all you can return is an > > rvalue in that case. So just return a (void*)0 in that case. > > > The builtin will return an r-value regardless. The idea is that if the > counter can't be used as an l-value, we wouldn't add an implicit > r-value -> l-value cast (an internal Clang AST node). In the cases > where it can be used as an l-value, we add the implicit cast. I > imagine GCC has some kind of analogue to that? The '(void *)0' case > could be reserved only for when the builtin is used as an l-value but > the r-value can't be cast as an l-value... I do not know what a r-value -> l-value cast is or why Clang needs it. I guess it backconverts an r-value which came from a l-value to an lvalue... The issue with returning r-value when the counter expression is not an lvalue referring to a struct member is that the user would also need yet another new mechanism to detect this case at compile-time. Martin
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Fri, Sep 6, 2024 at 11:16 PM Martin Uecker wrote:
>
> Am Samstag, dem 07.09.2024 um 00:12 + schrieb Qing Zhao:
> > Now, if
> >
> > 1. __builtin_get_counted_by should return a LVALUE instead of a pointer
> > (required by CLANG’s design)
> > And
> > 2. It’s better not to change the behavior of __builtin_choose_expr.
> >
> > Then the solution left is:
> >
> > __builtin_get_counted_by (p->FAM) returns a LVALUE as p->COUNT if p->FAM
> > has a counted_by attribute, if p->FAM does not have a counted_by attribute,
> > silently do nothing. (Or just issue warning if Linux is OKEY with such
> > waning).
>
> What does silently do nothing mean?
>
> /* do nothing */ = counter;
>
> will still fail to compile. So I guess you have something
> else in mind?
>
>
> The new _Generic selection also works if you return a
> lvalue of type void:
>
> struct foo x;
> _Generic(typeof(__counted_by(&x)), void: (int){ 0 },
> default: __counted_by(&x)) = 10;
>
> https://godbolt.org/z/41E3oj84o
>
> So why not do this then?
>
I'm for any solution that a.) works with the fewest changes needed to
other builtins, and 2.) isn't super convoluted. The '_Generic' looks
like a potential compromise.
-bw
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Fri, Sep 6, 2024 at 5:12 PM Qing Zhao wrote:
>
> Now, if
>
> 1. __builtin_get_counted_by should return a LVALUE instead of a pointer
> (required by CLANG’s design)
> And
> 2. It’s better not to change the behavior of __builtin_choose_expr.
>
> Then the solution left is:
>
> __builtin_get_counted_by (p->FAM) returns a LVALUE as p->COUNT if p->FAM has
> a counted_by attribute, if p->FAM does not have a counted_by attribute,
> silently do nothing. (Or just issue warning if Linux is OKEY with such
> waning).
>
> Is this acceptable?
>
We might have to finagle things a bit better, as the compiler will
probably be annoyed at whatever gets generated by the code when the
'counted_by' attribute isn't there. For example, do we implicitly
change it into '*(void *)0 = COUNT;' and expect that the
'__builtin_has_attribute' check was already done? Do we remove the
assignment expression entirely? If this is all happening inside
'__builtin_choose_expr' (or '_Generic'?) then it might not be too much
of a concern. But if it's outside of that builtin, trouble? I don't
rightly know the answers to those questions. I'm not a big fan of
turning an assignment expression into a no-op outside of
'__builtin_choose_expr'...
Let me think about it some more.
If all else fails and we need that warning, we could always add a
`-Wno-` to Linux. They have a long history of hating compiler
warnings, even in cases where they explicitly *ask* for the warnings,
so it's not likely to run into a NACK or anything...
> thanks.
>
> Qing
> > On Sep 6, 2024, at 16:59, Bill Wendling wrote:
> >
> > On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
> >>
> >> Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
> >>>
> >>>> On Sep 5, 2024, at 18:22, Bill Wendling wrote:
> >>>>
> >>>> Hi Qing,
> >>>>
> >>>> Sorry for my late reply.
> >>>>
> >>>> On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> Thanks for the information.
> >>>>>
> >>>>> Yes, providing a unary operator similar as __counted_by(PTR) as
> >>>>> suggested by multiple people previously is a cleaner approach.
> >>>>>
> >>>>> Then the programmer will use the following:
> >>>>>
> >>>>> __builtin_choose_expr(
> >>>>>__builtin_has_attribute (__p->FAM, "counted_by”)
> >>>>>__builtin_get_counted_by(__p->FAM) = COUNT, 0);
> >>>>>
> >>>>> From the programmer’s point of view, it’s cleaner too.
> >>>>>
> >>>>> However, there is one issue with “__builtin_choose_expr” currently in
> >>>>> GCC, its documentation explicitly mentions this limitation:
> >>>>> (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
> >>>>>
> >>>>> "Note: This construct is only available for C. Furthermore, the unused
> >>>>> expression (exp1 or exp2 depending on the value of const_exp) may still
> >>>>> generate syntax errors. This may change in future revisions.”
> >>>>>
> >>>>> So, due to this limitation, when there is no counted_by attribute, the
> >>>>> __builtin_get_counted_by() still is evaluated by the compiler and
> >>>>> errors is issued and the compilation stops, this can be show from the
> >>>>> small testing case:
> >>>>>
> >>>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
> >>>>>
> >>>>> struct flex {
> >>>>> unsigned int b;
> >>>>> int c[];
> >>>>> } *array_flex;
> >>>>>
> >>>>> #define MY_ALLOC(P, FAM, COUNT) ({ \
> >>>>> typeof(P) __p; \
> >>>>> unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> >>>>> __p = (typeof(P)) __builtin_malloc(__size); \
> >>>>> __builtin_choose_expr( \
> >>>>> __builtin_has_attribute (__p->FAM, counted_by), \
> >>>>> __builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
> >>>>> P = __p; \
> >>>>> })
> >>>>>
> >>>>> int main(int argc, char *argv[])
> >>>>> {
> >>>
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Fri, Sep 6, 2024 at 10:50 PM Martin Uecker wrote:
>
> Am Freitag, dem 06.09.2024 um 13:59 -0700 schrieb Bill Wendling:
> > On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
> > >
> > > Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
> > > >
> > > > > On Sep 5, 2024, at 18:22, Bill Wendling wrote:
> > > > >
> > > > > Hi Qing,
> > > > >
> > > > > Sorry for my late reply.
> > > > >
> > > > > On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao
> > > > > wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Thanks for the information.
> > > > > >
> > > > > > Yes, providing a unary operator similar as __counted_by(PTR) as
> > > > > > suggested by multiple people previously is a cleaner approach.
> > > > > >
> > > > > > Then the programmer will use the following:
> > > > > >
> > > > > > __builtin_choose_expr(
> > > > > > __builtin_has_attribute (__p->FAM, "counted_by”)
> > > > > > __builtin_get_counted_by(__p->FAM) = COUNT, 0);
> > > > > >
> > > > > > From the programmer’s point of view, it’s cleaner too.
> > > > > >
> > > > > > However, there is one issue with “__builtin_choose_expr” currently
> > > > > > in GCC, its documentation explicitly mentions this limitation:
> > > > > > (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
> > > > > >
> > > > > > "Note: This construct is only available for C. Furthermore, the
> > > > > > unused expression (exp1 or exp2 depending on the value of
> > > > > > const_exp) may still generate syntax errors. This may change in
> > > > > > future revisions.”
> > > > > >
> > > > > > So, due to this limitation, when there is no counted_by attribute,
> > > > > > the __builtin_get_counted_by() still is evaluated by the compiler
> > > > > > and errors is issued and the compilation stops, this can be show
> > > > > > from the small testing case:
> > > > > >
> > > > > > [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
> > > > > >
> > > > > > struct flex {
> > > > > > unsigned int b;
> > > > > > int c[];
> > > > > > } *array_flex;
> > > > > >
> > > > > > #define MY_ALLOC(P, FAM, COUNT) ({ \
> > > > > > typeof(P) __p; \
> > > > > > unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> > > > > > __p = (typeof(P)) __builtin_malloc(__size); \
> > > > > > __builtin_choose_expr( \
> > > > > >__builtin_has_attribute (__p->FAM, counted_by), \
> > > > > >__builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
> > > > > > P = __p; \
> > > > > > })
> > > > > >
> > > > > > int main(int argc, char *argv[])
> > > > > > {
> > > > > > MY_ALLOC(array_flex, c, 20);
> > > > > > return 0;
> > > > > > }
> > > > > > [opc@qinzhao-ol8u3-x86 gcc]$ sh t
> > > > > > ttt.c: In function ‘main’:
> > > > > > ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
> > > > > > ‘__builtin_counted_by_ref’
> > > > > > ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
> > > > > >
> > > > > > I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it
> > > > > > does parse the __builtin_counted_by_ref(__p->FAM) even when
> > > > > > __builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued
> > > > > > the error when parsing __builtin_counted_by_ref and stopped the
> > > > > > compilation.
> > > > > >
> > > > > > So, in order to support this approach, we first must fix the issue
> > > > > > in the current __builtin_choose_expr in GCC. Otherwise, it’s
> > > > > > impossible for the user to use this new builtin.
> > > > > >
> > > > > > Let me know your c
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Sat, Sep 07, 2024 at 07:50:29AM +0200, Martin Uecker wrote: > > 2. Apple's implementation supports expressions in the '__counted_by' > > attribute, thus the 'count' may be an R-value that can't have its > > address taken. > > > > [1] > > https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836/ > > Yes, this would be a limitation. But then you really can't return an lvalue either, all you can return is an rvalue in that case. So just return a (void*)0 in that case. Jakub
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Samstag, dem 07.09.2024 um 08:16 +0200 schrieb Martin Uecker:
> Am Samstag, dem 07.09.2024 um 00:12 + schrieb Qing Zhao:
> > Now, if
> >
> > 1. __builtin_get_counted_by should return a LVALUE instead of a pointer
> > (required by CLANG’s design)
> > And
> > 2. It’s better not to change the behavior of __builtin_choose_expr.
> >
> > Then the solution left is:
> >
> > __builtin_get_counted_by (p->FAM) returns a LVALUE as p->COUNT if p->FAM
> > has a counted_by attribute, if p->FAM does not have a counted_by attribute,
> > silently do nothing. (Or just issue warning if Linux is OKEY with such
> > waning).
>
> What does silently do nothing mean?
>
> /* do nothing */ = counter;
>
> will still fail to compile. So I guess you have something
> else in mind?
>
>
> The new _Generic selection also works if you return a
> lvalue of type void:
>
> struct foo x;
> _Generic(typeof(__counted_by(&x)), void: (int){ 0 },
> default: __counted_by(&x)) = 10;
>
> https://godbolt.org/z/41E3oj84o
>
> So why not do this then?
>
> Martin
Although the pointer design is cleaner. And if the __counted_by
is not a lvalue as allowed by clang, assigning to it will fail
anyway, so returning a null pointer also seems better in this
scenario.
Martin
>
> >
> > Is this acceptable?
> >
> > thanks.
> >
> > Qing
> > > On Sep 6, 2024, at 16:59, Bill Wendling wrote:
> > >
> > > On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
> > > >
> > > > Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
> > > > >
> > > > > > On Sep 5, 2024, at 18:22, Bill Wendling wrote:
> > > > > >
> > > > > > Hi Qing,
> > > > > >
> > > > > > Sorry for my late reply.
> > > > > >
> > > > > > On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > Thanks for the information.
> > > > > > >
> > > > > > > Yes, providing a unary operator similar as __counted_by(PTR) as
> > > > > > > suggested by multiple people previously is a cleaner approach.
> > > > > > >
> > > > > > > Then the programmer will use the following:
> > > > > > >
> > > > > > > __builtin_choose_expr(
> > > > > > >__builtin_has_attribute (__p->FAM, "counted_by”)
> > > > > > >__builtin_get_counted_by(__p->FAM) = COUNT, 0);
> > > > > > >
> > > > > > > From the programmer’s point of view, it’s cleaner too.
> > > > > > >
> > > > > > > However, there is one issue with “__builtin_choose_expr”
> > > > > > > currently in GCC, its documentation explicitly mentions this
> > > > > > > limitation:
> > > > > > > (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
> > > > > > >
> > > > > > > "Note: This construct is only available for C. Furthermore, the
> > > > > > > unused expression (exp1 or exp2 depending on the value of
> > > > > > > const_exp) may still generate syntax errors. This may change in
> > > > > > > future revisions.”
> > > > > > >
> > > > > > > So, due to this limitation, when there is no counted_by
> > > > > > > attribute, the __builtin_get_counted_by() still is evaluated by
> > > > > > > the compiler and errors is issued and the compilation stops, this
> > > > > > > can be show from the small testing case:
> > > > > > >
> > > > > > > [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
> > > > > > >
> > > > > > > struct flex {
> > > > > > > unsigned int b;
> > > > > > > int c[];
> > > > > > > } *array_flex;
> > > > > > >
> > > > > > > #define MY_ALLOC(P, FAM, COUNT) ({ \
> > > > > > > typeof(P) __p; \
> > > > > > > unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> > > > > > > __p = (typeof(P)) __builtin_mall
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Samstag, dem 07.09.2024 um 00:12 + schrieb Qing Zhao:
> Now, if
>
> 1. __builtin_get_counted_by should return a LVALUE instead of a pointer
> (required by CLANG’s design)
> And
> 2. It’s better not to change the behavior of __builtin_choose_expr.
>
> Then the solution left is:
>
> __builtin_get_counted_by (p->FAM) returns a LVALUE as p->COUNT if p->FAM has
> a counted_by attribute, if p->FAM does not have a counted_by attribute,
> silently do nothing. (Or just issue warning if Linux is OKEY with such
> waning).
What does silently do nothing mean?
/* do nothing */ = counter;
will still fail to compile. So I guess you have something
else in mind?
The new _Generic selection also works if you return a
lvalue of type void:
struct foo x;
_Generic(typeof(__counted_by(&x)), void: (int){ 0 },
default: __counted_by(&x)) = 10;
https://godbolt.org/z/41E3oj84o
So why not do this then?
Martin
>
> Is this acceptable?
>
> thanks.
>
> Qing
> > On Sep 6, 2024, at 16:59, Bill Wendling wrote:
> >
> > On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
> > >
> > > Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
> > > >
> > > > > On Sep 5, 2024, at 18:22, Bill Wendling wrote:
> > > > >
> > > > > Hi Qing,
> > > > >
> > > > > Sorry for my late reply.
> > > > >
> > > > > On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao
> > > > > wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Thanks for the information.
> > > > > >
> > > > > > Yes, providing a unary operator similar as __counted_by(PTR) as
> > > > > > suggested by multiple people previously is a cleaner approach.
> > > > > >
> > > > > > Then the programmer will use the following:
> > > > > >
> > > > > > __builtin_choose_expr(
> > > > > >__builtin_has_attribute (__p->FAM, "counted_by”)
> > > > > >__builtin_get_counted_by(__p->FAM) = COUNT, 0);
> > > > > >
> > > > > > From the programmer’s point of view, it’s cleaner too.
> > > > > >
> > > > > > However, there is one issue with “__builtin_choose_expr” currently
> > > > > > in GCC, its documentation explicitly mentions this limitation:
> > > > > > (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
> > > > > >
> > > > > > "Note: This construct is only available for C. Furthermore, the
> > > > > > unused expression (exp1 or exp2 depending on the value of
> > > > > > const_exp) may still generate syntax errors. This may change in
> > > > > > future revisions.”
> > > > > >
> > > > > > So, due to this limitation, when there is no counted_by attribute,
> > > > > > the __builtin_get_counted_by() still is evaluated by the compiler
> > > > > > and errors is issued and the compilation stops, this can be show
> > > > > > from the small testing case:
> > > > > >
> > > > > > [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
> > > > > >
> > > > > > struct flex {
> > > > > > unsigned int b;
> > > > > > int c[];
> > > > > > } *array_flex;
> > > > > >
> > > > > > #define MY_ALLOC(P, FAM, COUNT) ({ \
> > > > > > typeof(P) __p; \
> > > > > > unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> > > > > > __p = (typeof(P)) __builtin_malloc(__size); \
> > > > > > __builtin_choose_expr( \
> > > > > > __builtin_has_attribute (__p->FAM, counted_by), \
> > > > > > __builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
> > > > > > P = __p; \
> > > > > > })
> > > > > >
> > > > > > int main(int argc, char *argv[])
> > > > > > {
> > > > > > MY_ALLOC(array_flex, c, 20);
> > > > > > return 0;
> > > > > > }
> > > > > > [opc@qinzhao-ol8u3-x86 gcc]$ sh t
> > > > > > ttt.c: In function ‘main’:
> > > > > > ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
> &
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Freitag, dem 06.09.2024 um 13:59 -0700 schrieb Bill Wendling:
> On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
> >
> > Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
> > >
> > > > On Sep 5, 2024, at 18:22, Bill Wendling wrote:
> > > >
> > > > Hi Qing,
> > > >
> > > > Sorry for my late reply.
> > > >
> > > > On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > Thanks for the information.
> > > > >
> > > > > Yes, providing a unary operator similar as __counted_by(PTR) as
> > > > > suggested by multiple people previously is a cleaner approach.
> > > > >
> > > > > Then the programmer will use the following:
> > > > >
> > > > > __builtin_choose_expr(
> > > > > __builtin_has_attribute (__p->FAM, "counted_by”)
> > > > > __builtin_get_counted_by(__p->FAM) = COUNT, 0);
> > > > >
> > > > > From the programmer’s point of view, it’s cleaner too.
> > > > >
> > > > > However, there is one issue with “__builtin_choose_expr” currently in
> > > > > GCC, its documentation explicitly mentions this limitation:
> > > > > (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
> > > > >
> > > > > "Note: This construct is only available for C. Furthermore, the
> > > > > unused expression (exp1 or exp2 depending on the value of const_exp)
> > > > > may still generate syntax errors. This may change in future
> > > > > revisions.”
> > > > >
> > > > > So, due to this limitation, when there is no counted_by attribute,
> > > > > the __builtin_get_counted_by() still is evaluated by the compiler and
> > > > > errors is issued and the compilation stops, this can be show from the
> > > > > small testing case:
> > > > >
> > > > > [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
> > > > >
> > > > > struct flex {
> > > > > unsigned int b;
> > > > > int c[];
> > > > > } *array_flex;
> > > > >
> > > > > #define MY_ALLOC(P, FAM, COUNT) ({ \
> > > > > typeof(P) __p; \
> > > > > unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> > > > > __p = (typeof(P)) __builtin_malloc(__size); \
> > > > > __builtin_choose_expr( \
> > > > >__builtin_has_attribute (__p->FAM, counted_by), \
> > > > >__builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
> > > > > P = __p; \
> > > > > })
> > > > >
> > > > > int main(int argc, char *argv[])
> > > > > {
> > > > > MY_ALLOC(array_flex, c, 20);
> > > > > return 0;
> > > > > }
> > > > > [opc@qinzhao-ol8u3-x86 gcc]$ sh t
> > > > > ttt.c: In function ‘main’:
> > > > > ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
> > > > > ‘__builtin_counted_by_ref’
> > > > > ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
> > > > >
> > > > > I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it
> > > > > does parse the __builtin_counted_by_ref(__p->FAM) even when
> > > > > __builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued
> > > > > the error when parsing __builtin_counted_by_ref and stopped the
> > > > > compilation.
> > > > >
> > > > > So, in order to support this approach, we first must fix the issue in
> > > > > the current __builtin_choose_expr in GCC. Otherwise, it’s impossible
> > > > > for the user to use this new builtin.
> > > > >
> > > > > Let me know your comments and suggestions.
> > > > >
> > > > Do you need to emit a diagnostic if the FAM doesn't have the
> > > > counted_by attribute? It was originally supposed to "silently fail" if
> > > > it didn't. We may need to do the same for Clang if so.
> > >
> > > Yes, “silently fail” should workaround this problem if fixing the issue
> > > in the current __builtin_choose_expr is too complicate.
> &g
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Now, if
1. __builtin_get_counted_by should return a LVALUE instead of a pointer
(required by CLANG’s design)
And
2. It’s better not to change the behavior of __builtin_choose_expr.
Then the solution left is:
__builtin_get_counted_by (p->FAM) returns a LVALUE as p->COUNT if p->FAM has a
counted_by attribute, if p->FAM does not have a counted_by attribute, silently
do nothing. (Or just issue warning if Linux is OKEY with such waning).
Is this acceptable?
thanks.
Qing
> On Sep 6, 2024, at 16:59, Bill Wendling wrote:
>
> On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
>>
>> Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
>>>
>>>> On Sep 5, 2024, at 18:22, Bill Wendling wrote:
>>>>
>>>> Hi Qing,
>>>>
>>>> Sorry for my late reply.
>>>>
>>>> On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Thanks for the information.
>>>>>
>>>>> Yes, providing a unary operator similar as __counted_by(PTR) as suggested
>>>>> by multiple people previously is a cleaner approach.
>>>>>
>>>>> Then the programmer will use the following:
>>>>>
>>>>> __builtin_choose_expr(
>>>>>__builtin_has_attribute (__p->FAM, "counted_by”)
>>>>>__builtin_get_counted_by(__p->FAM) = COUNT, 0);
>>>>>
>>>>> From the programmer’s point of view, it’s cleaner too.
>>>>>
>>>>> However, there is one issue with “__builtin_choose_expr” currently in
>>>>> GCC, its documentation explicitly mentions this limitation:
>>>>> (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
>>>>>
>>>>> "Note: This construct is only available for C. Furthermore, the unused
>>>>> expression (exp1 or exp2 depending on the value of const_exp) may still
>>>>> generate syntax errors. This may change in future revisions.”
>>>>>
>>>>> So, due to this limitation, when there is no counted_by attribute, the
>>>>> __builtin_get_counted_by() still is evaluated by the compiler and errors
>>>>> is issued and the compilation stops, this can be show from the small
>>>>> testing case:
>>>>>
>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
>>>>>
>>>>> struct flex {
>>>>> unsigned int b;
>>>>> int c[];
>>>>> } *array_flex;
>>>>>
>>>>> #define MY_ALLOC(P, FAM, COUNT) ({ \
>>>>> typeof(P) __p; \
>>>>> unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
>>>>> __p = (typeof(P)) __builtin_malloc(__size); \
>>>>> __builtin_choose_expr( \
>>>>> __builtin_has_attribute (__p->FAM, counted_by), \
>>>>> __builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
>>>>> P = __p; \
>>>>> })
>>>>>
>>>>> int main(int argc, char *argv[])
>>>>> {
>>>>> MY_ALLOC(array_flex, c, 20);
>>>>> return 0;
>>>>> }
>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ sh t
>>>>> ttt.c: In function ‘main’:
>>>>> ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
>>>>> ‘__builtin_counted_by_ref’
>>>>> ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
>>>>>
>>>>> I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it does
>>>>> parse the __builtin_counted_by_ref(__p->FAM) even when
>>>>> __builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued the
>>>>> error when parsing __builtin_counted_by_ref and stopped the compilation.
>>>>>
>>>>> So, in order to support this approach, we first must fix the issue in the
>>>>> current __builtin_choose_expr in GCC. Otherwise, it’s impossible for the
>>>>> user to use this new builtin.
>>>>>
>>>>> Let me know your comments and suggestions.
>>>>>
>>>> Do you need to emit a diagnostic if the FAM doesn't have the
>>>> counted_by attribute? It was originally supposed to "silently fail" if
>>>> it didn't. We may need to do the same for Clang if so.
>>>
>>> Yes, “silently fail” should workaround thi
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Fri, Sep 6, 2024 at 12:32 PM Martin Uecker wrote:
>
> Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
> >
> > > On Sep 5, 2024, at 18:22, Bill Wendling wrote:
> > >
> > > Hi Qing,
> > >
> > > Sorry for my late reply.
> > >
> > > On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao wrote:
> > > >
> > > > Hi,
> > > >
> > > > Thanks for the information.
> > > >
> > > > Yes, providing a unary operator similar as __counted_by(PTR) as
> > > > suggested by multiple people previously is a cleaner approach.
> > > >
> > > > Then the programmer will use the following:
> > > >
> > > > __builtin_choose_expr(
> > > > __builtin_has_attribute (__p->FAM, "counted_by”)
> > > > __builtin_get_counted_by(__p->FAM) = COUNT, 0);
> > > >
> > > > From the programmer’s point of view, it’s cleaner too.
> > > >
> > > > However, there is one issue with “__builtin_choose_expr” currently in
> > > > GCC, its documentation explicitly mentions this limitation:
> > > > (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
> > > >
> > > > "Note: This construct is only available for C. Furthermore, the unused
> > > > expression (exp1 or exp2 depending on the value of const_exp) may still
> > > > generate syntax errors. This may change in future revisions.”
> > > >
> > > > So, due to this limitation, when there is no counted_by attribute, the
> > > > __builtin_get_counted_by() still is evaluated by the compiler and
> > > > errors is issued and the compilation stops, this can be show from the
> > > > small testing case:
> > > >
> > > > [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
> > > >
> > > > struct flex {
> > > > unsigned int b;
> > > > int c[];
> > > > } *array_flex;
> > > >
> > > > #define MY_ALLOC(P, FAM, COUNT) ({ \
> > > > typeof(P) __p; \
> > > > unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> > > > __p = (typeof(P)) __builtin_malloc(__size); \
> > > > __builtin_choose_expr( \
> > > >__builtin_has_attribute (__p->FAM, counted_by), \
> > > >__builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
> > > > P = __p; \
> > > > })
> > > >
> > > > int main(int argc, char *argv[])
> > > > {
> > > > MY_ALLOC(array_flex, c, 20);
> > > > return 0;
> > > > }
> > > > [opc@qinzhao-ol8u3-x86 gcc]$ sh t
> > > > ttt.c: In function ‘main’:
> > > > ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
> > > > ‘__builtin_counted_by_ref’
> > > > ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
> > > >
> > > > I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it
> > > > does parse the __builtin_counted_by_ref(__p->FAM) even when
> > > > __builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued the
> > > > error when parsing __builtin_counted_by_ref and stopped the compilation.
> > > >
> > > > So, in order to support this approach, we first must fix the issue in
> > > > the current __builtin_choose_expr in GCC. Otherwise, it’s impossible
> > > > for the user to use this new builtin.
> > > >
> > > > Let me know your comments and suggestions.
> > > >
> > > Do you need to emit a diagnostic if the FAM doesn't have the
> > > counted_by attribute? It was originally supposed to "silently fail" if
> > > it didn't. We may need to do the same for Clang if so.
> >
> > Yes, “silently fail” should workaround this problem if fixing the issue in
> > the current __builtin_choose_expr is too complicate.
> >
> > I will study a little bit on how to fix the issue in __builtin_choose_expr
> > first.
> >
> > Martin and Joseph, any comment or suggestion from you?
>
> My recommendation would be not to change __builtin_choose_expr.
>
> The design where __builtin_get_counted_by returns a null
> pointer constant (void*)0 seems good. Most users will
> get an error which I think is what we want and for those
> that want it to work even if the attribute is not there, the
> following code seems perfectly acceptable to me
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Freitag, dem 06.09.2024 um 13:59 + schrieb Qing Zhao:
>
> > On Sep 5, 2024, at 18:22, Bill Wendling wrote:
> >
> > Hi Qing,
> >
> > Sorry for my late reply.
> >
> > On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao wrote:
> > >
> > > Hi,
> > >
> > > Thanks for the information.
> > >
> > > Yes, providing a unary operator similar as __counted_by(PTR) as suggested
> > > by multiple people previously is a cleaner approach.
> > >
> > > Then the programmer will use the following:
> > >
> > > __builtin_choose_expr(
> > > __builtin_has_attribute (__p->FAM, "counted_by”)
> > > __builtin_get_counted_by(__p->FAM) = COUNT, 0);
> > >
> > > From the programmer’s point of view, it’s cleaner too.
> > >
> > > However, there is one issue with “__builtin_choose_expr” currently in
> > > GCC, its documentation explicitly mentions this limitation:
> > > (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
> > >
> > > "Note: This construct is only available for C. Furthermore, the unused
> > > expression (exp1 or exp2 depending on the value of const_exp) may still
> > > generate syntax errors. This may change in future revisions.”
> > >
> > > So, due to this limitation, when there is no counted_by attribute, the
> > > __builtin_get_counted_by() still is evaluated by the compiler and errors
> > > is issued and the compilation stops, this can be show from the small
> > > testing case:
> > >
> > > [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
> > >
> > > struct flex {
> > > unsigned int b;
> > > int c[];
> > > } *array_flex;
> > >
> > > #define MY_ALLOC(P, FAM, COUNT) ({ \
> > > typeof(P) __p; \
> > > unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> > > __p = (typeof(P)) __builtin_malloc(__size); \
> > > __builtin_choose_expr( \
> > >__builtin_has_attribute (__p->FAM, counted_by), \
> > >__builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
> > > P = __p; \
> > > })
> > >
> > > int main(int argc, char *argv[])
> > > {
> > > MY_ALLOC(array_flex, c, 20);
> > > return 0;
> > > }
> > > [opc@qinzhao-ol8u3-x86 gcc]$ sh t
> > > ttt.c: In function ‘main’:
> > > ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
> > > ‘__builtin_counted_by_ref’
> > > ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
> > >
> > > I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it does
> > > parse the __builtin_counted_by_ref(__p->FAM) even when
> > > __builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued the
> > > error when parsing __builtin_counted_by_ref and stopped the compilation.
> > >
> > > So, in order to support this approach, we first must fix the issue in the
> > > current __builtin_choose_expr in GCC. Otherwise, it’s impossible for the
> > > user to use this new builtin.
> > >
> > > Let me know your comments and suggestions.
> > >
> > Do you need to emit a diagnostic if the FAM doesn't have the
> > counted_by attribute? It was originally supposed to "silently fail" if
> > it didn't. We may need to do the same for Clang if so.
>
> Yes, “silently fail” should workaround this problem if fixing the issue in
> the current __builtin_choose_expr is too complicate.
>
> I will study a little bit on how to fix the issue in __builtin_choose_expr
> first.
>
> Martin and Joseph, any comment or suggestion from you?
My recommendation would be not to change __builtin_choose_expr.
The design where __builtin_get_counted_by returns a null
pointer constant (void*)0 seems good. Most users will
get an error which I think is what we want and for those
that want it to work even if the attribute is not there, the
following code seems perfectly acceptable to me:
auto p = __builtin_get_counted_by(__p->FAM)
*_Generic(p, void*: &(int){}, default: p) = 1;
Kees also seemed happy with it. And if I understood it correctly,
also Clang's bounds checking people can work with this.
Martin
Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option
On Fri, Sep 6, 2024, 9:38 AM Dora, Sunil Kumar < sunilkumar.d...@windriver.com> wrote: > Hi Andrew, > > Thank you for your feedback. Initially, we attempted to address the issue > by utilizing GCC’s response files. However, we discovered that the > COLLECT_GCC_OPTIONS variable already contains the expanded contents of > the response files. > > As a result, using response files only mitigates the multiplication factor > but does not bypass the 128KB limit. > I think you missed understood me fully. What I was saying instead of creating a string inside set_collect_gcc_options, create the response file and pass that via COLLECT_GCC_OPTIONS with the @file format. And then inside collect2.cc when using COLLECT_GCC_OPTIONS/extract_string instead read in the response file options if there was an @file instead of those 2 loops. This requires more than what you did. Oh and should be less memory hungry and maybe slightly faster. Thanks, Andrew I have included the response file usage logs and the complete history in > the Bugzilla report for your reference: Bugzilla Link > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527#c19>. > Following your suggestion, I have updated the logic to avoid hardcoding > /tmp. > Please find the revised version of patch at the following link: > > https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662519.html > > Thanks, > Sunil Dora > -- > *From:* Andrew Pinski > *Sent:* Friday, August 30, 2024 8:05 PM > *To:* Hemraj, Deepthi > *Cc:* gcc-patches@gcc.gnu.org ; rguent...@suse.de > ; jeffreya...@gmail.com ; > josmy...@redhat.com ; MacLeod, Randy < > randy.macl...@windriver.com>; Gowda, Naveen ; > Dora, Sunil Kumar > *Subject:* Re: [PATCH v2] GCC Driver : Enable very long gcc command-line > option > > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and > know the content is safe. > > On Fri, Aug 30, 2024 at 12:34 AM wrote: > > > > From: Deepthi Hemraj > > > > For excessively long environment variables i.e >128KB > > Store the arguments in a temporary file and collect them back together > in collect2. > > > > This commit patches for COLLECT_GCC_OPTIONS issue: > > GCC should not limit the length of command line passed to collect2. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527 > > > > The Linux kernel has the following limits on shell commands: > > I. Total number of bytes used to specify arguments must be under 128KB. > > II. Each environment variable passed to an executable must be under 128 > KiB > > > > In order to circumvent these limitations, many build tools support > > response-files, i.e. files that contain the arguments for the executed > > command. These are typically passed using @ syntax. > > > > Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the > > expanded command line to collect2. With many options, this exceeds the > limit II. > > > > GCC : Added Testcase for PR111527 > > > > TC1 : If the command line argument less than 128kb, gcc should use > > COLLECT_GCC_OPTION to communicate and compile fine. > > TC2 : If the command line argument in the range of 128kb to 2mb, > > gcc should copy arguments in a file and use FILE_GCC_OPTIONS > > to communicate and compile fine. > > TC3 : If the command line argument greater thean 2mb, gcc shuld > > fail the compile and report error. (Expected FAIL) > > > > Signed-off-by: sunil dora > > Signed-off-by: Topi Kuutela > > Signed-off-by: Deepthi Hemraj > > --- > > gcc/collect2.cc | 39 ++-- > > gcc/gcc.cc| 37 +-- > > gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 + > > gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++ > > gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 + > > gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++ > > gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++ > > 7 files changed, 159 insertions(+), 6 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp > > create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c > > create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c > > create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c > > create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c > > > > diff --git a/gcc/collect2.cc b/gcc/collect2.cc > > index 902014a9cc1..1f56963b1ce 100644 > > --- a/gcc/collect2.cc &g
Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option
Hi Andrew,
Thank you for your feedback. Initially, we attempted to address the issue by
utilizing GCC’s response files. However, we discovered that the
COLLECT_GCC_OPTIONS variable already contains the expanded contents of the
response files.
As a result, using response files only mitigates the multiplication factor but
does not bypass the 128KB limit.
I have included the response file usage logs and the complete history in the
Bugzilla report for your reference: Bugzilla
Link<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527#c19>.
Following your suggestion, I have updated the logic to avoid hardcoding /tmp.
Please find the revised version of patch at the following link:
https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662519.html
Thanks,
Sunil Dora
From: Andrew Pinski
Sent: Friday, August 30, 2024 8:05 PM
To: Hemraj, Deepthi
Cc: gcc-patches@gcc.gnu.org ; rguent...@suse.de
; jeffreya...@gmail.com ;
josmy...@redhat.com ; MacLeod, Randy
; Gowda, Naveen ;
Dora, Sunil Kumar
Subject: Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option
CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know
the content is safe.
On Fri, Aug 30, 2024 at 12:34 AM wrote:
>
> From: Deepthi Hemraj
>
> For excessively long environment variables i.e >128KB
> Store the arguments in a temporary file and collect them back together in
> collect2.
>
> This commit patches for COLLECT_GCC_OPTIONS issue:
> GCC should not limit the length of command line passed to collect2.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527
>
> The Linux kernel has the following limits on shell commands:
> I. Total number of bytes used to specify arguments must be under 128KB.
> II. Each environment variable passed to an executable must be under 128 KiB
>
> In order to circumvent these limitations, many build tools support
> response-files, i.e. files that contain the arguments for the executed
> command. These are typically passed using @ syntax.
>
> Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the
> expanded command line to collect2. With many options, this exceeds the limit
> II.
>
> GCC : Added Testcase for PR111527
>
> TC1 : If the command line argument less than 128kb, gcc should use
> COLLECT_GCC_OPTION to communicate and compile fine.
> TC2 : If the command line argument in the range of 128kb to 2mb,
> gcc should copy arguments in a file and use FILE_GCC_OPTIONS
> to communicate and compile fine.
> TC3 : If the command line argument greater thean 2mb, gcc shuld
> fail the compile and report error. (Expected FAIL)
>
> Signed-off-by: sunil dora
> Signed-off-by: Topi Kuutela
> Signed-off-by: Deepthi Hemraj
> ---
> gcc/collect2.cc | 39 ++--
> gcc/gcc.cc| 37 +--
> gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 +
> gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++
> gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 +
> gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++++++
> gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++
> 7 files changed, 159 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
>
> diff --git a/gcc/collect2.cc b/gcc/collect2.cc
> index 902014a9cc1..1f56963b1ce 100644
> --- a/gcc/collect2.cc
> +++ b/gcc/collect2.cc
> @@ -376,6 +376,39 @@ typedef int scanfilter;
>
> static void scan_prog_file (const char *, scanpass, scanfilter);
>
> +char* getenv_extended (const char* var_name)
> +{
> + int file_size;
> + char* buf = NULL;
> + const char* prefix = "/tmp";
> +
> + char* string = getenv (var_name);
> + if (strncmp (var_name, prefix, strlen(prefix)) == 0)
This is not what was meant by saying using the same env and supporting
response files.
Instead what Richard meant was use `@file` as the option that gets
passed via COLLECT_GCC_OPTIONS and then if you see `@` expand the
options like what is done for the normal command line.
Hard coding "/tmp" here is wrong because TMPDIR might not be set to
"/tmp" and even more with -save-temps, the response file should stay
around afterwards and be in the working directory rather than TMPDIR.
Thanks,
Andrew Pinski
> +{
> + FILE *fptr;
> + fptr = fopen (string, "r");
> + if (fptr =
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Sep 5, 2024, at 18:22, Bill Wendling wrote:
>
> Hi Qing,
>
> Sorry for my late reply.
>
> On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao wrote:
>>
>> Hi,
>>
>> Thanks for the information.
>>
>> Yes, providing a unary operator similar as __counted_by(PTR) as suggested by
>> multiple people previously is a cleaner approach.
>>
>> Then the programmer will use the following:
>>
>> __builtin_choose_expr(
>> __builtin_has_attribute (__p->FAM, "counted_by”)
>> __builtin_get_counted_by(__p->FAM) = COUNT, 0);
>>
>> From the programmer’s point of view, it’s cleaner too.
>>
>> However, there is one issue with “__builtin_choose_expr” currently in GCC,
>> its documentation explicitly mentions this limitation:
>> (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
>>
>> "Note: This construct is only available for C. Furthermore, the unused
>> expression (exp1 or exp2 depending on the value of const_exp) may still
>> generate syntax errors. This may change in future revisions.”
>>
>> So, due to this limitation, when there is no counted_by attribute, the
>> __builtin_get_counted_by() still is evaluated by the compiler and errors is
>> issued and the compilation stops, this can be show from the small testing
>> case:
>>
>> [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
>>
>> struct flex {
>> unsigned int b;
>> int c[];
>> } *array_flex;
>>
>> #define MY_ALLOC(P, FAM, COUNT) ({ \
>> typeof(P) __p; \
>> unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
>> __p = (typeof(P)) __builtin_malloc(__size); \
>> __builtin_choose_expr( \
>>__builtin_has_attribute (__p->FAM, counted_by), \
>>__builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
>> P = __p; \
>> })
>>
>> int main(int argc, char *argv[])
>> {
>> MY_ALLOC(array_flex, c, 20);
>> return 0;
>> }
>> [opc@qinzhao-ol8u3-x86 gcc]$ sh t
>> ttt.c: In function ‘main’:
>> ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
>> ‘__builtin_counted_by_ref’
>> ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
>>
>> I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it does
>> parse the __builtin_counted_by_ref(__p->FAM) even when
>> __builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued the error
>> when parsing __builtin_counted_by_ref and stopped the compilation.
>>
>> So, in order to support this approach, we first must fix the issue in the
>> current __builtin_choose_expr in GCC. Otherwise, it’s impossible for the
>> user to use this new builtin.
>>
>> Let me know your comments and suggestions.
>>
> Do you need to emit a diagnostic if the FAM doesn't have the
> counted_by attribute? It was originally supposed to "silently fail" if
> it didn't. We may need to do the same for Clang if so.
Yes, “silently fail” should workaround this problem if fixing the issue in the
current __builtin_choose_expr is too complicate.
I will study a little bit on how to fix the issue in __builtin_choose_expr
first.
Martin and Joseph, any comment or suggestion from you?
thanks.
Qing
>
> -bw
[PATCH v3] GCC Driver : Enable very long gcc command-line option
From: Deepthi Hemraj
For excessively long environment variables i.e >128KB
Store the arguments in a temporary file and collect them back together in
collect2.
This commit patches for COLLECT_GCC_OPTIONS issue:
GCC should not limit the length of command line passed to collect2.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527
The Linux kernel has the following limits on shell commands:
I. Total number of bytes used to specify arguments must be under 128KB.
II. Each environment variable passed to an executable must be under 128 KiB
In order to circumvent these limitations, many build tools support
response-files, i.e. files that contain the arguments for the executed
command. These are typically passed using @ syntax.
Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the
expanded command line to collect2. With many options, this exceeds the limit II.
GCC : Added Testcase for PR111527
TC1 : If the command line argument less than 128kb, gcc should use
COLLECT_GCC_OPTION to communicate and compile fine.
TC2 : If the command line argument in the range of 128kb to 2mb,
gcc should copy arguments in a file and use FILE_GCC_OPTIONS
to communicate and compile fine.
TC3 : If the command line argument greater thean 2mb, gcc shuld
fail the compile and report error. (Expected FAIL)
Signed-off-by: sunil dora
Signed-off-by: Topi Kuutela
---
gcc/collect2.cc | 42 --
gcc/gcc.cc| 37 +--
gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 +
gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++
gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 +
gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++
gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++
7 files changed, 162 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
diff --git a/gcc/collect2.cc b/gcc/collect2.cc
index 902014a9cc1..5b5f16ab46c 100644
--- a/gcc/collect2.cc
+++ b/gcc/collect2.cc
@@ -376,6 +376,42 @@ typedef int scanfilter;
static void scan_prog_file (const char *, scanpass, scanfilter);
+char* getenv_extended (const char* var_name)
+{
+ int file_size;
+ char* buf = NULL;
+ const char* prefix = "@";
+ size_t prefix_len = strlen(prefix);
+
+ char* string = getenv (var_name);
+ if (strncmp (string, prefix, prefix_len) == 0)
+{
+ FILE *fptr;
+ char *new_string = xstrdup(string + prefix_len);
+ fptr = fopen (new_string, "r");
+ if (fptr == NULL)
+ return (0);
+ /* Copy contents from temporary file to buffer */
+ if (fseek (fptr, 0, SEEK_END) == -1)
+ return (0);
+ file_size = ftell (fptr);
+ rewind (fptr);
+ buf = (char *) xmalloc (file_size + 1);
+ if (buf == NULL)
+ return (0);
+ if (fread ((void *) buf, file_size, 1, fptr) <= 0)
+ {
+ free (buf);
+ fatal_error (input_location, "fread failed");
+ return (0);
+ }
+ buf[file_size] = '\0';
+ free(new_string);
+ return buf;
+}
+ return string;
+}
+
/* Delete tempfiles and exit function. */
@@ -1004,7 +1040,7 @@ main (int argc, char **argv)
/* Now pick up any flags we want early from COLLECT_GCC_OPTIONS
The LTO options are passed here as are other options that might
be unsuitable for ld (e.g. -save-temps). */
-p = getenv ("COLLECT_GCC_OPTIONS");
+p = getenv_extended ("COLLECT_GCC_OPTIONS");
while (p && *p)
{
const char *q = extract_string (&p);
@@ -1200,7 +1236,7 @@ main (int argc, char **argv)
AIX support needs to know if -shared has been specified before
parsing commandline arguments. */
- p = getenv ("COLLECT_GCC_OPTIONS");
+ p = getenv_extended ("COLLECT_GCC_OPTIONS");
while (p && *p)
{
const char *q = extract_string (&p);
@@ -1594,7 +1630,7 @@ main (int argc, char **argv)
fprintf (stderr, "o_file = %s\n",
(o_file ? o_file : "not found"));
- ptr = getenv ("COLLECT_GCC_OPTIONS");
+ ptr = getenv_extended ("COLLECT_GCC_OPTIONS");
if (ptr)
fprintf (stderr, "COLLECT_GCC_OPTIONS = %s\n", ptr);
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index ae1d80fe00a..98c1dff6335 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -2953,12 +2953,43 @@ add_to_obstack (char *path, void *data)
return NULL;
}
-/* Add or change the value of an environment variable, outputting the
- change to standard error if in verbose mode. */
+/* A
New Ukrainian PO file for 'gcc' (version 14.2.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Ukrainian team of translators. The file is available at: https://translationproject.org/latest/gcc/uk.po (This file, 'gcc-14.2.0.uk.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [PATCH] ada: Fix gcc-interface/misc.cc compilation on SPARC
> commit 72c6938f29cbeddb3220720e68add4cf09ffd794 > Author: Eric Botcazou > Date: Sun Aug 25 15:20:59 2024 +0200 > > ada: Streamline handling of low-level peculiarities of record field > layout > > broke the Ada build on SPARC: > > In file included from ./tm_p.h:4, > from > /vol/gcc/src/hg/master/local/gcc/ada/gcc-interface/misc.cc:31: > /vol/gcc/src/hg/master/local/gcc/config/sparc/sparc-protos.h:46:47: error: > use of enum ‘memmodel’ without previous declaration 46 | extern void > sparc_emit_membar_for_model (enum memmodel, int, int); > | ^~~~ > > Fixed by including memmodel.h. Sorry about that, a small merge glitch, the fix is already in the pipeline. -- Eric Botcazou
[PATCH] ada: Fix gcc-interface/misc.cc compilation on SPARC
This patch commit 72c6938f29cbeddb3220720e68add4cf09ffd794 Author: Eric Botcazou Date: Sun Aug 25 15:20:59 2024 +0200 ada: Streamline handling of low-level peculiarities of record field layout broke the Ada build on SPARC: In file included from ./tm_p.h:4, from /vol/gcc/src/hg/master/local/gcc/ada/gcc-interface/misc.cc:31: /vol/gcc/src/hg/master/local/gcc/config/sparc/sparc-protos.h:46:47: error: use of enum ‘memmodel’ without previous declaration 46 | extern void sparc_emit_membar_for_model (enum memmodel, int, int); | ^~~~ Fixed by including memmodel.h. Bootstrapped without regressions on sparc-sun-solaris2.11 and i386-pc-solaris2.11. Ok for trunk? I guess this is obvious, though. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2024-09-05 Rainer Orth gcc/ada: * gcc-interface/misc.cc: Include memmodel.h. diff --git a/gcc/ada/gcc-interface/misc.cc b/gcc/ada/gcc-interface/misc.cc --- a/gcc/ada/gcc-interface/misc.cc +++ b/gcc/ada/gcc-interface/misc.cc @@ -28,6 +28,7 @@ #include "coretypes.h" #include "target.h" #include "tree.h" +#include "memmodel.h" #include "tm_p.h" #include "diagnostic.h" #include "opts.h"
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Hi Qing,
Sorry for my late reply.
On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao wrote:
>
> Hi,
>
> Thanks for the information.
>
> Yes, providing a unary operator similar as __counted_by(PTR) as suggested by
> multiple people previously is a cleaner approach.
>
> Then the programmer will use the following:
>
> __builtin_choose_expr(
> __builtin_has_attribute (__p->FAM, "counted_by”)
> __builtin_get_counted_by(__p->FAM) = COUNT, 0);
>
> From the programmer’s point of view, it’s cleaner too.
>
> However, there is one issue with “__builtin_choose_expr” currently in GCC,
> its documentation explicitly mentions this limitation:
> (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
>
> "Note: This construct is only available for C. Furthermore, the unused
> expression (exp1 or exp2 depending on the value of const_exp) may still
> generate syntax errors. This may change in future revisions.”
>
> So, due to this limitation, when there is no counted_by attribute, the
> __builtin_get_counted_by() still is evaluated by the compiler and errors is
> issued and the compilation stops, this can be show from the small testing
> case:
>
> [opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
>
> struct flex {
> unsigned int b;
> int c[];
> } *array_flex;
>
> #define MY_ALLOC(P, FAM, COUNT) ({ \
> typeof(P) __p; \
> unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> __p = (typeof(P)) __builtin_malloc(__size); \
> __builtin_choose_expr( \
> __builtin_has_attribute (__p->FAM, counted_by), \
> __builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
> P = __p; \
> })
>
> int main(int argc, char *argv[])
> {
> MY_ALLOC(array_flex, c, 20);
> return 0;
> }
> [opc@qinzhao-ol8u3-x86 gcc]$ sh t
> ttt.c: In function ‘main’:
> ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
> ‘__builtin_counted_by_ref’
> ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
>
> I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it does
> parse the __builtin_counted_by_ref(__p->FAM) even when
> __builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued the error
> when parsing __builtin_counted_by_ref and stopped the compilation.
>
> So, in order to support this approach, we first must fix the issue in the
> current __builtin_choose_expr in GCC. Otherwise, it’s impossible for the user
> to use this new builtin.
>
> Let me know your comments and suggestions.
>
Do you need to emit a diagnostic if the FAM doesn't have the
counted_by attribute? It was originally supposed to "silently fail" if
it didn't. We may need to do the same for Clang if so.
-bw
[committed, gcc-14] libstdc++: Fix std::variant to reject array types [PR116381]
Tested x86_64-linux. Pushed to gcc-14.
-- >8 --
For the backport, rejecting array types is only done in strict modes.
libstdc++-v3/ChangeLog:
PR libstdc++/116381
* include/std/variant (variant): Fix conditions for
static_assert to match the spec.
* testsuite/20_util/variant/types_neg.cc: New test.
(cherry picked from commit 1e10b3b8825ee398f077500af6ae1f5db180983a)
---
libstdc++-v3/include/std/variant | 11 +++
.../testsuite/20_util/variant/types_neg.cc | 18 ++
2 files changed, 25 insertions(+), 4 deletions(-)
create mode 100644 libstdc++-v3/testsuite/20_util/variant/types_neg.cc
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 4e56134a6f7..834bb548f13 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1374,10 +1374,13 @@ namespace __variant
static_assert(sizeof...(_Types) > 0,
"variant must have at least one alternative");
- static_assert(!(std::is_reference_v<_Types> || ...),
- "variant must have no reference alternative");
- static_assert(!(std::is_void_v<_Types> || ...),
- "variant must have no void alternative");
+#ifdef __STRICT_ANSI__
+ static_assert(((std::is_object_v<_Types> && !is_array_v<_Types>) && ...),
+ "variant alternatives must be non-array object types");
+#else
+ static_assert((std::is_object_v<_Types> && ...),
+ "variant alternatives must be object types");
+#endif
using _Base = __detail::__variant::_Variant_base<_Types...>;
diff --git a/libstdc++-v3/testsuite/20_util/variant/types_neg.cc
b/libstdc++-v3/testsuite/20_util/variant/types_neg.cc
new file mode 100644
index 000..7d970e961c2
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/types_neg.cc
@@ -0,0 +1,18 @@
+// { dg-do compile { target c++17 } }
+// { dg-add-options strict_std }
+
+# include
+
+std::variant<> v0; // { dg-error "here" }
+// { dg-error "must have at least one alternative" "" { target *-*-* } 0 }
+std::variant v1; // { dg-error "here" }
+std::variant v2; // { dg-error "here" }
+std::variant v3; // { dg-error "here" }
+std::variant v4; // { dg-error "here" }
+std::variant v5; // { dg-error "here" }
+std::variant v6; // { dg-error "here" }
+// { dg-error "must be non-array object types" "" { target *-*-* } 0 }
+
+// All of variant's base classes are instantiated before checking any
+// static_assert, so we get lots of errors before the expected errors above.
+// { dg-excess-errors "" }
--
2.46.0
[committed, wwwdocs] gcc-15: Fiji gfx803 device support removed
--- htdocs/gcc-15/changes.html | 7 +++ 1 file changed, 7 insertions(+) diff --git a/htdocs/gcc-15/changes.html b/htdocs/gcc-15/changes.html index edce138e..7c372688 100644 --- a/htdocs/gcc-15/changes.html +++ b/htdocs/gcc-15/changes.html @@ -123,6 +123,13 @@ a work-in-progress. +AMD Radeon (GCN) + + + Support for Fiji (gfx803) devices has been removed (this was already +deprecated in GCC 14). + + -- 2.45.2
[PATCH v12 2/4] gcc/: Rename array_type_nelts() => array_type_nelts_minus_one()
The old name was misleading.
While at it, also rename some temporary variables that are used with
this function, for consistency.
Link:
<https://inbox.sourceware.org/gcc-patches/9fffd80-dca-2c7e-14b-6c9b509a7...@redhat.com/T/#m2f661c67c8f7b2c405c8c7fc3152dd85dc729120>
gcc/ChangeLog:
* tree.cc (array_type_nelts, array_type_nelts_minus_one)
* tree.h (array_type_nelts, array_type_nelts_minus_one)
* expr.cc (count_type_elements)
* config/aarch64/aarch64.cc
(pure_scalable_type_info::analyze_array)
* config/i386/i386.cc (ix86_canonical_va_list_type):
Rename array_type_nelts() => array_type_nelts_minus_one()
The old name was misleading.
gcc/c/ChangeLog:
* c-decl.cc (one_element_array_type_p, get_parm_array_spec)
* c-fold.cc (c_fold_array_ref):
Rename array_type_nelts() => array_type_nelts_minus_one()
gcc/cp/ChangeLog:
* decl.cc (reshape_init_array)
* init.cc
(build_zero_init_1)
(build_value_init_noctor)
(build_vec_init)
(build_delete)
* lambda.cc (add_capture)
* tree.cc (array_type_nelts_top):
Rename array_type_nelts() => array_type_nelts_minus_one()
gcc/fortran/ChangeLog:
* trans-array.cc (structure_alloc_comps)
* trans-openmp.cc
(gfc_walk_alloc_comps)
(gfc_omp_clause_linear_ctor):
Rename array_type_nelts() => array_type_nelts_minus_one()
gcc/rust/ChangeLog:
* backend/rust-tree.cc (array_type_nelts_top):
Rename array_type_nelts() => array_type_nelts_minus_one()
Cc: Gabriel Ravier
Cc: Martin Uecker
Cc: Joseph Myers
Cc: Xavier Del Campo Romero
Cc: Jakub Jelinek
Suggested-by: Richard Biener
Signed-off-by: Alejandro Colomar
---
gcc/c/c-decl.cc | 10 +-
gcc/c/c-fold.cc | 7 ---
gcc/config/aarch64/aarch64.cc | 2 +-
gcc/config/i386/i386.cc | 2 +-
gcc/cp/decl.cc | 2 +-
gcc/cp/init.cc | 8
gcc/cp/lambda.cc | 3 ++-
gcc/cp/tree.cc | 2 +-
gcc/expr.cc | 8
gcc/fortran/trans-array.cc| 2 +-
gcc/fortran/trans-openmp.cc | 4 ++--
gcc/rust/backend/rust-tree.cc | 2 +-
gcc/tree.cc | 4 ++--
gcc/tree.h| 2 +-
14 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index aa7f69d1b7b..c73d3107efb 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5358,7 +5358,7 @@ one_element_array_type_p (const_tree type)
{
if (TREE_CODE (type) != ARRAY_TYPE)
return false;
- return integer_zerop (array_type_nelts (type));
+ return integer_zerop (array_type_nelts_minus_one (type));
}
/* Determine whether TYPE is a zero-length array type "[0]". */
@@ -6306,15 +6306,15 @@ get_parm_array_spec (const struct c_parm *parm, tree
attrs)
for (tree type = parm->specs->type; TREE_CODE (type) == ARRAY_TYPE;
type = TREE_TYPE (type))
{
- tree nelts = array_type_nelts (type);
- if (error_operand_p (nelts))
+ tree nelts_minus_one = array_type_nelts_minus_one (type);
+ if (error_operand_p (nelts_minus_one))
return attrs;
- if (TREE_CODE (nelts) != INTEGER_CST)
+ if (TREE_CODE (nelts_minus_one) != INTEGER_CST)
{
/* Each variable VLA bound is represented by the dollar
sign. */
spec += "$";
- tpbnds = tree_cons (NULL_TREE, nelts, tpbnds);
+ tpbnds = tree_cons (NULL_TREE, nelts_minus_one, tpbnds);
}
}
tpbnds = nreverse (tpbnds);
diff --git a/gcc/c/c-fold.cc b/gcc/c/c-fold.cc
index 57b67c74bd8..9ea174f79c4 100644
--- a/gcc/c/c-fold.cc
+++ b/gcc/c/c-fold.cc
@@ -73,11 +73,12 @@ c_fold_array_ref (tree type, tree ary, tree index)
unsigned elem_nchars = (TYPE_PRECISION (elem_type)
/ TYPE_PRECISION (char_type_node));
unsigned len = (unsigned) TREE_STRING_LENGTH (ary) / elem_nchars;
- tree nelts = array_type_nelts (TREE_TYPE (ary));
+ tree nelts_minus_one = array_type_nelts_minus_one (TREE_TYPE (ary));
bool dummy1 = true, dummy2 = true;
- nelts = c_fully_fold_internal (nelts, true, &dummy1, &dummy2, false, false);
+ nelts_minus_one = c_fully_fold_internal (nelts_minus_one, true, &dummy1,
+ &dummy2, false, false);
unsigned HOST_WIDE_INT i = tree_to_uhwi (index);
- if (!tree_int_cst_le (index, nelts)
+ if (!tree_int_cst_le (index, nelts_minus_one)
|| i >= len
|| i + elem_nchars > len)
return NULL_TREE;
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 27e24ba70ab..21606701725 100644
--- a/gcc/config/aarc
Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option
On Fri, Aug 30, 2024 at 12:34 AM wrote:
>
> From: Deepthi Hemraj
>
> For excessively long environment variables i.e >128KB
> Store the arguments in a temporary file and collect them back together in
> collect2.
>
> This commit patches for COLLECT_GCC_OPTIONS issue:
> GCC should not limit the length of command line passed to collect2.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527
>
> The Linux kernel has the following limits on shell commands:
> I. Total number of bytes used to specify arguments must be under 128KB.
> II. Each environment variable passed to an executable must be under 128 KiB
>
> In order to circumvent these limitations, many build tools support
> response-files, i.e. files that contain the arguments for the executed
> command. These are typically passed using @ syntax.
>
> Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the
> expanded command line to collect2. With many options, this exceeds the limit
> II.
>
> GCC : Added Testcase for PR111527
>
> TC1 : If the command line argument less than 128kb, gcc should use
> COLLECT_GCC_OPTION to communicate and compile fine.
> TC2 : If the command line argument in the range of 128kb to 2mb,
> gcc should copy arguments in a file and use FILE_GCC_OPTIONS
> to communicate and compile fine.
> TC3 : If the command line argument greater thean 2mb, gcc shuld
> fail the compile and report error. (Expected FAIL)
>
> Signed-off-by: sunil dora
> Signed-off-by: Topi Kuutela
> Signed-off-by: Deepthi Hemraj
> ---
> gcc/collect2.cc | 39 ++--
> gcc/gcc.cc | 37 +--
> gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 +
> gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++++++
> gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 +
> gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++
> gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++
> 7 files changed, 159 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
>
> diff --git a/gcc/collect2.cc b/gcc/collect2.cc
> index 902014a9cc1..1f56963b1ce 100644
> --- a/gcc/collect2.cc
> +++ b/gcc/collect2.cc
> @@ -376,6 +376,39 @@ typedef int scanfilter;
>
> static void scan_prog_file (const char *, scanpass, scanfilter);
>
> +char* getenv_extended (const char* var_name)
> +{
> + int file_size;
> + char* buf = NULL;
> + const char* prefix = "/tmp";
> +
> + char* string = getenv (var_name);
> + if (strncmp (var_name, prefix, strlen(prefix)) == 0)
This is not what was meant by saying using the same env and supporting
response files.
Instead what Richard meant was use `@file` as the option that gets
passed via COLLECT_GCC_OPTIONS and then if you see `@` expand the
options like what is done for the normal command line.
Hard coding "/tmp" here is wrong because TMPDIR might not be set to
"/tmp" and even more with -save-temps, the response file should stay
around afterwards and be in the working directory rather than TMPDIR.
Thanks,
Andrew Pinski
> +{
> + FILE *fptr;
> + fptr = fopen (string, "r");
> + if (fptr == NULL)
> + return (0);
> + /* Copy contents from temporary file to buffer */
> + if (fseek (fptr, 0, SEEK_END) == -1)
> + return (0);
> + file_size = ftell (fptr);
> + rewind (fptr);
> + buf = (char *) xmalloc (file_size + 1);
> + if (buf == NULL)
> + return (0);
> + if (fread ((void *) buf, file_size, 1, fptr) <= 0)
> + {
> + free (buf);
> + fatal_error (input_location, "fread failed");
> + return (0);
> + }
> + buf[file_size] = '\0';
> + return buf;
> +}
> + return string;
> +}
> +
>
> /* Delete tempfiles and exit function. */
>
> @@ -1004,7 +1037,7 @@ main (int argc, char **argv)
> /* Now pick up any flags we want early from COLLECT_GCC_OPTIONS
> The LTO options are passed here as are other options that might
> be unsuitable for ld (e.g. -save-temps). */
> -p = getenv ("COLLECT_GCC_OPTIONS");
> +p = getenv_extended ("COLLECT_GCC_OPTIONS");
> while (p && *p)
>{
> const char *q = extract_string (&p);
> @@ -1200,7 +1233,7 @@ main (int argc, char **argv)
> A
Re: PING ^1 [PATCH] GCC Driver : Enable very long gcc command-line option
Thank you, Richard, for your feedback. I have removed the FILE_GCC_OPTION and used COLLECT_GCC_OPTION exclusively to manage the long command line option. The revised V2 patch has been posted and can be found at the following link: https://gcc.gnu.org/pipermail/gcc-patches/2024-August/661842.html I would appreciate it if you could review the updated patch. Best Regards, Sunil Dora From: Richard Biener Sent: Tuesday, August 27, 2024 6:09 PM To: Dora, Sunil Kumar Cc: gcc-patches@gcc.gnu.org ; pins...@gmail.com ; jeffreya...@gmail.com ; josmy...@redhat.com ; Hemraj, Deepthi ; MacLeod, Randy ; Gowda, Naveen ; Kokkonda, Sundeep Subject: Re: PING ^1 [PATCH] GCC Driver : Enable very long gcc command-line option CAUTION: This email comes from a non Wind River email account! Do not click links or open attachments unless you recognize the sender and know the content is safe. On Tue, 27 Aug 2024, Dora, Sunil Kumar wrote: > Dear GCC Team, > > Please consider this as a gentle reminder to review the patch I posted at the > following link: [ > https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660223.html ]. > > BUG Link : [ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527 ] > > Your feedback or approval would be greatly appreciated. > > Best Regards, > Sunil Dora > > On 2024-08-13 2:30 a.m., > deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com> wrote: > > From: sunil dora <mailto:sunil.dora1...@gmail.com> > > For excessively long environment variables i.e >128KB > Store the arguments in a temporary file and collect them back together in > collect2. > > This commit patches for COLLECT_GCC_OPTIONS issue: > GCC should not limit the length of command line passed to collect2. > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527<https://urldefense.com/v3/__https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8ecOLOVBw$> > > The Linux kernel has the following limits on shell commands: > I. Total number of bytes used to specify arguments must be under 128KB. > II. Each environment variable passed to an executable must be under 128 KiB > > In order to circumvent these limitations, many build tools support > response-files, i.e. files that contain the arguments for the executed > command. These are typically passed using @ syntax. > > Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the > expanded command line to collect2. With many options, this exceeds the limit > II. > > GCC : Added Testcase for PR111527 > > TC1 : If the command line argument less than 128kb, gcc should use > COLLECT_GCC_OPTION to communicate and compile fine. > TC2 : If the command line argument in the range of 128kb to 2mb, > gcc should copy arguments in a file and use FILE_GCC_OPTIONS > to communicate and compile fine. > TC3 : If the command line argument greater thean 2mb, gcc shuld > fail the compile and report error. (Expected FAIL) Just as a random comment - I'd prefer if COLLECT_GCC_OPTIONS would allow response files instead of indirecting via a new fixed other environment like the proposed FILE_GCC_OPTIONS. That looks like a cleaner interface to me. Richard. > Signed-off-by: Topi Kuutela > <mailto:topi.kuut...@nokia.com> > Signed-off-by: sunil dora > <mailto:sunil.dora1...@gmail.com> > > --- > > gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$> >| 40 +++-- > > gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8fDr-4p_g$> > | 37 +-- > gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 + > gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++ > gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 + > gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++ > gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++ > 7 files changed, 160 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp > create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c > create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c > create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c > create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c > > diff --git > a/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvT
[PATCH v2] GCC Driver : Enable very long gcc command-line option
From: Deepthi Hemraj
For excessively long environment variables i.e >128KB
Store the arguments in a temporary file and collect them back together in
collect2.
This commit patches for COLLECT_GCC_OPTIONS issue:
GCC should not limit the length of command line passed to collect2.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527
The Linux kernel has the following limits on shell commands:
I. Total number of bytes used to specify arguments must be under 128KB.
II. Each environment variable passed to an executable must be under 128 KiB
In order to circumvent these limitations, many build tools support
response-files, i.e. files that contain the arguments for the executed
command. These are typically passed using @ syntax.
Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the
expanded command line to collect2. With many options, this exceeds the limit II.
GCC : Added Testcase for PR111527
TC1 : If the command line argument less than 128kb, gcc should use
COLLECT_GCC_OPTION to communicate and compile fine.
TC2 : If the command line argument in the range of 128kb to 2mb,
gcc should copy arguments in a file and use FILE_GCC_OPTIONS
to communicate and compile fine.
TC3 : If the command line argument greater thean 2mb, gcc shuld
fail the compile and report error. (Expected FAIL)
Signed-off-by: sunil dora
Signed-off-by: Topi Kuutela
Signed-off-by: Deepthi Hemraj
---
gcc/collect2.cc | 39 ++--
gcc/gcc.cc| 37 +--
gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 +
gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++
gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 +
gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++
gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++
7 files changed, 159 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
diff --git a/gcc/collect2.cc b/gcc/collect2.cc
index 902014a9cc1..1f56963b1ce 100644
--- a/gcc/collect2.cc
+++ b/gcc/collect2.cc
@@ -376,6 +376,39 @@ typedef int scanfilter;
static void scan_prog_file (const char *, scanpass, scanfilter);
+char* getenv_extended (const char* var_name)
+{
+ int file_size;
+ char* buf = NULL;
+ const char* prefix = "/tmp";
+
+ char* string = getenv (var_name);
+ if (strncmp (var_name, prefix, strlen(prefix)) == 0)
+{
+ FILE *fptr;
+ fptr = fopen (string, "r");
+ if (fptr == NULL)
+ return (0);
+ /* Copy contents from temporary file to buffer */
+ if (fseek (fptr, 0, SEEK_END) == -1)
+ return (0);
+ file_size = ftell (fptr);
+ rewind (fptr);
+ buf = (char *) xmalloc (file_size + 1);
+ if (buf == NULL)
+ return (0);
+ if (fread ((void *) buf, file_size, 1, fptr) <= 0)
+ {
+ free (buf);
+ fatal_error (input_location, "fread failed");
+ return (0);
+ }
+ buf[file_size] = '\0';
+ return buf;
+}
+ return string;
+}
+
/* Delete tempfiles and exit function. */
@@ -1004,7 +1037,7 @@ main (int argc, char **argv)
/* Now pick up any flags we want early from COLLECT_GCC_OPTIONS
The LTO options are passed here as are other options that might
be unsuitable for ld (e.g. -save-temps). */
-p = getenv ("COLLECT_GCC_OPTIONS");
+p = getenv_extended ("COLLECT_GCC_OPTIONS");
while (p && *p)
{
const char *q = extract_string (&p);
@@ -1200,7 +1233,7 @@ main (int argc, char **argv)
AIX support needs to know if -shared has been specified before
parsing commandline arguments. */
- p = getenv ("COLLECT_GCC_OPTIONS");
+ p = getenv_extended ("COLLECT_GCC_OPTIONS");
while (p && *p)
{
const char *q = extract_string (&p);
@@ -1594,7 +1627,7 @@ main (int argc, char **argv)
fprintf (stderr, "o_file = %s\n",
(o_file ? o_file : "not found"));
- ptr = getenv ("COLLECT_GCC_OPTIONS");
+ ptr = getenv_extended ("COLLECT_GCC_OPTIONS");
if (ptr)
fprintf (stderr, "COLLECT_GCC_OPTIONS = %s\n", ptr);
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index d07a8e172a4..3d7fd8dff5b 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -2953,12 +2953,43 @@ add_to_obstack (char *path, void *data)
return NULL;
}
-/* Add or change the value of an environment variable, outputting the
- change to standard error if in verbose mode. */
+/* Add or change the value of an environment variable,
+ * outputting the change to standa
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Hi,
Thanks for the information.
Yes, providing a unary operator similar as __counted_by(PTR) as suggested by
multiple people previously is a cleaner approach.
Then the programmer will use the following:
__builtin_choose_expr(
__builtin_has_attribute (__p->FAM, "counted_by”)
__builtin_get_counted_by(__p->FAM) = COUNT, 0);
From the programmer’s point of view, it’s cleaner too.
However, there is one issue with “__builtin_choose_expr” currently in GCC, its
documentation explicitly mentions this limitation:
(https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
"Note: This construct is only available for C. Furthermore, the unused
expression (exp1 or exp2 depending on the value of const_exp) may still
generate syntax errors. This may change in future revisions.”
So, due to this limitation, when there is no counted_by attribute, the
__builtin_get_counted_by() still is evaluated by the compiler and errors is
issued and the compilation stops, this can be show from the small testing case:
[opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
struct flex {
unsigned int b;
int c[];
} *array_flex;
#define MY_ALLOC(P, FAM, COUNT) ({ \
typeof(P) __p; \
unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
__p = (typeof(P)) __builtin_malloc(__size); \
__builtin_choose_expr( \
__builtin_has_attribute (__p->FAM, counted_by), \
__builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
P = __p; \
})
int main(int argc, char *argv[])
{
MY_ALLOC(array_flex, c, 20);
return 0;
}
[opc@qinzhao-ol8u3-x86 gcc]$ sh t
ttt.c: In function ‘main’:
ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
‘__builtin_counted_by_ref’
ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it does parse
the __builtin_counted_by_ref(__p->FAM) even when
__builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued the error
when parsing __builtin_counted_by_ref and stopped the compilation.
So, in order to support this approach, we first must fix the issue in the
current __builtin_choose_expr in GCC. Otherwise, it’s impossible for the user
to use this new builtin.
Let me know your comments and suggestions.
thanks.
Qing
> On Aug 27, 2024, at 14:46, Bill Wendling wrote:
>>>
>>> It would make it easier for your use case. I wonder though
>>> whether other people might want to have the compile time error
>>> when there is no attribute.
>>
>> Then this will go back to the previous discussion: whether we should go the
>> approach for the unary operator __counted_by(PTR), then the other builtin
>> __builtin_has_attribute(PTR, counted_by) should be provided to the user.
>>
>> For GCC, there is no issue, we already have the __builtin_has_attribute
>> (PTR, counted_by) available.
>> But CLANG doesn’t have this builtin currently, need to implement it before
>> the unary operator __counted_by(PTR).
>>
>> Let me know your opinion.
>
> We have a bit of an issue with the current design. The group at Apple,
> who are implementing a lot of the bounds safety checks need to prevent
> the address of the 'count' from being taken. See [1] for details, but
> the upshot is that they try to ensure that the 'count' and flexible
> array member aren't modified independently. The discussion is ongoing,
> but they suggested an alternative that is similar to ones discussed in
> this thread and the bugzilla.
>
> A brief description of their proposal is adding the builtin
> '__builtin_bounds_attr_arg'. It would take counted_by's argument and
> return the counter directly:
>
> __builtin_choose_expr(
> __builtin_has_attribute(__p->FAM, "counted_by"),
> __builtin_bounds_attr_arg(__p->FAM) = COUNT,
> (void)
> );
>
> This is similar to one of the original proposals for
> __builtin_get_counted_by, but we changed because Clang didn't have
> __builtin_has_attribute. Clang should implement
> __builtin_has_attribute regardless, and if this is a better way of
> doing things, and doesn't interfere with Apple's work, then I can add
> __builtin_has_attribute to Clang so that we "do the right thing."
>
> As I said though, the discussion is ongoing at [1], but just wanted to
> give everyone a warning, and feel free to jump in with comments /
> suggestions.
>
> [1]
> https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836
>
> -bw
RE: [gcc-wwwdocs PATCH] gcc-15: Mention recent update for x86_64 backend
> -Original Message- > From: Gerald Pfeifer > Sent: Thursday, August 29, 2024 3:20 AM > > On Wed, 28 Aug 2024, Haochen Jiang wrote: > > Sorry for the disturb since I mis-typoed gcc-patches to gcc-patchs, > > resend the patch. > > No worries. > > > This patch will add documentation for recent update in x86-64 backend. > > Thank you! > > > + Xeon Phi CPUs support (a.k.a. Knight Landing and Knight Mill) > > + were removed > > I believe "Support for Xeon Phi CPUs" or "Xeon Phi CPU support" would be > better, > though not 100% sure. > > > + in GCC 15. GCC will no longer accept -mavx5124fmaps, > > + -mavx5124vnniw, -mavx512er, > > + -mavx512pf, -mprefetchwt1, > > + -march=knl, -march=knm, - > mtune=knl > > + or -mtune=knm compiler switches. > > Is there a particular rationale for the order of switches? If not, I'd sort > them > alphabetically (which is partially the case already) and start with -march=... > > The patch is okay if you consider (which is not necessarily making) these > changes. I will change them and commit them tomorrow if there is no objection. Thx, Haochen > > Gerald
Re: [gcc-wwwdocs PATCH] gcc-15: Mention recent update for x86_64 backend
On Wed, 28 Aug 2024, Haochen Jiang wrote: > Sorry for the disturb since I mis-typoed gcc-patches to gcc-patchs, > resend the patch. No worries. > This patch will add documentation for recent update in x86-64 backend. Thank you! > + Xeon Phi CPUs support (a.k.a. Knight Landing and Knight Mill) were > removed I believe "Support for Xeon Phi CPUs" or "Xeon Phi CPU support" would be better, though not 100% sure. > + in GCC 15. GCC will no longer accept -mavx5124fmaps, > + -mavx5124vnniw, -mavx512er, > + -mavx512pf, -mprefetchwt1, > + -march=knl, -march=knm, > -mtune=knl > + or -mtune=knm compiler switches. Is there a particular rationale for the order of switches? If not, I'd sort them alphabetically (which is partially the case already) and start with -march=... The patch is okay if you consider (which is not necessarily making) these changes. Gerald
[committed][wwwdocs] Document libstdc++ changes in GCC 15
Pushed to wwwdocs. --- htdocs/gcc-15/changes.html | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/htdocs/gcc-15/changes.html b/htdocs/gcc-15/changes.html index d0d6d147..91c020dd 100644 --- a/htdocs/gcc-15/changes.html +++ b/htdocs/gcc-15/changes.html @@ -84,7 +84,24 @@ a work-in-progress. when parsing a template. - +Runtime Library (libstdc++) + + + Improved experimental support for C++26, including: + +views::concat. +Member visit. +Type-checking std::format args. + + + Improved experimental support for C++23, including: + + + Clarify handling of encodings in localized formatting of chrono types. + + + + -- 2.46.0
New Georgian PO file for 'gcc' (version 14.2.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Georgian team of translators. The file is available at: https://translationproject.org/latest/gcc/ka.po (This file, 'gcc-14.2.0.ka.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
[gcc-wwwdocs PATCH] gcc-15: Mention recent update for x86_64 backend
Hi all, Sorry for the disturb since I mis-typoed gcc-patches to gcc-patchs, resend the patch. This patch will add documentation for recent update in x86-64 backend. Ok for wwwdocs trunk? Thx, Haochen --- Mention AVX10.2 support and Xeon Phi removal in GCC 15. --- htdocs/gcc-15/changes.html | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/htdocs/gcc-15/changes.html b/htdocs/gcc-15/changes.html index d0d6d147..4cb0fa90 100644 --- a/htdocs/gcc-15/changes.html +++ b/htdocs/gcc-15/changes.html @@ -132,7 +132,23 @@ a work-in-progress. code like 1 << offset is not fast enough. - +IA-32/x86-64 + + + New ISA extension support for Intel AVX10.2 was added. + AVX10.2 intrinsics are available via the -mavx10.2 or + -mavx10.2-256 compiler switch with 256-bit vector size + support. 512-bit vector size support for AVX10.2 intrinsics are + available via the -mavx10.2-512 compiler switch. + + Xeon Phi CPUs support (a.k.a. Knight Landing and Knight Mill) were removed + in GCC 15. GCC will no longer accept -mavx5124fmaps, + -mavx5124vnniw, -mavx512er, + -mavx512pf, -mprefetchwt1, + -march=knl, -march=knm, -mtune=knl + or -mtune=knm compiler switches. + + -- 2.31.1
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Tue, Aug 27, 2024 at 6:58 AM Qing Zhao wrote:
> > On Aug 27, 2024, at 02:17, Martin Uecker wrote:
> > Am Montag, dem 26.08.2024 um 17:21 -0700 schrieb Kees Cook:
> >> On Mon, Aug 26, 2024 at 11:01:08PM +0200, Martin Uecker wrote:
> >>> Am Montag, dem 26.08.2024 um 13:30 -0700 schrieb Kees Cook:
> >>>> On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
> >>>>> Hi, Martin,
> >>>>>
> >>>>> Looks like that there is some issue when I tried to use the _Generic
> >>>>> for the testing cases, and then I narrowed down to a
> >>>>> small testing case that shows the problem without any change to GCC.
> >>>>>
> >>>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
> >>>>> struct annotated {
> >>>>> char b;
> >>>>> int c[];
> >>>>> } *array_annotated;
> >>>>> extern void * counted_by_ref (int *);
> >>>>>
> >>>>> int main(int argc, char *argv[])
> >>>>> {
> >>>>> typeof(counted_by_ref (array_annotated->c)) ret
> >>>>>= counted_by_ref (array_annotated->c);
> >>>>> _Generic (ret, void* : (void)0, default: *ret = 10);
> >>>>>
> >>>>> return 0;
> >>>>> }
> >>>>> [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
> >>>>> t1.c: In function ‘main’:
> >>>>> t1.c:12:44: warning: dereferencing ‘void *’ pointer
> >>>>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> >>>>> |^~~~
> >>>>> t1.c:12:49: error: invalid use of void expression
> >>>>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> >>>>> | ^
> >>>>
> >>>> I implemented it like this[1] in the Linux kernel. So yours could be:
> >>>>
> >>>> struct annotated {
> >>>> char b;
> >>>> int c[] __attribute__((counted_by(b));
> >>>> };
> >>>> extern struct annotated *array_annotated;
> >>>>
> >>>> int main(int argc, char *argv[])
> >>>> {
> >>>> typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
> >>>>void *: (size_t *)NULL,
> >>>>default: __builtin_get_counted_by(array_annotated->c)))
> >>>> ret = __builtin_get_counted_by(array_annotated->c);
> >>>> if (ret)
> >>>> *ret = 10;
> >>>>
> >>>> return 0;
> >>>> }
> >>>>
> >>>> It's a bit cumbersome, but it does what's needed.
> >>>>
> >>>> This is, however, just doing exactly what Bill has suggested: it is
> >>>> converting the (void *)NULL into (size_t *)NULL when there is no
> >>>> counted_by annotation...
> >>>>
> >>>> -Kees
> >>>>
> >>>> [1]
> >>>> https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
> >>>
> >>> Interesting. Will __builtin_get_counted_by(array_annotated->c) give
> >>> a null pointer (or an invalid pointer) of the correct type if
> >>> array_annotated is a null pointer of an annotated struct type?
> >>
> >> If you mean this part:
> >>
> >> typeof(P) __obj_ptr = NULL; \
> >> /* Just query the counter type for type_max checking. */ \
> >> typeof(_Generic(__flex_counter(__obj_ptr->FAM), \
> >> void *: (size_t *)NULL, \
> >> default: __flex_counter(__obj_ptr->FAM))) \
> >> __counter_type_ptr = NULL; \
> >>
> >> Where __obj_ptr starts as NULL, then yes. (Or at least, yes it does
> >> currently with Qing's GCC patch and Bill's Clang patch.)
> >
> > Does __builtin_get_counted_by not evaluate its argument?
>
> __builtin_get_counted_by currently is implemented as a C reserved words, and
> purely implemented in C parser as an C operator.
>
> So, it doesn’t apply complicated evaluations on its argument.
> I think that this should provide enough and simple functionality as needed.
> If no, please let me know.
>
>
> > In any
> > case, I think this should be documented whether this is
> > supposed to work (or not).
> Okay.
> >
> >>
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Aug 27, 2024, at 02:17, Martin Uecker wrote:
>
> Am Montag, dem 26.08.2024 um 17:21 -0700 schrieb Kees Cook:
>> On Mon, Aug 26, 2024 at 11:01:08PM +0200, Martin Uecker wrote:
>>> Am Montag, dem 26.08.2024 um 13:30 -0700 schrieb Kees Cook:
>>>> On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
>>>>> Hi, Martin,
>>>>>
>>>>> Looks like that there is some issue when I tried to use the _Generic for
>>>>> the testing cases, and then I narrowed down to a
>>>>> small testing case that shows the problem without any change to GCC.
>>>>>
>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
>>>>> struct annotated {
>>>>> char b;
>>>>> int c[];
>>>>> } *array_annotated;
>>>>> extern void * counted_by_ref (int *);
>>>>>
>>>>> int main(int argc, char *argv[])
>>>>> {
>>>>> typeof(counted_by_ref (array_annotated->c)) ret
>>>>>= counted_by_ref (array_annotated->c);
>>>>> _Generic (ret, void* : (void)0, default: *ret = 10);
>>>>>
>>>>> return 0;
>>>>> }
>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
>>>>> t1.c: In function ‘main’:
>>>>> t1.c:12:44: warning: dereferencing ‘void *’ pointer
>>>>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
>>>>> |^~~~
>>>>> t1.c:12:49: error: invalid use of void expression
>>>>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
>>>>> | ^
>>>>
>>>> I implemented it like this[1] in the Linux kernel. So yours could be:
>>>>
>>>> struct annotated {
>>>> char b;
>>>> int c[] __attribute__((counted_by(b));
>>>> };
>>>> extern struct annotated *array_annotated;
>>>>
>>>> int main(int argc, char *argv[])
>>>> {
>>>> typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
>>>>void *: (size_t *)NULL,
>>>>default: __builtin_get_counted_by(array_annotated->c)))
>>>> ret = __builtin_get_counted_by(array_annotated->c);
>>>> if (ret)
>>>> *ret = 10;
>>>>
>>>> return 0;
>>>> }
>>>>
>>>> It's a bit cumbersome, but it does what's needed.
>>>>
>>>> This is, however, just doing exactly what Bill has suggested: it is
>>>> converting the (void *)NULL into (size_t *)NULL when there is no
>>>> counted_by annotation...
>>>>
>>>> -Kees
>>>>
>>>> [1]
>>>> https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
>>>
>>> Interesting. Will __builtin_get_counted_by(array_annotated->c) give
>>> a null pointer (or an invalid pointer) of the correct type if
>>> array_annotated is a null pointer of an annotated struct type?
>>
>> If you mean this part:
>>
>> typeof(P) __obj_ptr = NULL; \
>> /* Just query the counter type for type_max checking. */ \
>> typeof(_Generic(__flex_counter(__obj_ptr->FAM), \
>> void *: (size_t *)NULL, \
>> default: __flex_counter(__obj_ptr->FAM))) \
>> __counter_type_ptr = NULL; \
>>
>> Where __obj_ptr starts as NULL, then yes. (Or at least, yes it does
>> currently with Qing's GCC patch and Bill's Clang patch.)
>
> Does __builtin_get_counted_by not evaluate its argument?
__builtin_get_counted_by currently is implemented as a C reserved words, and
purely implemented in C parser as an C operator.
So, it doesn’t apply complicated evaluations on its argument.
I think that this should provide enough and simple functionality as needed.
If no, please let me know.
> In any
> case, I think this should be documented whether this is
> supposed to work (or not).
Okay.
>
>>
>>> I also wonder a bit about the multiple macro evaluations of the arguments
>>> for P and SIZE.
>>
>> I tried to design it so they aren't used with anything that should
>> have side-effects.
>
> I was more concerned about the cost of macro expansions on
> compile times. I would do:
>
> __auto_type __FOO = (FOO);
>
> for all macro parameters that are evaluated multiple times
> and are expressions which might contain macros themselves.
>
> There is also the issue of evaluation of typeof for variably modified
> types, which might not currently affect the kernel, but this would
> also become safer for such types.
>
>
>> Anyway, if __builtin_get_counted_by returns (size_t *)NULL then I think
>> the _Generic wrapping isn't needed. That would make it easier to use?
>
> It would make it easier for your use case. I wonder though
> whether other people might want to have the compile time error
> when there is no attribute.
Then this will go back to the previous discussion: whether we should go the
approach for the unary operator __counted_by(PTR), then the other builtin
__builtin_has_attribute(PTR, counted_by) should be provided to the user.
For GCC, there is no issue, we already have the __builtin_has_attribute (PTR,
counted_by) available.
But CLANG doesn’t have this builtin currently, need to implement it before the
unary operator __counted_by(PTR).
Let me know your opinion.
Thanks
Qing
>
>
> Martin
>
>>
>> -Kees
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Aug 26, 2024, at 17:01, Martin Uecker wrote:
>
> Am Montag, dem 26.08.2024 um 13:30 -0700 schrieb Kees Cook:
>> On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
>>> Hi, Martin,
>>>
>>> Looks like that there is some issue when I tried to use the _Generic for
>>> the testing cases, and then I narrowed down to a
>>> small testing case that shows the problem without any change to GCC.
>>>
>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
>>> struct annotated {
>>> char b;
>>> int c[];
>>> } *array_annotated;
>>> extern void * counted_by_ref (int *);
>>>
>>> int main(int argc, char *argv[])
>>> {
>>> typeof(counted_by_ref (array_annotated->c)) ret
>>>= counted_by_ref (array_annotated->c);
>>> _Generic (ret, void* : (void)0, default: *ret = 10);
>>>
>>> return 0;
>>> }
>>> [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
>>> t1.c: In function ‘main’:
>>> t1.c:12:44: warning: dereferencing ‘void *’ pointer
>>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
>>> |^~~~
>>> t1.c:12:49: error: invalid use of void expression
>>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
>>> | ^
>>
>> I implemented it like this[1] in the Linux kernel. So yours could be:
>>
>> struct annotated {
>> char b;
>> int c[] __attribute__((counted_by(b));
>> };
>> extern struct annotated *array_annotated;
>>
>> int main(int argc, char *argv[])
>> {
>> typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
>>void *: (size_t *)NULL,
>>default: __builtin_get_counted_by(array_annotated->c)))
>> ret = __builtin_get_counted_by(array_annotated->c);
>> if (ret)
>> *ret = 10;
>>
>> return 0;
>> }
>>
>> It's a bit cumbersome, but it does what's needed.
>>
>> This is, however, just doing exactly what Bill has suggested: it is
>> converting the (void *)NULL into (size_t *)NULL when there is no
>> counted_by annotation...
>>
>> -Kees
>>
>> [1]
>> https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
>
> Interesting. Will __builtin_get_counted_by(array_annotated->c) give
> a null pointer (or an invalid pointer) of the correct type if
> array_annotated is a null pointer of an annotated struct type?
A little confused here:
1. can array_annotated->c be passed to __builtin_get_counted_by as
__builtin_get_counted_by(array_annotated->c) when array_annotated is NULL?
2. If YES, then this should cause a run time error?
Qing
>
> I also wonder a bit about the multiple macro evaluations of the arguments
> for P and SIZE.
>
> Martin
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Aug 26, 2024, at 16:30, Kees Cook wrote:
>
> On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
>> Hi, Martin,
>>
>> Looks like that there is some issue when I tried to use the _Generic for the
>> testing cases, and then I narrowed down to a
>> small testing case that shows the problem without any change to GCC.
>>
>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
>> struct annotated {
>> char b;
>> int c[];
>> } *array_annotated;
>> extern void * counted_by_ref (int *);
>>
>> int main(int argc, char *argv[])
>> {
>> typeof(counted_by_ref (array_annotated->c)) ret
>>= counted_by_ref (array_annotated->c);
>> _Generic (ret, void* : (void)0, default: *ret = 10);
>>
>> return 0;
>> }
>> [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
>> t1.c: In function ‘main’:
>> t1.c:12:44: warning: dereferencing ‘void *’ pointer
>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
>> |^~~~
>> t1.c:12:49: error: invalid use of void expression
>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
>> | ^
>
> I implemented it like this[1] in the Linux kernel. So yours could be:
>
> struct annotated {
> char b;
> int c[] __attribute__((counted_by(b));
> };
> extern struct annotated *array_annotated;
>
> int main(int argc, char *argv[])
> {
> typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
>void *: (size_t *)NULL,
>default: __builtin_get_counted_by(array_annotated->c)))
> ret = __builtin_get_counted_by(array_annotated->c);
> if (ret)
> *ret = 10;
>
> return 0;
> }
>
> It's a bit cumbersome, but it does what's needed.
>
> This is, however, just doing exactly what Bill has suggested: it is
> converting the (void *)NULL into (size_t *)NULL when there is no
> counted_by annotation...
That’s the reason why I returned a (size_t *) instead of a (void *) for the
NULL pointer when there is no counted_by annotation in the 1st version of the
patch, then the conversion from (void *) NULL to (size_t *) NULL in the source
code level will not be needed.
Then the above can be simplified as:
typeof (__builtin_get_counted_by(array_annotated->c)) ret =
__builtin_get_counted_by(array_annotated->c);
If (ret) *ret = 10;
I am wondering shall I still keep the (size_t *) for the returned NULL pointer
instead of the (void *)?
Qing
>
> -Kees
>
> [1]
> https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
>
> --
> Kees Cook
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Aug 26, 2024, at 15:46, Bill Wendling wrote: > > On Wed, Aug 21, 2024 at 8:43 AM Martin Uecker wrote: >> >> Am Mittwoch, dem 21.08.2024 um 15:24 + schrieb Qing Zhao: >>>> >>>> But if we changed it to return a void pointer, we could make this >>>> a compile-time check: >>>> >>>> auto ret = __builtin_get_counted_by(__p->FAM); >>>> >>>> _Generic(ret, void*: (void)0, default: *ret = COUNT); >>> >>> Is there any benefit to return a void pointer than a SIZE_T pointer for >>> the NULL pointer? >> >> Yes! You can test with _Generic (or __builtin_types_compatible_p) >> at compile-time based on the type whether you can set *ret to COUNT >> or not as in the example above. >> >> So it is not a weird run-time test which needs to be optimized >> away. >> > Using a '_Generic' moves so much of the work onto the programmer that > it would be far easier, and cleaner, for them simply to specify the > 'counter' field in the macro and be done with it. Something like: > > #define alloc(PTR, COUNT, FAM, COUNTER) > > If the FAM doesn't have a 'counted_by' field: > > #define alloc(PTR, COUNT, FAM) > > (It would use VAR_ARGS of course). Why not simply have the compiler > automatically adjust the return type? What do you mean by “have the compiler automatically adjust the return type”? From my current GCC implementation, if there is counted_by object associated with the flexible array member, then the returned pointer points to the counted_by object (which has its own original type), compiler does not need to _adjust_ the returned type. So, what do you mean by _adjust_? Qing > It's perfectly capable of Doing > the Right Thing(tm). Otherwise, this builtin becomes even less > desirable to use than it currently is.
Re: PING ^1 [PATCH] GCC Driver : Enable very long gcc command-line option
On Tue, 27 Aug 2024, Dora, Sunil Kumar wrote:
> Dear GCC Team,
>
> Please consider this as a gentle reminder to review the patch I posted at the
> following link: [
> https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660223.html ].
>
> BUG Link : [ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527 ]
>
> Your feedback or approval would be greatly appreciated.
>
> Best Regards,
> Sunil Dora
>
> On 2024-08-13 2:30 a.m.,
> deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com> wrote:
>
> From: sunil dora <mailto:sunil.dora1...@gmail.com>
>
> For excessively long environment variables i.e >128KB
> Store the arguments in a temporary file and collect them back together in
> collect2.
>
> This commit patches for COLLECT_GCC_OPTIONS issue:
> GCC should not limit the length of command line passed to collect2.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527<https://urldefense.com/v3/__https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8ecOLOVBw$>
>
> The Linux kernel has the following limits on shell commands:
> I. Total number of bytes used to specify arguments must be under 128KB.
> II. Each environment variable passed to an executable must be under 128 KiB
>
> In order to circumvent these limitations, many build tools support
> response-files, i.e. files that contain the arguments for the executed
> command. These are typically passed using @ syntax.
>
> Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the
> expanded command line to collect2. With many options, this exceeds the limit
> II.
>
> GCC : Added Testcase for PR111527
>
> TC1 : If the command line argument less than 128kb, gcc should use
> COLLECT_GCC_OPTION to communicate and compile fine.
> TC2 : If the command line argument in the range of 128kb to 2mb,
> gcc should copy arguments in a file and use FILE_GCC_OPTIONS
> to communicate and compile fine.
> TC3 : If the command line argument greater thean 2mb, gcc shuld
> fail the compile and report error. (Expected FAIL)
Just as a random comment - I'd prefer if COLLECT_GCC_OPTIONS would
allow response files instead of indirecting via a new fixed other
environment like the proposed FILE_GCC_OPTIONS.
That looks like a cleaner interface to me.
Richard.
> Signed-off-by: Topi Kuutela
> <mailto:topi.kuut...@nokia.com>
> Signed-off-by: sunil dora
> <mailto:sunil.dora1...@gmail.com>
>
> ---
>
> gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
>| 40 +++--
>
> gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8fDr-4p_g$>
> | 37 +--
> gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 +
> gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++
> gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 +
> gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++
> gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++
> 7 files changed, 160 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
>
> diff --git
> a/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
>
> b/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
> index 902014a9cc1..564d8968648 100644
> ---
> a/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
> +++
> b/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
> @@ -376,6 +376,40 @@ typedef int scanfilter;
>
> static void scan_prog_file (const char *, scanpass, scanfilter);
>
> +char* getenv_extended (const char* var_name)
> +{
> + int file_size;
>
PING ^1 [PATCH] GCC Driver : Enable very long gcc command-line option
Dear GCC Team,
Please consider this as a gentle reminder to review the patch I posted at the
following link: [
https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660223.html ].
BUG Link : [ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527 ]
Your feedback or approval would be greatly appreciated.
Best Regards,
Sunil Dora
On 2024-08-13 2:30 a.m.,
deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com> wrote:
From: sunil dora <mailto:sunil.dora1...@gmail.com>
For excessively long environment variables i.e >128KB
Store the arguments in a temporary file and collect them back together in
collect2.
This commit patches for COLLECT_GCC_OPTIONS issue:
GCC should not limit the length of command line passed to collect2.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527<https://urldefense.com/v3/__https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8ecOLOVBw$>
The Linux kernel has the following limits on shell commands:
I. Total number of bytes used to specify arguments must be under 128KB.
II. Each environment variable passed to an executable must be under 128 KiB
In order to circumvent these limitations, many build tools support
response-files, i.e. files that contain the arguments for the executed
command. These are typically passed using @ syntax.
Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the
expanded command line to collect2. With many options, this exceeds the limit II.
GCC : Added Testcase for PR111527
TC1 : If the command line argument less than 128kb, gcc should use
COLLECT_GCC_OPTION to communicate and compile fine.
TC2 : If the command line argument in the range of 128kb to 2mb,
gcc should copy arguments in a file and use FILE_GCC_OPTIONS
to communicate and compile fine.
TC3 : If the command line argument greater thean 2mb, gcc shuld
fail the compile and report error. (Expected FAIL)
Signed-off-by: Topi Kuutela
<mailto:topi.kuut...@nokia.com>
Signed-off-by: sunil dora
<mailto:sunil.dora1...@gmail.com>
---
gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
| 40 +++--
gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8fDr-4p_g$>
| 37 +--
gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 +
gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++
gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 +
gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++
gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++
7 files changed, 160 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
diff --git
a/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
b/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
index 902014a9cc1..564d8968648 100644
---
a/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
+++
b/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$>
@@ -376,6 +376,40 @@ typedef int scanfilter;
static void scan_prog_file (const char *, scanpass, scanfilter);
+char* getenv_extended (const char* var_name)
+{
+ int file_size;
+ char* buf = NULL;
+
+ char* string = getenv (var_name);
+ if (!string)
+{
+ char* string = getenv ("FILE_GCC_OPTIONS");
+ FILE *fptr;
+ fptr = fopen (string, "r");
+ if (fptr == NULL)
+ return (0);
+ /* Copy contents from temporary file to buffer */
+ if (fseek (fptr, 0, SEEK_END) == -1)
+ return (0);
+ file_size = ftell (fptr);
+ rewind (fptr);
+ buf = (char *) xmalloc (file_size + 1);
+ if (buf == NULL)
+ return (0);
+ if (fread ((void *) buf, file_size, 1, fptr) <= 0)
+ {
+ free (buf);
+ fatal_error (input_location, "fread failed");
+ return (0);
+ }
+ buf[file_si
New Chinese (simplified) PO file for 'gcc' (version 14.2.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Chinese (simplified) team of translators. The file is available at: https://translationproject.org/latest/gcc/zh_CN.po (This file, 'gcc-14.2.0.zh_CN.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Montag, dem 26.08.2024 um 17:21 -0700 schrieb Kees Cook:
> On Mon, Aug 26, 2024 at 11:01:08PM +0200, Martin Uecker wrote:
> > Am Montag, dem 26.08.2024 um 13:30 -0700 schrieb Kees Cook:
> > > On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
> > > > Hi, Martin,
> > > >
> > > > Looks like that there is some issue when I tried to use the _Generic
> > > > for the testing cases, and then I narrowed down to a
> > > > small testing case that shows the problem without any change to GCC.
> > > >
> > > > [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
> > > > struct annotated {
> > > > char b;
> > > > int c[];
> > > > } *array_annotated;
> > > > extern void * counted_by_ref (int *);
> > > >
> > > > int main(int argc, char *argv[])
> > > > {
> > > > typeof(counted_by_ref (array_annotated->c)) ret
> > > > = counted_by_ref (array_annotated->c);
> > > >_Generic (ret, void* : (void)0, default: *ret = 10);
> > > >
> > > > return 0;
> > > > }
> > > > [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
> > > > t1.c: In function ‘main’:
> > > > t1.c:12:44: warning: dereferencing ‘void *’ pointer
> > > >12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> > > > |^~~~
> > > > t1.c:12:49: error: invalid use of void expression
> > > >12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> > > > | ^
> > >
> > > I implemented it like this[1] in the Linux kernel. So yours could be:
> > >
> > > struct annotated {
> > > char b;
> > > int c[] __attribute__((counted_by(b));
> > > };
> > > extern struct annotated *array_annotated;
> > >
> > > int main(int argc, char *argv[])
> > > {
> > > typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
> > > void *: (size_t *)NULL,
> > > default: __builtin_get_counted_by(array_annotated->c)))
> > > ret = __builtin_get_counted_by(array_annotated->c);
> > > if (ret)
> > > *ret = 10;
> > >
> > > return 0;
> > > }
> > >
> > > It's a bit cumbersome, but it does what's needed.
> > >
> > > This is, however, just doing exactly what Bill has suggested: it is
> > > converting the (void *)NULL into (size_t *)NULL when there is no
> > > counted_by annotation...
> > >
> > > -Kees
> > >
> > > [1]
> > > https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
> >
> > Interesting. Will __builtin_get_counted_by(array_annotated->c) give
> > a null pointer (or an invalid pointer) of the correct type if
> > array_annotated is a null pointer of an annotated struct type?
>
> If you mean this part:
>
> typeof(P) __obj_ptr = NULL; \
> /* Just query the counter type for type_max checking. */ \
> typeof(_Generic(__flex_counter(__obj_ptr->FAM), \
> void *: (size_t *)NULL, \
> default: __flex_counter(__obj_ptr->FAM))) \
> __counter_type_ptr = NULL; \
>
> Where __obj_ptr starts as NULL, then yes. (Or at least, yes it does
> currently with Qing's GCC patch and Bill's Clang patch.)
Does __builtin_get_counted_by not evaluate its argument? In any
case, I think this should be documented whether this is
supposed to work (or not).
>
> > I also wonder a bit about the multiple macro evaluations of the arguments
> > for P and SIZE.
>
> I tried to design it so they aren't used with anything that should
> have side-effects.
I was more concerned about the cost of macro expansions on
compile times. I would do:
__auto_type __FOO = (FOO);
for all macro parameters that are evaluated multiple times
and are expressions which might contain macros themselves.
There is also the issue of evaluation of typeof for variably modified
types, which might not currently affect the kernel, but this would
also become safer for such types.
> Anyway, if __builtin_get_counted_by returns (size_t *)NULL then I think
> the _Generic wrapping isn't needed. That would make it easier to use?
It would make it easier for your use case. I wonder though
whether other people might want to have the compile time error
when there is no attribute.
Martin
>
> -Kees
>
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Mon, Aug 26, 2024 at 11:01:08PM +0200, Martin Uecker wrote:
> Am Montag, dem 26.08.2024 um 13:30 -0700 schrieb Kees Cook:
> > On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
> > > Hi, Martin,
> > >
> > > Looks like that there is some issue when I tried to use the _Generic for
> > > the testing cases, and then I narrowed down to a
> > > small testing case that shows the problem without any change to GCC.
> > >
> > > [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
> > > struct annotated {
> > > char b;
> > > int c[];
> > > } *array_annotated;
> > > extern void * counted_by_ref (int *);
> > >
> > > int main(int argc, char *argv[])
> > > {
> > > typeof(counted_by_ref (array_annotated->c)) ret
> > > = counted_by_ref (array_annotated->c);
> > >_Generic (ret, void* : (void)0, default: *ret = 10);
> > >
> > > return 0;
> > > }
> > > [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
> > > t1.c: In function ‘main’:
> > > t1.c:12:44: warning: dereferencing ‘void *’ pointer
> > >12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> > > |^~~~
> > > t1.c:12:49: error: invalid use of void expression
> > >12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> > > | ^
> >
> > I implemented it like this[1] in the Linux kernel. So yours could be:
> >
> > struct annotated {
> > char b;
> > int c[] __attribute__((counted_by(b));
> > };
> > extern struct annotated *array_annotated;
> >
> > int main(int argc, char *argv[])
> > {
> > typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
> >void *: (size_t *)NULL,
> >default: __builtin_get_counted_by(array_annotated->c)))
> > ret = __builtin_get_counted_by(array_annotated->c);
> > if (ret)
> > *ret = 10;
> >
> > return 0;
> > }
> >
> > It's a bit cumbersome, but it does what's needed.
> >
> > This is, however, just doing exactly what Bill has suggested: it is
> > converting the (void *)NULL into (size_t *)NULL when there is no
> > counted_by annotation...
> >
> > -Kees
> >
> > [1]
> > https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
>
> Interesting. Will __builtin_get_counted_by(array_annotated->c) give
> a null pointer (or an invalid pointer) of the correct type if
> array_annotated is a null pointer of an annotated struct type?
If you mean this part:
typeof(P) __obj_ptr = NULL; \
/* Just query the counter type for type_max checking. */ \
typeof(_Generic(__flex_counter(__obj_ptr->FAM), \
void *: (size_t *)NULL, \
default: __flex_counter(__obj_ptr->FAM))) \
__counter_type_ptr = NULL; \
Where __obj_ptr starts as NULL, then yes. (Or at least, yes it does
currently with Qing's GCC patch and Bill's Clang patch.)
> I also wonder a bit about the multiple macro evaluations of the arguments
> for P and SIZE.
I tried to design it so they aren't used with anything that should
have side-effects.
Anyway, if __builtin_get_counted_by returns (size_t *)NULL then I think
the _Generic wrapping isn't needed. That would make it easier to use?
-Kees
--
Kees Cook
New Chinese (simplified) PO file for 'gcc' (version 14.2.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Chinese (simplified) team of translators. The file is available at: https://translationproject.org/latest/gcc/zh_CN.po (This file, 'gcc-14.2.0.zh_CN.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Montag, dem 26.08.2024 um 13:30 -0700 schrieb Kees Cook:
> On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
> > Hi, Martin,
> >
> > Looks like that there is some issue when I tried to use the _Generic for
> > the testing cases, and then I narrowed down to a
> > small testing case that shows the problem without any change to GCC.
> >
> > [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
> > struct annotated {
> > char b;
> > int c[];
> > } *array_annotated;
> > extern void * counted_by_ref (int *);
> >
> > int main(int argc, char *argv[])
> > {
> > typeof(counted_by_ref (array_annotated->c)) ret
> > = counted_by_ref (array_annotated->c);
> >_Generic (ret, void* : (void)0, default: *ret = 10);
> >
> > return 0;
> > }
> > [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
> > t1.c: In function ‘main’:
> > t1.c:12:44: warning: dereferencing ‘void *’ pointer
> >12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> > |^~~~
> > t1.c:12:49: error: invalid use of void expression
> >12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> > | ^
>
> I implemented it like this[1] in the Linux kernel. So yours could be:
>
> struct annotated {
> char b;
> int c[] __attribute__((counted_by(b));
> };
> extern struct annotated *array_annotated;
>
> int main(int argc, char *argv[])
> {
> typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
> void *: (size_t *)NULL,
> default: __builtin_get_counted_by(array_annotated->c)))
> ret = __builtin_get_counted_by(array_annotated->c);
> if (ret)
> *ret = 10;
>
> return 0;
> }
>
> It's a bit cumbersome, but it does what's needed.
>
> This is, however, just doing exactly what Bill has suggested: it is
> converting the (void *)NULL into (size_t *)NULL when there is no
> counted_by annotation...
>
> -Kees
>
> [1]
> https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
Interesting. Will __builtin_get_counted_by(array_annotated->c) give
a null pointer (or an invalid pointer) of the correct type if
array_annotated is a null pointer of an annotated struct type?
I also wonder a bit about the multiple macro evaluations of the arguments
for P and SIZE.
Martin
>
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
> Hi, Martin,
>
> Looks like that there is some issue when I tried to use the _Generic for the
> testing cases, and then I narrowed down to a
> small testing case that shows the problem without any change to GCC.
>
> [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
> struct annotated {
> char b;
> int c[];
> } *array_annotated;
> extern void * counted_by_ref (int *);
>
> int main(int argc, char *argv[])
> {
> typeof(counted_by_ref (array_annotated->c)) ret
> = counted_by_ref (array_annotated->c);
>_Generic (ret, void* : (void)0, default: *ret = 10);
>
> return 0;
> }
> [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
> t1.c: In function ‘main’:
> t1.c:12:44: warning: dereferencing ‘void *’ pointer
>12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> |^~~~
> t1.c:12:49: error: invalid use of void expression
>12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> | ^
I implemented it like this[1] in the Linux kernel. So yours could be:
struct annotated {
char b;
int c[] __attribute__((counted_by(b));
};
extern struct annotated *array_annotated;
int main(int argc, char *argv[])
{
typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
void *: (size_t *)NULL,
default: __builtin_get_counted_by(array_annotated->c)))
ret = __builtin_get_counted_by(array_annotated->c);
if (ret)
*ret = 10;
return 0;
}
It's a bit cumbersome, but it does what's needed.
This is, however, just doing exactly what Bill has suggested: it is
converting the (void *)NULL into (size_t *)NULL when there is no
counted_by annotation...
-Kees
[1]
https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
--
Kees Cook
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Am Montag, dem 26.08.2024 um 19:30 + schrieb Qing Zhao:
> Hi, Martin,
>
> Looks like that there is some issue when I tried to use the _Generic for the
> testing cases, and then I narrowed down to a
> small testing case that shows the problem without any change to GCC.
>
> [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
> struct annotated {
> char b;
> int c[];
> } *array_annotated;
> extern void * counted_by_ref (int *);
>
> int main(int argc, char *argv[])
> {
> typeof(counted_by_ref (array_annotated->c)) ret
> = counted_by_ref (array_annotated->c);
>_Generic (ret, void* : (void)0, default: *ret = 10);
>
> return 0;
> }
> [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
> t1.c: In function ‘main’:
> t1.c:12:44: warning: dereferencing ‘void *’ pointer
>12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> |^~~~
> t1.c:12:49: error: invalid use of void expression
>12 | _Generic (ret, void* : (void)0, default: *ret = 10);
> | ^
>
> Actually, I debugged this issue into gcc’s C routine
> “c_parser_generic_selection”.
> And found that, the “default” branch of the _Generic is always parsed even
> though there is already
> a match in the previous conditions. Therefore, *ret = 10 is parsed even when
> ret is a void *, therefore the compilation error.
>
> So, I am not sure whether this is the correct behavior of the operator
> _Generic?
> Or is there any obvious error in the above small testing case?
> If So, then looks like that we cannot use the _Generic operator for this
> purpose.
>
> Any comments on this?
>
Ah, right. This is indeed the correct behavior for _Generic,
and I have overlooked this. One could work around it like this:
__auto_type ret = counted_by_ref (array_annotated->c);
*_Generic (ret, void*: &(int){ }, default: ret) = 10;
or, if one expects only specific types:
__auto_type ret = counted_by_ref (array_annotated->c);
_Generic (ret, void*: 0, int*: *(int*)ret = 10,
size_t*: *(size_t*)ret = 10);
But yes, a bit less elegant.
Martin
> Thanks a lot for your help.
>
> Qing
>
>
> > On Aug 21, 2024, at 11:43, Martin Uecker wrote:
> >
> > Am Mittwoch, dem 21.08.2024 um 15:24 + schrieb Qing Zhao:
> > > >
> > > > But if we changed it to return a void pointer, we could make this
> > > > a compile-time check:
> > > >
> > > > auto ret = __builtin_get_counted_by(__p->FAM);
> > > >
> > > > _Generic(ret, void*: (void)0, default: *ret = COUNT);
> > >
> > > Is there any benefit to return a void pointer than a SIZE_T pointer for
> > > the NULL pointer?
> >
> > Yes! You can test with _Generic (or __builtin_types_compatible_p)
> > at compile-time based on the type whether you can set *ret to COUNT
> > or not as in the example above.
> >
> > So it is not a weird run-time test which needs to be optimized
> > away.
> >
>
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Thu, Aug 22, 2024 at 8:03 AM Qing Zhao wrote: > > On Aug 21, 2024, at 18:08, Bill Wendling wrote: > >> For the unary operator __counted_by(PTR), “PTR” must have a counted_by > >> attribute, if not, there will be a compilation time error. > >> > >> Then the user could write the following code: > >> > >> If __builtin_has_attriubtes (PTR,counted_by) > >> __counted_by(PTR) = COUNT; > >> > >> > >> From the design point of view, I think this might be the cleanest solution. > >> > >> However, currently, CLANG doesn’t have __builtin_has_attributes. In order > >> to provide a consistent interface, __builtin_get_counted_by(PTR) is fine. > >> > > This was the confusion I had during our meeting today. For the above > > to be a compilation time error, we would have to diagnose it after > > DCE, which is okay, but seems like we're opening ourselves up to > > future issues when DCE misses. Maybe not the biggest concern, but... > > Does the DCE above mean "dead code elimination”? Yes. > If so, I am a little confused: CLANG has dead code elimination pass in the FE? Not that I'm aware of. > Could you explain a little bit here in details to clarify the issues? A small > example will be helpful. Clang's front-end goes through a few phases, of course: parsing, sema, LLVM IR code generation. I'm implementing our version partly in Sema and the rest in LLVM IR generation. During Sema, I check the 'counter's type and adjust the builtin to return a pointer to that type. Future checks determine that the types are compatible. Then IR generation converts the builtins into accesses to the counter. My worry about DCE isn't super high on my list of things to worry about, as it should eliminate a 'if (0) ...' pretty easily, but I don't like relying on future passes to clean up bad code. It's probably me just being too paranoid though, so we don't need to discuss it further. > (In the current GCC’s implementation, I implement this feature completely in > C parser). > -bw
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
On Wed, Aug 21, 2024 at 8:43 AM Martin Uecker wrote: > > Am Mittwoch, dem 21.08.2024 um 15:24 + schrieb Qing Zhao: > > > > > > But if we changed it to return a void pointer, we could make this > > > a compile-time check: > > > > > > auto ret = __builtin_get_counted_by(__p->FAM); > > > > > > _Generic(ret, void*: (void)0, default: *ret = COUNT); > > > > Is there any benefit to return a void pointer than a SIZE_T pointer for > > the NULL pointer? > > Yes! You can test with _Generic (or __builtin_types_compatible_p) > at compile-time based on the type whether you can set *ret to COUNT > or not as in the example above. > > So it is not a weird run-time test which needs to be optimized > away. > Using a '_Generic' moves so much of the work onto the programmer that it would be far easier, and cleaner, for them simply to specify the 'counter' field in the macro and be done with it. Something like: #define alloc(PTR, COUNT, FAM, COUNTER) If the FAM doesn't have a 'counted_by' field: #define alloc(PTR, COUNT, FAM) (It would use VAR_ARGS of course). Why not simply have the compiler automatically adjust the return type? It's perfectly capable of Doing the Right Thing(tm). Otherwise, this builtin becomes even less desirable to use than it currently is. > > > > Yes, I do feel that the approach __builtin_get_counted_by is not very > > > > good. > > > > Maybe it’s better to provide > > > > A. __builtin_set_counted_by > > > > or > > > > B. The unary operator __counted_by(PTR) to return a Lvalue, in this > > > > case, > > > > we need a __builtin_has_attribute first to check whether PTR has the > > > > counted_by attribute first. > > > > > > You could potentially do the same __counted_by and test for type void. > > > > > > _Generic(typeof(__counted_by(PTR)), void: (void)0, __counted_by(PTR) = > > > COUNT); > > > > Oh, so, is there any benefit for the unary operator __counted_by(PTR) than > > the current __builtin_get_counted_by? > > I don't know. You suggested it ;-) > > It probably makes it harder to test the type because you need the > typeof / C2Y Generic combination, but maybe there are other ways > to test. > > > Martin
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Hi, Martin,
Looks like that there is some issue when I tried to use the _Generic for the
testing cases, and then I narrowed down to a
small testing case that shows the problem without any change to GCC.
[opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
struct annotated {
char b;
int c[];
} *array_annotated;
extern void * counted_by_ref (int *);
int main(int argc, char *argv[])
{
typeof(counted_by_ref (array_annotated->c)) ret
= counted_by_ref (array_annotated->c);
_Generic (ret, void* : (void)0, default: *ret = 10);
return 0;
}
[opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
t1.c: In function ‘main’:
t1.c:12:44: warning: dereferencing ‘void *’ pointer
12 | _Generic (ret, void* : (void)0, default: *ret = 10);
|^~~~
t1.c:12:49: error: invalid use of void expression
12 | _Generic (ret, void* : (void)0, default: *ret = 10);
| ^
Actually, I debugged this issue into gcc’s C routine
“c_parser_generic_selection”.
And found that, the “default” branch of the _Generic is always parsed even
though there is already
a match in the previous conditions. Therefore, *ret = 10 is parsed even when
ret is a void *, therefore the compilation error.
So, I am not sure whether this is the correct behavior of the operator
_Generic?
Or is there any obvious error in the above small testing case?
If So, then looks like that we cannot use the _Generic operator for this
purpose.
Any comments on this?
Thanks a lot for your help.
Qing
> On Aug 21, 2024, at 11:43, Martin Uecker wrote:
>
> Am Mittwoch, dem 21.08.2024 um 15:24 + schrieb Qing Zhao:
>>>
>>> But if we changed it to return a void pointer, we could make this
>>> a compile-time check:
>>>
>>> auto ret = __builtin_get_counted_by(__p->FAM);
>>>
>>> _Generic(ret, void*: (void)0, default: *ret = COUNT);
>>
>> Is there any benefit to return a void pointer than a SIZE_T pointer for
>> the NULL pointer?
>
> Yes! You can test with _Generic (or __builtin_types_compatible_p)
> at compile-time based on the type whether you can set *ret to COUNT
> or not as in the example above.
>
> So it is not a weird run-time test which needs to be optimized
> away.
>
Re: [PATCH 1/4] testsuite: Use dg-compile, not gcc -c
> Since this is a pure compile test it makes sense to inform dejagnu of
> it.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.misc-tests/gcov-23.c: Use dg-compile, not gcc -c
OK,
Honza
> ---
> gcc/testsuite/gcc.misc-tests/gcov-23.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/gcc.misc-tests/gcov-23.c
> b/gcc/testsuite/gcc.misc-tests/gcov-23.c
> index 72849d80e3a..72ba0aa1389 100644
> --- a/gcc/testsuite/gcc.misc-tests/gcov-23.c
> +++ b/gcc/testsuite/gcc.misc-tests/gcov-23.c
> @@ -1,4 +1,5 @@
> -/* { dg-options "-fcondition-coverage -ftest-coverage -O2 -c" } */
> +/* { dg-options "-fcondition-coverage -ftest-coverage -O2" } */
> +/* { dg-do compile } */
>
> #include
> #include
> --
> 2.39.2
>
Ping [PATCH 0/4] Prime path coverage in gcc/gcov
Ping. On 8/15/24 10:15, Jørgen Kvalsvik wrote: Ping. Since the last patch I have fixed a few bugs in the path count limit aborting, and a few minor rephrases in docs. Jørgen Kvalsvik (4): testsuite: Use dg-compile, not gcc -c gcov: Cache source files gcov: branch, conds, calls in function summaries Add prime path coverage to gcc/gcov gcc/Makefile.in|6 +- gcc/builtins.cc|2 +- gcc/collect2.cc|5 +- gcc/common.opt | 16 + gcc/doc/gcov.texi | 155 ++ gcc/doc/invoke.texi| 36 + gcc/gcc.cc |4 +- gcc/gcov-counter.def |3 + gcc/gcov-io.h |3 + gcc/gcov.cc| 537 ++- gcc/ipa-inline.cc |2 +- gcc/passes.cc |4 +- gcc/path-coverage.cc | 782 + gcc/prime-paths.cc | 2031 gcc/profile.cc |6 +- gcc/selftest-run-tests.cc |1 + gcc/selftest.h |1 + gcc/testsuite/g++.dg/gcov/gcov-22.C| 170 ++ gcc/testsuite/gcc.misc-tests/gcov-23.c |3 +- gcc/testsuite/gcc.misc-tests/gcov-29.c | 869 ++ gcc/testsuite/gcc.misc-tests/gcov-30.c | 869 ++ gcc/testsuite/gcc.misc-tests/gcov-31.c | 35 + gcc/testsuite/gcc.misc-tests/gcov-32.c | 24 + gcc/testsuite/lib/gcov.exp | 92 +- gcc/tree-profile.cc| 11 +- 25 files changed, 5627 insertions(+), 40 deletions(-) create mode 100644 gcc/path-coverage.cc create mode 100644 gcc/prime-paths.cc create mode 100644 gcc/testsuite/g++.dg/gcov/gcov-22.C create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-29.c create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-30.c create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-31.c create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-32.c
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Aug 21, 2024, at 18:08, Bill Wendling wrote: > >>> >>> to test. >> >> For the unary operator __counted_by(PTR), “PTR” must have a counted_by >> attribute, if not, there will be a compilation time error. >> >> Then the user could write the following code: >> >> If __builtin_has_attriubtes (PTR,counted_by) >> __counted_by(PTR) = COUNT; >> >> >> From the design point of view, I think this might be the cleanest solution. >> >> However, currently, CLANG doesn’t have __builtin_has_attributes. In order >> to provide a consistent interface, __builtin_get_counted_by(PTR) is fine. >> > This was the confusion I had during our meeting today. For the above > to be a compilation time error, we would have to diagnose it after > DCE, which is okay, but seems like we're opening ourselves up to > future issues when DCE misses. Maybe not the biggest concern, but... Does the DCE above mean "dead code elimination”? If so, I am a little confused: CLANG has dead code elimination pass in the FE? Could you explain a little bit here in details to clarify the issues? A small example will be helpful. (In the current GCC’s implementation, I implement this feature completely in C parser). Thanks. Qing > > -bw
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
> On Aug 21, 2024, at 17:54, Bill Wendling wrote:
>
>> if (__builtin_get_counted_by(p->array)) {
>>size_t max_value =
>> type_max(typeof(*__builtin_get_counted_by(p->array)));
>>if (count > type_max)
>>...fail cleanly...
>>*__builtin_get_counted_by(p->array) = count;
>> }
>>
>> I don't strictly need to READ the value (but it seems nice). Currently I can
>> already do a READ with something like this:
>>
>> size_t count = __builtin_dynamic_object_size(p->array, 1) /
>> sizeof(*p->array);
>>
>> But I don't have a way to examine the counter _type_ without
>> __builtin_get_counted_by, so I prefer it over __builtin_set_counted_by.
>>
> Another thing to point out, the documented type signature of
> __builtin_get_counted_by() is more-or-less meaningless. It should be
> adjusted during SEMA to a pointer to the count's type. It's only "void
> *" if the 'counted_by' attribute.
For GCC’s implementation, the returned type of the __builtin_get_counted_by is:
The TYPE of the returned value must be a pointer type pointing to the
corresponding type of the counted-by object or a VOID pointer type in
case of a NULL pointer being returned.
Qing
>
> -bw
Re: [PATCH v1] Provide new GCC builtin __builtin_get_counted_by [PR116016]
Hi, Bill, Thank you for the info. > On Aug 21, 2024, at 17:36, Bill Wendling wrote: > >> >> Bill, could you please provide a little bit more info on the possibility of >> a new builtin __builtin_has_attribute() in CLANG? >> > From what I gathered, it would require some moderate surgery to the > Clang front-end to support "__builtin_has_attribute()". Parsing of > attributes is done only in the context of the "__attribute__" keyword > and would have to be refactored. That in itself would probably be a > nice clean-up to the Clang front-end. The bigger issue is how to > support an attribute as a node in the AST. We don't currently have a > node like that in our AST, because attributes are associated with an > AST node and aren't nodes in and of themselves. So it's a lot more > work than it appears on the surface. > > That said, Clang probably *should* support __builtin_has_attribute(). > The question is whether we should do it for this feature or wait? If > we do it for this feature, I imagine that Clang's implementation of > __builtin_get_counted_by() will be significantly delayed, but if it's > the correct thing to do then we should bite the bullet. >From the discussion so far, I think that __builtin_get_counted_by () might be >good enough. Let me know if there is any objection on this. Qing > > -bw >
Re: RFH: Debugging GCC segfault with LRA-enabled SH backend
On Thu, Aug 22, 2024 at 1:35 PM John Paul Adrian Glaubitz
wrote:
>
> On Thu, 2024-08-22 at 13:05 +0200, Richard Biener wrote:
> > > I'm not sure that bisecting works here as I suspect the issue is a result
> > > of the LRA switch.
> >
> > For sure. Still debugging/fixing the testsuite issue will be much easier.
> >
> > Does a int main(){} also segfault?
>
> I can run the LRA-enabled GCC normally, if you mean that:
>
> (unstable-sh4-sbuild)glaubitz@acrux:/srv/glaubitz/gcc/build$ ./prev-gcc/xgcc
> --version
> xgcc (GCC) 15.0.0 20240818 (experimental)
> Copyright (C) 2024 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
OK, then that compiler also successfully built the stage1 target libraries.
But no, I meant that if you build a int main(){} program and run
_that_, does it segfault?
That is, I suspect there is something broken with compiling memory accesses.
Check the testsuite. Best see what tests pass with LRA disabled and
then enable LRA
and see which tests then fail.
Richard.
> (unstable-sh4-sbuild)glaubitz@acrux:/srv/glaubitz/gcc/build$ ./gcc/xgcc
> --version
> xgcc (GCC) 15.0.0 20240818 (experimental)
> Copyright (C) 2024 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> (unstable-sh4-sbuild)glaubitz@acrux:/srv/glaubitz/gcc/build$
>
> Adrian
>
> --
> .''`. John Paul Adrian Glaubitz
> : :' : Debian Developer
> `. `' Physicist
> `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: RFH: Debugging GCC segfault with LRA-enabled SH backend
On Thu, 2024-08-22 at 13:05 +0200, Richard Biener wrote:
> > I'm not sure that bisecting works here as I suspect the issue is a result
> > of the LRA switch.
>
> For sure. Still debugging/fixing the testsuite issue will be much easier.
>
> Does a int main(){} also segfault?
I can run the LRA-enabled GCC normally, if you mean that:
(unstable-sh4-sbuild)glaubitz@acrux:/srv/glaubitz/gcc/build$ ./prev-gcc/xgcc
--version
xgcc (GCC) 15.0.0 20240818 (experimental)
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
(unstable-sh4-sbuild)glaubitz@acrux:/srv/glaubitz/gcc/build$ ./gcc/xgcc
--version
xgcc (GCC) 15.0.0 20240818 (experimental)
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
(unstable-sh4-sbuild)glaubitz@acrux:/srv/glaubitz/gcc/build$
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: RFH: Debugging GCC segfault with LRA-enabled SH backend
On Thu, Aug 22, 2024 at 12:54 PM John Paul Adrian Glaubitz
wrote:
>
> Hi Richard,
>
> On Thu, 2024-08-22 at 12:49 +0200, Richard Biener wrote:
> > If this is stage2 or stage3 it hints at a miscompile of the stage2/3
> > compiler. I'd concentrate on other
> > issues first and suggest to use --disable-bootstrap to see if that
> > gets you to running the testsuite.
>
> I have actually done that, just forgot to mention it here. The problem
> is that every test is failing so far. I'm testing on real hardware which
> is why it's been running for a few days already.
>
> I'm afraid I might have done something wrong running the tests.
>
> > Otherwise you need to bisect which stage2/3 object was miscompiled and
> > then investigate the nature
> > of the miscompilation. A much more tedious process than addressing
> > remaining testsuite execution
> > FAILs.
>
> I'm not sure that bisecting works here as I suspect the issue is a result
> of the LRA switch.
For sure. Still debugging/fixing the testsuite issue will be much easier.
Does a int main(){} also segfault?
Richard.
> Adrian
>
> --
> .''`. John Paul Adrian Glaubitz
> : :' : Debian Developer
> `. `' Physicist
> `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: RFH: Debugging GCC segfault with LRA-enabled SH backend
Hi Richard, On Thu, 2024-08-22 at 12:49 +0200, Richard Biener wrote: > If this is stage2 or stage3 it hints at a miscompile of the stage2/3 > compiler. I'd concentrate on other > issues first and suggest to use --disable-bootstrap to see if that > gets you to running the testsuite. I have actually done that, just forgot to mention it here. The problem is that every test is failing so far. I'm testing on real hardware which is why it's been running for a few days already. I'm afraid I might have done something wrong running the tests. > Otherwise you need to bisect which stage2/3 object was miscompiled and > then investigate the nature > of the miscompilation. A much more tedious process than addressing > remaining testsuite execution > FAILs. I'm not sure that bisecting works here as I suspect the issue is a result of the LRA switch. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: RFH: Debugging GCC segfault with LRA-enabled SH backend
On Thu, Aug 22, 2024 at 12:32 PM John Paul Adrian Glaubitz wrote: > > (Please CC me in the replies, I am not subscribed to the list) > > Hi, > > I am currently trying to switch the SH backend to use the LRA register > allocater > by default with the help of patches by Oleg and Kaz (CC'ed) to address various > issues when using LRA by default. The patches can all be found in the > corresponding > Bugzilla report [1]. > > Currently, I have applied the following patches from the bug report: > > - 58832 > - 58833 > - 58883 > - 58905 > > plus the following change to enable LRA by default: > > diff --git a/gcc/config/sh/sh.opt b/gcc/config/sh/sh.opt > index c44cfe70cb1..718dfb744ff 100644 > --- a/gcc/config/sh/sh.opt > +++ b/gcc/config/sh/sh.opt > @@ -299,5 +299,5 @@ Target Var(TARGET_FSRRA) > Enable the use of the fsrra instruction. > > mlra > -Target Var(sh_lra_flag) Init(0) Save > +Target Var(sh_lra_flag) Init(1) Save > Use LRA instead of reload (transitional). > > With these changes applied, I have configured and built GCC from Git as > follows: > > # ../configure --disable-multilib --enable-multiarch --enable-bootstrap > --enable-languages=c,c++ > # make -j32 > > which fails on QEMU with a segmentation fault: > > /srv/glaubitz/gcc/build/./gcc/xgcc -B/srv/glaubitz/gcc/build/./gcc/ > -B/usr/local/sh4-unknown-linux-gnu/bin/ \ > -B/usr/local/sh4-unknown-linux-gnu/lib/ -isystem > /usr/local/sh4-unknown-linux-gnu/include -isystem \ > /usr/local/sh4-unknown-linux-gnu/sys-include -fno-checking -g -O2 -O2 -g > -O2 -DIN_GCC \ > -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes > -Wmissing-prototypes \ > -Wold-style-definition -isystem ./include -fpic -DNO_FPSCR_VALUES -w > -Wno-sync-nand -g -DIN_LIBGCC2 \ > -fbuilding-libgcc -fno-stack-protector -fpic -DNO_FPSCR_VALUES -w > -Wno-sync-nand -I. -I. \ > -I../.././gcc -I../../../libgcc -I../../../libgcc/. -I../../../libgcc/../gcc > -I../../../libgcc/../include \ > -DHAVE_CC_TLS -o _paritysi2.o -MT _paritysi2.o -MD -MP -MF _paritysi2.dep > -DL_paritysi2 \ > -c ../../../libgcc/libgcc2.c -fvisibility=hidden -DHIDE_EXPORTS > during GIMPLE pass: waccess > ../../../libgcc/libgcc2.c: In function '__muldi3': > ../../../libgcc/libgcc2.c:538:1: internal compiler error: Segmentation fault > 538 | } > | ^ > during GIMPLE pass: waccess If this is stage2 or stage3 it hints at a miscompile of the stage2/3 compiler. I'd concentrate on other issues first and suggest to use --disable-bootstrap to see if that gets you to running the testsuite. Otherwise you need to bisect which stage2/3 object was miscompiled and then investigate the nature of the miscompilation. A much more tedious process than addressing remaining testsuite execution FAILs. Richard. > This is reproducible on real hardware (Renesas SH-7785LCR, Linux 6.5.0), so > it's not an emulation issue. > > I have tried to debug this issue with my Linux-SH-enabled GDB fork [2] and > got the following backtrace: > > (gdb) bt > #0 0x0109fee4 in wi::add_large(long long*, long long const*, unsigned int, > long long const*, unsigned int, unsigned int, signop, wi::overflow_type*) () > #1 0x00bdbc10 in > access_ref::add_offset(generic_wide_int > const&, > generic_wide_int > const&) () > #2 0x00bdd0e8 in compute_objsize_r(tree_node*, gimple*, bool, int, > access_ref*, ssa_name_limit_t&, pointer_query*) () > #3 0x in ?? () > (gdb) display/i $pc > 1: x/i $pc > => 0x109fee4 <_ZN2wi9add_largeEPxPKxjS2_jj6signopPNS_13overflow_typeE+84>: > mov.l @r2,r3 > (gdb) x/wx $r2 > 0x7c07eaa0: Cannot access memory at address 0x7c07eaa0 > (gdb) > > I have also tried disabling late combine by SH by default, but that didn't > help: > > diff --git a/gcc/config/sh/sh.cc b/gcc/config/sh/sh.cc > index 280588268ae..dca27893536 100644 > --- a/gcc/config/sh/sh.cc > +++ b/gcc/config/sh/sh.cc > @@ -1047,6 +1047,9 @@ sh_override_options_after_change (void) > str_align_functions = r; > } > } > + > +if (!OPTION_SET_P (flag_late_combine_instructions)) > + flag_late_combine_instructions = 0; > } > ^L > /* Print the operand address in x to the stream. */ > > Thus, I have now run out of ideas (as I'm not really a compiler expert). > > Does anyone else have any other suggestions? > > Thanks, > Adrian > > PS: Would be great if we could upstream Linux SH native GDB support from [2] > as well, in case any binutils-gdb maintainer is reading here. > > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212 > > [2] https://github.com/glaubitz/binutils-gdb/tree/linux-sh > > -- > .''`. John Paul Adrian Glaubitz > : :' : Debian Developer > `. `' Physicist > `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
