* 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
* 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
* 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 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 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
* 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
* 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
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).