This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC][PATCH v2] Initial support for C11 Annex K Bounds checking functions
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Ulrich Bayer <ubayer at sba-research dot org>
- Cc: <libc-alpha at sourceware dot org>
- Date: Wed, 5 Jun 2013 17:03:51 +0000
- Subject: Re: [RFC][PATCH v2] Initial support for C11 Annex K Bounds checking functions
- References: <5102DBFD dot 4060103 at sba-research dot org> <513DE7A1 dot 8080501 at sba-research dot org> <Pine dot LNX dot 4 dot 64 dot 1305151554100 dot 15900 at digraph dot polyomino dot org dot uk> <51AF6406 dot 30201 at sba-research dot org>
On Wed, 5 Jun 2013, Ulrich Bayer wrote:
> diff --git a/include/features.h b/include/features.h
> index ca83da0..f9b805d 100644
> --- a/include/features.h
> +++ b/include/features.h
> @@ -45,7 +45,8 @@
> _THREAD_SAFE Same as _REENTRANT, often used by other systems.
> _FORTIFY_SOURCE If set to numeric value > 0 additional security
> measures are defined, according to level.
> -
> + __STDC_WANT_LIB_EXT1__ Includes bounds checking functions (with postfix _s)
> + specified in Annex K of ISO C11.
> The `-ansi' switch to the GNU C compiler defines __STRICT_ANSI__.
> If none of these are defined, the default is to have _SVID_SOURCE,
> _BSD_SOURCE, and _POSIX_SOURCE set to one and _POSIX_C_SOURCE set to
> @@ -81,6 +82,8 @@
> __USE_REENTRANT Define reentrant/thread-safe *_r functions.
> __USE_FORTIFY_LEVEL Additional security measures used, according to level.
> __FAVOR_BSD Favor 4.3BSD things in cases of conflict.
> + __USE_BOUNDS_CHECKING Define the bounds checking interfaces from
> + C11 Annex K.
The definition of __STDC_WANT_LIB_EXT1__ is that it is only relevant on
inclusion of a header in K.3 (that is, <errno.h>, <stddef.h>, <stdint.h>,
<stdio.h>, <stdlib.h>, <string.h>, <time.h>, <wchar.h>) - and not relevant
on inclusion of any other ISO C header.
Thus, checking it only in <features.h>, then using __USE_BOUNDS_CHECKING
in other headers, is incorrect. Instead, the relevant headers need to
check __STDC_WANT_LIB_EXT1__ directly. The only thing <features.h> can
usefully do is to define __STDC_WANT_LIB_EXT1__ if the user defined
_GNU_SOURCE (if we want these interfaces to be part of the _GNU_SOURCE API
- and normally all interfaces do go in that API).
> +CFLAGS-libc_s := -D__STDC_WANT_LIB_EXT1__=1
If you include it in _GNU_SOURCE, this should no longer be necessary.
> diff --git a/lib_s/abort_handler_s.c b/lib_s/abort_handler_s.c
> new file mode 100644
> index 0000000..75b7fef
> --- /dev/null
> +++ b/lib_s/abort_handler_s.c
> @@ -0,0 +1,38 @@
> +/* Copyright (C) 2013 Free Software Foundation, Inc.
The first line of each new file should be a descriptive comment. The
copyright notice comes after that description of the file. (Comment
applies to other files as well.)
> +void
> +abort_handler_s (const char *restrict msg, void *restrict ptr, errno_t error)
Please include a comment on each function definition with a brief
description of the function semantics and those of the parameters, as
described in the GNU Coding Standards. (Likewise for other functions.)
> +/* The strcpy_s function copies the string s2 to s1.
> + Unlike the traditional strcpy, the size of the destination buffer s1
> + has to be specified. The function makes sure that it never
> + writes outside the specified bounds.
> +*/
> +extern errno_t strcpy_s (char * __restrict__ s1, rsize_t s1max,
> + const char * __restrict__ s2)
> + __THROW __nonnull ((1,3));
I don't think it's appropriate to use __THROW on any function with runtime
constraints; it's reasonable for someone to set a runtime constraint
handler that throws C++ exceptions.
Likewise, it's inappropriate to use the nonnull attribute on arguments
where the semantics if those arguments are NULL is specified (as a runtime
constraint violation), because GCC may optimize based on the argument not
being NULL - the attribute can be used only when a NULL argument means
undefined behavior.
(Likewise for other functions affected.)
> + "For implementations targeting machines with large address spaces,
> + it is recommended that RSIZE_MAX be defined as the smaller of the
> + size of the largest object supported or(SIZE_MAX >> 1), even if this
Missing space between "or" and '('.
--
Joseph S. Myers
joseph@codesourcery.com