From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id eH6TG5AoW2GidQAAWB0awg (envelope-from ) for ; Mon, 04 Oct 2021 12:15:12 -0400 Received: by simark.ca (Postfix, from userid 112) id 6E9E01EE1A; Mon, 4 Oct 2021 12:15:12 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id BFEE61E813 for ; Mon, 4 Oct 2021 12:15:11 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 78CAA3858010 for ; Mon, 4 Oct 2021 16:15:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 78CAA3858010 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1633364111; bh=HZkCqSwwZGzWspPCBZRQvtBxovFFvm452Y57LHniP7w=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=QQAHJlUCC3rssEmIJmzHwvFFCekeLPNDQki7mqtE43wPyAHnm33EQ0k0gQ0DwOWnY k7jDXhhARDvCODgMd/hFa++265EctRhtXIyPBUOvVXy8na9WKIP00K+r/Lva7xrTNH A2T/3K2serA9fvreGvVGlNeJKPJu5Y5IV+YOm7us= Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id D43B93858D28 for ; Mon, 4 Oct 2021 16:14:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D43B93858D28 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 1AC8B222DC; Mon, 4 Oct 2021 16:14:53 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 05B0B13AF3; Mon, 4 Oct 2021 16:14:53 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id nekmAH0oW2HCMgAAMHmgww (envelope-from ); Mon, 04 Oct 2021 16:14:53 +0000 Subject: Re: [PATCH 1/4] [gdb/symtab] Fix htab_find_slot call in read_call_site_scope To: Simon Marchi , gdb-patches@sourceware.org References: <20211001123328.22314-1-tdevries@suse.de> <113a7cab-f06b-32ad-caa1-b0c87e67335b@polymtl.ca> <24809991-4982-9951-a7f7-514a2d01cd10@polymtl.ca> <17aa014b-2500-14f0-81af-a5de2a98e657@suse.de> Message-ID: <6fbc3a03-fc42-8b14-4590-d60657e70446@suse.de> Date: Mon, 4 Oct 2021 18:14:52 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 10/4/21 5:41 PM, Simon Marchi wrote: > On 2021-10-04 08:46, Tom de Vries wrote: >> On 10/4/21 2:05 PM, Simon Marchi wrote: >>>>> Ah, I had not seen this comment. So it was on purpose. Still, I think >>>>> that it makes it more confusing than anything. The patch LGTM. >>>> >>>> And, this follow-up commit reverts everything except the comment. >>>> >>>> Any comments? >>>> >>>> Thanks, >>>> - Tom >>>> >>> >>> Is there a problem with having the lookups done with just the pc? >> >> Well, there's the problem that I describe in the commit message. I >> don't known of any other problem. >> >>> If we >>> were to replace this with some C++ hash table, say std::unordered_map, it >>> would be std::unordered_map. >> >> Right, because there's a separation between key and element. >> >>> So doing >>> the lookups using just the pc in the htab makes sense to me. >> >> I'm sorry, I don't really understand what you're trying to say here. >> >> Do you agree with the patch? Do you disagree with the patch. Are you >> suggesting an alternative solution? > > My thinking was: why not keep core_addr_eq and core_addr_hash, and pass > a pointer to a CORE_ADDR when calling htab_find_slot. And drop the > requirement for call_site::pc to be the first member of call_site. But > I now realize that this may not be a correct use of htab: htab_find > passes entries to the eq func. So if we have core_addr_eq as the eq > func and htab passes a call_site* entry to core_addr_eq, it won't work. > Now, we don't actually use htab_find, so it would still work for us. > But that would be like setting up a trap for whoever tries to use > htab_find in the future. > All find variants (with the explicit exception of find_empty_slot_for_expand) call eq_f on both an element argument and a hash table element. Consequently, if we break first-field-type compatibility between the two, the element argument must be of the same type as the hash table element, and the eq function must use hash table element type. This is not a future problem, this is an actual problem I ran into when I tried it out moving the pc to the second field position, and the problem is described in the commit log. > So, the patche LGTM. Ack, thanks for the review, I'll commit. Thanks, - Tom