[PATCH] Fix PR69951

[PATCH] Fix PR69951

[Richard Biener]

The following fixes PR69951, hopefully the last case of decl alias
issues with alias analysis.  This time it's points-to and the DECL_UIDs
used in points-to sets not being canonicalized.

The simplest (and cheapest) fix is to make aliases refer to the
ultimate alias target via their DECL_PT_UID which we conveniently
have available.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2016-02-26  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/69551
	* tree-ssa-structalias.c (get_constraint_for_ssa_var): When
	looking through aliases adjust DECL_PT_UID to refer to the
	ultimate alias target.

	* gcc.dg/torture/pr69951.c: New testcase.

Index: gcc/tree-ssa-structalias.c
===================================================================
*** gcc/tree-ssa-structalias.c	(revision 233693)
--- gcc/tree-ssa-structalias.c	(working copy)
*************** get_constraint_for_ssa_var (tree t, vec<
*** 2943,2948 ****
--- 2943,2956 ----
        if (node && node->alias && node->analyzed)
  	{
  	  node = node->ultimate_alias_target ();
+ 	  /* Canonicalize the PT uid of all aliases to the ultimate target.
+ 	     ???  Hopefully the set of aliases can't change in a way that
+ 	     changes the ultimate alias target.  */
+ 	  gcc_assert ((! DECL_PT_UID_SET_P (node->decl)
+ 		       || DECL_PT_UID (node->decl) == DECL_UID (node->decl))
+ 		      && (! DECL_PT_UID_SET_P (t)
+ 			  || DECL_PT_UID (t) == DECL_UID (node->decl)));
+ 	  DECL_PT_UID (t) = DECL_UID (node->decl);
  	  t = node->decl;
  	}
      }
Index: gcc/testsuite/gcc.dg/torture/pr69951.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr69951.c	(revision 0)
--- gcc/testsuite/gcc.dg/torture/pr69951.c	(working copy)
***************
*** 0 ****
--- 1,21 ----
+ /* { dg-do run } */
+ /* { dg-require-alias "" } */
+ 
+ extern void abort (void);
+ 
+ int a = 1, c = 1;
+ extern int b __attribute__((alias("a")));
+ extern int d __attribute__((alias("c")));
+ int main(int argc)
+ {
+   int *p, *q;
+   if (argc)
+     p = &c, q = &d;
+   else
+     p = &b, q = &d;
+   *p = 1;
+   *q = 2;
+   if (*p == 1)
+     abort();
+   return 0;
+ }

Re: [PATCH] Fix PR69951

[James Greenhalgh]

On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
> 
> The following fixes PR69951, hopefully the last case of decl alias
> issues with alias analysis.  This time it's points-to and the DECL_UIDs
> used in points-to sets not being canonicalized.
> 
> The simplest (and cheapest) fix is to make aliases refer to the
> ultimate alias target via their DECL_PT_UID which we conveniently
> have available.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> 
> Richard.
> 
> 2016-02-26  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/69551
> 	* tree-ssa-structalias.c (get_constraint_for_ssa_var): When
> 	looking through aliases adjust DECL_PT_UID to refer to the
> 	ultimate alias target.
> 
> 	* gcc.dg/torture/pr69951.c: New testcase.

I see this new testcase failing on an ARM target as so:

    /tmp/ccChjoFc.s: Assembler messages:
    /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b

    FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)

But I haven't managed to reproduce it outside of the test environment.

The fix looks trivial, rename b to anything else you fancy (well... stay
clear of add and ldr). I'll put a fix in myself if I can manage to get
this to reproduce - though if anyone else wants to do it I won't be
offended :-).

Thanks,
James

 

Re: [PATCH] Fix PR69951

[Richard Biener]

On Mon, 29 Feb 2016, James Greenhalgh wrote:

> On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
> > 
> > The following fixes PR69951, hopefully the last case of decl alias
> > issues with alias analysis.  This time it's points-to and the DECL_UIDs
> > used in points-to sets not being canonicalized.
> > 
> > The simplest (and cheapest) fix is to make aliases refer to the
> > ultimate alias target via their DECL_PT_UID which we conveniently
> > have available.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> > 
> > Richard.
> > 
> > 2016-02-26  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR tree-optimization/69551
> > 	* tree-ssa-structalias.c (get_constraint_for_ssa_var): When
> > 	looking through aliases adjust DECL_PT_UID to refer to the
> > 	ultimate alias target.
> > 
> > 	* gcc.dg/torture/pr69951.c: New testcase.
> 
> I see this new testcase failing on an ARM target as so:
> 
>     /tmp/ccChjoFc.s: Assembler messages:
>     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
> 
>     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
> 
> But I haven't managed to reproduce it outside of the test environment.
> 
> The fix looks trivial, rename b to anything else you fancy (well... stay
> clear of add and ldr). I'll put a fix in myself if I can manage to get
> this to reproduce - though if anyone else wants to do it I won't be
> offended :-).

Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
C symbol name, too.  In fact my cross arm as doesn't report this
warning (binutils 2.25.0)

> arm-suse-linux-gnueabi-as t.s -mwarn-syms
Assembler messages:
Error: unrecognized option -mwarn-syms

Richard.

Re: [PATCH] Fix PR69951

[James Greenhalgh]

On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
> On Mon, 29 Feb 2016, James Greenhalgh wrote:
> 
> > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
> > > 
> > > The following fixes PR69951, hopefully the last case of decl alias
> > > issues with alias analysis.  This time it's points-to and the DECL_UIDs
> > > used in points-to sets not being canonicalized.
> > > 
> > > The simplest (and cheapest) fix is to make aliases refer to the
> > > ultimate alias target via their DECL_PT_UID which we conveniently
> > > have available.
> > > 
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> > > 
> > > Richard.
> > > 
> > > 2016-02-26  Richard Biener  <rguenther@suse.de>
> > > 
> > > 	PR tree-optimization/69551
> > > 	* tree-ssa-structalias.c (get_constraint_for_ssa_var): When
> > > 	looking through aliases adjust DECL_PT_UID to refer to the
> > > 	ultimate alias target.
> > > 
> > > 	* gcc.dg/torture/pr69951.c: New testcase.
> > 
> > I see this new testcase failing on an ARM target as so:
> > 
> >     /tmp/ccChjoFc.s: Assembler messages:
> >     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
> > 
> >     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
> > 
> > But I haven't managed to reproduce it outside of the test environment.
> > 
> > The fix looks trivial, rename b to anything else you fancy (well... stay
> > clear of add and ldr). I'll put a fix in myself if I can manage to get
> > this to reproduce - though if anyone else wants to do it I won't be
> > offended :-).
> 
> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
> C symbol name, too.  In fact my cross arm as doesn't report this
> warning (binutils 2.25.0)
> 
> > arm-suse-linux-gnueabi-as t.s -mwarn-syms
> Assembler messages:
> Error: unrecognized option -mwarn-syms

Right, I've figured out the set of conditions... You need to be targeting
an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
from config/arm/linux-elf.h is pulled in. This causes us to emit:

b = a

Rather than

	.set	b,a

