From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id ZlPQH8f19WGEPAAAWB0awg (envelope-from ) for ; Sat, 29 Jan 2022 21:19:51 -0500 Received: by simark.ca (Postfix, from userid 112) id 7067F1F3B4; Sat, 29 Jan 2022 21:19:51 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A,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 C08D41ECEB for ; Sat, 29 Jan 2022 21:19:50 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 93E383858404 for ; Sun, 30 Jan 2022 02:19:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 93E383858404 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1643509189; bh=+zUfuDYWt8s3Gb86o0UIwnSdnj+Ww8chM+Fbga/nuOo=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=hVwybMBuIQ34pZkDNUjkbytXLOakDPu2YEnVKWroTAeRr4ijjs0hqCEhKcOfdoA6Y HZh6HDHJGyaqvhTWU1XCQzJSUPKDUSqF8NcGszDJf9zqqNTjAH/A6OF4jmVxRhc8bf Mb0ylFenrK9r5UePnpjd1NkLg0kNWJ66CQmKvTdU= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 7386E3858D28 for ; Sun, 30 Jan 2022 02:17:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7386E3858D28 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 20U2HiuC005639 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 29 Jan 2022 21:17:49 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 20U2HiuC005639 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 113731ECEB; Sat, 29 Jan 2022 21:17:43 -0500 (EST) Message-ID: <4f2da361-5a93-ee17-9bb5-e13653fc02d6@polymtl.ca> Date: Sat, 29 Jan 2022 21:17:43 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v4] gdb/tui/disassembly view: make symbol name appear on a line of its own Content-Language: en-US To: Andrew Burgess References: <20220124192811.1530670-1-simon.marchi@polymtl.ca> <20220128224140.GF425591@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 30 Jan 2022 02:17:44 +0000 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: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Cc: Vasili Burdo , Simon Marchi , gdb-patches@sourceware.org Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 2022-01-28 20:20, Simon Marchi via Gdb-patches wrote: >> This default `false` concerned my initially. The only times we call >> tui_disassemble is either because we want to redraw the screen >> contents, or we want to know what _would_ be drawn to the screen >> if/when we do the redraw. >> >> Only, now, we draw the screen differently for these two cases, so, my >> thinking goes, surely there's going to be some edge case where we ask, >> what address would be on the screen if .... and we'll get the wrong >> answer back. >> >> I played with this for a while, but couldn't get anything obvious to >> break - I suspect that if there are bugs, they are going to be super >> subtle, which addresses appear on the screen doesn't change much, >> usually just one instruction different I think, so maybe it doesn't >> matter. >> >> And given I couldn't spot anything, maybe I'm over thinking this, and >> there is no problem... >> >> I guess my question is, did you already consider this already? Is >> there a reason why having two strategies is known to be OK? > > No, I haven't considered this, it is a good question. I really don't > know the TUI code well (if at all), so my thinking was that if the TUI > experts say it's ok, it's because it's ok :). But indeed, it would be > good to understand exactly what happens here. > > I'll git a little bit. Vasili, if you happen to know why we have these > two behaviors (for_ui and !for_ui), feel free to answer. Ok, so it in fact "breaks" page up / down, in the sense that the number of lines scrolled up and down is not right. This makes sense, as the other uses of tui_disassemble are to help with scrolling. It is used to answer the question: assuming that we are currently showing disassembly starting at this PC, what would be the starting PC if going up/down N lines? Without this patch, let's say that my asm windows shows: │ 0x110c <__do_global_dtors_aux+76> nopl 0x0(%rax) │ │ 0x1110 endbr64 │ │ 0x1114 jmp 0x1080 │ │ 0x1119
push %rbp │ │ 0x111a mov %rsp,%rbp │ │ 0x111d movq $0x0,-0x8(%rbp) │ │ 0x1125 mov -0x8(%rbp),%rax │ │ 0x1129 mov (%rax),%eax │ │ 0x112b pop %rbp │ │ 0x112c ret │ Doing page down, the goal (at least my understanding of it) is that the last shown line becomes the first one. The result is: │ 0x112c ret │ │ 0x112d nopl (%rax) │ │ 0x1130 <__libc_csu_init> endbr64 │ │ 0x1134 <__libc_csu_init+4> push %r15 │ │ 0x1136 <__libc_csu_init+6> lea 0x2ceb(%rip),%r15 # 0x3e28 │ │ 0x113d <__libc_csu_init+13> push %r14 │ │ 0x113f <__libc_csu_init+15> mov %rdx,%r14 │ │ 0x1142 <__libc_csu_init+18> push %r13 │ │ 0x1144 <__libc_csu_init+20> mov %rsi,%r13 │ │ 0x1147 <__libc_csu_init+23> push %r12 │ With this patch applied, with the same starting position (0x110c being the first instruction shown), I have: │ __do_global_dtors_aux: │ │ 0x110c <+76> nopl 0x0(%rax) │ │ frame_dummy: │ │ 0x1110 <+0> endbr64 │ │ 0x1114 <+4> jmp 0x1080 │ │ main: │ │ 0x1119 <+0> push %rbp │ │ 0x111a <+1> mov %rsp,%rbp │ │ 0x111d <+4> movq $0x0,-0x8(%rbp) │ │ 0x1125 <+12> mov -0x8(%rbp),%rax │ After page down: │ main: │ │ 0x112c <+19> ret │ │ nopl (%rax) │ │ __libc_csu_init: │ │ 0x1130 <+0> endbr64 │ │ 0x1134 <+4> push %r15 │ │ 0x1136 <+6> lea 0x2ceb(%rip),%r15 # 0x3e28 │ │ 0x113d <+13> push %r14 │ │ 0x113f <+15> mov %rdx,%r14 │ │ 0x1142 <+18> push %r13 │ Since 0x1125 is the last instruction shown before page down, it should be the first one after page down. But it is 0x11c2 - the same as we would have gotten before the patch. It's clear now: since the code to compute where we land after a page down passes for_ui == false, that computation is done using the lines of the old layout, so the new starting address is the same, 0x112c. But since the layout shown in the UI is different, that means we completely miss instructions 0x1129 and 0x112b. So it is now obvious that the for_ui flag is wrong, we need to do the scrolling computation considering the new layout. Simon