From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64147 invoked by alias); 22 May 2019 19:37:06 -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 64139 invoked by uid 89); 22 May 2019 19:37:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=baton, business, yours, placed X-HELO: gateway24.websitewelcome.com Received: from gateway24.websitewelcome.com (HELO gateway24.websitewelcome.com) (192.185.51.162) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 May 2019 19:37:04 +0000 Received: from cm11.websitewelcome.com (cm11.websitewelcome.com [100.42.49.5]) by gateway24.websitewelcome.com (Postfix) with ESMTP id 912811EAD7 for ; Wed, 22 May 2019 14:19:02 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id TWlShBNE6dnCeTWlShrThb; Wed, 22 May 2019 14:19:02 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=y2pFUQmNc59I0xs2/wzKVoIJ+9cMBl/Zdu4vhhDtDOE=; b=E/BY5xzqSuFIHFqkkNRFcTeDim XlRp5ow2W+zh/4oC6+3QJe+W9/pwZWiiTTGgQ6KQwJ57e3hhR98zsqd77cYmN42jh31d+kaETJX5N pGHiRbb0zqx6IxR3gEeSYDKBP; Received: from nat.gnat.com ([205.232.38.191]:45382 helo=murgatroyd) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1hTWlS-003zCL-Cz; Wed, 22 May 2019 14:19:02 -0500 From: Tom Tromey To: Andrew Burgess Cc: gdb-patches@sourceware.org Subject: Re: [PATCHv2 5/5] gdb: Better support for dynamic properties with negative values References: <672dc208d6dfb9f068eb635e02bf85abb9630c74.1557439866.git.andrew.burgess@embecosm.com> Date: Wed, 22 May 2019 19:37:00 -0000 In-Reply-To: <672dc208d6dfb9f068eb635e02bf85abb9630c74.1557439866.git.andrew.burgess@embecosm.com> (Andrew Burgess's message of "Thu, 9 May 2019 23:22:16 +0100") Message-ID: <87lfyycn6i.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2019-05/txt/msg00499.txt.bz2 >>>>> "Andrew" == Andrew Burgess writes: Andrew> When the type of a property is smaller than the CORE_ADDR in which the Andrew> property value has been placed, and if the property is signed, then Andrew> sign extend the property value from its actual type up to the size of Andrew> CORE_ADDR. I wonder whether this should be using ULONGEST rather than CORE_ADDR now. Andrew> + /* If we have a valid return candidate and it's value Andrew> + is signed, we have to sign-extend the value because Andrew> + CORE_ADDR on 64bit machine has 8 bytes but address Andrew> + size of an 32bit application is bytes. */ Andrew> + const int addr_size Andrew> + = (dwarf2_per_cu_addr_size (baton->locexpr.per_cu) Andrew> + * TARGET_CHAR_BIT); I'm somewhat surprised there isn't an existing sign_extend function somewhere. Andrew> + const CORE_ADDR neg_mask = ((~0) << (addr_size - 1)); I tend to think this should say ~(CORE_ADDR) 0 rather than just ~0. Otherwise won't this do the wrong thing if sizeof(int) < sizeof(CORE_ADDR)? Andrew> + /* Check if signed bit is set and sign-extend values. */ Andrew> + if (*value & (neg_mask)) Andrew> + *value |= (neg_mask ); Extra parens and an extra space here. mips-tdep.c has the fun sign extension idiom: value = inst & 0x7ff; value = (value ^ 0x400) - 0x400; /* Sign-extend. */ (If the high bits are 0 you can skip the "&".) This one avoids the ~0 business. Another approach is to let the compiler handle it. LONGEST l = *value; l = (l << nbits) >> nbits; However this may be relying on undefined behavior. Yours is fine, though, I just noticed these while digging around and wanted to point them out :-) Tom