From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id AD8DB3870886 for ; Wed, 16 Mar 2022 14:03:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AD8DB3870886 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 22GE3Tx5031412 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 16 Mar 2022 10:03:34 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 22GE3Tx5031412 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id C45201EDF0; Wed, 16 Mar 2022 10:03:28 -0400 (EDT) Message-ID: <289d26f2-3101-7117-5a69-70939b2509d2@polymtl.ca> Date: Wed, 16 Mar 2022 10:03:26 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH 2/3] gdb: testsuite: add new gdb_attach to fix attach-pie-noexec.exp Content-Language: en-US To: Tiezhu Yang , gdb-patches@sourceware.org References: <1647424709-23680-1-git-send-email-yangtiezhu@loongson.cn> <1647424709-23680-3-git-send-email-yangtiezhu@loongson.cn> From: Simon Marchi In-Reply-To: <1647424709-23680-3-git-send-email-yangtiezhu@loongson.cn> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 16 Mar 2022 14:03:29 +0000 X-Spam-Status: No, score=-3038.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: Wed, 16 Mar 2022 14:03:45 -0000 On 2022-03-16 05:58, Tiezhu Yang wrote: > This commit adds gdb_attach to check various output of "attach" command, > and then use gdb_attach to fix the following issue. > > If /proc/sys/kernel/yama/ptrace_scope is 1, when execute the following > command without superuser: > > make check-gdb TESTS="gdb.base/attach-pie-noexec.exp" > > we can see the following messages in gdb/testsuite/gdb.log: > > (gdb) attach 6500 > Attaching to process 6500 > ptrace: Operation not permitted. > (gdb) PASS: gdb.base/attach-pie-noexec.exp: attach > > It is obviously wrong, the expected result should be UNSUPPORTED in such > a case. > > With this patch, we can see "Operation not permitted" in the log info, > and then we can do the following processes to test: > (1) set ptrace_scope as 0 > $ echo 0 | sudo tee /proc/sys/kernel/yama/ptrace_scope > $ make check-gdb TESTS="gdb.base/attach-pie-noexec.exp" > (2) use sudo > $ sudo make check-gdb TESTS="gdb.base/attach-pie-noexec.exp" > > Signed-off-by: Tiezhu Yang > --- > gdb/testsuite/gdb.base/attach-pie-noexec.exp | 4 +++- > gdb/testsuite/lib/gdb.exp | 15 +++++++++++++++ > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/gdb/testsuite/gdb.base/attach-pie-noexec.exp b/gdb/testsuite/gdb.base/attach-pie-noexec.exp > index 4712824..9a1bafe 100644 > --- a/gdb/testsuite/gdb.base/attach-pie-noexec.exp > +++ b/gdb/testsuite/gdb.base/attach-pie-noexec.exp > @@ -59,7 +59,9 @@ set testpid [spawn_id_get_pid $test_spawn_id] > > gdb_start > file delete -- $binfile > -gdb_test "attach $testpid" "Attaching to process $testpid\r\n.*" "attach" > +if {![gdb_attach $testpid]} { > + return 0 > +} > gdb_test "set architecture $arch" "The target architecture is set to \"$arch\"\\." > gdb_test "info shared" "From\[ \t\]+To\[ \t\]+Syms Read\[ \t\]+Shared Object Library\r\n0x.*" > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 8fafcc0..599e8cc 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -5102,6 +5102,21 @@ proc can_spawn_for_attach { } { > return 1 > } > > +# Check various output of "attach" command. Please document the possible return values and what they mean. > + > +proc gdb_attach { testpid } { > + gdb_test_multiple "attach $testpid" "gdb attach" { I think the test name can be just "attach" (we know it's gdb). Although I presume that we will eventually want to make the test name customizable. "attach" can still be the default though. > + -re -wrap "Attaching to process $testpid\r\n.*No executable file now.*" { The "No executable file now" message is just one particular message that can be printed. And it's kind of specific to the test you are updated, where the executable is deleted before attaching. If there was actually an executable to be found, the message would be different. I would suggest trying to catch the known error cases with some "-re -wrap" clauses, and then have a catch-all clause that assumes things went fine: gdb_test_multiple "attach $testpid" "attach" { -re -wrap "ptrace: Operation not permitted\\." { unsupported "$gdb_test_name (Operation not permitted)" return 0 } -re -wrap "" { pass $gdb_test_name return 1 } } Note that the "process $testpid" part is target-specific, so it's better if we don't match it. I don't see the need to match the "Attaching to" part either, although it wouldn't do any harm. As we find other ways attach can fail, we can add more patterns. It's hard to write it the other way around, because attach doesn't really give an "OK, attach worked!" message that we can match. I think that's fine... if gdb_attach reports that the attach worked even though it didn't, the followings checks in the test calling gdb_attach will likely fail themselves, so the problem will not go unnoticed. Simon