Writing it as "b = a" causes the warning added to resolve binutils
PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
that patch).

Resolving it by hacking the testcase would be one fix, but I wonder why the
ARM port prefers to emit "b = a" in a linux environment if .set does the
same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
remember? (AArch64 does the same thing, but the AArch64 gas port doesn't
have the PR18347 fix).

Cheers,
James

---

[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=18347

> 
> Richard.
> 

Re: [PATCH] Fix PR69951

[Richard Biener]

On Tue, 1 Mar 2016, James Greenhalgh wrote:

> On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
> > On Mon, 29 Feb 2016, James Greenhalgh wrote:
> > 
> > > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
> > > > 
> > > > The following fixes PR69951, hopefully the last case of decl alias
> > > > issues with alias analysis.  This time it's points-to and the DECL_UIDs
> > > > used in points-to sets not being canonicalized.
> > > > 
> > > > The simplest (and cheapest) fix is to make aliases refer to the
> > > > ultimate alias target via their DECL_PT_UID which we conveniently
> > > > have available.
> > > > 
> > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> > > > 
> > > > Richard.
> > > > 
> > > > 2016-02-26  Richard Biener  <rguenther@suse.de>
> > > > 
> > > > 	PR tree-optimization/69551
> > > > 	* tree-ssa-structalias.c (get_constraint_for_ssa_var): When
> > > > 	looking through aliases adjust DECL_PT_UID to refer to the
> > > > 	ultimate alias target.
> > > > 
> > > > 	* gcc.dg/torture/pr69951.c: New testcase.
> > > 
> > > I see this new testcase failing on an ARM target as so:
> > > 
> > >     /tmp/ccChjoFc.s: Assembler messages:
> > >     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
> > > 
> > >     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
> > > 
> > > But I haven't managed to reproduce it outside of the test environment.
> > > 
> > > The fix looks trivial, rename b to anything else you fancy (well... stay
> > > clear of add and ldr). I'll put a fix in myself if I can manage to get
> > > this to reproduce - though if anyone else wants to do it I won't be
> > > offended :-).
> > 
> > Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
> > C symbol name, too.  In fact my cross arm as doesn't report this
> > warning (binutils 2.25.0)
> > 
> > > arm-suse-linux-gnueabi-as t.s -mwarn-syms
> > Assembler messages:
> > Error: unrecognized option -mwarn-syms
> 
> Right, I've figured out the set of conditions... You need to be targeting
> an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
> from config/arm/linux-elf.h is pulled in. This causes us to emit:
> 
> b = a
> 
> Rather than
> 
> 	.set	b,a
> 
> Writing it as "b = a" causes the warning added to resolve binutils
> PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
> that patch).
> 
> Resolving it by hacking the testcase would be one fix, but I wonder why the
> ARM port prefers to emit "b = a" in a linux environment if .set does the
> same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
> remember? (AArch64 does the same thing, but the AArch64 gas port doesn't
> have the PR18347 fix).

So does b = a define a macro then and the warning is to avoid you
doing

 ldr = b

...

 ldr 0, 1 (or whatever correct ldr instruction)

and have that ldr replaced by b?

Then it's a bug to emit aliases in this form and I hope .set ldr, b
doesn't have the same effect.

Richard.

Re: [PATCH] Fix PR69951

[Ramana Radhakrishnan]

On 01/03/16 09:54, Richard Biener wrote:
> On Tue, 1 Mar 2016, James Greenhalgh wrote:
> 
>> On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
>>> On Mon, 29 Feb 2016, James Greenhalgh wrote:
>>>
>>>> On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
>>>>>
>>>>> The following fixes PR69951, hopefully the last case of decl alias
>>>>> issues with alias analysis.  This time it's points-to and the DECL_UIDs
>>>>> used in points-to sets not being canonicalized.
>>>>>
>>>>> The simplest (and cheapest) fix is to make aliases refer to the
>>>>> ultimate alias target via their DECL_PT_UID which we conveniently
>>>>> have available.
>>>>>
>>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>>>>>
>>>>> Richard.
>>>>>
>>>>> 2016-02-26  Richard Biener  <rguenther@suse.de>
>>>>>
>>>>> 	PR tree-optimization/69551
>>>>> 	* tree-ssa-structalias.c (get_constraint_for_ssa_var): When
>>>>> 	looking through aliases adjust DECL_PT_UID to refer to the
>>>>> 	ultimate alias target.
>>>>>
>>>>> 	* gcc.dg/torture/pr69951.c: New testcase.
>>>>
>>>> I see this new testcase failing on an ARM target as so:
>>>>
>>>>     /tmp/ccChjoFc.s: Assembler messages:
>>>>     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
>>>>
>>>>     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
>>>>
>>>> But I haven't managed to reproduce it outside of the test environment.
>>>>
>>>> The fix looks trivial, rename b to anything else you fancy (well... stay
>>>> clear of add and ldr). I'll put a fix in myself if I can manage to get
>>>> this to reproduce - though if anyone else wants to do it I won't be
>>>> offended :-).
>>>
>>> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
>>> C symbol name, too.  In fact my cross arm as doesn't report this
>>> warning (binutils 2.25.0)
>>>
>>>> arm-suse-linux-gnueabi-as t.s -mwarn-syms
>>> Assembler messages:
>>> Error: unrecognized option -mwarn-syms
>>
>> Right, I've figured out the set of conditions... You need to be targeting
>> an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
>> from config/arm/linux-elf.h is pulled in. This causes us to emit:
>>
>> b = a
>>
>> Rather than
>>
>> 	.set	b,a
>>
>> Writing it as "b = a" causes the warning added to resolve binutils
>> PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
>> that patch).
>>
>> Resolving it by hacking the testcase would be one fix, but I wonder why the
>> ARM port prefers to emit "b = a" in a linux environment if .set does the
>> same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
>> remember?
>> (AArch64 does the same thing, but the AArch64 gas port doesn't
>> have the PR18347 fix).
> 
> So does b = a define a macro then and the warning is to avoid you
> doing




I don't think this is a macro, b = a seems to be a way of setting the value of a to b. in the assembler. If a is an expression , then I believe the expression is resolved at assemble time - (b ends up being a symbol in the symbol table produced with the value of a) in this case the address of a. .set b, a achieves the same thing from my experiments and reading of the sources. The reason ports appear to choose not to use the .set a, b idiom is if the assembler syntax has hijacked the .set directive for something else. Thus I don't see why we use the ASM_OUTPUT_DEF form in the GNU/Linux port TBH rather than the .set form especially as we don't reuse .set for anything else in the ARM assembler port and SET_ASM_OP is defined in  config/arm/aout.h. 

The use of .set in the arm port of glibc for assembler code for the same purpose  seems to also vindicate that kind of thought.
 
No reasons were given here[1], maybe Nick or Richard remember from nearly 18 years ago ;)


Therefore this seems to be an assembler bug to me in that it doesn't allow such an assignment of values, and a backend wart to me that we have ASM_OUTPUT_DEF defined for no good reason. So, a patch that removes ASM_OUTPUT_DEF from linux-elf.h seems obvious to me pending testing.


