From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55456 invoked by alias); 15 Jan 2018 16:51:19 -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 52527 invoked by uid 89); 15 Jan 2018 16:50:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=metal, wish X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 15 Jan 2018 16:49:17 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AF180C047B62; Mon, 15 Jan 2018 16:48:56 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0371E6047A; Mon, 15 Jan 2018 16:48:55 +0000 (UTC) Subject: Re: [RFA] Fix scm-ports.exp regression To: Tom Tromey References: <20180103182048.8495-1-tom@tromey.com> <37ddb96c-2b32-2fcd-9c36-17f32d8701e3@redhat.com> <87efmykhw1.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Mon, 15 Jan 2018 16:51:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <87efmykhw1.fsf@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-01/txt/msg00275.txt.bz2 Hi Tromey, Sorry for the delay. On 01/09/2018 06:26 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves writes: > >>> I think the simplest fix is to use "print/u" rather than "print/d" to >>> get the value of sp_reg in the test case. > > Pedro> Can you expand a bit on this rationale, please? > > Pedro> There's: > Pedro> (parse-and-eval \"*(char*) \$sp\") > Pedro> in the context of the diff. Is that related? I ask because > Pedro> that "char" in there would look like something that could print > Pedro> as signed or unsigned depending on target. > > I don't think that is related. That expression has a dereference. > > What happens is that on x86, this: > > set sp_reg [get_integer_valueof "\$sp" 0] > > ... ends up setting sp_reg to a negative value, because > get_integer_valueof uses "print/d": > > print /d $sp > $1 = -11496 > > Then later the test suite does: > > gdb_test "guile (print (seek rw-mem-port (value->integer sp-reg) SEEK_SET))" \ > "= $sp_reg" \ > "seek to \$sp" > > ... expecting this value to be identical to the saved $sp_reg value. > However it gets: > > guile (print (seek rw-mem-port (value->integer sp-reg) SEEK_SET)) > = 4294955800 > > "print" is just a wrapper for guile's format: > > gdb_test_no_output "guile (define (print x) (format #t \"= ~A\" x) (newline))" > > The seek function returns a scm_t_off, so I would think that this sort > of printing is handled by guile, not by gdb. I see. So seemingly this is printing a scm_t_off, which seems to be a signed 64-bit integer: /usr/include/guile/2.0/libguile/scmconfig-32.h:82:typedef int64_t scm_t_int64; /usr/include/guile/2.0/libguile/scmconfig-32.h:119:typedef scm_t_int64 scm_t_off; /usr/include/guile/2.0/libguile/scmconfig-64.h:82:typedef int64_t scm_t_int64; /usr/include/guile/2.0/libguile/scmconfig-64.h:119:typedef scm_t_int64 scm_t_off; while $sp is 32-bit, and we're extracting it as a 32-bit signed integer (into $sp_reg). Here: (seek rw-mem-port (value->integer sp-reg) SEEK_SET) "sp-reg" is a pointer, and value->integer takes us to gdbscm_value_to_integer [I think], which converts the pointer to an unsigned integer, AFAICT, and then probably that gets cast/converted to scm_t_off when passed to guile's "seek", somewhere. And then 'seek' returns the same offset out, as an scm_t_off, and then guile's 'format' prints that. So pedantically, doing: "print (scm_t_off) $sp" (or really "print (int64_t) $sp", or even "print (long long) $sp"...) to extract $sp_reg would be a little more to the point, I guess. But it looks like on 64-bit archs, the API can't access memory addresses with the high bit set anyway (?) (not sure how to get those; maybe debugging some bare metal/kernel code), so the difference doesn't really matter much in practice. The patch is fine with me as is. I just wish the commit log were a little clearer with details such as the above. > IIRC what happened is that "print/d" slightly changed in some cases > during the scalar printing work, and what we're seeing is the result. Yes, before the rework, "/d" would still print integers as unsigned in some cases. Now it always prints them as signed, as if it you wrote something like this: (gdb) print (std::make_signed::type) EXPR instead of: (gdb) print /d EXPR with the difference that /d affects display only, unlike a cast which affects the actual value recorded in the value history. Thanks, Pedro Alves