From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by sourceware.org (Postfix) with ESMTPS id 84262398242D for ; Wed, 11 Aug 2021 15:58:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 84262398242D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wm1-x329.google.com with SMTP id m36-20020a05600c3b24b02902e67543e17aso4094579wms.0 for ; Wed, 11 Aug 2021 08:58:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Y2/LIvvpxMjjVneNp+74r10nNCgntbcGC5LODokVQZk=; b=bSuU2VIBVIVrmPDq0P9DF2CiHFqls26biIagSOk6g5tvmrCIexCQOdHmhx8VoOFRtQ RPPnwkROHtRwnHVEyYctQkIJROBamEAX5AVi04aUANwE5MY3i1GVb4oXdpc2exn/hVXo oh4fGgVgSYovE2PIbx08FDhkf1Kq3Mj+R2ieN/E0VEscay422+9hZ8vLRnF57X194a8Z K8KSfc0RhWZwtqonP2+/s61yAduC93vDfTGcoVCCHiUsybm7ZJFP5vc4YxLUgMm4wIka 4eKhI2hmKkFkK0GwMp7BxAEyBGEpS8E0usPWsRXBoxxCyJUyFMXopo/rrVzSvBARB8pW Uykg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Y2/LIvvpxMjjVneNp+74r10nNCgntbcGC5LODokVQZk=; b=izWfQjHVZLoxjVaUfavrBYoHYtbihJOphtw5IT+v2STGQ1P+CjVjNDo5pjqlfJZ/He q9DjSqM+zQgL07jwJZBx43f9hbsVoCi8lKqwlrVURzZxJHW27DCwl3uq3d1HamusIA+2 BW412EEN+Yp9kN9BwSijAmH9mQd5EJ0zkyHWMopo8cmRPCqsmqXsvtEbs4mZLyz3fRnA 3wyBHmqMwnKLvZ3nHojcHfHsKfrkMm1s84cxN16jeGyqiOJH0loHMT8nMforIioh4fvG yU8Ome31GoFz31Aw2D+15TLBgUl2HgNZXJeYITejwAk7JRZkhckPc+V5GktjbSiWyWjL NZDQ== X-Gm-Message-State: AOAM530SzIDkcqPvCIAgyD6f6VkQqvDrlf4CE98h+LwDK5D7hhL5oqv4 rXasoBef2kF716OJWxDg3aiZ7A== X-Google-Smtp-Source: ABdhPJzomzFPBouJdVZkJyHQpbBUb8hORur97w8dSBL/tjigCJoU9s8qx/c8alW0Ke1rVI9Ws3KJLg== X-Received: by 2002:a1c:23c7:: with SMTP id j190mr1630117wmj.165.1628697496578; Wed, 11 Aug 2021 08:58:16 -0700 (PDT) Received: from localhost (host86-161-16-194.range86-161.btcentralplus.com. [86.161.16.194]) by smtp.gmail.com with ESMTPSA id r8sm16494393wrj.11.2021.08.11.08.58.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Aug 2021 08:58:16 -0700 (PDT) Date: Wed, 11 Aug 2021 16:58:14 +0100 From: Andrew Burgess To: Carl Love Cc: gdb-patches@sourceware.org, Rogerio Alves , Ulrich Weigand Subject: Re: [PATCH] Improve forward progress test in gdb.python/python.exp Message-ID: <20210811155814.GH462163@embecosm.com> References: <4fd73f34dd7db949309de791ea5514890a0f9139.camel@us.ibm.com> <20210809094513.GE462163@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 16:55:10 up 9 days, 21:35, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no 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, 11 Aug 2021 15:58:28 -0000 * Carl Love [2021-08-09 08:54:16 -0700]: > > > > gdb_py_test_silent_cmd "python line = gdb.selected_frame().find_sal().line" "Get line number of func2 call site" 1 > > > + > > > +gdb_py_test_silent_cmd "python pc_call = gdb.selected_frame().pc()" "Get pc at return from func2 location" 1 > > > > I find the name for this test a little strange. I'd have expected > > "Get pc of func2 call site" in order to match the previous test. > > OK, I am fine with that it makes it more consistent > > > > > + > > > gdb_test "python print (gdb.find_pc_line(gdb.selected_frame().pc()).line == line)" "True" "test find_pc_line at func2 call site" > > > > > > gdb_py_test_silent_cmd "step" "Step into func2" 1 > > > gdb_py_test_silent_cmd "up" "Step out of func2" 1 > > > > > > -gdb_test "python print (gdb.find_pc_line(gdb.selected_frame().pc()).line > line)" "True" "test find_pc_line with resume address" > > > +gdb_py_test_silent_cmd "python pc_rtn = gdb.selected_frame().pc()" "Get pc at func2 return site" 1 > > > > I wonder if we can simplify the logic here? We always expect the pc > > to be bigger, right? So we can just test that in all cases. Once we > > know that is being tested, we can make the line number test simpler, > > like this: > > > > gdb_py_test_silent_cmd "python pc_rtn = gdb.selected_frame().pc()" \ > > "Get pc at func2 return site" 1 > > > > gdb_test "python print (pc_rtn > pc_call)" "True" \ > > "test resume address greater then call address" > > > > gdb_test "python print (gdb.find_pc_line(pc_rtn).line >= line)" "True" \ > > "test find_pc_line with resume address" > > > > I wrapped the lines to keep them under 80 characters, which is the > > correct style where possible. > > > > What do you think? > > > The above test sequence adds the PC check but doesn't change the line > test. So Powerpc will still fail the line test and thus we haven't > fixed the existing failure on Powerpc. It did change, previously the line check was '>', now I'm suggesting we use '>='. In your original email you wrote: > The current gdb.python/python.exp test fails on powerpc. The test > checks to see if the source code line for the function call is less > than the line number after the function call. The issue on powerpc is > the assembly instructions for the branch and the NOP following the > branch map to the same source code line. You suggest that the NOP is mapped to the _same_ source line, so I would expect the new test to pass, even on ppc. If the NOP isn't mapped to the same source line, then what is it mapped too? And you need to update your commit message. Thanks, Andrew > > My proposed patch checks the line numbers. If the line number check > "gdb.find_pc_line(pc_rtn).line >= line" is true the test passes. If > the line test fails, as it does on Powerpc, the pc test is then done to > decide if the test ultimately passes or fails. The result is that > even though the line check failed, the test passes on Powerpc because > we were able to show that the test made forward progress. The failure > on Powerpc is fixed. > > I agree that we should always expect the PC to be bigger, i.e. the test > made forward progress. My first thought was to just remove the line > test and go with the new PC test instead. My concern was I can't test > that change on all architectures. So I figured it best to leave the > line test as that presumably is OK on all other architectures. Just in > the case it fails (i.e. on Powerpc), do the additional PC check to make > a final determination if the test passes or fails. > > If everyone agrees that the PC test will work on all architectures, the > test could be changed to just do the PC check instead of the line > check. This would make the whole test simpler. Thoughts? > > Carl >