Nick , Richard - any thoughts ?


regards
Ramana

1. https://gcc.gnu.org/ml/gcc-patches/1998-10/msg00701.html

> 
> ...
> 
>  ldr 0, 1 (or whatever correct ldr instruction)
> 
> and have that ldr replaced by b?
> 
> Then it's a bug to emit aliases in this form and I hope .set ldr, b
> doesn't have the same effect.
> 
> Richard.
> 

Re: [PATCH] Fix PR69951

[Richard Biener]

On Tue, 1 Mar 2016, Ramana Radhakrishnan wrote:

> 
> 
> On 01/03/16 09:54, Richard Biener wrote:
> > On Tue, 1 Mar 2016, James Greenhalgh wrote:
> > 
> >> On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
> >>> On Mon, 29 Feb 2016, James Greenhalgh wrote:
> >>>
> >>>> On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
> >>>>>
> >>>>> The following fixes PR69951, hopefully the last case of decl alias
> >>>>> issues with alias analysis.  This time it's points-to and the DECL_UIDs
> >>>>> used in points-to sets not being canonicalized.
> >>>>>
> >>>>> The simplest (and cheapest) fix is to make aliases refer to the
> >>>>> ultimate alias target via their DECL_PT_UID which we conveniently
> >>>>> have available.
> >>>>>
> >>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> >>>>>
> >>>>> Richard.
> >>>>>
> >>>>> 2016-02-26  Richard Biener  <rguenther@suse.de>
> >>>>>
> >>>>> 	PR tree-optimization/69551
> >>>>> 	* tree-ssa-structalias.c (get_constraint_for_ssa_var): When
> >>>>> 	looking through aliases adjust DECL_PT_UID to refer to the
> >>>>> 	ultimate alias target.
> >>>>>
> >>>>> 	* gcc.dg/torture/pr69951.c: New testcase.
> >>>>
> >>>> I see this new testcase failing on an ARM target as so:
> >>>>
> >>>>     /tmp/ccChjoFc.s: Assembler messages:
> >>>>     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
> >>>>
> >>>>     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
> >>>>
> >>>> But I haven't managed to reproduce it outside of the test environment.
> >>>>
> >>>> The fix looks trivial, rename b to anything else you fancy (well... stay
> >>>> clear of add and ldr). I'll put a fix in myself if I can manage to get
> >>>> this to reproduce - though if anyone else wants to do it I won't be
> >>>> offended :-).
> >>>
> >>> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
> >>> C symbol name, too.  In fact my cross arm as doesn't report this
> >>> warning (binutils 2.25.0)
> >>>
> >>>> arm-suse-linux-gnueabi-as t.s -mwarn-syms
> >>> Assembler messages:
> >>> Error: unrecognized option -mwarn-syms
> >>
> >> Right, I've figured out the set of conditions... You need to be targeting
> >> an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
> >> from config/arm/linux-elf.h is pulled in. This causes us to emit:
> >>
> >> b = a
> >>
> >> Rather than
> >>
> >> 	.set	b,a
> >>
> >> Writing it as "b = a" causes the warning added to resolve binutils
> >> PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
> >> that patch).
> >>
> >> Resolving it by hacking the testcase would be one fix, but I wonder why the
> >> ARM port prefers to emit "b = a" in a linux environment if .set does the
> >> same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
> >> remember?
> >> (AArch64 does the same thing, but the AArch64 gas port doesn't
> >> have the PR18347 fix).
> > 
> > So does b = a define a macro then and the warning is to avoid you
> > doing
> 
> 
> 
> 
> I don't think this is a macro, b = a seems to be a way of setting the 
> value of a to b. in the assembler. If a is an expression , then I 
> believe the expression is resolved at assemble time - (b ends up being a 
> symbol in the symbol table produced with the value of a) in this case 
> the address of a. .set b, a achieves the same thing from my experiments 
> and reading of the sources. The reason ports appear to choose not to use 
> the .set a, b idiom is if the assembler syntax has hijacked the .set 
> directive for something else. Thus I don't see why we use the 
> ASM_OUTPUT_DEF form in the GNU/Linux port TBH rather than the .set form 
> especially as we don't reuse .set for anything else in the ARM assembler 
> port and SET_ASM_OP is defined in config/arm/aout.h.
> 
> The use of .set in the arm port of glibc for assembler code for the same 
> purpose seems to also vindicate that kind of thought.
>  No reasons were given here[1], maybe Nick or Richard remember from 
> nearly 18 years ago ;)
> 
> 
> Therefore this seems to be an assembler bug to me in that it doesn't 
> allow such an assignment of values, and a backend wart to me that we 
> have ASM_OUTPUT_DEF defined for no good reason. So, a patch that removes 
> ASM_OUTPUT_DEF from linux-elf.h seems obvious to me pending testing.
> 
> 
> Nick , Richard - any thoughts ?

So - why does it warn at all for this?  And why does it only warn
for b = a and not .set b, a?

IMHO the warning is simply bogus?

Richard.

> 
> regards
> Ramana
> 
> 1. https://gcc.gnu.org/ml/gcc-patches/1998-10/msg00701.html
> 
> > 
> > ...
> > 
> >  ldr 0, 1 (or whatever correct ldr instruction)
> > 
> > and have that ldr replaced by b?
> > 
> > Then it's a bug to emit aliases in this form and I hope .set ldr, b
> > doesn't have the same effect.
> > 
> > Richard.
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Fix PR69951

[Prathamesh Kulkarni]

