| Message ID | 3d422103bcd42547829b53fa07d3891e5bfabba3.1589552055.git.szabolcs.nagy@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: branch protection support | expand |
On 15/05/2020 11:40, Szabolcs Nagy wrote: > gcc -pg with -mbranch-protection=pac-ret passes signed return address > to _mcount, so _mcount now has to always strip pac from the frompc > since that's from user code that may be built with pac-ret. > > This is a backward incompatible _mcount abi change introduced by > return address signing support in gcc-7. Which change are you referring about specifically? Besides it, LGTM. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > TODO: fix -pg on the gcc side? > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94791 > --- > sysdeps/aarch64/machine-gmon.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h > index 730a23b781..328cbdda16 100644 > --- a/sysdeps/aarch64/machine-gmon.h > +++ b/sysdeps/aarch64/machine-gmon.h > @@ -30,5 +30,5 @@ static inline void mcount_internal (u_long frompc, u_long selfpc) > #define MCOUNT \ > void __mcount (void *frompc) \ > { \ > - mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \ > + mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \ > } >
The 05/26/2020 08:33, Adhemerval Zanella via Libc-alpha wrote: > > > On 15/05/2020 11:40, Szabolcs Nagy wrote: > > gcc -pg with -mbranch-protection=pac-ret passes signed return address > > to _mcount, so _mcount now has to always strip pac from the frompc > > since that's from user code that may be built with pac-ret. > > > > This is a backward incompatible _mcount abi change introduced by > > return address signing support in gcc-7. > > Which change are you referring about specifically? gcc-7 introduced -msigned-return-address (which was later deprecated by -mbranch-protection=pac-ret) and the code generation with -pg is not compatible with existing _mcount runtime implementations (because now _mcount needs unconditional xpac) i'm still trying to see if this _mcount change is needed or we can change gcc to do the right thing (and not pass signed address to _mcount), but i need internal consensus about such abi break first. > > Besides it, LGTM. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > > > TODO: fix -pg on the gcc side? > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94791 > > --- > > sysdeps/aarch64/machine-gmon.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h > > index 730a23b781..328cbdda16 100644 > > --- a/sysdeps/aarch64/machine-gmon.h > > +++ b/sysdeps/aarch64/machine-gmon.h > > @@ -30,5 +30,5 @@ static inline void mcount_internal (u_long frompc, u_long selfpc) > > #define MCOUNT \ > > void __mcount (void *frompc) \ > > { \ > > - mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \ > > + mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \ > > } > > --
On 26/05/2020 15:38, Szabolcs Nagy wrote: > The 05/26/2020 08:33, Adhemerval Zanella via Libc-alpha wrote: >> >> >> On 15/05/2020 11:40, Szabolcs Nagy wrote: >>> gcc -pg with -mbranch-protection=pac-ret passes signed return address >>> to _mcount, so _mcount now has to always strip pac from the frompc >>> since that's from user code that may be built with pac-ret. >>> >>> This is a backward incompatible _mcount abi change introduced by >>> return address signing support in gcc-7. >> >> Which change are you referring about specifically? > > gcc-7 introduced -msigned-return-address (which was > later deprecated by -mbranch-protection=pac-ret) and > the code generation with -pg is not compatible with > existing _mcount runtime implementations (because > now _mcount needs unconditional xpac) Thanks, could you add this explanation on commit message as well? > > i'm still trying to see if this _mcount change is > needed or we can change gcc to do the right thing > (and not pass signed address to _mcount), but i > need internal consensus about such abi break first. > >> >> Besides it, LGTM. >> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> >>> >>> TODO: fix -pg on the gcc side? >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94791 >>> --- >>> sysdeps/aarch64/machine-gmon.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h >>> index 730a23b781..328cbdda16 100644 >>> --- a/sysdeps/aarch64/machine-gmon.h >>> +++ b/sysdeps/aarch64/machine-gmon.h >>> @@ -30,5 +30,5 @@ static inline void mcount_internal (u_long frompc, u_long selfpc) >>> #define MCOUNT \ >>> void __mcount (void *frompc) \ >>> { \ >>> - mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \ >>> + mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \ >>> } >>> >
diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h index 730a23b781..328cbdda16 100644 --- a/sysdeps/aarch64/machine-gmon.h +++ b/sysdeps/aarch64/machine-gmon.h @@ -30,5 +30,5 @@ static inline void mcount_internal (u_long frompc, u_long selfpc) #define MCOUNT \ void __mcount (void *frompc) \ { \ - mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \ + mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \ }