From patchwork Fri Apr 15 20:23:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Galvan X-Patchwork-Id: 11769 Received: (qmail 117752 invoked by alias); 15 Apr 2016 20:24:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 112915 invoked by uid 89); 15 Apr 2016 20:24:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=2015-06, 201506, 1624, sans X-HELO: mail-qk0-f181.google.com Received: from mail-qk0-f181.google.com (HELO mail-qk0-f181.google.com) (209.85.220.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 15 Apr 2016 20:24:08 +0000 Received: by mail-qk0-f181.google.com with SMTP id r184so41736976qkc.1 for ; Fri, 15 Apr 2016 13:24:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=xE/L/PR8IiCK7FTkQ+l4SA9k0pvAL+Nno0j4aF3HN88=; b=Gne6x22U/Mq1MNgzezXUHblcAco1fU83r0TBtATCJdbZd0+Bvvpkx7HY3KZ4308sZf zN2t4Uxqnaw596D2kfzz+3YnmCNjAzrdWwavnNwmObS+sFet/JO+tX/S9F6VxQrwfnJZ 5wr98P2AoIQtZ3FUKQJpDWXma5ahRq2284f1KFdSg2jy0VOQ2GdmI+b8U5Np2flKq53f pVl3eBcgA3qMpRsgrsoAnBPDpQHhCmFRYPgo3pCp2u1PNoLqrQiQUaetcB7ooM6LQ4Be HI1GGGFxzMBpYVoQyM46cJIXAJy6leMIhf5CZo6+6dmBG0bcx4gxaUzYYdDEC/3ahIRb nmdg== X-Gm-Message-State: AOPr4FW4tP9qq7FGvkP3r3ncVKaA1QIhQiTlqCHilY5e36sZJsVcbjijDA1iWeb6x6KmjJGK X-Received: by 10.55.74.140 with SMTP id x134mr29481947qka.19.1460751846336; Fri, 15 Apr 2016 13:24:06 -0700 (PDT) Received: from martin-galvan.dominio.tallertechnologies.com ([200.69.202.173]) by smtp.gmail.com with ESMTPSA id 139sm20956386qho.2.2016.04.15.13.24.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 15 Apr 2016 13:24:05 -0700 (PDT) From: Martin Galvan To: gdb-patches@sourceware.org, palves@redhat.com, jan.kratochvil@redhat.com, lgustavo@codesourcery.com, daniel.gutson@tallertechnologies.com Subject: [PING 2][PATCH v3] Fix gdb crash when trying to print the address of a synthetic C++ reference Date: Fri, 15 Apr 2016 17:23:58 -0300 Message-Id: <1460751838-20954-1-git-send-email-martin.galvan@tallertechnologies.com> After compiling a program which uses C++ references some optimizations may convert the references into synthetic "pointers". Trying to print the address of one of such synthetic references causes gdb to crash with the following error: (gdb) print &ref /build/buildd/gdb-7.7.1/gdb/dwarf2loc.c:1624: internal-error: Should not be able to create a lazy value with an enclosing type A problem internal to GDB has been detected, further debugging may prove unreliable. Apparently, what was causing it was that value_addr returns a copy of the value that represents the reference with its type set to T* instead of T&. However, its enclosing_type is left untouched, which fails a check done in read_pieced_value. We only see the crash happen for references that are synthetic because they're treated as pieced values, thus the call to read_pieced_value. This was previously discussed here: https://sourceware.org/ml/gdb-patches/2015-06/msg00278.html though the proposed patch was somewhat different. In the previous thread (i.e. sans 'PING') I mentioned I'd try to use readjust_indirect_value_type, however I desisted because it seems a bit weird to pass arg2 as both a const and non-const parameter. On a related note, from what we discussed with Pedro it seems that in general there are all sorts of breakage when working with synthetic references. I reported this here: https://sourceware.org/bugzilla/show_bug.cgi?id=19893 I'll be working on fixing those other issues, but for now I think this patch can go in. I'll add new test cases as I go along. I have write access and copyright assignment; let me know if this is ok to commit or if you have any feedback. gdb/ChangeLog: 2016-04-08 Martin Galvan * valops.c (value_addr): For C++ references, set the copied value's enclosing_type as well. gdb/testsuite/ChangeLog: 2016-04-08 Martin Galvan * gdb.dwarf2/implref.exp: New file. --- gdb/testsuite/gdb.dwarf2/implref.exp | 99 ++++++++++++++++++++++++++++++++++++ gdb/valops.c | 16 ++++-- 2 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/implref.exp diff --git a/gdb/testsuite/gdb.dwarf2/implref.exp b/gdb/testsuite/gdb.dwarf2/implref.exp new file mode 100644 index 0000000..8599715 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/implref.exp @@ -0,0 +1,99 @@ +# Copyright 2016 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test C++ references marked with DW_OP_GNU_implicit_pointer. + +# TODO: Add more test statements after fixing bug #19893: +# https://sourceware.org/bugzilla/show_bug.cgi?id=19893. + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use gas. +if {![dwarf2_support]} { + return 0 +} + +# We'll place the output of Dwarf::assemble in implref.S. +standard_testfile main.c .S + +# ${testfile} is now "implref". srcfile2 is "implref.S". +set executable ${testfile} +set asm_file [standard_output_file ${srcfile2}] + +# Create the DWARF. We need a regular variable and a reference to it that'll +# be marked with DW_OP_GNU_implicit_pointer. +Dwarf::assemble $asm_file { + global srcdir subdir srcfile + + cu { version 3 addr_size 4 } { + DW_TAG_compile_unit { + {DW_AT_producer "GNU C++ 4.8.4"} + {DW_AT_language @DW_LANG_C_plus_plus} + {DW_AT_name "implref.S"} + } { + declare_labels int_label const_label variable_label ref_label + + int_label: DW_TAG_base_type { + {DW_AT_byte_size 4 DW_FORM_udata} + {DW_AT_encoding @DW_ATE_signed} + {DW_AT_name "int"} + } + + ref_label: DW_TAG_reference_type { + {DW_AT_byte_size 4 DW_FORM_udata} + {DW_AT_type :${int_label}} + } + + const_label: DW_TAG_const_type { + {DW_AT_type :${ref_label}} + } + + DW_TAG_subprogram { + {MACRO_AT_func { "main" "${srcdir}/${subdir}/${srcfile}" }} + {DW_AT_type :${int_label}} + {DW_AT_external 1 DW_FORM_flag} + } { + variable_label: DW_TAG_variable { + {DW_AT_name "var"} + {DW_AT_type :${int_label}} + {DW_AT_const_value 42 DW_FORM_udata} + } + + DW_TAG_variable { + {DW_AT_name "ref"} + {DW_AT_type :${const_label}} + {DW_AT_location {DW_OP_GNU_implicit_pointer ${variable_label} 0} SPECIAL_expr} + } + } + } + } +} + +if [prepare_for_testing ${testfile}.exp ${executable} "${asm_file} ${srcfile}" {}] { + return -1 +} + +# DW_OP_GNU_implicit_pointer implementation requires a valid frame. +if ![runto_main] { + return -1 +} + +gdb_test "print ref" " = \\(int &\\) " "print ref" +gdb_test "print &ref" " = \\(int \\*\\) " "print &ref" + +# gdb assumes C++ references are implemented as pointers, and print &(&ref) +# shows us the underlying pointer's address. +# Since in this case there's no physical pointer, gdb should tell us so. +gdb_test "print &\(&ref\)" "Attempt to take address of value not located in memory." "print &(&ref)" diff --git a/gdb/valops.c b/gdb/valops.c index 5a244a9..40559c2 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -1463,11 +1463,19 @@ value_addr (struct value *arg1) if (TYPE_CODE (type) == TYPE_CODE_REF) { /* Copy the value, but change the type from (T&) to (T*). We - keep the same location information, which is efficient, and - allows &(&X) to get the location containing the reference. */ + keep the same location information, which is efficient, and + allows &(&X) to get the location containing the reference. + Do the same to its enclosing type for consistency. */ + struct type *type_ptr = lookup_pointer_type (TYPE_TARGET_TYPE (type)); + + struct type *enclosing_type = check_typedef (value_enclosing_type (arg1)); + struct type *enclosing_type_ptr = + lookup_pointer_type (TYPE_TARGET_TYPE (enclosing_type)); + arg2 = value_copy (arg1); - deprecated_set_value_type (arg2, - lookup_pointer_type (TYPE_TARGET_TYPE (type))); + deprecated_set_value_type (arg2, type_ptr); + set_value_enclosing_type (arg2, enclosing_type_ptr); + return arg2; } if (TYPE_CODE (type) == TYPE_CODE_FUNC)