From patchwork Wed Nov 15 23:05:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 24277 Received: (qmail 127656 invoked by alias); 15 Nov 2017 23:05:15 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 127636 invoked by uid 89); 15 Nov 2017 23:05:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=detects X-HELO: mail-oi0-f66.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=GAIFVTROY7mFS2xGek3p9nIf0xWRoS8CDNZHaRePS20=; b=MpAyThNA+g5y8vo6RaWwqs1gnU2YDH9796RO5bAPO1jI0PVmkdZ1npMuSrMLIAB/O/ zv6aCRv8BpaYn8P3cZdO/KO3GGtCj6toWOzYU2XGeMtGQmsODleAUX02TPpurPdVsBwV 5LGVdcrvV5Wrc0KsA/43Hv5h/bkfsc6AWvpMrZ98PGgSkBACtZb88GqHzU1YafAvqIZM MjWm1CVAsmWUvkvb/IXM8LDuQVyq6WH6yX5owVhmccJDI+NdinJe3x6BE8Fsotlsm0t1 mexX5ri5KJ/Oxx6eFEy+SwSyr4tIU12L1BPDsyEEPzTOWGu2NrzuMpLi7lqkLGGbyQyo 2onA== X-Gm-Message-State: AJaThX5bVrCqGLC9auvp4AxqMkjPfgAM3oghtaDMh5lK/vBslQqAt7Ur W0db0ZIDT80RsqazOG73lBJkyQ== X-Google-Smtp-Source: AGs4zMZ/5rtExNx+hvnuvmY84VBcPjPzMoA3arU+w2pNCg9xxjKs9yo1F8YW440kGoXkXbpOjFnoxg== X-Received: by 10.202.104.143 with SMTP id o15mr3899656oik.16.1510787111159; Wed, 15 Nov 2017 15:05:11 -0800 (PST) Subject: Re: [PATCH] add attribute nonstring To: sellcey@cavium.com, Joseph Myers References: <11c4caec-df98-12e6-c6f5-cfc95eb8b8d6@gmail.com> <1510783744.1836.25.camel@cavium.com> Cc: GNU C Library , Paul Eggert From: Martin Sebor Message-ID: Date: Wed, 15 Nov 2017 16:05:07 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1510783744.1836.25.camel@cavium.com> On 11/15/2017 03:09 PM, Steve Ellcey wrote: > On Fri, 2017-11-10 at 16:11 -0700, Martin Sebor wrote: >> >> Okay, thanks. I've read that you run periodic Glibc builds with >> the latest GCC so I wanted to give you a heads up on this change >> and the patch to avoid breaking them. Unless that's not a concern >> (do you use -Werror in those builds?) what is the best way to >> coordinate this update? >> >> I CC Paul for his input in case updating tzcode first is important. >> >> Martin > > Martin, were you going to update this patch with Joseph's comments and > resubmit? The tz changes are checked in and the ifreq struct is taken > care of so all that remains are the changes to cdefs.h to define > __NONSTRING and then to use in utmp.h. I don't have any comments on > that change beyond what Joseph made but I did do a build and run on > aarch64 with the latest GCC to verify that those changes work for me. Sure. I've got the attached patch. According to the Linux utmp man page: String fields are terminated by a null byte ('\0') if they are shorter than the size of the field. The patch lets Glibc compile fine with the top of GCC 8 trunk. As a heads up, the new attribute triggers warnings with my patch that enhances GCC to check for string functions being passed arrays declared with the attribute. Some are false positives that I'll try to handle in GCC but at least one of these looks like a Glibc bug. Assuming the GCC patch gets approved there may be some more fallout for Glibc to deal with. ../sysdeps/unix/getlogin_r.c:83:23: warning: ‘strlen’ argument 1 declared attribute ‘nonstring’ [-Wstringop-overflow=] size_t needed = strlen (ut->ut_user) + 1; ^~~~~~~~~~~~~~~~~~~~ In file included from ../login/utmp.h:29, from ../include/utmp.h:2, from ../sysdeps/unix/getlogin_r.c:26, from ../sysdeps/unix/sysv/linux/getlogin_r.c:25: ../sysdeps/gnu/bits/utmp.h:65:8: note: argument ‘ut_user’ declared here char ut_user[UT_NAMESIZE] ^~~~~~~ Martin The -Wstringop-truncation option new in GCC 8 detects common misuses of the strncat and strncpy function that may result in truncating the copied string before the terminating NUL. To avoid false positive warnings for correct code that intentionally creates sequences of characters that aren't guaranteed to be NUL-terminated, arrays that are intended to store such sequences should be decorated with a new nonstring attribute. This change add this attribute to Glibc and uses it to suppress such false positives. ChangeLog: * misc/sys/cdefs.h (__attribute_nonstring__): New macro. * sysdeps/gnu/bits/utmp.h (struct utmp): Use it. diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h index cfd39d5..a603cb9 100644 --- a/misc/sys/cdefs.h +++ b/misc/sys/cdefs.h @@ -407,6 +407,15 @@ # endif #endif +#if __GNUC_PREREQ (8, 0) +/* Describes a char array whose address can safely be passed as the first + argument to strncpy and strncat, as the char array is not necessarily + a NUL-terminated string. */ +# define __attribute_nonstring__ __attribute__ ((__nonstring__)) +#else +# define __attribute_nonstring__ +#endif + #if (!defined _Static_assert && !defined __cplusplus \ && (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) < 201112 \ && (!__GNUC_PREREQ (4, 6) || defined __STRICT_ANSI__)) diff --git a/sysdeps/gnu/bits/utmp.h b/sysdeps/gnu/bits/utmp.h index 2ee11cb..71c9fa2 100644 --- a/sysdeps/gnu/bits/utmp.h +++ b/sysdeps/gnu/bits/utmp.h @@ -59,10 +59,13 @@ struct utmp { short int ut_type; /* Type of login. */ pid_t ut_pid; /* Process ID of login process. */ - char ut_line[UT_LINESIZE]; /* Devicename. */ + char ut_line[UT_LINESIZE] + __attribute_nonstring__; /* Devicename. */ char ut_id[4]; /* Inittab ID. */ - char ut_user[UT_NAMESIZE]; /* Username. */ - char ut_host[UT_HOSTSIZE]; /* Hostname for remote login. */ + char ut_user[UT_NAMESIZE] + __attribute_nonstring__; /* Username. */ + char ut_host[UT_HOSTSIZE] + __attribute_nonstring__; /* Hostname for remote login. */ struct exit_status ut_exit; /* Exit status of a process marked as DEAD_PROCESS. */ /* The ut_session and ut_tv fields must be the same size when compiled