From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) by sourceware.org (Postfix) with ESMTPS id A95923857831 for ; Wed, 4 May 2022 13:44:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A95923857831 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f45.google.com with SMTP id r1-20020a1c2b01000000b00394398c5d51so864579wmr.2 for ; Wed, 04 May 2022 06:44:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:from:to:references:in-reply-to :content-transfer-encoding; bh=nzYYW7XTNKBCdDfFJfmrR38ylhGrADvkSNO1Z9XSt+M=; b=2fABkxMNag0q2XM5sfVFX5P0wn9IDhphxYscSc8nSOXOyB0mSzcvrysNKL693vQHev D9rNQ+F+cnzzsQR3ZzMv6DHvCRJjyTA+zOUp4TiU1G7vkKOMCSm5LWVBgCTyFAB2pe03 DvrO6t9Yn25etUwsh++arR+L94bfGFReo8eF81b3Y5+HQwXOQEERyJeuAQfCC4uLWiDH WgzKi4xFYxom02N4L1nEvaN+0Aie0iDK4zXTPA41SbhITWhCUk+UaPIBJq2qvmNpjR0t 6tQZ1TobhRV0gZvg+2luU0RJyrGSE3JiA2lgODKXUTYg3r8iXr8N1YsP8CYPQsVj0ahX XohA== X-Gm-Message-State: AOAM530DmjVeO9uEpc6TLlF83cv6kbtzRVhlO1Uzmv6F9+uLgvtppfG+ tl2+/vNtOKuF4b53lBBC11I= X-Google-Smtp-Source: ABdhPJy1iF76udW71oIynPlKFkqLKsMKLPHER/4PgbGO8haksFkvOLSzaeRuVtXEnvWrWtRWpBGguA== X-Received: by 2002:a05:600c:25d2:b0:394:2db5:bc32 with SMTP id 18-20020a05600c25d200b003942db5bc32mr7811432wml.39.1651671879557; Wed, 04 May 2022 06:44:39 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id d21-20020a05600c34d500b003942a244ecasm3949422wmq.15.2022.05.04.06.44.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 May 2022 06:44:38 -0700 (PDT) Message-ID: <7244be2f-3a26-6f44-0c47-8d86865abfa8@palves.net> Date: Wed, 4 May 2022 14:44:37 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: Re: [PATCH 0/2] Fix gdbserver/linux memory access regression Content-Language: en-US From: Pedro Alves To: Luis Machado , gdb-patches@sourceware.org References: <20220419224739.3029868-1-pedro@palves.net> <26ee78d5-d9ff-3ec3-5767-c6ae8cd5afa0@palves.net> <082d3a0a-f6a4-0e40-4e27-623a9949186c@arm.com> <51c7d9e9-7d84-f826-be2d-be559847da9b@palves.net> <48db3b2b-46e3-1f30-2443-7d4b406b4c46@arm.com> <1b41b921-b79b-6168-96b1-58b9dea5508f@palves.net> In-Reply-To: <1b41b921-b79b-6168-96b1-58b9dea5508f@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Wed, 04 May 2022 13:44:42 -0000 On 2022-05-04 11:14, Pedro Alves wrote: > On 2022-05-04 10:52, Luis Machado wrote: >> On 5/4/22 10:45, Pedro Alves wrote: > >>> Can you show a backtrace?  If this is when reading memory, what code cares whether it's 64-bit?  Reading memory >>> out of /proc/pid/mem should not care about that. >> >> Here it is: >> >> #0  thread_regcache_data (thread=thread@entry=0x0) at ../../../repos/binutils-gdb/gdbserver/inferiors.cc:120 >> #1  0x0000aaaaaaabf0e8 in get_thread_regcache (thread=0x0, fetch=fetch@entry=0) at ../../../repos/binutils-gdb/gdbserver/regcache.cc:31 >> #2  0x0000aaaaaaad785c in is_64bit_tdesc () at ../../../repos/binutils-gdb/gdbserver/linux-aarch64-low.cc:194 >> #3  0x0000aaaaaaad8a48 in aarch64_target::sw_breakpoint_from_kind (this=, kind=4, size=0xffffffffef04) at ../../../repos/binutils-gdb/gdbserver/linux-aarch64-low.cc:3226 >> #4  0x0000aaaaaaabe220 in bp_size (bp=0xaaaaaab6f3d0) at ../../../repos/binutils-gdb/gdbserver/mem-break.cc:226 >> #5  check_mem_read (mem_addr=187649984471104, buf=buf@entry=0xaaaaaab625d0 "\006", mem_len=mem_len@entry=56) at ../../../repos/binutils-gdb/gdbserver/mem-break.cc:1862 >> #6  0x0000aaaaaaacc660 in read_inferior_memory (memaddr=, myaddr=0xaaaaaab625d0 "\006", len=56) at ../../../repos/binutils-gdb/gdbserver/target.cc:93 >> #7  0x0000aaaaaaac3d9c in gdb_read_memory (len=56, myaddr=0xaaaaaab625d0 "\006", memaddr=187649984471104) at ../../../repos/binutils-gdb/gdbserver/server.cc:1071 >> #8  gdb_read_memory (memaddr=187649984471104, myaddr=0xaaaaaab625d0 "\006", len=56) at ../../../repos/binutils-gdb/gdbserver/server.cc:1048 >> #9  0x0000aaaaaaac82a4 in process_serial_event () at ../../../repos/binutils-gdb/gdbserver/server.cc:4307 >> #10 handle_serial_event (err=, client_data=) at ../../../repos/binutils-gdb/gdbserver/server.cc:4520 >> #11 0x0000aaaaaaafbcd0 in gdb_wait_for_event (block=block@entry=1) at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:700 >> #12 0x0000aaaaaaafc0b0 in gdb_wait_for_event (block=1) at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:596 >> #13 gdb_do_one_event () at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:237 >> #14 0x0000aaaaaaacacb0 in start_event_loop () at ../../../repos/binutils-gdb/gdbserver/server.cc:3518 >> #15 captured_main (argc=4, argv=) at ../../../repos/binutils-gdb/gdbserver/server.cc:3998 >> #16 0x0000aaaaaaab66dc in main (argc=, argv=) at ../../../repos/binutils-gdb/gdbserver/server.cc:4084 >> >> -- >> >> This sequence of functions is invoked due to a series of conditions: >> >> 1 - The probe-based breakpoint mechanism failed (for some reason) so ... >> 2 - ... gdbserver has to know what type of architecture it is dealing with so it can pick the right breakpoint kind, so it wants to check if we have a 64-bit target >> 3 - To determine the size of a register, we need to fetch the register cache, and we do so through a thread point, which is now nullptr. >> > > Thanks. I believe the patch below should fix that particular instance. > Luis confirmed this looks good to him on IRC, so I've merged it, as below. >From 5890af36e5112bcbb8d7555e63570f68466e6944 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 4 May 2022 11:09:07 +0100 Subject: [PATCH] Fix GDBserver Aarch64 Linux regression Luis noticed that the recent changes to gdbserver to make it track process and threads independently regressed a few gdb.multi/*.exp tests for aarch64-linux. We started seeing the following internal error for gdb.multi/multi-target-continue.exp for example: Starting program: binutils-gdb/gdb/testsuite/outputs/gdb.multi/multi-target-continue/multi-target-continue ^M Error in re-setting breakpoint 2: Remote connection closed^M ../../../repos/binutils-gdb/gdb/thread.c:85: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed.^M A problem internal to GDB has been detected,^M further debugging may prove unreliable. A backtrace looks like: #0 thread_regcache_data (thread=thread@entry=0x0) at ../../../repos/binutils-gdb/gdbserver/inferiors.cc:120 #1 0x0000aaaaaaabf0e8 in get_thread_regcache (thread=0x0, fetch=fetch@entry=0) at ../../../repos/binutils-gdb/gdbserver/regcache.cc:31 #2 0x0000aaaaaaad785c in is_64bit_tdesc () at ../../../repos/binutils-gdb/gdbserver/linux-aarch64-low.cc:194 #3 0x0000aaaaaaad8a48 in aarch64_target::sw_breakpoint_from_kind (this=, kind=4, size=0xffffffffef04) at ../../../repos/binutils-gdb/gdbserver/linux-aarch64-low.cc:3226 #4 0x0000aaaaaaabe220 in bp_size (bp=0xaaaaaab6f3d0) at ../../../repos/binutils-gdb/gdbserver/mem-break.cc:226 #5 check_mem_read (mem_addr=187649984471104, buf=buf@entry=0xaaaaaab625d0 "\006", mem_len=mem_len@entry=56) at ../../../repos/binutils-gdb/gdbserver/mem-break.cc:1862 #6 0x0000aaaaaaacc660 in read_inferior_memory (memaddr=, myaddr=0xaaaaaab625d0 "\006", len=56) at ../../../repos/binutils-gdb/gdbserver/target.cc:93 #7 0x0000aaaaaaac3d9c in gdb_read_memory (len=56, myaddr=0xaaaaaab625d0 "\006", memaddr=187649984471104) at ../../../repos/binutils-gdb/gdbserver/server.cc:1071 #8 gdb_read_memory (memaddr=187649984471104, myaddr=0xaaaaaab625d0 "\006", len=56) at ../../../repos/binutils-gdb/gdbserver/server.cc:1048 #9 0x0000aaaaaaac82a4 in process_serial_event () at ../../../repos/binutils-gdb/gdbserver/server.cc:4307 #10 handle_serial_event (err=, client_data=) at ../../../repos/binutils-gdb/gdbserver/server.cc:4520 #11 0x0000aaaaaaafbcd0 in gdb_wait_for_event (block=block@entry=1) at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:700 #12 0x0000aaaaaaafc0b0 in gdb_wait_for_event (block=1) at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:596 #13 gdb_do_one_event () at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:237 #14 0x0000aaaaaaacacb0 in start_event_loop () at ../../../repos/binutils-gdb/gdbserver/server.cc:3518 #15 captured_main (argc=4, argv=) at ../../../repos/binutils-gdb/gdbserver/server.cc:3998 #16 0x0000aaaaaaab66dc in main (argc=, argv=) at ../../../repos/binutils-gdb/gdbserver/server.cc:4084 This sequence of functions is invoked due to a series of conditions: 1 - The probe-based breakpoint mechanism failed (for some reason) so ... 2 - ... gdbserver has to know what type of architecture it is dealing with so it can pick the right breakpoint kind, so it wants to check if we have a 64-bit target. 3 - To determine the size of a register, we currently fetch the current thread's register cache, and the current thread pointer is now nullptr. In #3, the current thread is nullptr because gdb_read_memory clears it on purpose, via set_desired_process, exactly to expose code relying on the current thread when it shouldn't. It was always possible to end up in this situation (when the current thread exits), but it was harder to reproduce before. This commit fixes it by tweaking is_64bit_tdesc to look at the current process's tdesc instead of the current thread's tdesc. Note that the thread's tdesc is itself filled from the process's tdesc, so this should be equivalent: struct regcache * get_thread_regcache (struct thread_info *thread, int fetch) { struct regcache *regcache; regcache = thread_regcache_data (thread); ... if (regcache == NULL) { struct process_info *proc = get_thread_process (thread); gdb_assert (proc->tdesc != NULL); regcache = new_register_cache (proc->tdesc); set_thread_regcache_data (thread, regcache); } ... Change-Id: Ibc809d7345e70a2f058b522bdc5cdbdca97e2cdc --- gdbserver/linux-aarch64-low.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc index c924821c25c..d1e7acb7b4a 100644 --- a/gdbserver/linux-aarch64-low.cc +++ b/gdbserver/linux-aarch64-low.cc @@ -191,9 +191,9 @@ struct arch_process_info static int is_64bit_tdesc (void) { - struct regcache *regcache = get_thread_regcache (current_thread, 0); - - return register_size (regcache->tdesc, 0) == 8; + /* We may not have a current thread at this point, so go straight to + the process's target description. */ + return register_size (current_process ()->tdesc) == 8; } static void base-commit: 901e4e8d5c4f1d32c50e4a31b228bc7ef4210775 -- 2.36.0