| Message ID | 20220830120743.1072319-1-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | nptl: x86_64: Use same code for CURRENT_STACK_FRAME and stackinfo_get_sp | expand |
On Tue, Aug 30, 2022 at 5:08 AM Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote: > > It avoids the possible warning of uninitialized 'frame' variable when > building with clang: > > ../sysdeps/nptl/jmp-unwind.c:27:42: error: variable 'frame' is > uninitialized when used here [-Werror,-Wuninitialized] > __pthread_cleanup_upto (env->__jmpbuf, CURRENT_STACK_FRAME); > > The resulting code is similar to CURRENT_STACK_FRAME. > > Checked on x86_64-linux-gnu. > --- > sysdeps/x86/nptl/pthreaddef.h | 4 +++- > sysdeps/x86_64/stackinfo.h | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/sysdeps/x86/nptl/pthreaddef.h b/sysdeps/x86/nptl/pthreaddef.h > index 63fdbcb27c..7df65931a3 100644 > --- a/sysdeps/x86/nptl/pthreaddef.h > +++ b/sysdeps/x86/nptl/pthreaddef.h > @@ -42,7 +42,9 @@ > #ifdef __x86_64__ > /* The frame pointer is not usable. */ > # define CURRENT_STACK_FRAME \ > - ({ register char *frame __asm__("rsp"); frame; }) > + ({ register void * p__ __asm__(RSP_REG); \ > + asm volatile("" : "=r" (p__)); \ > + p__; }) > #else > # define CURRENT_STACK_FRAME __builtin_frame_address (0) > #endif > diff --git a/sysdeps/x86_64/stackinfo.h b/sysdeps/x86_64/stackinfo.h > index 34c9d0b576..7354632132 100644 > --- a/sysdeps/x86_64/stackinfo.h > +++ b/sysdeps/x86_64/stackinfo.h > @@ -40,7 +40,9 @@ > for which they need to act as barriers as well, hence the additional > (unnecessary) parameters. */ > #define stackinfo_get_sp() \ > - ({ void *p__; asm volatile ("mov %%" RSP_REG ", %0" : "=r" (p__)); p__; }) > + ({ register void * p__ __asm__(RSP_REG); \ > + asm volatile("" : "=r" (p__)); \ > + p__; }) This change addresses a different issue. Maybe two separate commits? Otherwise LGTM. > #define stackinfo_sub_sp(ptr) \ > ({ ptrdiff_t d__; \ > asm volatile ("sub %%" RSP_REG " , %0" : "=r" (d__) : "0" (ptr)); \ > -- > 2.34.1 >
On 30/08/22 15:54, Noah Goldstein wrote: > On Tue, Aug 30, 2022 at 5:08 AM Adhemerval Zanella via Libc-alpha > <libc-alpha@sourceware.org> wrote: >> >> It avoids the possible warning of uninitialized 'frame' variable when >> building with clang: >> >> ../sysdeps/nptl/jmp-unwind.c:27:42: error: variable 'frame' is >> uninitialized when used here [-Werror,-Wuninitialized] >> __pthread_cleanup_upto (env->__jmpbuf, CURRENT_STACK_FRAME); >> >> The resulting code is similar to CURRENT_STACK_FRAME. >> >> Checked on x86_64-linux-gnu. >> --- >> sysdeps/x86/nptl/pthreaddef.h | 4 +++- >> sysdeps/x86_64/stackinfo.h | 4 +++- >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/sysdeps/x86/nptl/pthreaddef.h b/sysdeps/x86/nptl/pthreaddef.h >> index 63fdbcb27c..7df65931a3 100644 >> --- a/sysdeps/x86/nptl/pthreaddef.h >> +++ b/sysdeps/x86/nptl/pthreaddef.h >> @@ -42,7 +42,9 @@ >> #ifdef __x86_64__ >> /* The frame pointer is not usable. */ >> # define CURRENT_STACK_FRAME \ >> - ({ register char *frame __asm__("rsp"); frame; }) >> + ({ register void * p__ __asm__(RSP_REG); \ >> + asm volatile("" : "=r" (p__)); \ >> + p__; }) >> #else >> # define CURRENT_STACK_FRAME __builtin_frame_address (0) >> #endif >> diff --git a/sysdeps/x86_64/stackinfo.h b/sysdeps/x86_64/stackinfo.h >> index 34c9d0b576..7354632132 100644 >> --- a/sysdeps/x86_64/stackinfo.h >> +++ b/sysdeps/x86_64/stackinfo.h >> @@ -40,7 +40,9 @@ >> for which they need to act as barriers as well, hence the additional >> (unnecessary) parameters. */ >> #define stackinfo_get_sp() \ >> - ({ void *p__; asm volatile ("mov %%" RSP_REG ", %0" : "=r" (p__)); p__; }) >> + ({ register void * p__ __asm__(RSP_REG); \ >> + asm volatile("" : "=r" (p__)); \ >> + p__; }) > > This change addresses a different issue. Maybe two separate commits? I am not sure this characterizes as an 'issue', since code does work as is, the idea is to have the same definition. > > Otherwise LGTM. >> #define stackinfo_sub_sp(ptr) \ >> ({ ptrdiff_t d__; \ >> asm volatile ("sub %%" RSP_REG " , %0" : "=r" (d__) : "0" (ptr)); \ >> -- >> 2.34.1 >>
On Tue, Aug 30, 2022 at 12:08 PM Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote: > > > > On 30/08/22 15:54, Noah Goldstein wrote: > > On Tue, Aug 30, 2022 at 5:08 AM Adhemerval Zanella via Libc-alpha > > <libc-alpha@sourceware.org> wrote: > >> > >> It avoids the possible warning of uninitialized 'frame' variable when > >> building with clang: > >> > >> ../sysdeps/nptl/jmp-unwind.c:27:42: error: variable 'frame' is > >> uninitialized when used here [-Werror,-Wuninitialized] > >> __pthread_cleanup_upto (env->__jmpbuf, CURRENT_STACK_FRAME); > >> > >> The resulting code is similar to CURRENT_STACK_FRAME. > >> > >> Checked on x86_64-linux-gnu. > >> --- > >> sysdeps/x86/nptl/pthreaddef.h | 4 +++- > >> sysdeps/x86_64/stackinfo.h | 4 +++- > >> 2 files changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/sysdeps/x86/nptl/pthreaddef.h b/sysdeps/x86/nptl/pthreaddef.h > >> index 63fdbcb27c..7df65931a3 100644 > >> --- a/sysdeps/x86/nptl/pthreaddef.h > >> +++ b/sysdeps/x86/nptl/pthreaddef.h > >> @@ -42,7 +42,9 @@ > >> #ifdef __x86_64__ > >> /* The frame pointer is not usable. */ > >> # define CURRENT_STACK_FRAME \ > >> - ({ register char *frame __asm__("rsp"); frame; }) > >> + ({ register void * p__ __asm__(RSP_REG); \ > >> + asm volatile("" : "=r" (p__)); \ > >> + p__; }) > >> #else > >> # define CURRENT_STACK_FRAME __builtin_frame_address (0) > >> #endif > >> diff --git a/sysdeps/x86_64/stackinfo.h b/sysdeps/x86_64/stackinfo.h > >> index 34c9d0b576..7354632132 100644 > >> --- a/sysdeps/x86_64/stackinfo.h > >> +++ b/sysdeps/x86_64/stackinfo.h > >> @@ -40,7 +40,9 @@ > >> for which they need to act as barriers as well, hence the additional > >> (unnecessary) parameters. */ > >> #define stackinfo_get_sp() \ > >> - ({ void *p__; asm volatile ("mov %%" RSP_REG ", %0" : "=r" (p__)); p__; }) > >> + ({ register void * p__ __asm__(RSP_REG); \ > >> + asm volatile("" : "=r" (p__)); \ > >> + p__; }) > > > > This change addresses a different issue. Maybe two separate commits? > > I am not sure this characterizes as an 'issue', since code does work as is, > the idea is to have the same definition. Fair enough. > > > > > Otherwise LGTM. > >> #define stackinfo_sub_sp(ptr) \ > >> ({ ptrdiff_t d__; \ > >> asm volatile ("sub %%" RSP_REG " , %0" : "=r" (d__) : "0" (ptr)); \ > >> -- > >> 2.34.1 > >> LGTM.
diff --git a/sysdeps/x86/nptl/pthreaddef.h b/sysdeps/x86/nptl/pthreaddef.h index 63fdbcb27c..7df65931a3 100644 --- a/sysdeps/x86/nptl/pthreaddef.h +++ b/sysdeps/x86/nptl/pthreaddef.h @@ -42,7 +42,9 @@ #ifdef __x86_64__ /* The frame pointer is not usable. */ # define CURRENT_STACK_FRAME \ - ({ register char *frame __asm__("rsp"); frame; }) + ({ register void * p__ __asm__(RSP_REG); \ + asm volatile("" : "=r" (p__)); \ + p__; }) #else # define CURRENT_STACK_FRAME __builtin_frame_address (0) #endif diff --git a/sysdeps/x86_64/stackinfo.h b/sysdeps/x86_64/stackinfo.h index 34c9d0b576..7354632132 100644 --- a/sysdeps/x86_64/stackinfo.h +++ b/sysdeps/x86_64/stackinfo.h @@ -40,7 +40,9 @@ for which they need to act as barriers as well, hence the additional (unnecessary) parameters. */ #define stackinfo_get_sp() \ - ({ void *p__; asm volatile ("mov %%" RSP_REG ", %0" : "=r" (p__)); p__; }) + ({ register void * p__ __asm__(RSP_REG); \ + asm volatile("" : "=r" (p__)); \ + p__; }) #define stackinfo_sub_sp(ptr) \ ({ ptrdiff_t d__; \ asm volatile ("sub %%" RSP_REG " , %0" : "=r" (d__) : "0" (ptr)); \
It avoids the possible warning of uninitialized 'frame' variable when building with clang: ../sysdeps/nptl/jmp-unwind.c:27:42: error: variable 'frame' is uninitialized when used here [-Werror,-Wuninitialized] __pthread_cleanup_upto (env->__jmpbuf, CURRENT_STACK_FRAME); The resulting code is similar to CURRENT_STACK_FRAME. Checked on x86_64-linux-gnu. --- sysdeps/x86/nptl/pthreaddef.h | 4 +++- sysdeps/x86_64/stackinfo.h | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)