From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 4MgzBhduwWoSnTEAWB0awg (envelope-from ) for ; Sat, 03 Oct 2026 17:05:27 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=awQ4DipC; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 009741E050; Sat, 03 Oct 2026 17:05:26 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id B88131E047 for ; Sat, 03 Oct 2026 17:05:25 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 359AA4BA9004 for ; Sat, 3 Oct 2026 21:05:25 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 359AA4BA9004 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=awQ4DipC Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 876C84BA2E11 for ; Sat, 3 Oct 2026 21:04:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 876C84BA2E11 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 876C84BA2E11 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1791061498; cv=none; b=h4ThBM12yBDBwOhlR8jh875sQYWQJ8OVTFAujqFN5QfKdymKPz9R9GdSGH9oy0IbeFDpbibVhRpcqen4t+FCpmp9aio2ciV7BA2U1xT12s/gw8dy7ouWIKZucKq4MnAqb4IjYAJBaZ3or04vTWbnnahOAkO45RoxW+grXu6Iia8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1791061498; c=relaxed/simple; bh=/r+PLQMG40jwyDV+KhauoA5K2U82jLZ50CaL4kNHg3k=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=GqxQaLk8uxG1HKLFk1k57zI4FNMFVRyEHKWF4DcPpsYWTSXWWihkBoMU4UcwieoYA2c4vhB5AG1J53kfZuGPxvggXl7jbm3nnjtwxQxui3zqBDt4VOlzmhaSiz5fOM7Mif96GH2Wl8RCXwdN3dKWOkJySJX7BNn1ruAblljpH7Y= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=awQ4DipC DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 876C84BA2E11 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1791061498; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=L0Ch4kPDiGEaDOUCUHS/cS5kT0Z2FZi9I6QvGiZ4NZs=; b=awQ4DipCrvyvnS2G+cJPoZkMmFFw5GCWz5mew7Zg5l9Htstk+eRgw0mkQRgDQjU0WArNxw T/mgOU9l4PzgXk/+11wFJ42JUVu3i5hjV47t7Htt4MNgtG0NhWW14K3zhgUaAiCPsGnNBy lMK07Q7eDqdXqTItaJE0Q/AvSoZnzQc= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-596-rcQTnayINqC3dgZlGsRSug-1; Sat, 03 Oct 2026 17:04:56 -0400 X-MC-Unique: rcQTnayINqC3dgZlGsRSug-1 X-Mimecast-MFC-AGG-ID: rcQTnayINqC3dgZlGsRSug_1791061495 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-49cdc4080ddso6386285e9.1 for ; Sat, 03 Oct 2026 14:04:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1791061495; x=1791666295; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=L0Ch4kPDiGEaDOUCUHS/cS5kT0Z2FZi9I6QvGiZ4NZs=; b=B+24llvJUvG/G4kC+Bia8Jbng0lYCCX5Z7KEoLhE40FLQeTy67ShkKo48dq7D4Foea MdWtAgfChe5leQdd/yPKBLWOBTFNo4/wko8LJzIMUYkbiR+jz5bdITBl7IeNSlIU4WyH dSFRUZYBkWPg3jDqMpgx8mm23/D7cOLG1Oipw0MLHUsjYJqFTdaFhyKh9/cBpd13JGoL QLSaRWsAGAZoVz6tmfvvUxRzcboWJWu2tUsfxiZTh+zbVwW0No07JX60Bu1Etktq1WMQ MmS3+XJ1/aHf0gbhlKUTpDICeDIOVYN6g2pmD7oPb7SlU8H0OUKNGwu2Bxh6Fm5pY8Z/ e/fg== X-Gm-Message-State: AFuF++mxxSqQw0EMbUC0mcOxPbw6T0INxNwDrVqo+eqh7BrnB2wyKaSG 8yElzULGP5fQOQeCklLbZIL5SumaIZqFVKpSuc4mnSmYFtqLp56oba5Vvp2d19VDtXjScDVt7Al 1OOdy1vtpDASJ69gt8tvOg/sD/VBpHxG30zl6sfMin9/maPUzECnV+eQwL7EOEq2PZp511eyr5S 89819jSR9zK5eRLH6XUw0awwukxjOi86Psouwt6tAf/GihieI= X-Gm-Gg: AYBFou3151QPb5ITDLY6AROMj6k7ZGHwAHfu35YgyE27A0t+iq/89XPE5+CnXuTrbGa 3oWdQ9tASdLiCl12DKIb/HKzElavAJHfDoAsgI//YIOFOBN16awJIjzRrOou5OhWS1UGLCjeDoZ Ro19SILEssakRFryWRQBbv29lVGUY85sdZs6nrcNe3rHGyA+A5Pe3rq7lPhB5nimYb4z2+wKgD2 cXElvYmUfouEaTR5jusuo7thXKF4iaqhKMduBlQwdQnSae6axEeq5BBYos5IcPBXZ0+SUsRRL85 rfOrfhtO7F3p/QHGJNZiSZLcy5tYwgrcOQAadZcbtrKalBM9Y/VnCzd7kRCW+asm39EM X-Received: by 2002:a05:600c:1e24:b0:49f:ffa2:1c72 with SMTP id 5b1f17b1804b1-4a0275507d7mr110472135e9.11.1791061495156; Sat, 03 Oct 2026 14:04:55 -0700 (PDT) X-Received: by 2002:a05:600c:1e24:b0:49f:ffa2:1c72 with SMTP id 5b1f17b1804b1-4a0275507d7mr110471825e9.11.1791061494528; Sat, 03 Oct 2026 14:04:54 -0700 (PDT) Received: from localhost ([213.31.44.29]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4a0280b2e3csm238108595e9.5.2026.10.03.14.04.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 03 Oct 2026 14:04:53 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2] gdb: unify trivial_entry_value handling in DW_OP_entry_value Date: Sat, 3 Oct 2026 22:04:51 +0100 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: VTucti166fmEUAScd6UtpCXsuseNhcpMHC6rvPzirwo_1791061495 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit content-type: text/plain; charset="US-ASCII"; x-default=true X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org In V2: - Rebase and retest. - Spelling / grammar fixes in comments and commit messages - Reworded the commit message for increased clarity. - No code changes. Thanks, Andrew ~~~~~ This commit simplifies the DW_OP_entry_value handling in dwarf2/expr.c for the case where trivial_entry_value is true, as a consequence I hit a bug in dwarf_expr_context::fetch_result which I have also fixed. Currently GDB can handle two different DW_OP_entry_value DWARF expressions, DW_OP_reg or DW_OP_breg/DW_OP_deref, and the current trivial_entry_value is also split in two to mirror these two cases. The trivial_entry_value function returns true when the inferior is stopped at the first instruction of a function. This means that the value of the DW_OP_entry_value expression is just the current value of that DWARF expression. This commit moves the trivial_entry_value handling to earlier in the DW_OP_entry_value case statement block, and unifies the existing two cases. The new code performs a recursive evaluation of the DW_OP_entry_value DWARF expression. I took the m_stack save and restore idea from how DW_OP_fbreg is handled, though I used SCOPE_EXIT for the restore so that the restore happens even on the exception case. Also, unlike the fbreg case I also save and restore m_location as DW_OP_entry_value could be in the middle of a complex DWARF expression which has already set m_location, and m_pieces as we could also be in the middle of building a composite value. Finally, the result is pulled from m_stack by a call to fetch_result. The DWARF 5 spec says this about DW_OP_entry_value: The DW_OP_entry_value operation pushes the value that the described location held upon entering the current subprogram. It has two operands: an unsigned LEB128 length, followed by a block containing a DWARF expression or a register location description (see Section 2.6.1.1.3 on page 39). The length operand specifies the length in bytes of the block. If the block contains a DWARF expression, the DWARF expression is evaluated as if it had been evaluated upon entering the current subprogram. The DWARF expression assumes no values are present on the DWARF stack initially and results in exactly one value being pushed on the DWARF stack when completed. If the block contains a register location description, DW_OP_entry_value pushes the value that register had upon entering the current subprogram. >From this, and from how GDB currently handles DW_OP_breg/DW_OP_deref within the DW_OP_entry_value case, I believe that this means there are two possible results from a DW_OP_entry_value expression: (1) The result is left on the DWARF expression stack. (2) The result is a register location description, in which case the result is read from the register. Based on #1 I initially wanted to pass as_lval=false to fetch_result, passing as_lval=false means that m_location is forced to be DWARF_VALUE_STACK. However, this causes a problem when m_location is initially DWARF_VALUE_REGISTER, changing to DWARF_VALUE_STACK means that GDB reads the register number from the stack, which is never what we want. This got me looking at how as_lval is handled in fetch_result, and I'm not convinced that the current handling makes much sense. Converting from DWARF_VALUE_MEMORY to DWARF_VALUE_STACK is fine, this causes GDB to read the result from the expression stack rather than treating the value on the stack as a memory location, but for DWARF_VALUE_REGISTER, DWARF_VALUE_LITERAL, DWARF_VALUE_OPTIMIZED_OUT, and DWARF_VALUE_IMPLICIT_POINTER converting to DWARF_VALUE_STACK doesn't really work as there either isn't a value on the stack, or the value on the stack is an internal detail, e.g. the DWARF register number for DWARF_VALUE_REGISTER. I did consider changing this existing code block: if (!as_lval) this->m_location = DWARF_VALUE_STACK; to this: if (!as_lval && this->m_location == DWARF_VALUE_MEMORY) this->m_location = DWARF_VALUE_STACK; But I figured it would actually be neater if I pushed the as_lval handling down into the switch statement. The switch is now structured like this: switch (this->m_location) { ... case DWARF_VALUE_MEMORY: if (as_lval) { ... break; } [[fallthrough]]; case DWARF_VALUE_STACK: ... } So when as_lval is false we treat it as DWARF_VALUE_STACK just like before, but for any other location type we leave things unchanged. This makes more sense to me, this is the bug fix to which I referred at the top of this commit message. Back in the DW_OP_entry_value handling we can now pass as_lval=false to the fetch_result call, and we will correctly handle the two cases I identified above, a value on the stack will be treated as an actual value, while a register location description will result in the value being read from the register. I don't think this issue with fetch_result ever bit us before as the only place where fetch_result could be called with as_lval=false would only ever pass DWARF_VALUE_MEMORY or DWARF_VALUE_STACK, in which case the old behaviour was fine. There are no new tests added here. The changes in dwarf_expr_context::execute_stack_op are a refactor, and the bug in dwarf_expr_context::fetch_result could not previously be reached in a way that would cause incorrect behaviour. The fetch_result fix is covered by the existing gdb.dwarf2/dw2-entry-value*.exp tests after the refactor though. --- gdb/dwarf2/expr.c | 133 +++++++++++++++++++++++++++------------------- gdb/dwarf2/expr.h | 8 ++- 2 files changed, 84 insertions(+), 57 deletions(-) diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c index 934fda67ca6..347f7f9149c 100644 --- a/gdb/dwarf2/expr.c +++ b/gdb/dwarf2/expr.c @@ -1031,11 +1031,6 @@ dwarf_expr_context::fetch_result (struct type *type, struct type *subobj_type, } else { - /* If AS_LVAL is false, means that the implicit conversion - from a location description to value is expected. */ - if (!as_lval) - this->m_location = DWARF_VALUE_STACK; - switch (this->m_location) { case DWARF_VALUE_REGISTER: @@ -1070,21 +1065,26 @@ dwarf_expr_context::fetch_result (struct type *type, struct type *subobj_type, break; case DWARF_VALUE_MEMORY: - { - struct type *ptr_type; - CORE_ADDR address = this->fetch_address (0); - bool in_stack_memory = this->fetch_in_stack_memory (0); + /* If AS_LVAL is true, handle this memory location normally. + Otherwise, fall through to DWARF_VALUE_STACK and treat + the value on the DWARF stack as a plain value, not a + memory location description. */ + if (as_lval) + { + struct type *ptr_type; + CORE_ADDR address = this->fetch_address (0); + bool in_stack_memory = this->fetch_in_stack_memory (0); - /* DW_OP_deref_size (and possibly other operations too) may - create a pointer instead of an address. Ideally, the - pointer to address conversion would be performed as part - of those operations, but the type of the object to - which the address refers is not known at the time of - the operation. Therefore, we do the conversion here - since the type is readily available. */ + /* DW_OP_deref_size (and possibly other operations too) may + create a pointer instead of an address. Ideally, the + pointer to address conversion would be performed as part + of those operations, but the type of the object to + which the address refers is not known at the time of + the operation. Therefore, we do the conversion here + since the type is readily available. */ - switch (subobj_type->code ()) - { + switch (subobj_type->code ()) + { case TYPE_CODE_FUNC: case TYPE_CODE_METHOD: ptr_type = builtin_type (arch)->builtin_func_ptr; @@ -1092,16 +1092,16 @@ dwarf_expr_context::fetch_result (struct type *type, struct type *subobj_type, default: ptr_type = builtin_type (arch)->builtin_data_ptr; break; - } - address = value_as_address (value_from_pointer (ptr_type, address)); - - retval = value_at_lazy (subobj_type, address + subobj_offset, - m_frame); - if (in_stack_memory) - retval->set_stack (true); - } - break; + } + address = value_as_address (value_from_pointer (ptr_type, address)); + retval = value_at_lazy (subobj_type, address + subobj_offset, + m_frame); + if (in_stack_memory) + retval->set_stack (true); + break; + } + [[fallthrough]]; case DWARF_VALUE_STACK: { value *val = this->fetch (0); @@ -2357,26 +2357,60 @@ dwarf_expr_context::execute_stack_op (gdb::array_view expr) error (_("DW_OP_entry_value: too few bytes available.")); auto entry_value_expr = gdb::make_array_view (op_ptr, len); + op_ptr += len; + + if (trivial_entry_value (this->m_frame)) + { + /* Rather than create a whole new context, we simply back up + the current stack locally and install a new empty stack, + then reset it afterwards, effectively erasing whatever + the recursive call put there. */ + std::vector saved_stack = std::move (this->m_stack); + SCOPE_EXIT { this->m_stack = std::move (saved_stack); }; + this->m_stack.clear (); + + /* Back up the location as DW_OP_entry_value might appear as + part of some complex expression that has already set a + non-memory location. We don't need to reset m_location + back to a default value though as calling eval does that + for us. */ + scoped_restore restore_m_location = make_scoped_restore (&this->m_location); + + /* Back up, and arrange to restore, the saved pieces vector + as this is referenced by the fetch_result call below. It + would be unusual for DW_OP_entry_value to appear within a + composite location, but it's easy enough to handle this + case correctly, so let's do that. */ + std::vector saved_pieces = std::move (this->m_pieces); + SCOPE_EXIT { this->m_pieces = std::move (saved_pieces); }; + this->m_pieces.clear (); + + /* By this point m_initialized must be true, there is no point + backing this up and restoring it as evaluating + ENTRY_VALUE_EXPR will just set this to true again. */ + gdb_assert (this->m_initialized); + + /* Evaluate the entry expression. */ + eval (entry_value_expr); + + /* After evaluating ENTRY_VALUE_EXPR m_initialized should + still be true. */ + gdb_assert (this->m_initialized); + + /* The DWARF spec says the entry value block can contain + either a DWARF expression or a register location + description. Pass as_lval=false here so that + DWARF_VALUE_MEMORY locations will be treated as + DWARF_VALUE_STACK, treating the expression as a value, + not a location. */ + result_val = fetch_result (address_type, address_type, 0, + false); + break; + } + kind_u.dwarf_reg = dwarf_block_to_dwarf_reg (entry_value_expr); if (kind_u.dwarf_reg != -1) { - op_ptr += len; - - if (trivial_entry_value (this->m_frame)) - { - /* We can assume that DW_OP_entry_value (expr) == expr. - Handle DW_OP_regx, place register value on the - stack. */ - gdbarch *f_arch = get_frame_arch (this->m_frame); - int dwarf_regnum = kind_u.dwarf_reg; - int gdb_regnum - = dwarf_reg_to_regnum_or_error (f_arch, dwarf_regnum); - result_val - = value_from_register (address_type, gdb_regnum, - this->m_frame); - break; - } - this->push_dwarf_reg_entry_value (CALL_SITE_PARAMETER_DWARF_REG, kind_u, -1 /* deref_size */); @@ -2389,17 +2423,6 @@ dwarf_expr_context::execute_stack_op (gdb::array_view expr) { if (deref_size == -1) deref_size = this->m_addr_size; - op_ptr += len; - - if (trivial_entry_value (this->m_frame)) - { - /* We can assume that DW_OP_entry_value (expr) == expr. - Handle as DW_OP_bregx;DW_OP_deref_size. */ - CORE_ADDR addr - = read_addr_from_reg (this->m_frame, kind_u.dwarf_reg); - result_val = this->deref (addr, deref_size); - break; - } this->push_dwarf_reg_entry_value (CALL_SITE_PARAMETER_DWARF_REG, kind_u, deref_size); diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h index 02b0e41f6fd..7a632b2ba65 100644 --- a/gdb/dwarf2/expr.h +++ b/gdb/dwarf2/expr.h @@ -221,8 +221,12 @@ struct dwarf_expr_context /* Fetch the result of the expression evaluation in a form of a struct value, where TYPE, SUBOBJ_TYPE and SUBOBJ_OFFSET describe the source level representation of that result. - AS_LVAL defines if the fetched struct value is expected to - be a value or a location description. */ + + When AS_LVAL is false, any memory location descriptions + (DWARF_VALUE_MEMORY) are treated as values on the stack + (i.e. handled as DWARF_VALUE_STACK). Handling of any other + dwarf_value_location type is unchanged. When AS_LVAL is true + then all dwarf_value_location types are handled as normal. */ value *fetch_result (struct type *type, struct type *subobj_type, LONGEST subobj_offset, bool as_lval); base-commit: 36dd3acb47b80e430b712281cf55a02777677242 -- 2.25.4