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) Date: Sun, 27 Feb 2011 08:16:19 -0000 From: 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 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: