r281277 - [Sema] Fix PR30346: relax __builtin_object_size checks.

[Richard Smith via cfe-commits]

On Tue, Sep 13, 2016 at 10:44 AM, Joerg Sonnenberger via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> On Mon, Sep 12, 2016 at 11:50:36PM -0000, George Burgess IV via
> cfe-commits wrote:
> > Author: gbiv
> > Date: Mon Sep 12 18:50:35 2016
> > New Revision: 281277
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=281277&view=rev
> > Log:
> > [Sema] Fix PR30346: relax __builtin_object_size checks.
> >
> > This patch makes us act more conservatively when trying to determine
> > the objectsize for an array at the end of an object. This is in
> > response to code like the following:
> >
> > ```
> > struct sockaddr {
> >   /* snip */
> >   char sa_data[14];
> > };
> >
> > void foo(const char *s) {
> >   size_t slen = strlen(s) + 1;
> >   size_t added_len = slen <= 14 ? 0 : slen - 14;
> >   struct sockaddr *sa = malloc(sizeof(struct sockaddr) + added_len);
> >   strcpy(sa->sa_data, s);
> >   // ...
> > }
> > ```
> >
> > `__builtin_object_size(sa->sa_data, 1)` would return 14, when there
> > could be more than 14 bytes at `sa->sa_data`.
> >
> > Code like this is apparently not uncommon. FreeBSD's manual even
> > explicitly mentions this pattern:
> > https://www.freebsd.org/doc/en/books/developers-handbook/
> sockets-essential-functions.html
> > (section 7.5.1.1.2).
> >
> > In light of this, we now just give up on any array at the end of an
> > object if we can't find the object's initial allocation.
> >
> > I lack numbers for how much more conservative we actually become as a
> > result of this change, so I chose the fix that would make us as
> > compatible with GCC as possible. If we want to be more aggressive, I'm
> > happy to consider some kind of whitelist or something instead.
>
> IMO this should be restricted to code that explicitly disables C/C++
> aliasing rules.


Do you mean -fno-strict-aliasing or -fno-struct-path-tbaa or something else
here? (I think we're not doing anyone any favours by making _FORTIFY_SOURCE
say that a pattern is OK in cases when LLVM will in fact optimize on the
assumption that it's UB, but I don't recall how aggressive
-fstruct-path-tbaa is for trailing array members.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160913/afb8738c/attachment.html>