public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: Segher Boessenkool <segher@kernel.crashing.org>,
	Dimitar Dimitrov	<dimitar@dinux.eu>,
	Christophe Lyon <christophe.lyon@linaro.org>,
	"Thomas Preudhomme" <thomas.preudhomme@linaro.org>,
	"gcc-patches@gcc.gnu.org"	<gcc-patches@gcc.gnu.org>,
	"richard.sandiford@arm.com"	<richard.sandiford@arm.com>,
	"gdb@sourceware.org" <gdb@sourceware.org>
Subject: Re: [PATCH] [RFC] PR target/52813 and target/11807
Date: Mon, 17 Dec 2018 20:15:00 -0000	[thread overview]
Message-ID: <DB7PR07MB53539859098ACF34CF4C5A55E4BC0@DB7PR07MB5353.eurprd07.prod.outlook.com> (raw)
In-Reply-To: <871s6g0z5z.fsf@arm.com>

On 12/17/18 7:46 PM, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
>> On Mon, Dec 17, 2018 at 11:47:42AM +0000, Richard Sandiford wrote:
>>> Dimitar Dimitrov <dimitar@dinux.eu> writes:
>>>> On Sun, Dec 16 2018 at 14:36:26 EET Bernd Edlinger wrote:
>>>>> Hi,
>>>>>
>>>>> if I understood that right, then clobbering sp is and has always been
>>>>> ignored.
>>>
>>> PR77904 was about the clobber not being ignored, so the behaviour
>>> hasn't been consistent.
>>>
>>> I'm also not sure it was always ignored in recent sources.  The clobber
>>> does get added to the associated rtl insn, and it'd be surprising if
>>> that never had an effect.
>>
>> Yes, you will usually get a frame pointer.  My point was that the epilogue
>> will restore your stack pointer both with and without the asm clobber.
> 
> I'm not confident that's the only effect though.
> 
> Also, we didn't use a frame in PR77904, and using a frame would have
> been the wrong thing to do.
> 
>>> I don't think there's a good reason to treat this differently from the
>>> preexisting PIC register error.  If the argument for making it a warning
>>> rather than an error is that the asm might happen to work by accident,
>>> then the same is true for the PIC register.
>>
>> Yes.  As well as quite a few more registers, many of those specific to
>> the target.  And there are many more things you can do terribly wrong in
>> inline assembler, of course, most of which we can never detect.
> 
> Right.  And I don't think anyone's suggesting GCC can detect everything.
> It can only police the things it knows about, which include the input,
> output and clobber clauses.
> 
> What makes the PIC register and sp worth special attention is that
> changing their values would in general invalidate other code that GCC
> generates itself.  It's not just about whether the asm has the effect
> the author wanted (whatever that was).
> 
> FWIW, I don't think we should go on a proactive hunt for other registers
> to complain about.
> 

out of curiosity I looked at the clobber statement in gdb/nat/linux-ptrace.c:

           asm volatile ("pushq %0;"
                         ".globl linux_ptrace_test_ret_to_nx_instr;"
                         "linux_ptrace_test_ret_to_nx_instr:"
                         "ret"
                         : : "r" ((uint64_t) (uintptr_t) return_address)
                         : "%rsp", "memory");

it turns out to be a far jump, instruction.  And I wanted to find out what
removing the %rsp clobber actually does.  First there is a technical problem
with that, because gcc does not print an error, it is possbile to compile the
code without the sp clobber, but not to compare the code that would have been
generated if the error would only be a warning.  So I had to undo the patch
in order to see, what the sp clobber actually does, and I think Segher
mentioned that this might have an influence on the frame pointer, that turns
out to be true:

diff  linux-ptrace-spclobber.dis  linux-ptrace-noclobber.dis

