From patchwork Mon May 5 20:58:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 345916 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 1CB3C140317 for ; Tue, 6 May 2014 06:58:22 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; q=dns; s=default; b=Cf6UZjwlT7zfF7Sd g13fXZKGZAOASClZXbOf5JgBveG2VjclOgT66BxpbyDyBqbFnvDSCKWbw32cnZtn Aso+v92MeW/k0iKb9t2KXeBd1phtEsPsp8em9xuJLMz/H6r1Yaa1lnwa4BwmMWvs E6ZfslYv4udhkAC1BxMTYrZinQI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; s=default; bh=1uY4CpaUw7Ex+ChZH1lZYE mlRp4=; b=r+Y5JXHfb4cVg91WawSzeGt1wyPcoliZXpRjcYHGu7ugyufHmeTpPz wntC6jOtc3yUQgZAmVlLqdBi+S0O3cH+47KR8XOo41y7BrcveWtVMmzowLftcjK/ cpyUm4ZmCHj9L0iF2eqAiWcSVfSA4HTueLw3na+j2wNZ92TwaGy6g= Received: (qmail 10658 invoked by alias); 5 May 2014 20:58:16 -0000 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 Received: (qmail 10647 invoked by uid 89); 5 May 2014 20:58:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f173.google.com Received: from mail-wi0-f173.google.com (HELO mail-wi0-f173.google.com) (209.85.212.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 05 May 2014 20:58:13 +0000 Received: by mail-wi0-f173.google.com with SMTP id bs8so6241027wib.0 for ; Mon, 05 May 2014 13:58:10 -0700 (PDT) X-Received: by 10.180.90.51 with SMTP id bt19mr42857wib.22.1399323489974; Mon, 05 May 2014 13:58:09 -0700 (PDT) Received: from localhost ([2.26.169.52]) by mx.google.com with ESMTPSA id uc3sm20096472wib.10.2014.05.05.13.58.08 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 05 May 2014 13:58:09 -0700 (PDT) From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener , GCC Patches , Kenneth Zadeck , Mike Stump , rdsandiford@googlemail.com Cc: GCC Patches , Kenneth Zadeck , Mike Stump Subject: Re: [wide-int] Handle zero-precision INTEGER_CSTs again References: <871twcouo6.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <87ha541onw.fsf@talisman.default> <87d2fs1dou.fsf@talisman.default> Date: Mon, 05 May 2014 21:58:08 +0100 In-Reply-To: (Richard Biener's message of "Mon, 5 May 2014 19:32:31 +0200") Message-ID: <87bnvcx7rz.fsf@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Richard Biener writes: > On Mon, May 5, 2014 at 4:51 PM, Richard Sandiford > wrote: >> Richard Biener writes: >>> On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford >>> wrote: >>>> Richard Biener writes: >>>>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford >>>>> wrote: >>>>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after >>>>>> Richard's patch to remove min amd max values from zero-width bitfields, >>>>>> but a boostrap-ubsan showed otherwise. One source is in: >>>>>> >>>>>> null_pointer_node = build_int_cst (build_pointer_type >>>>>> (void_type_node), 0); >>>>>> >>>>>> if no_target, since the precision of the type comes from ptr_mode, >>>>>> which remains the default VOIDmode. Maybe that's a bug, but setting >>>>>> it to an arbitrary nonzero value would also be dangerous. >>>>> >>>>> Can you explain what 'no_target' should be? ptr_mode should never be >>>>> VOIDmode. >>>> >>>> Sorry, meant "no_backend" rather than "no_target". See do_compile. >>> >>> Ok. So we do >>> >>> /* This must be run always, because it is needed to compute the FP >>> predefined macros, such as __LDBL_MAX__, for targets using non >>> default FP formats. */ >>> init_adjust_machine_modes (); >>> >>> /* Set up the back-end if requested. */ >>> if (!no_backend) >>> backend_init (); >>> >>> where I think that init_adjust_machine_modes should initialize the >>> {byte,word,ptr}_mode globals. Move from init_emit_once. init_adjust_machine_modes is an auto-generated function so I ended up using a new function instead. Tested on x86_64-linux-gnu. OK to install? >>>>> So I don't think we want this patch. Instead stor-layout should >>>>> ICE on zero-precision integer/pointer types. >>>> >>>> What should happen for void_zero_node? >>> >>> Not sure what that beast is supposed to be or why it should be >>> of INTEGER_CST kind (it's not even initialized in any meaningful >>> way). >>> >>> That said, the wide-int code shouldn't be slowed down by >>> precision == 0 checks. We should never ever reach wide-int >>> with such a constant. >> >> void_zero_node is used for ubsan too, and survives into gimple. >> I did hit this in real tests, it wasn't just theoretical. > > Ugh - for what does it use that ... :/ > > Please remember how to trigger those issues and I'll happily have > a look after the merge. At the time it was just a normal bootstrap-ubsan, but that was before the zero-precision patch. Probably the best way of checking for zero-precision tree constants is to put an assert for nonzero precisions in: inline unsigned int wi::int_traits ::get_precision (const_tree tcst) { return TYPE_PRECISION (TREE_TYPE (tcst)); } and: template inline wi::extended_tree ::extended_tree (const_tree t) : m_t (t) { gcc_checking_assert (TYPE_PRECISION (TREE_TYPE (t)) <= N); } and then bootstrap-ubsan. But like Marek says, the ubsan code uses void_zero_node by name. Thanks, Richard gcc/ * emit-rtl.c (init_derived_machine_modes): New functionm, split out from... (init_emit_once): ...here. * rtl.h (init_derived_machine_modes): Declare. * toplev.c (do_compile): Call it even if no_backend. Index: gcc/emit-rtl.c =================================================================== --- gcc/emit-rtl.c 2014-05-03 20:18:49.157107743 +0100 +++ gcc/emit-rtl.c 2014-05-05 17:44:53.579038259 +0100 @@ -5620,6 +5620,30 @@ init_emit_regs (void) } } +/* Initialize global machine_mode variables. */ + +void +init_derived_machine_modes (void) +{ + byte_mode = VOIDmode; + word_mode = VOIDmode; + + for (enum machine_mode mode = GET_CLASS_NARROWEST_MODE (MODE_INT); + mode != VOIDmode; + mode = GET_MODE_WIDER_MODE (mode)) + { + if (GET_MODE_BITSIZE (mode) == BITS_PER_UNIT + && byte_mode == VOIDmode) + byte_mode = mode; + + if (GET_MODE_BITSIZE (mode) == BITS_PER_WORD + && word_mode == VOIDmode) + word_mode = mode; + } + + ptr_mode = mode_for_size (POINTER_SIZE, GET_MODE_CLASS (Pmode), 0); +} + /* Create some permanent unique rtl objects shared between all functions. */ void @@ -5643,36 +5667,6 @@ init_emit_once (void) reg_attrs_htab = htab_create_ggc (37, reg_attrs_htab_hash, reg_attrs_htab_eq, NULL); - /* Compute the word and byte modes. */ - - byte_mode = VOIDmode; - word_mode = VOIDmode; - double_mode = VOIDmode; - - for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT); - mode != VOIDmode; - mode = GET_MODE_WIDER_MODE (mode)) - { - if (GET_MODE_BITSIZE (mode) == BITS_PER_UNIT - && byte_mode == VOIDmode) - byte_mode = mode; - - if (GET_MODE_BITSIZE (mode) == BITS_PER_WORD - && word_mode == VOIDmode) - word_mode = mode; - } - - for (mode = GET_CLASS_NARROWEST_MODE (MODE_FLOAT); - mode != VOIDmode; - mode = GET_MODE_WIDER_MODE (mode)) - { - if (GET_MODE_BITSIZE (mode) == DOUBLE_TYPE_SIZE - && double_mode == VOIDmode) - double_mode = mode; - } - - ptr_mode = mode_for_size (POINTER_SIZE, GET_MODE_CLASS (Pmode), 0); - #ifdef INIT_EXPANDERS /* This is to initialize {init|mark|free}_machine_status before the first call to push_function_context_to. This is needed by the Chill front @@ -5695,6 +5689,8 @@ init_emit_once (void) else const_true_rtx = gen_rtx_CONST_INT (VOIDmode, STORE_FLAG_VALUE); + double_mode = mode_for_size (DOUBLE_TYPE_SIZE, MODE_FLOAT, 0); + REAL_VALUE_FROM_INT (dconst0, 0, 0, double_mode); REAL_VALUE_FROM_INT (dconst1, 1, 0, double_mode); REAL_VALUE_FROM_INT (dconst2, 2, 0, double_mode); Index: gcc/rtl.h =================================================================== --- gcc/rtl.h 2014-05-03 20:18:49.157107743 +0100 +++ gcc/rtl.h 2014-05-05 17:40:26.035785011 +0100 @@ -2545,6 +2545,7 @@ extern int get_max_insn_count (void); extern int in_sequence_p (void); extern void init_emit (void); extern void init_emit_regs (void); +extern void init_derived_machine_modes (void); extern void init_emit_once (void); extern void push_topmost_sequence (void); extern void pop_topmost_sequence (void); Index: gcc/toplev.c =================================================================== --- gcc/toplev.c 2014-04-26 16:05:43.076775028 +0100 +++ gcc/toplev.c 2014-05-05 17:41:00.168079259 +0100 @@ -1891,6 +1891,7 @@ do_compile (void) predefined macros, such as __LDBL_MAX__, for targets using non default FP formats. */ init_adjust_machine_modes (); + init_derived_machine_modes (); /* Set up the back-end if requested. */ if (!no_backend)