* V4 [PATCH 0/3] Switch to a new section if the SECTION_RETAIN bit doesn't match @ 2020-12-15 17:30 H.J. Lu 2020-12-15 17:30 ` V4 [PATCH 1/3] " H.J. Lu ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: H.J. Lu @ 2020-12-15 17:30 UTC (permalink / raw) To: gcc-patches When SECTION_RETAIN is used, definitions marked with used attribute and unmarked definitions are placed in a section with the same name. Instead of issue an error: [hjl@gnu-cfl-2 gcc]$ /usr/gcc-11.0.0-x32/bin/gcc -S c.c -fdiagnostics-plain-output c.c:2:49: error: ‘foo1’ causes a section type conflict with ‘foo2’ c.c:1:54: note: ‘foo2’ was declared here [hjl@gnu-cfl-2 gcc]$ the first patch switches to a new section if the SECTION_RETAIN bit doesn't match. The second optional patch issues a warning: [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S c.c c.c:2:49: warning: ‘foo1’ without ‘used’ attribute and ‘foo2’ with ‘used’ attribute are placed in a section with the same name [-Wattributes] 2 | const int __attribute__((section(".data.foo"))) foo1 = 1; | ^~~~ c.c:1:54: note: ‘foo2’ was declared here 1 | const int __attribute__((used,section(".data.foo"))) foo2 = 2; | [hjl@gnu-cfl-2 gcc]$ Changes from V3: 1. Add DECL_P (decl) to switch_to_section (). 2. Update comments for get_section (). 3. Require .init_array/.fini_array support for SHF_GNU_RETAIN. Changes from V2: 1. Add (new_section->common.flags & SECTION_NAMED) check since SHF_GNU_RETAIN section must be named. 2. Move c-c++-common/attr-used-9.c to the fist patch since there are no new warnings. 3. Check new warnings only for R_flag_in_section target. H.J. Lu (3): Switch to a new section if the SECTION_RETAIN bit doesn't match Warn used and not used symbols in section with the same name Require .init_array/.fini_array support for SHF_GNU_RETAIN gcc/defaults.h | 11 +++++ gcc/output.h | 2 +- gcc/testsuite/c-c++-common/attr-used-5.c | 27 ++++++++++++ gcc/testsuite/c-c++-common/attr-used-6.c | 27 ++++++++++++ gcc/testsuite/c-c++-common/attr-used-7.c | 9 ++++ gcc/testsuite/c-c++-common/attr-used-8.c | 9 ++++ gcc/testsuite/c-c++-common/attr-used-9.c | 28 ++++++++++++ gcc/testsuite/lib/target-supports.exp | 2 +- gcc/varasm.c | 56 ++++++++++++++++++++---- 9 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/attr-used-5.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-6.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-7.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-8.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-9.c -- 2.29.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* V4 [PATCH 1/3] Switch to a new section if the SECTION_RETAIN bit doesn't match 2020-12-15 17:30 V4 [PATCH 0/3] Switch to a new section if the SECTION_RETAIN bit doesn't match H.J. Lu @ 2020-12-15 17:30 ` H.J. Lu 2020-12-16 0:44 ` Jeff Law 2020-12-15 17:30 ` V4 [PATCH 2/3] Warn used and not used symbols in section with the same name H.J. Lu 2020-12-15 17:30 ` V4 [PATCH 3/3] Require .init_array/.fini_array support for SHF_GNU_RETAIN H.J. Lu 2 siblings, 1 reply; 8+ messages in thread From: H.J. Lu @ 2020-12-15 17:30 UTC (permalink / raw) To: gcc-patches When definitions marked with used attribute and unmarked definitions are placed in the section with the same name, switch to a new section if the SECTION_RETAIN bit doesn't match. gcc/ PR target/98146 * output.h (switch_to_section): Add a tree argument, default to nullptr. * varasm.c (get_section): If the SECTION_RETAIN bit doesn't match, return and switch to a new section later. (assemble_start_function): Pass decl to switch_to_section. (assemble_variable): Likewise. (switch_to_section): If the SECTION_RETAIN bit doesn't match, switch to a new section. gcc/testsuite/ PR target/98146 * c-c++-common/attr-used-5.c: New test. * c-c++-common/attr-used-6.c: Likewise. * c-c++-common/attr-used-7.c: Likewise. * c-c++-common/attr-used-8.c: Likewise. * c-c++-common/attr-used-9.c: Likewise. --- gcc/output.h | 2 +- gcc/testsuite/c-c++-common/attr-used-5.c | 26 ++++++++++++++++++ gcc/testsuite/c-c++-common/attr-used-6.c | 26 ++++++++++++++++++ gcc/testsuite/c-c++-common/attr-used-7.c | 8 ++++++ gcc/testsuite/c-c++-common/attr-used-8.c | 8 ++++++ gcc/testsuite/c-c++-common/attr-used-9.c | 28 +++++++++++++++++++ gcc/varasm.c | 34 ++++++++++++++++++++---- 7 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/attr-used-5.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-6.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-7.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-8.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-9.c diff --git a/gcc/output.h b/gcc/output.h index fa8ace1f394..1f9af46da1d 100644 --- a/gcc/output.h +++ b/gcc/output.h @@ -548,7 +548,7 @@ extern void switch_to_other_text_partition (void); extern section *get_cdtor_priority_section (int, bool); extern bool unlikely_text_section_p (section *); -extern void switch_to_section (section *); +extern void switch_to_section (section *, tree = nullptr); extern void output_section_asm_op (const void *); extern void record_tm_clone_pair (tree, tree); diff --git a/gcc/testsuite/c-c++-common/attr-used-5.c b/gcc/testsuite/c-c++-common/attr-used-5.c new file mode 100644 index 00000000000..9fc0d3834e9 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-5.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +struct dtv_slotinfo_list +{ + struct dtv_slotinfo_list *next; +}; + +extern struct dtv_slotinfo_list *list; + +static int __attribute__ ((section ("__libc_freeres_fn"))) +free_slotinfo (struct dtv_slotinfo_list **elemp) +{ + if (!free_slotinfo (&(*elemp)->next)) + return 0; + return 1; +} + +__attribute__ ((used, section ("__libc_freeres_fn"))) +static void free_mem (void) +{ + free_slotinfo (&list); +} + +/* { dg-final { scan-assembler "__libc_freeres_fn,\"ax\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler "__libc_freeres_fn,\"axR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-6.c b/gcc/testsuite/c-c++-common/attr-used-6.c new file mode 100644 index 00000000000..0cb82ade5a9 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-6.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +struct dtv_slotinfo_list +{ + struct dtv_slotinfo_list *next; +}; + +extern struct dtv_slotinfo_list *list; + +static int __attribute__ ((used, section ("__libc_freeres_fn"))) +free_slotinfo (struct dtv_slotinfo_list **elemp) +{ + if (!free_slotinfo (&(*elemp)->next)) + return 0; + return 1; +} + +__attribute__ ((section ("__libc_freeres_fn"))) +void free_mem (void) +{ + free_slotinfo (&list); +} + +/* { dg-final { scan-assembler "__libc_freeres_fn,\"ax\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler "__libc_freeres_fn,\"axR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-7.c b/gcc/testsuite/c-c++-common/attr-used-7.c new file mode 100644 index 00000000000..fba2706ffc1 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-7.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +int __attribute__((used,section(".data.foo"))) foo2 = 2; +int __attribute__((section(".data.foo"))) foo1 = 1; + +/* { dg-final { scan-assembler ".data.foo,\"aw\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler ".data.foo,\"awR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-8.c b/gcc/testsuite/c-c++-common/attr-used-8.c new file mode 100644 index 00000000000..4da4aabe573 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-8.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +int __attribute__((section(".data.foo"))) foo1 = 1; +int __attribute__((used,section(".data.foo"))) foo2 = 2; + +/* { dg-final { scan-assembler ".data.foo,\"aw\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler ".data.foo,\"awR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-9.c b/gcc/testsuite/c-c++-common/attr-used-9.c new file mode 100644 index 00000000000..cf3bde67622 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-9.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +struct dtv_slotinfo_list +{ + struct dtv_slotinfo_list *next; +}; + +extern struct dtv_slotinfo_list *list; + +static int __attribute__ ((used, section ("__libc_freeres_fn"))) +free_slotinfo (struct dtv_slotinfo_list **elemp) +{ + if (!free_slotinfo (&(*elemp)->next)) + return 0; + return 1; +} + +__attribute__ ((section ("__libc_freeres_fn"))) +static void free_mem (void) +/* { dg-warning "defined but not used" "" { target *-*-* } .-1 } */ +{ + free_slotinfo (&list); +} + +/* { dg-final { scan-assembler-not "__libc_freeres_fn\n" } } */ +/* { dg-final { scan-assembler-not "__libc_freeres_fn,\"ax\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler "__libc_freeres_fn,\"axR\"" { target R_flag_in_section } } } */ diff --git a/gcc/varasm.c b/gcc/varasm.c index c5487a78b13..e3e49d53e5d 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -281,7 +281,8 @@ get_noswitch_section (unsigned int flags, noswitch_section_callback callback) /* Return the named section structure associated with NAME. Create a new section with the given fields if no such structure exists. - When NOT_EXISTING, then fail if the section already exists. */ + When NOT_EXISTING, then fail if the section already exists. Return + the existing section if the SECTION_RETAIN bit doesn't match. */ section * get_section (const char *name, unsigned int flags, tree decl, @@ -343,6 +344,11 @@ get_section (const char *name, unsigned int flags, tree decl, sect->common.flags |= (SECTION_WRITE | SECTION_RELRO); return sect; } + /* If the SECTION_RETAIN bit doesn't match, return and switch + to a new section later. */ + if ((sect->common.flags & SECTION_RETAIN) + != (flags & SECTION_RETAIN)) + return sect; /* Sanity check user variables for flag changes. */ if (sect->named.decl != NULL && DECL_P (sect->named.decl) @@ -1879,7 +1885,7 @@ assemble_start_function (tree decl, const char *fnname) /* Switch to the correct text section for the start of the function. */ - switch_to_section (function_section (decl)); + switch_to_section (function_section (decl), decl); if (crtl->has_bb_partition && !hot_label_written) ASM_OUTPUT_LABEL (asm_out_file, crtl->subsections.hot_section_label); @@ -2375,7 +2381,7 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED, && (strcmp (sect->named.name, ".vtable_map_vars") == 0)) handle_vtv_comdat_section (sect, decl); else - switch_to_section (sect); + switch_to_section (sect, decl); if (align > BITS_PER_UNIT) ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (align / BITS_PER_UNIT)); assemble_variable_contents (decl, name, dont_output_data, @@ -7742,10 +7748,28 @@ output_section_asm_op (const void *directive) the current section is NEW_SECTION. */ void -switch_to_section (section *new_section) +switch_to_section (section *new_section, tree decl) { if (in_section == new_section) - return; + { + if (HAVE_GAS_SHF_GNU_RETAIN + && (new_section->common.flags & SECTION_NAMED) + && decl != nullptr + && DECL_P (decl) + && (!!DECL_PRESERVE_P (decl) + != !!(new_section->common.flags & SECTION_RETAIN))) + { + /* If the SECTION_RETAIN bit doesn't match, switch to a new + section. */ + if (DECL_PRESERVE_P (decl)) + new_section->common.flags |= SECTION_RETAIN; + else + new_section->common.flags &= ~(SECTION_RETAIN + | SECTION_DECLARED); + } + else + return; + } if (new_section->common.flags & SECTION_FORGET) in_section = NULL; -- 2.29.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: V4 [PATCH 1/3] Switch to a new section if the SECTION_RETAIN bit doesn't match 2020-12-15 17:30 ` V4 [PATCH 1/3] " H.J. Lu @ 2020-12-16 0:44 ` Jeff Law 2020-12-16 13:01 ` H.J. Lu 0 siblings, 1 reply; 8+ messages in thread From: Jeff Law @ 2020-12-16 0:44 UTC (permalink / raw) To: H.J. Lu, gcc-patches On 12/15/20 10:30 AM, H.J. Lu wrote: > When definitions marked with used attribute and unmarked definitions are > placed in the section with the same name, switch to a new section if the > SECTION_RETAIN bit doesn't match. > > gcc/ > > PR target/98146 > * output.h (switch_to_section): Add a tree argument, default to > nullptr. > * varasm.c (get_section): If the SECTION_RETAIN bit doesn't match, > return and switch to a new section later. > (assemble_start_function): Pass decl to switch_to_section. > (assemble_variable): Likewise. > (switch_to_section): If the SECTION_RETAIN bit doesn't match, > switch to a new section. > > gcc/testsuite/ > > PR target/98146 > * c-c++-common/attr-used-5.c: New test. > * c-c++-common/attr-used-6.c: Likewise. > * c-c++-common/attr-used-7.c: Likewise. > * c-c++-common/attr-used-8.c: Likewise. > * c-c++-common/attr-used-9.c: Likewise. So as an additional follow-up could you add to the get_section comment the special handling were add SECTION_WRITE and SECTION_RELRO to the flags. That's not something I would expect that function to do either. Consider that comment addition pre-approved. Jeff ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: V4 [PATCH 1/3] Switch to a new section if the SECTION_RETAIN bit doesn't match 2020-12-16 0:44 ` Jeff Law @ 2020-12-16 13:01 ` H.J. Lu 0 siblings, 0 replies; 8+ messages in thread From: H.J. Lu @ 2020-12-16 13:01 UTC (permalink / raw) To: Jeff Law; +Cc: GCC Patches, Jozef Lawrynowicz [-- Attachment #1: Type: text/plain, Size: 2345 bytes --] On Tue, Dec 15, 2020 at 4:44 PM Jeff Law <law@redhat.com> wrote: > > > > On 12/15/20 10:30 AM, H.J. Lu wrote: > > When definitions marked with used attribute and unmarked definitions are > > placed in the section with the same name, switch to a new section if the > > SECTION_RETAIN bit doesn't match. > > > > gcc/ > > > > PR target/98146 > > * output.h (switch_to_section): Add a tree argument, default to > > nullptr. > > * varasm.c (get_section): If the SECTION_RETAIN bit doesn't match, > > return and switch to a new section later. > > (assemble_start_function): Pass decl to switch_to_section. > > (assemble_variable): Likewise. > > (switch_to_section): If the SECTION_RETAIN bit doesn't match, > > switch to a new section. > > > > gcc/testsuite/ > > > > PR target/98146 > > * c-c++-common/attr-used-5.c: New test. > > * c-c++-common/attr-used-6.c: Likewise. > > * c-c++-common/attr-used-7.c: Likewise. > > * c-c++-common/attr-used-8.c: Likewise. > > * c-c++-common/attr-used-9.c: Likewise. > So as an additional follow-up could you add to the get_section comment > the special handling were add SECTION_WRITE and SECTION_RELRO to the > flags. That's not something I would expect that function to do either. > Consider that comment addition pre-approved. > I am checking the patch with this change: diff --git a/gcc/varasm.c b/gcc/varasm.c index 46f79ea28e6..267a052331d 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -282,7 +282,11 @@ get_noswitch_section (unsigned int flags, noswitch_section_callback callback) /* Return the named section structure associated with NAME. Create a new section with the given fields if no such structure exists. When NOT_EXISTING, then fail if the section already exists. Return - the existing section if the SECTION_RETAIN bit doesn't match. */ + the existing section if the SECTION_RETAIN bit doesn't match. Set + the SECTION_WRITE | SECTION_RELRO bits on the the existing section + if one of the section flags is SECTION_WRITE | SECTION_RELRO and the + other has none of these flags in named sections and either the section + hasn't been declared yet or has been declared as writable. */ section * get_section (const char *name, unsigned int flags, tree decl, Thanks. -- H.J. [-- Attachment #2: 0001-Switch-to-a-new-section-if-the-SECTION_RETAIN-bit-do.patch --] [-- Type: text/x-patch, Size: 9429 bytes --] From 22c1d0be1399becffac58d7509ab30a8ecd97a9a Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Thu, 3 Dec 2020 11:01:06 -0800 Subject: [PATCH] Switch to a new section if the SECTION_RETAIN bit doesn't match When definitions marked with used attribute and unmarked definitions are placed in the section with the same name, switch to a new section if the SECTION_RETAIN bit doesn't match. gcc/ PR target/98146 * output.h (switch_to_section): Add a tree argument, default to nullptr. * varasm.c (get_section): If the SECTION_RETAIN bit doesn't match, return and switch to a new section later. (assemble_start_function): Pass decl to switch_to_section. (assemble_variable): Likewise. (switch_to_section): If the SECTION_RETAIN bit doesn't match, switch to a new section. gcc/testsuite/ PR target/98146 * c-c++-common/attr-used-5.c: New test. * c-c++-common/attr-used-6.c: Likewise. * c-c++-common/attr-used-7.c: Likewise. * c-c++-common/attr-used-8.c: Likewise. * c-c++-common/attr-used-9.c: Likewise. --- gcc/output.h | 2 +- gcc/testsuite/c-c++-common/attr-used-5.c | 26 ++++++++++++++++ gcc/testsuite/c-c++-common/attr-used-6.c | 26 ++++++++++++++++ gcc/testsuite/c-c++-common/attr-used-7.c | 8 +++++ gcc/testsuite/c-c++-common/attr-used-8.c | 8 +++++ gcc/testsuite/c-c++-common/attr-used-9.c | 28 +++++++++++++++++ gcc/varasm.c | 38 ++++++++++++++++++++---- 7 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/attr-used-5.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-6.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-7.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-8.c create mode 100644 gcc/testsuite/c-c++-common/attr-used-9.c diff --git a/gcc/output.h b/gcc/output.h index fa8ace1f394..1f9af46da1d 100644 --- a/gcc/output.h +++ b/gcc/output.h @@ -548,7 +548,7 @@ extern void switch_to_other_text_partition (void); extern section *get_cdtor_priority_section (int, bool); extern bool unlikely_text_section_p (section *); -extern void switch_to_section (section *); +extern void switch_to_section (section *, tree = nullptr); extern void output_section_asm_op (const void *); extern void record_tm_clone_pair (tree, tree); diff --git a/gcc/testsuite/c-c++-common/attr-used-5.c b/gcc/testsuite/c-c++-common/attr-used-5.c new file mode 100644 index 00000000000..9fc0d3834e9 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-5.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +struct dtv_slotinfo_list +{ + struct dtv_slotinfo_list *next; +}; + +extern struct dtv_slotinfo_list *list; + +static int __attribute__ ((section ("__libc_freeres_fn"))) +free_slotinfo (struct dtv_slotinfo_list **elemp) +{ + if (!free_slotinfo (&(*elemp)->next)) + return 0; + return 1; +} + +__attribute__ ((used, section ("__libc_freeres_fn"))) +static void free_mem (void) +{ + free_slotinfo (&list); +} + +/* { dg-final { scan-assembler "__libc_freeres_fn,\"ax\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler "__libc_freeres_fn,\"axR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-6.c b/gcc/testsuite/c-c++-common/attr-used-6.c new file mode 100644 index 00000000000..0cb82ade5a9 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-6.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +struct dtv_slotinfo_list +{ + struct dtv_slotinfo_list *next; +}; + +extern struct dtv_slotinfo_list *list; + +static int __attribute__ ((used, section ("__libc_freeres_fn"))) +free_slotinfo (struct dtv_slotinfo_list **elemp) +{ + if (!free_slotinfo (&(*elemp)->next)) + return 0; + return 1; +} + +__attribute__ ((section ("__libc_freeres_fn"))) +void free_mem (void) +{ + free_slotinfo (&list); +} + +/* { dg-final { scan-assembler "__libc_freeres_fn,\"ax\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler "__libc_freeres_fn,\"axR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-7.c b/gcc/testsuite/c-c++-common/attr-used-7.c new file mode 100644 index 00000000000..fba2706ffc1 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-7.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +int __attribute__((used,section(".data.foo"))) foo2 = 2; +int __attribute__((section(".data.foo"))) foo1 = 1; + +/* { dg-final { scan-assembler ".data.foo,\"aw\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler ".data.foo,\"awR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-8.c b/gcc/testsuite/c-c++-common/attr-used-8.c new file mode 100644 index 00000000000..4da4aabe573 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-8.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +int __attribute__((section(".data.foo"))) foo1 = 1; +int __attribute__((used,section(".data.foo"))) foo2 = 2; + +/* { dg-final { scan-assembler ".data.foo,\"aw\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler ".data.foo,\"awR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-9.c b/gcc/testsuite/c-c++-common/attr-used-9.c new file mode 100644 index 00000000000..cf3bde67622 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-used-9.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O2" } */ + +struct dtv_slotinfo_list +{ + struct dtv_slotinfo_list *next; +}; + +extern struct dtv_slotinfo_list *list; + +static int __attribute__ ((used, section ("__libc_freeres_fn"))) +free_slotinfo (struct dtv_slotinfo_list **elemp) +{ + if (!free_slotinfo (&(*elemp)->next)) + return 0; + return 1; +} + +__attribute__ ((section ("__libc_freeres_fn"))) +static void free_mem (void) +/* { dg-warning "defined but not used" "" { target *-*-* } .-1 } */ +{ + free_slotinfo (&list); +} + +/* { dg-final { scan-assembler-not "__libc_freeres_fn\n" } } */ +/* { dg-final { scan-assembler-not "__libc_freeres_fn,\"ax\"" { target R_flag_in_section } } } */ +/* { dg-final { scan-assembler "__libc_freeres_fn,\"axR\"" { target R_flag_in_section } } } */ diff --git a/gcc/varasm.c b/gcc/varasm.c index c5487a78b13..cfec870e067 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -281,7 +281,12 @@ get_noswitch_section (unsigned int flags, noswitch_section_callback callback) /* Return the named section structure associated with NAME. Create a new section with the given fields if no such structure exists. - When NOT_EXISTING, then fail if the section already exists. */ + When NOT_EXISTING, then fail if the section already exists. Return + the existing section if the SECTION_RETAIN bit doesn't match. Set + the SECTION_WRITE | SECTION_RELRO bits on the the existing section + if one of the section flags is SECTION_WRITE | SECTION_RELRO and the + other has none of these flags in named sections and either the section + hasn't been declared yet or has been declared as writable. */ section * get_section (const char *name, unsigned int flags, tree decl, @@ -343,6 +348,11 @@ get_section (const char *name, unsigned int flags, tree decl, sect->common.flags |= (SECTION_WRITE | SECTION_RELRO); return sect; } + /* If the SECTION_RETAIN bit doesn't match, return and switch + to a new section later. */ + if ((sect->common.flags & SECTION_RETAIN) + != (flags & SECTION_RETAIN)) + return sect; /* Sanity check user variables for flag changes. */ if (sect->named.decl != NULL && DECL_P (sect->named.decl) @@ -1879,7 +1889,7 @@ assemble_start_function (tree decl, const char *fnname) /* Switch to the correct text section for the start of the function. */ - switch_to_section (function_section (decl)); + switch_to_section (function_section (decl), decl); if (crtl->has_bb_partition && !hot_label_written) ASM_OUTPUT_LABEL (asm_out_file, crtl->subsections.hot_section_label); @@ -2375,7 +2385,7 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED, && (strcmp (sect->named.name, ".vtable_map_vars") == 0)) handle_vtv_comdat_section (sect, decl); else - switch_to_section (sect); + switch_to_section (sect, decl); if (align > BITS_PER_UNIT) ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (align / BITS_PER_UNIT)); assemble_variable_contents (decl, name, dont_output_data, @@ -7742,10 +7752,28 @@ output_section_asm_op (const void *directive) the current section is NEW_SECTION. */ void -switch_to_section (section *new_section) +switch_to_section (section *new_section, tree decl) { if (in_section == new_section) - return; + { + if (HAVE_GAS_SHF_GNU_RETAIN + && (new_section->common.flags & SECTION_NAMED) + && decl != nullptr + && DECL_P (decl) + && (!!DECL_PRESERVE_P (decl) + != !!(new_section->common.flags & SECTION_RETAIN))) + { + /* If the SECTION_RETAIN bit doesn't match, switch to a new + section. */ + if (DECL_PRESERVE_P (decl)) + new_section->common.flags |= SECTION_RETAIN; + else + new_section->common.flags &= ~(SECTION_RETAIN + | SECTION_DECLARED); + } + else + return; + } if (new_section->common.flags & SECTION_FORGET) in_section = NULL; -- 2.29.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* V4 [PATCH 2/3] Warn used and not used symbols in section with the same name 2020-12-15 17:30 V4 [PATCH 0/3] Switch to a new section if the SECTION_RETAIN bit doesn't match H.J. Lu 2020-12-15 17:30 ` V4 [PATCH 1/3] " H.J. Lu @ 2020-12-15 17:30 ` H.J. Lu 2020-12-16 0:44 ` Jeff Law 2020-12-15 17:30 ` V4 [PATCH 3/3] Require .init_array/.fini_array support for SHF_GNU_RETAIN H.J. Lu 2 siblings, 1 reply; 8+ messages in thread From: H.J. Lu @ 2020-12-15 17:30 UTC (permalink / raw) To: gcc-patches When SECTION_RETAIN is used, issue a warning when a symbol without used attribute and a symbol with used attribute are placed in the section with the same name, like int __attribute__((used,section(".data.foo"))) foo2 = 2; int __attribute__((section(".data.foo"))) foo1 = 1; since assembler will put them in different sections with the same section name. gcc/ PR target/98146 * varasm.c (switch_to_section): Warn when a symbol without used attribute and a symbol with used attribute are placed in the section with the same name. gcc/testsuite/ PR target/98146 * c-c++-common/attr-used-5.c: Updated. * c-c++-common/attr-used-6.c: Likewise. * c-c++-common/attr-used-7.c: Likewise. * c-c++-common/attr-used-8.c: Likewise. --- gcc/testsuite/c-c++-common/attr-used-5.c | 1 + gcc/testsuite/c-c++-common/attr-used-6.c | 1 + gcc/testsuite/c-c++-common/attr-used-7.c | 1 + gcc/testsuite/c-c++-common/attr-used-8.c | 1 + gcc/varasm.c | 22 +++++++++++++++++++--- 5 files changed, 23 insertions(+), 3 deletions(-) diff --git a/gcc/testsuite/c-c++-common/attr-used-5.c b/gcc/testsuite/c-c++-common/attr-used-5.c index 9fc0d3834e9..ba59326e452 100644 --- a/gcc/testsuite/c-c++-common/attr-used-5.c +++ b/gcc/testsuite/c-c++-common/attr-used-5.c @@ -10,6 +10,7 @@ extern struct dtv_slotinfo_list *list; static int __attribute__ ((section ("__libc_freeres_fn"))) free_slotinfo (struct dtv_slotinfo_list **elemp) +/* { dg-warning "'.*' without 'used' attribute and '.*' with 'used' attribute are placed in a section with the same name" "" { target R_flag_in_section } .-1 } */ { if (!free_slotinfo (&(*elemp)->next)) return 0; diff --git a/gcc/testsuite/c-c++-common/attr-used-6.c b/gcc/testsuite/c-c++-common/attr-used-6.c index 0cb82ade5a9..5d20f875bf0 100644 --- a/gcc/testsuite/c-c++-common/attr-used-6.c +++ b/gcc/testsuite/c-c++-common/attr-used-6.c @@ -18,6 +18,7 @@ free_slotinfo (struct dtv_slotinfo_list **elemp) __attribute__ ((section ("__libc_freeres_fn"))) void free_mem (void) +/* { dg-warning "'.*' without 'used' attribute and '.*' with 'used' attribute are placed in a section with the same name" "" { target R_flag_in_section } .-1 } */ { free_slotinfo (&list); } diff --git a/gcc/testsuite/c-c++-common/attr-used-7.c b/gcc/testsuite/c-c++-common/attr-used-7.c index fba2706ffc1..75576bcabe5 100644 --- a/gcc/testsuite/c-c++-common/attr-used-7.c +++ b/gcc/testsuite/c-c++-common/attr-used-7.c @@ -3,6 +3,7 @@ int __attribute__((used,section(".data.foo"))) foo2 = 2; int __attribute__((section(".data.foo"))) foo1 = 1; +/* { dg-warning "'.*' without 'used' attribute and '.*' with 'used' attribute are placed in a section with the same name" "" { target R_flag_in_section } .-1 } */ /* { dg-final { scan-assembler ".data.foo,\"aw\"" { target R_flag_in_section } } } */ /* { dg-final { scan-assembler ".data.foo,\"awR\"" { target R_flag_in_section } } } */ diff --git a/gcc/testsuite/c-c++-common/attr-used-8.c b/gcc/testsuite/c-c++-common/attr-used-8.c index 4da4aabe573..e4982db1044 100644 --- a/gcc/testsuite/c-c++-common/attr-used-8.c +++ b/gcc/testsuite/c-c++-common/attr-used-8.c @@ -2,6 +2,7 @@ /* { dg-options "-Wall -O2" } */ int __attribute__((section(".data.foo"))) foo1 = 1; +/* { dg-warning "'.*' without 'used' attribute and '.*' with 'used' attribute are placed in a section with the same name" "" { target R_flag_in_section } .-1 } */ int __attribute__((used,section(".data.foo"))) foo2 = 2; /* { dg-final { scan-assembler ".data.foo,\"aw\"" { target R_flag_in_section } } } */ diff --git a/gcc/varasm.c b/gcc/varasm.c index e3e49d53e5d..72d5379303a 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -7761,11 +7761,27 @@ switch_to_section (section *new_section, tree decl) { /* If the SECTION_RETAIN bit doesn't match, switch to a new section. */ + tree used_decl, no_used_decl; + if (DECL_PRESERVE_P (decl)) - new_section->common.flags |= SECTION_RETAIN; + { + new_section->common.flags |= SECTION_RETAIN; + used_decl = decl; + no_used_decl = new_section->named.decl; + } else - new_section->common.flags &= ~(SECTION_RETAIN - | SECTION_DECLARED); + { + new_section->common.flags &= ~(SECTION_RETAIN + | SECTION_DECLARED); + used_decl = new_section->named.decl; + no_used_decl = decl; + } + warning (OPT_Wattributes, + "%+qD without %<used%> attribute and %qD with " + "%<used%> attribute are placed in a section with " + "the same name", no_used_decl, used_decl); + inform (DECL_SOURCE_LOCATION (used_decl), + "%qD was declared here", used_decl); } else return; -- 2.29.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: V4 [PATCH 2/3] Warn used and not used symbols in section with the same name 2020-12-15 17:30 ` V4 [PATCH 2/3] Warn used and not used symbols in section with the same name H.J. Lu @ 2020-12-16 0:44 ` Jeff Law 0 siblings, 0 replies; 8+ messages in thread From: Jeff Law @ 2020-12-16 0:44 UTC (permalink / raw) To: H.J. Lu, gcc-patches On 12/15/20 10:30 AM, H.J. Lu wrote: > When SECTION_RETAIN is used, issue a warning when a symbol without used > attribute and a symbol with used attribute are placed in the section with > the same name, like > > int __attribute__((used,section(".data.foo"))) foo2 = 2; > int __attribute__((section(".data.foo"))) foo1 = 1; > > since assembler will put them in different sections with the same section > name. > > gcc/ > > PR target/98146 > * varasm.c (switch_to_section): Warn when a symbol without used > attribute and a symbol with used attribute are placed in the > section with the same name. > > gcc/testsuite/ > > PR target/98146 > * c-c++-common/attr-used-5.c: Updated. > * c-c++-common/attr-used-6.c: Likewise. > * c-c++-common/attr-used-7.c: Likewise. > * c-c++-common/attr-used-8.c: Likewise. OK jeff ^ permalink raw reply [flat|nested] 8+ messages in thread
* V4 [PATCH 3/3] Require .init_array/.fini_array support for SHF_GNU_RETAIN 2020-12-15 17:30 V4 [PATCH 0/3] Switch to a new section if the SECTION_RETAIN bit doesn't match H.J. Lu 2020-12-15 17:30 ` V4 [PATCH 1/3] " H.J. Lu 2020-12-15 17:30 ` V4 [PATCH 2/3] Warn used and not used symbols in section with the same name H.J. Lu @ 2020-12-15 17:30 ` H.J. Lu 2020-12-16 0:45 ` Jeff Law 2 siblings, 1 reply; 8+ messages in thread From: H.J. Lu @ 2020-12-15 17:30 UTC (permalink / raw) To: gcc-patches Since SHF_GNU_RETAIN support doesn't work for crtstuff.c which switches the output section directly with asm statement: --- static void __attribute__((used)) __do_global_dtors_aux (void) { static _Bool completed; if (__builtin_expect (completed, 0)) return; completed = 1; } static void __attribute__((__used__)) call___do_global_dtors_aux (void) { asm ("\t.section\t.fini"); __do_global_dtors_aux (); asm ("\t.section\t.text"); } --- use SHF_GNU_RETAIN only if .init_array/.fini_array section is supported. gcc/ PR target/98146 * defaults.h (SUPPORTS_SHF_GNU_RETAIN): New. * varasm.c (get_section): Replace HAVE_GAS_SHF_GNU_RETAIN with SUPPORTS_SHF_GNU_RETAIN. (resolve_unique_section): Likewise. (get_variable_section): Likewise. (switch_to_section): Likewise. gcc/testsuite/ PR target/98146 * lib/target-supports.exp (check_effective_target_R_flag_in_section): Also check HAVE_INITFINI_ARRAY_SUPPORT != 0. --- gcc/defaults.h | 11 +++++++++++ gcc/testsuite/lib/target-supports.exp | 2 +- gcc/varasm.c | 8 ++++---- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/gcc/defaults.h b/gcc/defaults.h index f1a38626624..80a84dde2d6 100644 --- a/gcc/defaults.h +++ b/gcc/defaults.h @@ -286,6 +286,17 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #endif #endif +/* This determines whether or not we support marking sections with + SHF_GNU_RETAIN flag. Also require .init_array/.fini_array section + for constructors and destructors. */ +#ifndef SUPPORTS_SHF_GNU_RETAIN +#if HAVE_GAS_SHF_GNU_RETAIN && HAVE_INITFINI_ARRAY_SUPPORT +#define SUPPORTS_SHF_GNU_RETAIN 1 +#else +#define SUPPORTS_SHF_GNU_RETAIN 0 +#endif +#endif + /* This determines whether or not we support link-once semantics. */ #ifndef SUPPORTS_ONE_ONLY #ifdef MAKE_DECL_ONE_ONLY diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 3c02f763e7e..11343d0192f 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -10840,7 +10840,7 @@ proc check_effective_target_R_flag_in_section { } { set f [open $src "w"] puts $f "#include \"../../auto-host.h\"" - puts $f "#if HAVE_GAS_SHF_GNU_RETAIN == 0" + puts $f "#if HAVE_GAS_SHF_GNU_RETAIN == 0 || HAVE_INITFINI_ARRAY_SUPPORT == 0" puts $f "# error Assembler does not support 'R' flag in .section directive." puts $f "#endif" close $f diff --git a/gcc/varasm.c b/gcc/varasm.c index 72d5379303a..46f79ea28e6 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -293,7 +293,7 @@ get_section (const char *name, unsigned int flags, tree decl, slot = section_htab->find_slot_with_hash (name, htab_hash_string (name), INSERT); flags |= SECTION_NAMED; - if (HAVE_GAS_SHF_GNU_RETAIN + if (SUPPORTS_SHF_GNU_RETAIN && decl != nullptr && DECL_P (decl) && DECL_PRESERVE_P (decl)) @@ -483,7 +483,7 @@ resolve_unique_section (tree decl, int reloc ATTRIBUTE_UNUSED, if (DECL_SECTION_NAME (decl) == NULL && targetm_common.have_named_sections && (flag_function_or_data_sections - || (HAVE_GAS_SHF_GNU_RETAIN && DECL_PRESERVE_P (decl)) + || (SUPPORTS_SHF_GNU_RETAIN && DECL_PRESERVE_P (decl)) || DECL_COMDAT_GROUP (decl))) { targetm.asm_out.unique_section (decl, reloc); @@ -1223,7 +1223,7 @@ get_variable_section (tree decl, bool prefer_noswitch_p) vnode->get_constructor (); if (DECL_COMMON (decl) - && !(HAVE_GAS_SHF_GNU_RETAIN && DECL_PRESERVE_P (decl))) + && !(SUPPORTS_SHF_GNU_RETAIN && DECL_PRESERVE_P (decl))) { /* If the decl has been given an explicit section name, or it resides in a non-generic address space, then it isn't common, and shouldn't @@ -7752,7 +7752,7 @@ switch_to_section (section *new_section, tree decl) { if (in_section == new_section) { - if (HAVE_GAS_SHF_GNU_RETAIN + if (SUPPORTS_SHF_GNU_RETAIN && (new_section->common.flags & SECTION_NAMED) && decl != nullptr && DECL_P (decl) -- 2.29.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: V4 [PATCH 3/3] Require .init_array/.fini_array support for SHF_GNU_RETAIN 2020-12-15 17:30 ` V4 [PATCH 3/3] Require .init_array/.fini_array support for SHF_GNU_RETAIN H.J. Lu @ 2020-12-16 0:45 ` Jeff Law 0 siblings, 0 replies; 8+ messages in thread From: Jeff Law @ 2020-12-16 0:45 UTC (permalink / raw) To: H.J. Lu, gcc-patches On 12/15/20 10:30 AM, H.J. Lu wrote: > Since SHF_GNU_RETAIN support doesn't work for crtstuff.c which switches > the output section directly with asm statement: > > --- > static void __attribute__((used)) > __do_global_dtors_aux (void) > { > static _Bool completed; > > if (__builtin_expect (completed, 0)) > return; > completed = 1; > } > > static void __attribute__((__used__)) > call___do_global_dtors_aux (void) > { > asm ("\t.section\t.fini"); > __do_global_dtors_aux (); > asm ("\t.section\t.text"); > } > --- > > use SHF_GNU_RETAIN only if .init_array/.fini_array section is supported. > > gcc/ > > PR target/98146 > * defaults.h (SUPPORTS_SHF_GNU_RETAIN): New. > * varasm.c (get_section): Replace HAVE_GAS_SHF_GNU_RETAIN with > SUPPORTS_SHF_GNU_RETAIN. > (resolve_unique_section): Likewise. > (get_variable_section): Likewise. > (switch_to_section): Likewise. > > gcc/testsuite/ > > PR target/98146 > * lib/target-supports.exp > (check_effective_target_R_flag_in_section): Also check > HAVE_INITFINI_ARRAY_SUPPORT != 0. Ah. I didn't realize that you added this to the V4 patch. OK. jeff ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-12-16 13:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-15 17:30 V4 [PATCH 0/3] Switch to a new section if the SECTION_RETAIN bit doesn't match H.J. Lu 2020-12-15 17:30 ` V4 [PATCH 1/3] " H.J. Lu 2020-12-16 0:44 ` Jeff Law 2020-12-16 13:01 ` H.J. Lu 2020-12-15 17:30 ` V4 [PATCH 2/3] Warn used and not used symbols in section with the same name H.J. Lu 2020-12-16 0:44 ` Jeff Law 2020-12-15 17:30 ` V4 [PATCH 3/3] Require .init_array/.fini_array support for SHF_GNU_RETAIN H.J. Lu 2020-12-16 0:45 ` Jeff Law
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).