[asterisk-commits] kpfleming: branch 1.4 r129966 - /branches/1.4/main/astmm.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Jul 11 09:03:53 CDT 2008


Author: kpfleming
Date: Fri Jul 11 09:03:52 2008
New Revision: 129966

URL: http://svn.digium.com/view/asterisk?view=rev&rev=129966
Log:
fix a flaw found while experimenting with structure alignment and padding; low-fence checking would not work properly on 64-bit platforms, because the compiler was putting 4 bytes of padding between the fence field and the allocation memory block

added a very obvious runtime warning if this condition reoccurs, so the developer who broke it can be chastised into fixing it :-)


Modified:
    branches/1.4/main/astmm.c

Modified: branches/1.4/main/astmm.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/main/astmm.c?view=diff&rev=129966&r1=129965&r2=129966
==============================================================================
--- branches/1.4/main/astmm.c (original)
+++ branches/1.4/main/astmm.c Fri Jul 11 09:03:52 2008
@@ -31,6 +31,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <stddef.h>
 #include <time.h>
 
 #include "asterisk/cli.h"
@@ -66,14 +67,26 @@
 
 static FILE *mmlog;
 
+/* NOTE: Be EXTREMELY careful with modifying this structure; the total size of this structure
+   must result in 'automatic' alignment so that the 'fence' field lands exactly at the end of
+   the structure in memory (and thus immediately before the allocated region the fence is
+   supposed to be used to monitor). In other words, we cannot allow the compiler to insert
+   any padding between this structure and anything following it, so add up the sizes of all the
+   fields and compare to sizeof(struct ast_region)... if they don't match, then the compiler
+   is padding the structure and either the fields need to be rearranged to eliminate internal
+   padding, or a dummy field will need to be inserted before the 'fence' field to push it to
+   the end of the actual space it will consume. Note that this must be checked for both 32-bit
+   and 64-bit platforms, as the sizes of pointers and 'size_t' differ on these platforms.
+*/
+
 static struct ast_region {
 	struct ast_region *next;
+	size_t len;
 	char file[40];
 	char func[40];
 	unsigned int lineno;
 	enum func_type which;
 	unsigned int cache;		/* region was allocated as part of a cache pool */
-	size_t len;
 	unsigned int fence;
 	unsigned char data[0];
 } *regions[SOME_PRIME];
@@ -463,6 +476,11 @@
 void __ast_mm_init(void)
 {
 	char filename[PATH_MAX];
+	int pad;
+
+	if ((pad = (__alignof__(struct ast_region) - (offsetof(struct ast_region, data) % __alignof__(struct ast_region)))) != 0) {
+		ast_log(LOG_ERROR, "struct ast_region has %d bytes of padding! This must be eliminated for low-fence checking to work properly!\n", pad);
+	}
 
 	ast_cli_register_multiple(cli_memory, sizeof(cli_memory) / sizeof(struct ast_cli_entry));
 	




More information about the asterisk-commits mailing list