From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id TtHuHLhPemiifBcAWB0awg (envelope-from ) for ; Fri, 18 Jul 2025 09:44:24 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TPZ2TXwf; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 65F531E11C; Fri, 18 Jul 2025 09:44:24 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.8 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_SBL_CSS,RCVD_IN_VALIDITY_CERTIFIED, RCVD_IN_VALIDITY_RPBL,RCVD_IN_VALIDITY_SAFE autolearn=ham autolearn_force=no version=4.0.1 Received: from server2.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 ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 672911E089 for ; Fri, 18 Jul 2025 09:44:23 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EDA63385F022 for ; Fri, 18 Jul 2025 13:44:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EDA63385F022 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TPZ2TXwf Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id BDC99385E443 for ; Fri, 18 Jul 2025 13:43:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BDC99385E443 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BDC99385E443 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1752846226; cv=none; b=YBsybptR9BHWcRtIXcIwOu4IBJNmPVbMHB9g6EhUh0OSaMOAVHeKnn7z9qLXucDhB12mxXhZARk+sI0v6aTsftylr8tjQaoPxCzpzxnJ9YwO4gBGKRZDgy/7h9T6zQZcO+Pjx+Me7I8fXe7FrxHbCCyejL1Gu+zumG+8cI3fG7c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1752846226; c=relaxed/simple; bh=jydsyMINhP5Twvh7gJQHXov7qf9hCn0t3FlYOLedeSo=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=iYbjwDzYla12tpag2IJZLFV1iHWT/xnWnjMs36AMHhWcMDXU0WO6RNla2NtSFUsjltSWjX58LZAl/WCJVP5+Fng8sBhLjUhOtpo5pwxpncwcyQe+irK01lxDgwXds+SLg0/uW6tPOBybEG2+7W1CTzxpzCxnu1YMH21WUT5cJoU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BDC99385E443 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1752846226; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ssZ+24Hlc+vSJ1aIukjB86I8IF+PM9+1rYyjo36wmKM=; b=TPZ2TXwfCaHrGOwvA0F47gQclxtpq25xTp9ZcbEQxmGZhjo7y0/IJGe24+kvjGI27hh+Pi qgD7iGVzTe0o6RgOlE4t+ZL+E85iznLGUcQxlkR5pwQaIwDS834ylRRuabwWqC6E6AoRy5 mQIQZiiYVhp7EdJCeELlehx6Wz47uHI= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-685-Zfsqd37XN7GXe3MPKtkUBg-1; Fri, 18 Jul 2025 09:43:45 -0400 X-MC-Unique: Zfsqd37XN7GXe3MPKtkUBg-1 X-Mimecast-MFC-AGG-ID: Zfsqd37XN7GXe3MPKtkUBg_1752846224 Received: by mail-qt1-f197.google.com with SMTP id d75a77b69052e-4ab5310daedso47052091cf.0 for ; Fri, 18 Jul 2025 06:43:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752846224; x=1753451024; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ssZ+24Hlc+vSJ1aIukjB86I8IF+PM9+1rYyjo36wmKM=; b=hA7y8z7ydo2HEADIOtkXeibr2LmOlpoFy4/uvuO6mgTikkJdD7KTVKfJtfeNHJJ7WN LKKlJQ8m9ro3CF5h3riZER2P1j8S3nrEKFPMY3cuNjfUzCLJQIjUWop3h9ZfvxWKDU7z aVBOyGRrI0lhS9386PMImH2+3JkepIm9GLt/30E0VSzR/qXdqtRFKxOEVbhXKai1Ratr unBky8Hfpph//vpdnh4VryLY0gVEoYCket05TAPTVV0jePcFUpl3KdQbrR06SM7oCH6C dWtACdXHmoe54orH1Yo2ZLMgXHnTzln0DUQe7wshEQs38vy4hf92sgpV8cYQHwLWfd18 S/wg== X-Forwarded-Encrypted: i=1; AJvYcCVBuEnGECjVL+df3UUxq7Z3/Ck6yNIfB7LWyzyOAYzazdCkHZDRvDg3YbA83AriigmsDUE49oGHvD8rBw==@sourceware.org X-Gm-Message-State: AOJu0Yzgiuj3Oqh3IK+Pd7WRB6fNu5JlB+8QoQ3uOKLT5+s4J2Lzt6ZI Qioq6NxsLX9/Rw9JnoFlAsIU2xqtyJyWYVNou8pp5f723/Wv7prlgI56FuJRtZ1vm6Ijc+eToAX 9ksHpgG0/tyiqm3GRiT/cAOjLI9tBOo6MhzceWtBLmpiID9mPwXvQ5Io3sdF1vqk= X-Gm-Gg: ASbGncso8VWMChQ068eIFmolVL9Kf9k78kPIqS4qU8CalkhWFky/bNo2om9IqxFnesk +Owt40gO7nw7lkgGHSNpDixNw4l7bCkusZekQltunFpduT91W2IVhvqwlrAJTcU5S2HyaRjn42r EAaRWR2nDtoxPVRWc4nOTqND3NcVC+eRrG14osuA20LV2OW8urN2IFpw7bPpACSor4PI+9NPDsf KRLrGgmA1jckiLx3e3P8JrfjYaXyF5pz/LEaKhJll8zfYCgtQ9cxSOfIMmiSTi2h57/+i9I0I41 Ki2Ew5KvYUQecsiF0V6u8NOXxQ0vx4ewzhGb9uDnjoeBELmCgS4= X-Received: by 2002:a05:622a:1803:b0:4ab:536a:ad35 with SMTP id d75a77b69052e-4ab93d5cb07mr156963741cf.34.1752846224140; Fri, 18 Jul 2025 06:43:44 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHZHGuwDoSbm6MjyEWibIYpqdgK8Zhx+Byx+Y/LGgnqgQl9h6yeOUGOh3cwup5v6TBpuSxDBQ== X-Received: by 2002:a05:622a:1803:b0:4ab:536a:ad35 with SMTP id d75a77b69052e-4ab93d5cb07mr156963101cf.34.1752846223427; Fri, 18 Jul 2025 06:43:43 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:9a69::1000? ([2804:14d:8084:9a69::1000]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-4abb4b246b7sm7033731cf.57.2025.07.18.06.43.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 18 Jul 2025 06:43:42 -0700 (PDT) Message-ID: <91a1e38e-2978-4e81-a168-762ed55d3811@redhat.com> Date: Fri, 18 Jul 2025 10:43:39 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/3] gdb, amd64: extend the amd64 prologue analyzer to skip register pushes To: Pawel Kupczak , gdb-patches@sourceware.org References: <20250701104759.52595-1-pawel.kupczak@intel.com> <20250701104759.52595-2-pawel.kupczak@intel.com> From: Guinevere Larsen In-Reply-To: <20250701104759.52595-2-pawel.kupczak@intel.com> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: nkJL0iRk5tFBPb6pvQ-hqHNSYA0hms2B1uruvKD9etU_1752846224 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Hello! Thanks for working on this! I am not an authority on i386, but I did notice a few things that gave me pause so I'd like to ask them to make sure I understand the changes. Skipping the commit message: On 7/1/25 7:47 AM, Pawel Kupczak wrote: > --- > gdb/amd64-tdep.c | 53 +++++++++++- > .../amd64-extended-prologue-analysis.c | 49 +++++++++++ > .../amd64-extended-prologue-analysis.exp | 86 +++++++++++++++++++ > 3 files changed, 187 insertions(+), 1 deletion(-) > mode change 100644 => 100755 gdb/amd64-tdep.c > create mode 100644 gdb/testsuite/gdb.arch/amd64-extended-prologue-analysis.c > create mode 100644 gdb/testsuite/gdb.arch/amd64-extended-prologue-analysis.exp > > diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c > old mode 100644 > new mode 100755 > index 82dd1e07cf3..863b29a8c27 > --- a/gdb/amd64-tdep.c > +++ b/gdb/amd64-tdep.c > @@ -2553,6 +2553,56 @@ amd64_analyze_frame_setup (gdbarch *gdbarch, CORE_ADDR pc, > return pc; > } > > +/* Check whether PC points at code pushing registers onto the stack. If so, > + update CACHE and return pc after those pushes or CURRENT_PC, whichever is > + smaller. Otherwise, return PC passed to this function. */ > + > +static CORE_ADDR > +amd64_analyze_register_saves (CORE_ADDR pc, CORE_ADDR current_pc, > + amd64_frame_cache *cache) > +{ > + gdb_byte op; > + int offset = 0; > + > + /* There are at most 16 registers that would be pushed in the prologue. */ > + for (int i = 0; i < 16 && pc < current_pc; i++) > + { > + int reg = 0; > + int pc_offset = 0; > + > + if (target_read_code (pc, &op, 1) == -1) > + return pc; > + > + /* %r8 - %r15 prefix. */ > + if (op == 0x41) Looking over on the disassembler for record-full, I see that all 0x4- are considered prefixes, and (in 64 bit targets) they all have this effect. Is this something that could affect this prologue analyzer? ie, that some prefix like 0x40 is used to mean "push a register larger than 7", which would cause us to not skip that instruction? > + { > + reg += 8; > + pc_offset = 1; > + > + if (target_read_code (pc + 1, &op, 1) == -1) > + return pc; > + } > + > + /* push %rax|%rcx|%rdx|%rbx|%rsp|%rbp|%rsi|%rdi > + > + or with 0x41 prefix: > + push %r8|%r9|%r10|%r11|%r12|%r13|%r14|%r15. */ > + if (op < 0x50 || op > 0x57) > + break; This code also ignores the "push lz" and "push lb" instructions. Could they be used as prologue instructions? (again, this comes from the recording disassembler) If so, I would think this is worth explicity saying it in the comment at the top of the function, otherwise it could confuse us in the future. > + > + reg += op - 0x50; > + offset -= 8; > + > + int regnum = amd64_arch_reg_to_regnum (reg); > + cache->saved_regs[regnum] = offset; > + cache->sp_offset += 8; > + > + pc += 1 + pc_offset; > + } > + > + return pc; > +} > + > /* Do a limited analysis of the prologue at PC and update CACHE > accordingly. Bail out early if CURRENT_PC is reached. Return the > address where the analysis stopped. > @@ -2594,7 +2644,8 @@ amd64_analyze_prologue (gdbarch *gdbarch, CORE_ADDR pc, CORE_ADDR current_pc, > if (current_pc <= pc) > return current_pc; > > - return amd64_analyze_frame_setup (gdbarch, pc, current_pc, cache); > + pc = amd64_analyze_frame_setup (gdbarch, pc, current_pc, cache); > + return amd64_analyze_register_saves (pc, current_pc, cache); > } > > /* Work around false termination of prologue - GCC PR debug/48827. > diff --git a/gdb/testsuite/gdb.arch/amd64-extended-prologue-analysis.c b/gdb/testsuite/gdb.arch/amd64-extended-prologue-analysis.c > new file mode 100644 > index 00000000000..7d777d23236 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/amd64-extended-prologue-analysis.c > @@ -0,0 +1,49 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2025 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +int __attribute__ ((noinline)) > +bar (int x) > +{ > + return x + x; > +} > + > +/* This function should generate a prologue in shape of: > + push %rbp > + .cfi_def_cfa_offset 16 > + .cfi_offset %rbp, -16 > + mov %rsp, %rbp > + .cfi_def_cfa_register %rbp > + push %reg1 > + push %reg2 > + .cfi_offset %reg2, 32 > + .cfi_offset %reg1, 24 > + > + So to be able to unwind a register, GDB needs to skip prologue past > + register pushes (to access .cfi directives). */ In general, if possible, I think it's preferred that we actually use assembly instead of relying on compiler behavior. You should be able to use asm statements to create a fake function, and either the dwarf assembler to actually label it as such... Maybe even using global labels to do that more easily (not sure if GDB would use the prologue skip in that case, though). Checking the exp file, that is especially important because r12 is hardcoded but compilers don't promise to be at all consistent about which general purpose registers they'll use; and because you're relying on optimizations which can change drastically between compilers and versions. Case in point, this test will fail with clang. More below > +int __attribute__ ((noinline)) > +foo (int a, int b, int c, int d) > +{ > + a += bar (a) + bar (b) + bar (c); > + return a; > +} > + > +int > +main (int argc, char **argv) > +{ > + int a = foo (argc, argc + 1, argc + 2, argc * 2); > + return a; Clang seems to perform some tailcall optimization here, since GDB's backtrace looks like this: I also noticed that the test fails when using clang and getting a backtrace, with the following output: (gdb) bt #0  0x000055555555442b in foo () #1  0x00007ffff7cc45f5 in __libc_start_call_main () from /lib64/libc.so.6 #2  0x00007ffff7cc46a8 in __libc_start_main_impl () from /lib64/libc.so.6 #3  0x0000555555554345 in _start () And the disasembly of "main" looks like this: 0000000000000460
:  460:   55                      push   %rbp  461:   48 89 e5                mov    %rsp,%rbp  464:   8d 77 01                lea    0x1(%rdi),%esi  467:   8d 57 02                lea    0x2(%rdi),%edx  46a:   5d                      pop    %rbp  46b:   e9 b0 ff ff ff          jmp    420 So clang actually noticed that the 'a' variable isn't really necessary, and that the return of 'foo' is the return of main, so does a straight jump to foo. Maybe this can be solved by having 'a' be volatile, but I would still say that generating a .s file or a series of hand-written assembly instructions to test with is much more reliable. > +} (Skipping exp file, since I have no comments about it specifically) -- Cheers, Guinevere Larsen She/Her/Hers