[augeas] Upstream patch proposed to fix GCC optimization bug (RHBZ#651992).

Richard W.M. Jones rjones at fedoraproject.org
Thu Nov 18 10:34:23 UTC 2010


commit 26219689993c902530827448199f494a9359ae8d
Author: Richard W.M. Jones <rjones at redhat.com>
Date:   Thu Nov 18 10:32:58 2010 +0000

    Upstream patch proposed to fix GCC optimization bug (RHBZ#651992).

 ...-c17cd53d8d4a643e4f7277da37d21dc3bade5ed6.patch |   62 ++++++++++++++++++++
 augeas.spec                                        |   11 +++-
 2 files changed, 72 insertions(+), 1 deletions(-)
---
diff --git a/augeas.git-c17cd53d8d4a643e4f7277da37d21dc3bade5ed6.patch b/augeas.git-c17cd53d8d4a643e4f7277da37d21dc3bade5ed6.patch
new file mode 100644
index 0000000..51fd713
--- /dev/null
+++ b/augeas.git-c17cd53d8d4a643e4f7277da37d21dc3bade5ed6.patch
@@ -0,0 +1,62 @@
+From c17cd53d8d4a643e4f7277da37d21dc3bade5ed6 Mon Sep 17 00:00:00 2001
+From: Matthew Booth <mbooth at redhat.com>
+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
+
diff --git a/augeas.spec b/augeas.spec
index 75c6df7..e25b1f6 100644
--- a/augeas.spec
+++ b/augeas.spec
@@ -1,6 +1,6 @@
 Name:           augeas
 Version:        0.7.3
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        A library for changing configuration files
 
 Group:          System Environment/Libraries
@@ -12,6 +12,10 @@ BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildRequires:  readline-devel libselinux-devel
 Requires:       %{name}-libs = %{version}-%{release}
 
+# Upstream patch proposed to fix GCC optimization bug:
+# https://bugzilla.redhat.com/show_bug.cgi?id=651992
+Patch0:         augeas.git-c17cd53d8d4a643e4f7277da37d21dc3bade5ed6.patch
+
 %description
 A library for programmatically editing configuration files. Augeas parses
 configuration files into a tree structure, which it exposes through its
@@ -44,6 +48,8 @@ The libraries for %{name}.
 %prep
 %setup -q
 
+%patch0 -p1
+
 %build
 %configure --disable-static
 make %{?_smp_mflags}
@@ -85,6 +91,9 @@ rm -rf $RPM_BUILD_ROOT
 %{_libdir}/pkgconfig/augeas.pc
 
 %changelog
+* Thu Nov 18 2010 Richard W.M. Jones <rjones at redhat.com> - 0.7.3-2
+- Upstream patch proposed to fix GCC optimization bug (RHBZ#651992).
+
 * Fri Aug  6 2010 David Lutterkort <lutter at redhat.com> - 0.7.3-1
 - Remove upstream patches
 


More information about the scm-commits mailing list