From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 08D87385740E for ; Mon, 9 Aug 2021 15:54:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 08D87385740E Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 179Frlk3141904; Mon, 9 Aug 2021 11:54:21 -0400 Received: from ppma02wdc.us.ibm.com (aa.5b.37a9.ip4.static.sl-reverse.com [169.55.91.170]) by mx0a-001b2d01.pphosted.com with ESMTP id 3aam0m92g0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 09 Aug 2021 11:54:20 -0400 Received: from pps.filterd (ppma02wdc.us.ibm.com [127.0.0.1]) by ppma02wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 179FlFE6032033; Mon, 9 Aug 2021 15:54:19 GMT Received: from b03cxnp07029.gho.boulder.ibm.com (b03cxnp07029.gho.boulder.ibm.com [9.17.130.16]) by ppma02wdc.us.ibm.com with ESMTP id 3aapj8up09-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 09 Aug 2021 15:54:19 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 179FsImd38142400 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 9 Aug 2021 15:54:18 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E336E7806B; Mon, 9 Aug 2021 15:54:17 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8F16F7805C; Mon, 9 Aug 2021 15:54:17 +0000 (GMT) Received: from li-e362e14c-2378-11b2-a85c-87d605f3c641.ibm.com (unknown [9.211.107.50]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Mon, 9 Aug 2021 15:54:17 +0000 (GMT) Message-ID: From: Carl Love To: Andrew Burgess Cc: gdb-patches@sourceware.org, Rogerio Alves , Ulrich Weigand Date: Mon, 09 Aug 2021 08:54:16 -0700 In-Reply-To: <20210809094513.GE462163@embecosm.com> References: <4fd73f34dd7db949309de791ea5514890a0f9139.camel@us.ibm.com> <20210809094513.GE462163@embecosm.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-14.el8) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: Exox3xf5rosA4b9pM38iLUsSBb_t4lQV X-Proofpoint-ORIG-GUID: Exox3xf5rosA4b9pM38iLUsSBb_t4lQV Subject: RE: [PATCH] Improve forward progress test in gdb.python/python.exp X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.790 definitions=2021-08-09_05:2021-08-06, 2021-08-09 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 priorityscore=1501 clxscore=1015 mlxscore=0 bulkscore=0 adultscore=0 malwarescore=0 mlxlogscore=999 lowpriorityscore=0 impostorscore=0 phishscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2107140000 definitions=main-2108090113 X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP 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: Mon, 09 Aug 2021 15:54:24 -0000 > > 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. 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