From patchwork Mon Sep 16 14:15:32 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 173839 Delivered-To: patch@linaro.org Received: by 2002:a05:6e02:ce:0:0:0:0 with SMTP id r14csp3559488ilq; Mon, 16 Sep 2019 07:18:49 -0700 (PDT) X-Google-Smtp-Source: APXvYqxgegrOIXSOp0bZC5bA+sdiCVZfl5BvlGLbVT8uk5mihR2fTe5bd2VCmi20UaL5o7YNW8ko X-Received: by 2002:a17:906:b34c:: with SMTP id cd12mr280742ejb.48.1568643529878; Mon, 16 Sep 2019 07:18:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1568643529; cv=none; d=google.com; s=arc-20160816; b=DMk9ViOdQl0AwDFoSmROefggCHcvZNACvGCA+0JiK1dIL9sXguRH3Ioziu8zo+7J+W oIVGnqY8l5ckmU/e99sabAWPOChJrj5Y3OZ0tQYatVgPKeXEDUUQAWvn+75a/qoAFjhV 88sJUlYyUjtXc3sBiQuXdTcE4WadtnQo1pYknxFZVgOLNqdYLj5iGs1EXWzcRIukuBpw aJO/Uaz9xMDbpKfILqalQ6LrnafCheClYY8JMUgaNvRaSC0QXNrYKDEfOly1HO5OY0jN 2RXIT3t83mIbQLntE0pa1IQbpXNoTzLKSZ5N7UNshP6aS4wtfYnsWLoUEKGMyNBY9h4v hq1A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject :content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:to:from:dkim-signature; bh=QMiXNxopmp/Lfafa2rBITuXAv6CIJtSHxYMTRHI7jeA=; b=mQ/f9zP0b2KPqzjVhY1y89IOAdWw6GJewp/UWJz/iHjP0FYPcHeJ9Nj0OwLsLfkQue OP01UiD4p5CP9zcTjvGMXordXMgJvnM3yy7sppLoX5nd9T3Zw2QacYJP2rytSKgrZKlW yQn6jiLIViDMLxTq5evER1egCZFA6BV+8faz1XN5dWyG+DmWj3Ggu0F+nWVwtgmIWZ6+ TNLCluaT3yFF5e5+n/iIQAQz39rLAoWI7HW2UsYo3ffbd38aMJoo4RTLcOmM7KgRpRPV hlWydr0Y8e6OjYUhbbaCDt5zQpe7GeqRv5M/DWYz+hUEy+INASr5C5GA1Nc11VXznDPX 9Heg== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=sbvTCq9x; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id d23si22106454edb.55.2019.09.16.07.18.49 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 16 Sep 2019 07:18:49 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=sbvTCq9x; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1]:34858 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i9rq4-0004cH-1y for patch@linaro.org; Mon, 16 Sep 2019 10:18:48 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49614) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i9rnG-0004MM-Dn for qemu-devel@nongnu.org; Mon, 16 Sep 2019 10:15:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1i9rnE-0006tT-7V for qemu-devel@nongnu.org; Mon, 16 Sep 2019 10:15:54 -0400 Received: from mail-wr1-x443.google.com ([2a00:1450:4864:20::443]:33509) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1i9rnD-0006t3-O5 for qemu-devel@nongnu.org; Mon, 16 Sep 2019 10:15:52 -0400 Received: by mail-wr1-x443.google.com with SMTP id b9so5490514wrs.0 for ; Mon, 16 Sep 2019 07:15:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=QMiXNxopmp/Lfafa2rBITuXAv6CIJtSHxYMTRHI7jeA=; b=sbvTCq9xJLnYTTXmU+ghUk3Ze9TNp67+MuZaKRNtNc/py54VOl3kf2F0CqwOxPR3ec Nnn2nGdzip3bWKK/bzCyryi8VsVMCTfN5ceBagb2V+R5HKM8D21fFXjIpXjGML4mQXYH z1tWHQL+TeHfJLnvQsf+1LxcpeTD2yNtilMN6Lc0oun+FJuQiVO4xOQCoF/kGmIXAOyM NZ7XZfLko24omWFK3sDArecVfXyVT8iDDsCnsKn0U6sH7P/3+6DhdYV4CrZJCETWCRcT XwrRyNyLSHYZKxIDY/WLMueSl41J4TFOSEFQaVafzZohsc6Wg/6trvZQDabxs0LbCLSH /9Cw== 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=QMiXNxopmp/Lfafa2rBITuXAv6CIJtSHxYMTRHI7jeA=; b=WRpv1/jK2Te+rnhA8zgeq/l5XcbcR4dLqYRMdz6TU5xgkDwxZM7fLfMBa5pKwo5XzI Tlp0bBeWF/W9MY4QuTbm/qU7s7+2CsXia4NMLbPStV+k0TCxd8w2lt77UUBCYRGIWCQ0 uSNrr3ufbJ2eMk80eBEOlB74Xff1FmzEoAMajxvjqpmfx2z1ZKOyfxqa+fnzg75l1JeZ c5iQj54uOR27/k695Vui1h8l2tEI2Fbn9LZrjVLBHqlb3Q0eqdi6Bv1md/cCN6sFZQr/ Dsl8tqHY9aLgYzOzbs2pGcAh3OILwpNCn5DKp9OoFdDeIozA0T98+bQx1I5tk9j/tc6N FErw== X-Gm-Message-State: APjAAAUbAN8mlqlYk/pbx87lrfH3d/w6lu7TbESzz8ogWFSRyNHPyS4t WNTKfgi9IK4JyGrk6z4GSEomSw== X-Received: by 2002:a5d:4a84:: with SMTP id o4mr294643wrq.165.1568643350660; Mon, 16 Sep 2019 07:15:50 -0700 (PDT) Received: from orth.archaic.org.uk (orth.archaic.org.uk. [81.2.115.148]) by smtp.gmail.com with ESMTPSA id s9sm15185884wme.36.2019.09.16.07.15.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Sep 2019 07:15:50 -0700 (PDT) From: Peter Maydell To: qemu-arm@nongnu.org, qemu-devel@nongnu.org Date: Mon, 16 Sep 2019 15:15:32 +0100 Message-Id: <20190916141544.17540-4-peter.maydell@linaro.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190916141544.17540-1-peter.maydell@linaro.org> References: <20190916141544.17540-1-peter.maydell@linaro.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::443 Subject: [Qemu-devel] [PATCH v2 03/15] target/arm/arm-semi: Correct comment about gdb syscall races X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?utf-8?q?Alex_Benn=C3=A9e?= Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" In arm_gdb_syscall() we have a comment suggesting a race because the syscall completion callback might not happen before the gdb_do_syscallv() call returns. The comment is correct that the callback may not happen but incorrect about the effects. Correct it and note the important caveat that callers must never do any work of any kind after return from arm_gdb_syscall() that depends on its return value. Signed-off-by: Peter Maydell --- I'll come back to do the cleanup later, but I preferred not to tangle it up with the rest of the refactoring in this series; I also think it's probably easier done afterwards rather than before. --- target/arm/arm-semi.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) -- 2.20.1 Reviewed-by: Richard Henderson diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c index 51b55816faf..302529f2278 100644 --- a/target/arm/arm-semi.c +++ b/target/arm/arm-semi.c @@ -217,10 +217,21 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb, gdb_do_syscallv(cb, fmt, va); va_end(va); - /* FIXME: we are implicitly relying on the syscall completing - * before this point, which is not guaranteed. We should - * put in an explicit synchronization between this and - * the callback function. + /* + * FIXME: in softmmu mode, the gdbstub will schedule our callback + * to occur, but will not actually call it to complete the syscall + * until after this function has returned and we are back in the + * CPU main loop. Therefore callers to this function must not + * do anything with its return value, because it is not necessarily + * the result of the syscall, but could just be the old value of X0. + * The only thing safe to do with this is that the callers of + * do_arm_semihosting() will write it straight back into X0. + * (In linux-user mode, the callback will have happened before + * gdb_do_syscallv() returns.) + * + * We should tidy this up so neither this function nor + * do_arm_semihosting() return a value, so the mistake of + * doing something with the return value is not possible to make. */ return is_a64(env) ? env->xregs[0] : env->regs[0];