From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16185 invoked by alias); 4 Dec 2009 18:35:29 -0000 Received: (qmail 16166 invoked by uid 22791); 4 Dec 2009 18:35:28 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Dec 2009 18:35:18 +0000 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nB4IYXte014658 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 4 Dec 2009 13:34:33 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx08.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id nB4IYW1Q031547; Fri, 4 Dec 2009 13:34:32 -0500 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id nB4IYVqO023746; Fri, 4 Dec 2009 13:34:31 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id 573A4378183; Fri, 4 Dec 2009 11:34:31 -0700 (MST) From: Tom Tromey To: Hilfinger@adacore.com Cc: gdb-patches@sourceware.org Subject: Re: [RFA] 64-bit range types in GDB References: <20091204081343.A8718227B32@nile.gnat.com> Reply-To: tromey@redhat.com Date: Fri, 04 Dec 2009 18:35:00 -0000 In-Reply-To: <20091204081343.A8718227B32@nile.gnat.com> (Paul Hilfinger's message of "Fri, 4 Dec 2009 03:13:43 -0500 (EST)") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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 X-SW-Source: 2009-12/txt/msg00062.txt.bz2 >>>>> "Paul" == Paul Hilfinger writes: Paul> It required doing some violence to struct main_type, Paul> unfortunately, but as a side effect, the representation of range Paul> types is a bit less of an abuse of abstraction (IMHO, anyway). I agree, I think this is better. Overall this patch looks great. I have a couple nits to pick. Paul> I apologize to those of you who will have to modify your Paul> type-manipulation routines in .gdbinit files. I'm interested in seeing this sort of thing, so if you have gdb-specific gdbinit hacks, please email them to me :-) Paul> +/* Fix the RANGE_TYPE flags if we think they are incorrect. Paul> + This is a temporary (?) hack to work around problems with handling Paul> + of >32bit range types on older compilers. On pure 32-bit hosts, Paul> + the compiler does not always emit the bounds as expected. Paul> + FIXME: pnh/2009-08-05. */ Paul> + Paul> +static void Paul> +fixup_range_type_hack (struct type *range_type, struct die_info *die, Paul> + struct dwarf2_cu *cu) On irc, Joel said that he thought that this was perhaps no longer needed. Could you comment? Paul> + _("Suspicious DW_AT_byte_size value treated as zero instead of 0x%llx"), Paul> + (long long) DW_UNSND (attr)); Are %ll and `long long' really portable? I think you need something else here. There are a few cases. [check_typedef] Paul> - const int low_bound = TYPE_LOW_BOUND (range_type); Paul> - const int high_bound = TYPE_HIGH_BOUND (range_type); Paul> + const LONGEST low_bound = TYPE_LOW_BOUND (range_type); Paul> + const LONGEST high_bound = TYPE_HIGH_BOUND (range_type); Paul> int nb_elements; Paul> if (high_bound < low_bound) Paul> nb_elements = 0; Paul> else Paul> - nb_elements = high_bound - low_bound + 1; Paul> + nb_elements = (int) (high_bound - low_bound + 1); Can't this overflow? There's a subsequent multiplication, too... I don't know what would be best to do here. Changing TYPE_LENGTH to a LONGEST seems tough to swallow, but so does throwing an error. Also, what does this code mean if either the high or low bound is undefined? Maybe those cases should choose the min or max of the underlying integral type? Paul> + struct range_bounds { Brace on its own line. Paul> + TYPE_RANGE_DATA (tp) = ((struct range_bounds *) Paul> + TYPE_ALLOC (tp, sizeof (struct range_bounds))); Paul> + memset (TYPE_RANGE_DATA (tp), 0, sizeof (struct range_bounds)); Use TYPE_ZALLOC instead. thanks, Tom