[PATCH] Do not allow .global on section symbols

[PATCH] Do not allow .global on section symbols

[Nick Clifton]

Hi Guys,

  A customer recently discovered that GAS did not stop you using
  .globl on a section symbol.  Unfortunately it did corrupt the symbol
  table that was produced (moving sections symbols into the local
  symbol area), which then resulted in seg faults inside the linker.
  To see this, assemble and link the following x86 test program:

          .section ddd,"a"
          .align 1
          .global  ddd 
      
      ddd_data:
          .space              0x0064
      
          .section ccc, "ax"
          .align 2
      
      ccc_code:
           mov %eax,ddd_data

  The patch below fixes this by preventing .globl from changing a
  section symbol's status and instead making it issue a warning
  message.

  I was not sure however, how to add this code as a test case.  I did
  not want to add it to the assembler's testsuite, since it needs the
  linker in order to demonstrate the fault, and I did not want to add
  it to the linker's testsuite since it contains ELF and x86 specific
  code.  Any suggestions for how to incorporate it as a test as most
  welcome.

Cheers
        Nick

2001-06-05  Nick Clifton  <nickc@cambridge.redhat.com>

	* symbols.c (S_SET_EXTERNAL): Do not override a section symbol's
	status.

Index: gas/symbols.c
===================================================================
RCS file: /cvs/src/src/gas/symbols.c,v
retrieving revision 1.24
diff -p -r1.24 symbols.c
*** symbols.c	2001/05/25 10:07:43	1.24
--- symbols.c	2001/06/05 08:22:06
*************** S_SET_EXTERNAL (s)
*** 1824,1829 ****
--- 1824,1840 ----
        /* Let .weak override .global.  */
        return;
      }
+   if (s->bsym->flags & BSF_SECTION_SYM)
+     {
+       char * file;
+       unsigned int line;
+       
+       /* Do not reassign section symbols.  */
+       as_where (& file, & line);
+       as_warn_where (file, line,
+ 		     _("Section symbols are already global"));
+       return;
+     }
    s->bsym->flags |= BSF_GLOBAL;
    s->bsym->flags &= ~(BSF_LOCAL | BSF_WEAK);
  }

Re: [PATCH] Do not allow .global on section symbols

[Ian Lance Taylor]

Nick Clifton <nickc@cambridge.redhat.com> writes:

>   I was not sure however, how to add this code as a test case.  I did
>   not want to add it to the assembler's testsuite, since it needs the
>   linker in order to demonstrate the fault, and I did not want to add
>   it to the linker's testsuite since it contains ELF and x86 specific
>   code.  Any suggestions for how to incorporate it as a test as most
>   welcome.

Now that you've added the warning, you can just test for the warning.
It seems to me that using the linker won't help anyhow, since you can
presumably no longer recreate the bug.  However, another option would
be to use objdump; presumably the difference in the symbol would be
displayed by objdump -t.

Ian

Re: [PATCH] Do not allow .global on section symbols

[H . J . Lu]

On Tue, Jun 05, 2001 at 11:29:30AM -0700, Ian Lance Taylor wrote:
> Nick Clifton <nickc@cambridge.redhat.com> writes:
> 
> >   I was not sure however, how to add this code as a test case.  I did
> >   not want to add it to the assembler's testsuite, since it needs the
> >   linker in order to demonstrate the fault, and I did not want to add
> >   it to the linker's testsuite since it contains ELF and x86 specific
> >   code.  Any suggestions for how to incorporate it as a test as most
> >   welcome.
> 
> Now that you've added the warning, you can just test for the warning.
> It seems to me that using the linker won't help anyhow, since you can
> presumably no longer recreate the bug.  However, another option would
> be to use objdump; presumably the difference in the symbol would be
> displayed by objdump -t.
> 

How about this patch?


H.J.
-----
2001-06-05  H.J. Lu  <hjl@gnu.org>

	* gas/elf/elf.exp (run_list_test): New.
	Run section2 with run_list_test.

	* gas/elf/section2.e: New file.
	* gas/elf/section2.l: Likewise.
	* gas/elf/section2.s

Index: gas/elf/elf.exp
===================================================================
RCS file: /work/cvs/gnu/binutils/gas/testsuite/gas/elf/elf.exp,v
retrieving revision 1.7
diff -u -p -r1.7 elf.exp
--- gas/elf/elf.exp	2001/05/21 18:39:53	1.7
+++ gas/elf/elf.exp	2001/06/05 20:22:41
@@ -2,6 +2,33 @@
 # elf tests
 #
 
+proc run_list_test { name opts } {
+    global READELF
+    global srcdir subdir
+    set testname "elf $name list"
+    set file $srcdir/$subdir/$name
+    gas_run ${name}.s "$opts -o dump.o" ">&dump.out"
+    if { [regexp_diff "dump.out" "${file}.l"] } then {
+	fail $testname
+	verbose "output is [file_contents "dump.out"]" 2
+	return
+    }
+    send_log "$READELF -s dump.o > dump.out\n"
+    catch "exec $READELF -s dump.o > dump.out\n" comp_output
+    if ![string match "" $comp_output] then {
+	send_log "$comp_output\n"
+	fail $testname
+	return
+    }
+    verbose_eval {[file_contents "dump.out"]} 3
+    if { [regexp_diff "dump.out" "${file}.e"] } then {
+	fail $testname
+	verbose "output is [file_contents "dump.out"]" 2
+	return
+    }
+    pass $testname
+}
+
 # We're testing bits in obj-elf -- don't run on anything else.
 if { ([istarget "*-*-elf*"]		
       || [istarget "*-*-linux*"]
@@ -14,4 +41,5 @@ if { ([istarget "*-*-elf*"]		
     run_dump_test "ehopt0"
     run_dump_test "section0" 
     run_dump_test "section1" 
+    run_list_test "section2" "-al"
 }
--- /dev/null	Fri Mar 23 20:37:44 2001
+++ gas/elf/section2.s	Tue Jun  5 11:39:10 2001
@@ -0,0 +1,3 @@
+          .section A
+          .global A
+	  .byte 49
--- /dev/null	Fri Mar 23 20:37:44 2001
+++ gas/elf/section2.l	Tue Jun  5 12:04:53 2001
@@ -0,0 +1,8 @@
+.*: Assembler messages:
+.*:2: Warning: Section symbols are already global
+GAS LISTING .*
+
+
+   1              	          .section A
+   2              	          .global A
+   3 0000 31       		  .byte 49
--- /dev/null	Fri Mar 23 20:37:44 2001
+++ gas/elf/section2.e	Tue Jun  5 13:05:50 2001
@@ -0,0 +1,8 @@
+
+Symbol table '.symtab' contains 5 entries:
+   Num:    Value  Size Type    Bind   Vis      Ndx Name
+     0: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND 
+     1: 00000000     0 SECTION LOCAL  DEFAULT    1 
+     2: 00000000     0 SECTION LOCAL  DEFAULT    2 
+     3: 00000000     0 SECTION LOCAL  DEFAULT    3 
+     4: 00000000     0 SECTION LOCAL  DEFAULT    4