From c17cd53d8d4a643e4f7277da37d21dc3bade5ed6 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Fri, 12 Nov 2010 11:24:30 +0000 Subject: [PATCH] Fix crasher in ast.c:dict_pos on i686 with gcc 4.5 and -O2 This patch works around what appears to be an optimization bug in gcc 4.5. The symptom of the bug is that dict_pos, called from dict_lookup, receives an invalid value when accessing dict->used. The following ticket describes the bug, and includes a simple test case: https://fedorahosted.org/augeas/ticket/149 Adding a printf to dict_lookup immediately before the dict_pos call reveals that dict->used has a value of 30 in the crashing case on my F14 system. A printf added to the first line of dict_pos shows dict->used has an apparent value of 16777246. This causes an invalid array lookup shortly afterwards, which causes the crash. 16777246, interestingly, is 2^24 + 30, where 24 is the size of the dict->used bitfield. This should obviously not be possible, suggesting either a compiler bug or undefined behaviour due in incorrect use of the language. Having re-read the relevant section of K&R, I can't see anything about this use of bitfields which might be a problem, except that: 'Fields may be declared only as ints; for portability, specify signed or unsigned explicitly.' uint32_t resolves to 'unsigned int' on i686, and in any case, replacing it with 'unsigned int' does not solve the problem. As it stands, I believe this is a bug in the optimizer. The above ticket contains an alternate patch which copies dict in dict_pos, which also prevents the crash. However, adding anything to dict_pos which prevents the compiler from optimising dict away achieves the same result. For example: printf("%p", dict); This patch instead removes the bitfields in struct dict. While this is a workaround, these bitfields save at most 1 word of memory per struct, but at the cost of mis-aligned access. The patch does not increase the size of dict_max_size to UINT32_MAX, as the lower maximum serves as a better guard against runaway memory allocation. --- src/ast.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ast.c b/src/ast.c index 4a3402d..6efcc3e 100644 --- a/src/ast.c +++ b/src/ast.c @@ -54,9 +54,9 @@ struct dict_node { string */ struct dict { struct dict_node **nodes; - uint32_t size : 24; - uint32_t used : 24; - uint32_t marked : 1; + uint32_t size; + uint32_t used; + bool marked; }; static const int dict_initial_size = 2; -- 1.5.5.6