From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9AEBC4363C for ; Sat, 3 Oct 2020 01:14:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 43458206F8 for ; Sat, 3 Oct 2020 01:14:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="EEHUpZgD" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725554AbgJCBOO (ORCPT ); Fri, 2 Oct 2020 21:14:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41568 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725379AbgJCBON (ORCPT ); Fri, 2 Oct 2020 21:14:13 -0400 Received: from mail-vs1-xe41.google.com (mail-vs1-xe41.google.com [IPv6:2607:f8b0:4864:20::e41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 892EAC0613D0 for ; Fri, 2 Oct 2020 18:14:13 -0700 (PDT) Received: by mail-vs1-xe41.google.com with SMTP id 7so1445230vsp.6 for ; Fri, 02 Oct 2020 18:14:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=/yGtfiNcgJrUat/PWCT0Vc/JxltyE26ffS/ZkS/4iVQ=; b=EEHUpZgDu5KE0Q4tVfxvN0JPZZ0/MAbXwZ0v3s7R7HzmdBlmtc1hZGyazpuRU81d71 Eg8HwoMkYH3Jd01wFLHa3xEV+yN7M1+6dCWhDhTWTFanWXZWWjvhyIR3lZeGST8UlMBF NZrosBDT/ia0VweqWqc+jw+PvqUEEyKL/CteWVsDRP1zKsXvtNC5+BWY56c2VVJZYWUy 6xhkyP1sP62oKUXoXu92gc7VsaMyAX7jqxsMKj2XM89aOyqYK+qyV7EcXo5KFcnkAgix +PXit5f9d8M2ihWsRXRc1Pnu+jEwUbTp6FnGxt4OkNLfYGLtQf8cFuaQHWnQ7KbH3tj1 DPQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/yGtfiNcgJrUat/PWCT0Vc/JxltyE26ffS/ZkS/4iVQ=; b=KV5M6/te5vQ6302V5GvdbAE+whjNZbKphw8CzSagpW0V3czS7F4uQQuR92tLqnWFqS I0azDTnzB//oPkb2dH97cv8HWjblFChh8mb6mPRYfHJTUpBs7/n1LElgb0Vy6lcHTkqP 1UFkcIr4Pp19pJrd3L2ZfML+kBnhJy9RgqqCZ0gvU4vUH50srz5P26Nm6LIiLjV7+0zY b5U7IHNyOss7j9eAAyZPuRrtakZ9HLxcrtsY0wX80AMutcDYN0LjyNh+Kbkn4Wy4p3xb fEVWEJ5MKXPsyoCEdXMcfgLYghugXZYAu6EnEau6P0eRmHLLgwWd4xfaanMTILjWyVn6 yItA== X-Gm-Message-State: AOAM533C5WKiCl67LtyC+Vrf0IQglOAu0sK+TNvTEdze552kJyGjvjLW lS3qzaQz2LVRHTo4VWp9eayuv8kyDUlMP3/MiRQ50A== X-Google-Smtp-Source: ABdhPJxpHQAdVC98juCmyvvbjXri9dDMJtKbGXfTsyo9dQXQXpMX25+5vDA76DsF+/oUSHm8NZya2GYwd6KHIDR2i70= X-Received: by 2002:a67:8a8a:: with SMTP id m132mr574047vsd.14.1601687652296; Fri, 02 Oct 2020 18:14:12 -0700 (PDT) MIME-Version: 1.0 References: <20200908151223.GS6642@arm.com> In-Reply-To: <20200908151223.GS6642@arm.com> From: Peter Collingbourne Date: Fri, 2 Oct 2020 18:14:01 -0700 Message-ID: Subject: Re: [PATCH v10 2/7] arch: move SA_* definitions to generic headers To: Dave Martin Cc: Catalin Marinas , Evgenii Stepanov , Kostya Serebryany , Vincenzo Frascino , Will Deacon , Oleg Nesterov , "Eric W. Biederman" , "James E.J. Bottomley" , Parisc List , Andrey Konovalov , Kevin Brodsky , David Spickett , Linux ARM , Richard Henderson Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org On Tue, Sep 8, 2020 at 8:12 AM Dave Martin wrote: > > On Fri, Aug 21, 2020 at 10:10:12PM -0700, Peter Collingbourne wrote: > > Most architectures with the exception of alpha, mips, parisc and > > sparc use the same values for these flags. Move their definitions into > > asm-generic/signal-defs.h and allow the architectures with non-standard > > values to override them. Also, document the non-standard flag values > > in order to make it easier to add new generic flags in the future. > > > > Signed-off-by: Peter Collingbourne > > While this looks reasonable, I've just realised that you strip the "U" > from some arches' definitions here. > > So, on powerpc and x86, this changes the type of flags other than > SA_RESETHAND from unsigned int to int. > > While I can't see this breaking any sensible use of these flags, there's > a chance that there is software relying on this distinction by > accident. While it's true that it's technically possible that making these signed could change semantics, I'm having trouble seeing a realistic way in which software could be relying on this. Can you see one? I can think of cases like if the code does something like left shifts one of the flag bits into the sign bit (technically undefined behavior) and then right shifts it back (in C this would need to all be done in a single expression without storing to a variable; in C++ I suppose you could use auto to preserve the signedness in a variable's type). For example: int x = (SA_NODEFER << 1) >> 1; would give a different value to x if we made SA_NODEFER signed. But I wouldn't really expect software to be doing this sort of thing even accidentally, or much more than or'ing the flags together and assigning them to a variable, or passing them as a parameter, or some other operation which would fix the type. I believe that the kernel's uapi guarantee applies at the binary level, not at the source level. If that were not the case, I think we would not be allowed to add any new declaration to an existing .h file for fear of conflicting with a user program's identically spelled declaration. And that seems more likely to me than software that would do this sort of thing. > I wonder whether it's worth doing something like > > #ifdef ARCH_WANT_STRICTLY_UNSIGNED_SA_FLAGS > #define __SA_FLAG_VAL(x) x ## U > #else > #define __SA_FLAG_VAL(x) x > #endif > > #ifndef SA_NOCLDSTOP > #define SA_NOCLDSTOP __SA_FLAG_VAL(0x00000001) > #endif > > /* ... */ If we do this I would mildly prefer to keep the existing #defines in the arch-specific headers as if the arch had different flag values, as this would leave the arch-specific legacy cruft in the arch-specific headers where it belongs. > Mind you, the historical situation also has issues, e.g. because > sa_flags in struct sigaction is an int, assigning > > struct sigaction sa; > > sa.sa_flags = SA_RESETHAND; > > implies an overflow and so isn't portably safe (at least in theory). I > guess we are getting away with it today. Preserving the situation by > keeping the "U"s where appropriate would at least avoid making the > situation worse. I believe that the result of this assignment (involving an unsigned to signed conversion) is implementation defined and not undefined (which would be problematic). And in all the implementations that matter, as well as the C++ standard starting with C++20, this is a no-op cast assuming two's complement. I'm not sure what this has to do with making the constants signed because, as you pointed out, SA_RESETHAND would remain unsigned despite the absence of 'U' because its value does not fit in an int. Peter From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C0732C4363C for ; Sat, 3 Oct 2020 01:16:10 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 70EAD206F8 for ; Sat, 3 Oct 2020 01:16:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Q2Bpe/Kk"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="EEHUpZgD" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 70EAD206F8 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=i169lOk/0NXUn7Tay6HwrN4QWxKKdugLdhlw/fcydGk=; b=Q2Bpe/KkvH5pHcn69mT85nFOm 46qfuqGlHrX2rAl44y/n9Zvvs7s0aFvffixojS+yHiUDNln3B7FjZqbv0LmcttkoNgpgEQg9mcmMK JIZ8fmCYTIw+p3QTivmxOEeD2jtuGK2Oz9gISQLghO8nbQKO0xIcVRY5ojUxghfViHp/0FQPX5uAY 0TU6kaWMJnwXxNrVK7Hv4wsEi7vy02RxfBEf9dcL3AZEUVnFvO5g5So9ztAsDIrY1z7RcwNIsLSqV ozyvP4pb2lq21EA0KQmg8HdbWjtHS4ln26zG3w2zVe6bm9G9cbh4V/R+TqFPRTUAfwD+5ATOIwukS DKHcR7c1Q==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kOW7u-0001xS-HY; Sat, 03 Oct 2020 01:14:18 +0000 Received: from mail-vs1-xe41.google.com ([2607:f8b0:4864:20::e41]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kOW7r-0001x8-S7 for linux-arm-kernel@lists.infradead.org; Sat, 03 Oct 2020 01:14:17 +0000 Received: by mail-vs1-xe41.google.com with SMTP id g11so1447944vsp.13 for ; Fri, 02 Oct 2020 18:14:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=/yGtfiNcgJrUat/PWCT0Vc/JxltyE26ffS/ZkS/4iVQ=; b=EEHUpZgDu5KE0Q4tVfxvN0JPZZ0/MAbXwZ0v3s7R7HzmdBlmtc1hZGyazpuRU81d71 Eg8HwoMkYH3Jd01wFLHa3xEV+yN7M1+6dCWhDhTWTFanWXZWWjvhyIR3lZeGST8UlMBF NZrosBDT/ia0VweqWqc+jw+PvqUEEyKL/CteWVsDRP1zKsXvtNC5+BWY56c2VVJZYWUy 6xhkyP1sP62oKUXoXu92gc7VsaMyAX7jqxsMKj2XM89aOyqYK+qyV7EcXo5KFcnkAgix +PXit5f9d8M2ihWsRXRc1Pnu+jEwUbTp6FnGxt4OkNLfYGLtQf8cFuaQHWnQ7KbH3tj1 DPQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/yGtfiNcgJrUat/PWCT0Vc/JxltyE26ffS/ZkS/4iVQ=; b=gHHlvNUhLHOWyFxilcAc4fzJmptbkWjyypEZVmjhBDubh5BMKt6mFT/sQVHLq8TFMD D6J6KHmhj6/ZIODajdaKjkerzpY5/LnS0Ylsr8IIvVrZMMh50MXf54vWB4c6NwqFjxyH 2ewmbLBExPAJQ+4qvjdxhHUBnTBuF9t7/SQspkQIaFDnq72U7YdgOD/mOsyCAb2rfOJI X5aLcVqB1PBP0DhxsEaPpzzx5evTD8PrVUxZOnj3llrEJG+uVfPGDGN56wFp/sJoJtag Gqcy8FVRdP+Yl2jq8qJattNO+CK9sc6S1YMI/jltw+nCl1IHrhbLFFeTVnutBm+tRwCj WZXg== X-Gm-Message-State: AOAM532+wNje+8+YonzOaz+9C5zBKr7T+fiAVcM52HgL+ccd6XKN2bMn ihY3zjgi4KrhVuZuv+YowpDecJhxt+/2nM87/AsFgw== X-Google-Smtp-Source: ABdhPJxpHQAdVC98juCmyvvbjXri9dDMJtKbGXfTsyo9dQXQXpMX25+5vDA76DsF+/oUSHm8NZya2GYwd6KHIDR2i70= X-Received: by 2002:a67:8a8a:: with SMTP id m132mr574047vsd.14.1601687652296; Fri, 02 Oct 2020 18:14:12 -0700 (PDT) MIME-Version: 1.0 References: <20200908151223.GS6642@arm.com> In-Reply-To: <20200908151223.GS6642@arm.com> From: Peter Collingbourne Date: Fri, 2 Oct 2020 18:14:01 -0700 Message-ID: Subject: Re: [PATCH v10 2/7] arch: move SA_* definitions to generic headers To: Dave Martin X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201002_211416_013144_6F71AD3D X-CRM114-Status: GOOD ( 26.80 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Linux ARM , Parisc List , Catalin Marinas , Kevin Brodsky , Oleg Nesterov , "James E.J. Bottomley" , Kostya Serebryany , "Eric W. Biederman" , Andrey Konovalov , David Spickett , Vincenzo Frascino , Will Deacon , Evgenii Stepanov , Richard Henderson Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Sep 8, 2020 at 8:12 AM Dave Martin wrote: > > On Fri, Aug 21, 2020 at 10:10:12PM -0700, Peter Collingbourne wrote: > > Most architectures with the exception of alpha, mips, parisc and > > sparc use the same values for these flags. Move their definitions into > > asm-generic/signal-defs.h and allow the architectures with non-standard > > values to override them. Also, document the non-standard flag values > > in order to make it easier to add new generic flags in the future. > > > > Signed-off-by: Peter Collingbourne > > While this looks reasonable, I've just realised that you strip the "U" > from some arches' definitions here. > > So, on powerpc and x86, this changes the type of flags other than > SA_RESETHAND from unsigned int to int. > > While I can't see this breaking any sensible use of these flags, there's > a chance that there is software relying on this distinction by > accident. While it's true that it's technically possible that making these signed could change semantics, I'm having trouble seeing a realistic way in which software could be relying on this. Can you see one? I can think of cases like if the code does something like left shifts one of the flag bits into the sign bit (technically undefined behavior) and then right shifts it back (in C this would need to all be done in a single expression without storing to a variable; in C++ I suppose you could use auto to preserve the signedness in a variable's type). For example: int x = (SA_NODEFER << 1) >> 1; would give a different value to x if we made SA_NODEFER signed. But I wouldn't really expect software to be doing this sort of thing even accidentally, or much more than or'ing the flags together and assigning them to a variable, or passing them as a parameter, or some other operation which would fix the type. I believe that the kernel's uapi guarantee applies at the binary level, not at the source level. If that were not the case, I think we would not be allowed to add any new declaration to an existing .h file for fear of conflicting with a user program's identically spelled declaration. And that seems more likely to me than software that would do this sort of thing. > I wonder whether it's worth doing something like > > #ifdef ARCH_WANT_STRICTLY_UNSIGNED_SA_FLAGS > #define __SA_FLAG_VAL(x) x ## U > #else > #define __SA_FLAG_VAL(x) x > #endif > > #ifndef SA_NOCLDSTOP > #define SA_NOCLDSTOP __SA_FLAG_VAL(0x00000001) > #endif > > /* ... */ If we do this I would mildly prefer to keep the existing #defines in the arch-specific headers as if the arch had different flag values, as this would leave the arch-specific legacy cruft in the arch-specific headers where it belongs. > Mind you, the historical situation also has issues, e.g. because > sa_flags in struct sigaction is an int, assigning > > struct sigaction sa; > > sa.sa_flags = SA_RESETHAND; > > implies an overflow and so isn't portably safe (at least in theory). I > guess we are getting away with it today. Preserving the situation by > keeping the "U"s where appropriate would at least avoid making the > situation worse. I believe that the result of this assignment (involving an unsigned to signed conversion) is implementation defined and not undefined (which would be problematic). And in all the implementations that matter, as well as the C++ standard starting with C++20, this is a no-op cast assuming two's complement. I'm not sure what this has to do with making the constants signed because, as you pointed out, SA_RESETHAND would remain unsigned despite the absence of 'U' because its value does not fit in an int. Peter _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel