Message ID | 20190613151145.22243-2-raoni@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | Fix, refactor and cleanup of __ppc_get_timebase_freq related code | expand |
On 13/06/2019 12:11, Raoni Fassina Firmino wrote: > From: Stan Shebs <stanshebs@google.com> > > __ppc_get_timebase_freq() always return 0 when using static linked > glibc. > > This is a minimal example.c to reproduce: > > /******************************/ > #include <inttypes.h> > #include <stdint.h> > #include <stdio.h> > #include <sys/platform/ppc.h> > > int main() { > uint64_t freq = __ppc_get_timebase_freq(); > printf("Time Base frequency = %"PRIu64" Hz\n", freq); > if (freq == 0) > return -1; > return 0; > } > /******************************/ > > Compile command: gcc -static example.c > > This bug has been reproduced, fixed and tested on all powerpc platforms > (ppc32, ppc64 and ppc64le). > > The underlying code of __ppc_get_timebase_freq uses __get_timebase_freq > that has a different implementation for shared and static version of > glibc. In the static version, there is an incorrect sense in the if > check for the fd returned when opening /proc/cpuinfo. > > This solution is mostly a cherry-pick from: > > commit 4791e4f773d060c1a37b27aac5b03cdfa9327afc > Author: Stan Shebs <stanshebs@google.com> > Date: Fri May 17 12:25:19 2019 -0700 > Subject: Fix sense of a test in the static-linking version of ppc get_clockfreq > > That is in branch glibc/google/grte/v5-2.27/master and was mentioned for > inclusion on master here: > > https://www.sourceware.org/ml/libc-alpha/2019-05/msg00409.html > > Adapted from original fix for get_clockfreq. That code was moved to > get_timebase_freq. > > ChangeLog: > > 2019-06-13 Stan Shebs <stanshebs@google.com> > Raoni Fassina Firmino <raoni@linux.ibm.com> > > [BZ #24640] > * sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c > [!SHARED] (__get_timebase_freq): Fix sense of a test in the > static-linking version. Please include the testcase [1] on this patch. LGTM with this change. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> [1] https://sourceware.org/ml/libc-alpha/2019-06/msg00285.html > --- > sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c b/sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c > index 23e7694d87..c245e97526 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c > +++ b/sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c > @@ -39,7 +39,7 @@ __get_timebase_freq (void) > timebase : 33333333 > We search for this line and convert the number into an integer. */ > int fd = __open_nocancel ("/proc/cpuinfo", O_RDONLY); > - if (__glibc_likely (fd != -1)) > + if (__glibc_unlikely (fd == -1)) > return result; > > /* The timebase will be in the 1st 1024 bytes for systems with up >
diff --git a/sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c b/sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c index 23e7694d87..c245e97526 100644 --- a/sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c +++ b/sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c @@ -39,7 +39,7 @@ __get_timebase_freq (void) timebase : 33333333 We search for this line and convert the number into an integer. */ int fd = __open_nocancel ("/proc/cpuinfo", O_RDONLY); - if (__glibc_likely (fd != -1)) + if (__glibc_unlikely (fd == -1)) return result; /* The timebase will be in the 1st 1024 bytes for systems with up
From: Stan Shebs <stanshebs@google.com> __ppc_get_timebase_freq() always return 0 when using static linked glibc. This is a minimal example.c to reproduce: /******************************/ #include <inttypes.h> #include <stdint.h> #include <stdio.h> #include <sys/platform/ppc.h> int main() { uint64_t freq = __ppc_get_timebase_freq(); printf("Time Base frequency = %"PRIu64" Hz\n", freq); if (freq == 0) return -1; return 0; } /******************************/ Compile command: gcc -static example.c This bug has been reproduced, fixed and tested on all powerpc platforms (ppc32, ppc64 and ppc64le). The underlying code of __ppc_get_timebase_freq uses __get_timebase_freq that has a different implementation for shared and static version of glibc. In the static version, there is an incorrect sense in the if check for the fd returned when opening /proc/cpuinfo. This solution is mostly a cherry-pick from: commit 4791e4f773d060c1a37b27aac5b03cdfa9327afc Author: Stan Shebs <stanshebs@google.com> Date: Fri May 17 12:25:19 2019 -0700 Subject: Fix sense of a test in the static-linking version of ppc get_clockfreq That is in branch glibc/google/grte/v5-2.27/master and was mentioned for inclusion on master here: https://www.sourceware.org/ml/libc-alpha/2019-05/msg00409.html Adapted from original fix for get_clockfreq. That code was moved to get_timebase_freq. ChangeLog: 2019-06-13 Stan Shebs <stanshebs@google.com> Raoni Fassina Firmino <raoni@linux.ibm.com> [BZ #24640] * sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c [!SHARED] (__get_timebase_freq): Fix sense of a test in the static-linking version. --- sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)