From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2829 invoked by alias); 7 Jan 2014 20:16:59 -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 2814 invoked by uid 89); 7 Jan 2014 20:16:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: homiemail-a83.g.dreamhost.com Received: from caibbdcaabde.dreamhost.com (HELO homiemail-a83.g.dreamhost.com) (208.113.200.134) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Jan 2014 20:16:56 +0000 Received: from homiemail-a83.g.dreamhost.com (localhost [127.0.0.1]) by homiemail-a83.g.dreamhost.com (Postfix) with ESMTP id 7CC015E07D; Tue, 7 Jan 2014 12:16:55 -0800 (PST) Received: from redwood.eagercon.com (c-50-148-128-197.hsd1.ca.comcast.net [50.148.128.197]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: eager@eagerm.com) by homiemail-a83.g.dreamhost.com (Postfix) with ESMTPSA id 5FB745E078; Tue, 7 Jan 2014 12:16:55 -0800 (PST) Message-ID: <52CC60B7.60004@eagerm.com> Date: Tue, 07 Jan 2014 20:16:00 -0000 From: Michael Eager User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Tom Tromey CC: "gdb-patches@sourceware.org" Subject: Re: [PATCH] Fix check for ICC incomplete struct types References: <52CC3790.4030702@eagerm.com> <87bnznhbtn.fsf@fleche.redhat.com> In-Reply-To: <87bnznhbtn.fsf@fleche.redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-01/txt/msg00162.txt.bz2 On 01/07/14 10:43, Tom Tromey wrote: >>>>>> "Michael" == Michael Eager writes: > > Michael> GDB contains code in read_structure_type() which is supposed > Michael> to check for incorrect DWARF generated by ICC for an incomplete > Michael> structure type. The code is incomplete, in that it doesn't > Michael> check for length == 0, and it doesn't set the STUB flag. > > Thanks. > > Michael> - if (producer_is_icc (cu)) > Michael> + if (producer_is_icc (cu) && (TYPE_LENGTH (type) == 0)) > Michael> { > Michael> /* ICC does not output the required DW_AT_declaration > Michael> on incomplete types, but gives them a size of zero. */ > Michael> + TYPE_STUB (type) = 1; > Michael> } > Michael> else > Michael> TYPE_STUB_SUPPORTED (type) = 1; > > It seems to me that TYPE_STUB_SUPPORTED should be set unconditionally > (and, btw, eww, what a big hack TYPE_STUB_SUPPORTED is). Without the > TYPE_STUB_SUPPORTED setting, TYPE_IS_OPAQUE does not honor TYPE_STUB. > > Then the icc check moved to the next stanza, say like: > > TYPE_STUB_SUPPORTED (type) = 1; > if (die_is_declaration (die, cu)) > TYPE_STUB (type) = 1; > else if (producer_is_icc (cu) && TYPE_LENGTH (type) == 0) > { > /* ICC does not output the required DW_AT_declaration > on incomplete types, but gives them a size of zero. */ > TYPE_STUB (type) = 1; > } > else if (attr == NULL && die->child == NULL > && producer_is_realview (cu->producer)) > /* RealView does not output the required DW_AT_declaration > on incomplete types. */ > TYPE_STUB (type) = 1; > > What do you think of this? TYPE_IS_OPAQUE does honor TYPE_STUB and checks it before TYPE_STUB_SUPPORTED. Which is absolutely screwy, since TYPE_STUB_SUPPORTED is set for all non-ICC producers. #define TYPE_IS_OPAQUE(thistype) \ (((TYPE_CODE (thistype) == TYPE_CODE_STRUCT) \ || (TYPE_CODE (thistype) == TYPE_CODE_UNION)) \ && (TYPE_NFIELDS (thistype) == 0) \ && (!HAVE_CPLUS_STRUCT (thistype) \ || TYPE_NFN_FIELDS (thistype) == 0) \ && (TYPE_STUB (thistype) || !TYPE_STUB_SUPPORTED (thistype))) So the last expression for non-ICC is equivalent to (TYPE_STUB (thistype) || 0)) The only other place that TYPE_STUB is set in dwarf2read.c: read_structure_type() is for TYPE_CODE_ENUM, which is excluded from TYPE_IS_OPAQUE. (Not that I understand why.) It is also set in gdbtypes.c: allocate_stub_method() with TYPE_CODE_METHOD, which is excluded from the test. I don't understand why TYPE_CODE_CLASS is excluded from TYPE_IS_OPAQUE either, since everything else is the same for structs and classes. A test with ICC (after the patch) doesn't show any difference whether TYPE_STUB_SUPPORTED is set or not, as expected. As far as I can see, TYPE_STUB_SUPPORTED does nothing. I'm happy to remove it and all the associated dreck and reorder the test as you suggest. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077