From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by sourceware.org (Postfix) with ESMTPS id 559F03857C6F for ; Thu, 8 Oct 2020 15:18:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 559F03857C6F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x330.google.com with SMTP id d81so6861636wmc.1 for ; Thu, 08 Oct 2020 08:18:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=LumuNvSghH+MmPvUqoSXk5ErwRKRTL2fKI6JFAdtrL0=; b=Q51cZvRGQ1bMy1R556vaGCKkYNUhLd+PE4sTN/rZGrvEuS+JOWVOEO8qUw4viXUVdD W3Vx38/yA1YLdy1XpJQltIwFwYVK1/EB2SPOa63e2w/924U6WpazyPpxWSpJeCX8TvuA KgtpZnvfp8ctITxiauwT4wQqK1jqIJF3Fnt8JxsEfNkHL4uxL1iLQ8P3YpwYL/buFyO1 bm3ak5GG0i3W9lDM/0TB+TDjH/O7pbik1W25PQY2+JzcS27cdX5RPQe3HYKsFd8fD5Lx Ss0PsLsJ5Uh3l4EW5FiqAA6PL04Ypzf8XudUIrP7ZO3+JAp5usnpeLaCsAsvudxDGCkE XvSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=LumuNvSghH+MmPvUqoSXk5ErwRKRTL2fKI6JFAdtrL0=; b=tHGLFEPme1Z+HhzwFt9Lo3YH9c0njsi/N1kPQ/mEHVxFg4ZZlaWnyuJ2Jc53tT4ze4 wbFnjvqt+z5Slk18WHSZITQxPzCt763f683/59QIAI2qU51mIH/AnwGWdOSY/SS8J7ys 3sMjO3ovh+wqyJHaqu9DGWXmA5Ir8BkPWnP4c733teHTziU2p5uvKFAiQ47+tabsm1pG FkGf6BwcqGfvAmZl/VAzk8tGsf5XIF69II4vT+VEafT+wn3NMOBxWhzdXQoacaqU6/4K ZN9pihpNxiS2AEhlxed1socVy9hB0zD67E+72L9fimm/aPMNL9wOAGHSTg+GQvbj/+G3 SAKg== X-Gm-Message-State: AOAM531uJanD4XjCkQd6GkwZ/WrUmFO9/R/0GwgJ+ikv2H+2KxFcIffJ kyEvPCO4rjteVU2Thk2eY/4JfyOfDTRjmg== X-Google-Smtp-Source: ABdhPJycczEKcImmv/+sLlnL6h3zYb6gT1TjmkU/9DAZ/bcqxFk2qO7vMewvoRFEWoTzoDJHfb/kiA== X-Received: by 2002:a1c:2b05:: with SMTP id r5mr9334717wmr.179.1602170282817; Thu, 08 Oct 2020 08:18:02 -0700 (PDT) Received: from localhost (host109-148-134-240.range109-148.btcentralplus.com. [109.148.134.240]) by smtp.gmail.com with ESMTPSA id j1sm8129109wrc.28.2020.10.08.08.18.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Oct 2020 08:18:01 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org, Shahab Vahedi Subject: [PATCH] gdb: Delay releasing target_desc_up in more cases Date: Thu, 8 Oct 2020 16:17:58 +0100 Message-Id: <20201008151758.2116169-1-andrew.burgess@embecosm.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20201008092854.GM605036@embecosm.com> References: <20201008092854.GM605036@embecosm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Oct 2020 15:18:14 -0000 This fixes up the places that I missed in the previous commit. Shahab, this makes a minor change to the ARC gdbserver that I have not tested. It's a trivial change so I'm 99.9% sure it's fine... if you had time to apply this and rebuild just to confirm it's all good that would be awesome. Thanks, Andrew --- After commit: commit 51a948fdf0e14fb69ab9e0c79ae8b2415801f9a3 Date: Mon Jul 20 14:18:04 2020 +0100 gdb: Have allocate_target_description return a unique_ptr There were a few places where we could (should?) have delayed releasing the target_desc_up until a little later. This commit catches these cases. In the case of ARC, the target_desc_up is now exposed right out to gdbserver, which means making a small change there too. There should be no user visible changes after this commit. gdb/ChangeLog: * arch/aarch32.c (aarch32_create_target_description): Release the target_desc_up as late as possible. * arch/aarch64.c (aarch64_create_target_description): Likewise. * arch/amd64.c (amd64_create_target_description): Likewise. * arch/arc.c (arc_create_target_description): Return a target_desc_up, don't release it. (arc_lookup_target_description): Move target_desc_up into the cache, and return a borrowed pointer. * arch/arm.c (arm_create_target_description): Release the target_desc_up as late as possible. * arch/i386.c (i386_create_target_description): Likewise. * arch/tic6x.c (tic6x_create_target_description): Likewise. gdbserver/ChangeLog: * linux-arc-low.cc (arc_linux_read_description): Release the unique_ptr returned from arc_create_target_description. --- gdb/ChangeLog | 15 +++++++++++++++ gdb/arch/aarch32.c | 12 ++++++------ gdb/arch/aarch64.c | 14 +++++++------- gdb/arch/amd64.c | 27 ++++++++++++++------------- gdb/arch/arc.c | 25 ++++++++++++++----------- gdb/arch/arm.c | 16 ++++++++-------- gdb/arch/i386.c | 24 ++++++++++++------------ gdb/arch/tic6x.c | 14 +++++++------- gdbserver/ChangeLog | 5 +++++ gdbserver/linux-arc-low.cc | 6 +++--- 10 files changed, 91 insertions(+), 67 deletions(-) diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c index bf7a33230e0..0a869ec9b91 100644 --- a/gdb/arch/aarch32.c +++ b/gdb/arch/aarch32.c @@ -26,18 +26,18 @@ target_desc * aarch32_create_target_description () { - target_desc *tdesc = allocate_target_description ().release (); + target_desc_up tdesc = allocate_target_description (); #ifndef IN_PROCESS_AGENT - set_tdesc_architecture (tdesc, "arm"); + set_tdesc_architecture (tdesc.get (), "arm"); #endif long regnum = 0; - regnum = create_feature_arm_arm_core (tdesc, regnum); + regnum = create_feature_arm_arm_core (tdesc.get (), regnum); /* Create a vfpv3 feature, then a blank NEON feature. */ - regnum = create_feature_arm_arm_vfpv3 (tdesc, regnum); - tdesc_create_feature (tdesc, "org.gnu.gdb.arm.neon"); + regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum); + tdesc_create_feature (tdesc.get (), "org.gnu.gdb.arm.neon"); - return tdesc; + return tdesc.release (); } diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c index c0af7b0906f..f89d5e014df 100644 --- a/gdb/arch/aarch64.c +++ b/gdb/arch/aarch64.c @@ -29,23 +29,23 @@ target_desc * aarch64_create_target_description (uint64_t vq, bool pauth_p) { - target_desc *tdesc = allocate_target_description ().release (); + target_desc_up tdesc = allocate_target_description (); #ifndef IN_PROCESS_AGENT - set_tdesc_architecture (tdesc, "aarch64"); + set_tdesc_architecture (tdesc.get (), "aarch64"); #endif long regnum = 0; - regnum = create_feature_aarch64_core (tdesc, regnum); + regnum = create_feature_aarch64_core (tdesc.get (), regnum); if (vq == 0) - regnum = create_feature_aarch64_fpu (tdesc, regnum); + regnum = create_feature_aarch64_fpu (tdesc.get (), regnum); else - regnum = create_feature_aarch64_sve (tdesc, regnum, vq); + regnum = create_feature_aarch64_sve (tdesc.get (), regnum, vq); if (pauth_p) - regnum = create_feature_aarch64_pauth (tdesc, regnum); + regnum = create_feature_aarch64_pauth (tdesc.get (), regnum); - return tdesc; + return tdesc.release (); } diff --git a/gdb/arch/amd64.c b/gdb/arch/amd64.c index b11a4fdc0fc..60d997dbee6 100644 --- a/gdb/arch/amd64.c +++ b/gdb/arch/amd64.c @@ -40,39 +40,40 @@ target_desc * amd64_create_target_description (uint64_t xcr0, bool is_x32, bool is_linux, bool segments) { - target_desc *tdesc = allocate_target_description ().release (); + target_desc_up tdesc = allocate_target_description (); #ifndef IN_PROCESS_AGENT - set_tdesc_architecture (tdesc, is_x32 ? "i386:x64-32" : "i386:x86-64"); + set_tdesc_architecture (tdesc.get (), + is_x32 ? "i386:x64-32" : "i386:x86-64"); if (is_linux) - set_tdesc_osabi (tdesc, "GNU/Linux"); + set_tdesc_osabi (tdesc.get (), "GNU/Linux"); #endif long regnum = 0; if (is_x32) - regnum = create_feature_i386_x32_core (tdesc, regnum); + regnum = create_feature_i386_x32_core (tdesc.get (), regnum); else - regnum = create_feature_i386_64bit_core (tdesc, regnum); + regnum = create_feature_i386_64bit_core (tdesc.get (), regnum); - regnum = create_feature_i386_64bit_sse (tdesc, regnum); + regnum = create_feature_i386_64bit_sse (tdesc.get (), regnum); if (is_linux) - regnum = create_feature_i386_64bit_linux (tdesc, regnum); + regnum = create_feature_i386_64bit_linux (tdesc.get (), regnum); if (segments) - regnum = create_feature_i386_64bit_segments (tdesc, regnum); + regnum = create_feature_i386_64bit_segments (tdesc.get (), regnum); if (xcr0 & X86_XSTATE_AVX) - regnum = create_feature_i386_64bit_avx (tdesc, regnum); + regnum = create_feature_i386_64bit_avx (tdesc.get (), regnum); if ((xcr0 & X86_XSTATE_MPX) && !is_x32) - regnum = create_feature_i386_64bit_mpx (tdesc, regnum); + regnum = create_feature_i386_64bit_mpx (tdesc.get (), regnum); if (xcr0 & X86_XSTATE_AVX512) - regnum = create_feature_i386_64bit_avx512 (tdesc, regnum); + regnum = create_feature_i386_64bit_avx512 (tdesc.get (), regnum); if ((xcr0 & X86_XSTATE_PKRU) && !is_x32) - regnum = create_feature_i386_64bit_pkeys (tdesc, regnum); + regnum = create_feature_i386_64bit_pkeys (tdesc.get (), regnum); - return tdesc; + return tdesc.release (); } diff --git a/gdb/arch/arc.c b/gdb/arch/arc.c index dff457576e1..abed8a66847 100644 --- a/gdb/arch/arc.c +++ b/gdb/arch/arc.c @@ -34,11 +34,11 @@ #define STATIC_IN_GDB #endif -STATIC_IN_GDB target_desc * +STATIC_IN_GDB target_desc_up arc_create_target_description (const struct arc_arch_features &features) { /* Create a new target description. */ - target_desc *tdesc = allocate_target_description ().release (); + target_desc_up tdesc = allocate_target_description (); #ifndef IN_PROCESS_AGENT std::string arch_name; @@ -57,7 +57,7 @@ arc_create_target_description (const struct arc_arch_features &features) gdb_assert_not_reached (msg.c_str ()); } - set_tdesc_architecture (tdesc, arch_name.c_str ()); + set_tdesc_architecture (tdesc.get (), arch_name.c_str ()); #endif long regnum = 0; @@ -65,12 +65,12 @@ arc_create_target_description (const struct arc_arch_features &features) switch (features.isa) { case ARC_ISA_ARCV1: - regnum = create_feature_arc_v1_core (tdesc, regnum); - regnum = create_feature_arc_v1_aux (tdesc, regnum); + regnum = create_feature_arc_v1_core (tdesc.get (), regnum); + regnum = create_feature_arc_v1_aux (tdesc.get (), regnum); break; case ARC_ISA_ARCV2: - regnum = create_feature_arc_v2_core (tdesc, regnum); - regnum = create_feature_arc_v2_aux (tdesc, regnum); + regnum = create_feature_arc_v2_core (tdesc.get (), regnum); + regnum = create_feature_arc_v2_aux (tdesc.get (), regnum); break; default: std::string msg = string_printf @@ -111,12 +111,15 @@ arc_lookup_target_description (const struct arc_arch_features &features) if (it != arc_tdesc_cache.end ()) return it->second.get (); - target_desc *tdesc = arc_create_target_description (features); + target_desc_up tdesc = arc_create_target_description (features); - /* Add the newly created target description to the repertoire. */ - arc_tdesc_cache.emplace (features, tdesc); - return tdesc; + /* Add to the cache, and return a pointer borrowed from the + target_desc_up. This is safe as the cache (and the pointers + contained within it) are not deleted until GDB exits. */ + target_desc *ptr = tdesc.get (); + arc_tdesc_cache.emplace (features, std::move (tdesc)); + return ptr; } #endif /* !GDBSERVER */ diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c index dc67e40f9cf..406a8e72915 100644 --- a/gdb/arch/arm.c +++ b/gdb/arch/arm.c @@ -374,18 +374,18 @@ shifted_reg_val (struct regcache *regcache, unsigned long inst, target_desc * arm_create_target_description (arm_fp_type fp_type) { - target_desc *tdesc = allocate_target_description ().release (); + target_desc_up tdesc = allocate_target_description (); #ifndef IN_PROCESS_AGENT if (fp_type == ARM_FP_TYPE_IWMMXT) - set_tdesc_architecture (tdesc, "iwmmxt"); + set_tdesc_architecture (tdesc.get (), "iwmmxt"); else - set_tdesc_architecture (tdesc, "arm"); + set_tdesc_architecture (tdesc.get (), "arm"); #endif long regnum = 0; - regnum = create_feature_arm_arm_core (tdesc, regnum); + regnum = create_feature_arm_arm_core (tdesc.get (), regnum); switch (fp_type) { @@ -393,22 +393,22 @@ arm_create_target_description (arm_fp_type fp_type) break; case ARM_FP_TYPE_VFPV2: - regnum = create_feature_arm_arm_vfpv2 (tdesc, regnum); + regnum = create_feature_arm_arm_vfpv2 (tdesc.get (), regnum); break; case ARM_FP_TYPE_VFPV3: - regnum = create_feature_arm_arm_vfpv3 (tdesc, regnum); + regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum); break; case ARM_FP_TYPE_IWMMXT: - regnum = create_feature_arm_xscale_iwmmxt (tdesc, regnum); + regnum = create_feature_arm_xscale_iwmmxt (tdesc.get (), regnum); break; default: error (_("Invalid Arm FP type: %d"), fp_type); } - return tdesc; + return tdesc.release (); } /* See arch/arm.h. */ diff --git a/gdb/arch/i386.c b/gdb/arch/i386.c index 13201db1d74..d823c1ac32a 100644 --- a/gdb/arch/i386.c +++ b/gdb/arch/i386.c @@ -35,39 +35,39 @@ target_desc * i386_create_target_description (uint64_t xcr0, bool is_linux, bool segments) { - target_desc *tdesc = allocate_target_description ().release (); + target_desc_up tdesc = allocate_target_description (); #ifndef IN_PROCESS_AGENT - set_tdesc_architecture (tdesc, "i386"); + set_tdesc_architecture (tdesc.get (), "i386"); if (is_linux) - set_tdesc_osabi (tdesc, "GNU/Linux"); + set_tdesc_osabi (tdesc.get (), "GNU/Linux"); #endif long regnum = 0; if (xcr0 & X86_XSTATE_X87) - regnum = create_feature_i386_32bit_core (tdesc, regnum); + regnum = create_feature_i386_32bit_core (tdesc.get (), regnum); if (xcr0 & X86_XSTATE_SSE) - regnum = create_feature_i386_32bit_sse (tdesc, regnum); + regnum = create_feature_i386_32bit_sse (tdesc.get (), regnum); if (is_linux) - regnum = create_feature_i386_32bit_linux (tdesc, regnum); + regnum = create_feature_i386_32bit_linux (tdesc.get (), regnum); if (segments) - regnum = create_feature_i386_32bit_segments (tdesc, regnum); + regnum = create_feature_i386_32bit_segments (tdesc.get (), regnum); if (xcr0 & X86_XSTATE_AVX) - regnum = create_feature_i386_32bit_avx (tdesc, regnum); + regnum = create_feature_i386_32bit_avx (tdesc.get (), regnum); if (xcr0 & X86_XSTATE_MPX) - regnum = create_feature_i386_32bit_mpx (tdesc, regnum); + regnum = create_feature_i386_32bit_mpx (tdesc.get (), regnum); if (xcr0 & X86_XSTATE_AVX512) - regnum = create_feature_i386_32bit_avx512 (tdesc, regnum); + regnum = create_feature_i386_32bit_avx512 (tdesc.get (), regnum); if (xcr0 & X86_XSTATE_PKRU) - regnum = create_feature_i386_32bit_pkeys (tdesc, regnum); + regnum = create_feature_i386_32bit_pkeys (tdesc.get (), regnum); - return tdesc; + return tdesc.release (); } diff --git a/gdb/arch/tic6x.c b/gdb/arch/tic6x.c index dad4dd85a4d..d52ec3ac7b5 100644 --- a/gdb/arch/tic6x.c +++ b/gdb/arch/tic6x.c @@ -28,20 +28,20 @@ target_desc * tic6x_create_target_description (enum c6x_feature feature) { - target_desc *tdesc = allocate_target_description ().release (); + target_desc_up tdesc = allocate_target_description (); - set_tdesc_architecture (tdesc, "tic6x"); - set_tdesc_osabi (tdesc, "GNU/Linux"); + set_tdesc_architecture (tdesc.get (), "tic6x"); + set_tdesc_osabi (tdesc.get (), "GNU/Linux"); long regnum = 0; - regnum = create_feature_tic6x_core (tdesc, regnum); + regnum = create_feature_tic6x_core (tdesc.get (), regnum); if (feature == C6X_GP || feature == C6X_C6XP) - regnum = create_feature_tic6x_gp (tdesc, regnum); + regnum = create_feature_tic6x_gp (tdesc.get (), regnum); if (feature == C6X_C6XP) - regnum = create_feature_tic6x_c6xp (tdesc, regnum); + regnum = create_feature_tic6x_c6xp (tdesc.get (), regnum); - return tdesc; + return tdesc.release (); } diff --git a/gdbserver/linux-arc-low.cc b/gdbserver/linux-arc-low.cc index 1f370ef7964..d101bd250eb 100644 --- a/gdbserver/linux-arc-low.cc +++ b/gdbserver/linux-arc-low.cc @@ -112,12 +112,12 @@ arc_linux_read_description (void) #else arc_arch_features features (4, ARC_ISA_ARCV2); #endif - struct target_desc *tdesc = arc_create_target_description (features); + target_desc_up tdesc = arc_create_target_description (features); static const char *expedite_regs[] = { "sp", "status32", nullptr }; - init_target_desc (tdesc, expedite_regs); + init_target_desc (tdesc.get (), expedite_regs); - return tdesc; + return tdesc.release (); } void -- 2.25.4