Re: [PATCH] ELF: make .struct interact with .previous

Re: [PATCH] ELF: make .struct interact with .previous

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

>>> Alan Modra <amodra@bigpond.net.au> 18.04.05 01:34:05 >>>
>The struct test is using a .short, which looks like a data allocation
>pseudo-op to me.  If .short should have different alignment when you
>happen to use it in the absolute section, then md_cons_align should
>reflect that fact.

I agree to this; therefore I'd suggest the below adjustment to the test.

Jan

gas/testsuite/
2005-04-15  Jan Beulich  <jbeulich@novell.com>

	* gas/elf/struct.s: Adjust to not get into alignment issues.
	* gas/elf/struct.d: Adjust for the above and the test's name.

--- cpp/binutils/mainline-cvs/2005-04-11/gas/testsuite/gas/elf/struct.d.0	2005-04-12 11:48:51.000000000 +0200
+++ cpp/binutils/mainline-cvs/2005-04-11/gas/testsuite/gas/elf/struct.d	2005-04-18 09:54:43.846300704 +0200
@@ -1,10 +1,10 @@
 #nm: --extern-only
-#name: struct
+#name: ELF struct
 
 # Test the .struct pseudo-op.
 
 0+0 D l1
 0+4 D l2
-0+1 A w1
-0+3 A w2
-0+5 A w3
+0+2 A w1
+0+4 A w2
+0+6 A w3
--- cpp/binutils/mainline-cvs/2005-04-11/gas/testsuite/gas/elf/struct.s.0	2005-04-12 11:45:06.000000000 +0200
+++ cpp/binutils/mainline-cvs/2005-04-11/gas/testsuite/gas/elf/struct.s	2005-04-18 09:48:26.401681008 +0200
@@ -1,7 +1,7 @@
 	.globl	w1, w2, w3, l1, l2
 	.data
 l1:	.long	0
-	.struct 1
+	.struct 2
 w1:	.short	0
 w2:	.short	0
 w3:	.short	0



