Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On 5/1/2017 6:17 PM, Hal Finkel wrote: However, the example can also be written as: struct X { int a, b; }; X x { 50, 100 }; X *o = (X*) int a_is_b = o->a; // This is UB (or so we say)? and then the pointer arithmetic considerations don't seem to apply. I know

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On Mon, May 1, 2017 at 7:06 PM, Daniel Berlin wrote: > On Mon, May 1, 2017 at 3:58 PM, John McCall wrote: > >> On Mon, May 1, 2017 at 6:40 PM, Daniel Berlin >> wrote: >> >>> On Mon, May 1, 2017 at 3:09 PM, Daniel Berlin

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On 05/01/2017 02:35 PM, Krzysztof Parzyszek via cfe-commits wrote: On 5/1/2017 2:16 PM, Hal Finkel via cfe-commits wrote: On 05/01/2017 12:49 PM, Daniel Berlin wrote: On 04/21/2017 06:03 AM, Hal Finkel via Phabricator wrote: ... Our struct-path TBAA does the following: struct X

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On Mon, May 1, 2017 at 3:58 PM, John McCall wrote: > On Mon, May 1, 2017 at 6:40 PM, Daniel Berlin wrote: > >> On Mon, May 1, 2017 at 3:09 PM, Daniel Berlin >> wrote: >> >>> On Mon, May 1, 2017 at 2:07 PM, John McCall

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On Mon, May 1, 2017 at 6:40 PM, Daniel Berlin wrote: > On Mon, May 1, 2017 at 3:09 PM, Daniel Berlin wrote: > >> On Mon, May 1, 2017 at 2:07 PM, John McCall wrote: >> >>> On Mon, May 1, 2017 at 3:31 PM, Daniel Berlin

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On Mon, May 1, 2017 at 3:09 PM, Daniel Berlin wrote: > > > On Mon, May 1, 2017 at 2:07 PM, John McCall wrote: > >> On Mon, May 1, 2017 at 3:31 PM, Daniel Berlin >> wrote: >> >>> So you believe that you can index into an object

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On Mon, May 1, 2017 at 2:07 PM, John McCall wrote: > On Mon, May 1, 2017 at 3:31 PM, Daniel Berlin wrote: > >> So you believe that you can index into an object randomly by pointer arithmetic and pull out a different field? >>> >>> For starters,

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On Mon, May 1, 2017 at 3:31 PM, Daniel Berlin wrote: > So you believe that you can index into an object randomly by pointer >>> arithmetic and pull out a different field? >>> >> >> For starters, this is illegal because you don't know where the padding >> bytes are. >> You

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On 05/01/2017 02:31 PM, Daniel Berlin wrote: So you believe that you can index into an object randomly by pointer arithmetic and pull out a different field? For starters, this is illegal because you don't know where the padding bytes are. You cannot assume that X.a + 1

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On 5/1/2017 2:16 PM, Hal Finkel via cfe-commits wrote: On 05/01/2017 12:49 PM, Daniel Berlin wrote: On 04/21/2017 06:03 AM, Hal Finkel via Phabricator wrote: ... Our struct-path TBAA does the following: struct X { int a, b; }; X x { 50, 100 }; X *o = (X*) (((int*) ) +

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

> > >> >> > So you believe that you can index into an object randomly by pointer > arithmetic and pull out a different field? > > For starters, this is illegal because you don't know where the padding > bytes are. > You cannot assume that X.a + 1 == X.b > "Implementation alignment requirements

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On Mon, May 1, 2017 at 12:16 PM, Hal Finkel wrote: > > On 05/01/2017 12:49 PM, Daniel Berlin wrote: > > > > On Fri, Apr 21, 2017 at 4:03 AM, Hal Finkel via Phabricator < > revi...@reviews.llvm.org> wrote: > >> hfinkel added a comment. >> >> In

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On 05/01/2017 12:49 PM, Daniel Berlin wrote: On Fri, Apr 21, 2017 at 4:03 AM, Hal Finkel via Phabricator > wrote: hfinkel added a comment. In https://reviews.llvm.org/D32199#732737

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

On Fri, Apr 21, 2017 at 4:03 AM, Hal Finkel via Phabricator < revi...@reviews.llvm.org> wrote: > hfinkel added a comment. > > In https://reviews.llvm.org/D32199#732737, @rsmith wrote: > > > In https://reviews.llvm.org/D32199#732189, @hfinkel wrote: > > > > > In

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

Richard, et al., Any feedback on my comments below on what C/C++ allows? I'd like to just be missing something ;) Thanks again, Hal On 04/21/2017 06:03 AM, Hal Finkel via Phabricator wrote: ... Our struct-path TBAA does the following: struct X { int a, b; }; X x { 50, 100 }; X

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. In https://reviews.llvm.org/D32199#732737, @rsmith wrote: > In https://reviews.llvm.org/D32199#732189, @hfinkel wrote: > > > In https://reviews.llvm.org/D32199#731472, @rsmith wrote: > > > > > 1. C's "effective type" rule allows writes to set the type pretty much > > >

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

rsmith added a comment. In https://reviews.llvm.org/D32199#732189, @hfinkel wrote: > In https://reviews.llvm.org/D32199#731472, @rsmith wrote: > > > 1. C's "effective type" rule allows writes to set the type pretty much > > unconditionally, unless the storage is for a variable with a declared

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. In https://reviews.llvm.org/D32199#732382, @rjmccall wrote: > If you're going to try to enforce the declared type of memory, you'll also > need something like the C effective type rule to handle char buffers in C++. > As far as I can tell, it's not actually legal

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

rjmccall added a comment. If you're going to try to enforce the declared type of memory, you'll also need something like the C effective type rule to handle char buffers in C++. As far as I can tell, it's not actually legal under the spec to cast an array of chars to an arbitrary type and

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. In https://reviews.llvm.org/D32199#731472, @rsmith wrote: > > As I've currently implemented it, both reads and writes set the type of > > previously-unknown storage, and after that it says fixed (unless you memcpy > > to it, memset it, or its lifetime ends (the type

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. In https://reviews.llvm.org/D32199#731472, @rsmith wrote: > > As I've currently implemented it, both reads and writes set the type of > > previously-unknown storage, and after that it says fixed (unless you memcpy > > to it, memset it, or its lifetime ends (the type

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

rsmith added a comment. > As I've currently implemented it, both reads and writes set the type of > previously-unknown storage, and after that it says fixed (unless you memcpy > to it, memset it, or its lifetime ends (the type gets reset on > lifetime.start/end and for malloc/allocas/etc.).

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. In https://reviews.llvm.org/D32199#731332, @rsmith wrote: > > ! In https://reviews.llvm.org/D32199#731252, @hfinkel wrote: > > > >> How about renaming this to something more like `-fsanitize=type`? > > > > I'm fine with that. Do you like TypeSanitizer or

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

rsmith added a comment. > ! In https://reviews.llvm.org/D32199#731252, @hfinkel wrote: > >> How about renaming this to something more like `-fsanitize=type`? > > I'm fine with that. Do you like TypeSanitizer or TypeAccessSantizer or > TypeAliasingSanitizer best? I think calling it a type

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. In https://reviews.llvm.org/D32199#731249, @hfinkel wrote: > In https://reviews.llvm.org/D32199#731144, @rsmith wrote: > > > > > > ... > > > I would also expect that we will extend this in future to assign types to > > storage even in cases where there is no store (for

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. In https://reviews.llvm.org/D32199#731144, @rsmith wrote: > ... > I would also expect that we will extend this in future to assign types to > storage even in cases where there is no store (for instance, we should be > able to catch `float f() { int n; return

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

rsmith added a comment. I don't like calling this a "TBAA sanitizer". What we're sanitizing is the object model and effective type rules; it seems irrelevant which specific compiler analysis passes would result in your program misbehaving if you break the rules. I would also expect that we

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel updated this revision to Diff 95829. hfinkel added a comment. Updated per review comments (use only no_sanitize("tbaa") instead of adding no_sanitize_tbaa and don't touch freebsd for now). https://reviews.llvm.org/D32199 Files: include/clang/Basic/Sanitizers.def

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. In https://reviews.llvm.org/D32199#730716, @filcab wrote: > Missing a `sanitizer-ld.c` test for freebsd. Thanks for pointing this out. I'm going to remove the freebsd change for now. I suspect it works, but I've not tested it yet. Comment at:

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

filcab added a comment. Missing a `sanitizer-ld.c` test for freebsd. Comment at: include/clang/Basic/Attr.td:1849 + GNU<"no_sanitize_memory">, + GNU<"no_sanitize_tbaa">]; let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag,

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel added a comment. As a note: As follow-up work, we might want to extend Clang to add TBAA metadata to allocas and lifetime.start intrinsics so that the instrumentation pass can mark the types of memory upon declaration/construction instead of only upon first access.

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

hfinkel created this revision. Herald added subscribers: mcrosier, emaste. This patch introduces the runtime components of a TBAA sanitizer: a sanitizer for type-based aliasing violations. C/C++ have type-based aliasing rules, and LLVM's optimizer can exploit these given TBAA metadata added