[1/3] gas/ELF: restrict visibility changes

Message ID c3efa225-2026-4d16-93b2-f5f83ba1b98d@suse.com
State New
Headers
Series ELF: visibility handling |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jan Beulich April 10, 2026, 1:30 p.m. UTC
  Since the spec mandates that the most restricting visibility ought to
propagate when linking, the same rule should apply when assembling.
  

Patch

--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -404,21 +404,24 @@  static void
 obj_elf_visibility (int visibility)
 {
   int c;
-  symbolS *symbolP;
-  asymbol *bfdsym;
-  elf_symbol_type *elfsym;
 
   do
     {
-      symbolP = get_sym_from_input_line_and_check ();
-
-      bfdsym = symbol_get_bfdsym (symbolP);
-      elfsym = elf_symbol_from (bfdsym);
+      symbolS *symbolP = get_sym_from_input_line_and_check ();
+      const asymbol *bfdsym = symbol_get_bfdsym (symbolP);
+      elf_symbol_type *elfsym = elf_symbol_from (bfdsym);
+      int current = ELF_ST_VISIBILITY (elfsym->internal_elf_sym.st_other);
 
       gas_assert (elfsym);
 
-      elfsym->internal_elf_sym.st_other &= ~3;
-      elfsym->internal_elf_sym.st_other |= visibility;
+      if (!current || visibility <= current)
+	{
+	  elfsym->internal_elf_sym.st_other &= ~ELF_ST_VISIBILITY (~0);
+	  elfsym->internal_elf_sym.st_other |= visibility;
+	}
+      else
+	as_warn (_("visibility of `%s' is already `%s'"), S_GET_NAME (symbolP),
+		 current == STV_HIDDEN ? "hidden" : "internal");
 
       c = *input_line_pointer;
       if (c == ',')
--- a/gas/testsuite/gas/elf/elf.exp
+++ b/gas/testsuite/gas/elf/elf.exp
@@ -237,6 +237,7 @@  if { [is_elf_format] } then {
 	# The alpha port uses .set for state, e.g. nomacro.
 	run_dump_test "symtab"
     }
+    run_dump_test "visibility"
     run_dump_test "symver"
     run_dump_test "pr21661"
     run_dump_test "pr14891"
--- a/gas/testsuite/gas/elf/pseudo.l
+++ b/gas/testsuite/gas/elf/pseudo.l
@@ -4,6 +4,7 @@ 
 [^:]*:6: Error: Missing symbol name in directive
 [^:]*:8: Error: Missing symbol name in directive
 [^:]*:10: Error: Missing symbol name in directive
+[^:]*:10: Warning: visibility of .* is already .internal.
 [^:]*:12: Error: Missing symbol name in directive
 [^:]*:14: Error: Missing symbol name in directive
 [^:]*:14: Error: expected comma after name in .symver
--- /dev/null
+++ b/gas/testsuite/gas/elf/visibility.d
@@ -0,0 +1,12 @@ 
+#name: diagnostics for visibility directives
+#readelf: -s -W
+#warning_output: visibility.l
+#target: [supports_gnu_unique]
+
+#...
+ +[0-9]+: +0+ +0 +(NOTYPE|OBJECT) +GLOBAL +INTERNAL +[1-9] +gd
+ +[0-9]+: +0+1 +0 +(NOTYPE|OBJECT) +WEAK +INTERNAL +[1-9] +wd
+ +[0-9]+: +0+2 +0 +OBJECT +UNIQUE +HIDDEN +[1-9] +gu
+ +[0-9]+: +0+ +0 +(NOTYPE|OBJECT) +GLOBAL +INTERNAL +UND +ge
+ +[0-9]+: +0+ +0 +(NOTYPE|OBJECT) +WEAK +HIDDEN +UND +we
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/elf/visibility.l
@@ -0,0 +1,5 @@ 
+[^:]*: Assembler messages:
+[^:]*:4: Warning: visibility of .gd. is already .internal.
+[^:]*:5: Warning: visibility of .gd. is already .internal.
+[^:]*:21: Warning: visibility of .ge. is already .hidden.
+[^:]*:26: Warning: visibility of .we. is already .hidden.
--- /dev/null
+++ b/gas/testsuite/gas/elf/visibility.s
@@ -0,0 +1,28 @@ 
+	.data
+	.global gd
+	.internal gd
+	.hidden gd
+	.protected gd
+gd:	.dc.b 0
+
+	.weak wd
+	.protected wd
+	.hidden wd
+	.internal wd
+wd:	.dc.b 0
+
+	.type gu, %gnu_unique_object
+	.hidden gu
+	.hidden gu
+gu:	.dc.b 0
+
+	.global ge
+	.hidden ge
+	.protected ge
+	.internal ge
+
+	.weak we
+	.hidden we
+	.protected we
+	.p2align 3
+	.dc.a we