Re: gas rearranging elf file symbols

Re: gas rearranging elf file symbols

[Jan Beulich]

This doesn't look correct to me. What I believe needs to be cut down is
the number of symbols resulting from (from the assembler's perspective)
consecutive # <line> <file> preprocessor-generated constructs (because
all of them except the last can only be meaningless except for
dependency determination). Explicit .file/.appfile directives should, in
my opinion, always be honoured. If you really want just a single file
symbol, then I'd still see a need to honor the first (or last)
.file/.appfile in preference over any preprocessor-generated ones. Jan

>>> Alan Modra <amodra@bigpond.net.au> 20.10.04 04:29:03 >>>
On Fri, Oct 08, 2004 at 03:59:29PM +0930, Alan Modra wrote:
> STT_FILE
>     Conventionally, the symbol's name gives the name of the source
file
>     associated with the object file. A file symbol has STB_LOCAL
>     binding, its section index is SHN_ABS, and it precedes the other
>     STB_LOCAL symbols for the file, if it is present.
> 
> From that wording you could also infer that there should only be one
> file symbol, that of the main source file.

I'm inclined to implement this.  gcc/gas seem to only emit one file
symbol for C source, so I think it reasonable to do the same for
assembly.  The difference in number of symbols can be quite
significant,
for example glibc/io/write.o on powerpc-linux currently has 38 file
symbols (in a total of 49 symbols).

	* config/obj-elf.c (elf_file_symbol): Only emit one file
symbol.

