I'm going to set a record by nacking my own patch.  On second 
consideration I think memset may drop the volatile and thus may still be 
optimized away.  Will look for another portable way to avoid the 
optimization here.

-Joel

On 08/21/2013 11:57 AM, Joel Schopp wrote:
> The clang compiler has a nice static analyzer.  Clang is also getting
> performance competitive with gcc and is pretty popular on some platforms.
> For the most part it is compatible with gcc language extensions
>
> There are two places where it isn't.
>
> The first is the use of inline in header files which c99 has defined
> sanely and gcc c89 extensions haven't.  The inline was only a hint anyway,
> with the compiler able to inline without it and not forced to inline with
> it.
>
> The second is that clang has no way to define per function
> optimization levels like gcc does.  Luckily the only optimization we want
> to avoid here is memset() being optmized away, which we can do with
> the standard and portable use of volatile variables.
>
> Signed-off-by: Joel Schopp <[email protected]>
> ---
>   src/include/spi_utils.h    |  8 +-------
>   src/include/tcsps.h        |  9 ++-------
>   src/include/tspps.h        |  4 ++--
>   src/tcs/ps/ps_utils.c      |  8 --------
>   src/tspi/tsp_context_mem.c | 14 +++++++++++---
>   5 files changed, 16 insertions(+), 27 deletions(-)
>
> diff --git a/src/include/spi_utils.h b/src/include/spi_utils.h
> index 11255b2..0b0bc02 100644
> --- a/src/include/spi_utils.h
> +++ b/src/include/spi_utils.h
> @@ -44,16 +44,10 @@ MUTEX_DECLARE_EXTERN(mem_cache_lock);
>   #define TSS_PSFILE_INCREMENT_NUM_KEYS       1
>   #define TSS_PSFILE_DECREMENT_NUM_KEYS       0
>
> -#ifdef __GNUC__
> -#define __no_optimize __attribute__((optimize("O0")))
> -#else
> -#define __no_optimize
> -#endif
> -
>   void *calloc_tspi(TSS_HCONTEXT, UINT32);
>   TSS_RESULT free_tspi(TSS_HCONTEXT, void *);
>   TSS_RESULT __tspi_add_mem_entry(TSS_HCONTEXT, void *);
> -void * __no_optimize __tspi_memset(void *, int, size_t);
> +void * __tspi_memset(void *, int, size_t);
>
>   /* secrets.c */
>
> diff --git a/src/include/tcsps.h b/src/include/tcsps.h
> index 8754296..92a4efe 100644
> --- a/src/include/tcsps.h
> +++ b/src/include/tcsps.h
> @@ -23,13 +23,8 @@ int                   get_file();
>   int            put_file(int);
>   void                   close_file(int);
>   void                   ps_destroy();
> -#ifdef SOLARIS
> -TSS_RESULT  read_data(int, void *, UINT32);
> -TSS_RESULT  write_data(int, void *, UINT32);
> -#else
> -inline TSS_RESULT  read_data(int, void *, UINT32);
> -inline TSS_RESULT  write_data(int, void *, UINT32);
> -#endif
> +TSS_RESULT         read_data(int, void *, UINT32);
> +TSS_RESULT         write_data(int, void *, UINT32);
>   int            write_key_init(int, UINT32, UINT32, UINT32);
>   TSS_RESULT     cache_key(UINT32, UINT16, TSS_UUID *, TSS_UUID *, UINT16, 
> UINT32, UINT32);
>   TSS_RESULT     UnloadBlob_KEY_PS(UINT16 *, BYTE *, TSS_KEY *);
> diff --git a/src/include/tspps.h b/src/include/tspps.h
> index 17b0aab..6906369 100644
> --- a/src/include/tspps.h
> +++ b/src/include/tspps.h
> @@ -18,8 +18,8 @@
>
>   TSS_RESULT     get_file(int *);
>   int            put_file(int);
> -inline TSS_RESULT  read_data(int, void *, UINT32);
> -inline TSS_RESULT  write_data(int, void *, UINT32);
> +TSS_RESULT         read_data(int, void *, UINT32);
> +TSS_RESULT         write_data(int, void *, UINT32);
>   UINT32                 psfile_get_num_keys(int);
>   TSS_RESULT     psfile_get_parent_uuid_by_uuid(int, TSS_UUID *, TSS_UUID *);
>   TSS_RESULT     psfile_remove_key_by_uuid(int, TSS_UUID *);
> diff --git a/src/tcs/ps/ps_utils.c b/src/tcs/ps/ps_utils.c
> index 2e7f502..35ac89f 100644
> --- a/src/tcs/ps/ps_utils.c
> +++ b/src/tcs/ps/ps_utils.c
> @@ -42,11 +42,7 @@
>   struct key_disk_cache *key_disk_cache_head = NULL;
>
>
> -#ifdef SOLARIS
>   TSS_RESULT
> -#else
> -inline TSS_RESULT
> -#endif
>   read_data(int fd, void *data, UINT32 size)
>   {
>       int rc;
> @@ -64,11 +60,7 @@ read_data(int fd, void *data, UINT32 size)
>   }
>
>
> -#ifdef SOLARIS
>   TSS_RESULT
> -#else
> -inline TSS_RESULT
> -#endif
>   write_data(int fd, void *data, UINT32 size)
>   {
>       int rc;
> diff --git a/src/tspi/tsp_context_mem.c b/src/tspi/tsp_context_mem.c
> index 2982df9..bf9c299 100644
> --- a/src/tspi/tsp_context_mem.c
> +++ b/src/tspi/tsp_context_mem.c
> @@ -257,9 +257,17 @@ free_tspi(TSS_HCONTEXT tspContext, void *memPointer)
>       return result;
>   }
>
> -/* definition for a memset that cannot be optimized away */
> -void * __no_optimize
> -__tspi_memset(void *s, int c, size_t n)
> +/* There are a few ways to have the compiler not optimize away
> + * memset and thus leave sensative data in freed memory.  The
> + * first is to use the gcc __attribute__((optimize("O0")), but
> + * this has portibility problems.  The next way is to read the data
> + * again, but the compiler might be smart enough to optimize out the
> + * read and the extra execution time is unnecessary
> + * Instead here we declare the variables volatile, which explictly tells
> + * the compiler not to try anything clever with them because
> + * there may be readers/writers that it doesn't know about.
> + */
> +__tspi_memset(volatile void *s, volatile int c, volatile size_t n)
>   {
>       return memset(s, c, n);
>   }
>


------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
TrouSerS-tech mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/trousers-tech

Reply via email to