On 1 March 2016 at 16:19, Richard Biener <rguenther@suse.de> wrote:
> On Tue, 1 Mar 2016, Ramana Radhakrishnan wrote:
>
>>
>>
>> On 01/03/16 09:54, Richard Biener wrote:
>> > On Tue, 1 Mar 2016, James Greenhalgh wrote:
>> >
>> >> On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
>> >>> On Mon, 29 Feb 2016, James Greenhalgh wrote:
>> >>>
>> >>>> On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
>> >>>>>
>> >>>>> The following fixes PR69951, hopefully the last case of decl alias
>> >>>>> issues with alias analysis.  This time it's points-to and the DECL_UIDs
>> >>>>> used in points-to sets not being canonicalized.
>> >>>>>
>> >>>>> The simplest (and cheapest) fix is to make aliases refer to the
>> >>>>> ultimate alias target via their DECL_PT_UID which we conveniently
>> >>>>> have available.
>> >>>>>
>> >>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>> >>>>>
>> >>>>> Richard.
>> >>>>>
>> >>>>> 2016-02-26  Richard Biener  <rguenther@suse.de>
>> >>>>>
>> >>>>>         PR tree-optimization/69551
>> >>>>>         * tree-ssa-structalias.c (get_constraint_for_ssa_var): When
>> >>>>>         looking through aliases adjust DECL_PT_UID to refer to the
>> >>>>>         ultimate alias target.
>> >>>>>
>> >>>>>         * gcc.dg/torture/pr69951.c: New testcase.
>> >>>>
>> >>>> I see this new testcase failing on an ARM target as so:
>> >>>>
>> >>>>     /tmp/ccChjoFc.s: Assembler messages:
>> >>>>     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
>> >>>>
>> >>>>     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
>> >>>>
>> >>>> But I haven't managed to reproduce it outside of the test environment.
>> >>>>
>> >>>> The fix looks trivial, rename b to anything else you fancy (well... stay
>> >>>> clear of add and ldr). I'll put a fix in myself if I can manage to get
>> >>>> this to reproduce - though if anyone else wants to do it I won't be
>> >>>> offended :-).
>> >>>
>> >>> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
>> >>> C symbol name, too.  In fact my cross arm as doesn't report this
>> >>> warning (binutils 2.25.0)
>> >>>
>> >>>> arm-suse-linux-gnueabi-as t.s -mwarn-syms
>> >>> Assembler messages:
>> >>> Error: unrecognized option -mwarn-syms
>> >>
>> >> Right, I've figured out the set of conditions... You need to be targeting
>> >> an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
>> >> from config/arm/linux-elf.h is pulled in. This causes us to emit:
>> >>
>> >> b = a
>> >>
>> >> Rather than
>> >>
>> >>    .set    b,a
>> >>
>> >> Writing it as "b = a" causes the warning added to resolve binutils
>> >> PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
>> >> that patch).
>> >>
>> >> Resolving it by hacking the testcase would be one fix, but I wonder why the
>> >> ARM port prefers to emit "b = a" in a linux environment if .set does the
>> >> same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
>> >> remember?
>> >> (AArch64 does the same thing, but the AArch64 gas port doesn't
>> >> have the PR18347 fix).
>> >
>> > So does b = a define a macro then and the warning is to avoid you
>> > doing
>>
>>
>>
>>
>> I don't think this is a macro, b = a seems to be a way of setting the
>> value of a to b. in the assembler. If a is an expression , then I
>> believe the expression is resolved at assemble time - (b ends up being a
>> symbol in the symbol table produced with the value of a) in this case
>> the address of a. .set b, a achieves the same thing from my experiments
>> and reading of the sources. The reason ports appear to choose not to use
>> the .set a, b idiom is if the assembler syntax has hijacked the .set
>> directive for something else. Thus I don't see why we use the
>> ASM_OUTPUT_DEF form in the GNU/Linux port TBH rather than the .set form
>> especially as we don't reuse .set for anything else in the ARM assembler
>> port and SET_ASM_OP is defined in config/arm/aout.h.
>>
>> The use of .set in the arm port of glibc for assembler code for the same
>> purpose seems to also vindicate that kind of thought.
>>  No reasons were given here[1], maybe Nick or Richard remember from
>> nearly 18 years ago ;)
>>
>>
>> Therefore this seems to be an assembler bug to me in that it doesn't
>> allow such an assignment of values, and a backend wart to me that we
>> have ASM_OUTPUT_DEF defined for no good reason. So, a patch that removes
>> ASM_OUTPUT_DEF from linux-elf.h seems obvious to me pending testing.
>>
>>
>> Nick , Richard - any thoughts ?
>
> So - why does it warn at all for this?  And why does it only warn
> for b = a and not .set b, a?
The rationale for that appears to be in comment 3 for binutils PR18347:
https://sourceware.org/bugzilla/show_bug.cgi?id=18347

"As far as adding some way to suppress the warning... Instruction set
extensions mean
that an acceptable symbol one day will cause a warning tomorrow.
Having some way to suppress
the warning would be good. If it's possible, I'd suggest keeping the
warning on definitions of the form "X=Y",
and having no warning on the rather more explicit (but equivalent,
right?) ".set X, Y".
If the user really wants a symbol with that kind of name, they
oughtn't mind being explicit about it.

Thanks,
Prathamesh
>
> IMHO the warning is simply bogus?
>
> Richard.
>
>>
>> regards
>> Ramana
>>
>> 1. https://gcc.gnu.org/ml/gcc-patches/1998-10/msg00701.html
>>
>> >
>> > ...
>> >
>> >  ldr 0, 1 (or whatever correct ldr instruction)
>> >
>> > and have that ldr replaced by b?
>> >
>> > Then it's a bug to emit aliases in this form and I hope .set ldr, b
>> > doesn't have the same effect.
>> >
>> > Richard.
>> >
>>
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Fix PR69951

[Richard Earnshaw (lists)]

On 01/03/16 10:49, Richard Biener wrote:
> On Tue, 1 Mar 2016, Ramana Radhakrishnan wrote:
> 
>>
>>
>> On 01/03/16 09:54, Richard Biener wrote:
>>> On Tue, 1 Mar 2016, James Greenhalgh wrote:
>>>
>>>> On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
>>>>> On Mon, 29 Feb 2016, James Greenhalgh wrote:
>>>>>
>>>>>> On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
>>>>>>>
>>>>>>> The following fixes PR69951, hopefully the last case of decl alias
>>>>>>> issues with alias analysis.  This time it's points-to and the DECL_UIDs
>>>>>>> used in points-to sets not being canonicalized.
>>>>>>>
>>>>>>> The simplest (and cheapest) fix is to make aliases refer to the
>>>>>>> ultimate alias target via their DECL_PT_UID which we conveniently
>>>>>>> have available.
>>>>>>>
>>>>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>>>>>>>
>>>>>>> Richard.
>>>>>>>
>>>>>>> 2016-02-26  Richard Biener  <rguenther@suse.de>
>>>>>>>
>>>>>>> 	PR tree-optimization/69551
>>>>>>> 	* tree-ssa-structalias.c (get_constraint_for_ssa_var): When
>>>>>>> 	looking through aliases adjust DECL_PT_UID to refer to the
>>>>>>> 	ultimate alias target.
>>>>>>>
>>>>>>> 	* gcc.dg/torture/pr69951.c: New testcase.
>>>>>>
>>>>>> I see this new testcase failing on an ARM target as so:
>>>>>>
>>>>>>     /tmp/ccChjoFc.s: Assembler messages:
>>>>>>     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
>>>>>>
>>>>>>     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
>>>>>>
>>>>>> But I haven't managed to reproduce it outside of the test environment.
>>>>>>
>>>>>> The fix looks trivial, rename b to anything else you fancy (well... stay
>>>>>> clear of add and ldr). I'll put a fix in myself if I can manage to get
>>>>>> this to reproduce - though if anyone else wants to do it I won't be
>>>>>> offended :-).
>>>>>
>>>>> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
>>>>> C symbol name, too.  In fact my cross arm as doesn't report this
>>>>> warning (binutils 2.25.0)
>>>>>
>>>>>> arm-suse-linux-gnueabi-as t.s -mwarn-syms
>>>>> Assembler messages:
>>>>> Error: unrecognized option -mwarn-syms
>>>>
>>>> Right, I've figured out the set of conditions... You need to be targeting
>>>> an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
>>>> from config/arm/linux-elf.h is pulled in. This causes us to emit:
>>>>
>>>> b = a
>>>>
>>>> Rather than
>>>>
>>>> 	.set	b,a
>>>>
>>>> Writing it as "b = a" causes the warning added to resolve binutils
>>>> PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
>>>> that patch).
>>>>
>>>> Resolving it by hacking the testcase would be one fix, but I wonder why the
>>>> ARM port prefers to emit "b = a" in a linux environment if .set does the
>>>> same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
>>>> remember?
>>>> (AArch64 does the same thing, but the AArch64 gas port doesn't
>>>> have the PR18347 fix).
>>>
>>> So does b = a define a macro then and the warning is to avoid you
>>> doing
>>
>>
>>
>>
>> I don't think this is a macro, b = a seems to be a way of setting the 
>> value of a to b. in the assembler. If a is an expression , then I 
>> believe the expression is resolved at assemble time - (b ends up being a 
>> symbol in the symbol table produced with the value of a) in this case 
>> the address of a. .set b, a achieves the same thing from my experiments 
>> and reading of the sources. The reason ports appear to choose not to use 
>> the .set a, b idiom is if the assembler syntax has hijacked the .set 
>> directive for something else. Thus I don't see why we use the 
>> ASM_OUTPUT_DEF form in the GNU/Linux port TBH rather than the .set form 
>> especially as we don't reuse .set for anything else in the ARM assembler 
>> port and SET_ASM_OP is defined in config/arm/aout.h.
>>
>> The use of .set in the arm port of glibc for assembler code for the same 
>> purpose seems to also vindicate that kind of thought.
>>  No reasons were given here[1], maybe Nick or Richard remember from 
>> nearly 18 years ago ;)
>>
>>
>> Therefore this seems to be an assembler bug to me in that it doesn't 
>> allow such an assignment of values, and a backend wart to me that we 
>> have ASM_OUTPUT_DEF defined for no good reason. So, a patch that removes 
>> ASM_OUTPUT_DEF from linux-elf.h seems obvious to me pending testing.
>>
>>
>> Nick , Richard - any thoughts ?
> 
> So - why does it warn at all for this?  And why does it only warn
> for b = a and not .set b, a?
> 