[-- Attachment #2: binutils-mainline-elf-struct-2.patch --]
[-- Type: text/plain, Size: 1022 bytes --]

I agree to this; therefore I'd suggest the below adjustment to the test.

Jan

gas/testsuite/
2005-04-15  Jan Beulich  <jbeulich@novell.com>

	* gas/elf/struct.s: Adjust to not get into alignment issues.
	* gas/elf/struct.d: Adjust for the above and the test's name.

--- cpp/binutils/mainline-cvs/2005-04-11/gas/testsuite/gas/elf/struct.d.0	2005-04-12 11:48:51.000000000 +0200
+++ cpp/binutils/mainline-cvs/2005-04-11/gas/testsuite/gas/elf/struct.d	2005-04-18 09:54:43.846300704 +0200
@@ -1,10 +1,10 @@
 #nm: --extern-only
-#name: struct
+#name: ELF struct
 
 # Test the .struct pseudo-op.
 
 0+0 D l1
 0+4 D l2
-0+1 A w1
-0+3 A w2
-0+5 A w3
+0+2 A w1
+0+4 A w2
+0+6 A w3
--- cpp/binutils/mainline-cvs/2005-04-11/gas/testsuite/gas/elf/struct.s.0	2005-04-12 11:45:06.000000000 +0200
+++ cpp/binutils/mainline-cvs/2005-04-11/gas/testsuite/gas/elf/struct.s	2005-04-18 09:48:26.401681008 +0200
@@ -1,7 +1,7 @@
 	.globl	w1, w2, w3, l1, l2
 	.data
 l1:	.long	0
-	.struct 1
+	.struct 2
 w1:	.short	0
 w2:	.short	0
 w3:	.short	0

Re: [PATCH] ELF: make .struct interact with .previous

[H. J. Lu]

On Mon, Apr 18, 2005 at 09:57:19AM +0200, Jan Beulich wrote:
> >>> Alan Modra <amodra@bigpond.net.au> 18.04.05 01:34:05 >>>
> >The struct test is using a .short, which looks like a data allocation
> >pseudo-op to me.  If .short should have different alignment when you
> >happen to use it in the absolute section, then md_cons_align should
> >reflect that fact.
> 
> I agree to this; therefore I'd suggest the below adjustment to the test.
> 
> Jan
> 
> gas/testsuite/
> 2005-04-15  Jan Beulich  <jbeulich@novell.com>
> 
> 	* gas/elf/struct.s: Adjust to not get into alignment issues.
> 	* gas/elf/struct.d: Adjust for the above and the test's name.
> 

You are assuming .short will be aligned at 2. The example in document
has

            .struct 0
     field1:
             .struct field1 + 4
     field2:
             .struct field2 + 4
     field3:


H.J.

Re: [PATCH] ELF: make .struct interact with .previous

[James E Wilson]

On Mon, 2005-04-18 at 00:57, Jan Beulich wrote:
> 	* gas/elf/struct.s: Adjust to not get into alignment issues.
> 	* gas/elf/struct.d: Adjust for the above and the test's name.

.short auto-aligns on IA-64, as it does for some other targets also, so
if you want a predictable address, you need to align before emitting the
short.

Yes, this looks like the right fix to me.  Thanks.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: [PATCH] ELF: make .struct interact with .previous

[H. J. Lu]

On Mon, Apr 18, 2005 at 01:12:01PM -0700, James E Wilson wrote:
> On Mon, 2005-04-18 at 00:57, Jan Beulich wrote:
> > 	* gas/elf/struct.s: Adjust to not get into alignment issues.
> > 	* gas/elf/struct.d: Adjust for the above and the test's name.
> 
> .short auto-aligns on IA-64, as it does for some other targets also, so
> if you want a predictable address, you need to align before emitting the
> short.
> 
> Yes, this looks like the right fix to me.  Thanks.

Has this patch been checked in?


H.J.

Re: [PATCH] ELF: make .struct interact with .previous

[Jan Beulich]

>>> "H. J. Lu" <hjl@lucon.org> 19.04.05 23:27:46 >>>
>On Mon, Apr 18, 2005 at 01:12:01PM -0700, James E Wilson wrote:
>> On Mon, 2005-04-18 at 00:57, Jan Beulich wrote:
>> > 	* gas/elf/struct.s: Adjust to not get into alignment issues.
>> > 	* gas/elf/struct.d: Adjust for the above and the test's name.
>> 
>> .short auto-aligns on IA-64, as it does for some other targets also, so
>> if you want a predictable address, you need to align before emitting the
>> short.
>> 
>> Yes, this looks like the right fix to me.  Thanks.
>
>Has this patch been checked in?

Not yet - it is my understanding that it still needs approval from a general maintainer, as this is general (or to be precise, ELF-specific) code.

Jan

Re: [PATCH] ELF: make .struct interact with .previous

[Nick Clifton]

Hi Jan,

>>>On Mon, 2005-04-18 at 00:57, Jan Beulich wrote:
>>>
>>>>	* gas/elf/struct.s: Adjust to not get into alignment issues.
>>>>	* gas/elf/struct.d: Adjust for the above and the test's name.
>>>
>>>.short auto-aligns on IA-64, as it does for some other targets also, so
>>>if you want a predictable address, you need to align before emitting the
>>>short.
>>>
>>>Yes, this looks like the right fix to me.  Thanks.
>>
>>Has this patch been checked in?

> Not yet - it is my understanding that it still needs approval from a general maintainer, as this is general (or to be precise, ELF-specific) code.

In which case please consider this patch approved. :-)

Cheers
   Nick

Re: [PATCH] ELF: make .struct interact with .previous

[Jan Beulich]

>You are assuming .short will be aligned at 2. The example in document
>has
>
>            .struct 0
>     field1:
>             .struct field1 + 4
>     field2:
>             .struct field2 + 4
>     field3:

This usage seems ill to me anyway (and it kills what the test tests for - the use of .previous). Yes, I assume .short is (at most) 2-byte aligned; if that's too little, it could be further bumped.

Jan

[PATCH] ELF: make .struct interact with .previous

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2863 bytes --]

struct did not interact correctly with ELF's .previous.

Built and tested on i686-pc-linux-gnu.

Jan

