From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by sourceware.org (Postfix) with ESMTPS id AEF7C3938C32 for ; Mon, 3 May 2021 11:18:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AEF7C3938C32 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-x333.google.com with SMTP id j3-20020a05600c4843b02901484662c4ebso3879429wmo.0 for ; Mon, 03 May 2021 04:18:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=cTpEmiCRTaJzLy8rAPVfla5Z8IUq2BFYLmKuAfTMv7A=; b=UJuINJiRgJ2gz4PwEOphsk0j3xpJLzkP8YZa3lylT/T8e3hgDnt/D9qE8jj+/kVig5 5vzORBMWSAN7Tfv3AwPM+WTq/DVEx5DhfSDD0Btqb0sBWUe78+LS2aFaJh/DG1Ce9Ihe kNz9uv8zmi6ecXS7zPXqOGuOcnFcOrXH+MoOBTCZ5CXMQuy4OBjsZSu66wEf79faJi/D jNV1bCaWrjDlpd09dbnBvX1ekly18k/lN+iUO6OcuocDgbEjlw0eAFbOQpqKUOWEbJpV pIPeCCNBo9PtDMcJlPXoYEY1hvyN+ukLlgHZ4l5gsTtvNeuI7WJkihHxYDoXAzvFdDpe bsAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=cTpEmiCRTaJzLy8rAPVfla5Z8IUq2BFYLmKuAfTMv7A=; b=OBB5EObG4n5rA5VdO+8rQ9Dh1QGoHlQTUfJHLIyhvcUXknlGmlvnMvNjiCF7sZlJV0 EFkvFBZoS3X7Foj38XhEGp+9NoI+ub50LPybDf47+EkYuXqbpydK0T2x/QdY6mdgW9mx Ata6cPi3w5l/5ZL2s3qT2oE7dqYuvHUK5LSbhCfwQq3Yd+0uDZorKbCn76NCQJp+494n ZM3OabkA61TgY0mrg7Sbgb6nF30ePboy82HZBnGakxAoVev6VDTwPtLm2maIu6rMaI7g IqyY8V7hUF9WLpezJKsC7ZMbyMC4pQ5ATOpdsdXKS7YKFge1INy94BTWCpyYpbeanF6V tlXQ== X-Gm-Message-State: AOAM5310btD6zy69KKjM2D5GKMxh55gWwbSMvywV4wlLizrsgTDYYj3q uWz0uY4D3XxUrY0C+b76OvS+fw== X-Google-Smtp-Source: ABdhPJwSHWaRK8PJimn4/vhplO4bmiXodYRoMzzW3ucMDwqj69iEGYAuxzRcsEmDQ69OxxxHxzGjmg== X-Received: by 2002:a1c:f20f:: with SMTP id s15mr32474209wmc.61.1620040723790; Mon, 03 May 2021 04:18:43 -0700 (PDT) Received: from localhost (host109-151-46-70.range109-151.btcentralplus.com. [109.151.46.70]) by smtp.gmail.com with ESMTPSA id t10sm18690941wmf.16.2021.05.03.04.18.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 May 2021 04:18:43 -0700 (PDT) Date: Mon, 3 May 2021 12:18:42 +0100 From: Andrew Burgess To: Tom de Vries Cc: gdb-patches@sourceware.org, Ludovic =?iso-8859-1?Q?Court=E8s?= Subject: Re: [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns Message-ID: <20210503111842.GA6612@embecosm.com> References: <20210503085428.GA20738@delia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210503085428.GA20738@delia> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 12:11:28 up 5 min, 1 X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-12.0 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: Mon, 03 May 2021 11:18:46 -0000 * Tom de Vries [2021-05-03 10:54:29 +0200]: > Hi, > > Since gdb commit 880ae75a2b7 "gdb delay guile initialization until > gdbscm_finish_initialization" I'm running into: > ... > (gdb) print My_Var > 10.0^M > free(): invalid pointer^M > ERROR: GDB process no longer exists > GDB process exited with wait status 5995 exp9 0 0 CHILDKILLED SIGABRT SIGABRT > UNRESOLVED: gdb.ada/fixed_cmp.exp: gnat_encodings=all: print My_Var > 10.0 > ... > > The problem is that both gdb and libguile try to set the libgmp memory functions, > and since the gdb commit the ones from libguile are effective, which results > in gdb freeing some memory in a way that is not compatible with the way that > memory was actually allocated. > > The fact that libguile tries to set the libgmp memory functions is a bug which > should be fixed starting version v3.0.6. > > Meanwhile, work around this in gdb by not allowing libguile to set the libgomp > memory functions. Thanks for looking into this, and sorry for causing the breakage. I had a read through the bug, and this solution seems to make sense, however, I had two thoughts. First, I already had to solve a similar problem for Python when doing this work. For Python the issue related to which signal handlers were installed. Though the problem was Python specific, I figured that it was cleaner to make the solution generic, so I placed the fix in gdb/extension.c:ext_lang_initialization - look for the use of scoped_default_sigint. I wonder if this fix should similarly be placed at this level? I'll leave this choice up to you, I don't feel strongly on this, but thought it might be worth mentioning. For the second thought, see below... > > Tested on x86_64-linux. > > Any comments? > > Thanks, > - Tom > > [gdb/guile] Don't allow libguile to change libgmp mem fns > > gdb/ChangeLog: > > 2021-05-03 Tom de Vries > > PR guile/27806 > * guile/guile.c (gdbscm_initialize): Save and restore libgmp memory > functions. > > --- > gdb/guile/guile.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c > index bdf15cd498b..6ee8b3f47ce 100644 > --- a/gdb/guile/guile.c > +++ b/gdb/guile/guile.c > @@ -662,10 +662,32 @@ gdbscm_initialize (const struct extension_language_defn *extlang) > { > gdb::block_signals blocker; > > + /* There are libguile versions (f.i. v3.0.5) that by default call > + mp_get_memory_functions during initialization to install custom > + libgmp memory functions. This is considered a bug and should be > + fixed starting v3.0.6. > + Before gdb commit 880ae75a2b7 "gdb delay guile initialization until > + gdbscm_finish_initialization", that bug had no effect for gdb, > + because gdb subsequently called mp_get_memory_functions to install > + its own custom functions in _initialize_gmp_utils. However, since > + aforementioned gdb commit the initialization order is reversed, > + allowing libguile to install a custom malloc that is incompatible > + with the custom free as used in gmp-utils.c, resulting in a > + "double free or corruption (out)" error. > + Work around the libguile bug by saving the libgmp memory functions > + before guile initialization, and restoring them afterwards. */ > + void *(*alloc_func) (size_t); > + void *(*realloc_func) (void *, size_t, size_t); > + void (*free_func) (void *, size_t); > + mp_get_memory_functions (&alloc_func, &realloc_func, &free_func); I think any time we do SAVE-VALUE -> WORK -> RESTORE-VALUE, we should be wrapping this up in an RAII class. Right now I don't believe scm_with_guile can throw an exception, but you never know how the code will change in the future, and creating an RAII class now just makes things future proof. Thanks, Andrew > + > /* scm_with_guile is the most portable way to initialize Guile. Plus > we need to initialize the Guile support while in Guile mode (e.g., > called from within a call to scm_with_guile). */ > scm_with_guile (call_initialize_gdb_module, NULL); > + > + /* Restore libgmp memory functions. */ > + mp_set_memory_functions (alloc_func, realloc_func, free_func); > } > > /* Set Guile's backtrace to match the "set guile print-stack" default.