From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id XiSzBrKdlWCGTQAAWB0awg (envelope-from ) for ; Fri, 07 May 2021 16:06:10 -0400 Received: by simark.ca (Postfix, from userid 112) id 0CCDA1F11C; Fri, 7 May 2021 16:06:10 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.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 232341E783 for ; Fri, 7 May 2021 16:06:09 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 939243A3E402; Fri, 7 May 2021 20:06:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 939243A3E402 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1620417968; bh=FYymx5UVudbSpoelwPi/33FJIDR/Jg7jmdflCJkpgKI=; 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=jhwwL7fRrVz6uVPtT6/uvbv/eS64BSlxRA9mYjs4FyoqIVy1gAudbnaO13fxZIXL6 0q48RqkKK3ho/ZBTjbuKauCj+EE+PQJmYtotEWrjUvDl4kgi7GS6liHq6FR4rrkM1c ThQI87jR8+SC5lMbs7F0oU4yN1lTJf7jRNxZQizc= Received: from mail-qk1-x72e.google.com (mail-qk1-x72e.google.com [IPv6:2607:f8b0:4864:20::72e]) by sourceware.org (Postfix) with ESMTPS id DDF7B3A16403 for ; Fri, 7 May 2021 20:06:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DDF7B3A16403 Received: by mail-qk1-x72e.google.com with SMTP id o5so9786604qkb.0 for ; Fri, 07 May 2021 13:06:04 -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=FYymx5UVudbSpoelwPi/33FJIDR/Jg7jmdflCJkpgKI=; b=SCBgakPEQ6i5bC3WSA8L3yq2pio+ByWrIggL3+djuueVPrDzj5EBySXsiGGkxW3XgS fHLEnhaUxt2tOOZAocog2jqgekUrxU25INWAO0PzUIfZDsIwjpF/d+9awsyTwu5VJ/Uj MNSWUSRzds0u5t58NHL3YywXMZmGPT8En8KMdeQeKrsqhv43Zb1CUvTOZg4SEMsiTpKO Ue/JZ5TQGaAeaE6zcFU/8bOzwFDcVZZ+uklqGE3BPIM3MTo2x123VDjLVQvQXdZomWX4 iLDl7IgPye1RYf2wWnsSqhi5c+QelhxF9sumS0OutEi/Jgsdob8HCWpDUleGu3aoKeav 9kAw== X-Gm-Message-State: AOAM532H2dCndBfQRIfDchUZ5Y4440uwLGlB3N3Ju2mLO19B1SMyh0Kv TOTUBKcYzZ+Sjy+qvOlTjjx2fKEdw6SEfg== X-Google-Smtp-Source: ABdhPJx0bHQdQklCTHvD3T5rRrmhGkDrtvM39YWacIGBm5oVtvlD9dzLSGur25ASjclfNvFm1XZgBw== X-Received: by 2002:a05:620a:40ce:: with SMTP id g14mr11302036qko.190.1620417964409; Fri, 07 May 2021 13:06:04 -0700 (PDT) Received: from ?IPv6:2804:7f0:4841:40ad:2455:cbab:c687:242e? ([2804:7f0:4841:40ad:2455:cbab:c687:242e]) by smtp.gmail.com with ESMTPSA id y26sm5783202qtf.66.2021.05.07.13.06.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 07 May 2021 13:06:04 -0700 (PDT) Subject: Re: [PATCH][gdb/tdep] Use pid to choose x86_64 process 64/32-bitness To: Simon Marchi , Tom de Vries , gdb-patches@sourceware.org References: <20210507084402.GA14817@delia> Message-ID: <5566f655-e6da-ac0e-2b81-b467c484b61b@linaro.org> Date: Fri, 7 May 2021 17:06:01 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed 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: Luis Machado via Gdb-patches Reply-To: Luis Machado Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 5/7/21 4:27 PM, Simon Marchi via Gdb-patches wrote: > On 2021-05-07 4:44 a.m., Tom de Vries wrote: >> Hi, >> >> In a linux kernel mailing list discussion, it was mentioned that "gdb has >> this odd thing where it takes the 64-bit vs 32-bit data for the whole process >> from one thread, and picks the worst possible thread to do it (ie explicitly >> not even the main thread, ...)" [1]. >> >> The picking of the thread is done here in >> x86_linux_nat_target::read_description: >> ... >> /* GNU/Linux LWP ID's are process ID's. */ >> tid = inferior_ptid.lwp (); >> if (tid == 0) >> tid = inferior_ptid.pid (); /* Not a threaded program. */ >> ... >> >> To understand what this code does, let's investigate a scenario in which >> inferior_ptid.lwp () != inferior_ptid.pid (). >> >> Say we start exec jit-attach-pie, identified with pid x. The main thread >> starts another thread that sleeps, and then the main thread waits for the >> sleeping thread. So we have two threads, identified with LWP IDs x and x+1: >> ... >> PID LWP CMD >> x x ./jit-attach-pie >> x x+1 ./jit-attach-pie >> ... >> [ The thread with LWP x is known as the thread group leader. ] >> >> When attaching to this exec using the pid, gdb does a stop_all_threads which >> iterates over all the threads, first LWP x, and then LWP x+1. >> >> So the state we arrive with at x86_linux_nat_target::read_description is: >> ... >> (gdb) p inferior_ptid >> $1 = {m_pid = x, m_lwp = x+1, m_tid = 0} >> ... >> and consequently we probe 64/32-bitness from thread LWP x+1. >> >> [ Note that this is different from when gdb doesn't attach but instead >> launches the exec itself, in which case there's no stop_all_threads needed, >> and the probed thread is LWP x. ] > > Well, it's mostly because when running, there's only one thread to begin > with, and the io_uring threads are likely not there yet. > >> According to aforementioned remark, a better choice would have been the main >> thread, that is, LWP x. > > That doesn't tell the whole story though. Up to now, probing the > non-main thread was maybe a slightly strange thing to do, but it worked > because all threads were set up the same way for that matter. It worked > under the assumption that all threads had the same CS/DS contents (the > registers we use to detect architecture). The failure happens when > there are io_uring threads present. These threads are of a strange kind > because they are like any userspace threads, except that they never > return to userspace. Because of that, they don't have a meaningful > userspace register state, reading their registers returns garbage. So > if we choose one of these threads for probing the architecture of the > process (which, as you mentioned below, is bogus, but that's how it > works now), then we get bad results. > >>>From what I read (from the lkml thread you linked), the kernel's plan is > to mitigate it by filling the register state of these threads to > something GDB is happy with, even though that's unnecessary otherwise. > I don't know what's the state of that though. But I agree it would be > good to fix this on our side as well. > >> >> This patch implement that choice, by simply doing: >> ... >> tid = inferior_ptid.pid (); > > I think that's ok. > >> ... >> >> The fact that gdb makes a per-process choice for 64/32-bitness is a problem in >> itself: each thread can be in either 64 or 32 bit mode. That is a problem >> that this patch doesn't fix. > > Not only that, but from what I understood, it's not right to decide once > and for all whether a thread is 32 or 64. It could switch between the > two modes. So all we can really know is what mode a thread is > currently, while it is stopped. But who knows what mode it's going to > be in 5 instructions from now. > > This actually is a problem to people who debug early boot code with > QEMU, because there the "thread" goes from 32 to 64 bit mode at some > point. To handle this, it sounds like GDB should re-evaluate the > architecture of each thread every time it stops. That sounds expensive, > but maybe this mode could exist behind a knob that is disabled by > default, for these special cases. > > There might also be features that can only work if we assume the > architecture of a thread never changes, I'm not sure. > > Also, we already have kind of partial machinery for having threads with > different architectures, that's why we have > target_ops::thread_architecture. But read_description is used to set > inferior::gdbarch, which is kind of the "default" arch of the process, > used for many things. And the default implementation of > thread_architecture, process_stratum_target::thread_architecture, just > returns inf->gdbarch. So in practice, today, for Linux it's true that > we have one single architecture for the whole inferior. The exception > is aarch64-linux, which can return different architectures for different > SVE configurations. A while ago I suggested having a better mechanism to update the architecture of each thread mid-execution (in order to support SVE dynamic vector lenghts for gdbserver), including remote packet changes. But the feedback was that it would be too much overhead. In summary, assuming a per-process architecture is not right for SVE and it sounds like we should be moving towards per-thread architectures in the future. We'll need to dump per-thread GDB_TDESC notes for a core file as well.