* [PATCH] Fixed problem with BTF defining smaller enums.
@ 2023-11-13 22:37 Cupertino Miranda
2023-11-14 16:28 ` David Faust
0 siblings, 1 reply; 5+ messages in thread
From: Cupertino Miranda @ 2023-11-13 22:37 UTC (permalink / raw)
To: gcc-patches; +Cc: jose.marchesi, elena.zannoni, david.faust, Cupertino Miranda
This patch fixes a BTF, which would become invalid when having
smaller then 4 byte definitions of enums.
For example, when using the __attribute__((mode(byte))) in the enum
definition.
Two problems were identified:
- it would incorrectly create an entry for enum64 when the size of the
enum was different then 4.
- it would allocate less then 4 bytes for the value entry in BTF, in
case the type was smaller.
BTF generated was validated against clang.
gcc/ChangeLog:
* bpfout.cc (btf_calc_num_vbytes): Fixed logic for enum64.
(btf_asm_enum_const): Corrected logic for enum64 and smaller
than 4 bytes values.
---
gcc/btfout.cc | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index e07fed302c24..d2263ec6eec3 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -299,7 +299,7 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
break;
case BTF_KIND_ENUM:
- vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
+ vlen_bytes += (dtd->dtd_data.ctti_size > 4)
? vlen * sizeof (struct btf_enum64)
: vlen * sizeof (struct btf_enum);
break;
@@ -914,13 +914,13 @@ btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd, unsigned int idx)
{
dw2_asm_output_data (4, dmd->dmd_name_offset, "ENUM_CONST '%s' idx=%u",
dmd->dmd_name, idx);
- if (size == 4)
- dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
- else
+ if (size > 4)
{
- dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
+ dw2_asm_output_data (4, dmd->dmd_value & 0xfffffffe, "bte_value_lo32");
dw2_asm_output_data (4, (dmd->dmd_value >> 32) & 0xffffffff, "bte_value_hi32");
}
+ else
+ dw2_asm_output_data (size < 4 ? 4 : size, dmd->dmd_value, "bte_value");
}
/* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO. */
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fixed problem with BTF defining smaller enums.
2023-11-13 22:37 [PATCH] Fixed problem with BTF defining smaller enums Cupertino Miranda
@ 2023-11-14 16:28 ` David Faust
2023-11-27 17:21 ` [PATCH v2] " Cupertino Miranda
0 siblings, 1 reply; 5+ messages in thread
From: David Faust @ 2023-11-14 16:28 UTC (permalink / raw)
To: Cupertino Miranda; +Cc: gcc-patches, jose.marchesi, elena.zannoni
Hi Cupertino,
A couple of comments inline.
On 11/13/23 14:37, Cupertino Miranda wrote:
> This patch fixes a BTF, which would become invalid when having
> smaller then 4 byte definitions of enums.
> For example, when using the __attribute__((mode(byte))) in the enum
> definition.
Please add a test for this case in the BTF testsuite.
>
> Two problems were identified:
> - it would incorrectly create an entry for enum64 when the size of the
> enum was different then 4.
> - it would allocate less then 4 bytes for the value entry in BTF, in
> case the type was smaller.
>
> BTF generated was validated against clang.
>
> gcc/ChangeLog:
> * bpfout.cc (btf_calc_num_vbytes): Fixed logic for enum64.
> (btf_asm_enum_const): Corrected logic for enum64 and smaller
> than 4 bytes values.
> ---
> gcc/btfout.cc | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index e07fed302c24..d2263ec6eec3 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -299,7 +299,7 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
> break;
>
> case BTF_KIND_ENUM:
> - vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
> + vlen_bytes += (dtd->dtd_data.ctti_size > 4)
> ? vlen * sizeof (struct btf_enum64)
> : vlen * sizeof (struct btf_enum);
> break;
> @@ -914,13 +914,13 @@ btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd, unsigned int idx)
> {
> dw2_asm_output_data (4, dmd->dmd_name_offset, "ENUM_CONST '%s' idx=%u",
> dmd->dmd_name, idx);
> - if (size == 4)
> - dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
> - else
> + if (size > 4)
> {
> - dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
> + dw2_asm_output_data (4, dmd->dmd_value & 0xfffffffe, "bte_value_lo32");
I don't understand the mask change here.
Why clear the low bit of the enum value?
> dw2_asm_output_data (4, (dmd->dmd_value >> 32) & 0xffffffff, "bte_value_hi32");
> }
> + else
> + dw2_asm_output_data (size < 4 ? 4 : size, dmd->dmd_value, "bte_value");
In the else case isn't size guaranteed <= 4?
In which case, 'size < 4 ? 4 : size' always evaluates to 4.
So I would suggest to just write a literal 4 to keep it simple.
> }
>
> /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO. */
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] Fixed problem with BTF defining smaller enums.
2023-11-14 16:28 ` David Faust
@ 2023-11-27 17:21 ` Cupertino Miranda
2023-11-27 19:01 ` David Faust
0 siblings, 1 reply; 5+ messages in thread
From: Cupertino Miranda @ 2023-11-27 17:21 UTC (permalink / raw)
To: David Faust; +Cc: gcc-patches, jose.marchesi, elena.zannoni
[-- Attachment #1: Type: text/plain, Size: 274 bytes --]
Hi everyone,
David: Thanks for the v1 review.
This version adds the following;
- test case,
- improves condition logic,
- fixes mask typo.
Looking forward to your review.
v1 at: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636391.html
Cheers,
Cupertino
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-Fixed-problem-with-BTF-defining-smaller-enums.patch --]
[-- Type: text/x-diff, Size: 3092 bytes --]
commit 3f89d352a4ee90882089142d743f8a748013b5fe
Author: Cupertino Miranda <cupertino.miranda@oracle.com>
Date: Fri Nov 10 14:02:30 2023 +0000
Fixed problem with BTF defining smaller enums.
This patch fixes a BTF, which would become invalid when having
smaller then 4 byte definitions of enums.
For example, when using the __attribute__((mode(byte))) in the enum
definition.
Two problems were identified:
- it would incorrectly create an entry for enum64 when the size of the
enum was different then 4.
- it would allocate less then 4 bytes for the value entry in BTF, in
case the type was smaller.
BTF generated was validated against clang.
gcc/ChangeLog:
* bpfout.cc (btf_calc_num_vbytes): Fixed logic for enum64.
(btf_asm_enum_const): Corrected logic for enum64 and smaller
than 4 bytes values.
gcc/testsuite/ChangeLog:
gcc.dg/debug/btf/btf-enum-small.c: Added test.
diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index e07fed302c24..5f2e99ce4725 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -299,7 +299,7 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
break;
case BTF_KIND_ENUM:
- vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
+ vlen_bytes += (dtd->dtd_data.ctti_size > 4)
? vlen * sizeof (struct btf_enum64)
: vlen * sizeof (struct btf_enum);
break;
@@ -914,8 +914,8 @@ btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd, unsigned int idx)
{
dw2_asm_output_data (4, dmd->dmd_name_offset, "ENUM_CONST '%s' idx=%u",
dmd->dmd_name, idx);
- if (size == 4)
- dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
+ if (size <= 4)
+ dw2_asm_output_data (size < 4 ? 4 : size, dmd->dmd_value, "bte_value");
else
{
dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c
new file mode 100644
index 000000000000..eb8a1bd2c438
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c
@@ -0,0 +1,28 @@
+/* Test BTF generation for small enums. */
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -gbtf -dA" } */
+
+/* { dg-final { scan-assembler-not "bte_value_lo32" } } */
+/* { dg-final { scan-assembler-not "bte_value_hi32" } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x6000002\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times " ENUM_CONST 'eSMALL' idx=0" 1 } } */
+/* { dg-final { scan-assembler-times " ENUM_CONST 'eSMALLY' idx=1" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"eSMALL.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"eSMALLY.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "bte_value" 2 } } */
+
+enum smalled_enum
+{
+ eSMALL,
+ eSMALLY,
+} __attribute__((mode(byte)));
+
+struct root_struct {
+ enum smalled_enum esmall;
+};
+
+enum smalled_enum
+foo(struct root_struct *root) {
+ return root->esmall;
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Fixed problem with BTF defining smaller enums.
2023-11-27 17:21 ` [PATCH v2] " Cupertino Miranda
@ 2023-11-27 19:01 ` David Faust
2023-11-28 12:57 ` Cupertino Miranda
0 siblings, 1 reply; 5+ messages in thread
From: David Faust @ 2023-11-27 19:01 UTC (permalink / raw)
To: Cupertino Miranda; +Cc: gcc-patches, jose.marchesi, elena.zannoni
Hi Cupertino,
On 11/27/23 09:21, Cupertino Miranda wrote:
> Hi everyone,
>
> David: Thanks for the v1 review.
>
> This version adds the following;
> - test case,
> - improves condition logic,
> - fixes mask typo.
>
> Looking forward to your review.
v2 LGTM, please apply.
Thanks!
>
> v1 at: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636391.html
>
> Cheers,
> Cupertino
>
>
> 0004-Fixed-problem-with-BTF-defining-smaller-enums.patch
>
> commit 3f89d352a4ee90882089142d743f8a748013b5fe
> Author: Cupertino Miranda <cupertino.miranda@oracle.com>
> Date: Fri Nov 10 14:02:30 2023 +0000
>
> Fixed problem with BTF defining smaller enums.
>
> This patch fixes a BTF, which would become invalid when having
> smaller then 4 byte definitions of enums.
> For example, when using the __attribute__((mode(byte))) in the enum
> definition.
>
> Two problems were identified:
> - it would incorrectly create an entry for enum64 when the size of the
> enum was different then 4.
> - it would allocate less then 4 bytes for the value entry in BTF, in
> case the type was smaller.
>
> BTF generated was validated against clang.
>
> gcc/ChangeLog:
> * bpfout.cc (btf_calc_num_vbytes): Fixed logic for enum64.
> (btf_asm_enum_const): Corrected logic for enum64 and smaller
> than 4 bytes values.
>
> gcc/testsuite/ChangeLog:
> gcc.dg/debug/btf/btf-enum-small.c: Added test.
>
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index e07fed302c24..5f2e99ce4725 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -299,7 +299,7 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
> break;
>
> case BTF_KIND_ENUM:
> - vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
> + vlen_bytes += (dtd->dtd_data.ctti_size > 4)
> ? vlen * sizeof (struct btf_enum64)
> : vlen * sizeof (struct btf_enum);
> break;
> @@ -914,8 +914,8 @@ btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd, unsigned int idx)
> {
> dw2_asm_output_data (4, dmd->dmd_name_offset, "ENUM_CONST '%s' idx=%u",
> dmd->dmd_name, idx);
> - if (size == 4)
> - dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
> + if (size <= 4)
> + dw2_asm_output_data (size < 4 ? 4 : size, dmd->dmd_value, "bte_value");
> else
> {
> dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c
> new file mode 100644
> index 000000000000..eb8a1bd2c438
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c
> @@ -0,0 +1,28 @@
> +/* Test BTF generation for small enums. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -gbtf -dA" } */
> +
> +/* { dg-final { scan-assembler-not "bte_value_lo32" } } */
> +/* { dg-final { scan-assembler-not "bte_value_hi32" } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x6000002\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times " ENUM_CONST 'eSMALL' idx=0" 1 } } */
> +/* { dg-final { scan-assembler-times " ENUM_CONST 'eSMALLY' idx=1" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"eSMALL.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"eSMALLY.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "bte_value" 2 } } */
> +
> +enum smalled_enum
> +{
> + eSMALL,
> + eSMALLY,
> +} __attribute__((mode(byte)));
> +
> +struct root_struct {
> + enum smalled_enum esmall;
> +};
> +
> +enum smalled_enum
> +foo(struct root_struct *root) {
> + return root->esmall;
> +}
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Fixed problem with BTF defining smaller enums.
2023-11-27 19:01 ` David Faust
@ 2023-11-28 12:57 ` Cupertino Miranda
0 siblings, 0 replies; 5+ messages in thread
From: Cupertino Miranda @ 2023-11-28 12:57 UTC (permalink / raw)
To: David Faust; +Cc: gcc-patches, jose.marchesi, elena.zannoni
Thanks! Committed!
David Faust writes:
> Hi Cupertino,
>
> On 11/27/23 09:21, Cupertino Miranda wrote:
>> Hi everyone,
>>
>> David: Thanks for the v1 review.
>>
>> This version adds the following;
>> - test case,
>> - improves condition logic,
>> - fixes mask typo.
>>
>> Looking forward to your review.
>
> v2 LGTM, please apply.
> Thanks!
>
>>
>> v1 at: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636391.html
>>
>> Cheers,
>> Cupertino
>>
>>
>> 0004-Fixed-problem-with-BTF-defining-smaller-enums.patch
>>
>> commit 3f89d352a4ee90882089142d743f8a748013b5fe
>> Author: Cupertino Miranda <cupertino.miranda@oracle.com>
>> Date: Fri Nov 10 14:02:30 2023 +0000
>>
>> Fixed problem with BTF defining smaller enums.
>>
>> This patch fixes a BTF, which would become invalid when having
>> smaller then 4 byte definitions of enums.
>> For example, when using the __attribute__((mode(byte))) in the enum
>> definition.
>>
>> Two problems were identified:
>> - it would incorrectly create an entry for enum64 when the size of the
>> enum was different then 4.
>> - it would allocate less then 4 bytes for the value entry in BTF, in
>> case the type was smaller.
>>
>> BTF generated was validated against clang.
>>
>> gcc/ChangeLog:
>> * bpfout.cc (btf_calc_num_vbytes): Fixed logic for enum64.
>> (btf_asm_enum_const): Corrected logic for enum64 and smaller
>> than 4 bytes values.
>>
>> gcc/testsuite/ChangeLog:
>> gcc.dg/debug/btf/btf-enum-small.c: Added test.
>>
>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>> index e07fed302c24..5f2e99ce4725 100644
>> --- a/gcc/btfout.cc
>> +++ b/gcc/btfout.cc
>> @@ -299,7 +299,7 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>> break;
>>
>> case BTF_KIND_ENUM:
>> - vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
>> + vlen_bytes += (dtd->dtd_data.ctti_size > 4)
>> ? vlen * sizeof (struct btf_enum64)
>> : vlen * sizeof (struct btf_enum);
>> break;
>> @@ -914,8 +914,8 @@ btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd, unsigned int idx)
>> {
>> dw2_asm_output_data (4, dmd->dmd_name_offset, "ENUM_CONST '%s' idx=%u",
>> dmd->dmd_name, idx);
>> - if (size == 4)
>> - dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
>> + if (size <= 4)
>> + dw2_asm_output_data (size < 4 ? 4 : size, dmd->dmd_value, "bte_value");
>> else
>> {
>> dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c
>> new file mode 100644
>> index 000000000000..eb8a1bd2c438
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-small.c
>> @@ -0,0 +1,28 @@
>> +/* Test BTF generation for small enums. */
>> +
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -gbtf -dA" } */
>> +
>> +/* { dg-final { scan-assembler-not "bte_value_lo32" } } */
>> +/* { dg-final { scan-assembler-not "bte_value_hi32" } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0x6000002\[\t \]+\[^\n\]*btt_info" 1 } } */
>> +/* { dg-final { scan-assembler-times " ENUM_CONST 'eSMALL' idx=0" 1 } } */
>> +/* { dg-final { scan-assembler-times " ENUM_CONST 'eSMALLY' idx=1" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"eSMALL.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"eSMALLY.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "bte_value" 2 } } */
>> +
>> +enum smalled_enum
>> +{
>> + eSMALL,
>> + eSMALLY,
>> +} __attribute__((mode(byte)));
>> +
>> +struct root_struct {
>> + enum smalled_enum esmall;
>> +};
>> +
>> +enum smalled_enum
>> +foo(struct root_struct *root) {
>> + return root->esmall;
>> +}
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-28 12:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-13 22:37 [PATCH] Fixed problem with BTF defining smaller enums Cupertino Miranda
2023-11-14 16:28 ` David Faust
2023-11-27 17:21 ` [PATCH v2] " Cupertino Miranda
2023-11-27 19:01 ` David Faust
2023-11-28 12:57 ` Cupertino Miranda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).