| Message ID | 20170718135454.17754-1-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On 2017-07-18 06:54 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > On some flavours of GCC the src to fwts_safe_memread throws up > a longjmp clobber warning: > > fwts_safe_mem.c: In function ‘fwts_safe_memread’: > fwts_safe_mem.c:69:35: error: argument ‘src’ might be clobbered by ‘longjmp’ > or ‘vfork’ [-Werror=clobbered] > int fwts_safe_memread(const void *src, const size_t n) > ^ > cc1: all warnings being treated as errors > > We either disable -Wclobber (which I don't want to do since it is useful > to catch these potential errors) or work out a case by case workaround. > The easiest fix for these is to crank down the optimisation level to zero > so that the compiler stashes the src argument onto the stack and we don't > end up with clobbering. > > So.. add a bunch of macros to use a gcc attribute if it is supported > and force the function to be optimized at level 0. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/include/fwts.h | 24 ++++++++++++++++++++++++ > src/lib/src/fwts_safe_mem.c | 4 ++-- > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/src/lib/include/fwts.h b/src/lib/include/fwts.h > index a56ce24f..325e4ee4 100644 > --- a/src/lib/include/fwts.h > +++ b/src/lib/include/fwts.h > @@ -44,6 +44,30 @@ > #undef FWTS_HAS_UEFI > #endif > > +/* verision 3-tuple into integer */ > +#define _VER_(major, minor, patchlevel) \ > + ((major * 10000) + (minor * 100) + patchlevel) > + > +/* check version of GNU GCC */ > +#if defined(__GNUC__) && defined(__GNUC_MINOR__) > +#if defined(__GNUC_PATCHLEVEL__) > +#define NEED_GNUC(major, minor, patchlevel) \ > + _VER_(major, minor, patchlevel) <= _VER_(__GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__) > +#else > +#define NEED_GNUC(major, minor, patchlevel) \ > + _VER_(major, minor, patchlevel) <= _VER_(__GNUC__, __GNUC_MINOR__, 0) > +#endif > +#else > +#define NEED_GNUC(major, minor, patchlevel) (0) > +#endif > + > +/* -O0 attribute support */ > +#if defined(__GNUC__) && !defined(__clang__) && NEED_GNUC(4,6,0) > +#define OPTIMIZE0 __attribute__((optimize("-O0"))) > +#else > +#define OPTIMIZE0 > +#endif > + > #define FWTS_UNUSED(var) (void)var > > #define FWTS_JSON_DATA_PATH DATAROOTDIR "/fwts" > diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c > index 216ad8e2..1feb1ff2 100644 > --- a/src/lib/src/fwts_safe_mem.c > +++ b/src/lib/src/fwts_safe_mem.c > @@ -46,7 +46,7 @@ static void sig_handler(int dummy) > * may segfault or throw a bus error if the src > * address is corrupt > */ > -int fwts_safe_memcpy(void *dst, const void *src, const size_t n) > +int OPTIMIZE0 fwts_safe_memcpy(void *dst, const void *src, const size_t n) > { > if (sigsetjmp(jmpbuf, 1) != 0) > return FWTS_ERROR; > @@ -66,7 +66,7 @@ int fwts_safe_memcpy(void *dst, const void *src, const size_t n) > * SIGSEGV/SIGBUS errors and returns FWTS_ERROR if it is not > * readable or FWTS_OK if it's OK. > */ > -int fwts_safe_memread(const void *src, const size_t n) > +int OPTIMIZE0 fwts_safe_memread(const void *src, const size_t n) > { > const volatile uint8_t *ptr = src; > const volatile uint8_t *end = ptr + n; > Acked-by: Alex Hung <alex.hung@canonical.com>
On 07/18/2017 09:54 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > On some flavours of GCC the src to fwts_safe_memread throws up > a longjmp clobber warning: > > fwts_safe_mem.c: In function ‘fwts_safe_memread’: > fwts_safe_mem.c:69:35: error: argument ‘src’ might be clobbered by ‘longjmp’ > or ‘vfork’ [-Werror=clobbered] > int fwts_safe_memread(const void *src, const size_t n) > ^ > cc1: all warnings being treated as errors > > We either disable -Wclobber (which I don't want to do since it is useful > to catch these potential errors) or work out a case by case workaround. > The easiest fix for these is to crank down the optimisation level to zero > so that the compiler stashes the src argument onto the stack and we don't > end up with clobbering. > > So.. add a bunch of macros to use a gcc attribute if it is supported > and force the function to be optimized at level 0. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/include/fwts.h | 24 ++++++++++++++++++++++++ > src/lib/src/fwts_safe_mem.c | 4 ++-- > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/src/lib/include/fwts.h b/src/lib/include/fwts.h > index a56ce24f..325e4ee4 100644 > --- a/src/lib/include/fwts.h > +++ b/src/lib/include/fwts.h > @@ -44,6 +44,30 @@ > #undef FWTS_HAS_UEFI > #endif > > +/* verision 3-tuple into integer */ > +#define _VER_(major, minor, patchlevel) \ > + ((major * 10000) + (minor * 100) + patchlevel) > + > +/* check version of GNU GCC */ > +#if defined(__GNUC__) && defined(__GNUC_MINOR__) > +#if defined(__GNUC_PATCHLEVEL__) > +#define NEED_GNUC(major, minor, patchlevel) \ > + _VER_(major, minor, patchlevel) <= _VER_(__GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__) > +#else > +#define NEED_GNUC(major, minor, patchlevel) \ > + _VER_(major, minor, patchlevel) <= _VER_(__GNUC__, __GNUC_MINOR__, 0) > +#endif > +#else > +#define NEED_GNUC(major, minor, patchlevel) (0) > +#endif > + > +/* -O0 attribute support */ > +#if defined(__GNUC__) && !defined(__clang__) && NEED_GNUC(4,6,0) > +#define OPTIMIZE0 __attribute__((optimize("-O0"))) > +#else > +#define OPTIMIZE0 > +#endif > + > #define FWTS_UNUSED(var) (void)var > > #define FWTS_JSON_DATA_PATH DATAROOTDIR "/fwts" > diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c > index 216ad8e2..1feb1ff2 100644 > --- a/src/lib/src/fwts_safe_mem.c > +++ b/src/lib/src/fwts_safe_mem.c > @@ -46,7 +46,7 @@ static void sig_handler(int dummy) > * may segfault or throw a bus error if the src > * address is corrupt > */ > -int fwts_safe_memcpy(void *dst, const void *src, const size_t n) > +int OPTIMIZE0 fwts_safe_memcpy(void *dst, const void *src, const size_t n) > { > if (sigsetjmp(jmpbuf, 1) != 0) > return FWTS_ERROR; > @@ -66,7 +66,7 @@ int fwts_safe_memcpy(void *dst, const void *src, const size_t n) > * SIGSEGV/SIGBUS errors and returns FWTS_ERROR if it is not > * readable or FWTS_OK if it's OK. > */ > -int fwts_safe_memread(const void *src, const size_t n) > +int OPTIMIZE0 fwts_safe_memread(const void *src, const size_t n) > { > const volatile uint8_t *ptr = src; > const volatile uint8_t *end = ptr + n; > Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/lib/include/fwts.h b/src/lib/include/fwts.h index a56ce24f..325e4ee4 100644 --- a/src/lib/include/fwts.h +++ b/src/lib/include/fwts.h @@ -44,6 +44,30 @@ #undef FWTS_HAS_UEFI #endif +/* verision 3-tuple into integer */ +#define _VER_(major, minor, patchlevel) \ + ((major * 10000) + (minor * 100) + patchlevel) + +/* check version of GNU GCC */ +#if defined(__GNUC__) && defined(__GNUC_MINOR__) +#if defined(__GNUC_PATCHLEVEL__) +#define NEED_GNUC(major, minor, patchlevel) \ + _VER_(major, minor, patchlevel) <= _VER_(__GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__) +#else +#define NEED_GNUC(major, minor, patchlevel) \ + _VER_(major, minor, patchlevel) <= _VER_(__GNUC__, __GNUC_MINOR__, 0) +#endif +#else +#define NEED_GNUC(major, minor, patchlevel) (0) +#endif + +/* -O0 attribute support */ +#if defined(__GNUC__) && !defined(__clang__) && NEED_GNUC(4,6,0) +#define OPTIMIZE0 __attribute__((optimize("-O0"))) +#else +#define OPTIMIZE0 +#endif + #define FWTS_UNUSED(var) (void)var #define FWTS_JSON_DATA_PATH DATAROOTDIR "/fwts" diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c index 216ad8e2..1feb1ff2 100644 --- a/src/lib/src/fwts_safe_mem.c +++ b/src/lib/src/fwts_safe_mem.c @@ -46,7 +46,7 @@ static void sig_handler(int dummy) * may segfault or throw a bus error if the src * address is corrupt */ -int fwts_safe_memcpy(void *dst, const void *src, const size_t n) +int OPTIMIZE0 fwts_safe_memcpy(void *dst, const void *src, const size_t n) { if (sigsetjmp(jmpbuf, 1) != 0) return FWTS_ERROR; @@ -66,7 +66,7 @@ int fwts_safe_memcpy(void *dst, const void *src, const size_t n) * SIGSEGV/SIGBUS errors and returns FWTS_ERROR if it is not * readable or FWTS_OK if it's OK. */ -int fwts_safe_memread(const void *src, const size_t n) +int OPTIMIZE0 fwts_safe_memread(const void *src, const size_t n) { const volatile uint8_t *ptr = src; const volatile uint8_t *end = ptr + n;