From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [63.128.21.74]) by sourceware.org (Postfix) with ESMTP id 1C28E385C426 for ; Tue, 24 Mar 2020 18:24:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1C28E385C426 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-38-RdETUGohOradIqp4DHo9KQ-1; Tue, 24 Mar 2020 14:23:54 -0400 X-MC-Unique: RdETUGohOradIqp4DHo9KQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 90CA4801E6D for ; Tue, 24 Mar 2020 18:23:53 +0000 (UTC) Received: from localhost (unused-10-15-17-54.yyz.redhat.com [10.15.17.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id 64F6D94974; Tue, 24 Mar 2020 18:23:53 +0000 (UTC) From: Sergio Durigan Junior To: Kevin Buettner Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 0/5] Improve ptrace-error detection Organization: Red Hat Canada References: <20200226200542.746617-1-sergiodj@redhat.com> <20200317154719.2078283-1-sergiodj@redhat.com> <20200319175319.3bb071e7@f31-4.lan> Date: Tue, 24 Mar 2020 14:23:53 -0400 Message-ID: <87blolbohy.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-19.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_2, RCVD_IN_DNSWL_NONE, 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: Tue, 24 Mar 2020 18:24:14 -0000 On Thursday, March 19 2020, Kevin Buettner wrote: > Hi Sergio, Hey Kevin, Thanks for the review. > I'm still reviewing this patch set, but noticed the following during test= ing: > > < PASS: gdb.base/attach-twice.exp: attach > --- >> XFAIL: gdb.base/attach-twice.exp: attach > > and: > > < PASS: gdb.base/attach.exp: do_attach_failure_tests: fail to attach agai= n > --- >> FAIL: gdb.base/attach.exp: do_attach_failure_tests: fail to attach again > > For gdb.base/attach-twice.exp, the relevant sections of the log files loo= k > like this: > > (gdb) spawn /mesquite2/sourceware-git/f31-ptrace-error-detection/bld/gdb/= testsuite/outputs/gdb.base/attach-twice/attach-twice > attach 1113400 > Attaching to program: /mesquite2/sourceware-git/f31-ptrace-error-detectio= n/bld/gdb/testsuite/outputs/gdb.base/attach-twice/attach-twice, process 111= 3400 > warning: process 1113400 is already traced by process 1113405 > ptrace: Operation not permitted. > (gdb) PASS: gdb.base/attach-twice.exp: attach > > Versus: > > (gdb) spawn /mesquite2/sourceware-git/f31-ptrace-error-detection/bld/gdb/= testsuite/outputs/gdb.base/attach-twice/attach-twice > attach 1113182 > Attaching to program: /mesquite2/sourceware-git/f31-ptrace-error-detectio= n/bld/gdb/testsuite/outputs/gdb.base/attach-twice/attach-twice, process 111= 3182 > warning: ptrace: Operation not permitted > process 1113182 is already traced by process 1113187 > There might be restrictions preventing ptrace from working. Please see > the appendix "Linux kernel ptrace restrictions" in the GDB documentation > for more details. > If you are debugging the inferior remotely, the ptrace restriction(s) mus= t > be disabled in the target system (e.g., where GDBserver is running). > (gdb) XFAIL: gdb.base/attach-twice.exp: attach Ah, good catch. I can reproduce these here (obviously), which makes me wonder why I missed them. I think I may have looked at them and thought they were racy. > It seems to me that this should still PASS; I think the regex for that > test simply needs to be updated. (You could also add another case.) Yeah. > I've also looked at the logs for gdb.base/attach.exp. The output is > similar to that shown above. Again, I think that the regex needs to > be updated. You know, I thought it was just going to be a matter of expanding the regexp in order to match the extra text, but then I started thinking if there was a better way to do this. I mean, in these two specific cases (attach.exp and attach-twice.exp) we *know* what is wrong: we're trying to attach twice to the same process. So GDB knows this is the problem, and when we print the whole "There might be restrictions preventing ptrace from working..." text, it can confuse the user. Looking at nat/linux-ptrace.c:linux_ptrace_attach_fail_reason, I see that my patch is currently doing: std::string linux_ptrace_attach_fail_reason (pid_t pid, int err) { std::string result =3D linux_ptrace_attach_fail_reason_1 (pid); std::string ptrace_restrict =3D linux_ptrace_restricted_fail_reason (er= r); if (!ptrace_restrict.empty ()) result +=3D "\n" + ptrace_restrict; IOW, it's always appending the result of "linux_ptrace_restricted_fail_reason" to the string that will be printed. Upon a closer inspection of "linux_ptrace_attach_fail_reason_1"'s comment, we see: /* Find all possible reasons we could fail to attach PID and return these as a string. An empty string is returned if we didn't find any reason. Helper for linux_ptrace_attach_fail_reason and linux_ptrace_attach_fail_reason_lwp. */ static std::string linux_ptrace_attach_fail_reason_1 (pid_t pid) IOW, if the string returned by it is not empty, it means that the function was able to determine the reason for the ptrace failure. After seeing this, I decided that the best approach is to call "linux_ptrace_restricted_fail_reason" only if "linux_ptrace_attach_fail_reason_1" returns an empty string. With this, the output generated when the user tries to attach twice to the same process is kept minimal and concise. It was still necessary to make a small adjustment in both testcases because the order of the warnings was reversed: we now first print the message saying that the process is already attached to GDB, and then print the ptrace strerror string. > If it's the case that the old output might still be produced by some > platforms, care must be taken to ensure that both cases (still) pass. Right. I believe that with the change I described above it won't be necessary to worry about arch-specific cases. Thanks, --=20 Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/