Message ID | 5A674841.9040007@arm.com |
---|---|
State | New |
Headers | show |
Series | aarch64: Revert the change of the __reserved member of mcontext_t | expand |
On 01/23/2018 03:35 PM, Szabolcs Nagy wrote: > The uc_mcontext.__reserved member of ucontext_t is a user visible API, > that should not be changed, because this is the only way to access cpu > states of various extensions of linux asm/sigcontext.h, it does not > violate namespace rules either, so revert this part of the commit > > commit 4fa9b3bfe6759c82beb4b043a54a3598ca467289 > Commit: Joseph Myers<joseph@codesourcery.com> > > Fix mcontext_t sigcontext namespace (bug 21457). > > (In principle the user can type cast &uc_mcontext to struct sigcontext* > to use the linux sigcontext fields, but that's not the existing practice > since mcontext_t used to be a typedef of struct sigcontext.) > > I plan to commit this soon for 2.27 and backport it to 2.26 if there are > no comments. I think this is the right thing to do, but unfortunately, this back-and-forth is a bit painful. Thanks, Florian
On 23/01/18 15:16, Florian Weimer wrote: > On 01/23/2018 03:35 PM, Szabolcs Nagy wrote: >> The uc_mcontext.__reserved member of ucontext_t is a user visible API, >> that should not be changed, because this is the only way to access cpu >> states of various extensions of linux asm/sigcontext.h, it does not >> violate namespace rules either, so revert this part of the commit >> >> commit 4fa9b3bfe6759c82beb4b043a54a3598ca467289 >> Commit: Joseph Myers<joseph@codesourcery.com> >> >> Fix mcontext_t sigcontext namespace (bug 21457). >> >> (In principle the user can type cast &uc_mcontext to struct sigcontext* >> to use the linux sigcontext fields, but that's not the existing practice >> since mcontext_t used to be a typedef of struct sigcontext.) >> >> I plan to commit this soon for 2.27 and backport it to 2.26 if there are >> no comments. > > I think this is the right thing to do, but unfortunately, this back-and-forth is a bit painful. > ok, i opened bug 22742 because it's a user visible change. committed now on 2.27 (will backport it in a day or two).
Hi Nagy, Le 24/01/2018 à 12:55, Szabolcs Nagy a écrit : > On 23/01/18 15:16, Florian Weimer wrote: >> On 01/23/2018 03:35 PM, Szabolcs Nagy wrote: >>> The uc_mcontext.__reserved member of ucontext_t is a user visible API, >>> that should not be changed, because this is the only way to access cpu >>> states of various extensions of linux asm/sigcontext.h, it does not >>> violate namespace rules either, so revert this part of the commit >>> >>> commit 4fa9b3bfe6759c82beb4b043a54a3598ca467289 >>> Commit: Joseph Myers<joseph@codesourcery.com> >>> >>> Fix mcontext_t sigcontext namespace (bug 21457). >>> >>> (In principle the user can type cast &uc_mcontext to struct sigcontext* >>> to use the linux sigcontext fields, but that's not the existing practice >>> since mcontext_t used to be a typedef of struct sigcontext.) >>> >>> I plan to commit this soon for 2.27 and backport it to 2.26 if there are >>> no comments. >> >> I think this is the right thing to do, but unfortunately, this back-and-forth is a bit painful. >> > > ok, i opened bug 22742 because it's a user visible change. > > committed now on 2.27 (will backport it in a day or two). > Thanks for the fix. Best regards, Romain
On 24/01/18 11:55, Szabolcs Nagy wrote: > On 23/01/18 15:16, Florian Weimer wrote: >> On 01/23/2018 03:35 PM, Szabolcs Nagy wrote: >>> The uc_mcontext.__reserved member of ucontext_t is a user visible API, >>> that should not be changed, because this is the only way to access cpu >>> states of various extensions of linux asm/sigcontext.h, it does not >>> violate namespace rules either, so revert this part of the commit >>> >>> commit 4fa9b3bfe6759c82beb4b043a54a3598ca467289 >>> Commit: Joseph Myers<joseph@codesourcery.com> >>> >>> Fix mcontext_t sigcontext namespace (bug 21457). >>> >>> (In principle the user can type cast &uc_mcontext to struct sigcontext* >>> to use the linux sigcontext fields, but that's not the existing practice >>> since mcontext_t used to be a typedef of struct sigcontext.) >>> >>> I plan to commit this soon for 2.27 and backport it to 2.26 if there are >>> no comments. >> >> I think this is the right thing to do, but unfortunately, this back-and-forth is a bit painful. >> > > ok, i opened bug 22742 because it's a user visible change. > > committed now on 2.27 (will backport it in a day or two). > the problematic commit is not present in 2.26, so this is not a 2.26 regression, i'm not sure how i missed that (i checked the git log of 2.26 branch several times). so there is no user visible api break, sorry for the noise.
On 01/26/2018 04:02 PM, Szabolcs Nagy wrote: > On 24/01/18 11:55, Szabolcs Nagy wrote: >> On 23/01/18 15:16, Florian Weimer wrote: >>> On 01/23/2018 03:35 PM, Szabolcs Nagy wrote: >>>> The uc_mcontext.__reserved member of ucontext_t is a user visible API, >>>> that should not be changed, because this is the only way to access cpu >>>> states of various extensions of linux asm/sigcontext.h, it does not >>>> violate namespace rules either, so revert this part of the commit >>>> >>>> commit 4fa9b3bfe6759c82beb4b043a54a3598ca467289 >>>> Commit: Joseph Myers<joseph@codesourcery.com> >>>> >>>> Fix mcontext_t sigcontext namespace (bug 21457). >>>> >>>> (In principle the user can type cast &uc_mcontext to struct sigcontext* >>>> to use the linux sigcontext fields, but that's not the existing >>>> practice >>>> since mcontext_t used to be a typedef of struct sigcontext.) >>>> >>>> I plan to commit this soon for 2.27 and backport it to 2.26 if there >>>> are >>>> no comments. >>> >>> I think this is the right thing to do, but unfortunately, this >>> back-and-forth is a bit painful. >>> >> >> ok, i opened bug 22742 because it's a user visible change. >> >> committed now on 2.27 (will backport it in a day or two). >> > > the problematic commit is not present in 2.26, so > this is not a 2.26 regression, i'm not sure how i > missed that (i checked the git log of 2.26 branch > several times). > > so there is no user visible api break, sorry for the noise. Right, my mistake. Thanks for fixing master! Florian
diff --git a/sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h b/sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h index fe72fdd265..f1b3ab59e2 100644 --- a/sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h +++ b/sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h @@ -56,7 +56,11 @@ typedef struct unsigned long long int __ctx(sp); unsigned long long int __ctx(pc); unsigned long long int __ctx(pstate); - unsigned char __glibc_reserved1[4096] __attribute__ ((__aligned__ (16))); + /* This field contains extension records for additional processor + state such as the FP/SIMD state. It has to match the definition + of the corresponding field in the sigcontext struct, see the + arch/arm64/include/uapi/asm/sigcontext.h linux header for details. */ + unsigned char __reserved[4096] __attribute__ ((__aligned__ (16))); } mcontext_t; /* Userlevel context. */ diff --git a/sysdeps/unix/sysv/linux/aarch64/ucontext_i.sym b/sysdeps/unix/sysv/linux/aarch64/ucontext_i.sym index 479bdda5c6..ab3930c173 100644 --- a/sysdeps/unix/sysv/linux/aarch64/ucontext_i.sym +++ b/sysdeps/unix/sysv/linux/aarch64/ucontext_i.sym @@ -38,7 +38,7 @@ oX0 mcontext (regs) oSP mcontext (sp) oPC mcontext (pc) oPSTATE mcontext (pstate) -oEXTENSION mcontext (__glibc_reserved1) +oEXTENSION mcontext (__reserved) #define fpsimd_context(member) offsetof (struct fpsimd_context, member)