Because b is the mnemonic for a branch instruction.  What follows is
probably garbage in reality if you start from that point.

> IMHO the warning is simply bogus?

I think the grammar for the '=' assignment is at best dubious, given
that the LHS could be any label and there's no guarantee that won't
conflict with a mnemonic.  .set is clearly superior in that respect.

R.

> 
> Richard.
> 
>>
>> regards
>> Ramana
>>
>> 1. https://gcc.gnu.org/ml/gcc-patches/1998-10/msg00701.html
>>
>>>
>>> ...
>>>
>>>  ldr 0, 1 (or whatever correct ldr instruction)
>>>
>>> and have that ldr replaced by b?
>>>
>>> Then it's a bug to emit aliases in this form and I hope .set ldr, b
>>> doesn't have the same effect.
>>>
>>> Richard.
>>>
>>
>>
> 

Re: [PATCH] Fix PR69951

[Christophe Lyon]

On 1 March 2016 at 10:51, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
>> On Mon, 29 Feb 2016, James Greenhalgh wrote:
>>
>> > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
>> > >
>> > > The following fixes PR69951, hopefully the last case of decl alias
>> > > issues with alias analysis.  This time it's points-to and the DECL_UIDs
>> > > used in points-to sets not being canonicalized.
>> > >
>> > > The simplest (and cheapest) fix is to make aliases refer to the
>> > > ultimate alias target via their DECL_PT_UID which we conveniently
>> > > have available.
>> > >
>> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>> > >
>> > > Richard.
>> > >
>> > > 2016-02-26  Richard Biener  <rguenther@suse.de>
>> > >
>> > >   PR tree-optimization/69551
>> > >   * tree-ssa-structalias.c (get_constraint_for_ssa_var): When
>> > >   looking through aliases adjust DECL_PT_UID to refer to the
>> > >   ultimate alias target.
>> > >
>> > >   * gcc.dg/torture/pr69951.c: New testcase.
>> >
>> > I see this new testcase failing on an ARM target as so:
>> >
>> >     /tmp/ccChjoFc.s: Assembler messages:
>> >     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
>> >
>> >     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
>> >
>> > But I haven't managed to reproduce it outside of the test environment.
>> >
>> > The fix looks trivial, rename b to anything else you fancy (well... stay
>> > clear of add and ldr). I'll put a fix in myself if I can manage to get
>> > this to reproduce - though if anyone else wants to do it I won't be
>> > offended :-).
>>
>> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
>> C symbol name, too.  In fact my cross arm as doesn't report this
>> warning (binutils 2.25.0)
>>
>> > arm-suse-linux-gnueabi-as t.s -mwarn-syms
>> Assembler messages:
>> Error: unrecognized option -mwarn-syms
>
> Right, I've figured out the set of conditions... You need to be targeting
> an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
> from config/arm/linux-elf.h is pulled in. This causes us to emit:
>
> b = a
>
> Rather than
>
>         .set    b,a
>
> Writing it as "b = a" causes the warning added to resolve binutils
> PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
> that patch).
>
James,

What happens for you on arm-none-eabi configurations?
I'm using binutils-2.25, so I can't see this warning whatever the target.
However, I'm using qemu-arm and this test fails on arm-none-eabi,
because argc is set to 0 during the startup-code.

As I understand it, qemu-arm considers the code page as readonly,
and thus the GetCmdLine semi hosting call cannot write argc/argv
back to memory in the same code page (I'm using libgloss' crt0).

I tried to play with qemu-system-arm, but then libgloss' crt0 does not
set the reset vector and the simulation does random things, starting at
address 0.

Am I missing some newlib/libgloss configuration bits, or should I
send a newlib patch to address this?

Thanks

Christophe.


