[PATCH v2 AArch64]: Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern.
Tue Sep 16 22:03:00 GMT 2014
On Tue, Sep 16, 2014 at 10:36:08PM +0100, Andrew Pinski wrote:
> On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh
> <firstname.lastname@example.org> wrote:
> > On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote:
> >> Hi maintainers,
> >> I just added "=r" and retested it.
> > I had a very similar patch to this sitting in my local tree. However,
> > I am surprised you have left operand 3 as an output operand. In my tree
> > I had marked operand 3 as "&r".
> > What do you think?
> The clobber needs to be "=&r" as you are writing to the register and
> not just reading from it. I think this is causing some issues
> including linaro bugzilla #667
(+CC Matthias Klose and Steve McIntyre who have also been in contact with me
regarding this bug)
I've seen this bug locally, and had considered sending the patch you
suggested, which does indeed fix the bug. However, it feels wrong as
the operand is not a formal output of the pattern. It is clobbered - and
indeed earlyclobbered - so yes it is written to, but it isn't an output.
This makes the fix look like a band-aid around the real problem.
The bug looks similar to pr52573 - regrename fails to spot that it should
not rename to a register used in an earlyclobber operand of any type, rather
than just an output+earlyclobber operand as it does now.
I've played about with a fix that sits in regrename, and forces it to think
of all earlyclobber operands as starting and ending chains but this didn't
bootstrap clean - we end up with what I believe are false reports of stack
smashing in libstdc++.
I was planning to look again at my approach tomorrow, I would like to
convince myself that this isn't a deficiency in regrename before I would
support just marking this operand "=&r".
If you have any other suggestions, or if "=&r" is actually correct and
I am misreading the documentation please let me know.
More information about the Gcc-patches