From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Mark H Weaver Newsgroups: gmane.lisp.guile.bugs Subject: bug#32786: (system* ...) call somehow interferes with atomic-box on armv7l Date: Thu, 27 Sep 2018 01:49:23 -0400 Message-ID: <87sh1vcwws.fsf@netris.org> References: <20180920171306.GA9185@k> <87fty2v8xo.fsf@netris.org> <20180922043938.GA4539@blacky> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1538027291 31464 195.159.176.226 (27 Sep 2018 05:48:11 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 27 Sep 2018 05:48:11 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) Cc: 32786@debbugs.gnu.org To: =?UTF-8?Q?=D0=9C=D0=B8=D1=85=D0=B0=D0=B8=D0=BB_?= =?UTF-8?Q?=D0=91=D0=B0=D1=85=D1=82=D0=B5=D1=80=D0=B5=D0=B2?= Original-X-From: bug-guile-bounces+guile-bugs=m.gmane.org@gnu.org Thu Sep 27 07:48:07 2018 Return-path: Envelope-to: guile-bugs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1g5P9i-000844-CX for guile-bugs@m.gmane.org; Thu, 27 Sep 2018 07:48:06 +0200 Original-Received: from localhost ([::1]:33862 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g5PBo-0003R5-V1 for guile-bugs@m.gmane.org; Thu, 27 Sep 2018 01:50:16 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:36724) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g5PBf-0003MV-LY for bug-guile@gnu.org; Thu, 27 Sep 2018 01:50:12 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g5PBa-00082C-W0 for bug-guile@gnu.org; Thu, 27 Sep 2018 01:50:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:51579) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1g5PBa-00081z-Nc for bug-guile@gnu.org; Thu, 27 Sep 2018 01:50:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1g5PBa-0005te-H9 for bug-guile@gnu.org; Thu, 27 Sep 2018 01:50:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Mark H Weaver Original-Sender: "Debbugs-submit" Resent-CC: bug-guile@gnu.org Resent-Date: Thu, 27 Sep 2018 05:50:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32786 X-GNU-PR-Package: guile X-GNU-PR-Keywords: Original-Received: via spool by 32786-submit@debbugs.gnu.org id=B32786.153802738422636 (code B ref 32786); Thu, 27 Sep 2018 05:50:02 +0000 Original-Received: (at 32786) by debbugs.gnu.org; 27 Sep 2018 05:49:44 +0000 Original-Received: from localhost ([127.0.0.1]:55837 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1g5PBI-0005t2-4r for submit@debbugs.gnu.org; Thu, 27 Sep 2018 01:49:44 -0400 Original-Received: from world.peace.net ([64.112.178.59]:40328) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1g5PBG-0005sn-T5 for 32786@debbugs.gnu.org; Thu, 27 Sep 2018 01:49:43 -0400 Original-Received: from mhw by world.peace.net with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1g5PBA-0003vR-KO; Thu, 27 Sep 2018 01:49:36 -0400 In-Reply-To: <20180922043938.GA4539@blacky> ("=?UTF-8?Q?=D0=9C=D0=B8=D1=85=D0=B0=D0=B8=D0=BB_?= =?UTF-8?Q?=D0=91=D0=B0=D1=85=D1=82=D0=B5=D1=80=D0=B5=D0=B2?="'s message of "Sat, 22 Sep 2018 09:39:38 +0500") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane.org@gnu.org Original-Sender: "bug-guile" Xref: news.gmane.org gmane.lisp.guile.bugs:9173 Archived-At: --=-=-= Content-Type: text/plain Hi, Thanks for the additional details. I was able to reproduce the bug, and I believe I now see the problem. 'atomic-box-compare-and-swap!' is implemented using 'atomic_compare_exchange_weak' (if available), but neglects to handle the case where 'atomic_compare_exchange_weak' may spuriously fail. In that case, the box is left unchanged, although the observed value was equal to the expected value. What's happening here is that the 'atomic-box-compare-and-swap!' in 'sleep-loop' fails spuriously, leaving the box in state #:accepted although it returns #:accepted as its result. When the main loop discovers this, it changes the state to #:need-to-sleep, although the thread has already ended. To confirm this hypothesis, I added a print statement to the main loop showing the state of the box that it observed during the protocol exchange, and indeed it sees #:accepted the first time it checks, and #:need-to-sleep in all later iterations. I've attached a proposed patch that I hope will fix this problem. If you'd be willing to test it, I'd be grateful, but otherwise I'll try to test it in the next week or so. My access to armv7l boxes is somewhat limited. Thanks for this report. Mark --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-UNTESTED-Fix-atomic-box-compare-and-swap.patch Content-Description: [PATCH] UNTESTED: Fix 'atomic-box-compare-and-swap!' >From 958d37686a9ac65f48cb2ca32a341cf182c24b5a Mon Sep 17 00:00:00 2001 From: Mark H Weaver Date: Thu, 27 Sep 2018 01:00:11 -0400 Subject: [PATCH] UNTESTED: Fix 'atomic-box-compare-and-swap!'. Fixes . 'scm_atomic_compare_and_swap_scm' uses 'atomic_compare_exchange_weak' if it's available on the host platform. 'atomic_compare_exchange_weak' may fail spuriously, leaving the atomic object unchanged even when it contained the expected value. 'atomic-box-compare-and-swap!' uses 'scm_atomic_compare_and_swap_scm' without checking its return value, and therefore may return the expected value while leaving the box unchanged. This contradicts the documentation, which explicitly states that "you can know if the swap worked by checking if the return value is 'eq?' to EXPECTED.". * libguile/atomic.c (scm_atomic_box_compare_and_swap_x): If 'scm_atomic_compare_and_swap_scm' returns false and the observed value is equal to 'expected', then try again. * libguile/vm-engine.c (atomic-box-compare-and-swap!): Ditto. --- libguile/atomic.c | 13 +++++++++---- libguile/vm-engine.c | 13 ++++++++----- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/libguile/atomic.c b/libguile/atomic.c index 950874030..504643912 100644 --- a/libguile/atomic.c +++ b/libguile/atomic.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2016 Free Software Foundation, Inc. +/* Copyright (C) 2016, 2018 Free Software Foundation, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public License @@ -95,10 +95,15 @@ SCM_DEFINE (scm_atomic_box_compare_and_swap_x, "if the return value is @code{eq?} to @var{expected}.") #define FUNC_NAME s_scm_atomic_box_compare_and_swap_x { + SCM result = expected; + SCM_VALIDATE_ATOMIC_BOX (1, box); - scm_atomic_compare_and_swap_scm (scm_atomic_box_loc (box), - &expected, desired); - return expected; + while (!scm_atomic_compare_and_swap_scm (scm_atomic_box_loc (box), + &result, desired) + && scm_is_eq (result, expected)) + { + } + return result; } #undef FUNC_NAME diff --git a/libguile/vm-engine.c b/libguile/vm-engine.c index 19ff3e498..9650e33eb 100644 --- a/libguile/vm-engine.c +++ b/libguile/vm-engine.c @@ -3868,16 +3868,19 @@ VM_NAME (scm_i_thread *thread, struct scm_vm *vp, { scm_t_uint16 dst, box; scm_t_uint32 expected, desired; - SCM scm_box, scm_expected; + SCM scm_box, scm_expected, scm_result; UNPACK_12_12 (op, dst, box); UNPACK_24 (ip[1], expected); UNPACK_24 (ip[2], desired); scm_box = SP_REF (box); VM_VALIDATE_ATOMIC_BOX (scm_box, "atomic-box-compare-and-swap!"); - scm_expected = SP_REF (expected); - scm_atomic_compare_and_swap_scm (scm_atomic_box_loc (scm_box), - &scm_expected, SP_REF (desired)); - SP_SET (dst, scm_expected); + scm_result = scm_expected = SP_REF (expected); + while (!scm_atomic_compare_and_swap_scm (scm_atomic_box_loc (scm_box), + &scm_result, SP_REF (desired)) + && scm_is_eq (scm_result, scm_expected)) + { + } + SP_SET (dst, scm_result); NEXT (3); } -- 2.19.0 --=-=-=--