From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67101 invoked by alias); 8 Nov 2018 08:56:15 -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 67085 invoked by uid 89); 8 Nov 2018 08:56:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 08 Nov 2018 08:56:13 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id A52A9AA7C; Thu, 8 Nov 2018 08:56:10 +0000 (UTC) Subject: Re: [PATCH][RFC] Sanitize equals and hash functions in hash-tables. To: Jeff Law , 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> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <492d87a7-0210-0df3-f484-f126baa6866c@suse.cz> Date: Thu, 08 Nov 2018 08:56:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00523.txt.bz2 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 I'm fine with having the patch in in next stage1 after the problems will be fixed. Martin > >> + >> +/* Verify that all existing elements in th hash table which are > s/th/the/ > > > Jeff >