From patchwork Thu Apr 14 20:40:35 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Diego Novillo X-Patchwork-Id: 91292 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id BD654B6F6B for ; Fri, 15 Apr 2011 06:40:55 +1000 (EST) Received: (qmail 9054 invoked by alias); 14 Apr 2011 20:40:54 -0000 Received: (qmail 9044 invoked by uid 22791); 14 Apr 2011 20:40:53 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 14 Apr 2011 20:40:48 +0000 Received: from wpaz37.hot.corp.google.com (wpaz37.hot.corp.google.com [172.24.198.101]) by smtp-out.google.com with ESMTP id p3EKegHw001589 for ; Thu, 14 Apr 2011 13:40:42 -0700 Received: from ywg4 (ywg4.prod.google.com [10.192.7.4]) by wpaz37.hot.corp.google.com with ESMTP id p3EKe5VF023822 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 14 Apr 2011 13:40:36 -0700 Received: by ywg4 with SMTP id 4so595018ywg.24 for ; Thu, 14 Apr 2011 13:40:36 -0700 (PDT) MIME-Version: 1.0 Received: by 10.150.114.6 with SMTP id m6mr1952725ybc.184.1302813635591; Thu, 14 Apr 2011 13:40:35 -0700 (PDT) Received: by 10.150.212.19 with HTTP; Thu, 14 Apr 2011 13:40:35 -0700 (PDT) In-Reply-To: <20110414192853.GR17079@tyan-ft48-01.lab.bos.redhat.com> References: <20110414185053.6C532A00D@topo.tor.corp.google.com> <20110414192853.GR17079@tyan-ft48-01.lab.bos.redhat.com> Date: Thu, 14 Apr 2011 16:40:35 -0400 Message-ID: Subject: Re: [lto/pph] Do not pack more bits than requested (issue4415044) From: Diego Novillo To: Jakub Jelinek Cc: reply@codereview.appspotmail.com, rguenther@suse.de, gcc-patches@gcc.gnu.org X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Thu, Apr 14, 2011 at 15:28, Jakub Jelinek wrote: > If bitpack_word_t has BITS_PER_BITPACK_WORD bits, then for > nbits = BITS_PER_BITPACK_WORD this will be undefined. > Use say > mask = ((bitpack_word_t) 2 << (nbits - 1)) - 1; > or something similar (assertion ensures that nbits isn't 0). Quite right, thanks. In the meantime, I've changed my mind with this. I think it's safer if we just assert that the value we are about to pack fit in the number of bits the caller specified. The only problematic user is pack_ts_type_value_fields when it tries to pack a -1 for the type's alias set. I think we should just stream that as an integer and not go through the bitpacking overhead. For now, I'm applying this to the pph branch. Tested on x86_64. No LTO failures. Diego. * lto-streamer-out.c (pack_ts_type_value_fields): Pack all bits of -1 value. * lto-streamer.h (bitpack_create): Assert that the value to pack does not overflow NBITS. * lto-streamer-in.c (unpack_ts_type_value_fields): Unpack BITS_PER_BITPACK_WORD bits for TYPE_ALIAS_SET. diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c index 97b86ce..f04e031 100644 --- a/gcc/lto-streamer-in.c +++ b/gcc/lto-streamer-in.c @@ -1734,7 +1734,7 @@ unpack_ts_type_value_fields (struct bitpack_d *bp, tree expr) TYPE_USER_ALIGN (expr) = (unsigned) bp_unpack_value (bp, 1); TYPE_READONLY (expr) = (unsigned) bp_unpack_value (bp, 1); TYPE_ALIGN (expr) = (unsigned) bp_unpack_value (bp, HOST_BITS_PER_INT); - TYPE_ALIAS_SET (expr) = bp_unpack_value (bp, HOST_BITS_PER_INT); + TYPE_ALIAS_SET (expr) = bp_unpack_value (bp, BITS_PER_BITPACK_WORD); } diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c index 3ccad8b..89ad9c5 100644 --- a/gcc/lto-streamer-out.c +++ b/gcc/lto-streamer-out.c @@ -518,7 +518,8 @@ pack_ts_type_value_fields (struct bitpack_d *bp, tree expr) bp_pack_value (bp, TYPE_USER_ALIGN (expr), 1); bp_pack_value (bp, TYPE_READONLY (expr), 1); bp_pack_value (bp, TYPE_ALIGN (expr), HOST_BITS_PER_INT); - bp_pack_value (bp, TYPE_ALIAS_SET (expr) == 0 ? 0 : -1, HOST_BITS_PER_INT); + bp_pack_value (bp, TYPE_ALIAS_SET (expr) == 0 ? 0 : -1, + BITS_PER_BITPACK_WORD); } diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h index 0d49430..73afd46 100644 --- a/gcc/lto-streamer.h +++ b/gcc/lto-streamer.h @@ -1190,18 +1190,15 @@ bitpack_create (struct lto_output_stream *s) static inline void bp_pack_value (struct bitpack_d *bp, bitpack_word_t val, unsigned nbits) { - bitpack_word_t mask, word; + bitpack_word_t word = bp->word; int pos = bp->pos; - word = bp->word; - + /* We shouldn't try to pack more bits than can fit in a bitpack word. */ gcc_assert (nbits > 0 && nbits <= BITS_PER_BITPACK_WORD); - /* Make sure that VAL only has the lower NBITS set. Generate a - mask with the lower NBITS set and use it to filter the upper - bits from VAL. */ - mask = ((bitpack_word_t) 1 << nbits) - 1; - val = val & mask; + /* The value to pack should not overflow NBITS. */ + gcc_assert (nbits == BITS_PER_BITPACK_WORD + || val <= ((bitpack_word_t) 1 << nbits)); /* If val does not fit into the current bitpack word switch to the next one. */