From patchwork Tue Feb 18 21:14:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 38216 Received: (qmail 64833 invoked by alias); 18 Feb 2020 21:14:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 64819 invoked by uid 89); 18 Feb 2020 21:14:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.7 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: EUR02-AM5-obe.outbound.protection.outlook.com Received: from mail-oln040092067037.outbound.protection.outlook.com (HELO EUR02-AM5-obe.outbound.protection.outlook.com) (40.92.67.37) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Feb 2020 21:14:34 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=b7nqpALaoM8OHLdilf55QFbLJ4Ml8GSvzP4GoQjZ0WeECfDXUjZfXIBu24WNU0ucqND9+m3W5ejN/juFQGPKnw/fvfliFIo3rPWMLPoAZM+bAif4KiR1lPKLN4ZKLJZ2BEdHav37PXgynxbJzRHrTyJL9wK0bVg6nsGV8OQHdv9+hsDXPgS3ypF6xp2htC8QkXDvGlM2z/6OKHOIyJ+5jXM0/4F6AYNL7JFsxB0KD3wzusMl3JE/yB3PAuWOhono3b5mcFlQGmaDUX+iR+y2gNclJj58nr1sg4jMIKCR3chFzEVByGlefqZyyGTF2nhrrpdkIT77tHrtRZKn7qszhA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Czz7a27pkAg/jcS9jcELCjh/SvFKO3bJR43VwWTUfpI=; b=fWOgqgztVhlnQjDXPmujV5o1cjhWdcJjpPX659REvESeLKb+giU4ib3yUdmfFD0D7uqJl4UHaW4jzU5PMINhQht9L+ChaZynmXGkfz8qEvJkdktTT/fTPERbhRvGtP/K2Y5tBx3+nGAXbu53oR9aEsaiPRVxriK8wRkRW73MHGZbDH+ZhZqsbRvFLz9fDnWrbH4jHGu51a6kg8BFuQYPOkCjSKl8XaHgVeapxjo5L1qovuxcX/DLO9qgjvAxTrJ73ouqCENELlmjJ/ow0SVaBPdI4eZEbGsd7k0TdLMdLLOAfJIwtABlLWANj32a/VoW5pNX+jmeYBWIIvXhuy5h+w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from AM5EUR02FT051.eop-EUR02.prod.protection.outlook.com (10.152.8.57) by AM5EUR02HT091.eop-EUR02.prod.protection.outlook.com (10.152.9.129) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2729.22; Tue, 18 Feb 2020 21:14:30 +0000 Received: from AM6PR03MB5170.eurprd03.prod.outlook.com (10.152.8.58) by AM5EUR02FT051.mail.protection.outlook.com (10.152.9.5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2729.22 via Frontend Transport; Tue, 18 Feb 2020 21:14:30 +0000 Received: from AM6PR03MB5170.eurprd03.prod.outlook.com ([fe80::1956:d274:cab3:b4dd]) by AM6PR03MB5170.eurprd03.prod.outlook.com ([fe80::1956:d274:cab3:b4dd%6]) with mapi id 15.20.2729.032; Tue, 18 Feb 2020 21:14:30 +0000 Received: from [192.168.1.101] (92.77.140.102) by ZR0P278CA0046.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:1d::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2729.23 via Frontend Transport; Tue, 18 Feb 2020 21:14:30 +0000 From: Bernd Edlinger To: Simon Marchi , "gdb-patches@sourceware.org" Subject: Re: [PATCH ?] Fix gdb build with gcc-4.8.x Date: Tue, 18 Feb 2020 21:14:30 +0000 Message-ID: References: <317b68d6-df81-d8e2-6e81-eb2903434ed4@simark.ca> In-Reply-To: <317b68d6-df81-d8e2-6e81-eb2903434ed4@simark.ca> x-microsoft-original-message-id: <314d2011-cb92-6732-8807-1350f861e076@hotmail.de> x-ms-exchange-antispam-messagedata: Ri/kiP+qlcXFrRZYPVJuWcKzNE+VP4jZZeeVbjUfFI8oorxvz5xW3S+ZivdA6dOIGR8YvJBdgbLkSsTDxsQdnAw8c0uDDrXJq/kN3zSsQeEOSPOESxmTI4z3VTF08DDXrE0H/6mK0XP7h3PJ/wCyog== x-ms-exchange-transport-forked: True MIME-Version: 1.0 On 2/18/20 9:27 PM, Simon Marchi wrote: > On 2020-02-18 2:06 p.m., Bernd Edlinger wrote: >> Hi, >> >> I noticed that gdb cannot be built any more with gcc-4.8.4 >> since Simon's patch which introduced the >> std::unique_ptr. >> >> The failure mode is as follows: >> >> CXX amd64-tdep.o >> ../../binutils-gdb/gdb/amd64-tdep.c: In function ‘displaced_step_closure_up amd64_displaced_step_copy_insn(gdbarch*, CORE_ADDR, CORE_ADDR, regcache*)’: >> ../../binutils-gdb/gdb/amd64-tdep.c:1514:10: error: cannot bind ‘std::unique_ptr’ lvalue to ‘std::unique_ptr&&’ >> return dsc; >> ^ >> In file included from /usr/include/c++/4.8/memory:81:0, >> from ../../binutils-gdb/gdb/../gdbsupport/common-exceptions.h:25, >> from ../../binutils-gdb/gdb/../gdbsupport/common-defs.h:140, >> from ../../binutils-gdb/gdb/defs.h:28, >> from ../../binutils-gdb/gdb/amd64-tdep.c:22: >> /usr/include/c++/4.8/bits/unique_ptr.h:169:2: error: initializing argument 1 of ‘std::unique_ptr<_Tp, _Dp>::unique_ptr(std::unique_ptr<_Up, _Ep>&&) [with _Up = amd64_displaced_step_closure; _Ep = std::default_delete; = void; _Tp = displaced_step_closure; _Dp = std::default_delete]’ >> unique_ptr(unique_ptr<_Up, _Ep>&& __u) noexcept >> ^ >> ../../binutils-gdb/gdb/amd64-tdep.c:1515:1: error: control reaches end of non-void function [-Werror=return-type] >> } >> ^ >> cc1plus: all warnings being treated as errors >> >> >> It continues to work with gcc-5.4.0, though. I don't know what >> is with gcc-4.9.x. >> >> I have two possible workarounds for this attached to this as >> variant-1 patch and variant-2 patch respectively. I personally >> would prefer variant-2 which makes displaced_step_closure_up a >> wrapper class around unique_ptr and >> avoids to trigger this compiler bug by being slightly simpler as >> the original, I think the issue always starts when the argument >> to displaced_step_closure_up (std::unique_ptr &up) is >> using double-ampersand. >> >> So we have three possible ways to deal with this: >> variant-1: simplify the code where the type cast happens, >> variant-2: use a simplified wrapper clase, and >> variant-3: do nothing about it, and document that gcc-5.4.0 is >> or newer is required. >> >> What do you think? >> >> >> Thanks >> Bernd. >> > > Hi Bernd, > > I don't like variant 2, because it changes the API/contract of > std::unique_ptr. It allows doing > > std::unique_ptr dsc; > displaced_step_closure_up hello (dsc); > > Which would not be possible if displaced_step_closure_up was > a simple typedef. In our code base, the types that end with _up > are known to be typedefs to std::unique_ptr, and I don't think it > would be a good idea to provide such a type with a semantic that > differs from std::unique_ptr. > > So I would prefer something along the lines of variant 1, but with > a small comment at each site saying that this is to work around > a problem with g++ 4.8. > Okay, works for me, so then I added just one short comment at each site. Is it OK for trunk? Thanks Bernd. From 43e6c14091dee9c32fe1dee62466450e57f6c13a Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 16 Feb 2020 21:43:33 +0100 Subject: [PATCH] Fix build with gcc-4.8.x Use an explicit conversion from unique_ptr to displaced_step_closure_up to avoid a compiler bug with gcc-4.8.4: ../../binutils-gdb/gdb/amd64-tdep.c:1514:10: error: cannot bind 'std::unique_ptr' lvalue to 'std::unique_ptr&&' gdb: 2020-02-16 Bernd Edlinger * aarch64-tdep.c (aarch64_displaced_step_copy_insn): Use an explicit conversion. * amd64-tdep.c (amd64_displaced_step_copy_insn): Likewise. * arm-linux-tdep.c (arm_linux_displaced_step_copy_insn): Likewise. * i386-tdep.c (i386_displaced_step_copy_insn): Likewise. * rs6000-tdep.c (ppc_displaced_step_copy_insn): Likewise. * s390-tdep.c (s390_displaced_step_copy_insn): Likewise. --- gdb/aarch64-tdep.c | 3 ++- gdb/amd64-tdep.c | 3 ++- gdb/arm-linux-tdep.c | 3 ++- gdb/i386-tdep.c | 3 ++- gdb/rs6000-tdep.c | 3 ++- gdb/s390-tdep.c | 3 ++- 6 files changed, 12 insertions(+), 6 deletions(-) diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index bfacfb0..31b90c8 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -3053,7 +3053,8 @@ aarch64_displaced_step_copy_insn (struct gdbarch *gdbarch, dsc = NULL; } - return dsc; + /* This is a work around for a problem with g++ 4.8. */ + return displaced_step_closure_up (dsc.release ()); } /* Implement the "displaced_step_fixup" gdbarch method. */ diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 9264fe4..5c56a97 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -1511,7 +1511,8 @@ amd64_displaced_step_copy_insn (struct gdbarch *gdbarch, displaced_step_dump_bytes (gdb_stdlog, buf, len); } - return dsc; + /* This is a work around for a problem with g++ 4.8. */ + return displaced_step_closure_up (dsc.release ()); } static int diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c index ccb556b..f60cb51 100644 --- a/gdb/arm-linux-tdep.c +++ b/gdb/arm-linux-tdep.c @@ -1131,7 +1131,8 @@ arm_linux_displaced_step_copy_insn (struct gdbarch *gdbarch, arm_displaced_init_closure (gdbarch, from, to, dsc.get ()); - return dsc; + /* This is a work around for a problem with g++ 4.8. */ + return displaced_step_closure_up (dsc.release ()); } /* Implementation of `gdbarch_stap_is_single_operand', as defined in diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 9771421..19876c3 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -831,7 +831,8 @@ i386_displaced_step_copy_insn (struct gdbarch *gdbarch, displaced_step_dump_bytes (gdb_stdlog, buf, len); } - return closure; + /* This is a work around for a problem with g++ 4.8. */ + return displaced_step_closure_up (closure.release ()); } /* Fix up the state of registers and memory after having single-stepped diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 513ce6a..2c41e1c 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -894,7 +894,8 @@ ppc_displaced_step_copy_insn (struct gdbarch *gdbarch, displaced_step_dump_bytes (gdb_stdlog, buf, len); } - return closure; + /* This is a work around for a problem with g++ 4.8. */ + return displaced_step_closure_up (closure.release ()); } /* Fix up the state of registers and memory after having single-stepped diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c index 51d0203..d8c28c7 100644 --- a/gdb/s390-tdep.c +++ b/gdb/s390-tdep.c @@ -477,7 +477,8 @@ s390_displaced_step_copy_insn (struct gdbarch *gdbarch, displaced_step_dump_bytes (gdb_stdlog, buf, len); } - return closure; + /* This is a work around for a problem with g++ 4.8. */ + return displaced_step_closure_up (closure.release ()); } /* Fix up the state of registers and memory after having single-stepped -- 1.9.1