From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94007 invoked by alias); 29 Jan 2016 03:53:36 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 93990 invoked by uid 89); 29 Jan 2016 03:53:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=knack, study, problem! X-HELO: mail-qg0-f66.google.com Received: from mail-qg0-f66.google.com (HELO mail-qg0-f66.google.com) (209.85.192.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 29 Jan 2016 03:53:34 +0000 Received: by mail-qg0-f66.google.com with SMTP id b35so4558502qge.2 for ; Thu, 28 Jan 2016 19:53:33 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=H8mtKWUjrDsbq3bVkEp6LSS3NPU5Igl00/jM8qkeSfc=; b=BuQd0i1i+fJapV9YC7RrtoSq9ESeGwWVu4Jh63/kaMRxKF2AuI3yR/FnMuaxM9eBth HACnmAiPSML1Ih8Xv+x+LSj5X4nq9ZgMcwCulGI785ta+uM0Npcm5U/QjoR8FIS+6AST WjUZ61ZlvZr4O8n5OltsiF7y4SRvKz1Y0atfwT755KGXjbyovHECgT+QlQ5vTfgz2X/d qrDA6MjHpIe9nK6YL3nVAYGdzC9IP7iedd91rYWws/nkRC/qW1kEcRGqF/miQyTde2Ol uxOwqzzkl1zs1bDRNPECFi7hajM++ipClF+Ghlc1FUn7WecET96X3aCoKvojOslXGzFi A+RA== X-Gm-Message-State: AG10YORAA3pwo7TyomMxKMvErMVgg/l2dLngtiw8bGhABeIQuBuW+3Df+AkSXlPl9gM0Ag== X-Received: by 10.140.40.134 with SMTP id x6mr8078275qgx.8.1454039611980; Thu, 28 Jan 2016 19:53:31 -0800 (PST) Received: from [192.168.0.26] (71-212-229-169.hlrn.qwest.net. [71.212.229.169]) by smtp.gmail.com with ESMTPSA id g189sm5917481qhg.2.2016.01.28.19.53.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Jan 2016 19:53:31 -0800 (PST) Message-ID: <56AAE239.5010500@gmail.com> Date: Fri, 29 Jan 2016 03:53:00 -0000 From: Martin Sebor User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Marek Polacek , GCC Patches , Jason Merrill Subject: Re: C++ PATCH for c++/69509 and c++/69516 (infinite recursion with invalid VLAs in constexpr functions) References: <20160128203305.GE25193@redhat.com> In-Reply-To: <20160128203305.GE25193@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-01/txt/msg02272.txt.bz2 On 01/28/2016 01:33 PM, Marek Polacek wrote: > This patch fixes some problems with VLAs and constexprs. (The runtime aspect > of this matter is being tracked in PR69517.) It does two things: > > 1) Changes build_vec_init slightly to prevent the compiler from getting into > infinite recursion. Currently, we emit the following sequence when we're > initializing e.g. n = 1; int a[n] = { 1, 2 }; (cleanup_point junk omitted): > > int a[0:(sizetype) (SAVE_EXPR <(ssizetype) n + -1>)]; > (void) (int[0:(sizetype) (SAVE_EXPR <(ssizetype) n + -1>)] *) int * D.2256; > (void) (D.2256 = (int *) &a) > int * D.2257; > (void) (D.2257 = D.2256) > long int D.2258; > (void) (D.2258 = (long int) (SAVE_EXPR <(ssizetype) n + -1>)) // == 0 > (void) (*D.2257 = 1) > (void) ++D.2257 > (void) --D.2258 // == -1 > (void) (*D.2257 = 2) > (void) ++D.2257 > (void) --D.2258 // == -2 > > while (1) > { > if (D.2258 == -1) // never triggers > goto ; > (void) (*D.2257 = 0) > (void) ++D.2257 > (void) --D.2258; > } > :; > ... > > So we first write the elements from the initializer and then we default-initialize > any remaining elements. But the iterator == -1 check is never true for invalid > code, which means the compiler will get stuck in the while loop forever, leading > to crash -- it tries to cxx_eval_* the body of the loop over and over again. > > Fixed by changing == -1 into <= -1; this should only ever happen for invalid code, > but we don't want to ICE in any case. > > This also "fixes" the problem in PR69517 -- the program could get into infinite > recursion as well, because it was evaluating the sequence above at runtime. But > since it's invoking UB, it doesn't matter much. > > 2) Moves the check for "array subscript out of bound" a tad above, because for > invalid VLAs we can't rely on the bool "found" -- e.g. for the example above > it will find all indexes in the initializer, so "found" is true, which means > we wouldn't get to the out-of-bounds check below. I haven't had time to study the code but I did apply the patch and play with it a bit. It sure does fix the infinite loop/ recursion problem! That said, while the test case below is rejected with the array being of type int (as it should be), it's accepted as is, with the struct. I don't know enough to tell if it's because of the change you mention or if it's a latent bug. The test case is also accepted with an invalid argument to foo (negative or excessive), both which should be rejected as well. You're clearly good and efficient at fixing things. I seem to have a knack for breaking them. Please let me know if I should open a new bug for this. struct A { constexpr A (int = 0) { } constexpr operator int () const { return 0; } }; constexpr int foo (int n) { A a [n] = { 1, 2, 3, 4, 5, 6 }; return a [99]; } struct B { unsigned b: foo (1) + 1; }; Martin