From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23716 invoked by alias); 4 Jul 2013 17:08:31 -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 23705 invoked by uid 89); 4 Jul 2013 17:08:30 -0000 X-Spam-SWARE-Status: No, score=-7.5 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_XZ autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 04 Jul 2013 17:08:29 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r64H8RmS032715 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 4 Jul 2013 13:08:27 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r64H7MXd027173; Thu, 4 Jul 2013 13:08:27 -0400 Message-ID: <51D5ABC7.5020507@redhat.com> Date: Thu, 04 Jul 2013 17:08:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Andrew Burgess CC: "gdb-patches@sourceware.org" Subject: Re: [3/3] [PATCH] value_optimized_out and value_fetch_lazy References: <51B5A95F.7090400@broadcom.com> <51C1D347.3020906@redhat.com> <51D1C52A.1070603@broadcom.com> <51D4822B.2010504@redhat.com> <51D5A58C.8040305@broadcom.com> In-Reply-To: <51D5A58C.8040305@broadcom.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-07/txt/msg00176.txt.bz2 On 07/04/2013 05:40 PM, Andrew Burgess wrote: > On 03/07/2013 8:57 PM, Pedro Alves wrote: >> On 07/01/2013 07:06 PM, Andrew Burgess wrote: >>> On 19/06/2013 4:50 PM, Pedro Alves wrote: >>>> On 06/10/2013 11:24 AM, Andrew Burgess wrote: >>>> >>>>> I ran into an issue with gdb that appears to be caused by an incorrect >>>>> use of value_optimized_out. >>>>> >>>> >>>> I'm finding the patch a bit hard to read though. Could you >>>> split it up? >>> >>> This tiny patch notices that when we mark a value as optimized out >>> we can also mark the value as no longer lazy. >>> This patch is not required, but felt like a good thing to me, not >>> sure if everyone will agree though. >>> >>> Should I apply? >>> >>> Thanks >>> Andrew >>> >>> >>> >>> >>> gdb/ChangeLog >>> >>> 2013-07-01 Andrew Burgess >>> >>> * value.c (set_value_optimized_out): A value that is optimized out >>> is no longer lazy. >> >> Hmm. >> >> I tried going a step further. If the value is optimized >> out, then we don't really need its contents buffer. >> >> That is: >> >> gdb/value.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/gdb/value.c b/gdb/value.c >> index 970fb66..50f8889 100644 >> --- a/gdb/value.c >> +++ b/gdb/value.c >> @@ -690,6 +690,8 @@ allocate_value_lazy (struct type *type) >> void >> allocate_value_contents (struct value *val) >> { >> + gdb_assert (!val->optimized_out); >> + >> if (!val->contents) >> val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type)); >> } >> @@ -1064,6 +1066,13 @@ void >> set_value_optimized_out (struct value *value, int val) >> { >> value->optimized_out = val; >> + >> + if (val) >> + { >> + set_value_lazy (value, 0); >> + xfree (value->contents); >> + value->contents = NULL; >> + } >> } >> >> and then ran the testsuite. Sure enough, that catches a gdb crash. >> In gdb.dwarf2/dw2-op-out-param.exp: >> >> #0 0x000000000056bed0 in extract_signed_integer (addr=0x0, len=8, byte_order=BFD_ENDIAN_LITTLE) at ../../src/gdb/findvar.c:78 >> #1 0x000000000057e761 in unpack_long (type=0x1793f80, valaddr=0x0) at ../../src/gdb/value.c:2453 >> #2 0x000000000059baae in print_scalar_formatted (valaddr=0x0, type=0x1793f80, options=0x7fffdb2376c0, size=0, stream=0x1929a00) at ../../src/gdb/printcmd.c:404 >> #3 0x000000000059763b in val_print_scalar_formatted (type=0x1793f80, valaddr=0x0, embedded_offset=0, val=0x19dfcd0, options=0x7fffdb2376c0, size=0, stream=0x1929a00) >> at ../../src/gdb/valprint.c:970 >> #4 0x00000000006d1f2f in c_val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, original_value=0x19dfcd0, options= >> 0x7fffdb237860) at ../../src/gdb/c-valprint.c:407 >> #5 0x0000000000596f4a in val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, val=0x19dfcd0, options=0x7fffdb237940, >> language=0x8d34a0) at ../../src/gdb/valprint.c:778 >> #6 0x00000000006d3491 in cp_print_value_fields (type=0x1796110, real_type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, >> options=0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:363 >> #7 0x00000000006d37ef in cp_print_value_fields_rtti (type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options= >> 0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:458 >> #8 0x00000000006d1e45 in c_val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, original_value=0x19dfcd0, options= >> 0x7fffdb237d40) at ../../src/gdb/c-valprint.c:393 >> #9 0x0000000000596f4a in val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=0x7fffdb237e60, >> language=0x8d34a0) at ../../src/gdb/valprint.c:778 >> #10 0x0000000000597157 in common_val_print (val=0x19dfcd0, stream=0x1929a00, recurse=2, options=0x7fffdb237e60, language=0x8d34a0) at ../../src/gdb/valprint.c:840 >> #11 0x00000000005d9ea6 in print_frame_arg (arg=0x7fffdb237f30) at ../../src/gdb/stack.c:284 >> #12 0x00000000005daa49 in print_frame_args (func=0x1796af0, frame=0x1b5b800, num=-1, stream=0x168fd10) at ../../src/gdb/stack.c:645 >> #13 0x00000000005db9df in print_frame (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1, sal=...) at ../../src/gdb/stack.c:1172 >> #14 0x00000000005daf95 in print_frame_info (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1) at ../../src/gdb/stack.c:824 >> #15 0x00000000005dcdc5 in backtrace_command_1 (count_exp=0x0, show_locals=0, no_filters=0, from_tty=1) at ../../src/gdb/stack.c:1763 >> #16 0x00000000005dd1b6 in backtrace_command (arg=0x0, from_tty=1) at ../../src/gdb/stack.c:1860 >> #17 0x00000000004dc53f in do_cfunc (c=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:113 >> #18 0x00000000004df5d4 in cmd_func (cmd=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:1888 >> #19 0x00000000006e43d1 in execute_command (p=0x14623a2 "", from_tty=1) at ../../src/gdb/top.c:478 >> >> Odd that this bit of val_print_scalar_formatted: >> >> /* A scalar object that does not have all bits available can't be >> printed, because all bits contribute to its representation. */ >> if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset, >> TARGET_CHAR_BIT * TYPE_LENGTH (type))) >> val_print_optimized_out (stream); >> >> didn't catch this. I then added this further. >> >> --- c/gdb/value.c >> +++ w/gdb/value.c >> @@ -1078,6 +1078,8 @@ set_value_optimized_out (struct value *value, int val) >> int >> value_entirely_optimized_out (const struct value *value) >> { >> + gdb_assert (value->optimized_out || !value->lazy); >> + >> if (!value->optimized_out) >> return 0; >> if (value->lval != lval_computed >> @@ -1089,6 +1091,8 @@ value_entirely_optimized_out (const struct value *value) >> int >> value_bits_valid (const struct value *value, int offset, int length) >> { >> + gdb_assert (value->optimized_out || !value->lazy); >> + >> if (!value->optimized_out) >> return 1; >> if (value->lval != lval_computed >> >> And I see: >> >> Breakpoint 2, 0x000000000040058f in breakpt () >> (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for struct_param_two_reg_pieces >> bt >> #0 0x000000000040058f in breakpt () >> #1 0x00000000004005c2 in struct_param_two_reg_pieces (operand0=../../src/gdb/value.c:1081: internal-error: value_entirely_optimized_out: Assertion `value->optimized_out || !value->lazy' failed. >> A problem internal to GDB has been detected, >> further debugging may prove unreliable. >> Quit this debugging session? (y or n) FAIL: gdb.dwarf2/dw2-op-out-param.exp: Backtrace for test struct_param_two_reg_pieces (GDB internal error) >> Resyncing due to internal error. >> >> ... and then I ran out of time. Gotta run an errand. :-) >> > > The issue here is that optimized_out does not mean "the value is > optimized out", but instead (in the general case) means this value is at > least partially optimized out. > > The problem case are computed values, where if part of the value is > missing then the optimized_out flag is set, but we still rely on being > able to access the contents of the value, with the result that your > patch above, freeing the contents, causes problems. Right. > I don't include the patch here, but as an experiment I made a quick hack > where all the calls to set_value_optimized_out inside dwarf2loc.c call a > new method set_value_partially_optimized_out, this new method sets the > optimized_out flag but does not free the contents buffer, all the tests > now pass. I know this is a hack, we create computed values in other > situations, but the case of computed value and partially optimized out > values only appear to be tested for the dwarf case. Yep. I'm actually about to post a patch in this direction. I'm just splitting it into smaller unrelated chunks. -- Pedro Alves