From patchwork Wed Jan 4 13:05:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Andre Vieira (lists)" X-Patchwork-Id: 710965 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ttrdc5HLWz9vDS for ; Thu, 5 Jan 2017 00:05:22 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="bZrvfgFa"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=P/cyYGrK0ou1UF6KM FuwFBbgw6Orx/xaRK6Wywv8l4ibsgernE1IvBshmTbUSnKjTcuieu0cVgKacHjbW adfK0E2D4AwoBMaDnt7cHpL2qVt1BNgybyzoJ3vxnIxyMerJAuYf25ZbkJ3M7gia 8Q/rYI3M8WVLGIB6b9VR4eTgaM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=MzaTptX4o7g790Zh/DXNfVG 2iVQ=; b=bZrvfgFapspNh7oUKVOIw+tpk3r4jgmanmkQC2CtjBanse41rdKBWhr HsSJxJHFd05Vs3BA9rEzcTDYbyXZtbz1Pc6sbNGc3ufdnq0/YNMTj/9snTjWpelN 4R9hXUmRyEjMHTqS6frnYC1NXVUazxHITHVxz2fQ0OYxsFmN3VhY= Received: (qmail 75731 invoked by alias); 4 Jan 2017 13:05:12 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 75706 invoked by uid 89); 4 Jan 2017 13:05:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00, KAM_LOTSOFHASH, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=tkachov, Tkachov, H*f:sk:586CDCC, H*i:sk:586CDCC X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 Jan 2017 13:05:06 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D66A8AD7; Wed, 4 Jan 2017 05:05:04 -0800 (PST) Received: from [10.2.206.251] (e107157-lin.cambridge.arm.com [10.2.206.251]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 64AB43F242; Wed, 4 Jan 2017 05:05:04 -0800 (PST) Subject: Re: [PATCH][ARM] ARMv8-M Security Extensions: Warn for unused result for some intrinsics To: Kyrill Tkachov , GCC Patches References: <586CDAD3.6010408@arm.com> <586CDCC3.4010301@foss.arm.com> From: "Andre Vieira (lists)" Message-ID: <586CF2FF.30709@arm.com> Date: Wed, 4 Jan 2017 13:05:03 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <586CDCC3.4010301@foss.arm.com> X-IsSubscribed: yes On 04/01/17 11:30, Kyrill Tkachov wrote: > Hi Andre, > > On 04/01/17 11:21, Andre Vieira (lists) wrote: >> Hello, >> >> This patch adds the attribute "warn_unused_result" to the following >> intrinsics: >> __cmse_TT{,A,AT,T}_fptr >> cmse_TT{,A,AT,T} >> cmse_nonsecure_caller >> cmse_check_address_range >> >> If the result of these intrinsics is not used it means the result of the >> checks they perform are never used and that could become the source of a >> security vulnerability in the user's code. We hope this will limit >> these. >> >> Due to the current limitations of "warn_unused_result", adding them to >> the __cmse_TT*_fptr intrinsics is pointless since the user will most >> likely use the macro 'cmse_TT*_fptr' instead, which casts the result of >> __cmse_TT*_fptr and that seems to be enough to count as a "use". I >> decided to leave them in there anyway in case the warning becomes a bit >> smarter in the future. Warnings for cmse_check_pointed_object will never >> be issued for the same reason. Also if you assign the result of any of >> these intrinsics to a variable you never use, you will only get a >> warning about an unused variable, though this warning is not turned on >> by default. >> >> Ran cmse regression tests for arm-none-eabi both ARMv8-M Baseline and >> Mainline. >> >> Is this OK for stage 3? >> >> Cheers, >> Andre >> >> gcc/ChangeLog: >> 2017-01-04 Andre Vieira >> >> * config/gcc/arm_cmse.h (__cmse_TT_fptr,__cmse_TTA_fptr, >> __cmse_TTAT_fptr,__cmse_TTT_fptr,cmse_TT, cmse_TTA, cmse_TTAT, >> cmse_TTT, cmse_nonsecure_caller, cmse_check_address_range): >> Add warn_unused_result attribute to function declaration. >> >> gcc/testsuite/ChangeLog: >> 2017-01-04 Andre Vieira >> >> * gcc.target/arm/cmse/cmse-3.c: Add warning tests for the >> warn_unused_result warning. > > > diff --git a/gcc/config/arm/arm_cmse.h b/gcc/config/arm/arm_cmse.h > index > 82b58b1c4f4a12ba6062e2cc2632653788d0eeb7..d37f4e2b446c3c80d56af8b633323837f327973f > 100644 > --- a/gcc/config/arm/arm_cmse.h > +++ b/gcc/config/arm/arm_cmse.h > @@ -116,11 +116,13 @@ typedef void (*__cmse_fptr)(void); > } > > __extension__ static __inline __attribute__ ((__always_inline__)) > +__attribute__ ((__warn_unused_result__)) > > Don't add a second __attribute__ annotation, change the first one to be: > __attribute__ ((__always_inline__, __warn_unused_result__)) > > > Ok with that change. > Thanks, > Kyrill > Hi Kyrill, Missed that for some reason... Changes don't affect ChangeLogs. Cheers, Andre diff --git a/gcc/config/arm/arm_cmse.h b/gcc/config/arm/arm_cmse.h index 82b58b1c4f4a12ba6062e2cc2632653788d0eeb7..04983582dac633b649d08ce673e0678c059acf05 100644 --- a/gcc/config/arm/arm_cmse.h +++ b/gcc/config/arm/arm_cmse.h @@ -115,24 +115,28 @@ typedef void (*__cmse_fptr)(void); return __result; \ } -__extension__ static __inline __attribute__ ((__always_inline__)) +__extension__ static __inline +__attribute__ ((__always_inline__, __warn_unused_result__)) cmse_address_info_t __cmse_TT_fptr (__cmse_fptr __p) __CMSE_TT_ASM () -__extension__ static __inline __attribute__ ((__always_inline__)) +__extension__ static __inline +__attribute__ ((__always_inline__, __warn_unused_result__)) cmse_address_info_t cmse_TT (void *__p) __CMSE_TT_ASM () #define cmse_TTT_fptr(p) (__cmse_TTT_fptr ((__cmse_fptr)(p))) -__extension__ static __inline __attribute__ ((__always_inline__)) +__extension__ static __inline +__attribute__ ((__always_inline__, __warn_unused_result__)) cmse_address_info_t __cmse_TTT_fptr (__cmse_fptr __p) __CMSE_TT_ASM (t) -__extension__ static __inline __attribute__ ((__always_inline__)) +__extension__ static __inline +__attribute__ ((__always_inline__, __warn_unused_result__)) cmse_address_info_t cmse_TTT (void *__p) __CMSE_TT_ASM (t) @@ -141,12 +145,14 @@ __CMSE_TT_ASM (t) #define cmse_TTA_fptr(p) (__cmse_TTA_fptr ((__cmse_fptr)(p))) -__extension__ static __inline __attribute__ ((__always_inline__)) +__extension__ static __inline +__attribute__ ((__always_inline__, __warn_unused_result__)) cmse_address_info_t __cmse_TTA_fptr (__cmse_fptr __p) __CMSE_TT_ASM (a) -__extension__ static __inline __attribute__ ((__always_inline__)) +__extension__ static __inline +__attribute__ ((__always_inline__, __warn_unused_result__)) cmse_address_info_t cmse_TTA (void *__p) __CMSE_TT_ASM (a) @@ -154,17 +160,18 @@ __CMSE_TT_ASM (a) #define cmse_TTAT_fptr(p) (__cmse_TTAT_fptr ((__cmse_fptr)(p))) __extension__ static __inline cmse_address_info_t -__attribute__ ((__always_inline__)) +__attribute__ ((__always_inline__, __warn_unused_result__)) __cmse_TTAT_fptr (__cmse_fptr __p) __CMSE_TT_ASM (at) __extension__ static __inline cmse_address_info_t -__attribute__ ((__always_inline__)) +__attribute__ ((__always_inline__, __warn_unused_result__)) cmse_TTAT (void *__p) __CMSE_TT_ASM (at) /* FIXME: diagnose use outside cmse_nonsecure_entry functions. */ -__extension__ static __inline int __attribute__ ((__always_inline__)) +__extension__ static __inline int +__attribute__ ((__always_inline__, __warn_unused_result__)) cmse_nonsecure_caller (void) { return __builtin_arm_cmse_nonsecure_caller (); @@ -184,7 +191,7 @@ cmse_nonsecure_caller (void) #define CMSE_MPU_READWRITE 1 #define CMSE_MPU_READ 8 -__extension__ void * +__extension__ void * __attribute__ ((__warn_unused_result__)) cmse_check_address_range (void *, size_t, int); #define cmse_check_pointed_object(p, f) \ diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c index 7f92a4c28b3333e4c8fdc256211f3ed74a383cd4..fd3cd282546b9eee10b7d5730f9096084502c492 100644 --- a/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c @@ -43,3 +43,12 @@ typedef void __attribute__ ((cmse_nonsecure_call)) baz2 (long long a, int b, str typedef struct span __attribute__ ((cmse_nonsecure_call)) qux2 (void); /* { dg-error "not available to functions that return value on the stack" } */ typedef void __attribute__ ((cmse_nonsecure_call)) norf2 (int a, ...); /* { dg-error "not available to functions with variable number of arguments" } */ + +#include + +void foo3 (void * p, size_t s, int r) +{ + cmse_TT (p); /* { dg-warning "ignoring return value of" } */ + cmse_check_address_range (p, s, r); /* { dg-warning "ignoring return value of" } */ + cmse_nonsecure_caller (); /* { dg-warning "ignoring return value of" } */ +}