Message ID | 1459888769-18875-1-git-send-email-donb@codesourcery.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 36385 invoked by alias); 5 Apr 2016 20:39:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 36365 invoked by uid 89); 5 Apr 2016 20:39:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=vaue X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 05 Apr 2016 20:39:32 +0000 Received: from svr-orw-fem-02x.mgc.mentorg.com ([147.34.96.206] helo=SVR-ORW-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1anXl4-0001Nr-On from Don_Breazeal@mentor.com ; Tue, 05 Apr 2016 13:39:30 -0700 Received: from build4-lucid-cs (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.3.224.2; Tue, 5 Apr 2016 13:39:30 -0700 Received: by build4-lucid-cs (Postfix, from userid 1905) id C0B7141304; Tue, 5 Apr 2016 13:39:29 -0700 (PDT) From: Don Breazeal <donb@codesourcery.com> To: <gdb-patches@sourceware.org>, <qiyaoltc@gmail.com>, <palves@redhat.com> Subject: Re: [PATCH] Eliminate -var-create error for optzd ptr to struct Date: Tue, 5 Apr 2016 13:39:29 -0700 Message-ID: <1459888769-18875-1-git-send-email-donb@codesourcery.com> In-Reply-To: <57040B42.1040608@redhat.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes |
Commit Message
Don Breazeal
April 5, 2016, 8:39 p.m. UTC
On 4/5/2016 12:00 PM, Pedro Alves wrote: > On 04/05/2016 07:50 PM, Don Breazeal wrote: >> Hi Yao, >> >> On 4/5/2016 5:52 AM, Yao Qi wrote: >>> Don Breazeal <donb@codesourcery.com> writes: >> >>>> + CATCH (ex, RETURN_MASK_ERROR) >>>> + { >>>> + /* If we get an error, assume the value is not optimized out. */ >>>> + return 0; >>> >>> Why don't we fall back to checking value->optimized_out below? Some >>> bits/pieces of value are optimized out, but reading the rest of >>> bits/piece may trigger the memory error. In this case, the value is >>> optimized out too. We can do this... >>> >>> TRY >>> { >>> value_fetch_lazy (value); >>> } >>> CATCH (ex, RETURN_MASK_ERROR) >>> { >>> /* Fall back to checking value->optimized_out. */ >>> } >>> END_CATCH >>> >>> What do you think? >> >> Of course, that makes more sense, thanks. >> >>> Note that, after this patch, value_optimized_out will no longer throw >>> exceptions, some TRY/CATCH in value_optimized_out's callers can be >>> removed, such as gdbscm_value_optimized_out_p and >>> valpy_get_is_optimized_out. This can be done in a follow-up patch. >> >> I've done a more thorough audit of the call sites for value_optimized_out >> than I did previously, and the only places where an enclosing TRY/CATCH can >> be removed are the two you name. There is one other place where it is >> called inside a TRY/CATCH, but there are other functions that could throw >> errors called there as well. >> >> I will create a follow-up patch. >> >> I changed this patch as you suggest above, as well as changing >> RETURN_MASK_ERROR to RETURN_MASK_ALL. The TRY/CATCH blocks that are >> going to be removed use RETURN_MASK_ALL, and I thought that this patch >> should maintain the same level of coverage. > > Please don't. A RETURN_MASK_ALL swallows Ctrl-C/QUIT, and that's almost > always a bug. The cases you mention translate a QUIT to a python/scheme > exception, which is not the same as just swallowing the exception. Patch below changes that back. Pedro, thanks for clarifying. --Don ---- This patch eliminates an error thrown when accessing the value of a pointer to a structure where the pointer has been optimized out and 'set print object' is 'on'. The error shows up as the rather ugly value of the pointer variable in Eclipse. If 'set print object' is 'on', GDB tries to determine the actual (derived) type of the object rather than the declared type, which requires dereferencing the pointer, which in this cases throws an error because the pointer has been optimized out. The fix is to simply ignore the 'print object on' setting for pointers or references to structures when they have been optimized out. This means we just get the declared type instead of the actual type, because in this case that's the best that we can do. To implement the fix, value_optimized_out was modified so that it no longer throws an error when it fails to fetch the specified value. Instead, it just checks value->optimized_out. If we can't definitively say that the value is optimized out, then we assume it is not. I'm working on setting things in motion for a patch to Eclipse that recognizes optimized-out pointer-to-struct in this scenario and prevents any subsequent attempt to dereference it from that end. Tested on bare-metal powerpc board with Linux x86 host and Linux native x86_64. gdb/ChangeLog: 2016-04-05 Don Breazeal <donb@codesourcery.com> * value.c (value_actual_type): Don't try to get rtti type of the value if it has been optimized out. (value_optimized_out): If a memory access error occurs, just check vaue->optimized_out. --- gdb/value.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
Comments
Don Breazeal <donb@codesourcery.com> writes: >> Please don't. A RETURN_MASK_ALL swallows Ctrl-C/QUIT, and that's almost >> always a bug. The cases you mention translate a QUIT to a python/scheme >> exception, which is not the same as just swallowing the exception. > > Patch below changes that back. Pedro, thanks for clarifying. > No, it doesn't. > @@ -1433,7 +1434,17 @@ value_optimized_out (struct value *value) > /* We can only know if a value is optimized out once we have tried to > fetch it. */ > if (VEC_empty (range_s, value->optimized_out) && value->lazy) > - value_fetch_lazy (value); > + { > + TRY > + { > + value_fetch_lazy (value); > + } > + CATCH (ex, RETURN_MASK_ALL) It should be RETURN_MASK_ERROR. > + { > + /* Fall back to checking value->optimized_out. */ > + } > + END_CATCH > + } Otherwise, patch is good to me.
On 4/6/2016 2:04 AM, Yao Qi wrote: > Don Breazeal <donb@codesourcery.com> writes: > >>> Please don't. A RETURN_MASK_ALL swallows Ctrl-C/QUIT, and that's almost >>> always a bug. The cases you mention translate a QUIT to a python/scheme >>> exception, which is not the same as just swallowing the exception. >> >> Patch below changes that back. Pedro, thanks for clarifying. >> > > No, it doesn't. > >> @@ -1433,7 +1434,17 @@ value_optimized_out (struct value *value) >> /* We can only know if a value is optimized out once we have tried to >> fetch it. */ >> if (VEC_empty (range_s, value->optimized_out) && value->lazy) >> - value_fetch_lazy (value); >> + { >> + TRY >> + { >> + value_fetch_lazy (value); >> + } >> + CATCH (ex, RETURN_MASK_ALL) > > It should be RETURN_MASK_ERROR. Sorry about that. /me shakes head at self for goofing up the easy stuff. > >> + { >> + /* Fall back to checking value->optimized_out. */ >> + } >> + END_CATCH >> + } > > Otherwise, patch is good to me. > Thanks Yao. This is now corrected and pushed in. Question: in light of Pedro's comments regarding gdbscm_value_optimized_out_p and valpy_get_is_optimized_out: > A RETURN_MASK_ALL swallows Ctrl-C/QUIT, and that's almost > always a bug. The cases you mention translate a QUIT to > a python/scheme exception, which is not the same as just > swallowing the exception. Would you still like for me to follow up with a patch to remove the TRY/CATCH blocks around the calls to value_optimized_out in those two functions? Or do we want to leave it as-is so that the QUIT handling remains unchanged? Thanks --Don
On 04/06/2016 10:41 PM, Don Breazeal wrote: > On 4/6/2016 2:04 AM, Yao Qi wrote: > Question: in light of Pedro's comments regarding > gdbscm_value_optimized_out_p and valpy_get_is_optimized_out: > >> A RETURN_MASK_ALL swallows Ctrl-C/QUIT, and that's almost >> always a bug. The cases you mention translate a QUIT to >> a python/scheme exception, which is not the same as just >> swallowing the exception. > > Would you still like for me to follow up with a patch to remove the > TRY/CATCH blocks around the calls to value_optimized_out in those two > functions? Or do we want to leave it as-is so that the QUIT handling > remains unchanged? Please leave them as is. We need to translate gdb exceptions to Python/Scheme exceptions at the gdb <-> extension-language boundaries. Those gdb functions in question will be returning to the Python/Scheme runtimes, which are not prepared to unwind correctly on a gdb exception. Thanks, Pedro Alves
On 06/04/16 22:41, Don Breazeal wrote: > Would you still like for me to follow up with a patch to remove the > TRY/CATCH blocks around the calls to value_optimized_out in those two > functions? Or do we want to leave it as-is so that the QUIT handling > remains unchanged? No, I was wrong on that point. Please leave it as-is.
diff --git a/gdb/value.c b/gdb/value.c index 8268b08..8a210e7 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -1205,7 +1205,8 @@ value_actual_type (struct value *value, int resolve_simple_types, if ((TYPE_CODE (result) == TYPE_CODE_PTR || TYPE_CODE (result) == TYPE_CODE_REF) && TYPE_CODE (check_typedef (TYPE_TARGET_TYPE (result))) - == TYPE_CODE_STRUCT) + == TYPE_CODE_STRUCT + && !value_optimized_out (value)) { struct type *real_type; @@ -1433,7 +1434,17 @@ value_optimized_out (struct value *value) /* We can only know if a value is optimized out once we have tried to fetch it. */ if (VEC_empty (range_s, value->optimized_out) && value->lazy) - value_fetch_lazy (value); + { + TRY + { + value_fetch_lazy (value); + } + CATCH (ex, RETURN_MASK_ALL) + { + /* Fall back to checking value->optimized_out. */ + } + END_CATCH + } return !VEC_empty (range_s, value->optimized_out); }