[cfe-dev] RFC: Source-level attribute for conservative __builtin_object_size(..., 1)

[Hal Finkel via cfe-dev]

----- Original Message -----
> From: "Bob Wilson via cfe-dev" <cfe-dev at lists.llvm.org>
> To: "Duncan P. N. Exon Smith" <dexonsmith at apple.com>
> Cc: "Richard Smith" <richard at metafoo.co.uk>, "CFE Dev" <cfe-dev at lists.llvm.org>, "George Burgess IV via llvm-commits"
> <llvm-commits at lists.llvm.org>
> Sent: Monday, March 7, 2016 4:30:57 PM
> Subject: Re: [cfe-dev] RFC: Source-level attribute for conservative __builtin_object_size(..., 1)
> 
> 
> > On Feb 25, 2016, at 12:43 PM, Duncan P. N. Exon Smith
> > <dexonsmith at apple.com> wrote:
> > 
> > Context
> > =======
> > 
> > r245403 and follow-ups improved __builtin_object_size(ptr, 1).  The
> > type=1
> > version of __builtin_object_size returns the maximum possible
> > distance
> > between `ptr` and the end of the subobject it's part of.  E.g., in:
> > ```
> > struct S {
> >   int i[2];
> >   char c[5];
> > };
> > ```
> > then __builtin_object_size(s.i, 1) should return 8, and
> > __builtin_object_size(s.c + 2, 1) should return 3.
> > 
> > r250488 relaxed the rules for 0- or 1-sized arrays.  Consider
> > ```
> > struct S {
> >   int i[2];
> >   char c[0];
> > };
> > ```
> > The expectation is that S will be over-allocated by some number of
> > bytes,
> > so that `S::c` has a runtime-determined array size.  r250488
> > changed
> > __builtin_object_size(ptr, 1) to be more conservative in this case,
> > falling
> > back to "type=0".  (Type=0 returns the maximum possible distance
> > between
> > `ptr` and the end of the allocation.)
> > 
> > Problem
> > =======
> > 
> > We use __builtin_object_size(ptr, 1) in our strcpy implementation
> > when
> > _FORTIFY_SOURCE=2 (the default).  The code is something equivalent
> > to:
> > ```
> > #define strcpy(a, b, sz) \
> >   __builtin_strcpy_chk(a, b, sz, __builtin_object_size(a, 1))
> > ```
> > 
> > This is causing a problem for some
> > structs in our headers that end with a char array that has a size >
> > 1 and
> > that are expected to be over-allocated.
> > 
> > One example is `sockaddr_un`.  From the unix(4) manpage:
> > http://www.unix.com/man-page/FreeBSD/4/unix/
> > ```
> > struct sockaddr_un {
> >   u_char  sun_len;
> >   u_char  sun_family;
> >   char    sun_path[104];
> > };
> > ```
> > 
> > This has been around a long time.  Unfortunately,
> > `sizeof(sockaddr_un)`
> > cannot really change, and there's a ton of code out there that uses
> > strcpy() on this struct.  We need some way for clang to be
> > conservative
> > in this case.
> > 
> > Solution
> > ========
> > 
> > We're thinking about adding an attribute, something like:
> > ```
> > struct sockaddr_un {
> >   u_char  sun_len;
> >   u_char  sun_family;
> >   char    sun_path[104] __attribute__((variable_length_array));
> > };
> > ```
> > (is there a better name?).  This would allow us to decorate our
> > headers,
> > and explicitly opt-in on a struct-by-struct basis to the
> > conservative
> > behaviour that r250488 gives to 0- and 1-sized arrays.
> > 
> > The only other option we can think of us to be conservative
> > whenever the
> > the subobject is part of an array at the end of a struct.
> > 
> > Thoughts?
> 
> Ping. Does anyone have feedback on this?

If you can add an attribute to your headers, this seems like a good solution. We might also need the conservative option for dealing with older headers. Regarding the attribute, is is really a property of the array member, or a property of the structure (i.e. that it will be overallocated)?

FWIW, the proposed attribute seems independently useful for eliminating unnecessary warnings. For example, if we have:

void foo(struct sockaddr_un *s) {
  s->sun_path[106] = 0;
}

we generate this:

warning: array index 106 is past the end of the array (which contains 104 elements) [-Warray-bounds]
  s->sun_path[106] = 0;

which, as you describe the situation, is not useful.

 -Hal

> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory