From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85388 invoked by alias); 4 May 2016 16:24:30 -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 85375 invoked by uid 89); 4 May 2016 16:24:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=HX-Received:10.66.199.66 X-HELO: mail-pa0-f44.google.com Received: from mail-pa0-f44.google.com (HELO mail-pa0-f44.google.com) (209.85.220.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 04 May 2016 16:24:19 +0000 Received: by mail-pa0-f44.google.com with SMTP id bt5so25543379pac.3 for ; Wed, 04 May 2016 09:24:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=KJ33I/uBNiDZl5XLeGlVh4tvCitoYe77SV93XbsUDM0=; b=ETEsJ/luNcRjQaTJtxNa5Qt7fpGz0up/FjUrrHcjbHDFNh5n0uZ0BCbCBVX4VBMKWN fg4RVoipcb2w72lai0DSGGua5fUCnvqBgnOVJp9W/g5LXMnUNUazALXnLaQZoBKVqSds f3O+PwTEwp5svOfFpQGg7fxMis8QQIgkSVo+Mfnkd0FS+CMmEcLqhgOX2WY+opsxBUTq qpdt94Le/DHWZv8dOxqYeyfQ0jbbBsjHCD3E3ySb1DY57FORuOIeaTwUOHGKNMr8Oj0m W9+4d1Gj2tdyp5Niz8uwMumBZtH9TAXBXE5+ua4HxC9sxwLwGpuu0hMu4KUYRIeU6iiK YEwA== X-Gm-Message-State: AOPr4FXYw9zcD1rahTrWZMjlV0CsSNpfk36AzG0eIi4lTaiirbknWWH9XARWDitYFOzXTw== X-Received: by 10.66.199.66 with SMTP id ji2mr13337097pac.34.1462379057569; Wed, 04 May 2016 09:24:17 -0700 (PDT) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id z17sm7244003pfi.61.2016.05.04.09.24.13 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 04 May 2016 09:24:16 -0700 (PDT) From: Yao Qi To: Pedro Alves Cc: Yao Qi , Antoine Tremblay , gdb-patches@sourceware.org Subject: Re: [PATCH 1/4] Teach arm unwinders to terminate gracefully References: <1452188697-23870-1-git-send-email-antoine.tremblay@ericsson.com> <1452188697-23870-2-git-send-email-antoine.tremblay@ericsson.com> <86io1ung0a.fsf@gmail.com> <56CEE928.2080704@redhat.com> Date: Wed, 04 May 2016 16:24:00 -0000 In-Reply-To: <56CEE928.2080704@redhat.com> (Pedro Alves's message of "Thu, 25 Feb 2016 11:44:40 +0000") Message-ID: <86r3dhdc63.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-05/txt/msg00060.txt.bz2 Pedro Alves writes: Hi Pedro, After trying two different approaches of ultimate-fallback unwinder, and think again about 'wrapping methods of struct frame_unwind', I think 'wrapping struct frame_unwind methods' still works, maybe. > There are a few constraints that we need to keep in mind: > > - Frames that only have the PC available should have distinct frame ids, > and it should be distinct from outer_frame_id. (See frame_id_build_una= vailable_stack calls). > This can be done by wrapping fi->unwind->this_id with TRY/CATCH, call get_frame_pc_if_available, if PC is available, call frame_id_build_unavailable_stack otherwise, fall back to outer_frame_id. I did that in my patch below, > This makes e.g., the frame_id_eq check in tfind_1 work as intended, see: > https://sourceware.org/ml/gdb-patches/2013-12/msg00535.html > > - When an unwind sniffer throws, it'll destroy its > struct frame_unwind_cache. So if we don't catch the error, the > frame's this_id method can't return something more detailed than > outer_frame_id. unwinder->sniffer is wrapped by TRY/CATCH nowadays, so we don't have to change anything. > > I don't see this done by wrapping methods of 'struct frame_unwind'. > > I think it'd work to have an ultimate-fallback unwinder that > frame_unwind_find_by_frame returns instead of the internal error at > the end. This would return UNWIND_UNAVAILABLE or UNWIND_MEMORY_ERROR > in the unwinder->stop_reason method, depending on the error the last regi= stered > unwinder thrown. (That last unwinder will always be the arch's > heuristic unwinder.) In my patch, this_frame->unwind->stop_reason is wrapped by TRY/CATCH, and the stop_reason is set to UNWIND_UNAVAILABLE if error is NOT_AVAILABLE_ERROR. I don't set stop_reason to UNWIND_MEMORY_ERROR as you suggested, because I think it can be a follow-up improvement. Let us focus on unavailable things first. > And it would return frame_id_build_unavailable_stack(PC) in the unwinder-= >this_id > method if the last error was UNWIND_UNAVAILABLE, outer_frame_id otherwise > (or we add a new frame_id_build_stackless function, to go along with > frame_id_build_unavailable_stack). I think my patch can do this. The patch below is an RFC. Run gdb.trace/*.exp tests on both x86_64-linux and aarch64-linux. What do you think? This patch https://sourceware.org/ml/gdb-patches/2016-04/msg00429.html is applied locally to make sure x86 prologue unwinders are selected by gdb. --=20 Yao (=E9=BD=90=E5=B0=A7) diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 6f2e38e..52d89b7f 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -559,8 +559,7 @@ aarch64_make_prologue_cache (struct frame_info *this_fr= ame, void **this_cache) } CATCH (ex, RETURN_MASK_ERROR) { - if (ex.error !=3D NOT_AVAILABLE_ERROR) - throw_exception (ex); + throw_exception (ex); } END_CATCH =20 @@ -687,8 +686,7 @@ aarch64_make_stub_cache (struct frame_info *this_frame,= void **this_cache) } CATCH (ex, RETURN_MASK_ERROR) { - if (ex.error !=3D NOT_AVAILABLE_ERROR) - throw_exception (ex); + throw_exception (ex); } END_CATCH =20 diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 0065523..192d27b 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -2510,8 +2510,7 @@ amd64_frame_cache (struct frame_info *this_frame, voi= d **this_cache) } CATCH (ex, RETURN_MASK_ERROR) { - if (ex.error !=3D NOT_AVAILABLE_ERROR) - throw_exception (ex); + throw_exception (ex); } END_CATCH =20 @@ -2638,8 +2637,7 @@ amd64_sigtramp_frame_cache (struct frame_info *this_f= rame, void **this_cache) } CATCH (ex, RETURN_MASK_ERROR) { - if (ex.error !=3D NOT_AVAILABLE_ERROR) - throw_exception (ex); + throw_exception (ex); } END_CATCH =20 @@ -2819,8 +2817,7 @@ amd64_epilogue_frame_cache (struct frame_info *this_f= rame, void **this_cache) } CATCH (ex, RETURN_MASK_ERROR) { - if (ex.error !=3D NOT_AVAILABLE_ERROR) - throw_exception (ex); + throw_exception (ex); } END_CATCH =20 diff --git a/gdb/frame.c b/gdb/frame.c index d621dd7..60deca3 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -478,7 +478,26 @@ compute_frame_id (struct frame_info *fi) /* Find THIS frame's ID. */ /* Default to outermost if no ID is found. */ fi->this_id.value =3D outer_frame_id; - fi->unwind->this_id (fi, &fi->prologue_cache, &fi->this_id.value); + + TRY + { + fi->unwind->this_id (fi, &fi->prologue_cache, &fi->this_id.value); + } + CATCH (ex, RETURN_MASK_ERROR) + { + if (ex.error =3D=3D NOT_AVAILABLE_ERROR) + { + CORE_ADDR pc; + + /* Fall back to outer_frame_id if PC isn't available. */ + if (get_frame_pc_if_available (fi, &pc)) + fi->this_id.value =3D frame_id_build_unavailable_stack (pc); + } + else + throw_exception (ex); + } + END_CATCH + gdb_assert (frame_id_p (fi->this_id.value)); fi->this_id.p =3D 1; if (frame_debug) @@ -1882,9 +1901,20 @@ get_prev_frame_always_1 (struct frame_info *this_fra= me) =20 /* Check that this frame is unwindable. If it isn't, don't try to unwind to the prev frame. */ - this_frame->stop_reason - =3D this_frame->unwind->stop_reason (this_frame, - &this_frame->prologue_cache); + TRY + { + this_frame->stop_reason + =3D this_frame->unwind->stop_reason (this_frame, + &this_frame->prologue_cache); + } + CATCH (ex, RETURN_MASK_ERROR) + { + if (ex.error =3D=3D NOT_AVAILABLE_ERROR) + this_frame->stop_reason =3D UNWIND_UNAVAILABLE; + else + throw_exception (ex); + } + END_CATCH =20 if (this_frame->stop_reason !=3D UNWIND_NO_REASON) { diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 83a4881..f52eb0f 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -2074,8 +2074,7 @@ i386_frame_cache (struct frame_info *this_frame, void= **this_cache) } CATCH (ex, RETURN_MASK_ERROR) { - if (ex.error !=3D NOT_AVAILABLE_ERROR) - throw_exception (ex); + throw_exception (ex); } END_CATCH =20 @@ -2254,8 +2253,7 @@ i386_epilogue_frame_cache (struct frame_info *this_fr= ame, void **this_cache) } CATCH (ex, RETURN_MASK_ERROR) { - if (ex.error !=3D NOT_AVAILABLE_ERROR) - throw_exception (ex); + throw_exception (ex); } END_CATCH =20 @@ -2450,8 +2448,7 @@ i386_sigtramp_frame_cache (struct frame_info *this_fr= ame, void **this_cache) } CATCH (ex, RETURN_MASK_ERROR) { - if (ex.error !=3D NOT_AVAILABLE_ERROR) - throw_exception (ex); + throw_exception (ex); } END_CATCH =20