[GAS][ARM]Positively emit symbols for alignment.

[GAS][ARM]Positively emit symbols for alignment.

[Renlin Li]

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

Hi all,

In arm_init_frag(), when ARM or THUMB are recorded,  it will simply 
return without doing anything. This is not correct in a few cases.

For the following two cases:
case 1:
         nop        ----> MAP_ARM
         .long   0 ---> MAP_DATA
         .align  4 --->
                       ---->(1)
                       ----> start new frag
         .word   0x12345678

(1) paddings here are all treated as data, because the previous state is 
MAP_DATA.

case 2:
         nop       ----> MAP_ARM
         .long   0 ---> MAP_DATA
         .byte   1
         .align  2 ---> (1)
                      ----> (2)
                      ----> start new frag
         .long   0x12345678 ----> (3)

stage 1: while parsing the input file:
(1) arm_init_frag early returns, No state change, no new symbol. (3)No 
state change, no new symbol.
stage 2: Later, while writing the object file:
(2)MAP_DATA, MAP_ARM are inserted by insert_data_mapping_symol to handle 
alignment. (3) interpreted as instruction because of the MAP_ARM 
inserted at (2)


This change might generate redundant symbols, some of them can be 
removed later. In check_mapping_symbols(), the symbols at the end of a 
section, and overlapping symbols will be removed.

gas checked without any new issues. Okay to commit?

By the way, Nick, aarch64 has the same issue. The same can be done to 
aarch64_init_frag(). Another way is to remove 
fragP->tc_frag_data.recorded. It's only used in a few places. But I am 
sure what dose it intended to do, Could you please explain a little bit 
for me?

Regards,
Renlin Li


gas/ChangeLog:

2015-04-24  Renlin Li  <renlin.li@arm.com>

     * config/tc-arm.c (arm_init_frag): Always emit mapping symbols.

gas/testsuite/ChangeLog:

2015-04-24  Renlin Li  <renlin.li@arm.com>

     * gas/arm/thumb2_vpool_be.d: Adjust the desired output.
     * gas/arm/vldconst_be.d: Ditto.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 3.diff --]
[-- Type: text/x-patch; name=3.diff, Size: 4066 bytes --]

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index b17ea6c..5fa2790 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -21015,24 +21015,22 @@ arm_init_frag (fragS * fragP, int max_chars)
   /* If the current ARM vs THUMB mode has not already
      been recorded into this frag then do so now.  */
   if ((fragP->tc_frag_data.thumb_mode & MODE_RECORDED) == 0)
-    {
       fragP->tc_frag_data.thumb_mode = thumb_mode | MODE_RECORDED;
 
-      /* Record a mapping symbol for alignment frags.  We will delete this
-	 later if the alignment ends up empty.  */
-      switch (fragP->fr_type)
-	{
-	  case rs_align:
-	  case rs_align_test:
-	  case rs_fill:
-	    mapping_state_2 (MAP_DATA, max_chars);
-	    break;
-	  case rs_align_code:
-	    mapping_state_2 (thumb_mode ? MAP_THUMB : MAP_ARM, max_chars);
-	    break;
-	  default:
-	    break;
-	}
+  /* Record a mapping symbol for alignment frags.  We will delete this
+     later if the alignment ends up empty.  */
+  switch (fragP->fr_type)
+    {
+    case rs_align:
+    case rs_align_test:
+    case rs_fill:
+      mapping_state_2 (MAP_DATA, max_chars);
+      break;
+    case rs_align_code:
+      mapping_state_2 (thumb_mode ? MAP_THUMB : MAP_ARM, max_chars);
+      break;
+    default:
+      break;
     }
 }
 