> Resolving it by hacking the testcase would be one fix, but I wonder why the
> ARM port prefers to emit "b = a" in a linux environment if .set does the
> same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
> remember? (AArch64 does the same thing, but the AArch64 gas port doesn't
> have the PR18347 fix).
>
> Cheers,
> James
>
> ---
>
> [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=18347
>
>>
>> Richard.
>>

Re: [PATCH] Fix PR69951

[James Greenhalgh]

On Tue, Mar 01, 2016 at 05:56:30PM +0100, Christophe Lyon wrote:
> On 1 March 2016 at 10:51, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> > On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
> >> On Mon, 29 Feb 2016, James Greenhalgh wrote:
> >>
> >> > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
> >> > >
> >> > > The following fixes PR69951, hopefully the last case of decl alias
> >> > > issues with alias analysis.  This time it's points-to and the DECL_UIDs
> >> > > used in points-to sets not being canonicalized.
> >> > >
> >> > > The simplest (and cheapest) fix is to make aliases refer to the
> >> > > ultimate alias target via their DECL_PT_UID which we conveniently
> >> > > have available.
> >> > >
> >> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> >> > >
> >> > > Richard.
> >> > >
> >> > > 2016-02-26  Richard Biener  <rguenther@suse.de>
> >> > >
> >> > >   PR tree-optimization/69551
> >> > >   * tree-ssa-structalias.c (get_constraint_for_ssa_var): When
> >> > >   looking through aliases adjust DECL_PT_UID to refer to the
> >> > >   ultimate alias target.
> >> > >
> >> > >   * gcc.dg/torture/pr69951.c: New testcase.
> >> >
> >> > I see this new testcase failing on an ARM target as so:
> >> >
> >> >     /tmp/ccChjoFc.s: Assembler messages:
> >> >     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
> >> >
> >> >     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
> >> >
> >> > But I haven't managed to reproduce it outside of the test environment.
> >> >
> >> > The fix looks trivial, rename b to anything else you fancy (well... stay
> >> > clear of add and ldr). I'll put a fix in myself if I can manage to get
> >> > this to reproduce - though if anyone else wants to do it I won't be
> >> > offended :-).
> >>
> >> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
> >> C symbol name, too.  In fact my cross arm as doesn't report this
> >> warning (binutils 2.25.0)
> >>
> >> > arm-suse-linux-gnueabi-as t.s -mwarn-syms
> >> Assembler messages:
> >> Error: unrecognized option -mwarn-syms
> >
> > Right, I've figured out the set of conditions... You need to be targeting
> > an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
> > from config/arm/linux-elf.h is pulled in. This causes us to emit:
> >
> > b = a
> >
> > Rather than
> >
> >         .set    b,a
> >
> > Writing it as "b = a" causes the warning added to resolve binutils
> > PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
> > that patch).
> >
> James,
> 
> What happens for you on arm-none-eabi configurations?
> I'm using binutils-2.25, so I can't see this warning whatever the target.
> However, I'm using qemu-arm and this test fails on arm-none-eabi,
> because argc is set to 0 during the startup-code.
> 
> As I understand it, qemu-arm considers the code page as readonly,
> and thus the GetCmdLine semi hosting call cannot write argc/argv
> back to memory in the same code page (I'm using libgloss' crt0).
> 
> I tried to play with qemu-system-arm, but then libgloss' crt0 does not
> set the reset vector and the simulation does random things, starting at
> address 0.
> 
> Am I missing some newlib/libgloss configuration bits, or should I
> send a newlib patch to address this?

Hi Christophe,

I didn't get this running under arm-none-eabi. The testcase does have
undefined behaviour (too few arguments to main), but I'd be surprised if
that was the issue... I'm sure the testcase could be rewritten to avoid
the dependence on argc if this proves an issue for other bare-metal
testers running under an emulator.

Thanks,
James

> > Resolving it by hacking the testcase would be one fix, but I wonder why the
> > ARM port prefers to emit "b = a" in a linux environment if .set does the
> > same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
> > remember? (AArch64 does the same thing, but the AArch64 gas port doesn't
> > have the PR18347 fix).
> >
> > Cheers,
> > James
> >
> > ---
> >
> > [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=18347
> >
> >>
> >> Richard.
> >>
> 

Re: [PATCH] Fix PR69951

[Christophe Lyon]

On 2 March 2016 at 10:16, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Tue, Mar 01, 2016 at 05:56:30PM +0100, Christophe Lyon wrote:
>> On 1 March 2016 at 10:51, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>> > On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
>> >> On Mon, 29 Feb 2016, James Greenhalgh wrote:
>> >>
>> >> > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
>> >> > >
>> >> > > The following fixes PR69951, hopefully the last case of decl alias
>> >> > > issues with alias analysis.  This time it's points-to and the DECL_UIDs
>> >> > > used in points-to sets not being canonicalized.
>> >> > >
>> >> > > The simplest (and cheapest) fix is to make aliases refer to the
>> >> > > ultimate alias target via their DECL_PT_UID which we conveniently
>> >> > > have available.
>> >> > >
>> >> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>> >> > >
>> >> > > Richard.
>> >> > >
>> >> > > 2016-02-26  Richard Biener  <rguenther@suse.de>
>> >> > >
>> >> > >   PR tree-optimization/69551
>> >> > >   * tree-ssa-structalias.c (get_constraint_for_ssa_var): When
>> >> > >   looking through aliases adjust DECL_PT_UID to refer to the
>> >> > >   ultimate alias target.
>> >> > >
>> >> > >   * gcc.dg/torture/pr69951.c: New testcase.
>> >> >
>> >> > I see this new testcase failing on an ARM target as so:
>> >> >
>> >> >     /tmp/ccChjoFc.s: Assembler messages:
>> >> >     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
>> >> >
>> >> >     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
>> >> >
>> >> > But I haven't managed to reproduce it outside of the test environment.
>> >> >
>> >> > The fix looks trivial, rename b to anything else you fancy (well... stay
>> >> > clear of add and ldr). I'll put a fix in myself if I can manage to get
>> >> > this to reproduce - though if anyone else wants to do it I won't be
>> >> > offended :-).
>> >>
>> >> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
>> >> C symbol name, too.  In fact my cross arm as doesn't report this
>> >> warning (binutils 2.25.0)
>> >>
>> >> > arm-suse-linux-gnueabi-as t.s -mwarn-syms
>> >> Assembler messages:
>> >> Error: unrecognized option -mwarn-syms
>> >
>> > Right, I've figured out the set of conditions... You need to be targeting
>> > an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
>> > from config/arm/linux-elf.h is pulled in. This causes us to emit:
>> >
>> > b = a
>> >
>> > Rather than
>> >
>> >         .set    b,a
>> >
>> > Writing it as "b = a" causes the warning added to resolve binutils
>> > PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
>> > that patch).
>> >
>> James,
>>
>> What happens for you on arm-none-eabi configurations?
>> I'm using binutils-2.25, so I can't see this warning whatever the target.
>> However, I'm using qemu-arm and this test fails on arm-none-eabi,
>> because argc is set to 0 during the startup-code.
>>
>> As I understand it, qemu-arm considers the code page as readonly,
>> and thus the GetCmdLine semi hosting call cannot write argc/argv
>> back to memory in the same code page (I'm using libgloss' crt0).
>>
>> I tried to play with qemu-system-arm, but then libgloss' crt0 does not
>> set the reset vector and the simulation does random things, starting at
>> address 0.
>>
>> Am I missing some newlib/libgloss configuration bits, or should I
>> send a newlib patch to address this?
>
> Hi Christophe,
>
> I didn't get this running under arm-none-eabi. The testcase does have
> undefined behaviour (too few arguments to main), but I'd be surprised if
> that was the issue... I'm sure the testcase could be rewritten to avoid
> the dependence on argc if this proves an issue for other bare-metal
> testers running under an emulator.
>

Indeed, I'm wondering why the testcase depends on argc beeing non-zero?

