From patchwork Sun Feb 27 18:16:19 2011
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: Avoid uninitialized padding bits in REAL_CST/CONST_DOUBLE (PR
middle-end/47903)
X-Patchwork-Submitter: Jakub Jelinek
X-Patchwork-Id: 84699
Message-Id: <20110227181619.GE30899@tyan-ft48-01.lab.bos.redhat.com>
To: Richard Guenther
Cc: gcc-patches@gcc.gnu.org
Date: Sun, 27 Feb 2011 19:16:19 +0100
From: Jakub Jelinek
List-Id:
On Sun, Feb 27, 2011 at 03:51:47PM +0100, Richard Guenther wrote:
> On Sun, Feb 27, 2011 at 1:13 PM, Jakub Jelinek wrote:
> > In some places CONST_DOUBLE bits are compared as the HWIs, thus it is
> > undesirable to have uninitialized bits among them. At least on LP64 hosts
> > there are 32 bits of padding between uexp and sign fields.
> > Most of the places in gcc that work with REAL_VALUE_TYPE seem to actually
> > either clear the whole REAL_VALUE_TYPE by memset, or start by copying
> > to result one of the arguments. The exception seems to be real_arithmetic
> > for PLUS/MINUS/MULT/RDIV, where the result might be the same as either
> > of the operands, or might be completely different. Either real_arithmetic
> > in those 4 cases could compare result ptr with both operands and if it is
> > not equal to either of them, clear it, or the following patch adjusts
> > the callers where they do it.
>
> Hm, it seems fishy to compare CONST_DOUBLEs as HWIs if they have
> padding. But well - I guess it was always that way.
In loc_cmp case it wants stable comparison for sorting, and real comparison
isn't suitable for that, as it e.g. considers +0. and -0. to be equal.
But the code in real_value_from_int_cst is quite old:
31038 kenner /* Clear all bits of the real value type so that we can later do
31038 kenner bitwise comparisons to see if two values are the same. */
69587 ghazi memset (&d, 0, sizeof d);
so I think it loc_cmp isn't the only one which does something like that.
> That said, I still would prefer the clearing to be done in
> real_arithmetic. Just patching the existing callers won't save us
> from introducing more cases like this.
Here it is, bootstrapped/regtested on x86_64-linux and i686-linux,
verified with valgrind on the testcase:
2011-02-27 Jakub Jelinek
PR middle-end/47903
* real.c (real_arithmetic) : Clear padding bits in *r first if
r isn't op0 nor op1.
Jakub
--- gcc/real.c.jj 2010-12-02 13:15:24.000000000 +0100
+++ gcc/real.c 2011-02-27 16:55:43.474134359 +0100
@@ -1009,15 +1009,25 @@ real_arithmetic (REAL_VALUE_TYPE *r, int
switch (code)
{
case PLUS_EXPR:
+ /* Clear any padding areas in *r if it isn't equal to one of the
+ operands so that we can later do bitwise comparisons later on. */
+ if (r != op0 && r != op1)
+ memset (r, '\0', sizeof (*r));
return do_add (r, op0, op1, 0);
case MINUS_EXPR:
+ if (r != op0 && r != op1)
+ memset (r, '\0', sizeof (*r));
return do_add (r, op0, op1, 1);
case MULT_EXPR:
+ if (r != op0 && r != op1)
+ memset (r, '\0', sizeof (*r));
return do_multiply (r, op0, op1);
case RDIV_EXPR:
+ if (r != op0 && r != op1)
+ memset (r, '\0', sizeof (*r));
return do_divide (r, op0, op1);
case MIN_EXPR: