From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 817B73858D34 for ; Thu, 18 Jun 2020 19:59:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 817B73858D34 Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-165-AFJQAx1JNbi9p32Eqq3HbA-1; Thu, 18 Jun 2020 15:59:14 -0400 X-MC-Unique: AFJQAx1JNbi9p32Eqq3HbA-1 Received: by mail-wr1-f72.google.com with SMTP id f5so3187407wrv.22 for ; Thu, 18 Jun 2020 12:59:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Hqt4qvnr5dh68Q864sy4fP4+loEvchWNIPP5mMRmPe8=; b=MvMAMUrcBHeoqJdWdIeCYL84fF/KDfSL4i1Yh0tG2LiZKzV728MuVMZiJ6ePNLD3qm USTboRM3Qvp5yHdoAGR0hCn4QIMbz2lEl1DcYbxnm6m4S8SCpSSKnbDqMyLwPxGxuWkJ +DdthRsPikByExr83+3Lpq3am+BaTAjivwaeXswkwzbUPWvd0XiOyfgLL0/EEg8gtLSY uIOt7jDWNFydlSW2adf9HiVzddOsEKGc09ZsvZ2r3gepzQtahY38Fa/k3SUpOz5Au3u2 5tARku5caPfa4MOdl4Tsqani9nbI4VN+Eyo1ooLTwLeSeBt4GKtl8P4YXMAwhkV42Vq6 josg== X-Gm-Message-State: AOAM533/bwWtu2RcYfTQGzz+PC8GQEvT11pxpbIgQz+n34145449onOK T96ebmn+1fKf3Xrka5DY/8XtylFU3OfmEof8EUT7JrqC7rkfanzLQhD+pwKS4iMzylGPsPpWrRA lrrMw6GCVsQm6aiHQ9G4XUg== X-Received: by 2002:a7b:c405:: with SMTP id k5mr5528369wmi.155.1592510353178; Thu, 18 Jun 2020 12:59:13 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyPMTXysvjNdKKS5kC03SlrKBXWc2Dl03OLzi4kH5n5RwkJz6d++LmObfU7akdbf+aLVUPKBg== X-Received: by 2002:a7b:c405:: with SMTP id k5mr5528352wmi.155.1592510352903; Thu, 18 Jun 2020 12:59:12 -0700 (PDT) Received: from ?IPv6:2001:8a0:f922:c400:56ee:75ff:fe8d:232b? ([2001:8a0:f922:c400:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id z9sm4469817wmi.41.2020.06.18.12.59.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 18 Jun 2020 12:59:12 -0700 (PDT) Subject: Re: [PATCH 28/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) To: Tom Tromey , Pedro Alves via Gdb-patches References: <20200414175434.8047-1-palves@redhat.com> <20200414175434.8047-29-palves@redhat.com> <87v9lyq701.fsf@tromey.com> From: Pedro Alves Message-ID: <676648e1-e8f3-419f-439a-4ec3a65f42cb@redhat.com> Date: Thu, 18 Jun 2020 20:59:11 +0100 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: <87v9lyq701.fsf@tromey.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , X-List-Received-Date: Thu, 18 Jun 2020 19:59:17 -0000 Hi Tom, Sorry for the delay in responding. I hate when I end up doing this. On 4/17/20 7:53 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves via Gdb-patches writes: > > Pedro> My next attempt is the one that I'm proposing, which is to bite the > Pedro> bullet and break the connection between inferior_ptid and > Pedro> inferior_thread(), aka the current thread. I.e., make the current > Pedro> thread be a global thread_info pointer that is written to directly by > Pedro> switch_to_thread, etc., and making inferior_thread() return that > Pedro> pointer, instead of having inferior_thread() lookup up the > Pedro> inferior_ptid thread, by ptid_t. > > Does this mean that now the current thread and current inferior can get > out of sync? They already could, because the current thread was based on inferior_ptid, while the current inferior is stored in a global "inferior *current_inferior_" pointer. > Or that these can be out of sync with inferior_ptid? Yes, before, inferior_ptid could get out of sync with the current inferior, but not with the current thread. Afterwards, the current "thread_info *" can also get out of sync with inferior_ptid. That's why most of the series eliminates writes to inferior_ptid directly. Over time, inferior_ptid usages have been disappearing, replaced by references to "thread_info *" or "inferior *" objects directly. I think that's much better than referring to objects via integers that may be ambiguous between targets, and worrying about what target_ops owns what in the ptid_t fields, how to extend ptid for different types of execution objects, etc. I think that ideally, in the GDB core, references to inferior_ptid would virtually disappear. I see this series as another step in that direction. > > This patch looks like it tries to avoid that when writing to the current > thread -- but in the cover letter you mentioned that there are other > assignments to inferior_ptid. I think you are referring to this comment: > "After this, inferior_ptid still exists, but it is mostly read-only and > mainly used by target backend code. It is also relied on by a number > of target methods as a global input argument. E.g., the target_resume > interface and the memory reading routines -- we still need it there > because we need to be able to access memory off of processes for which > we don't have a corresponding inferior/thread object, like when > handling forks. Maybe we could pass down a context explicitly to > target_read_memory, etc." In the target_resume case, we always switch inferior_ptid using switch_to_thread or similar, so not a big deal. The worrying case I found is the memory access routines. TBC, that's a case where the connection between current thread and inferior_ptid was already being broken previously. Let me expand on the "handling forks" bit: When we detach breakpoints from a fork child that we're about to detach from due to "set detach-on-fork off", we need to uninstall memory breakpoints from the child, because breakpoint instructions that were inserted on the parent end up in the child as well, because the child's address space is just a copy of the parent's. So to do that, we switch inferior_ptid to the fork child, and call the breakpoint removal target method, so that it applies to the fork child. Note that there are no thread_info or inferior objects for that fork child in the thread/inferior lists at all. So if something in those code breakpoint removal paths (all the way down the the memory xfer routines) calls inferior_thread() it would already assert for not finding the thread in the list. The patch series doesn't change that. > I wouldn't block the patches based on this but I'd like to understand > the direction. I guess I'd prefer to remove globals and possibilities > for divergence if it's possible. Me too, but it's just that inferior_ptid is so pervasive that it's impossible to eliminate all in one go. Thanks, Pedro Alves