From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4492 invoked by alias); 20 May 2019 22:07:06 -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 3890 invoked by uid 89); 20 May 2019 22:07:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 May 2019 22:07:04 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D672D59454; Mon, 20 May 2019 22:06:58 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-8.rdu2.redhat.com [10.10.112.8]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0F7D161B89; Mon, 20 May 2019 22:06:53 +0000 (UTC) Subject: Re: [PATCH][RFC] Sanitize equals and hash functions in hash-tables. To: =?UTF-8?Q?Martin_Li=c5=a1ka?= , Jakub Jelinek Cc: Alexander Monakov , gcc-patches@gcc.gnu.org, Nathan Sidwell , Jason Merrill , Paul Richard Thomas , Martin Jambor References: <23ffca95-6492-e609-aebb-bbdd83b5185d@suse.cz> <20181030100342.GN11625@tucnak> <32744d50-09fd-496c-e97e-9ec478d64ec4@suse.cz> <492d87a7-0210-0df3-f484-f126baa6866c@suse.cz> <47fcf0aa-4b89-5354-1b59-4e6c623f5c3a@suse.cz> From: Jeff Law Openpgp: preference=signencrypt Message-ID: Date: Mon, 20 May 2019 22:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <47fcf0aa-4b89-5354-1b59-4e6c623f5c3a@suse.cz> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-05/txt/msg01328.txt.bz2 On 5/13/19 1:41 AM, Martin Liška wrote: > On 11/8/18 9:56 AM, Martin Liška wrote: >> On 11/7/18 11:23 PM, Jeff Law wrote: >>> On 10/30/18 6:28 AM, Martin Liška wrote: >>>> On 10/30/18 11:03 AM, Jakub Jelinek wrote: >>>>> On Mon, Oct 29, 2018 at 04:14:21PM +0100, Martin Liška wrote: >>>>>> +hashtab_chk_error () >>>>>> +{ >>>>>> + fprintf (stderr, "hash table checking failed: " >>>>>> + "equal operator returns true for a pair " >>>>>> + "of values with a different hash value"); >>>>> BTW, either use internal_error here, or at least if using fprintf >>>>> terminate with \n, in your recent mail I saw: >>>>> ...different hash valueduring RTL pass: vartrack >>>>> ^^^^^^ >>>> Sure, fixed in attached patch. >>>> >>>> Martin >>>> >>>>>> + gcc_unreachable (); >>>>>> +} >>>>> Jakub >>>>> >>>> 0001-Sanitize-equals-and-hash-functions-in-hash-tables.patch >>>> >>>> From 0d9c979c845580a98767b83c099053d36eb49bb9 Mon Sep 17 00:00:00 2001 >>>> From: marxin >>>> Date: Mon, 29 Oct 2018 09:38:21 +0100 >>>> Subject: [PATCH] Sanitize equals and hash functions in hash-tables. >>>> >>>> --- >>>> gcc/hash-table.h | 40 +++++++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 39 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/gcc/hash-table.h b/gcc/hash-table.h >>>> index bd83345c7b8..694eedfc4be 100644 >>>> --- a/gcc/hash-table.h >>>> +++ b/gcc/hash-table.h >>>> @@ -503,6 +503,7 @@ private: >>>> >>>> value_type *alloc_entries (size_t n CXX_MEM_STAT_INFO) const; >>>> value_type *find_empty_slot_for_expand (hashval_t); >>>> + void verify (const compare_type &comparable, hashval_t hash); >>>> bool too_empty_p (unsigned int); >>>> void expand (); >>>> static bool is_deleted (value_type &v) >>>> @@ -882,8 +883,12 @@ hash_table >>>> if (insert == INSERT && m_size * 3 <= m_n_elements * 4) >>>> expand (); >>>> >>>> - m_searches++; >>>> +#if ENABLE_EXTRA_CHECKING >>>> + if (insert == INSERT) >>>> + verify (comparable, hash); >>>> +#endif >>>> >>>> + m_searches++; >>>> value_type *first_deleted_slot = NULL; >>>> hashval_t index = hash_table_mod1 (hash, m_size_prime_index); >>>> hashval_t hash2 = hash_table_mod2 (hash, m_size_prime_index); >>>> @@ -930,6 +935,39 @@ hash_table >>>> return &m_entries[index]; >>>> } >>>> >>>> +#if ENABLE_EXTRA_CHECKING >>>> + >>>> +/* Report a hash table checking error. */ >>>> + >>>> +ATTRIBUTE_NORETURN ATTRIBUTE_COLD >>>> +static void >>>> +hashtab_chk_error () >>>> +{ >>>> + fprintf (stderr, "hash table checking failed: " >>>> + "equal operator returns true for a pair " >>>> + "of values with a different hash value\n"); >>>> + gcc_unreachable (); >>>> +} >>> I think an internal_error here is probably still better than a simple >>> fprintf, even if the fprintf is terminated with a \n :-) >> Fully agree with that, but I see a lot of build errors when using internal_error. >> >>> The question then becomes can we bootstrap with this stuff enabled and >>> if not, are we likely to soon? It'd be a shame to put it into >>> EXTRA_CHECKING, but then not be able to really use EXTRA_CHECKING >>> because we've got too many bugs to fix. >> Unfortunately it's blocked with these 2 PRs: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87845 >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87847 > Hi. > > I've just added one more PR: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90450 > > I'm sending updated version of the patch that provides a disablement for the 3 PRs > with a new function disable_sanitize_eq_and_hash. > > With that I can bootstrap and finish tests. However, I've done that with a patch > limits maximal number of checks: So rather than call the disable_sanitize_eq_and_hash, can you have its state set up when you instantiate the object? It's not a huge deal, just thinking about loud. So how do we want to go forward, particularly the EXTRA_EXTRA checking issue :-) Jeff