From patchwork Fri Oct 7 11:14:24 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 118299 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 87D0FB70B7 for ; Fri, 7 Oct 2011 22:14:54 +1100 (EST) Received: (qmail 3333 invoked by alias); 7 Oct 2011 11:14:51 -0000 Received: (qmail 3323 invoked by uid 22791); 7 Oct 2011 11:14:48 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 07 Oct 2011 11:14:29 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p97BERw7014819 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 7 Oct 2011 07:14:27 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [10.16.42.4]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p97BEQH9001157 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 7 Oct 2011 07:14:27 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (localhost.localdomain [127.0.0.1]) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4) with ESMTP id p97BEPJD008373; Fri, 7 Oct 2011 13:14:25 +0200 Received: (from jakub@localhost) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4/Submit) id p97BEPJR008371; Fri, 7 Oct 2011 13:14:25 +0200 Date: Fri, 7 Oct 2011 13:14:24 +0200 From: Jakub Jelinek To: Jason Merrill Cc: Diego Novillo , reply@codereview.appspotmail.com, crowl@google.com, gcc-patches@gcc.gnu.org Subject: [PATCH] Fix htab lookup bug in reregister_specialization (issue5190046, take 2) Message-ID: <20111007111424.GJ19412@tyan-ft48-01.lab.bos.redhat.com> Reply-To: Jakub Jelinek References: <20111005194938.E11AA1DA1A5@topo.tor.corp.google.com> <20111005200539.GX19412@tyan-ft48-01.lab.bos.redhat.com> <4E8CC631.9070400@redhat.com> <20111005211535.GY19412@tyan-ft48-01.lab.bos.redhat.com> <4E8D0802.7030400@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4E8D0802.7030400@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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 Wed, Oct 05, 2011 at 09:44:34PM -0400, Jason Merrill wrote: > On 10/05/2011 05:15 PM, Jakub Jelinek wrote: > >If optimize_specialization_lookup_p (tmpl) doesn't change return value in > >between the two calls, then you are right. But perhaps in that case > >you could avoid the second call and use slot != NULL instead. > > That makes sense too. Here is a modified patch then, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Would just the reregister_specialization change be ok for 4.6 too? 2011-10-07 Jakub Jelinek Diego Novillo * pt.c (reregister_specialization): Use htab_find instead of htab_find_slot with INSERT. (maybe_process_partial_specialization, lookup_template_class_1): Change slot variable type to void ** to avoid aliasing problems. (register_specialization): Likewise. Use slot != NULL instead of more expensive !optimize_specialization_lookup_p (tmpl) test. Jakub --- gcc/cp/pt.c.jj 2011-10-03 14:27:49.000000000 +0200 +++ gcc/cp/pt.c 2011-10-07 09:14:12.000000000 +0200 @@ -892,7 +892,8 @@ maybe_process_partial_specialization (tr instantiation. Reassign it to the new member specialization template. */ spec_entry elt; - spec_entry **slot; + spec_entry *entry; + void **slot; elt.tmpl = most_general_template (tmpl); elt.args = CLASSTYPE_TI_ARGS (inst); @@ -903,10 +904,10 @@ maybe_process_partial_specialization (tr elt.tmpl = tmpl; elt.args = INNERMOST_TEMPLATE_ARGS (elt.args); - slot = (spec_entry **) - htab_find_slot (type_specializations, &elt, INSERT); - *slot = ggc_alloc_spec_entry (); - **slot = elt; + slot = htab_find_slot (type_specializations, &elt, INSERT); + entry = ggc_alloc_spec_entry (); + *entry = elt; + *slot = entry; } else if (COMPLETE_OR_OPEN_TYPE_P (inst)) /* But if we've had an implicit instantiation, that's a @@ -1294,7 +1295,7 @@ register_specialization (tree spec, tree hashval_t hash) { tree fn; - spec_entry **slot = NULL; + void **slot = NULL; spec_entry elt; gcc_assert (TREE_CODE (tmpl) == TEMPLATE_DECL && DECL_P (spec)); @@ -1327,10 +1328,10 @@ register_specialization (tree spec, tree if (hash == 0) hash = hash_specialization (&elt); - slot = (spec_entry **) + slot = htab_find_slot_with_hash (decl_specializations, &elt, hash, INSERT); if (*slot) - fn = (*slot)->spec; + fn = ((spec_entry *) *slot)->spec; else fn = NULL_TREE; } @@ -1423,11 +1424,12 @@ register_specialization (tree spec, tree && !check_specialization_namespace (tmpl)) DECL_CONTEXT (spec) = DECL_CONTEXT (tmpl); - if (!optimize_specialization_lookup_p (tmpl)) + if (slot != NULL /* !optimize_specialization_lookup_p (tmpl) */) { + spec_entry *entry = ggc_alloc_spec_entry (); gcc_assert (tmpl && args && spec); - *slot = ggc_alloc_spec_entry (); - **slot = elt; + *entry = elt; + *slot = entry; if (TREE_CODE (spec) == FUNCTION_DECL && DECL_NAMESPACE_SCOPE_P (spec) && PRIMARY_TEMPLATE_P (tmpl) && DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (tmpl)) == NULL_TREE) @@ -1639,19 +1641,19 @@ iterative_hash_template_arg (tree arg, h bool reregister_specialization (tree spec, tree tinfo, tree new_spec) { - spec_entry **slot; + spec_entry *entry; spec_entry elt; elt.tmpl = most_general_template (TI_TEMPLATE (tinfo)); elt.args = TI_ARGS (tinfo); elt.spec = NULL_TREE; - slot = (spec_entry **) htab_find_slot (decl_specializations, &elt, INSERT); - if (*slot) + entry = (spec_entry *) htab_find (decl_specializations, &elt); + if (entry != NULL) { - gcc_assert ((*slot)->spec == spec || (*slot)->spec == new_spec); + gcc_assert (entry->spec == spec || entry->spec == new_spec); gcc_assert (new_spec != NULL_TREE); - (*slot)->spec = new_spec; + entry->spec = new_spec; return 1; } @@ -7042,7 +7044,7 @@ lookup_template_class_1 (tree d1, tree a { tree templ = NULL_TREE, parmlist; tree t; - spec_entry **slot; + void **slot; spec_entry *entry; spec_entry elt; hashval_t hash; @@ -7480,10 +7482,11 @@ lookup_template_class_1 (tree d1, tree a SET_TYPE_TEMPLATE_INFO (t, build_template_info (found, arglist)); elt.spec = t; - slot = (spec_entry **) htab_find_slot_with_hash (type_specializations, - &elt, hash, INSERT); - *slot = ggc_alloc_spec_entry (); - **slot = elt; + slot = htab_find_slot_with_hash (type_specializations, + &elt, hash, INSERT); + entry = ggc_alloc_spec_entry (); + *entry = elt; + *slot = entry; /* Note this use of the partial instantiation so we can check it later in maybe_process_partial_specialization. */