gopalshankar / address-sanitizer

Automatically exported from code.google.com/p/address-sanitizer
0 stars 0 forks source link

kasan: instrumentation of atomic operations #289

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Atomic operations are implemented in assembly and so are not instrumented by 
kasan.
Please incorporate the attached patch with atomics instrumentation.

Original issue reported on code.google.com by dvyu...@google.com on 4 Apr 2014 at 7:49

Attachments:

GoogleCodeExporter commented 9 years ago
What about adding support for inline asm into ASan instrumentation code? 
Detection and handling of "m", "=m" and "+m" should be fairly straightforward...

Original comment by tetra2...@gmail.com on 4 Apr 2014 at 7:59

GoogleCodeExporter commented 9 years ago
> What about adding support for inline asm into ASan instrumentation code? 
Detection and handling of "m", "=m" and "+m" should be fairly straightforward...

1. Assembly instrumentation is non-trivial amount of work.
2. We don't want to do it in gcc (clang if anything).
3. I suspect that we actually don't want to instrument most of kernel assembly 
code (e.g. task switching, interrupt thunks, syscall thunks, user memory access 
routines, etc).

Original comment by dvyu...@google.com on 4 Apr 2014 at 8:11

GoogleCodeExporter commented 9 years ago
> 2. We don't want to do it in gcc (clang if anything).

Why? I thought they are compatible at least in this regard.

> 3. I suspect that we actually don't want to instrument most of kernel 
assembly code

And I'd say this feature is useful for non-kernel code as well. This can be a 
separate issue though.

> (e.g. task switching, interrupt thunks, syscall thunks, user memory access 
routines, etc).

I see. Can we attach attributes to inline asm? no_sanitize_address may come to 
rescue.

Original comment by tetra2...@gmail.com on 4 Apr 2014 at 8:52

GoogleCodeExporter commented 9 years ago
> Why? I thought they are compatible at least in this regard.

By "We don't want" I meant literally *we*. If somebody wants to contribute such 
a patch to gcc, I do not mind.

> And I'd say this feature is useful for non-kernel code as well. This can be a 
separate issue though.

Yes. And yes.

> I see. Can we attach attributes to inline asm? no_sanitize_address may come 
to rescue.

I don't know.

Original comment by dvyu...@google.com on 4 Apr 2014 at 9:57

GoogleCodeExporter commented 9 years ago
>> By "We don't want" I meant literally *we*. If somebody wants to contribute 
such a patch to gcc, I do not mind.

"We" prefer to hack clang. Once the feature is stable in clang,
we'll want to have it in GCC as well, but not before.

Original comment by konstant...@gmail.com on 4 Apr 2014 at 10:13

GoogleCodeExporter commented 9 years ago
Got it. I'll file an issue for this (not sure when I'll get to it though, I'm 
completely planned up until July).

Original comment by tetra2...@gmail.com on 4 Apr 2014 at 10:56

GoogleCodeExporter commented 9 years ago
Do you think it's even possible in gcc?  It doesn't know how to parse inline 
assembly at all.  I think there are people in the community who are strongly 
against it, given the whole kerfuffle over how the Linux kernel does 'gcc -S | 
sed' with invalid inline asm to extract offsets of C structs.

Original comment by rnk@google.com on 4 Apr 2014 at 5:59

GoogleCodeExporter commented 9 years ago
We don't need to parse inline asm itself - just go through the list of 
constraints, filter out m's and instrument them.

Original comment by tetra2...@gmail.com on 4 Apr 2014 at 6:29

GoogleCodeExporter commented 9 years ago
https://github.com/google/kasan/commit/f24bb70fc5305ed50a46b4e192c0749de32e2e8d

Original comment by pre...@google.com on 1 Aug 2014 at 11:13