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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 38650C433EF for ; Mon, 15 Nov 2021 22:14:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 19AC561B2A for ; Mon, 15 Nov 2021 22:14:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233484AbhKOWRa (ORCPT ); Mon, 15 Nov 2021 17:17:30 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:24491 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349320AbhKOWQA (ORCPT ); Mon, 15 Nov 2021 17:16:00 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1637014376; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oVrT+kPL+/IZxc9ZH3XRHjagD4ZEneavCS/VVElDdZ8=; b=PfOOwyWbNhiz3HkI1/eZjihVwEJYXp6qWbeE4ubO/Jmizp4AzYOWylOjeP/2Neh/CKxGc/ 3uFvlMAxSXe+IkCZFIkHPr2bBe8qJ1RXwlAOORCwWDRHPNo2fKcdD2uTqvx0mf9cCnznmn KrHUWb6FK1juLAuPsd62pYYohF/xPRM= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-580-bkDRkG8qOoWXd8CRajdO0w-1; Mon, 15 Nov 2021 17:12:55 -0500 X-MC-Unique: bkDRkG8qOoWXd8CRajdO0w-1 Received: by mail-qv1-f70.google.com with SMTP id ke1-20020a056214300100b003b5a227e98dso17348684qvb.14 for ; Mon, 15 Nov 2021 14:12:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=oVrT+kPL+/IZxc9ZH3XRHjagD4ZEneavCS/VVElDdZ8=; b=21eoayUd9iwzxAus0S+2ZTTNH+7r9YeMRpTRBiQUsEtVojto925oX36e7FLbN0ZhI0 SSaAtdnvJakF9dir2g/eBbRrf/FJdLo2yGJ52tmRdWOYVbIgJE05+3p+Cqv+ECiOMRcZ n0yX3qXRAmn/TlkLMGD2HR2lPkDbO1/0NF9iNOVhbFw19j2zikmlTbhHx3quTsbdV2GJ SKjyLuC4lMKrm3oC/WYtG3JfGVUF9r/Klb+oGWyVbrepkkEV5Y+gKjWhkO6MjCZ7R/gZ 8b29AhJEJ6L5zKg9y7Od3ouFXYWzEKm+qTJQeg0ewDjxxrsuB28I/Ul0t05d1BUQ2FrA ThyA== X-Gm-Message-State: AOAM533Djuy3JaF/CdfC98OSMAIRoREHy/5L3/U/BFNCZou1aBspV8Eb 2Tygb7zzB/cSzWZ2Jg3y2ZvsQqXmxFg+zbWiX1CsyCxYnsmHXO/dIPjMvGYZkOf0AsolDCREYRg 6L904P3kWXUgcQuBpNtyNkuVJo0fxew== X-Received: by 2002:a05:620a:14bb:: with SMTP id x27mr2095908qkj.382.1637014374705; Mon, 15 Nov 2021 14:12:54 -0800 (PST) X-Google-Smtp-Source: ABdhPJyYaLbZB4mu4VUS5sZ9b9+mcJnS1O3SmjBaGR6XISBtCd4QTIaqWVfcK2v0sVE3mACvpa1RoQ== X-Received: by 2002:a05:620a:14bb:: with SMTP id x27mr2095882qkj.382.1637014374421; Mon, 15 Nov 2021 14:12:54 -0800 (PST) Received: from t14s.localdomain (c-73-69-212-193.hsd1.nh.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id az14sm3037208qkb.97.2021.11.15.14.12.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 Nov 2021 14:12:53 -0800 (PST) Message-ID: <15adb3a2a70b0d2973c30dd6d7da383ad62f413a.camel@redhat.com> Subject: Re: [PATCH 2/6] Add returns_zero_on_success/failure attributes From: David Malcolm To: Prathamesh Kulkarni Cc: gcc-patches@gcc.gnu.org, linux-toolchains@vger.kernel.org Date: Mon, 15 Nov 2021 17:12:52 -0500 In-Reply-To: References: <20211113203732.2098220-1-dmalcolm@redhat.com> <20211113203732.2098220-4-dmalcolm@redhat.com> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmalcolm@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-toolchains@vger.kernel.org On Mon, 2021-11-15 at 12:33 +0530, Prathamesh Kulkarni wrote: > On Sun, 14 Nov 2021 at 02:07, David Malcolm via Gcc-patches > wrote: > > > > This patch adds two new attributes.  The followup patch makes use of > > the attributes in -fanalyzer. [...snip...] > > +/* Handle "returns_zero_on_failure" and "returns_zero_on_success" > > attributes; > > +   arguments as in struct attribute_spec.handler.  */ > > + > > +static tree > > +handle_returns_zero_on_attributes (tree *node, tree name, tree, > > int, > > +                                  bool *no_add_attrs) > > +{ > > +  if (!INTEGRAL_TYPE_P (TREE_TYPE (*node))) > > +    { > > +      error ("%qE attribute on a function not returning an > > integral type", > > +            name); > > +      *no_add_attrs = true; > > +    } > > +  return NULL_TREE; > Hi David, Thanks for the ideas. > Just curious if a warning should be emitted if the function is marked > with the attribute but it's return value isn't actually 0 ? That sounds like a worthwhile extension of the idea. It should be possible to identify functions that can't return zero or non-zero that have been marked as being able to. That said: (a) if you apply the attribute to a function pointer for a callback, you could have an implementation of the callback that always fails and returns, say, -1; should the warning complain that the function has the "returns_zero_on_success" property and is always returning -1? (b) the attributes introduce a concept of "success" vs "failure", which might be hard for a machine to determine. It's only used later on in terms of the events presented to the user, so that -fanalyzer can emit e.g. "when 'copy_from_user' fails", which IMHO is easier to read than "when 'copy_from_user' returns non-zero". > > There are other constants like -1 or 1 that are often used to indicate > error, so maybe tweak the attribute to > take the integer as an argument ? > Sth like returns_int_on_success(cst) / returns_int_on_failure(cst) ? Those could work nicely; I like the idea of them being supplementary to the returns_zero_on_* ones. I got the urge to bikeshed about wording; some ideas: success_return_value(CST) failure_return_value(CST) or maybe additionally: success_return_range(LOWER_BOUND_CST, UPPER_BOUND_CST) failure_return_range(LOWER_BOUND_CST, UPPER_BOUND_CST) I can also imagine a sets_errno_on_failure attribute being useful (and perhaps a "doesnt_touch_errno"???) > Also, would it make sense to extend it for pointers too for returning > NULL on success / failure ? Possibly expressible by generalizing it to allow pointer types, or by adding this pair: returns_null_on_failure returns_null_on_success or by using the "range" idea above. In terms of scope, for the trust boundary stuff, I want to be able to express the idea that a call can succeed vs fail, what the success vs failure is in terms of nonzero vs zero, and to be able to wire up the heuristic that if it looks like a "copy function" (use of access attributes and a size), that success/failure can mean "copies all of it" vs "copies none of it" (which seems to get decent test coverage on the Linux kernel with the copy_from/to_user fns). Thanks Dave > > Thanks, > Prathamesh [...snip...]