449,590c449,593
<  5c0: 55                      push   %rbp
<  5c1: 45 31 c9                xor    %r9d,%r9d
<  5c4: 41 b8 ff ff ff ff       mov    $0xffffffff,%r8d
<  5ca: b9 22 00 00 00          mov    $0x22,%ecx
<  5cf: ba 03 00 00 00          mov    $0x3,%edx
<  5d4: be 02 00 00 00          mov    $0x2,%esi
<  5d9: 31 ff                   xor    %edi,%edi
<  5db: 48 89 e5                mov    %rsp,%rbp
<  5de: 41 57                   push   %r15
<  5e0: 41 56                   push   %r14
<  5e2: 41 55                   push   %r13
<  5e4: 41 54                   push   %r12
<  5e6: 53                      push   %rbx
<  5e7: 48 81 ec f8 00 00 00    sub    $0xf8,%rsp
[snip]
---
>  5c0: 41 56                   push   %r14
>  5c2: 45 31 c9                xor    %r9d,%r9d
>  5c5: 41 b8 ff ff ff ff       mov    $0xffffffff,%r8d
>  5cb: b9 22 00 00 00          mov    $0x22,%ecx
>  5d0: 41 55                   push   %r13
>  5d2: ba 03 00 00 00          mov    $0x3,%edx
>  5d7: be 02 00 00 00          mov    $0x2,%esi
>  5dc: 31 ff                   xor    %edi,%edi
>  5de: 41 54                   push   %r12
>  5e0: 55                      push   %rbp
>  5e1: 53                      push   %rbx
>  5e2: 48 81 ec f0 00 00 00    sub    $0xf0,%rsp


So for me this looks not at all trivial to see if this
would work without the sp clobber, since the stack frame
might be completely different without that sp clobber.

I wonder what gdb developers think about the sp clobber
here, if it is easy to fix or if it gives trouble to them.


Thanks
Bernd.

  reply	other threads:[~2018-12-17 20:15 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-16 14:36 Bernd Edlinger
2018-12-16 16:14 ` Dimitar Dimitrov
2018-12-17 11:47   ` Richard Sandiford
2018-12-17 12:54     ` Christophe Lyon
2018-12-17 13:35       ` Richard Sandiford
2018-12-17 13:42         ` Christophe Lyon
2018-12-17 14:05           ` Bernd Edlinger
2018-12-17 14:10         ` Bernd Edlinger
2018-12-17 15:55     ` Segher Boessenkool
2018-12-17 18:46       ` Richard Sandiford
2018-12-17 20:15         ` Bernd Edlinger [this message]
2018-12-19  6:40           ` Dimitar Dimitrov
2018-12-19  9:29             ` Segher Boessenkool
2018-12-18 14:16     ` Bernd Edlinger
2018-12-18 15:14       ` Bernd Edlinger
2019-01-07  9:23   ` Jakub Jelinek
2019-01-07 21:51     ` Bernd Edlinger
2019-01-08 12:03       ` Richard Sandiford
2019-01-10 13:21         ` Segher Boessenkool
2019-01-10 21:23           ` Richard Sandiford
2019-01-10 21:26             ` Jakub Jelinek
2019-01-10 21:56               ` Richard Sandiford
2019-01-11 12:26                 ` Segher Boessenkool
2019-01-10 22:32             ` Bernd Edlinger
2019-01-11 12:18             ` Segher Boessenkool
2019-01-11 12:23               ` Richard Sandiford
2019-01-11 22:59         ` Jeff Law
2019-01-17 14:27           ` Christophe Lyon
2019-01-18  9:49             ` Richard Sandiford
  -- strict thread matches above, loose matches on Subject: below --
2018-12-09 10:09 Dimitar Dimitrov
2018-12-10 11:21 ` Richard Sandiford
2018-12-10 19:36   ` Dimitar Dimitrov
2018-12-11 15:52     ` Richard Sandiford
2018-12-12  9:42       ` Christophe Lyon
2018-12-12 10:03         ` Christophe Lyon
2018-12-12 16:39           ` Dimitar Dimitrov
2018-12-12 10:30         ` Thomas Preudhomme
2018-12-12 11:21           ` Thomas Preudhomme
2018-12-12 13:19             ` Christophe Lyon
2018-12-12 15:13               ` Christophe Lyon
2018-12-12 15:35                 ` Thomas Preudhomme
2018-12-12 16:26               ` Dimitar Dimitrov
2018-12-13 14:49                 ` Segher Boessenkool
2018-12-13 22:21                   ` Dimitar Dimitrov
2018-12-14  8:52                     ` Segher Boessenkool
2018-12-16  8:43                       ` Dimitar Dimitrov
2018-12-17 15:23                         ` Segher Boessenkool
2018-12-14 13:49               ` Richard Sandiford
2018-12-15 15:38                 ` Segher Boessenkool
2018-12-12 11:24 ` Andreas Schwab

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB7PR07MB53539859098ACF34CF4C5A55E4BC0@DB7PR07MB5353.eurprd07.prod.outlook.com \
    --to=bernd.edlinger@hotmail.de \
    --cc=christophe.lyon@linaro.org \
    --cc=dimitar@dinux.eu \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gdb@sourceware.org \
    --cc=richard.sandiford@arm.com \
    --cc=segher@kernel.crashing.org \
    --cc=thomas.preudhomme@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).