From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=)
Subject: bug#28211: Stack marking issue in multi-threaded code
Date: Fri, 29 Jun 2018 23:18:31 +0200
Message-ID: <87muvdthp4.fsf@gnu.org>
References: <877exuj58y.fsf@gnu.org> <87d0yo1tie.fsf@gnu.org>
	<87fu3124nt.fsf@gnu.org> <87d0y5k6sl.fsf@netris.org>
	<871sel6vnq.fsf@igalia.com> <87fu30dmx3.fsf@netris.org>
	<87tvrg3q1d.fsf@igalia.com> <87a7rdvdm9.fsf_-_@gnu.org>
	<87in61y1m0.fsf@netris.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Return-path: <bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:34483)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1fZ0nG-0003uZ-VF
	for bug-guix@gnu.org; Fri, 29 Jun 2018 17:19:03 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1fZ0nG-0000O2-2n
	for bug-guix@gnu.org; Fri, 29 Jun 2018 17:19:02 -0400
Received: from debbugs.gnu.org ([208.118.235.43]:60486)
	by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
	(Exim 4.71) (envelope-from <Debian-debbugs@debbugs.gnu.org>)
	id 1fZ0nF-0000Np-U9
	for bug-guix@gnu.org; Fri, 29 Jun 2018 17:19:01 -0400
Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
Resent-Message-ID: <handler.28211.B28211.153030712326592@debbugs.gnu.org>
In-Reply-To: <87in61y1m0.fsf@netris.org> (Mark H. Weaver's message of "Fri, 29
	Jun 2018 12:54:47 -0400")
List-Id: Bug reports for GNU Guix <bug-guix.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/bug-guix>,
	<mailto:bug-guix-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/bug-guix/>
List-Post: <mailto:bug-guix@gnu.org>
List-Help: <mailto:bug-guix-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/bug-guix>,
	<mailto:bug-guix-request@gnu.org?subject=subscribe>
Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org
Sender: "bug-Guix" <bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org>
To: Mark H Weaver <mhw@netris.org>
Cc: Andy Wingo <wingo@igalia.com>, 28211@debbugs.gnu.org

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> ludo@gnu.org (Ludovic Court=C3=A8s) writes:

[...]

>> I=E2=80=99m thinking we could perhaps add a compiler barrier before =E2=
=80=98vp->fp =3D new_fp=E2=80=99
>> statements, but in practice it=E2=80=99s not necessary here (x86_64, gcc=
 7).
>>
>> Thoughts?

I just pushed the patch as 23af45e248e8e2bec99c712842bf24d6661abbe2.

> Indeed, we need something to ensure that the compiler won't reorder
> these writes.  My recommendation would be to use the 'volatile'
> qualifier in the declarations of both the 'fp' field of 'struct scm_vm'
> and the stack element modified by SCM_FRAME_SET_DYNAMIC_LINK.
>
> Maybe something like this:
>
> diff --git a/libguile/frames.h b/libguile/frames.h
> index ef2db3df5..8554f886b 100644
> --- a/libguile/frames.h
> +++ b/libguile/frames.h
> @@ -89,6 +89,7 @@
>  union scm_vm_stack_element
>  {
>    scm_t_uintptr as_uint;
> +  volatile scm_t_uintptr as_volatile_uint;

[...]

> +#define SCM_FRAME_SET_DYNAMIC_LINK(fp, dl) ((fp)[1].as_volatile_uint =3D=
 ((dl) - (fp)))

I=E2=80=99m not sure this is exactly what we need.

What I had in mind, to make sure the vp->fp assignment really happens
after the SCM_FRAME_SET_DYNAMIC_LINK, was to do something like this:

  SCM_FRAME_SET_RETURN_ADDRESS (new_fp, =E2=80=A6);
  SCM_FRAME_SET_DYNAMIC_LINK (new_fp, =E2=80=A6);

  asm volatile ("" : : : "memory");

  vp->fp =3D new_fp;

I think that more accurately reflects what we want, WDYT?

It=E2=80=99s GCC-specific though (I don=E2=80=99t think Clang implements =
=E2=80=98asm=E2=80=99 in a
compatible way, does it?) and I suppose we=E2=80=99d have to ignore the non=
-GCC
case.

Thoughts?

Ludo=E2=80=99.