gas/
2005-04-13  Jan Beulich  <jbeulich@novell.com>

	* config/obj-elf.c (obj_elf_struct): New.
	(elf_pseudo_table). Use it for .offset and .struct.

gas/testsuite/
2005-04-13  Jan Beulich  <jbeulich@novell.com>

	* gas/elf/struct.[sd]: New.
	* gas/elf/elf.exp: Run new test.

--- /home/jbeulich/src/binutils/mainline/2005-04-11/gas/config/obj-elf.c	2005-02-28 08:27:13.000000000 +0100
+++ 2005-04-11/gas/config/obj-elf.c	2005-04-13 15:25:24.000000000 +0200
@@ -65,6 +65,7 @@ static void obj_elf_subsection (int);
 static void obj_elf_popsection (int);
 static void obj_elf_tls_common (int);
 static void obj_elf_lcomm (int);
+static void obj_elf_struct (int);
 
 static const pseudo_typeS elf_pseudo_table[] =
 {
@@ -113,6 +114,8 @@ static const pseudo_typeS elf_pseudo_tab
 
   /* We need to trap the section changing calls to handle .previous.  */
   {"data", obj_elf_data, 0},
+  {"offset", obj_elf_struct, 0},
+  {"struct", obj_elf_struct, 0},
   {"text", obj_elf_text, 0},
 
   {"tls_common", obj_elf_tls_common, 0},
@@ -1036,6 +1039,24 @@ obj_elf_text (int i)
 #endif
 }
 
