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 9E97E3858D1E for ; Wed, 25 Jan 2023 03:45:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9E97E3858D1E 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 30P3j3Jq011299 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 24 Jan 2023 22:45:07 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 30P3j3Jq011299 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1674618308; bh=VBojdNhPArM86QFpvEAnmGsiVTe5poFK5HUVOXNwE/k=; h=Date:Subject:To:References:From:In-Reply-To:From; b=Wy9mRbJBndek8/vlNKPo5FstUNc+1B2x//jZNS43aHXo+45LQAY8jfGHCfwwXbMhK E+W9g1YYOkI6LEY9FEa0JqrXLP1IaknjAOSXjPwZynrWZmRjWpd4isJLv2ZGk+puoV dJVwU3M71YwvyIkGO6MJzxvJST1XXCRs1BZF82no= Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 7668F1E110; Tue, 24 Jan 2023 22:45:02 -0500 (EST) Message-ID: Date: Tue, 24 Jan 2023 22:45:00 -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: en-US 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> From: Simon Marchi In-Reply-To: <0fdfcde0-f51f-6f59-82c3-1089d2d016f0@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 25 Jan 2023 03:45:03 +0000 X-Spam-Status: No, score=-3032.5 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/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.) - 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 with frame_find_by_id. In the case without the pretty printer, where things appear to work, it means that frame_find_by_id returns the "real" sentinel frame, part of the "real" stack frame chain, and not the sentinel that is attached to the user-created frame. Things still kinda work in the end because those two sentinel frames behave the same, as far as "prev register" is concerned. Ultimately, I don't really understand how this "frame view" feature is useful. Sure, we pretend that the frame's sp and pc is what the user gave, but what about the other registers? It seems to me like you are very likely to get bogus values. The patch I had in mind was to give user-created frames a special unwinder, rather than letting other unwinders (the arch-specific ones, the dwarf2 one, etc) kick in. That unwinder would always return the unwind stop reason UNWIND_OUTERMOST, which effectively makes the user-created frame an outer frame, and we would never try to unwind any register value from it. It makes the test pass, because it breaks the chain of event explained above. But it's probably just papering over the actual problem. Simon