diff mbox

Fix PR69951

Message ID CAKdteOb+A2UB21=z1c9YfRWq1dpBrpQsjNqQFq06Hmq_n5APcw@mail.gmail.com
State New
Headers show

Commit Message

Christophe Lyon March 3, 2016, 7:57 p.m. UTC 
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.

>>> >>

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

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

Patch

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;