diff --git a/gas/testsuite/gas/arm/thumb2_vpool_be.d b/gas/testsuite/gas/arm/thumb2_vpool_be.d
index 816cddc..15aafa5 100644
--- a/gas/testsuite/gas/arm/thumb2_vpool_be.d
+++ b/gas/testsuite/gas/arm/thumb2_vpool_be.d
@@ -125,8 +125,7 @@ Disassembly of section .text:
 000001c8 <thumb2_ldr\+0x1c8> 0ff00fff 	.word	0x0ff00fff
 000001cc <thumb2_ldr\+0x1cc> f0000000 	.word	0xf0000000
 000001d0 <thumb2_ldr\+0x1d0> ed9f 1b01 	vldr	d1, \[pc, #4\]	; 000001d8 <thumb2_ldr\+0x1d8>
-000001d4 <thumb2_ldr\+0x1d4> 0000      	movs	r0, r0
-000001d6 <thumb2_ldr\+0x1d6> 0000      	movs	r0, r0
+000001d4 <thumb2_ldr\+0x1d4> 00000000 	.word	0x00000000
 000001d8 <thumb2_ldr\+0x1d8> 0000fff0 	.word	0x0000fff0
 000001dc <thumb2_ldr\+0x1dc> 00000000 	.word	0x00000000
 000001e0 <thumb2_ldr\+0x1e0> f101 0000 	add.w	r0, r1, #0
@@ -150,8 +149,7 @@ Disassembly of section .text:
 00000228 <thumb2_ldr\+0x228> eddf 7a03 	vldr	s15, \[pc, #12\]	; 00000238 <thumb2_ldr\+0x238>
 0000022c <thumb2_ldr\+0x22c> eddf 0b14 	vldr	d16, \[pc, #80\]	; 00000280 <thumb2_ldr\+0x280>
 00000230 <thumb2_ldr\+0x230> eddf 1b15 	vldr	d17, \[pc, #84\]	; 00000288 <thumb2_ldr\+0x288>
-00000234 <thumb2_ldr\+0x234> 0000      	movs	r0, r0
-00000236 <thumb2_ldr\+0x236> 0000      	movs	r0, r0
+00000234 <thumb2_ldr\+0x234> 00000000 	.word	0x00000000
 00000238 <thumb2_ldr\+0x238> 0000fff0 	.word	0x0000fff0
 0000023c <thumb2_ldr\+0x23c> 00000000 	.word	0x00000000
 00000240 <thumb2_ldr\+0x240> ff000000 	.word	0xff000000
diff --git a/gas/testsuite/gas/arm/vldconst_be.d b/gas/testsuite/gas/arm/vldconst_be.d
index cf3dbf9..f99371b 100644
--- a/gas/testsuite/gas/arm/vldconst_be.d
+++ b/gas/testsuite/gas/arm/vldconst_be.d
@@ -236,7 +236,7 @@ Disassembly of section .text:
 00000388 <foo\+0x388> 0000fff0 	.word	0x0000fff0
 0000038c <foo\+0x38c> 00000000 	.word	0x00000000
 00000390 <foo\+0x390> ed9f1b00 	vldr	d1, \[pc\]	; 00000398 <foo\+0x398>
-00000394 <foo\+0x394> 00000000 	andeq	r0, r0, r0
+00000394 <foo\+0x394> 00000000 	.word	0x00000000
 00000398 <foo\+0x398> 0000fff0 	.word	0x0000fff0
 0000039c <foo\+0x39c> 00000000 	.word	0x00000000
 000003a0 <foo\+0x3a0> e2810000 	add	r0, r1, #0
@@ -260,7 +260,7 @@ Disassembly of section .text:
 000003e8 <foo\+0x3e8> eddf7a02 	vldr	s15, \[pc, #8\]	; 000003f8 <foo\+0x3f8>
 000003ec <foo\+0x3ec> eddf0b13 	vldr	d16, \[pc, #76\]	; 00000440 <foo\+0x440>
 000003f0 <foo\+0x3f0> eddf1b14 	vldr	d17, \[pc, #80\]	; 00000448 <foo\+0x448>
-000003f4 <foo\+0x3f4> 00000000 	andeq	r0, r0, r0
+000003f4 <foo\+0x3f4> 00000000 	.word	0x00000000
 000003f8 <foo\+0x3f8> 0000fff0 	.word	0x0000fff0
 000003fc <foo\+0x3fc> 00000000 	.word	0x00000000
 00000400 <foo\+0x400> ff000000 	.word	0xff000000

Re: [GAS][ARM]Positively emit symbols for alignment.

[Nicholas Clifton]

Hi Renlin,

> gas checked without any new issues. Okay to commit?

Yes. please do.

> By the way, Nick, aarch64 has the same issue. The same can be done to
> aarch64_init_frag(). Another way is to remove
> fragP->tc_frag_data.recorded. It's only used in a few places. But I am
> sure what dose it intended to do, Could you please explain a little bit
> for me?

I think that it is meant to be the equivalent of the MODE_RECORDED bit 
used in tc-arm.c.

A patch to fix tc-aarch64.c in a similar way to tc-arm.c is pre-approved.

Cheers
   Nick

Re: [GAS][ARM]Positively emit symbols for alignment.

[Hans-Peter Nilsson]

> From: Renlin Li <renlin.li@arm.com>
> Date: Fri, 24 Apr 2015 14:53:01 +0200

> gas checked without any new issues.

(I hope you tested binutils and ld too, but...)

>      * config/tc-arm.c (arm_init_frag): Always emit mapping symbols.

Maybe you refer to some other target (which one?) than
arm-unknown-eabi, because for arm-unknown-eabi, this patch
caused failures in the ld testsuite:

Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-arm/arm-elf.exp ...
FAIL: IFUNC test 2
FAIL: IFUNC test 10

I'm guessing you just need to adjust the
expected-result-templates for the "mapping symbols".

brgds, H-P

Re: [GAS][ARM]Positively emit symbols for alignment. --> [PATCH][LD][ARM] Fix two test case failures.

[Renlin Li]

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

Hi Hans-Peter,

Thank you for your report! I have attached the patch to fix the test cases.

Previously, the padding bytes are treated as instruction, because data 
mapping symbols are missing. With the new patch, data mapping symbols 
are emitted, the bytes thereafter are correctly interpreted as data.

Okay to commit?

Regards,
Renlin Li

ld/testsuite/ChangeLog:

2015-04-29  Renlin Li  <renlin.li@arm.com>

     * ld-arm/ifunc-10.dd: Adjust expected output.
     * ld-arm/ifunc-2.dd: Likewise.

On 28/04/15 22:07, Hans-Peter Nilsson wrote:
> Maybe you refer to some other target (which one?) than
> arm-unknown-eabi, because for arm-unknown-eabi, this patch
> caused failures in the ld testsuite:
>
> Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-arm/arm-elf.exp ...
> FAIL: IFUNC test 2
> FAIL: IFUNC test 10
>
> I'm guessing you just need to adjust the
> expected-result-templates for the "mapping symbols".
>
> brgds, H-P
>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ld.diff --]
[-- Type: text/x-patch; name=ld.diff, Size: 1047 bytes --]

diff --git a/ld/testsuite/ld-arm/ifunc-10.dd b/ld/testsuite/ld-arm/ifunc-10.dd
index 88bae50..d96c086 100644
--- a/ld/testsuite/ld-arm/ifunc-10.dd
+++ b/ld/testsuite/ld-arm/ifunc-10.dd
@@ -267,7 +267,7 @@ Disassembly of section \.text:
 
 0000a010 <tbf1>:
     a010:	46f7      	mov	pc, lr
-    a012:	0000      	movs	r0, r0
+    a012:	0000      	.short	0x0000
     a014:	e1a0f00e 	mov	pc, lr
     a018:	e1a0f00e 	mov	pc, lr
     a01c:	e1a0f00e 	mov	pc, lr
diff --git a/ld/testsuite/ld-arm/ifunc-2.dd b/ld/testsuite/ld-arm/ifunc-2.dd
index f6d57fa..91eab54 100644
--- a/ld/testsuite/ld-arm/ifunc-2.dd
+++ b/ld/testsuite/ld-arm/ifunc-2.dd
@@ -84,10 +84,10 @@ Disassembly of section \.text:
 
 0000a00c <f4>:
     a00c:	46f7      	mov	pc, lr
-    a00e:	0000      	movs	r0, r0
+    a00e:	0000      	.short	0x0000
     a010:	e1a0f00e 	mov	pc, lr
     a014:	46f7      	mov	pc, lr
-    a016:	0000      	movs	r0, r0
+    a016:	0000      	.short	0x0000
     a018:	e1a0f00e 	mov	pc, lr
     a01c:	46f7      	mov	pc, lr
 	\.\.\.

Re: [GAS][ARM]Positively emit symbols for alignment. --> [PATCH][LD][ARM] Fix two test case failures.

[Richard Earnshaw]

On 29/04/15 10:21, Renlin Li wrote:
> Hi Hans-Peter,
> 
> Thank you for your report! I have attached the patch to fix the test cases.
> 
> Previously, the padding bytes are treated as instruction, because data 
> mapping symbols are missing. With the new patch, data mapping symbols 
> are emitted, the bytes thereafter are correctly interpreted as data.
> 
> Okay to commit?
> 

OK.

R.

> Regards,
> Renlin Li
> 
> ld/testsuite/ChangeLog:
> 
> 2015-04-29  Renlin Li  <renlin.li@arm.com>
> 
>      * ld-arm/ifunc-10.dd: Adjust expected output.
>      * ld-arm/ifunc-2.dd: Likewise.
> 
> On 28/04/15 22:07, Hans-Peter Nilsson wrote:
>> Maybe you refer to some other target (which one?) than
>> arm-unknown-eabi, because for arm-unknown-eabi, this patch
>> caused failures in the ld testsuite:
>>
>> Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-arm/arm-elf.exp ...
>> FAIL: IFUNC test 2
>> FAIL: IFUNC test 10
>>
>> I'm guessing you just need to adjust the
>> expected-result-templates for the "mapping symbols".
>>
>> brgds, H-P
>>
>>
>> ld.diff
>>
>>
>> diff --git a/ld/testsuite/ld-arm/ifunc-10.dd b/ld/testsuite/ld-arm/ifunc-10.dd
>> index 88bae50..d96c086 100644
>> --- a/ld/testsuite/ld-arm/ifunc-10.dd
>> +++ b/ld/testsuite/ld-arm/ifunc-10.dd
>> @@ -267,7 +267,7 @@ Disassembly of section \.text:
>>  
>>  0000a010 <tbf1>:
>>      a010:	46f7      	mov	pc, lr
>> -    a012:	0000      	movs	r0, r0
>> +    a012:	0000      	.short	0x0000
>>      a014:	e1a0f00e 	mov	pc, lr
>>      a018:	e1a0f00e 	mov	pc, lr
>>      a01c:	e1a0f00e 	mov	pc, lr
>> diff --git a/ld/testsuite/ld-arm/ifunc-2.dd b/ld/testsuite/ld-arm/ifunc-2.dd
>> index f6d57fa..91eab54 100644
>> --- a/ld/testsuite/ld-arm/ifunc-2.dd
>> +++ b/ld/testsuite/ld-arm/ifunc-2.dd
>> @@ -84,10 +84,10 @@ Disassembly of section \.text:
>>  
>>  0000a00c <f4>:
>>      a00c:	46f7      	mov	pc, lr
>> -    a00e:	0000      	movs	r0, r0
>> +    a00e:	0000      	.short	0x0000
>>      a010:	e1a0f00e 	mov	pc, lr
>>      a014:	46f7      	mov	pc, lr
>> -    a016:	0000      	movs	r0, r0
>> +    a016:	0000      	.short	0x0000
>>      a018:	e1a0f00e 	mov	pc, lr
>>      a01c:	46f7      	mov	pc, lr
>>  	\.\.\.

Commit: [GAS][AARCH64]Positively emit symbols for alignment.

[Renlin Li]

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

Hi Nick,

On 24/04/15 17:40, Nicholas Clifton wrote:
> Hi Renlin,
>
>> gas checked without any new issues. Okay to commit?
> Yes. please do.
>
>> By the way, Nick, aarch64 has the same issue. The same can be done to
>> aarch64_init_frag(). Another way is to remove
>> fragP->tc_frag_data.recorded. It's only used in a few places. But I am
>> sure what dose it intended to do, Could you please explain a little bit
>> for me?
> I think that it is meant to be the equivalent of the MODE_RECORDED bit
> used in tc-arm.c.
>
> A patch to fix tc-aarch64.c in a similar way to tc-arm.c is pre-approved.

A similar patch (check the attachment) applies to aarch64 is committed.

Binutils, gas, ld checked without new issues.

Thank you,
Renlin Li


2015-05-05  Renlin Li  <renlin.li@arm.com>

         * config/tc-aarch64.c (aarch64_init_frag): Always generate 
mapping symbols.

gas/testsuite/ChangeLog:

2015-05-05  Renlin Li  <renlin.li@arm.com>

         * gas/aarch64/mapping_5.d: New.
         * gas/aarch64/mapping_5.s: New.
         * gas/aarch64/mapping_6.d: New.
         * gas/aarch64/mapping_6.s: New.

>
> Cheers
>     Nick
>



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-6.diff --]
[-- Type: text/x-patch; name=aarch64-6.diff, Size: 2796 bytes --]

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 646deb6..12012de 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -6028,21 +6028,20 @@ aarch64_init_frag (fragS * fragP, int max_chars)
   /* Record a mapping symbol for alignment frags.  We will delete this
      later if the alignment ends up empty.  */
   if (!fragP->tc_frag_data.recorded)
+    fragP->tc_frag_data.recorded = 1;
+
+  switch (fragP->fr_type)
     {
-      fragP->tc_frag_data.recorded = 1;
-      switch (fragP->fr_type)
-	{
-	case rs_align:
-	case rs_align_test:
-	case rs_fill:
-	  mapping_state_2 (MAP_DATA, max_chars);
-	  break;
-	case rs_align_code:
-	  mapping_state_2 (MAP_INSN, max_chars);
-	  break;
-	default:
-	  break;
-	}
+    case rs_align:
+    case rs_align_test:
+    case rs_fill:
+      mapping_state_2 (MAP_DATA, max_chars);
+      break;
+    case rs_align_code:
+      mapping_state_2 (MAP_INSN, max_chars);
+      break;
+    default:
+      break;
     }
 }
 \f
diff --git a/gas/testsuite/gas/aarch64/mapping_5.d b/gas/testsuite/gas/aarch64/mapping_5.d
new file mode 100644
index 0000000..4778b3d
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mapping_5.d
@@ -0,0 +1,13 @@
+#objdump: --syms --special-syms
+#name: AArch64 Mapping Symbols Test 5
+
+.*: +file format.*aarch64.*
+
+SYMBOL TABLE:
+[0]+00 l    d  .text	[0]+00 .text
+[0]+00 l    d  .data	[0]+00 .data
+[0]+00 l    d  .bss	[0]+00 .bss
+[0]+00 l       .text	[0]+00 \$x
+[0]+04 l       .text	[0]+00 \$d
+[0]+08 l       .text	[0]+00 \$x
+[0]+10 l       .text	[0]+00 \$d
diff --git a/gas/testsuite/gas/aarch64/mapping_5.s b/gas/testsuite/gas/aarch64/mapping_5.s
new file mode 100644
index 0000000..2cb95ed
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mapping_5.s
@@ -0,0 +1,5 @@
+	.text
+	nop
+	.long	0
+	.align	4
+	.word	0x12345678
diff --git a/gas/testsuite/gas/aarch64/mapping_6.d b/gas/testsuite/gas/aarch64/mapping_6.d
new file mode 100644
index 0000000..3a5dfb8
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mapping_6.d
@@ -0,0 +1,14 @@
+#objdump: --syms --special-syms
+#name: AArch64 Mapping Symbols Test 6
+
+.*: +file format.*aarch64.*
+
+SYMBOL TABLE:
+[0]+00 l    d  .text	[0]+00 .text
+[0]+00 l    d  .data	[0]+00 .data
+[0]+00 l    d  .bss	[0]+00 .bss
+[0]+00 l       .text	[0]+00 \$x
+[0]+04 l       .text	[0]+00 \$d
+[0]+10 l       .text	[0]+00 \$d
+[0]+09 l       .text	[0]+00 \$d
+[0]+0c l       .text	[0]+00 \$x
diff --git a/gas/testsuite/gas/aarch64/mapping_6.s b/gas/testsuite/gas/aarch64/mapping_6.s
new file mode 100644
index 0000000..4e0e2c5
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mapping_6.s
@@ -0,0 +1,6 @@
+	.text
+	nop
+	.long	0
+	.byte	1
+	.align	4
+	.word	0x12345678

[2.25][ARM][GAS]Backport "Positively emit symbols for alignment."

[Renlin Li]

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

Hi all,

This is a merged backport patch from the following three. They together 
fix one problem, So I merge them into one backport patch. The 
description of problem is quoted below.

The initial patch:
https://sourceware.org/ml/binutils/2015-04/msg00381.html

Fix two failing test case:
https://sourceware.org/ml/binutils/2015-04/msg00457.html

Fix a bug in the initial patch:
https://sourceware.org/ml/binutils/2015-06/msg00045.html


Binutils, ld, gas regression test Okay. Okay to commit?

On 24/04/15 13:53, Renlin Li wrote:
> Hi all,
>
> In arm_init_frag(), when ARM or THUMB are recorded,  it will simply
> return without doing anything. This is not correct in a few cases.
>
> For the following two cases:
> case 1:
>           nop        ----> MAP_ARM
>           .long   0 ---> MAP_DATA
>           .align  4 --->
>                         ---->(1)
>                         ----> start new frag
>           .word   0x12345678
>
> (1) paddings here are all treated as data, because the previous state is
> MAP_DATA.
>
> case 2:
>           nop       ----> MAP_ARM
>           .long   0 ---> MAP_DATA
>           .byte   1
>           .align  2 ---> (1)
>                        ----> (2)
>                        ----> start new frag
>           .long   0x12345678 ----> (3)
>
> stage 1: while parsing the input file:
> (1) arm_init_frag early returns, No state change, no new symbol. (3)No
> state change, no new symbol.
> stage 2: Later, while writing the object file:
> (2)MAP_DATA, MAP_ARM are inserted by insert_data_mapping_symol to handle
> alignment. (3) interpreted as instruction because of the MAP_ARM
> inserted at (2)
>
>
> This change might generate redundant symbols, some of them can be
> removed later. In check_mapping_symbols(), the symbols at the end of a
> section, and overlapping symbols will be removed.
>
> gas checked without any new issues. Okay to commit?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: arm-backport.diff --]
[-- Type: text/x-patch; name=arm-backport.diff, Size: 5361 bytes --]

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index f631a02..1714ec8 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -20970,27 +20970,29 @@ arm_init_frag (fragS * fragP, int max_chars ATTRIBUTE_UNUSED)
 void
 arm_init_frag (fragS * fragP, int max_chars)
 {
+  int frag_thumb_mode;
   /* If the current ARM vs THUMB mode has not already
      been recorded into this frag then do so now.  */
   if ((fragP->tc_frag_data.thumb_mode & MODE_RECORDED) == 0)
-    {
-      fragP->tc_frag_data.thumb_mode = thumb_mode | MODE_RECORDED;
+    fragP->tc_frag_data.thumb_mode = thumb_mode | MODE_RECORDED;
 
-      /* Record a mapping symbol for alignment frags.  We will delete this
-	 later if the alignment ends up empty.  */
-      switch (fragP->fr_type)
-	{
-	  case rs_align:
-	  case rs_align_test:
-	  case rs_fill:
-	    mapping_state_2 (MAP_DATA, max_chars);
-	    break;
-	  case rs_align_code:
-	    mapping_state_2 (thumb_mode ? MAP_THUMB : MAP_ARM, max_chars);
-	    break;
-	  default:
-	    break;
-	}
+  frag_thumb_mode = fragP->tc_frag_data.thumb_mode ^ MODE_RECORDED;
+
+
+  /* Record a mapping symbol for alignment frags.  We will delete this
+     later if the alignment ends up empty.  */
+  switch (fragP->fr_type)
+    {
+    case rs_align:
+    case rs_align_test:
+    case rs_fill:
+      mapping_state_2 (MAP_DATA, max_chars);
+      break;
+    case rs_align_code:
+      mapping_state_2 (frag_thumb_mode ? MAP_THUMB : MAP_ARM, max_chars);
+      break;
+    default:
+      break;
     }
 }
 
diff --git a/gas/testsuite/gas/arm/thumb2_vpool_be.d b/gas/testsuite/gas/arm/thumb2_vpool_be.d
index 816cddc..15aafa5 100644
--- a/gas/testsuite/gas/arm/thumb2_vpool_be.d
+++ b/gas/testsuite/gas/arm/thumb2_vpool_be.d
@@ -125,8 +125,7 @@ Disassembly of section .text:
 000001c8 <thumb2_ldr\+0x1c8> 0ff00fff 	.word	0x0ff00fff
 000001cc <thumb2_ldr\+0x1cc> f0000000 	.word	0xf0000000
 000001d0 <thumb2_ldr\+0x1d0> ed9f 1b01 	vldr	d1, \[pc, #4\]	; 000001d8 <thumb2_ldr\+0x1d8>
-000001d4 <thumb2_ldr\+0x1d4> 0000      	movs	r0, r0
-000001d6 <thumb2_ldr\+0x1d6> 0000      	movs	r0, r0
+000001d4 <thumb2_ldr\+0x1d4> 00000000 	.word	0x00000000
 000001d8 <thumb2_ldr\+0x1d8> 0000fff0 	.word	0x0000fff0
 000001dc <thumb2_ldr\+0x1dc> 00000000 	.word	0x00000000
 000001e0 <thumb2_ldr\+0x1e0> f101 0000 	add.w	r0, r1, #0
@@ -150,8 +149,7 @@ Disassembly of section .text:
 00000228 <thumb2_ldr\+0x228> eddf 7a03 	vldr	s15, \[pc, #12\]	; 00000238 <thumb2_ldr\+0x238>
 0000022c <thumb2_ldr\+0x22c> eddf 0b14 	vldr	d16, \[pc, #80\]	; 00000280 <thumb2_ldr\+0x280>
 00000230 <thumb2_ldr\+0x230> eddf 1b15 	vldr	d17, \[pc, #84\]	; 00000288 <thumb2_ldr\+0x288>
-00000234 <thumb2_ldr\+0x234> 0000      	movs	r0, r0
-00000236 <thumb2_ldr\+0x236> 0000      	movs	r0, r0
+00000234 <thumb2_ldr\+0x234> 00000000 	.word	0x00000000
 00000238 <thumb2_ldr\+0x238> 0000fff0 	.word	0x0000fff0
 0000023c <thumb2_ldr\+0x23c> 00000000 	.word	0x00000000
 00000240 <thumb2_ldr\+0x240> ff000000 	.word	0xff000000
diff --git a/gas/testsuite/gas/arm/vldconst_be.d b/gas/testsuite/gas/arm/vldconst_be.d
index cf3dbf9..f99371b 100644
--- a/gas/testsuite/gas/arm/vldconst_be.d
+++ b/gas/testsuite/gas/arm/vldconst_be.d
@@ -236,7 +236,7 @@ Disassembly of section .text:
 00000388 <foo\+0x388> 0000fff0 	.word	0x0000fff0
 0000038c <foo\+0x38c> 00000000 	.word	0x00000000
 00000390 <foo\+0x390> ed9f1b00 	vldr	d1, \[pc\]	; 00000398 <foo\+0x398>
-00000394 <foo\+0x394> 00000000 	andeq	r0, r0, r0
+00000394 <foo\+0x394> 00000000 	.word	0x00000000
 00000398 <foo\+0x398> 0000fff0 	.word	0x0000fff0
 0000039c <foo\+0x39c> 00000000 	.word	0x00000000
 000003a0 <foo\+0x3a0> e2810000 	add	r0, r1, #0
@@ -260,7 +260,7 @@ Disassembly of section .text:
 000003e8 <foo\+0x3e8> eddf7a02 	vldr	s15, \[pc, #8\]	; 000003f8 <foo\+0x3f8>
 000003ec <foo\+0x3ec> eddf0b13 	vldr	d16, \[pc, #76\]	; 00000440 <foo\+0x440>
 000003f0 <foo\+0x3f0> eddf1b14 	vldr	d17, \[pc, #80\]	; 00000448 <foo\+0x448>
-000003f4 <foo\+0x3f4> 00000000 	andeq	r0, r0, r0
+000003f4 <foo\+0x3f4> 00000000 	.word	0x00000000
 000003f8 <foo\+0x3f8> 0000fff0 	.word	0x0000fff0
 000003fc <foo\+0x3fc> 00000000 	.word	0x00000000
 00000400 <foo\+0x400> ff000000 	.word	0xff000000
diff --git a/ld/testsuite/ld-arm/ifunc-10.dd b/ld/testsuite/ld-arm/ifunc-10.dd
index 88bae50..d96c086 100644
--- a/ld/testsuite/ld-arm/ifunc-10.dd
+++ b/ld/testsuite/ld-arm/ifunc-10.dd
@@ -267,7 +267,7 @@ Disassembly of section \.text:
 
 0000a010 <tbf1>:
     a010:	46f7      	mov	pc, lr
-    a012:	0000      	movs	r0, r0
+    a012:	0000      	.short	0x0000
     a014:	e1a0f00e 	mov	pc, lr
     a018:	e1a0f00e 	mov	pc, lr
     a01c:	e1a0f00e 	mov	pc, lr
diff --git a/ld/testsuite/ld-arm/ifunc-2.dd b/ld/testsuite/ld-arm/ifunc-2.dd
index f6d57fa..91eab54 100644
--- a/ld/testsuite/ld-arm/ifunc-2.dd
+++ b/ld/testsuite/ld-arm/ifunc-2.dd
@@ -84,10 +84,10 @@ Disassembly of section \.text:
 
 0000a00c <f4>:
     a00c:	46f7      	mov	pc, lr
-    a00e:	0000      	movs	r0, r0
+    a00e:	0000      	.short	0x0000
     a010:	e1a0f00e 	mov	pc, lr
     a014:	46f7      	mov	pc, lr
-    a016:	0000      	movs	r0, r0
+    a016:	0000      	.short	0x0000
     a018:	e1a0f00e 	mov	pc, lr
     a01c:	46f7      	mov	pc, lr
 	\.\.\.

Re: [2.25][ARM][GAS]Backport "Positively emit symbols for alignment."

[Tristan Gingold]

> On 16 Jun 2015, at 10:24, Renlin Li <renlin.li@arm.com> wrote:
> 
> Hi all,
> 
> This is a merged backport patch from the following three. They together fix one problem, So I merge them into one backport patch. The description of problem is quoted below.
> 
> The initial patch:
> https://sourceware.org/ml/binutils/2015-04/msg00381.html
> 
> Fix two failing test case:
> https://sourceware.org/ml/binutils/2015-04/msg00457.html
> 
> Fix a bug in the initial patch:
> https://sourceware.org/ml/binutils/2015-06/msg00045.html
> 
> 
> Binutils, ld, gas regression test Okay. Okay to commit?

Yes, ok for backporting.

> 
> On 24/04/15 13:53, Renlin Li wrote:
>> Hi all,
>> 
>> In arm_init_frag(), when ARM or THUMB are recorded,  it will simply
>> return without doing anything. This is not correct in a few cases.
>> 
>> For the following two cases:
>> case 1:
>>          nop        ----> MAP_ARM
>>          .long   0 ---> MAP_DATA
>>          .align  4 --->
>>                        ---->(1)
>>                        ----> start new frag
>>          .word   0x12345678
>> 
>> (1) paddings here are all treated as data, because the previous state is
>> MAP_DATA.
>> 
>> case 2:
>>          nop       ----> MAP_ARM
>>          .long   0 ---> MAP_DATA
>>          .byte   1
>>          .align  2 ---> (1)
>>                       ----> (2)
>>                       ----> start new frag
>>          .long   0x12345678 ----> (3)
>> 
>> stage 1: while parsing the input file:
>> (1) arm_init_frag early returns, No state change, no new symbol. (3)No
>> state change, no new symbol.
>> stage 2: Later, while writing the object file:
>> (2)MAP_DATA, MAP_ARM are inserted by insert_data_mapping_symol to handle
>> alignment. (3) interpreted as instruction because of the MAP_ARM
>> inserted at (2)
>> 
>> 
>> This change might generate redundant symbols, some of them can be
>> removed later. In check_mapping_symbols(), the symbols at the end of a
>> section, and overlapping symbols will be removed.
>> 
>> gas checked without any new issues. Okay to commit?
> <arm-backport.diff>

Re: [2.25][ARM][GAS]Backport "Positively emit symbols for alignment."

[Renlin Li]

Sorry for the noise, I forgot to add the ChangeLog. Here they are.

gas/ChangeLog:

2015-06-16  Renlin Li  <renlin.li@arm.com>
          backport from mainline
          2015-04-28  Renlin Li  <renlin.li@arm.com>
          2015-06-03  Renlin Li  <renlin.li@arm.com>

         * config/tc-arm.c (arm_init_frag): Always emit mapping symbols.

gas/testsuite/ChangeLog:

2015-06-16  Renlin Li  <renlin.li@arm.com>
         backport from mainline
         2015-04-28  Renlin Li  <renlin.li@arm.com>

         * gas/arm/thumb2_vpool_be.d: Adjust the desired output.
         * gas/arm/vldconst_be.d: Likewise.

ld/testsuite/ChangeLog:

2015-06-16  Renlin Li  <renlin.li@arm.com>
          backport from mainline
          2015-04-29  Renlin Li  <renlin.li@arm.com>

         * ld-arm/ifunc-10.dd: Adjust the desired output.
         * ld-arm/ifunc-2.dd: Likewise.



On 16/06/15 09:24, Renlin Li wrote:
> Hi all,
>
> This is a merged backport patch from the following three. They together
> fix one problem, So I merge them into one backport patch. The
> description of problem is quoted below.
>
> The initial patch:
> https://sourceware.org/ml/binutils/2015-04/msg00381.html
>
> Fix two failing test case:
> https://sourceware.org/ml/binutils/2015-04/msg00457.html
>
> Fix a bug in the initial patch:
> https://sourceware.org/ml/binutils/2015-06/msg00045.html
>
>
> Binutils, ld, gas regression test Okay. Okay to commit?
>
> On 24/04/15 13:53, Renlin Li wrote:
>> Hi all,
>>
>> In arm_init_frag(), when ARM or THUMB are recorded,  it will simply
>> return without doing anything. This is not correct in a few cases.
>>
>> For the following two cases:
>> case 1:
>>            nop        ----> MAP_ARM
>>            .long   0 ---> MAP_DATA
>>            .align  4 --->
>>                          ---->(1)
>>                          ----> start new frag
>>            .word   0x12345678
>>
>> (1) paddings here are all treated as data, because the previous state is
>> MAP_DATA.
>>
>> case 2:
>>            nop       ----> MAP_ARM
>>            .long   0 ---> MAP_DATA
>>            .byte   1
>>            .align  2 ---> (1)
>>                         ----> (2)
>>                         ----> start new frag
>>            .long   0x12345678 ----> (3)
>>
>> stage 1: while parsing the input file:
>> (1) arm_init_frag early returns, No state change, no new symbol. (3)No
>> state change, no new symbol.
>> stage 2: Later, while writing the object file:
>> (2)MAP_DATA, MAP_ARM are inserted by insert_data_mapping_symol to handle
>> alignment. (3) interpreted as instruction because of the MAP_ARM
>> inserted at (2)
>>
>>
>> This change might generate redundant symbols, some of them can be
>> removed later. In check_mapping_symbols(), the symbols at the end of a
>> section, and overlapping symbols will be removed.
>>
>> gas checked without any new issues. Okay to commit?

[2.25][AARCH64][GAS]Backport "Positively emit symbols for alignment."

[Renlin Li]

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

Hi all,

This is a backport patch from the discussion here:

https://sourceware.org/ml/binutils/2015-05/msg00027.html
https://sourceware.org/ml/binutils/2015-05/msg00027.html

The expected results of testcases are modified to account the status of 
binutils 2.25.

Binutils, ld, gas regression test Okay. Okay to commit for branch 2.25?


Regards,
Renlin Li


gas/ChangeLog:

2015-06-17  Renlin Li  <renlin.li@arm.com>

         Backport from mainline.
         2015-05-05  Renlin Li  <renlin.li@arm.com>

         * config/tc-aarch64.c (aarch64_init_frag): Always generate 
mapping symbols.

gas/testsuite/ChangeLog:

2015-06-17  Renlin Li  <renlin.li@arm.com>

         Backport from mainline.
         2015-05-05  Renlin Li  <renlin.li@arm.com>

         * gas/aarch64/mapping_5.d: New.
         * gas/aarch64/mapping_5.s: New.
         * gas/aarch64/mapping_6.d: New.
         * gas/aarch64/mapping_6.s: New.


On 24/04/15 13:53, Renlin Li wrote:
> Hi all,
>
> In arm_init_frag(), when ARM or THUMB are recorded,  it will simply 
> return without doing anything. This is not correct in a few cases.
>
> For the following two cases:
> case 1:
>         nop        ----> MAP_ARM
>         .long   0 ---> MAP_DATA
>         .align  4 --->
>                       ---->(1)
>                       ----> start new frag
>         .word   0x12345678
>
> (1) paddings here are all treated as data, because the previous state 
> is MAP_DATA.
>
> case 2:
>         nop       ----> MAP_ARM
>         .long   0 ---> MAP_DATA
>         .byte   1
>         .align  2 ---> (1)
>                      ----> (2)
>                      ----> start new frag
>         .long   0x12345678 ----> (3)
>
> stage 1: while parsing the input file:
> (1) arm_init_frag early returns, No state change, no new symbol. (3)No 
> state change, no new symbol.
> stage 2: Later, while writing the object file:
> (2)MAP_DATA, MAP_ARM are inserted by insert_data_mapping_symol to 
> handle alignment. (3) interpreted as instruction because of the 
> MAP_ARM inserted at (2)
>
>
> This change might generate redundant symbols, some of them can be 
> removed later. In check_mapping_symbols(), the symbols at the end of a 
> section, and overlapping symbols will be removed.
>
> gas checked without any new issues. Okay to commit?
>
> By the way, Nick, aarch64 has the same issue. The same can be done to 
> aarch64_init_frag(). Another way is to remove 
> fragP->tc_frag_data.recorded. It's only used in a few places. But I am 
> sure what dose it intended to do, Could you please explain a little 
> bit for me?
>
> Regards,
> Renlin Li
>
>
> gas/ChangeLog:
>
> 2015-04-24  Renlin Li  <renlin.li@arm.com>
>
>     * config/tc-arm.c (arm_init_frag): Always emit mapping symbols.
>
> gas/testsuite/ChangeLog:
>
> 2015-04-24  Renlin Li  <renlin.li@arm.com>
>
>     * gas/arm/thumb2_vpool_be.d: Adjust the desired output.
>     * gas/arm/vldconst_be.d: Ditto.
>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-backport.diff --]
[-- Type: text/x-patch; name=aarch64-backport.diff, Size: 3081 bytes --]

commit 262a89bcf8dd349e065d46896965c49054c8a034
Author: Renlin Li <renlin.li@arm.com>
Date:   Wed Jun 3 16:34:10 2015 +0100

    aarch64 backport
    
    Change-Id: Ie8d550c3518a25de0094fe7dc7bcfecd1ac77356

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 62a1a3f..015b0b7 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -5914,21 +5914,20 @@ aarch64_init_frag (fragS * fragP, int max_chars)
   /* Record a mapping symbol for alignment frags.  We will delete this
      later if the alignment ends up empty.  */
   if (!fragP->tc_frag_data.recorded)
+    fragP->tc_frag_data.recorded = 1;
+
+  switch (fragP->fr_type)
     {
-      fragP->tc_frag_data.recorded = 1;
-      switch (fragP->fr_type)
-	{
-	case rs_align:
-	case rs_align_test:
-	case rs_fill:
-	  mapping_state_2 (MAP_DATA, max_chars);
-	  break;
-	case rs_align_code:
-	  mapping_state_2 (MAP_INSN, max_chars);
-	  break;
-	default:
-	  break;
-	}
+    case rs_align:
+    case rs_align_test:
+    case rs_fill:
+      mapping_state_2 (MAP_DATA, max_chars);
+      break;
+    case rs_align_code:
+      mapping_state_2 (MAP_INSN, max_chars);
+      break;
+    default:
+      break;
     }
 }
 \f
diff --git a/gas/testsuite/gas/aarch64/mapping_5.d b/gas/testsuite/gas/aarch64/mapping_5.d
new file mode 100644
index 0000000..8a85038
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mapping_5.d
@@ -0,0 +1,14 @@
+#objdump: --syms --special-syms
+#name: AArch64 Mapping Symbols Test 5
+
+.*: +file format.*aarch64.*
+
+SYMBOL TABLE:
+[0]+00 l    d  .text	[0]+00 .text
+[0]+00 l    d  .data	[0]+00 .data
+[0]+00 l    d  .bss	[0]+00 .bss
+[0]+00 l       .text	[0]+00 \$x
+[0]+04 l       .text	[0]+00 \$d
+[0]+08 l       .text	[0]+00 \$x
+[0]+10 l       .text	[0]+00 \$d
+[0]+14 l       .text	[0]+00 \$x
diff --git a/gas/testsuite/gas/aarch64/mapping_5.s b/gas/testsuite/gas/aarch64/mapping_5.s
new file mode 100644
index 0000000..2cb95ed
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mapping_5.s
@@ -0,0 +1,5 @@
+	.text
+	nop
+	.long	0
+	.align	4
+	.word	0x12345678
diff --git a/gas/testsuite/gas/aarch64/mapping_6.d b/gas/testsuite/gas/aarch64/mapping_6.d
new file mode 100644
index 0000000..df6bdaa
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mapping_6.d
@@ -0,0 +1,15 @@
+#objdump: --syms --special-syms
+#name: AArch64 Mapping Symbols Test 6
+
+.*: +file format.*aarch64.*
+
+SYMBOL TABLE:
+[0]+00 l    d  .text	[0]+00 .text
+[0]+00 l    d  .data	[0]+00 .data
+[0]+00 l    d  .bss	[0]+00 .bss
+[0]+00 l       .text	[0]+00 \$x
+[0]+04 l       .text	[0]+00 \$d
+[0]+10 l       .text	[0]+00 \$d
+[0]+14 l       .text	[0]+00 \$x
+[0]+09 l       .text	[0]+00 \$d
+[0]+0c l       .text	[0]+00 \$x
diff --git a/gas/testsuite/gas/aarch64/mapping_6.s b/gas/testsuite/gas/aarch64/mapping_6.s
new file mode 100644
index 0000000..4e0e2c5
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mapping_6.s
@@ -0,0 +1,6 @@
+	.text
+	nop
+	.long	0
+	.byte	1
+	.align	4
+	.word	0x12345678

Re: [2.25][AARCH64][GAS]Backport "Positively emit symbols for alignment."

[Tristan Gingold]

> On 17 Jun 2015, at 11:38, Renlin Li <renlin.li@arm.com> wrote:
> 
> Hi all,
> 
> This is a backport patch from the discussion here:
> 
> https://sourceware.org/ml/binutils/2015-05/msg00027.html
> https://sourceware.org/ml/binutils/2015-05/msg00027.html
> 
> The expected results of testcases are modified to account the status of binutils 2.25.
> 
> Binutils, ld, gas regression test Okay. Okay to commit for branch 2.25?

Yes, this is ok for 2.25

> 
> 
> Regards,
> Renlin Li
> 
> 
> gas/ChangeLog:
> 
> 2015-06-17  Renlin Li  <renlin.li@arm.com>
> 
>        Backport from mainline.
>        2015-05-05  Renlin Li  <renlin.li@arm.com>
> 
>        * config/tc-aarch64.c (aarch64_init_frag): Always generate mapping symbols.
> 
> gas/testsuite/ChangeLog:
> 
> 2015-06-17  Renlin Li  <renlin.li@arm.com>
> 
>        Backport from mainline.
>        2015-05-05  Renlin Li  <renlin.li@arm.com>
> 
>        * gas/aarch64/mapping_5.d: New.
>        * gas/aarch64/mapping_5.s: New.
>        * gas/aarch64/mapping_6.d: New.
>        * gas/aarch64/mapping_6.s: New.
> 
> 
> On 24/04/15 13:53, Renlin Li wrote:
>> Hi all,
>> 
>> In arm_init_frag(), when ARM or THUMB are recorded,  it will simply return without doing anything. This is not correct in a few cases.
>> 
>> For the following two cases:
>> case 1:
>>        nop        ----> MAP_ARM
>>        .long   0 ---> MAP_DATA
>>        .align  4 --->
>>                      ---->(1)
>>                      ----> start new frag
>>        .word   0x12345678
>> 
>> (1) paddings here are all treated as data, because the previous state is MAP_DATA.
>> 
>> case 2:
>>        nop       ----> MAP_ARM
>>        .long   0 ---> MAP_DATA
>>        .byte   1
>>        .align  2 ---> (1)
>>                     ----> (2)
>>                     ----> start new frag
>>        .long   0x12345678 ----> (3)
>> 
>> stage 1: while parsing the input file:
>> (1) arm_init_frag early returns, No state change, no new symbol. (3)No state change, no new symbol.
>> stage 2: Later, while writing the object file:
>> (2)MAP_DATA, MAP_ARM are inserted by insert_data_mapping_symol to handle alignment. (3) interpreted as instruction because of the MAP_ARM inserted at (2)
>> 
>> 
>> This change might generate redundant symbols, some of them can be removed later. In check_mapping_symbols(), the symbols at the end of a section, and overlapping symbols will be removed.
>> 
>> gas checked without any new issues. Okay to commit?
>> 
>> By the way, Nick, aarch64 has the same issue. The same can be done to aarch64_init_frag(). Another way is to remove fragP->tc_frag_data.recorded. It's only used in a few places. But I am sure what dose it intended to do, Could you please explain a little bit for me?
>> 
>> Regards,
>> Renlin Li
>> 
>> 
>> gas/ChangeLog:
>> 
>> 2015-04-24  Renlin Li  <renlin.li@arm.com>
>> 
>>    * config/tc-arm.c (arm_init_frag): Always emit mapping symbols.
>> 
>> gas/testsuite/ChangeLog:
>> 
>> 2015-04-24  Renlin Li  <renlin.li@arm.com>
>> 
>>    * gas/arm/thumb2_vpool_be.d: Adjust the desired output.
>>    * gas/arm/vldconst_be.d: Ditto.
>> 
> 
> <aarch64-backport.diff>