> Thanks,
> James
>
>> > Resolving it by hacking the testcase would be one fix, but I wonder why the
>> > ARM port prefers to emit "b = a" in a linux environment if .set does the
>> > same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
>> > remember? (AArch64 does the same thing, but the AArch64 gas port doesn't
>> > have the PR18347 fix).
>> >
>> > Cheers,
>> > James
>> >
>> > ---
>> >
>> > [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=18347
>> >
>> >>
>> >> Richard.
>> >>
>>

Re: [PATCH] Fix PR69951

[Christophe Lyon]

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

On 2 March 2016 at 10:49, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> On 2 March 2016 at 10:16, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>> On Tue, Mar 01, 2016 at 05:56:30PM +0100, Christophe Lyon wrote:
>>> On 1 March 2016 at 10:51, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>>> > On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
>>> >> On Mon, 29 Feb 2016, James Greenhalgh wrote:
>>> >>
>>> >> > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
>>> >> > >
>>> >> > > The following fixes PR69951, hopefully the last case of decl alias
>>> >> > > issues with alias analysis.  This time it's points-to and the DECL_UIDs
>>> >> > > used in points-to sets not being canonicalized.
>>> >> > >
>>> >> > > The simplest (and cheapest) fix is to make aliases refer to the
>>> >> > > ultimate alias target via their DECL_PT_UID which we conveniently
>>> >> > > have available.
>>> >> > >
>>> >> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>>> >> > >
>>> >> > > Richard.
>>> >> > >
>>> >> > > 2016-02-26  Richard Biener  <rguenther@suse.de>
>>> >> > >
>>> >> > >   PR tree-optimization/69551
>>> >> > >   * tree-ssa-structalias.c (get_constraint_for_ssa_var): When
>>> >> > >   looking through aliases adjust DECL_PT_UID to refer to the
>>> >> > >   ultimate alias target.
>>> >> > >
>>> >> > >   * gcc.dg/torture/pr69951.c: New testcase.
>>> >> >
>>> >> > I see this new testcase failing on an ARM target as so:
>>> >> >
>>> >> >     /tmp/ccChjoFc.s: Assembler messages:
>>> >> >     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
>>> >> >
>>> >> >     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
>>> >> >
>>> >> > But I haven't managed to reproduce it outside of the test environment.
>>> >> >
>>> >> > The fix looks trivial, rename b to anything else you fancy (well... stay
>>> >> > clear of add and ldr). I'll put a fix in myself if I can manage to get
>>> >> > this to reproduce - though if anyone else wants to do it I won't be
>>> >> > offended :-).
>>> >>
>>> >> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
>>> >> C symbol name, too.  In fact my cross arm as doesn't report this
>>> >> warning (binutils 2.25.0)
>>> >>
>>> >> > arm-suse-linux-gnueabi-as t.s -mwarn-syms
>>> >> Assembler messages:
>>> >> Error: unrecognized option -mwarn-syms
>>> >
>>> > Right, I've figured out the set of conditions... You need to be targeting
>>> > an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
>>> > from config/arm/linux-elf.h is pulled in. This causes us to emit:
>>> >
>>> > b = a
>>> >
>>> > Rather than
>>> >
>>> >         .set    b,a
>>> >
>>> > Writing it as "b = a" causes the warning added to resolve binutils
>>> > PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
>>> > that patch).
>>> >
>>> James,
>>>
>>> What happens for you on arm-none-eabi configurations?
>>> I'm using binutils-2.25, so I can't see this warning whatever the target.
>>> However, I'm using qemu-arm and this test fails on arm-none-eabi,
>>> because argc is set to 0 during the startup-code.
>>>
>>> As I understand it, qemu-arm considers the code page as readonly,
>>> and thus the GetCmdLine semi hosting call cannot write argc/argv
>>> back to memory in the same code page (I'm using libgloss' crt0).
>>>
>>> I tried to play with qemu-system-arm, but then libgloss' crt0 does not
>>> set the reset vector and the simulation does random things, starting at
>>> address 0.
>>>
>>> Am I missing some newlib/libgloss configuration bits, or should I
>>> send a newlib patch to address this?
>>
>> Hi Christophe,
>>
>> I didn't get this running under arm-none-eabi. The testcase does have
>> undefined behaviour (too few arguments to main), but I'd be surprised if
>> that was the issue... I'm sure the testcase could be rewritten to avoid
>> the dependence on argc if this proves an issue for other bare-metal
>> testers running under an emulator.
>>
>
> Indeed, I'm wondering why the testcase depends on argc beeing non-zero?
>

To avoid modifying the testcase too much, I propose to replace
if (argc)
by
if (argc >= 0)
as in the attached patch.
This does make the trick on arm-non-eabi.

OK?

Christophe.


