filcab added a comment.

In https://reviews.llvm.org/D52615#1276938, @rjmccall wrote:

> In https://reviews.llvm.org/D52615#1275946, @filcab wrote:
>
> > In https://reviews.llvm.org/D52615#1272725, @rjmccall wrote:
> >
> > > In https://reviews.llvm.org/D52615#1266320, @filcab wrote:
> > >
> > > > In https://reviews.llvm.org/D52615#1254567, @rsmith wrote:
> > > >
> > > > > This seems like an unreasonably long flag name. Can you try to find a 
> > > > > shorter name for it?
> > > >
> > > >
> > > > `-fsanitize-poison-extra-operator-new`?
> > > >  Not as explicit, but maybe ok if documented?
> > >
> > >
> > > `-fsanitize-address-poison-array-cookie`?
> >
> >
> > I strongly dislike this one because "poison array cookie", in general, is 
> > always enabled (it's actually triggered by a runtime flag). This flag is 
> > about poisoning it in more cases (cases where the standard doesn't 
> > completely disallow accesses to the cookie, so we have to have a flag and 
> > can't enable it all the time).
>
>
> Hmm.  This naming discussion might be a proxy for another debate.  I 
> understand the argument that programs are allowed to assume a particular C++ 
> ABI and therefore access the array cookie.  And I can understand having an 
> option that makes ASan stricter and disallow accessing the array cookie 
> anyway.  But I don't understand why this analysis is in any way 
> case-specific, and the discussion you've linked doesn't seem particularly 
> clarifying about that point.  Can you explain this a little?


If you don't have a class-member operator new[], then you don't expect to be 
able to have access to the memory occupied by the array cookie, since you never 
"see" a pointer that points to it.
But if you have a class-member operator new[], then you can keep a copy of all 
the pointers you've returned, and you're allowed to read from them, even if 
they contain an array cookie (you're just reading from memory you've returned 
to users).
With this flag, we disallow this second case, at the expense of having ASan 
trigger an error on code that is standards compliant. We're making it more 
strict, therefore we start emitting ASan errors on the test I was removing in 
https://reviews.llvm.org/D41664. Kostya's comment on me removing that test was 
that the code is valid, so ASan shouldn't reject it by default (but ok to have 
flags that make it more strict).

Thank you,
Filipe

P.S: We still have "placement new" (`::operator new[](size_t, void*)`as a 
special case, which will not have an array cookie, so we don't poison it. This 
flag only applies to user-defined, class-specific allocation functions.

P.P.S: Sorry if I misunderstood your question. Please let me know.


Repository:
  rC Clang

https://reviews.llvm.org/D52615



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to