From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 723E03858C54 for ; Fri, 21 Jul 2023 07:24:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 723E03858C54 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1689924276; 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=QtwVd6UJlXW1622fwrrjj+k7hmgd2SadMkJi7yu6il8=; b=JqIPtNR4OSw5JZvsdK/JoTUlw6PWm14vEkmairWqF2Lqp2p28KSqt63XNZs9OnqNn3dWOI 7zvFT6o85EmOcU3VlzO59XevPxCHL0bg1d4fMWrMzZLuUOkdANr4LMww8w+tG3ZkVOJNyW 3jEIRQzGDzZkgz8m4TBRJLW2xtmMtEw= Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-470-BxNZAXsmO82sRclOCO0HqQ-1; Fri, 21 Jul 2023 03:24:33 -0400 X-MC-Unique: BxNZAXsmO82sRclOCO0HqQ-1 Received: by mail-qt1-f200.google.com with SMTP id d75a77b69052e-3f9e556c7d8so22442691cf.0 for ; Fri, 21 Jul 2023 00:24:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689924272; x=1690529072; 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=QtwVd6UJlXW1622fwrrjj+k7hmgd2SadMkJi7yu6il8=; b=OnLGcKzg5AM2sErbpacuGUwuopV5vRO/inKYwRzdN4L/gwLTIoKWFrFySm8ITL/vzD kax9xE68vdbd8ElMI6zrMm0bM4pe2OAcRbaFMKHu3nqbx2QplpoeQ4ldM83726E73TI2 aS+2wnIB38bn+LxlhaoYcrhQicQIEPsOJbCxFmqmzaNhV1apvHrq4UaPng8ji3JSfoHE wvkAiyHQ1aY3JokuEGkWaqWTqhcGU3jAusDcQGc0+RSvYx7oCQkWAqvfLsBTjUBYbf9V yMOMSxcZnnyOOKh3aqE12odfjtSJhtM/jOoNPx3uFA6jaqyW5MnCPxGm0QugEEJg5qnY Htmw== X-Gm-Message-State: ABy/qLZozJN59chxPtHO2b9KozDzf3KNn0JvwrLcnrj92tZaiDzRwNOU ntUQsB2RrfQY9u5meH3XeCgHPcHSLxe9QluBRg0oDZ4DbOU+WmueorPTzqYqk6ZbOPjdF5kBQmk AkGaRRqudc0vVIJZSCuE9W5rpQJIV+w== X-Received: by 2002:ac8:5e4c:0:b0:405:37a8:7403 with SMTP id i12-20020ac85e4c000000b0040537a87403mr1178648qtx.6.1689924272549; Fri, 21 Jul 2023 00:24:32 -0700 (PDT) X-Google-Smtp-Source: APBJJlHS/DfyMbQkj5UKZdya7kNOyXl8Vw6FFwRSJfXQjXnLlYCqx4EJpE9xbHnHWw5uwlEQVxFXBw== X-Received: by 2002:ac8:5e4c:0:b0:405:37a8:7403 with SMTP id i12-20020ac85e4c000000b0040537a87403mr1178637qtx.6.1689924272260; Fri, 21 Jul 2023 00:24:32 -0700 (PDT) Received: from [192.168.0.129] (ip-94-112-225-44.bb.vodafone.cz. [94.112.225.44]) by smtp.gmail.com with ESMTPSA id e7-20020ac81307000000b004053d6d7a26sm1028736qtj.40.2023.07.21.00.24.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 21 Jul 2023 00:24:31 -0700 (PDT) Message-ID: <63d4dcba-923a-9a66-23f3-9135bb1b4521@redhat.com> Date: Fri, 21 Jul 2023 09:24:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH 2/2 ] PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp To: Carl Love , Simon Marchi , Tom de Vries , Ulrich Weigand , "gdb-patches@sourceware.org" , "pedro@palves.net" References: <71aa635593df0677811afb85409aa190bcfa4f6a.camel@us.ibm.com> <15864a6b87b25c93e99a28149f23138267735f2a.camel@us.ibm.com> <041f62e9f26fd4a536bc90c34f072985582e6237.camel@de.ibm.com> <46c2c756475ba5923d7eed97996632a08285dd42.camel@us.ibm.com> <65861786-069e-53a1-ca17-a525b6629c95@suse.de> <5be0c849abeef84d34a6ff255fb2705ca5dcb035.camel@us.ibm.com> <5e60a837-b21c-011f-c94e-e8bbf7645c5d@simark.ca> <7639de48695d52a806627b0a91979ad2e5fd9b42.camel@us.ibm.com> <9cf51eb9-c731-6f42-ab2b-a37048f25d12@simark.ca> <60c362e6dadd05754907af5f10e6f3c0423e1901.camel@us.ibm.com> <2a1d55cb-118b-d942-b315-a5f2348894f5@simark.ca> <0ab7af40f2f8d1edbceea851c37282a03c830567.camel@us.ibm.com> <90c98190-f8eb-1d64-43ce-8bab67d5caef@simark.ca> <6c58d4ce-d9ff-e1d9-398b-724859394f22@redhat.com> <38ab07a7a217bcc05be78c82d351aef73b411fb9.camel@us.ibm.com> From: Bruno Larsen In-Reply-To: <38ab07a7a217bcc05be78c82d351aef73b411fb9.camel@us.ibm.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_MANYTO,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 20/07/2023 16:45, Carl Love wrote: > Bruno: > > On Thu, 2023-07-20 at 14:01 +0200, Bruno Larsen wrote: >> (gdb) disas /s >> (snip) >> 81 b = 5; >> 0x00000000004011bb <+43>: movl $0x5,-0x18(%rbp) >> >> 82 >> 83 function1 (a, b); // CALL VIA LEP >> 0x00000000004011c2 <+50>: mov -0x14(%rbp),%edi >> 0x00000000004011c5 <+53>: mov -0x18(%rbp),%esi >> => 0x00000000004011c8 <+56>: call 0x401140 >> (gdb) rn >> 81 b = 5; >> >> Basically, the best way to fix this solution is to get reverse-next >> and >> reverse-step to properly figure out the stepping range and not have >> a >> reverse-next that ends up in the same line > --------------------- > > Per my reply to Simon in "Re: [EXTERNAL] Re: [PATCH 2/2 v5] Fix reverse > stepping multiple contiguous PC ranges over the line table." on > 6/23/2023. Where he was wondering why control.step_range_start was not > set to the "real" range.... > > > This is the code in the Patch: >>> +{ >>> + /* The line table may have multiple entries for the same source >>> code line. >>> + Given the PC, check the line table and return the PC that >>> corresponds >>> + to the line table entry for the source line that PC is >>> in. */ >>> + CORE_ADDR start_line_pc = ecs->event_thread- >>>> control.step_range_start; >>> + gdb::optional real_range_start; >>> + >>> + /* Call find_line_range_start to get the smallest address in the >>> + linetable for multiple Line X entries in the line table. */ >>> + real_range_start = find_line_range_start (pc); >>> + >>> + if (real_range_start.has_value ()) >>> + start_line_pc = *real_range_start; >>> + >>> + return start_line_pc; > > Simon's comment about the code: > >> When I read this, I wonder: why was control.step_range_start not set >> to >> the "real" range start in the first place (not only in the context of >> reverse execution, every time it is set)? It would seem more robust >> than patching it afterwards in some very specific spots. >> >> I could see some benefits for range-stepping uses cases too (relevant >> when debugging remotely). Using your example here: >> >> Line X - [0x0 - 0x8] >> Line X - [0x8 - 0x10] >> Line X - [0x10 - 0x18] >> >> Imagine we are stopped at 0x14, and we type "next", and 0x14 is a >> conditional jump to 0x5. It seems like current GDB would send a >> "range >> step" request to GDBserver, to step in the [0x10, 0x18[ range. When >> reaching 0x5, execution would stop, and GDB would resume it again >> with >> the [0x0,0x8[ range. When reaching 0x8, it would stop again, GDB >> would >> resume it with [0x8,0x10[, and so on. If GDB could send a "range >> step" >> request with the [0x0,0x18[ range, it would avoid those unnecessary >> intermediary stop. >> > My reply to Simon's comment: > > We looked at trying to set control.step_range_start to the real start > range initially. Ulrich brought this point up in our internal review > of the patch. > > So, when I am in function finish_backward() in infcmd.c I have no way > to determine what the previous PC was. If I assume it was the previous > value, i.e. pc - 4byes (on PowerPC). I get a gdb internal error. It > seems that I am not allowed to change the line range to something that > does not include the current pc value. > > ../../binutils-gdb-reverse-multiple-contiguous/gdb/infrun.c:2740: > internal-error: resume_1: > Assertion `pc_in_thread_step_range (pc, tp)' failed. > > In order to make that work, we concluded that it would probably entail > a much bigger change to how reverse execution works which would be > beyond the scope of what this patch is trying to fix. So, being able > to do what I believe you want to do is in theory possible but it would > require a larger, independent change to what this patch is trying to > fix. > > --------------------------------------- > > I think what Bruno is again asking is to have control.step_range_start > set to the real start range initially, i.e. what Simon asked about. I > think to do that, we would need to make significant changes to how > reverse execution works to allow us to make that change. It didn't > appear to be a straight forward fix to me. I may be wrong. Maybe > someone sees a good way to make that work that I am missing. So it > looks like this patch fixes most issues but not all of the problems > with reverse-step and reverse-next. It might be good to try and fix > this additional scenario in a separate patch, not sure??? I just tested with the newest version of the patch relating to reverse stepping over contiguous lines, but that didn't make a difference. GCC continued broken. I think the problem is that GCC doesn't have contiguous ranges, it has a single range that doesn't contain the PC My (probably naive) thinking is to set step_range_end to the starting PC of the following line, and step_range_start to be the real start of the current line at the moment the command is parsed. This sounds like it would solve the GCC problem, but I assume I'm missing something. Unfortunately, I don't have the time to test this theory myself, but I'll happily test any patches you submit :). If you want to repeat my test, I want the behavior of gdb.reverse/finish-reverse-next from gcc to match clang's (on x86 machines), that is, if you reverse finish from function1, a single "rn" command would land you on the b=5 line. -- Cheers, Bruno