From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id BB7623858C30 for ; Mon, 20 Nov 2023 23:36:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BB7623858C30 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BB7623858C30 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700523411; cv=none; b=AU3ZJWY9kwVzHvwZjP5N/UAiCJmVnKiLVHV69Zgjt4Y8suK8V370YHuzU+4mIoB+tRL30aNoKRHs1b3lwOIduQaOj18Obih3EYUpFHDiHCDfdJGcBqpY7BIebCJi/Oc80scVHFeoqxGwlh7giuCSc5WQylwOPNVTH6AKRYFJwCg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700523411; c=relaxed/simple; bh=nDg3jw9lknOtPKLBjaDXeTrpYUelOh34LovnSArnBvg=; h=DKIM-Signature:Message-ID:Subject:From:To:Date:MIME-Version; b=urpnKN2bJZdsI3QHoO53mUeppLiuf+1nu5egUBAhoKRzhk8Oe0Ps85084pYvUFsJuJYTWaJGCNmzeySdgfiELGBcsi+gd0HliX/EWcRY+N4vKZDm74SrGVNS1B7zrrcqHoYUK47gfn+YwQoPqszefLlql1Z5CBEZkkj5UfD6O54= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700523409; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wgVCrEIgo/BWPtGXmOG6lTaXeo5PlQocD/ab++ETYls=; b=im4nSJdZH7NmiyJiD+xCgqrCEz3ogVnXR/DxIuDSHv3NOYoo48iEj/byB2JfFpVJnplt77 f4FWbBTattGzXtUxmxXtjEqMklafUyTt7WDgNEUIZvT6RnPpkskFsHvEcByR/Q1WUruiZl H+GHgI28ieSr9qPYWLrqjxZkQMNv7UM= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-655-V3iNgVNgNNa4r0d9QEdVOA-1; Mon, 20 Nov 2023 18:36:47 -0500 X-MC-Unique: V3iNgVNgNNa4r0d9QEdVOA-1 Received: by mail-qk1-f200.google.com with SMTP id af79cd13be357-7788fa5f1b0so649401285a.2 for ; Mon, 20 Nov 2023 15:36:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700523407; x=1701128207; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=NexTx3J0kc+U+m8Yvj3Xt+ChO2Xm1Dix5xGB1XYjGGo=; b=YIDVa5PyjD4Bz/hX/zrJ924zqnC7fWpIgzW7P41XEAOVn8KYMFQlzsUIEzoN8LlMgf 1ejViI4sND4zDxDVf2t61kf2Ew0Z4McbISIH5jbEq1SI44HqwDdemd8A9pQlkTnUEwRt 28Kw4pTJTafhBa2tzZebtHFJtWXLlo6QYmPW/qvV+Eu7CPVBB3U9kGzKA6grlvj6hQeu OewzEZqy/OLhZM+YIESM3yOLEtQkbKqiLQkert1+voSPHO/O1/GNKtzEDCYV1PD3qyx4 F1O2g1dq+3Tz92EGsclMqLXXzBI3ksYWibIb4yWxaLxOzYMSQl61CJc3kpzPtW9RtNST k9mA== X-Gm-Message-State: AOJu0YxccXS5JMhO2cdjO+ypEfk6UkznK7M1HBxr6/qAnemPJmmE2li/ lsslZsfnkVJwBeqF9BY3dB3O+hIZm9leT9czW5p87YlUPk+2e3utGIsQsAU9hrcHS1yvyIYaFvV Oc/ieaEk= X-Received: by 2002:a05:620a:1201:b0:774:30b7:ed93 with SMTP id u1-20020a05620a120100b0077430b7ed93mr9029887qkj.29.1700523407233; Mon, 20 Nov 2023 15:36:47 -0800 (PST) X-Google-Smtp-Source: AGHT+IELRYEmqJn+N9UEnihpKfscmGBWpPRLp5R7rlZ9YZ/ArYxIo1khD7TVcYbqaMQgVjEk89ircQ== X-Received: by 2002:a05:620a:1201:b0:774:30b7:ed93 with SMTP id u1-20020a05620a120100b0077430b7ed93mr9029873qkj.29.1700523406827; Mon, 20 Nov 2023 15:36:46 -0800 (PST) Received: from t14s.localdomain (c-76-28-97-5.hsd1.ma.comcast.net. [76.28.97.5]) by smtp.gmail.com with ESMTPSA id q19-20020a05620a0d9300b007743446efd1sm3096728qkl.35.2023.11.20.15.36.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Nov 2023 15:36:46 -0800 (PST) Message-ID: <79b0df5fec3b02dfcfc312b2560e190d5519d78e.camel@redhat.com> Subject: Re: [PATCH] libgccjit: Add ways to set the personality function From: David Malcolm To: Antoni Boucher , jit@gcc.gnu.org, gcc-patches@gcc.gnu.org Date: Mon, 20 Nov 2023 18:36:45 -0500 In-Reply-To: <4568a2ea1baa146f2f4b83636d8329c6dd351599.camel@zoho.com> References: <4568a2ea1baa146f2f4b83636d8329c6dd351599.camel@zoho.com> User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, 2023-11-17 at 17:48 -0500, Antoni Boucher wrote: > Hi. > This adds functions to set the personality function (bug 112603). >=20 > I'm not sure I can make a test for this: it seems the personality > function will not be set if there are no try/catch inside the > functions. > Do you know a way to keep the personality function that is set in > this > case? >=20 > Or should we wait until I send the patch for try/catch? Thanks for the patch > diff --git a/gcc/expr.cc b/gcc/expr.cc > index 556bcf7ef59..25d50289b24 100644 > --- a/gcc/expr.cc > +++ b/gcc/expr.cc > @@ -13559,6 +13559,14 @@ build_personality_function (const char *lang) > =20 > name =3D ACONCAT (("__", lang, "_personality", unwind_and_version, NUL= L)); > =20 > + return build_personality_function_with_name (name); > +} > + > +tree > +build_personality_function_with_name (const char *name) > +{ > + tree decl, type; > + > type =3D build_function_type_list (unsigned_type_node, > =09=09=09=09 integer_type_node, integer_type_node, > =09=09=09=09 long_long_unsigned_type_node, I confess I'm not very familiar with personalities, sorry; hopefully a reviewer who is can take a look at the non-jit parts of the patch, though FWIW they look fairly trivial. > diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst > index ebede440ee4..31c3ef6401a 100644 > --- a/gcc/jit/docs/topics/compatibility.rst > +++ b/gcc/jit/docs/topics/compatibility.rst > @@ -378,3 +378,13 @@ alignment of a variable: > -------------------- > ``LIBGCCJIT_ABI_25`` covers the addition of > :func:`gcc_jit_type_get_restrict` > + > +.. _LIBGCCJIT_ABI_26: > + > +``LIBGCCJIT_ABI_26`` > +-------------------- > +``LIBGCCJIT_ABI_26`` covers the addition of functions to set the persona= lity > +function: > + > + * :func:`gcc_jit_function_set_personality_function` > + * :func:`gcc_jit_set_global_personality_function_name` Obviously you're going to need to update the number if the other patch goes in first. > diff --git a/gcc/jit/docs/topics/functions.rst b/gcc/jit/docs/topics/func= tions.rst > index cf5cb716daf..e59885c3549 100644 > --- a/gcc/jit/docs/topics/functions.rst > +++ b/gcc/jit/docs/topics/functions.rst > @@ -197,6 +197,34 @@ Functions > =20 > .. type:: gcc_jit_case > =20 > +.. function:: void > + gcc_jit_function_set_personality_function (gcc_jit_functi= on *fn, > + gcc_jit_functi= on *personality_func) > + > + Set the personality function of ``fn`` to ``personality_func``. > + > + were added in :ref:`LIBGCCJIT_ABI_26`; you can test for their presenc= e > + using Likewise here. Is there a URL in the main GCC docs we can link to that describes what this is for? Are there restrictions on what a personality_func is? Sorry for my ignorance here. > + > + .. code-block:: c > + > + #ifdef LIBGCCJIT_HAVE_PERSONALITY_FUNCTION > + > +.. function:: void > + gcc_jit_set_global_personality_function_name (char* name) > + > + Set the global personality function. > + > + This is mainly useful to prevent the optimizer from unsetting the per= sonality > + function set on other functions. > + > + were added in :ref:`LIBGCCJIT_ABI_26`; you can test for their presenc= e > + using Likewise here: ABI numbering, and a URL for more info on what this is. > + > + .. code-block:: c > + > + #ifdef LIBGCCJIT_HAVE_PERSONALITY_FUNCTION > + > Blocks > ------ > .. type:: gcc_jit_block > diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc > index a729086bafb..c9dedd59b24 100644 > --- a/gcc/jit/dummy-frontend.cc > +++ b/gcc/jit/dummy-frontend.cc > @@ -146,6 +146,20 @@ const struct attribute_spec jit_format_attribute_tab= le[] =3D > { NULL, 0, 0, false, false, false, false, NULL, NU= LL } > }; > =20 > +char* jit_personality_func_name =3D NULL; > +static tree personality_decl; > + > +/* FIXME: This is a hack to preserve trees that we create from the > + garbage collector. */ > + > +static GTY (()) tree jit_gc_root; > + > +void > +jit_preserve_from_gc (tree t) > +{ > + jit_gc_root =3D tree_cons (NULL_TREE, t, jit_gc_root); > +} If I'm reading the patch correctly, this is only used by jit_langhook_eh_personality, to preserve personality_decl. Can't you just add a GTY (()) marker to personality_decl's decl instead, as: static GTY (()) tree personality_decl; [...] > diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc > index 0451b4df7f9..67d9036249e 100644 > --- a/gcc/jit/libgccjit.cc > +++ b/gcc/jit/libgccjit.cc > @@ -3590,6 +3590,28 @@ gcc_jit_context_add_command_line_option (gcc_jit_c= ontext *ctxt, > ctxt->add_command_line_option (optname); > } > =20 > +/* Public entrypoint. See description in libgccjit.h. > + After error-checking, the real work is done by the > + gcc::jit::recording::function::set_personality_function method, in > + jit-recording.c. */ > + > +void > +gcc_jit_function_set_personality_function (gcc_jit_function *fn, > +=09=09=09=09=09 gcc_jit_function *personality_func) > +{ > + RETURN_IF_FAIL (fn, NULL, NULL, "NULL function"); I see various unconditional dereferences of m_personality_function, so am I right in assuming that personality_function must be non-NULL? If so we should document that, and reject NULL here. > + > + fn->set_personality_function (personality_func); > +} > + > +extern char* jit_personality_func_name; > + > +void > +gcc_jit_set_global_personality_function_name (char* name) > +{ > + jit_personality_func_name =3D name; This probably should be per-context state, rather than a global variable. So I think this needs a gcc_jit_context * param, which must be non-null, and we'd have a new field m_personality_func_name of the recording::context, rather than the global. Also everywhere else in the API we take a copy of the string, rather than via a pointer. Is there a reason for doing this here? Otherwise, this param should be a const char *, and we should xstrdup it (and free any existing value). [...] =20 > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map > index 8b90a0e2ff3..2a0eb819a52 100644 > --- a/gcc/jit/libgccjit.map > +++ b/gcc/jit/libgccjit.map > @@ -276,3 +276,9 @@ LIBGCCJIT_ABI_25 { > global: > gcc_jit_type_get_restrict; > } LIBGCCJIT_ABI_24; > + > +LIBGCCJIT_ABI_26 { > + global: > + gcc_jit_set_global_personality_function_name; > + gcc_jit_function_set_personality_function; > +} LIBGCCJIT_ABI_25; Same note as above about ABI numbering. > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite= /jit.dg/all-non-failing-tests.h > index e762563f9bd..3785a788ffa 100644 > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h > @@ -322,6 +322,9 @@ > /* test-setting-alignment.c: This can't be in the testcases array as it > is target-specific. */ > =20 > +/* test-personality-function.c: This can't be in the testcases array as = it > + is target-specific. */ ...and because it modifies per-context state. [...] Hope the above makes sense; sorry again about my ignorance of the underlying personality stuff. Dave