From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103374 invoked by alias); 22 Jan 2020 13:30:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 103221 invoked by uid 89); 22 Jan 2020 13:30:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=pcs, stopped, HX-Received:a0c X-HELO: mail-qv1-f66.google.com Received: from mail-qv1-f66.google.com (HELO mail-qv1-f66.google.com) (209.85.219.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 Jan 2020 13:30:39 +0000 Received: by mail-qv1-f66.google.com with SMTP id dp13so3189230qvb.7 for ; Wed, 22 Jan 2020 05:30:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:from:to:cc:references:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=vkwst2Sfa7EUvYXqqxt+KWFxs0p8cNV4oSzGBOFUPfw=; b=v0MmX+NOsayMc/l0fIqfMy1NcB9tnXKAbjV3rYm15oH5BoOYwUQETjK1r6oXMScqLj TU85t7AY+nj6kOMudoADugZP1feE0n4/OGdSqcI0vr7mSyI2I73jDFO0Y7hbrm47qqEe tDXocQ+cd79O2kWXy9GgR/qV0OCShqf3HpnNKp01Cz90FwK5LejMzoflQRQyXnh48WPK xSEfbiYo2Kn3RTvKSyJFN5xJaQHcv7jzSYPpsfYhutNRyAv5W76TurS2rDbbTubvS2SC DcMVImsChjw5ou5GMmAmYffdLcck4hYWVCHG1y3DjvwvmA3n0ZvL5jVMKe5ye9Wklzed iBgA== Return-Path: Received: from [192.168.0.185] ([177.158.84.16]) by smtp.gmail.com with ESMTPSA id f42sm21123063qta.0.2020.01.22.05.30.31 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Jan 2020 05:30:33 -0800 (PST) Subject: [PING] [PATCH] Harden gdb.base/step-over-syscall.exp From: Luis Machado To: gdb-patches@sourceware.org Cc: alan.hayward@arm.com References: <20200115203645.26360-1-luis.machado@linaro.org> Message-ID: Date: Wed, 22 Jan 2020 13:39:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200115203645.26360-1-luis.machado@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg00671.txt.bz2 ping? https://sourceware.org/ml/gdb-patches/2020-01/msg00432.html On 1/15/20 5:36 PM, Luis Machado wrote: > There are a couple problems with this test. > > First > -- > > gdb.base/step-over-syscall.exp records the address of a syscall instruction > within fork/vfork/clone functions and also the address of the instruction > after that syscall instruction. > > It uses these couples addresses to make sure we stepped over a syscall > instruction (fork/vfork/clone events) correctly. > > The way the test fetches the addresses of the instructions is by stepi-ing > its way through the fork/vfork/clone functions until it finds a match for > a syscall. Then it stepi's once again to get the address of the next > instruction. > > This assumes that stepi-ing over a syscall is working correctly and landing > in the right PC. This is not the case for AArch64/Linux, where we're > landing a couple instructions after the syscall in some cases. > > The following patch lets the test execute as before, but adds a new instruction > address check using the x command as opposed to stepi. > > I didn't want to change how the test works since we may also be > interested in checking if stepi-ing over the syscall under different > conditions (displaced stepping on/off) yields the same results. I don't > feel strongly about this, so i'm OK with changing how we compare PC's for > the entire test if folks decide it is reasonable. > > Second > -- > > FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: continue to vfork (3rd time) (the program exited) > FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: continue to syscall insn vfork (the program is no longer running) > FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: single step over vfork (the program is no longer running) > > Depending on the glibc version we may have different code generated for the > fork/vfork/clone functions. > > I ran into the situation where vfork for newer glibc's on AArch64/Linux is > very short, so "break vfork" will put a breakpoint right at the syscall > instruction, which is something the testcase isn't expecting (a off-by-1 > of sorts). > > The patch adds extra code to handle this case. If the test detects we're > already sitting at a syscall instruction, it records the address and moves > on to record the address after that particular instruction. > > Another measure is to "break *$syscall" instead of "break $syscall". That > guarantees we're stopping at the first instruction of the syscall function, > if it ever happens that the syscall instruction is the first instruction of > those functions. > > With these changes i can fix some failures for aarch64-linux-gnu and also > expose the problems i've reported here: > > https://sourceware.org/ml/gdb-patches/2019-12/msg01071.html > > These tests now fail for aarch64-linux-gnu (patch for this is going through > reviews): > > FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: pc after stepi matches insn addr after syscall > FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=on: pc after stepi matches insn addr after syscall > > I've queued a test run on the buildbot to make sure things are sane for other > architectures, and i've tested it on aarch64-linux-gnu. > > gdb/testsuite/ChangeLog: > > 2020-01-15 Luis Machado > > * gdb.base/step-over-syscall.exp (setup): Check if we're already > sitting at a syscall instruction when we hit the syscall function's > breakpoint. > Check PC against one obtained with the x command. > (step_over_syscall): Don't continue to the syscall instruction if > we're already there. > --- > gdb/testsuite/gdb.base/step-over-syscall.exp | 91 ++++++++++++++------ > 1 file changed, 66 insertions(+), 25 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp > index b373c169c0..4d9488b1d4 100644 > --- a/gdb/testsuite/gdb.base/step-over-syscall.exp > +++ b/gdb/testsuite/gdb.base/step-over-syscall.exp > @@ -46,7 +46,8 @@ proc_with_prefix check_pc_after_cross_syscall { syscall syscall_insn_next_addr } > > proc setup { syscall } { > global gdb_prompt syscall_insn > - > + global hex > + set next_insn_addr 0 > set testfile "step-over-$syscall" > > clean_restart $testfile > @@ -62,7 +63,7 @@ proc setup { syscall } { > gdb_test_no_output "set displaced-stepping off" \ > "set displaced-stepping off during test setup" > > - gdb_test "break $syscall" "Breakpoint \[0-9\]* at .*" > + gdb_test "break \*$syscall" "Breakpoint \[0-9\]* at .*" > > gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, (.* in |__libc_|)$syscall \\(\\).*" \ > "continue to $syscall (1st time)" > @@ -75,37 +76,72 @@ proc setup { syscall } { > # Hit the breakpoint on $syscall for the second time. In this time, > # the address of syscall insn and next insn of syscall are recorded. > > - gdb_test "display/i \$pc" ".*" > - > - # Single step until we see a syscall insn or we reach the > - # upper bound of loop iterations. > - set msg "find syscall insn in $syscall" > - set steps 0 > - set max_steps 1000 > - gdb_test_multiple "stepi" $msg { > - -re ".*$syscall_insn.*$gdb_prompt $" { > - pass $msg > + # Check if the first instruction we stopped at is the syscall one. > + set syscall_insn_addr 0 > + set test "fetch first stop pc" > + gdb_test_multiple "display/i \$pc" $test { > + -re "display/i .*: x/i .*=> ($hex) .*:.*$syscall_insn.*$gdb_prompt $" { > + set syscall_insn_addr $expect_out(1,string) > + pass $test > } > - -re "x/i .*=>.*\r\n$gdb_prompt $" { > - incr steps > - if {$steps == $max_steps} { > - fail $msg > - } else { > - send_gdb "stepi\n" > - exp_continue > + -re "display/i.*" { > + pass $test > + } > + } > + > + # If we are not at the syscall instruction yet, keep looking for it with > + # stepi commands. > + if {$syscall_insn_addr == 0} { > + # Single step until we see a syscall insn or we reach the > + # upper bound of loop iterations. > + set msg "find syscall insn in $syscall" > + set steps 0 > + set max_steps 1000 > + gdb_test_multiple "stepi" $msg { > + -re ".*$syscall_insn.*$gdb_prompt $" { > + pass $msg > + } > + -re "x/i .*=>.*\r\n$gdb_prompt $" { > + incr steps > + if {$steps == $max_steps} { > + fail $msg > + } else { > + send_gdb "stepi\n" > + exp_continue > + } > } > } > + > + if {$steps == $max_steps} { > + return { -1, -1 } > + } > + > + set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0" \ > + "pc before stepi"] > } > > - if {$steps == $max_steps} { > - return { -1, -1 } > + # We have found the syscall instruction. Now record the next instruction. > + # Use the X command instead of stepi since we can't guarantee > + # stepi is working properly. > + set test "pc after syscall instruction" > + gdb_test_multiple "x/2i \$pc" $test { > + -re "x/2i .*=> $hex .*:.*$syscall_insn.* ($hex) .*:.*$gdb_prompt $" { > + set next_insn_addr $expect_out(2,string) > + pass $test > + } > } > > - set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0" \ > - "pc before stepi"] > if {[gdb_test "stepi" "x/i .*=>.*" "stepi $syscall insn"] != 0} { > return { -1, -1 } > } > + > + set pc_after_stepi [get_hexadecimal_valueof "\$pc" "0" \ > + "pc after stepi with x command"] > + > + if {$next_insn_addr != $pc_after_stepi} { > + fail "pc after stepi matches insn addr after syscall" > + } > + > return [list $syscall_insn_addr [get_hexadecimal_valueof "\$pc" \ > "0" "pc after stepi"]] > } > @@ -156,8 +192,13 @@ proc step_over_syscall { syscall } { > } > } > > - gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \ > - "continue to syscall insn $syscall" > + # Check if the syscall breakpoint is at the syscall instruction > + # address. If so, no need to continue, otherwise we will run the > + # inferior to completion. > + if {$syscall_insn_addr != [get_hexadecimal_valueof "\$pc" "0"]} { > + gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \ > + "continue to syscall insn $syscall" > + } > > gdb_test_no_output "set displaced-stepping $displaced" > >