filcab added a comment.
Thanks for the review.
Unfortunately, I forgot to edit the commit message. Let's hope no one gets too
confused (phab reviews will be up anyway, so should be easy to figure out).
Filipe
Repository:
rL LLVM
https://reviews.llvm.org/D52615
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346001: Change
-fsanitize-address-poison-class-member-array-new-cookie to -fsanitize…
(authored by filcab, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346001: Change
-fsanitize-address-poison-class-member-array-new-cookie to -fsanitize…
(authored by filcab, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52615?vs=172376=172392#toc
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks, LGTM.
Repository:
rC Clang
https://reviews.llvm.org/D52615
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
filcab updated this revision to Diff 172376.
filcab added a comment.
Reworded with feedback from the review.
Repository:
rC Clang
https://reviews.llvm.org/D52615
Files:
docs/ClangCommandLineReference.rst
docs/UsersManual.rst
include/clang/Driver/Options.td
rjmccall added inline comments.
Comment at: docs/ClangCommandLineReference.rst:805
-Enable poisoning array cookies when using class member operator new\[\] in
AddressSanitizer
+Enable poisoning array cookies when using custom operator new\[\] in
AddressSanitizer
filcab updated this revision to Diff 172179.
filcab added a comment.
Expand yet a bit more.
Repository:
rC Clang
https://reviews.llvm.org/D52615
Files:
docs/ClangCommandLineReference.rst
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Driver/SanitizerArgs.h
filcab updated this revision to Diff 172149.
filcab added a comment.
Expanded a bit more on the documentation, mentioning that user code might be
technically allowed to access those array cookies, but that users might not
want to allow it.
Repository:
rC Clang
rjmccall added a comment.
Thanks, and I agree with renaming the existing option.
Comment at: docs/ClangCommandLineReference.rst:805
-Enable poisoning array cookies when using class member operator new\[\] in
AddressSanitizer
+Enable poisoning array cookies when using custom
filcab added a comment.
In https://reviews.llvm.org/D52615#1278000, @rjmccall wrote:
> So, three points:
>
> - That's not class-member-specific; you can have a placement `operator new[]`
> at global scope that isn't the special `void*` placement operator and
> therefore still has a cookie, and
filcab updated this revision to Diff 171661.
filcab added a comment.
Missed the change in some places
Repository:
rC Clang
https://reviews.llvm.org/D52615
Files:
docs/ClangCommandLineReference.rst
docs/UsersManual.rst
include/clang/Driver/Options.td
filcab updated this revision to Diff 171660.
filcab added a comment.
Update with name change, using rjmccall's suggestion
Repository:
rC Clang
https://reviews.llvm.org/D52615
Files:
include/clang/Driver/Options.td
include/clang/Driver/SanitizerArgs.h
rjmccall added a comment.
So, three points:
- That's not class-member-specific; you can have a placement `operator new[]`
at global scope that isn't the special `void*` placement operator and therefore
still has a cookie, and it would have just as much flexibility as a
class-member override
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:
> > >
> > > >
rjmccall added a comment.
Oops, sorry for the unedited comment; it's fixed on Phab.
Repository:
rC Clang
https://reviews.llvm.org/D52615
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rjmccall added a comment.
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:
> > >
> > > >
filcab added a comment.
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
> >
rjmccall added a comment.
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`?
>
filcab added a comment.
Ping!
Thank you,
Filipe
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
filcab added a comment.
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?
Thank you,
Filipe
rsmith added a comment.
This seems like an unreasonably long flag name. Can you try to find a shorter
name for it?
Repository:
rC Clang
https://reviews.llvm.org/D52615
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
filcab created this revision.
filcab added reviewers: rjmccall, kcc, rsmith.
Repository:
rC Clang
https://reviews.llvm.org/D52615
Files:
include/clang/Driver/SanitizerArgs.h
lib/Driver/SanitizerArgs.cpp
test/Driver/fsanitize.c
Index: test/Driver/fsanitize.c
22 matches
Mail list logo
