Hi Joel,

the thing here is that disabling O0 attributes is almost always a security 
concern.
The reason is that GCC and Clang will "optimize out" a memset(0) that comes 
before a free since "that does not change program's behaviour".
OpenSSL and other just try to be so complex that the compiler will not recognize
this pattern, but this is a "cats and mice game" (german saying)... ;-)

The file spi_utils.h / ps_utils.c for example use O0 in order to prevent 
information
leak on free calls.
Specifically, the last commit to spi_utils.h is 
"[29a8b1] tspi: add a memset that shouldn't be optimized out"
and goes back to some discussion between myself and Kent...

I understand your commit and the CLang advantages, but I would recomment to
not comment sloppily that "not disabling the optimization seems not so scary".

Best regards,
Andreas

________________________________________
Von: Joel Schopp [[email protected]]
Gesendet: Mittwoch, 21. August 2013 00:29
An: [email protected]
Cc: Yunlian Jiang; Richard Maciel Costa
Betreff: [TrouSerS-tech] [PATCH] make trousers compile with clang

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 but still declares itself __GNUC__ .
In this case not disabling the optimization seems not so scary, so we
just add another check for clang and call it happy.

Signed-off-by: Joel Schopp <[email protected]>
---
 src/include/spi_utils.h |  5 ++++-
 src/include/tcsps.h     |  9 ++-------
 src/include/tspps.h     |  4 ++--
 src/tcs/ps/ps_utils.c   | 14 ++++----------
 4 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/src/include/spi_utils.h b/src/include/spi_utils.h
index 11255b2..9f88e47 100644
--- a/src/include/spi_utils.h
+++ b/src/include/spi_utils.h
@@ -44,7 +44,10 @@ MUTEX_DECLARE_EXTERN(mem_cache_lock);
 #define TSS_PSFILE_INCREMENT_NUM_KEYS  1
 #define TSS_PSFILE_DECREMENT_NUM_KEYS  0

-#ifdef __GNUC__
+/* clang defines support for gnuc exentions, but doesn't support optimize
+ * it also doesn't support per function optimization in any form
+ */
+#if defined(__GNUC__) && ! defined(__clang__)
 #define __no_optimize __attribute__((optimize("O0")))
 #else
 #define __no_optimize
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..57aab82 100644
--- a/src/tcs/ps/ps_utils.c
+++ b/src/tcs/ps/ps_utils.c
@@ -42,11 +42,8 @@
 struct key_disk_cache *key_disk_cache_head = NULL;


-#ifdef SOLARIS
-TSS_RESULT
-#else
-inline TSS_RESULT
-#endif
+
+TSS_RESULT
 read_data(int fd, void *data, UINT32 size)
 {
        int rc;
@@ -64,11 +61,8 @@ read_data(int fd, void *data, UINT32 size)
 }


-#ifdef SOLARIS
-TSS_RESULT
-#else
-inline TSS_RESULT
-#endif
+
+TSS_RESULT
 write_data(int fd, void *data, UINT32 size)
 {
        int rc;
--
1.8.1.2


------------------------------------------------------------------------------
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

------------------------------------------------------------------------------
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