Any objections from gdb folks?

Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.c,v
retrieving revision 1.86
diff -u -p -r1.86 obj-elf.c
--- gas/config/obj-elf.c	8 Sep 2004 20:52:48 -0000	1.86
+++ gas/config/obj-elf.c	20 Oct 2004 02:23:13 -0000
@@ -238,19 +238,24 @@ elf_sec_sym_ok_for_reloc (asection *sec)
 void
 elf_file_symbol (const char *s)
 {
-  symbolS *sym;
+  if (symbol_rootP == NULL
+      || symbol_rootP->bsym == NULL
+      || (symbol_rootP->bsym->flags & BSF_FILE) == 0)
+    {
+      symbolS *sym;
 
-  sym = symbol_new (s, absolute_section, 0, NULL);
-  symbol_set_frag (sym, &zero_address_frag);
-  symbol_get_bfdsym (sym)->flags |= BSF_FILE;
+      sym = symbol_new (s, absolute_section, 0, NULL);
+      symbol_set_frag (sym, &zero_address_frag);
+      symbol_get_bfdsym (sym)->flags |= BSF_FILE;
 
-  if (symbol_rootP != sym)
-    {
-      symbol_remove (sym, &symbol_rootP, &symbol_lastP);
-      symbol_insert (sym, symbol_rootP, &symbol_rootP,
&symbol_lastP);
+      if (symbol_rootP != sym)
+	{
+	  symbol_remove (sym, &symbol_rootP, &symbol_lastP);
+	  symbol_insert (sym, symbol_rootP, &symbol_rootP,
&symbol_lastP);
 #ifdef DEBUG
-      verify_symbol_chain (symbol_rootP, symbol_lastP);
+	  verify_symbol_chain (symbol_rootP, symbol_lastP);
 #endif
+	}
     }
 
 #ifdef NEED_ECOFF_DEBUG

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: gas rearranging elf file symbols

[Alan Modra]

On Wed, Oct 20, 2004 at 08:42:12AM +0200, Jan Beulich wrote:
> Explicit .file/.appfile directives should, in
> my opinion, always be honoured.

Yes, you are right.  I should modify the patch so that I only discard
multiple file symbols generated by # preprocessor lines.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: gas rearranging elf file symbols

[Alan Modra]

On Thu, Oct 21, 2004 at 09:18:39AM +0930, Alan Modra wrote:
> On Wed, Oct 20, 2004 at 08:42:12AM +0200, Jan Beulich wrote:
> > Explicit .file/.appfile directives should, in
> > my opinion, always be honoured.
> 
> Yes, you are right.  I should modify the patch so that I only discard
> multiple file symbols generated by # preprocessor lines.

Since I heard no complaints from the gdb folk about this idea, I'm
applying the following.

gas/ChangeLog
	* obj.h (struct format_ops <app_file>): Add int param.
	* read.h (s_app_file_string): Likewise.
	* read.c (s_app_file_string): Likewise.
	(s_app_file): Adjust s_app_file_string call.
	* config/tc-mips.c (s_mips_file): Likewise.
	* config/obj-coff.h (obj_app_file): Add app param.
	* config/obj-ecoff.h (obj_app_file): Likewise.
	* config/obj-multi.h (obj_app_file): Likewise.
	* config/obj-elf.h (elf_file_symbol): Likewise.
	* config/obj-elf.c (elf_file_symbol): Only emit one file symbol
	if called for # preprocessor lines.

Index: gas/obj.h
===================================================================
RCS file: /cvs/src/src/gas/obj.h,v
retrieving revision 1.7
diff -u -p -r1.7 obj.h
--- gas/obj.h	3 Dec 2003 03:39:58 -0000	1.7
+++ gas/obj.h	10 Nov 2004 02:40:28 -0000
@@ -51,7 +51,7 @@ struct format_ops {
   unsigned dfl_leading_underscore : 1;
   unsigned emit_section_symbols : 1;
   void (*begin) (void);
-  void (*app_file) (const char *);
+  void (*app_file) (const char *, int);
   void (*frob_symbol) (symbolS *, int *);
   void (*frob_file) (void);
   void (*frob_file_before_adjust) (void);
Index: gas/read.c
===================================================================
RCS file: /cvs/src/src/gas/read.c,v
retrieving revision 1.79
diff -u -p -r1.79 read.c
--- gas/read.c	13 Aug 2004 19:53:51 -0000	1.79
+++ gas/read.c	10 Nov 2004 02:40:30 -0000
@@ -1570,7 +1570,7 @@ s_data (int ignore ATTRIBUTE_UNUSED)
    .file.  */
 
 void
-s_app_file_string (char *file)
+s_app_file_string (char *file, int appfile)
 {
 #ifdef LISTING
   if (listing)
@@ -1578,7 +1578,7 @@ s_app_file_string (char *file)
 #endif
   register_dependency (file);
 #ifdef obj_app_file
-  obj_app_file (file);
+  obj_app_file (file, appfile);
 #endif
 }
 
@@ -1606,7 +1606,7 @@ s_app_file (int appfile)
 
       demand_empty_rest_of_line ();
       if (!may_omit)
-	s_app_file_string (s);
+	s_app_file_string (s, appfile);
     }
 }
 
Index: gas/read.h
===================================================================
RCS file: /cvs/src/src/gas/read.h,v
retrieving revision 1.23
diff -u -p -r1.23 read.h
--- gas/read.h	9 Feb 2004 12:12:42 -0000	1.23
+++ gas/read.h	10 Nov 2004 02:40:30 -0000
@@ -139,7 +139,7 @@ extern void bss_alloc (symbolS *, addres
 extern offsetT parse_align (int);
 extern symbolS *s_comm_internal (int, symbolS *(*) (int, symbolS *, addressT));
 extern symbolS *s_lcomm_internal (int, symbolS *, addressT);
-extern void s_app_file_string (char *);
+extern void s_app_file_string (char *, int);
 extern void s_app_file (int);
 extern void s_app_line (int);
 extern void s_bad_endr (int);
Index: gas/config/obj-coff.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-coff.h,v
retrieving revision 1.22
diff -u -p -r1.22 obj-coff.h
--- gas/config/obj-coff.h	8 Nov 2004 13:17:22 -0000	1.22
+++ gas/config/obj-coff.h	10 Nov 2004 02:40:31 -0000
@@ -397,7 +397,7 @@ extern int coff_n_line_nos;
 extern void coff_add_linesym PARAMS ((symbolS *));
 
 void c_dot_file_symbol PARAMS ((const char *filename));
-#define obj_app_file c_dot_file_symbol
+#define obj_app_file(name, app) c_dot_file_symbol (name)
 
 extern void coff_frob_symbol PARAMS ((symbolS *, int *));
 extern void coff_adjust_symtab PARAMS ((void));
@@ -840,7 +840,7 @@ extern segT obj_coff_add_segment PARAMS 
 extern void obj_coff_section PARAMS ((int));
 
 extern void c_dot_file_symbol PARAMS ((char *filename));
-#define obj_app_file c_dot_file_symbol
+#define obj_app_file(name, app) c_dot_file_symbol (name)
 extern void obj_extra_stuff PARAMS ((object_headers * headers));
 
 extern segT s_get_segment PARAMS ((symbolS *ptr));
Index: gas/config/obj-ecoff.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-ecoff.h,v
retrieving revision 1.4
diff -u -p -r1.4 obj-ecoff.h
--- gas/config/obj-ecoff.h	5 Sep 2002 00:01:16 -0000	1.4
+++ gas/config/obj-ecoff.h	10 Nov 2004 02:40:31 -0000
@@ -63,7 +63,7 @@ extern void ecoff_frob_file PARAMS ((voi
 #define obj_symbol_new_hook ecoff_symbol_new_hook
 
 /* Record file switches in the ECOFF symbol table.  */
-#define obj_app_file(name) ecoff_new_file (name)
+#define obj_app_file(name, app) ecoff_new_file (name)
 
 /* At the moment we don't want to do any stabs processing in read.c.  */
 #define OBJ_PROCESS_STAB(seg, what, string, type, other, desc) \
Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.c,v
retrieving revision 1.86
diff -u -p -r1.86 obj-elf.c
--- gas/config/obj-elf.c	8 Sep 2004 20:52:48 -0000	1.86
+++ gas/config/obj-elf.c	10 Nov 2004 02:40:32 -0000
@@ -236,21 +236,27 @@ elf_sec_sym_ok_for_reloc (asection *sec)
 }
 
 void
-elf_file_symbol (const char *s)
+elf_file_symbol (const char *s, int appfile)
 {
-  symbolS *sym;
+  if (!appfile
+      || symbol_rootP == NULL
+      || symbol_rootP->bsym == NULL
+      || (symbol_rootP->bsym->flags & BSF_FILE) == 0)
+    {
+      symbolS *sym;
 
-  sym = symbol_new (s, absolute_section, 0, NULL);
-  symbol_set_frag (sym, &zero_address_frag);
-  symbol_get_bfdsym (sym)->flags |= BSF_FILE;
+      sym = symbol_new (s, absolute_section, 0, NULL);
+      symbol_set_frag (sym, &zero_address_frag);
+      symbol_get_bfdsym (sym)->flags |= BSF_FILE;
 
-  if (symbol_rootP != sym)
-    {
-      symbol_remove (sym, &symbol_rootP, &symbol_lastP);
-      symbol_insert (sym, symbol_rootP, &symbol_rootP, &symbol_lastP);
+      if (symbol_rootP != sym)
+	{
+	  symbol_remove (sym, &symbol_rootP, &symbol_lastP);
+	  symbol_insert (sym, symbol_rootP, &symbol_rootP, &symbol_lastP);
 #ifdef DEBUG
-      verify_symbol_chain (symbol_rootP, symbol_lastP);
+	  verify_symbol_chain (symbol_rootP, symbol_lastP);
 #endif
+	}
     }
 
 #ifdef NEED_ECOFF_DEBUG
Index: gas/config/obj-elf.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.h,v
retrieving revision 1.22
diff -u -p -r1.22 obj-elf.h
--- gas/config/obj-elf.h	13 Dec 2003 12:57:40 -0000	1.22
+++ gas/config/obj-elf.h	10 Nov 2004 02:40:32 -0000
@@ -159,7 +159,7 @@ extern void elf_frob_file_after_relocs (
 #ifndef obj_app_file
 #define obj_app_file elf_file_symbol
 #endif
-extern void elf_file_symbol (const char *);
+extern void elf_file_symbol (const char *, int);
 
 extern void obj_elf_section_change_hook (void);
 
Index: gas/config/obj-multi.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-multi.h,v
retrieving revision 1.8
diff -u -p -r1.8 obj-multi.h
--- gas/config/obj-multi.h	5 Sep 2002 00:01:16 -0000	1.8
+++ gas/config/obj-multi.h	10 Nov 2004 02:40:32 -0000
@@ -37,9 +37,9 @@
 	 ? (*this_format->begin) ()			\
 	 : (void) 0)
 
-#define obj_app_file(NAME)				\
+#define obj_app_file(NAME, APPFILE)			\
 	(this_format->app_file				\
-	 ? (*this_format->app_file) (NAME)		\
+	 ? (*this_format->app_file) (NAME, APPFILE)	\
 	 : (void) 0)
 
 #define obj_frob_symbol(S,P)				\
Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.273
diff -u -p -r1.273 tc-mips.c
--- gas/config/tc-mips.c	7 Oct 2004 22:29:18 -0000	1.273
+++ gas/config/tc-mips.c	10 Nov 2004 02:40:39 -0000
@@ -13508,7 +13508,7 @@ s_mips_file (int x ATTRIBUTE_UNUSED)
       if (filename != NULL && ! first_file_directive)
 	{
 	  (void) new_logical_line (filename, -1);
-	  s_app_file_string (filename);
+	  s_app_file_string (filename, 0);
 	}
       first_file_directive = 1;
     }

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: gas rearranging elf file symbols

[Alan Modra]

On Fri, Oct 08, 2004 at 03:59:29PM +0930, Alan Modra wrote:
> STT_FILE
>     Conventionally, the symbol's name gives the name of the source file
>     associated with the object file. A file symbol has STB_LOCAL
>     binding, its section index is SHN_ABS, and it precedes the other
>     STB_LOCAL symbols for the file, if it is present.
> 
> From that wording you could also infer that there should only be one
> file symbol, that of the main source file.

I'm inclined to implement this.  gcc/gas seem to only emit one file
symbol for C source, so I think it reasonable to do the same for
assembly.  The difference in number of symbols can be quite significant,
for example glibc/io/write.o on powerpc-linux currently has 38 file
symbols (in a total of 49 symbols).

	* config/obj-elf.c (elf_file_symbol): Only emit one file symbol.

Any objections from gdb folks?

Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.c,v
retrieving revision 1.86
diff -u -p -r1.86 obj-elf.c
--- gas/config/obj-elf.c	8 Sep 2004 20:52:48 -0000	1.86
+++ gas/config/obj-elf.c	20 Oct 2004 02:23:13 -0000
@@ -238,19 +238,24 @@ elf_sec_sym_ok_for_reloc (asection *sec)
 void
 elf_file_symbol (const char *s)
 {
-  symbolS *sym;
+  if (symbol_rootP == NULL
+      || symbol_rootP->bsym == NULL
+      || (symbol_rootP->bsym->flags & BSF_FILE) == 0)
+    {
+      symbolS *sym;
 
-  sym = symbol_new (s, absolute_section, 0, NULL);
-  symbol_set_frag (sym, &zero_address_frag);
-  symbol_get_bfdsym (sym)->flags |= BSF_FILE;
+      sym = symbol_new (s, absolute_section, 0, NULL);
+      symbol_set_frag (sym, &zero_address_frag);
+      symbol_get_bfdsym (sym)->flags |= BSF_FILE;
 
-  if (symbol_rootP != sym)
-    {
-      symbol_remove (sym, &symbol_rootP, &symbol_lastP);
-      symbol_insert (sym, symbol_rootP, &symbol_rootP, &symbol_lastP);
+      if (symbol_rootP != sym)
+	{
+	  symbol_remove (sym, &symbol_rootP, &symbol_lastP);
+	  symbol_insert (sym, symbol_rootP, &symbol_rootP, &symbol_lastP);
 #ifdef DEBUG
-      verify_symbol_chain (symbol_rootP, symbol_lastP);
+	  verify_symbol_chain (symbol_rootP, symbol_lastP);
 #endif
+	}
     }
 
 #ifdef NEED_ECOFF_DEBUG

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: gas rearranging elf file symbols

[Jan Beulich]

Exactly. I see these two interpretations of the wording: Either there
can be just (at most) one such symbol (for the main source file), which
is the way gcc handles it. Or there can be multiple of them, each one
preceding the local symbols resulting from that file. But I cannot
interpret the wording in a way that would make it match the current
implementation.

Jan

>>> Alan Modra <amodra@bigpond.net.au> 08.10.04 08:29:29 >>>
On Fri, Oct 08, 2004 at 03:23:19PM +0930, Alan Modra wrote:
> the correct file.  You can't do better for globals of course, since
> global symbols are placed after local symbols, losing any
association
> you might infer from the symbol ordering.

Actually, you can't do much about local symbols either.  The ELF gABI
says:

STT_FILE
    Conventionally, the symbol's name gives the name of the source
file
    associated with the object file. A file symbol has STB_LOCAL
    binding, its section index is SHN_ABS, and it precedes the other
    STB_LOCAL symbols for the file, if it is present.

From that wording you could also infer that there should only be one
file symbol, that of the main source file.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: gas rearranging elf file symbols

[Alan Modra]

On Fri, Oct 08, 2004 at 03:23:19PM +0930, Alan Modra wrote:
> the correct file.  You can't do better for globals of course, since
> global symbols are placed after local symbols, losing any association
> you might infer from the symbol ordering.

Actually, you can't do much about local symbols either.  The ELF gABI
says:

STT_FILE
    Conventionally, the symbol's name gives the name of the source file
    associated with the object file. A file symbol has STB_LOCAL
    binding, its section index is SHN_ABS, and it precedes the other
    STB_LOCAL symbols for the file, if it is present.

From that wording you could also infer that there should only be one
file symbol, that of the main source file.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: gas rearranging elf file symbols

[Alan Modra]

On Wed, Oct 06, 2004 at 10:31:30AM +0200, Jan Beulich wrote:
> After enabling elf_find_function to actually report a filename if any
> can be found, it became immediately obvious that gas prevents this from
> being fully functional. This is because gas (a) emits all the file
> symbols (resulting from # <line> "<filename>", .file, or .appfile) in a
> block and (b) inverts the sequence in which they were encountered. With
> this, associating a local symbol with its (pseudo-)source file name is
> impossible.
> 
> Is anyone able to explain why this works the way it does currently?

I'm not sure why the file symbols are reversed, but it looks to be
intentional.  See config/obj-elf.c:elf_file_symbol.  This has the effect
of putting the sym for the main source file last, which then is the one
that always matches in elf_find_function.  As you say, that might not be
the correct file.  You can't do better for globals of course, since
global symbols are placed after local symbols, losing any association
you might infer from the symbol ordering.

While looking at this, I noticed that all file syms starting with ".."
were being dropped.

	* syms.c (bfd_is_local_label): Return false for file symbols.

Index: bfd/syms.c
===================================================================
RCS file: /cvs/src/src/bfd/syms.c,v
retrieving revision 1.37
diff -u -p -r1.37 syms.c
--- bfd/syms.c	5 Oct 2004 11:12:56 -0000	1.37
+++ bfd/syms.c	8 Oct 2004 05:52:23 -0000
@@ -355,7 +355,7 @@ bfd_is_local_label (bfd *abfd, asymbol *
   /* The BSF_SECTION_SYM check is needed for IA-64, where every label that
      starts with '.' is local.  This would accidentally catch section names
      if we didn't reject them here.  */
-  if ((sym->flags & (BSF_GLOBAL | BSF_WEAK | BSF_SECTION_SYM)) != 0)
+  if ((sym->flags & (BSF_GLOBAL | BSF_WEAK | BSF_FILE | BSF_SECTION_SYM)) != 0)
     return FALSE;
   if (sym->name == NULL)
     return FALSE;

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

gas rearranging elf file symbols

[Jan Beulich]

After enabling elf_find_function to actually report a filename if any
can be found, it became immediately obvious that gas prevents this from
being fully functional. This is because gas (a) emits all the file
symbols (resulting from # <line> "<filename>", .file, or .appfile) in a
block and (b) inverts the sequence in which they were encountered. With
this, associating a local symbol with its (pseudo-)source file name is
impossible.

Is anyone able to explain why this works the way it does currently?
This may have been inherited from COFF (where a comment says reordering
is done intentionally), but it would seem wrong there, too.

Thanks, Jan