This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Variadic macros in installed headers


On 11/23/19 6:43 AM, Florian Weimer wrote:
This is about bug 25219, which proposes this patch:

+#if __GNUC_PREREQ (10, 0)
+/* Denotes a function pointer argument can be used to access an array
+   of N elements according to access mode.  */
+#  define __rdonly(...)  \
+  __attribute__ ((__access__ (__read_only__, __VA_ARGS__)))
+#  define __rdwr(...)  \
+  __attribute__ ((__access__ (__read_write__, __VA_ARGS__)))
+#  define __wronly(...) \
+  __attribute__ ((__access__ (__write_only__, __VA_ARGS__)))
+#else
+#  define __rdonly(...)
+#  define __rdwr(...)
+#  define __wronly(...)
+#endif

This makes the relevant headers incompatible with C89.  I think the
attribute definition either needs to be changed to require parenthesis
around the arguments (similar to __atribute__ itself), or we need to
use multiple macro definitions for each access mode.

The patch in the bug just a very small proof of concept to show
how the attribute can be used to improve buffer overflow detection.
I've been testing GCC with a bigger patch that annotates a bunch
more Glibc functions, but I haven't had time to finish it and
I don't expect to until at least the end of GCC stage 3.  I opened
the Glibc bug as a heads up that this feature is available now, in
case someone (such as yourself) wants to experiment it or even get
it done in the meantime.  Feedback on the feature (bugs and all)
is of course welcome -- thank you!

The syntax of the new attribute follows the pattern of attribute
format and I don't think it can easily be changed.  I suspected
the variadic macros would be a problem for Glibc but I'm not sure
how to get around it other than by providing two macros for each
access mode.  I'm open to ideas.


I also dislike that the attribute does not use symbolic expressions.
It means that it cannot be used to annotate functions like asctime_r
directly.  C++-like pointer-pairs also need auxiliary function
definitions.

The attribute is not without limitations.  Dealing with fixed-size
buffers is one of the things I've been pondering for a while but
haven't yet decided on a good solution for.  Another is marking up
strings.  Yet another is arrays of arrays.

One of the uses for this infrastructure that Joseph pointed out is
VLAs (as function arguments).  I hope to add that in GCC 11 so that
something like

  void f (int a[n], unsigned n);

will be automatically annotated as if it had been written like so:

  __attribute__ ((access (read_write, 1, 2)))
  void f (int a[n], unsigned n);

I thought a natural extension of the notation for fixed size arrays
might be to use the constant bound as the least number of elements,
like this:

  __attribute__ ((access (write_only, 2))) char*
  asctime_r (const struct tm *restrict tm, char buf[restrict 26]);

The only alternatives that come to mind are various extensions
of the attribute, none of which I'm happy with.

I'm not sure I know what you mean by C++-like pointer-pairs.

Martin

PS C++ contracts expose a much more powerful and general interface
than this.  I'm not up on the details of the latest proposals but
I don't think the following would be too far off the mark:

  [[assert: __builtin_object_size (buf) >= 26]] char*
  asctime_r (const struct tm *restrict tm, char *restrict buf);

Of course, since __builtin_object_size is limited to constants
(for now) the above wouldn't be much of an improvement over
what's possible today with the cumbersome checking wrappers.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]