From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id b/rUC6L+omoojz0AWB0awg (envelope-from ) for ; Thu, 10 Sep 2026 15:01:54 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; secure) header.d=adacore.com header.i=@adacore.com header.a=rsa-sha256 header.s=google header.b=iHPXSZ/S; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 1BBDB1E09E; Thu, 10 Sep 2026 15:01:54 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 47EA71E033 for ; Thu, 10 Sep 2026 15:01:53 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id F3A3C48FDEC6 for ; Thu, 10 Sep 2026 19:01:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F3A3C48FDEC6 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, secure) header.d=adacore.com header.i=@adacore.com header.a=rsa-sha256 header.s=google header.b=iHPXSZ/S Received: from mail-ot1-x32f.google.com (mail-ot1-x32f.google.com [IPv6:2607:f8b0:4864:20::32f]) by sourceware.org (Postfix) with ESMTPS id 48D8B48FBC8C for ; Thu, 10 Sep 2026 19:01:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 48D8B48FBC8C Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 48D8B48FBC8C Authentication-Results: sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::32f ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789066866; cv=none; b=XK0PmrUQ5Xrl6zsLJRXpJH+vsyiFlj38KkMOKa1XKc9Ih9mbO0hZoXruUgNuEXlJ7OcRiKipmhbFmYoArj5LDldNPa90Z9iwAxWKKqCSMSyVscByRKZe3vwaWOmWXSF7l8Fg88qmyQYQjyWeSNnGMcUPzXMD/c5YjYwK19k2rW8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789066866; c=relaxed/simple; bh=9soqJ4Z3lFfSANHgS/qkCB/ngV/dPiOzA/0HuA0QliA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=gioZmR0v4rF09H5PfaehkZ3rdbW+QbIPPUoNoTFnl2R6BjWxJ0sgvtolSTh8aYDvqVhbFS+VxTT4XD0/c3MQKISW9Bg/kpmHpHEbZsUvGGX/2lA18d+lBeCes0ZXgcPV0byYLUmKAjUgojF856p4dK6sBg+GItfX43pnbrTIVHM= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (2048-bit key, secure) header.d=adacore.com header.i=@adacore.com header.a=rsa-sha256 header.s=google header.b=iHPXSZ/S DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 48D8B48FBC8C Received: by mail-ot1-x32f.google.com with SMTP id 46e09a7af769-7f4f53975e6so101705a34.3 for ; Thu, 10 Sep 2026 12:01:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1789066865; x=1789671665; darn=sourceware.org; h=content-type:mime-version:user-agent:message-id:date:references :in-reply-to:subject:cc:to:from:from:to:cc:subject:date:message-id :reply-to:content-type; bh=MbC+9Di/Z6xEe8jn6TDxaqAFF60kylIBfnO0Zz8eCtg=; b=iHPXSZ/SH/4rkA5WOD5HB1OMcvifPrH8ZNDbvbTSYw6Ckwoo7QKipzkukNQL7pnGXa zaAycdZTgM6QabFCasDviSS+FdClTH8kiNWghhezhVbCjJ2b5A13N8h1Gw05V7GWZDvE rQfHiKk7m+2td0hdITsAYX/xm9urWpGQs5dzy7f4IuwVjCcwtkPqjwL+LP67k9wiD6Tj pyMsdn62eLyXvKiHp7bYUjz7wtys7J7pDgjQwbiduNRU8a4tZRPjr+tG16iifeI4FRRN seA29/1avWsoGyp8VwzjfZienjZw61w+Vo6MyVvv4uGeiyCSiU0FQarlJD/rfWfWalSu gaCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1789066865; x=1789671665; h=content-type:mime-version:user-agent:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to:content-type; bh=MbC+9Di/Z6xEe8jn6TDxaqAFF60kylIBfnO0Zz8eCtg=; b=TJLqpL4OUJ7dozy9iFYQ7TCdvAICOeGwsdsm8W/SLUTe85OehH2RnUdIlX2hT+iosV cqyF6EQD+epe6ljtMXX1RXTnRwkZwG691RCywd1pOvIkHzHkf0BHgts5YFjNVSWSB5sK 94KhwX4Iqa3QQtVve3tYgBs+vuunH0EBXC04sJ+wd3dRPZho5WrRNVt0MlvEUGwywHS9 GYM3dBIdDWy+syktpc8bpbmbLfrCCkddkydfSgE4QeRLeH3W7DB2L9ToNCr3cgL9t162 9fAyD6zyP7iNsTzUt3Eto67H6NitK1IlgcBxJWK16mDf3zWDmF1nZ8fvAeUnQs4CSAGP o1xg== X-Forwarded-Encrypted: i=1; AKwUvBw+/YgiByLJCe+pY5PHUsAAdWULBSAa3IqTE3WfSjxAJr7ORJLJMlTyq6DRQmdzIU9NgUWazJ0NLuCqeA==@sourceware.org X-Gm-Message-State: AFuF++kl3VwPIZQaE3XIR+D5A32esXcTXt71xSplMqYu8raQj27vdkH0 5W48SsQ7LFX35MgXZ2vmwwuknN0yQaGhDZbM9yonEbSzWDAeMBIXRO9pa45ODzprJw== X-Gm-Gg: AYBFou25e3Z94LweJNYnWXt3kq50752ZPtX1gh3tQGm4031farNPOitLMIT9t5HHL48 R9mbaSQB4khQBb1bCCevBHOxHE5uxHeXja72RXDKoXvHna0Ub74FpBkqjHE+t3AGUwxehf3UElD p0ZW4qsoZScFJVWdqB9DOaQoW06OqK93+FL40l2Lc7GIXb5w1u14YNU6aWxezzurrrefQhILzAH NHAhW+cUmr8vns8dY3wEMEahO37yjOwmRjJTbXVNR4fSzEKoYf4M6kt5b+AtpdA/zb0m2dezfAb Q0/R8Hr4+Wqpra2QKl1Wt0ufia3Tg8SpHlzXcX+W0QT/atBnF8TZnnNwEfv7xe+WHbdVfMXRWrw arNActpBQfQy/8SppRWi3nkmrwL3u1T9hZYxs8OdwCHPrXJ+3AAWQjuCuaGzb5zY833fAU7q3Ty gop6GngeDk9n+XH8gHYMZNY5qiLt9ARbgf1be4cPGcJ4xrkd4K6AaJ/hTIY6S88Cjfgfd2/MggK VCs+XrugwrKE6ek X-Received: by 2002:a05:6830:4888:b0:7fa:ac4f:79e with SMTP id 46e09a7af769-8040166356amr135936a34.26.1789066863831; Thu, 10 Sep 2026 12:01:03 -0700 (PDT) Received: from bapiya (97-122-117-2.hlrn.qwest.net. [97.122.117.2]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-803f66fda4esm302286a34.16.2026.09.10.12.01.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Sep 2026 12:01:03 -0700 (PDT) From: Tom Tromey To: Tom de Vries Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH] Rewrite gdb_mpz::export_bits In-Reply-To: (Tom de Vries's message of "Thu, 10 Sep 2026 16:41:16 +0200") References: <20260909180822.2255847-1-tromey@adacore.com> X-Attribution: Tom Date: Thu, 10 Sep 2026 13:01:02 -0600 Message-ID: <875x0d0w41.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Tom> If so, I prefer "truncated" to "masked". I changed the name. >> + if (sign < 0 && masked.sgn () != 0) Tom> I think it's clearer to do this, because there's less state to keep Tom> going forward: Tom> ... Tom> if (masked.sgn () == 0) Tom> { Tom> memset (buf.data (), 0, buf.size ()); Tom> return; Tom> } Tom> if (sign < 0) Tom> ... Tom> This starts to get verbose, but after factoring out some lambda functions: I don't like lambda functions in a situation like this. IMO they often just obfuscate the code. Instead I rearranged the code a bit in v2. This makes it a little simpler. Tom commit b107ae9e505d14a46211dd6832b69b8bd70fc16b Author: Tom Tromey Date: Wed Sep 9 11:22:04 2026 -0600 Rewrite gdb_mpz::export_bits A couple of bugs point out that, when multiplication overflows, gdb does not compute the correct result. This is caused by some bugs in gdb_mpz::export_bits. This patch rewrites part of this function, hopefully now getting the correct answer. I think the new code should be somewhat simpler to follow. A new selftest is added, derived from the code in the bug report. This rewrite doesn't try to minimize allocations, the way the previous one did. I tend to doubt that matters, and this is one of the readability improvements IMO. Regression tested on x86-64 Fedora 43. I am not sure but it might be worth applying this to gdb 18; your thoughts appreciated. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34601 diff --git a/gdb/gmp-utils.c b/gdb/gmp-utils.c index b7fed9a82d1..21f474798eb 100644 --- a/gdb/gmp-utils.c +++ b/gdb/gmp-utils.c @@ -128,38 +128,24 @@ gdb_mpz::export_bits (gdb::array_view buf, int endian, bool unsigned_p hi.str ().c_str ()); } - const gdb_mpz *exported_val = this; - gdb_mpz un_signed; - if (sign < 0) - { - /* mpz_export does not handle signed values, so create a positive - value whose bit representation as an unsigned of the same length - would be the same as our negative value. */ - gdb_mpz neg_offset = gdb_mpz::pow (2, buf.size () * HOST_CHAR_BIT); - un_signed = *exported_val + neg_offset; - exported_val = &un_signed; - } + gdb_mpz truncated = *this; + truncated.mask (buf.size () * HOST_CHAR_BIT); - /* If the value is too large, truncate it. */ - if (!safe - && mpz_sizeinbase (exported_val->m_val, 2) > buf.size () * HOST_CHAR_BIT) + /* It's possible that the above results in zero, which has to be + handled specially. */ + if (truncated.sgn () == 0) { - /* If we don't already have a copy, make it now. */ - if (exported_val != &un_signed) - { - un_signed = *exported_val; - exported_val = &un_signed; - } - - un_signed.mask (buf.size () * HOST_CHAR_BIT); + memset (buf.data (), 0, buf.size ()); + return; } - /* It's possible that one of the above results in zero, which has to - be handled specially. */ - if (exported_val->sgn () == 0) + if (sign < 0) { - memset (buf.data (), 0, buf.size ()); - return; + /* mpz_export does not handle signed values, so create a + positive value whose bit representation as an unsigned of the + same length would be the same as our negative value. */ + gdb_mpz neg_offset = gdb_mpz::pow (2, buf.size () * HOST_CHAR_BIT); + truncated += neg_offset; } /* Do the export into a buffer allocated by GMP itself; that way, @@ -174,8 +160,9 @@ gdb_mpz::export_bits (gdb::array_view buf, int endian, bool unsigned_p size_t word_countp; gdb::unique_xmalloc_ptr exported - (mpz_export (NULL, &word_countp, -1 /* order */, buf.size () /* size */, - endian, 0 /* nails */, exported_val->m_val)); + (mpz_export (nullptr, &word_countp, -1 /* order */, + buf.size () /* size */, endian, 0 /* nails */, + truncated.m_val)); gdb_assert (word_countp == 1); diff --git a/gdb/unittests/gmp-utils-selftests.c b/gdb/unittests/gmp-utils-selftests.c index 1912d346c17..417abae5463 100644 --- a/gdb/unittests/gmp-utils-selftests.c +++ b/gdb/unittests/gmp-utils-selftests.c @@ -80,6 +80,12 @@ gdb_mpz_as_integer () v -= 1; SELF_CHECK (v.as_integer () == ul_expected); + + /* This is from PR gdb/34601. */ + LONGEST neg = (LONGEST) 0x8000000000000001ull; + gdb_mpz a (neg); + gdb_mpz b (0x1234); + SELF_CHECK ((a * b).as_integer_truncate () == 0x1234); } /* A helper function which calls the given gdb_mpz object's as_integer