From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 309753857C62 for ; Thu, 21 Jan 2021 08:29:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 309753857C62 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 4EE4EAD0B; Thu, 21 Jan 2021 08:29:36 +0000 (UTC) To: Simon Marchi , gdb-patches@sourceware.org References: <20210118103101.GA29963@delia> <3dcf9e40-5409-9015-b09b-56e9fcf0e752@polymtl.ca> From: Tom de Vries Subject: Re: [PATCH][gdb/testsuite] Fix gdb.python/py-finish-breakpoint2.exp with -m32 Message-ID: Date: Thu, 21 Jan 2021 09:29:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 In-Reply-To: <3dcf9e40-5409-9015-b09b-56e9fcf0e752@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_NUMSUBJECT, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 21 Jan 2021 08:29:39 -0000 On 1/21/21 8:04 AM, Simon Marchi wrote: > On 2021-01-18 5:31 a.m., Tom de Vries wrote: >> Hi, >> >> When running test-case gdb.python/py-finish-breakpoint2.exp with target board >> unix/-m32, we run into: >> ... >> (gdb) continue^M >> Continuing.^M >> Exception #10^M >> ^M >> Breakpoint 3, throw_exception_1 (e=10) at py-finish-breakpoint2.cc:23^M >> 23 throw new int (e);^M >> (gdb) FAIL: gdb.python/py-finish-breakpoint2.exp: \ >> check FinishBreakpoint in catch() >> ... >> With -m64, the test passes. >> >> Relevant bit of the source: >> ... >> 36 try >> 37 { >> 38 throw_exception_1 (10); >> 39 } >> 40 catch (const int *e) >> 41 { >> 42 std::cerr << "Exception #" << *e << std::endl; >> 43 } >> 44 i += 1; /* Break after exception 1. */ >> ... >> >> The -m64 scenario in more detail: >> - the test-case runs to throw_exception_1. >> - it installs a FinishBreakpoint, which is a temporary breakpoint set at the >> return address of a frame. >> - for -m64, that address is: >> 400c47: 83 45 e4 01 addl $0x1,-0x1c(%rbp) >> which corresponds the "i += 1 at line 44" >> - the test-case then continues >> - an exception is throw in throw_exection_1 >> - the exception is caught at line 40, and a message is printed >> - line 44 is executed, and the FinishBreakpoint triggers. >> >> With -m32, we have instead: >> - the address where the finish breakpoint is set is: >> 8048a0a: 83 c4 10 add $0x10,%esp >> which is the lasn insn generated for the call at line 38 >> - the test-case continues >> - an exception is throw in throw_exection_1 >> - consequently, the FinishBreakpoint is not triggered. >> >> In conclusion, the test worked by accident for -m64, because the first insn >> after the call to throw_exception_1 is also the first insn after the try. >> And that just happens to be not the case for -m32. >> >> Fix this by removing this part of the test. >> >> Tested on x86_64-linux. >> >> Any comments? >> >> Thanks, >> - Tom >> >> [gdb/testsuite] Fix gdb.python/py-finish-breakpoint2.exp with -m32 >> >> gdb/testsuite/ChangeLog: >> >> 2021-01-18 Tom de Vries >> >> * gdb.python/py-finish-breakpoint2.exp: Remove part of test that >> works by accident. >> >> --- >> gdb/testsuite/gdb.python/py-finish-breakpoint2.exp | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp b/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp >> index 10c4b6e81b8..4193f073996 100644 >> --- a/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp >> +++ b/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp >> @@ -46,12 +46,6 @@ gdb_test "source $pyfile" ".*Python script imported.*" \ >> >> gdb_breakpoint "throw_exception_1" >> gdb_test "continue" "Breakpoint .*throw_exception_1.*" "run to exception 1" >> - >> -gdb_test "python print (len(gdb.breakpoints()))" "3" "check BP count" >> -gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" "init ExceptionFinishBreakpoint" "set FinishBP after the exception" >> -gdb_test "continue" ".*stopped at ExceptionFinishBreakpoint.*" "check FinishBreakpoint in catch()" >> -gdb_test "python print (len(gdb.breakpoints()))" "3" "check finish BP removal" >> - >> gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to second exception" >> gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" "init ExceptionFinishBreakpoint" "set FinishBP after the exception" >> gdb_test "continue" ".*exception did not finish.*" "FinishBreakpoint with exception thrown not caught" >> > > Hmm I'd like to understand better the original intent of the test. > > Clearly, it tries to test two different things, because the first > time it expects to see the print from the "stop" method, and the > second time it expects to see the print from the "out_of_scope" > method. Agreed, the intent of the test is to test both scenarios, in the context of throwing an exception. > But I don't really understand what's different in both > cases. Both times it just runs to throw_exception_1 and sets > a FinishBreakpoint. The throw_exception_1 function just throws, > so presumably, we should expect the same outcome both times, > don't we? And that outcome should be that the out_of_scope > method gets called, right? > Well, the difference between the two scenarios is in the call stack at the point that we set the FinishBreakpoint: - in the first scenario, the call stack is: main -> throw_exception_1 - in the second scenario, the call stack is: main -> throw_exception -> throw_exception_1 In the second scenario, the FinishBreakpoint is set in the throw_exception function on the insn after the call to throw_exception_1. This guarantees that FinishBreakpoint.stop will not trigger, so FinishBreakpoint.out_of_scope will get called. In the first scenario, the FinishBreakpoint is set on the insn after the call to throw_exception_1 in main. For -m32, that happens to be an insn in the try clause, and FinishBreakpoint.stop will not trigger. For -m64, that happens to be the location after the try/catch, and FinishBreakpoint.stop will trigger. The different outcomes for -m32 and -m64 are both valid given the semantics of FinishBreakpoint, which is "set at the return address of a frame". It all depends where that return address is. There is no guarantee that the return address is an insn that uniquely represent the function return control path. > When I try to replicate the bit you removed by hand, I indeed > get two different things with -m32 and -m64, and both are not > the one I expect. With -m64, "stop", gets called: > > $ ./gdb -q -nx --data-directory=data-directory m64 -ex "b throw_exception_1" -ex r -x ~/src/binutils-gdb/gdb/testsuite/gdb.python/py-finish-breakpoint2.py -ex "python ExceptionFinishBreakpoint(gdb.newest_frame())" > Reading symbols from m64... > Breakpoint 1 at 0x11f7: file /home/simark/src/binutils-gdb/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc, line 23. > Starting program: /home/simark/build/binutils-gdb/gdb/m64 > > Breakpoint 1, throw_exception_1 (e=10) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc:23 > 23 throw new int (e); > Python script imported > init ExceptionFinishBreakpoint > (gdb) continue > Continuing. > Exception #10 > stopped at ExceptionFinishBreakpoint > Right, the FinishBreakpoint is set at the insn after the try/catch and the stop triggers. > And with -m32, nothing gets gets called, the FinishBreakpoint > just never hits (we just hit throw_exception_1 later): > Right, the FinishBreakpoint is set in the try clause and stop doesn't trigger. If we run to exit, we'll hit out_of_scope. > ./gdb -q -nx --data-directory=data-directory m32 -ex "b throw_exception_1" -ex r -x ~/src/binutils-gdb/gdb/testsuite/gdb.python/py-finish-breakpoint2.py -ex "python ExceptionFinishBreakpoint(gdb.newest_frame())" > Reading symbols from m32... > Breakpoint 1 at 0x1261: file /home/simark/src/binutils-gdb/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc, line 23. > Starting program: /home/simark/build/binutils-gdb/gdb/m32 > > Breakpoint 1, throw_exception_1 (e=10) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc:23 > 23 throw new int (e); > Python script imported > init ExceptionFinishBreakpoint > (gdb) continue > Continuing. > Exception #10 > > Breakpoint 1, throw_exception_1 (e=10) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc:23 > 23 throw new int (e); > (gdb) > > > Surely, there is some bug in there. Shouldn't we at least > see the same behavior for -m32 and -m64 for what I shown > above? Can you help me understand what is happening here? I don't see a bug here. I think different behaviour for -m32 and -m64 (and for other architectures) is possible. All I see here is a test that depends on a coincidence. Perhaps it helps to think of a Finish Breakpoint as a "Return address Breakpoint". Thanks, - Tom