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 96C783858D1E for ; Mon, 30 Jan 2023 16:20:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 96C783858D1E Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca 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 30UGKKBB008647 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 30 Jan 2023 11:20:25 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 30UGKKBB008647 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1675095625; bh=FvFNpmd34VeZPUyR4w+gImySj49D9UBwTg4crUSng6M=; h=Date:Subject:To:References:From:In-Reply-To:From; b=U4H9kMQAqUBn3v73AQxNHzpnC8ImXyZu9iZvj+O5QKkp+epJU/Y/w6lFEG8svW6s3 mq0mRc3qQMmRcTdCI0vYBeSRyCwmwyo69YsHV2iA7eBLS32H+EX2s76EzWmGotfWqZ BTMCRC1CevsEezHSta9dzqx1wn/pLd9MuvGejlaM= Received: from [172.16.0.192] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 237681E112; Mon, 30 Jan 2023 11:20:20 -0500 (EST) Message-ID: Date: Mon, 30 Jan 2023 11:20:19 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 11/13] gdb: make user-created frames reinflatable Content-Language: fr To: Luis Machado , Tom de Vries , gdb-patches@sourceware.org References: <20221214033441.499512-1-simon.marchi@polymtl.ca> <20221214033441.499512-12-simon.marchi@polymtl.ca> <257ccf41-56aa-6ea7-ea41-99e2d937c1eb@suse.de> <0fdfcde0-f51f-6f59-82c3-1089d2d016f0@arm.com> <391a1df6-8eec-c086-146f-a90c8f09c448@arm.com> From: Simon Marchi In-Reply-To: <391a1df6-8eec-c086-146f-a90c8f09c448@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 30 Jan 2023 16:20:20 +0000 X-Spam-Status: No, score=-3032.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 1/30/23 03:49, Luis Machado wrote: > On 1/25/23 03:45, Simon Marchi wrote: >> >> >> On 1/24/23 03:22, Luis Machado wrote: >>> On 1/24/23 03:55, Simon Marchi wrote: >>>> >>>> >>>> On 1/23/23 09:34, Luis Machado wrote: >>>>> On 1/23/23 12:57, Tom de Vries wrote: >>>>>> On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote: >>>>>>> gdb/testsuite/gdb.base/frame-view.exp | 47 ++++++++++++-- >>>>>> >>>>>> Hi, >>>>>> >>>>>> on aarch64-linux I get: >>>>>> ... >>>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error) >>>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again (GDB internal error) >>>>> >>>>> >>>>> I can reproduce this as well. It also happens on arm-linux Ubuntu 22.04/20.04, with a similar kind of backtrace. >>>> >>>> I can reproduce too, I have a potential fix. I'll try to send it >>>> tomorrow. >>> >>> Let me know if I can help with testing it on one of the targets. >> >> Ok, so I have a patch that fixes the test, but I'm not sure it's really >> good. >> >> First, here's my analysis of what I think is happening. >> >> - When we create the user frame (the "select-frame view" command), we >> create a sentinel frame just for our user-created frame, in >> create_new_frame. This sentinel frame has the same id as the regular >> sentinel frame. >> - When printing the frame, after doing the "select-frame view" >> command, the argument's pretty printer is invoked, which does an >> inferior function call (this is the point of the test). This clears >> the frame cache, including the "real" sentinel frame, which sets the >> sentinel_frame global to nullptr. >> - Later in the frame-printing process (when printing the second >> argument), the auto-reinflation mechanism (which would have been the >> manual reinflate call before, doesn't matter here) re-creates the >> user frame by calling create_new_frame again, creating its own >> special sentinel frame again. However, note that the "real" sentinel >> frame, the sentinel_frame global, is still nullptr. If the selected >> frame had been a regular frame, we would have called >> get_current_frame at some point during the reinflation, which would >> have re-created the "real" sentinel frame. But it's not the case >> here. >> - Deep down the stack, something wants to fill in the unwind stop >> reason for frame 0, which requires trying to unwind frame 1. This >> leads us to trying to unwind the PC of frame 1: >> >> #0 gdbarch_unwind_pc (gdbarch=0xffff8d010080, next_frame=...) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:2955 >> #1 0x000000000134569c in dwarf2_tailcall_sniffer_first (this_frame=..., tailcall_cachep=0xffff773fcae0, entry_cfa_sp_offsetp=0xfffff7f7d450) >> at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:390 >> #2 0x0000000001355d84 in dwarf2_frame_cache (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1089 >> #3 0x00000000013562b0 in dwarf2_frame_unwind_stop_reason (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1101 >> #4 0x0000000001990f64 in get_prev_frame_always_1 (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2281 >> #5 0x0000000001993034 in get_prev_frame_always (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2376 >> #6 0x000000000199b814 in get_frame_unwind_stop_reason (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:3051 >> #7 0x0000000001359cd8 in dwarf2_frame_cfa (this_frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1356 >> #8 0x000000000132122c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d8883ee "\217\002", op_end=0xffff8d8883ee "\217\002") >> at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:2110 >> #9 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d8883ed "\234\217\002", len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239 >> #10 0x000000000131d68c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d88840e "", op_end=0xffff8d88840e "") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1811 >> #11 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239 >> #12 0x0000000001314c3c in dwarf_expr_context::evaluate (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2, as_lval=true, per_cu=0xffff90b03700, frame=..., addr_info=0x0, >> type=0xffff8f6c8400, subobj_type=0xffff8f6c8400, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1078 >> #13 0x000000000149f9e0 in dwarf2_evaluate_loc_desc_full (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, >> subobj_type=0xffff8f6c8400, subobj_byte_offset=0, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1513 >> #14 0x00000000014a0100 in dwarf2_evaluate_loc_desc (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, as_lval=true) >> at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1557 >> #15 0x00000000014aa584 in locexpr_read_variable (symbol=0xffff8f6cd770, frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3052 >> - AArch64 defines a special "prev register" function, >> aarch64_dwarf2_prev_register, to handle unwinding the PC. This >> function does >> >> frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM); >> >> (I must admit that I don't really understand this part, it seems to >> me like you would want to unwind LR's value from this_frame->next... >> but anyway.) > > It is somewhat historical, and it is something I'm planning to address eventually. Right now, though, we hardwire the PC to > obtaining LR. Ok, I'd be interesting in knowing more about the situation, for my own general knowledge. I might be understanding the GDB unwinding wrong of the ARM-specific unwinding wrong. >> - frame_unwind_register_unsigned ultimately creates a lazy register >> value, saving the frame id of this_frame->next. this_frame is the >> user-created frame, to this_frame->next is the special sentinel frame >> we created for it. >> - When time comes to un-lazify the value, value_fetch_lazy_register >> calls frame_find_by_id, to find the frame with the id we saved. >> - frame_find_by_id sees it's the sentinel frame id, so returns the >> sentinel_frame global, which is, if you remember, nullptr. >> - We hit the `gdb_assert (next_frame != NULL)` assertion. >> >> I have a hard time pinpointing a single thing that is wrong. The fact >> that the user-created frames have their own sentinel frame, which share >> the same id as the real sentinel frame, that sounds like it can't work > > From your (very detailed) explanation, it does sound a bit iffy to abuse the frame id mechanism and use > the same id as the existing real frame. Yeah, my latest attempt at fixing it revolves around giving the user-created frame's sentinels their own specific ID. Simon