[Bug ports/16169] New: AArch64 clone.S is missing some CFI markup

[Bug ports/16169] New: AArch64 clone.S is missing some CFI markup

[tromey at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

            Bug ID: 16169
           Summary: AArch64 clone.S is missing some CFI markup
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ports
          Assignee: unassigned at sourceware dot org
          Reporter: tromey at redhat dot com
                CC: carlos at redhat dot com, roland at gnu dot org

Created attachment 7277
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7277&action=edit
untested patch

While debugging bug #16155 I noticed that the AArch64
clone.S does not properly note that the stack ends.
Other arches (I checked ARM and x86-64) do this with
CFI directives.

I'm attaching a completely untested (not even compiled)
patch that shows what I think ought to be done.  The
key bit is marking the link register as unavailable.


Also, I noticed that the AArch64 clone.S does not
implement this part of AAPCS64:

"The end of the frame record chain is indicated by the address zero in the
address for the previous frame."

This is in Section 5.2.3 The Frame Pointer.

At least based on my reading of the x86-64 clone.S, the
code run in the new clone ought to ensure this; but I am
not completely certain.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[tromey at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Tom Tromey <tromey at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://sourceware.org/bugz
                   |                            |illa/show_bug.cgi?id=16155

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[carlos at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Carlos O'Donell <carlos at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marcus.shawcroft at linaro dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[carlos at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Carlos O'Donell <carlos at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |review?(marcus.shawcroft@li
                   |                            |naro.org)

--- Comment #1 from Carlos O'Donell <carlos at redhat dot com> ---
Marcus,

Do you know if anyone has patches to fix this already at Linaro?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[ryan.arnold at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Ryan S. Arnold <ryan.arnold at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ryan.arnold at linaro dot org

--- Comment #2 from Ryan S. Arnold <ryan.arnold at linaro dot org> ---
Carlos, as far as I know we don't currently have a patch to fix this.

Tom

Shouldn't that first cfi_endproc be located before the system call?

        /* X0:flags, x1:newsp, x2:parenttidptr, x3:newtls, x4:childtid.  */
        cfi_endproc
        svc     0x0

The rest of the patch look valid/correct.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[marcus.shawcroft at linaro dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #3 from marcus.shawcroft at linaro dot org <marcus.shawcroft at linaro dot org> ---
I'm not aware of a fix for this at the moment.

On 13 November 2013 22:32,  <sourceware-bugzilla@sourceware.org> wrote:
>
> Carlos O'Donell <carlos@redhat.com> has asked marcus.shawcroft@linaro.org
> <marcus.shawcroft@linaro.org> for review:
> Bug 16169: AArch64 clone.S is missing some CFI markup
> https://sourceware.org/bugzilla/show_bug.cgi?id=16169
>
>
> ------- Additional Comments from Carlos O'Donell <carlos@redhat.com>
> Marcus,
>
> Do you know if anyone has patches to fix this already at Linaro?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[tromey at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #4 from Tom Tromey <tromey at redhat dot com> ---
(In reply to Ryan S. Arnold from comment #2)

> Shouldn't that first cfi_endproc be located before the system call?
> 
>         /* X0:flags, x1:newsp, x2:parenttidptr, x3:newtls, x4:childtid.  */
>         cfi_endproc
>         svc     0x0
> 
> The rest of the patch look valid/correct.

For the placement of the cfi_endproc, I followed ARM,
since I ran across this:

https://sourceware.org/ml/libc-ports/2010-01/msg00014.html

But there doesn't seem to be any consistency, as x86-64 puts
it before the syscall.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[ryan.arnold at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #5 from Ryan S. Arnold <ryan.arnold at linaro dot org> ---
(In reply to Tom Tromey from comment #4)
> (In reply to Ryan S. Arnold from comment #2)
> 
> > Shouldn't that first cfi_endproc be located before the system call?
> > 
> >         /* X0:flags, x1:newsp, x2:parenttidptr, x3:newtls, x4:childtid.  */
> >         cfi_endproc
> >         svc     0x0
> > 
> > The rest of the patch look valid/correct.
> 
> For the placement of the cfi_endproc, I followed ARM,
> since I ran across this:
> 
> https://sourceware.org/ml/libc-ports/2010-01/msg00014.html
> 
> But there doesn't seem to be any consistency, as x86-64 puts
> it before the syscall.

Yeah, I was looking at the x86_64 example as well.  PowerPC does the same as
x86.  It seems that other examples of using cfi_endproc are inclusive of the
instructions which exit the current routine.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[tromey at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #6 from Tom Tromey <tromey at redhat dot com> ---
Just a friendly ping -- I wasn't sure what to do with this patch.
I can write up a ChangeLog and submit it if you want; but I don't
have any particularly good way to test it (even compile it) right now.
I suppose it would be best if you tested it, as I assume you're
probably already set up for that.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[ryan.arnold at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #7 from Ryan S. Arnold <ryan.arnold at linaro dot org> ---
(In reply to Tom Tromey from comment #6)
> Just a friendly ping -- I wasn't sure what to do with this patch.
> I can write up a ChangeLog and submit it if you want; but I don't
> have any particularly good way to test it (even compile it) right now.
> I suppose it would be best if you tested it, as I assume you're
> probably already set up for that.

Hi Tom,

I'll test the patch soon (once I have an Aarch64 environment setup).

I'll be creating a patch for zeroing the stack pointer as well.

Ryan

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[jan.kratochvil at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Jan Kratochvil <jan.kratochvil at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jan.kratochvil at redhat dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[mjw at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Mark Wielaard <mjw at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjw at redhat dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[mjw at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #8 from Mark Wielaard <mjw at redhat dot com> ---
I haven't tested this patch yet, but I noticed the same issue when trying to
create an unwinder for aarch64. Not clearing the cfi for the link register in
clone caused some trouble finding the end of the call stack in threads. If
this, or a similar patch to end the cfi, could be applied that would be
appreciated.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[carlos at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Carlos O'Donell <carlos at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |2.19

--- Comment #9 from Carlos O'Donell <carlos at redhat dot com> ---
(In reply to Mark Wielaard from comment #8)
> I haven't tested this patch yet, but I noticed the same issue when trying to
> create an unwinder for aarch64. Not clearing the cfi for the link register
> in clone caused some trouble finding the end of the call stack in threads.
> If this, or a similar patch to end the cfi, could be applied that would be
> appreciated.

I strongly suggest Tom repost to libc-alpha and CC Marcus Shawcroft
<marcus.shawcroft@linaro.org> for review since he can test it. Indicate that
this is a bug and should be fixed before 2.19 goes out the door.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[tromey at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #10 from Tom Tromey <tromey at redhat dot com> ---
> I strongly suggest Tom repost to libc-alpha and CC Marcus Shawcroft
> <marcus.shawcroft@linaro.org> for review since he can test it. Indicate that
> this is a bug and should be fixed before 2.19 goes out the door.

Thanks Carlos.
I'm sending it now.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[cvs-commit at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #11 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  3a3acb6afc753475675b5724f206e619d0c9590d (commit)
       via  b67e7fe0f9f9b0f6c4386a0eccba83178098438c (commit)
      from  91cbd0bc72387d0729136e44002082fbdd9cd42d (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=3a3acb6afc753475675b5724f206e619d0c9590d

commit 3a3acb6afc753475675b5724f206e619d0c9590d
Author: Tom Tromey <tromey@redhat.com>
Date:   Mon Jan 20 12:58:03 2014 +0000

    [AArch64] BZ #16169 Add CFI directives to clone.S

    [BZ #16169] Add CFI directives to the AArch64 clone.S implementation
    and ensure that the FP in the child is zero'd in order to comply with
    AAPCS.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=b67e7fe0f9f9b0f6c4386a0eccba83178098438c

commit b67e7fe0f9f9b0f6c4386a0eccba83178098438c
Author: Marcus Shawcroft <marcus.shawcroft@linaro.org>
Date:   Wed Jan 1 17:29:23 2014 +0000

    [AArch64] Define BE loader name.

-----------------------------------------------------------------------

Summary of changes:
 NEWS                                               |    8 ++--
 ports/ChangeLog.aarch64                            |   18 ++++++++++
 .../powerpc64 => ports/sysdeps/aarch64}/configure  |   34 ++++++++++++-------
 ports/sysdeps/aarch64/configure.ac                 |   22 +++++++++++++
 ports/sysdeps/aarch64/shlib-versions               |    5 +++
 ports/sysdeps/unix/sysv/linux/aarch64/Makefile     |   15 +++++++++
 ports/sysdeps/unix/sysv/linux/aarch64/clone.S      |    7 +++-
 ports/sysdeps/unix/sysv/linux/aarch64/ldconfig.h   |    1 +
 8 files changed, 92 insertions(+), 18 deletions(-)
 copy {sysdeps/unix/sysv/linux/powerpc/powerpc64 =>
ports/sysdeps/aarch64}/configure (84%)
 create mode 100644 ports/sysdeps/aarch64/configure.ac

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[cvs-commit at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=16169

--- Comment #12 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, release/2.18/master has been updated
       via  c616775d2c6df31786b71b4e135fba90b4a63f89 (commit)
      from  2a8396b962b8c57a2581023b62766a3ac44c3ba4 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c616775d2c6df31786b71b4e135fba90b4a63f89

commit c616775d2c6df31786b71b4e135fba90b4a63f89
Author: Tom Tromey <tromey@redhat.com>
Date:   Mon Jan 20 12:58:03 2014 +0000

    [AArch64] BZ #16169 Add CFI directives to clone.S

    [BZ #16169] Add CFI directives to the AArch64 clone.S implementation
    and ensure that the FP in the child is zero'd in order to comply with
    AAPCS.
    (cherry picked from commit 3a3acb6afc753475675b5724f206e619d0c9590d)

    Conflicts:

        NEWS

-----------------------------------------------------------------------

Summary of changes:
 NEWS                                          |    2 +-
 ports/ChangeLog.aarch64                       |    6 ++++++
 ports/sysdeps/unix/sysv/linux/aarch64/clone.S |    7 ++++++-
 3 files changed, 13 insertions(+), 2 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[marcus.shawcroft at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Marcus Shawcroft <marcus.shawcroft at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Marcus Shawcroft <marcus.shawcroft at linaro dot org> ---
Resolved on master.
Back ported to 2.18

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug ports/16169] AArch64 clone.S is missing some CFI markup

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16169

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://sourceware.org/bugz
                   |                            |illa/show_bug.cgi?id=26853

-- 
You are receiving this mail because:
You are on the CC list for the bug.