Message ID | 87jzq3az8b.fsf@oracle.com |
---|---|
State | New |
Headers | show |
Series | [v2] Fixed problem with BTF defining smaller enums. | expand |
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; > +} >
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; >> +} >>
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; +}