From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27632 invoked by alias); 5 Dec 2009 12:18:10 -0000 Received: (qmail 27623 invoked by uid 22791); 5 Dec 2009 12:18:09 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 05 Dec 2009 12:18:02 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 207402BAB9A; Sat, 5 Dec 2009 07:18:01 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id SUbqnXGjlRAR; Sat, 5 Dec 2009 07:18:01 -0500 (EST) Received: from nile.gnat.com (nile.gnat.com [205.232.38.5]) by rock.gnat.com (Postfix) with ESMTP id E7C442BAB82; Sat, 5 Dec 2009 07:18:00 -0500 (EST) Received: by nile.gnat.com (Postfix, from userid 1345) id DB060227B32; Sat, 5 Dec 2009 07:18:00 -0500 (EST) From: Paul Hilfinger To: tromey@redhat.com CC: brobecker@adacore.com, gdb-patches@sourceware.org In-reply-to: (message from Tom Tromey on Fri, 04 Dec 2009 11:34:31 -0700) Subject: Re: [RFA] 64-bit range types in GDB Reply-to: Hilfinger@adacore.com References: <20091204081343.A8718227B32@nile.gnat.com> Message-Id: <20091205121800.DB060227B32@nile.gnat.com> Date: Sat, 05 Dec 2009 12:18:00 -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 X-SW-Source: 2009-12/txt/msg00068.txt.bz2 Tom, Thanks for the (blindingly) fast response. Is Joel blackmailing you or something? I could have a word with him if so. > 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? In fact, at one point I HAD removed this function, but then discovered to my annoyance that it was still needed somewhere. I'll have to go back and reconstruct (from some months back) just what the problem was and whether it has become OBE. > 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. Yeah, it seems that long long either appears in system-specific code (like linux-nat) where one can probably assume its existence, or conditionalized as in printcmd.c. > > [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. > Hmm. Sticky. There's this comment on 'length' in struct type: /* Length of storage for a value of this type. This is what sizeof(type) would return; use it for address arithmetic, memory reads and writes, etc. .... But then the code goes on to define 'unsigned length'. However, according to the C standard, the type should really be size_t---and technically that should be the TARGET's size_t. I suspect this part of the representation hasn't been updated since the days when people actually used 16-bit address spaces. What do you suppose people would say to using size_t here at least? In any case, you are correct that there ought to be some provision for overflow here. > 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? That also will require some thought. Some undefined bounds seem to be carelessness on the part of the compiler, and I suspect that returning a null range might be safer. > 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. OK and OK. Thanks for the input. I should be submitting a revision soon. Paul