+/* Change to the *ABS* section.  */
+
+void
+obj_elf_struct (int i)
+{
+#ifdef md_flush_pending_output
+  md_flush_pending_output ();
+#endif
+
+  previous_section = now_seg;
+  previous_subsection = now_subseg;
+  s_struct (i);
+
+#ifdef md_elf_section_change_hook
+  md_elf_section_change_hook ();
+#endif
+}
+
 static void
 obj_elf_subsection (int ignore ATTRIBUTE_UNUSED)
 {
--- /home/jbeulich/src/binutils/mainline/2005-04-11/gas/testsuite/gas/elf/elf.exp	2005-01-18 10:43:34.000000000 +0100
+++ 2005-04-11/gas/testsuite/gas/elf/elf.exp	2005-04-13 15:26:20.000000000 +0200
@@ -61,6 +61,7 @@ if { ([istarget "*-*-*elf*"]		
     run_dump_test "section3" 
     run_dump_test "section4"
     run_list_test "section5" "" "-al" "-SW" "| grep \" \\\\.test\\\[0-9\\\]\"" 
+    run_dump_test "struct" 
     run_dump_test "symver" 
     run_list_test "type" "" "" "-s" "| grep \"1 \\\[FONT\\\]\"" 
 }
--- /home/jbeulich/src/binutils/mainline/2005-04-11/gas/testsuite/gas/elf/struct.d	1970-01-01 01:00:00.000000000 +0100
+++ 2005-04-11/gas/testsuite/gas/elf/struct.d	2005-04-12 11:48:51.000000000 +0200
@@ -0,0 +1,10 @@
+#nm: --extern-only
+#name: struct
+
+# Test the .struct pseudo-op.
+
+0+0 D l1
+0+4 D l2
+0+1 A w1
+0+3 A w2
+0+5 A w3
--- /home/jbeulich/src/binutils/mainline/2005-04-11/gas/testsuite/gas/elf/struct.s	1970-01-01 01:00:00.000000000 +0100
+++ 2005-04-11/gas/testsuite/gas/elf/struct.s	2005-04-12 11:45:06.000000000 +0200
@@ -0,0 +1,9 @@
+	.globl	w1, w2, w3, l1, l2
+	.data
+l1:	.long	0
+	.struct 1
+w1:	.short	0
+w2:	.short	0
+w3:	.short	0
+	.previous
+l2:	.long	0



[-- Attachment #2: binutils-mainline-elf-struct.patch --]
[-- Type: text/plain, Size: 2760 bytes --]

struct did not interact correctly with ELF's .previous.

Built and tested on i686-pc-linux-gnu.

Jan

gas/
2005-04-13  Jan Beulich  <jbeulich@novell.com>

	* config/obj-elf.c (obj_elf_struct): New.
	(elf_pseudo_table). Use it for .offset and .struct.

gas/testsuite/
2005-04-13  Jan Beulich  <jbeulich@novell.com>

	* gas/elf/struct.[sd]: New.
	* gas/elf/elf.exp: Run new test.

--- /home/jbeulich/src/binutils/mainline/2005-04-11/gas/config/obj-elf.c	2005-02-28 08:27:13.000000000 +0100
+++ 2005-04-11/gas/config/obj-elf.c	2005-04-13 15:25:24.000000000 +0200
@@ -65,6 +65,7 @@ static void obj_elf_subsection (int);
 static void obj_elf_popsection (int);
 static void obj_elf_tls_common (int);
 static void obj_elf_lcomm (int);
+static void obj_elf_struct (int);
 
 static const pseudo_typeS elf_pseudo_table[] =
 {
@@ -113,6 +114,8 @@ static const pseudo_typeS elf_pseudo_tab
 
   /* We need to trap the section changing calls to handle .previous.  */
   {"data", obj_elf_data, 0},
+  {"offset", obj_elf_struct, 0},
+  {"struct", obj_elf_struct, 0},
   {"text", obj_elf_text, 0},
 
   {"tls_common", obj_elf_tls_common, 0},
@@ -1036,6 +1039,24 @@ obj_elf_text (int i)
 #endif
 }
 
+/* Change to the *ABS* section.  */
+
+void
+obj_elf_struct (int i)
+{
+#ifdef md_flush_pending_output
+  md_flush_pending_output ();
+#endif
+
+  previous_section = now_seg;
+  previous_subsection = now_subseg;
+  s_struct (i);
+
+#ifdef md_elf_section_change_hook
+  md_elf_section_change_hook ();
+#endif
+}
+
 static void
 obj_elf_subsection (int ignore ATTRIBUTE_UNUSED)
 {
--- /home/jbeulich/src/binutils/mainline/2005-04-11/gas/testsuite/gas/elf/elf.exp	2005-01-18 10:43:34.000000000 +0100
+++ 2005-04-11/gas/testsuite/gas/elf/elf.exp	2005-04-13 15:26:20.000000000 +0200
@@ -61,6 +61,7 @@ if { ([istarget "*-*-*elf*"]		
     run_dump_test "section3" 
     run_dump_test "section4"
     run_list_test "section5" "" "-al" "-SW" "| grep \" \\\\.test\\\[0-9\\\]\""
+    run_dump_test "struct" 
     run_dump_test "symver" 
     run_list_test "type" "" "" "-s" "| grep \"1 \\\[FONT\\\]\""
 }
--- /home/jbeulich/src/binutils/mainline/2005-04-11/gas/testsuite/gas/elf/struct.d	1970-01-01 01:00:00.000000000 +0100
+++ 2005-04-11/gas/testsuite/gas/elf/struct.d	2005-04-12 11:48:51.000000000 +0200
@@ -0,0 +1,10 @@
+#nm: --extern-only
+#name: struct
+
+# Test the .struct pseudo-op.
+
+0+0 D l1
+0+4 D l2
+0+1 A w1
+0+3 A w2
+0+5 A w3
--- /home/jbeulich/src/binutils/mainline/2005-04-11/gas/testsuite/gas/elf/struct.s	1970-01-01 01:00:00.000000000 +0100
+++ 2005-04-11/gas/testsuite/gas/elf/struct.s	2005-04-12 11:45:06.000000000 +0200
@@ -0,0 +1,9 @@
+	.globl	w1, w2, w3, l1, l2
+	.data
+l1:	.long	0
+	.struct 1
+w1:	.short	0
+w2:	.short	0
+w3:	.short	0
+	.previous
+l2:	.long	0

Re: [PATCH] ELF: make .struct interact with .previous

[Nick Clifton]

Hi Jan,

> gas/
> 2005-04-13  Jan Beulich  <jbeulich@novell.com>
> 
> 	* config/obj-elf.c (obj_elf_struct): New.
> 	(elf_pseudo_table). Use it for .offset and .struct.
> 
> gas/testsuite/
> 2005-04-13  Jan Beulich  <jbeulich@novell.com>
> 
> 	* gas/elf/struct.[sd]: New.
> 	* gas/elf/elf.exp: Run new test.

Approved - please apply.

Cheers
   Nick

Re: [PATCH] ELF: make .struct interact with .previous

[H. J. Lu]

On Fri, Apr 15, 2005 at 11:51:11AM +0100, Nick Clifton wrote:
> Hi Jan,
> 
> >gas/
> >2005-04-13  Jan Beulich  <jbeulich@novell.com>
> >
> >	* config/obj-elf.c (obj_elf_struct): New.
> >	(elf_pseudo_table). Use it for .offset and .struct.
> >
> >gas/testsuite/
> >2005-04-13  Jan Beulich  <jbeulich@novell.com>
> >
> >	* gas/elf/struct.[sd]: New.
> >	* gas/elf/elf.exp: Run new test.
> 
> Approved - please apply.
> 

This test failed on Linux/ia64:

/net/gnu/export/linux/src/binutils/binutils/gas/testsuite/lib/run
../as-new   -o dump.o
/net/gnu/export/linux/src/binutils/binutils/gas/testsuite/gas/elf/struct.s
/export/build/linux/binutils-debug/build-ia64-linux/gas/testsuite/../../binutils/nm-new
--extern-only dump.o > dump.out
regexp_diff match failure
regexp "^0+3 A w2$"
line   "0000000000000004 A w2"
regexp_diff match failure
regexp "^0+5 A w3$" line   "0000000000000006 A w3"
FAIL: struct

H.J.

Re: [PATCH] ELF: make .struct interact with .previous

[H. J. Lu]

On Fri, Apr 15, 2005 at 10:31:12AM -0700, H. J. Lu wrote:
> On Fri, Apr 15, 2005 at 11:51:11AM +0100, Nick Clifton wrote:
> > Hi Jan,
> > 
> > >gas/
> > >2005-04-13  Jan Beulich  <jbeulich@novell.com>
> > >
> > >	* config/obj-elf.c (obj_elf_struct): New.
> > >	(elf_pseudo_table). Use it for .offset and .struct.
> > >
> > >gas/testsuite/
> > >2005-04-13  Jan Beulich  <jbeulich@novell.com>
> > >
> > >	* gas/elf/struct.[sd]: New.
> > >	* gas/elf/elf.exp: Run new test.
> > 
> > Approved - please apply.
> > 
> 
> This test failed on Linux/ia64:
> 
> /net/gnu/export/linux/src/binutils/binutils/gas/testsuite/lib/run
> ../as-new   -o dump.o
> /net/gnu/export/linux/src/binutils/binutils/gas/testsuite/gas/elf/struct.s
> /export/build/linux/binutils-debug/build-ia64-linux/gas/testsuite/../../binutils/nm-new
> --extern-only dump.o > dump.out
> regexp_diff match failure
> regexp "^0+3 A w2$"
> line   "0000000000000004 A w2"
> regexp_diff match failure
> regexp "^0+5 A w3$" line   "0000000000000006 A w3"
> FAIL: struct
> 

I don't think we should call md_cons_align for ABS section. This patch
fixes the failure.


H.J.
----
2005-04-15  H.J. Lu  <hongjiu.lu@intel.com>

	* read.c (cons_worker): Don't call md_cons_align for ABS
	section.

--- gas/read.c.abs	2005-04-11 08:54:47.000000000 -0700
+++ gas/read.c	2005-04-15 11:30:32.000000000 -0700
@@ -3411,7 +3411,8 @@ cons_worker (register int nbytes,	/* 1=.
 #endif
 
 #ifdef md_cons_align
-  md_cons_align (nbytes);
+  if (now_seg != absolute_section)
+    md_cons_align (nbytes);
 #endif
 
   c = 0;

Re: [PATCH] ELF: make .struct interact with .previous

[Alan Modra]

On Fri, Apr 15, 2005 at 11:37:18AM -0700, H. J. Lu wrote:
> I don't think we should call md_cons_align for ABS section. This patch
> fixes the failure.

Why not?  If you need to do something special for the absolute section,
then md_cons_align itself should handle that.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [PATCH] ELF: make .struct interact with .previous

[H. J. Lu]

On Sat, Apr 16, 2005 at 07:30:16AM +0930, Alan Modra wrote:
> On Fri, Apr 15, 2005 at 11:37:18AM -0700, H. J. Lu wrote:
> > I don't think we should call md_cons_align for ABS section. This patch
> > fixes the failure.
> 
> Why not?  If you need to do something special for the absolute section,
> then md_cons_align itself should handle that.

What special alignment can a backend can do for the absolute section?


H.J.

Re: [PATCH] ELF: make .struct interact with .previous

[H. J. Lu]

On Fri, Apr 15, 2005 at 05:36:47PM -0700, H. J. Lu wrote:
> On Sat, Apr 16, 2005 at 07:30:16AM +0930, Alan Modra wrote:
> > On Fri, Apr 15, 2005 at 11:37:18AM -0700, H. J. Lu wrote:
> > > I don't think we should call md_cons_align for ABS section. This patch
> > > fixes the failure.
> > 
> > Why not?  If you need to do something special for the absolute section,
> > then md_cons_align itself should handle that.
> 
> What special alignment can a backend can do for the absolute section?
> 

There are

@item md_cons_align
@cindex md_cons_align
You may define this macro to do any special alignment before a data
allocation pseudo-op.

But there is no data allocation involved for the absolute section.


H.J.

Re: [PATCH] ELF: make .struct interact with .previous

[Alan Modra]

On Fri, Apr 15, 2005 at 05:43:57PM -0700, H. J. Lu wrote:
> On Fri, Apr 15, 2005 at 05:36:47PM -0700, H. J. Lu wrote:
> > On Sat, Apr 16, 2005 at 07:30:16AM +0930, Alan Modra wrote:
> > > On Fri, Apr 15, 2005 at 11:37:18AM -0700, H. J. Lu wrote:
> > > > I don't think we should call md_cons_align for ABS section. This patch
> > > > fixes the failure.
> > > 
> > > Why not?  If you need to do something special for the absolute section,
> > > then md_cons_align itself should handle that.
> > 
> > What special alignment can a backend can do for the absolute section?
> > 
> 
> There are
> 
> @item md_cons_align
> @cindex md_cons_align
> You may define this macro to do any special alignment before a data
> allocation pseudo-op.
> 
> But there is no data allocation involved for the absolute section.

The struct test is using a .short, which looks like a data allocation
pseudo-op to me.  If .short should have different alignment when you
happen to use it in the absolute section, then md_cons_align should
reflect that fact.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [PATCH] ELF: make .struct interact with .previous

[H. J. Lu]

On Mon, Apr 18, 2005 at 09:04:05AM +0930, Alan Modra wrote:
> On Fri, Apr 15, 2005 at 05:43:57PM -0700, H. J. Lu wrote:
> > On Fri, Apr 15, 2005 at 05:36:47PM -0700, H. J. Lu wrote:
> > > On Sat, Apr 16, 2005 at 07:30:16AM +0930, Alan Modra wrote:
> > > > On Fri, Apr 15, 2005 at 11:37:18AM -0700, H. J. Lu wrote:
> > > > > I don't think we should call md_cons_align for ABS section. This patch
> > > > > fixes the failure.
> > > > 
> > > > Why not?  If you need to do something special for the absolute section,
> > > > then md_cons_align itself should handle that.
> > > 
> > > What special alignment can a backend can do for the absolute section?
> > > 
> > 
> > There are
> > 
> > @item md_cons_align
> > @cindex md_cons_align
> > You may define this macro to do any special alignment before a data
> > allocation pseudo-op.
> > 
> > But there is no data allocation involved for the absolute section.
> 
> The struct test is using a .short, which looks like a data allocation
> pseudo-op to me.  If .short should have different alignment when you
> happen to use it in the absolute section, then md_cons_align should
> reflect that fact.
> 

I guess it boilds down to what .short means in the absolute section.
Do any other assemblers support it? If yes, what do they do? If not,
gas should document what .short should do in the absolute section.


H.J.


H.J.