Re: FESCo revote on "Add -fno-omit-frame-pointer" Change proposal [was Re: Schedule for Tuesday's FESCo Meeting (2023-01-03)]

[Daan De Meyer]

>  I think it would save everyone a bit of time if we restricted the
> change
>  to x86-64.  We do not have much experience with the -mbackchain flag
>  that was added at the last minute on s390x.  The change owners have
>  stated that they aren't interested in s390x.  IBM doesn't want this.
>  Platform Tools does not want it.  I doubt the desktop team does GNOME
>  performance analysis on s390x on Fedora.  I'm not even sure if the tools
>  support backchain-based unwinding; it's not a frame pointer after all.
>  Maybe -mbackchain won't cause any issues in after all, but we just don't
>  have the time to test this before the mass rebuild. 
I had a look at the kernel unwinding code for s390 and it seems to use
the backchain if it's available and fall back to the frame pointer otherwise.
So from our end (change proposal authors) we're OK with dropping
mbackchain for s390 and only using fno-omit-frame-pointer for s390.
We'll open a PR to change this in the rpm macros.

>  As Jakub and I have repeatedly explained, -fno-omit-frame-pointer on
>  i686 is known to break certain packages (although I worked around this
>  in glibc last year), simply because the reduced number of registers
>  makes it impossible for GCC to compile certain functions with inline
>  assembly in them.  As with s390x, the concrete impact is not known at
>  this point, and we are out of time for test builds. 
Given these issues should manifest as compilation failures, we should notice
very clearly once the mass rebuild starts if there's a bigger problem. If there's
only a few packages that run into issues, they can opt-out. If there's larger
problems,
we can remove frame pointers from i686.

>  Using -mno-omit-leaf-frame-pointer for aarch64 seems to be another
>  last-minute addition without any clear justification.  (On AArch64, the
>  link register allows one to recover the address of the immediate caller
>  even if a leaf function does not have a frame pointer.  That's not
>  possible on x86-64, where the caller's address must be read from the
>  stack, and that has to be based on the frame pointer.)  Just because the
>  compiler option is there to enable doesn't mean it does anything useful
>  in this context. 
As I mentioned in the fesco ticket, the kernel unwinder looks at the
frame pointer register (x29) first when starting an unwind on aarch64 before looking
at the link register. As such, it seems logical to require frame pointers to be available
in leaf functions so the frame pointer register is available to start unwinding. I'm
happy
to be proven wrong here so we can remove mno-omit-leaf-frame-pointer for aarch64.

Cheers,

Daan De Meyer

________________________________________
From: Daan De Meyer <daandemeyer(a)meta.com&gt;
Sent: 09 January 2023 19:21
To: Matthew Miller; Development discussions related to Fedora
Subject: Re: FESCo revote on "Add -fno-omit-frame-pointer" Change proposal [was
Re: Schedule for Tuesday's FESCo Meeting (2023-01-03)]

>  I think it would save everyone a bit of time if we restricted the
> change
>  to x86-64.  We do not have much experience with the -mbackchain flag
>  that was added at the last minute on s390x.  The change owners have
>  stated that they aren't interested in s390x.  IBM doesn't want this.
>  Platform Tools does not want it.  I doubt the desktop team does GNOME
>  performance analysis on s390x on Fedora.  I'm not even sure if the tools
>  support backchain-based unwinding; it's not a frame pointer after all.
>  Maybe -mbackchain won't cause any issues in after all, but we just don't
>  have the time to test this before the mass rebuild. 
I had a look at the kernel unwinding code for s390 and it seems to use
the backchain if it's available and fall back to the frame pointer otherwise.
So from our end (change proposal authors) we're OK with dropping
mbackchain for s390 and only using fno-omit-frame-pointer for s390.
We'll open a PR to change this in the rpm macros.

>  As Jakub and I have repeatedly explained, -fno-omit-frame-pointer on
>  i686 is known to break certain packages (although I worked around this
>  in glibc last year), simply because the reduced number of registers
>  makes it impossible for GCC to compile certain functions with inline
>  assembly in them.  As with s390x, the concrete impact is not known at
>  this point, and we are out of time for test builds. 
Given these issues should manifest as compilation failures, we should notice
very clearly once the mass rebuild starts if there's a bigger problem. If there's
only a few packages that run into issues, they can opt-out. If there's larger
problems,
we can remove frame pointers from i686.

>  Using -mno-omit-leaf-frame-pointer for aarch64 seems to be another
>  last-minute addition without any clear justification.  (On AArch64, the
>  link register allows one to recover the address of the immediate caller
>  even if a leaf function does not have a frame pointer.  That's not
>  possible on x86-64, where the caller's address must be read from the
>  stack, and that has to be based on the frame pointer.)  Just because the
>  compiler option is there to enable doesn't mean it does anything useful
>  in this context. 
As I mentioned in the fesco ticket, the kernel unwinder looks at the
frame pointer register (x29) first when starting an unwind on aarch64 before looking
at the link register. As such, it seems logical to require frame pointers to be available
in leaf functions so the frame pointer register is available to start unwinding. I'm
happy
to be proven wrong here so we can remove mno-omit-leaf-frame-pointer for aarch64.

Cheers,

Daan De Meyer

________________________________________
From: Florian Weimer <fweimer(a)redhat.com&gt;
Sent: 09 January 2023 17:54
To: Matthew Miller
Cc: devel(a)lists.fedoraproject.org
Subject: Re: FESCo revote on "Add -fno-omit-frame-pointer" Change proposal [was
Re: Schedule for Tuesday's FESCo Meeting (2023-01-03)]

!-------------------------------------------------------------------|
  This Message Is From an External Sender

|-------------------------------------------------------------------!

* Matthew Miller:

>  BUT, I do not think it was done with malice, as "deliberately
> rigged"
>  implies. I don't see that at all -- I see excitement and interest in moving
>  forward on something that already has taken a long time, and looming
>  practical deadlines. 
No one spoke out when the tools team was called untrustworthy on the
FESCo ticket.  We can try explain it as an accident that the toolchain
team was sidelined afterwards.  But the overall sequence of events
certainly looks rather odd.

There are infrastructure problems as well.  Notifications in the Fedora
wiki system are broken (email notifications are spotty, but not
completely defunct; that did not matter here because the new FESCo
ticket was added to the change page only after the second vote), and
missing notifications for label updates on FESCo tickets (so it's hard
to spot that an issue is scheduled for discussion).  So unless you are
in the in-group or invited, it's hard to contribute.

All these issues contribute to a work environment that I find very
problematic.  The individual aspects are probably not deliberate, but
there's still an impact.

>  And, from a practical point of view, since this passed with six +1
> votes,
>  I'm not sure what benefit canceling and re-voting would really add. 
I'm pretty sure no one considered the impact on non-x86-64
architectures, given that the change as voted and submitted as a PR
would have broken the ppc64le and s390x buildroots.

I think it would save everyone a bit of time if we restricted the change
to x86-64.  We do not have much experience with the -mbackchain flag
that was added at the last minute on s390x.  The change owners have
stated that they aren't interested in s390x.  IBM doesn't want this.
Platform Tools does not want it.  I doubt the desktop team does GNOME
performance analysis on s390x on Fedora.  I'm not even sure if the tools
support backchain-based unwinding; it's not a frame pointer after all.
Maybe -mbackchain won't cause any issues in after all, but we just don't
have the time to test this before the mass rebuild.

As Jakub and I have repeatedly explained, -fno-omit-frame-pointer on
i686 is known to break certain packages (although I worked around this
in glibc last year), simply because the reduced number of registers
makes it impossible for GCC to compile certain functions with inline
assembly in them.  As with s390x, the concrete impact is not known at
this point, and we are out of time for test builds.

Using -mno-omit-leaf-frame-pointer for aarch64 seems to be another
last-minute addition without any clear justification.  (On AArch64, the
link register allows one to recover the address of the immediate caller
even if a leaf function does not have a frame pointer.  That's not
possible on x86-64, where the caller's address must be read from the
stack, and that has to be based on the frame pointer.)  Just because the
compiler option is there to enable doesn't mean it does anything useful
in this context.

Thanks,
Florian
_______________________________________________
devel mailing list -- devel(a)lists.fedoraproject.org
To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/[e-mail]
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue