ACK: [PATCH] fwts_safe_mem: workaround longjmp clobber warnings

[ivanhu]

On 07/18/2017 09:54 PM, Colin King wrote:
> From: Colin Ian King <colin.king at 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 at 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 at canonical.com>