>> Thanks,
>> James
>>
>>> > Resolving it by hacking the testcase would be one fix, but I wonder why the
>>> > ARM port prefers to emit "b = a" in a linux environment if .set does the
>>> > same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
>>> > remember? (AArch64 does the same thing, but the AArch64 gas port doesn't
>>> > have the PR18347 fix).
>>> >
>>> > Cheers,
>>> > James
>>> >
>>> > ---
>>> >
>>> > [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=18347
>>> >
>>> >>
>>> >> Richard.
>>> >>
>>>

[-- Attachment #2: pr69951-test.log.txt --]
[-- Type: text/plain, Size: 104 bytes --]

2016-03-04  Christophe Lyon  <christophe.lyon@linaro.org>

	* gcc.dg/torture/pr69951.c: Accept argc==0.

[-- Attachment #3: pr69951-test.patch.txt --]
[-- Type: text/plain, Size: 390 bytes --]

diff --git a/gcc/testsuite/gcc.dg/torture/pr69951.c b/gcc/testsuite/gcc.dg/torture/pr69951.c
index cb46fef..be9a027 100644
--- a/gcc/testsuite/gcc.dg/torture/pr69951.c
+++ b/gcc/testsuite/gcc.dg/torture/pr69951.c
@@ -9,7 +9,7 @@ extern int d __attribute__((alias("c")));
 int main(int argc)
 {
   int *p, *q;
-  if (argc)
+  if (argc >= 0)
     p = &c, q = &d;
   else
     p = &b, q = &d;

Re: [PATCH] Fix PR69951

[Richard Biener]

On Thu, 3 Mar 2016, Christophe Lyon wrote:

> On 2 March 2016 at 10:49, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> > On 2 March 2016 at 10:16, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> >> On Tue, Mar 01, 2016 at 05:56:30PM +0100, Christophe Lyon wrote:
> >>> On 1 March 2016 at 10:51, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> >>> > On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
> >>> >> On Mon, 29 Feb 2016, James Greenhalgh wrote:
> >>> >>
> >>> >> > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
> >>> >> > >
> >>> >> > > The following fixes PR69951, hopefully the last case of decl alias
> >>> >> > > issues with alias analysis.  This time it's points-to and the DECL_UIDs
> >>> >> > > used in points-to sets not being canonicalized.
> >>> >> > >
> >>> >> > > The simplest (and cheapest) fix is to make aliases refer to the
> >>> >> > > ultimate alias target via their DECL_PT_UID which we conveniently
> >>> >> > > have available.
> >>> >> > >
> >>> >> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> >>> >> > >
> >>> >> > > Richard.
> >>> >> > >
> >>> >> > > 2016-02-26  Richard Biener  <rguenther@suse.de>
> >>> >> > >
> >>> >> > >   PR tree-optimization/69551
> >>> >> > >   * tree-ssa-structalias.c (get_constraint_for_ssa_var): When
> >>> >> > >   looking through aliases adjust DECL_PT_UID to refer to the
> >>> >> > >   ultimate alias target.
> >>> >> > >
> >>> >> > >   * gcc.dg/torture/pr69951.c: New testcase.
> >>> >> >
> >>> >> > I see this new testcase failing on an ARM target as so:
> >>> >> >
> >>> >> >     /tmp/ccChjoFc.s: Assembler messages:
> >>> >> >     /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
> >>> >> >
> >>> >> >     FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
> >>> >> >
> >>> >> > But I haven't managed to reproduce it outside of the test environment.
> >>> >> >
> >>> >> > The fix looks trivial, rename b to anything else you fancy (well... stay
> >>> >> > clear of add and ldr). I'll put a fix in myself if I can manage to get
> >>> >> > this to reproduce - though if anyone else wants to do it I won't be
> >>> >> > offended :-).
> >>> >>
> >>> >> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
> >>> >> C symbol name, too.  In fact my cross arm as doesn't report this
> >>> >> warning (binutils 2.25.0)
> >>> >>
> >>> >> > arm-suse-linux-gnueabi-as t.s -mwarn-syms
> >>> >> Assembler messages:
> >>> >> Error: unrecognized option -mwarn-syms
> >>> >
> >>> > Right, I've figured out the set of conditions... You need to be targeting
> >>> > an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
> >>> > from config/arm/linux-elf.h is pulled in. This causes us to emit:
> >>> >
> >>> > b = a
> >>> >
> >>> > Rather than
> >>> >
> >>> >         .set    b,a
> >>> >
> >>> > Writing it as "b = a" causes the warning added to resolve binutils
> >>> > PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
> >>> > that patch).
> >>> >
> >>> James,
> >>>
> >>> What happens for you on arm-none-eabi configurations?
> >>> I'm using binutils-2.25, so I can't see this warning whatever the target.
> >>> However, I'm using qemu-arm and this test fails on arm-none-eabi,
> >>> because argc is set to 0 during the startup-code.
> >>>
> >>> As I understand it, qemu-arm considers the code page as readonly,
> >>> and thus the GetCmdLine semi hosting call cannot write argc/argv
> >>> back to memory in the same code page (I'm using libgloss' crt0).
> >>>
> >>> I tried to play with qemu-system-arm, but then libgloss' crt0 does not
> >>> set the reset vector and the simulation does random things, starting at
> >>> address 0.
> >>>
> >>> Am I missing some newlib/libgloss configuration bits, or should I
> >>> send a newlib patch to address this?
> >>
> >> Hi Christophe,
> >>
> >> I didn't get this running under arm-none-eabi. The testcase does have
> >> undefined behaviour (too few arguments to main), but I'd be surprised if
> >> that was the issue... I'm sure the testcase could be rewritten to avoid
> >> the dependence on argc if this proves an issue for other bare-metal
> >> testers running under an emulator.
> >>
> >
> > Indeed, I'm wondering why the testcase depends on argc beeing non-zero?
> >
> 
> To avoid modifying the testcase too much, I propose to replace
> if (argc)
> by
> if (argc >= 0)
> as in the attached patch.
> This does make the trick on arm-non-eabi.
> 
> OK?

Ok.

Richard.

> Christophe.
> 
> 
> >> Thanks,
> >> James
> >>
> >>> > Resolving it by hacking the testcase would be one fix, but I wonder why the
> >>> > ARM port prefers to emit "b = a" in a linux environment if .set does the
> >>> > same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
> >>> > remember? (AArch64 does the same thing, but the AArch64 gas port doesn't
> >>> > have the PR18347 fix).
> >>> >
> >>> > Cheers,
> >>> > James
> >>> >
> >>> > ---
> >>> >
> >>> > [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=18347
> >>> >
> >>> >>
> >>> >> Richard.
> >>> >>
> >>>
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

[Patch ARM] Delete ASM_OUTPUT_DEF and fall back to default .set directive

[James Greenhalgh]

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


Hi,

gcc.dg/torture/pr69951.c has been failing for arm*-*-linux* targets, as
we put out "b = a" as a way of defining a symbol alias, which trips an
assembler warning if the left hand side is an instruction name (such as 'b'
for branch, see [1] for context).

We don't want to do this, a simple .set directive will suffice. This can
be achieved by deleting the definition of ASM_OUTPUT_DEF in
config/arm/linux-elf.h . This will cause us to fall back to the default
definition, as we have SET_ASM_OP defined through config/elfos.h for all
ARM ports. This patch makes that change.

Built and tested on an arm-none-linux-gnueabihf box with no issues.

OK?

Thanks,
James

---
2016-03-31  James Greenhalgh  <james.greenhalgh@arm.com>

	* config/arm/linux-elf.h (ASM_OUTPUT_DEF): Delete.

---

[1]: https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00018.html


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Patch-ARM-Delete-ASM_OUTPUT_DEF-and-fall-back-to-def.patch --]
[-- Type: text/x-patch; name="0001-Patch-ARM-Delete-ASM_OUTPUT_DEF-and-fall-back-to-def.patch", Size: 609 bytes --]

diff --git a/gcc/config/arm/linux-elf.h b/gcc/config/arm/linux-elf.h
index 472141d..a94bd2d 100644
--- a/gcc/config/arm/linux-elf.h
+++ b/gcc/config/arm/linux-elf.h
@@ -85,17 +85,6 @@
     }						\
   while (0)
 
-/* This is how we tell the assembler that two symbols have the same value.  */
-#define ASM_OUTPUT_DEF(FILE, NAME1, NAME2) \
-  do					   \
-    {					   \
-      assemble_name (FILE, NAME1); 	   \
-      fputs (" = ", FILE);		   \
-      assemble_name (FILE, NAME2);	   \
-      fputc ('\n', FILE);		   \
-    }					   \
-  while (0)
-
 #undef  FPUTYPE_DEFAULT
 #define FPUTYPE_DEFAULT "vfp"
 

Re: [Patch ARM] Delete ASM_OUTPUT_DEF and fall back to default .set directive

[Ramana Radhakrishnan]

On 31/03/16 16:41, James Greenhalgh wrote:
> 
> Hi,
> 
> gcc.dg/torture/pr69951.c has been failing for arm*-*-linux* targets, as
> we put out "b = a" as a way of defining a symbol alias, which trips an
> assembler warning if the left hand side is an instruction name (such as 'b'
> for branch, see [1] for context).
> 
> We don't want to do this, a simple .set directive will suffice. This can
> be achieved by deleting the definition of ASM_OUTPUT_DEF in
> config/arm/linux-elf.h . This will cause us to fall back to the default
> definition, as we have SET_ASM_OP defined through config/elfos.h for all
> ARM ports. This patch makes that change.
> 
> Built and tested on an arm-none-linux-gnueabihf box with no issues.
> 
> OK?


Ok.

Ramana
> 
> Thanks,
> James
> 
> ---
> 2016-03-31  James Greenhalgh  <james.greenhalgh@arm.com>
> 
> 	* config/arm/linux-elf.h (ASM_OUTPUT_DEF): Delete.
> 
> ---
> 
> [1]: https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00018.html
>