should asan catch tihs?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

should asan catch tihs?

Rafael Espíndola
I just tried asan on an optimized  32 bit build of
-------------------------------------
#include <stdint.h>
__attribute__((noinline))
 void f(uint64_t *p) {
  *p = 42;
}
int main() {
  void *p;
  f((uint64_t*)&p);
}
------------------------------------

and it correctly catches the invalid access. If I comment the
attribute, the optimizers find and exploit the undefined behavior and
asan fails to report it. Is this the expected behavior? Is this
something that needs -fcatch-undefined-behavior instead?

Cheers,
Rafael
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: should asan catch tihs?

Eli Friedman-2
On Fri, Nov 2, 2012 at 6:27 PM, Rafael Espíndola
<[hidden email]> wrote:

> I just tried asan on an optimized  32 bit build of
> -------------------------------------
> #include <stdint.h>
> __attribute__((noinline))
>  void f(uint64_t *p) {
>   *p = 42;
> }
> int main() {
>   void *p;
>   f((uint64_t*)&p);
> }
> ------------------------------------
>
> and it correctly catches the invalid access. If I comment the
> attribute, the optimizers find and exploit the undefined behavior and
> asan fails to report it. Is this the expected behavior? Is this
> something that needs -fcatch-undefined-behavior instead?

For performance reasons, asan runs at the end of the optimization
pipeline, so it doesn't check loads which get removed by the IR
optimizers.

-Eli

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: should asan catch tihs?

Kostya Serebryany
Also note that this is not the kind of bug for which asan is good.
If we are dereferencing an uninitialized pointer, there is a high chance that the program will SEGV w/o any tool.
If we are unlucky and the garbage is accidentally equal to some valid address, asan will not catch it either. 
Valgrind (and work-in-progress MemorySanitizer) will catch this. 

--kcc 

On Sat, Nov 3, 2012 at 5:38 AM, Eli Friedman <[hidden email]> wrote:
On Fri, Nov 2, 2012 at 6:27 PM, Rafael Espíndola
<[hidden email]> wrote:
> I just tried asan on an optimized  32 bit build of
> -------------------------------------
> #include <stdint.h>
> __attribute__((noinline))
>  void f(uint64_t *p) {
>   *p = 42;
> }
> int main() {
>   void *p;
>   f((uint64_t*)&p);
> }
> ------------------------------------
>
> and it correctly catches the invalid access. If I comment the
> attribute, the optimizers find and exploit the undefined behavior and
> asan fails to report it. Is this the expected behavior? Is this
> something that needs -fcatch-undefined-behavior instead?

For performance reasons, asan runs at the end of the optimization
pipeline, so it doesn't check loads which get removed by the IR
optimizers.

-Eli

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: should asan catch tihs?

John Criswell-4
On 11/3/12 4:06 AM, Kostya Serebryany wrote:
Also note that this is not the kind of bug for which asan is good.
If we are dereferencing an uninitialized pointer, there is a high chance that the program will SEGV w/o any tool.
If we are unlucky and the garbage is accidentally equal to some valid address, asan will not catch it either. 
Valgrind (and work-in-progress MemorySanitizer) will catch this.

Kostya, I think you're misreading the test case.  He's storing a 64-bit value into a 32-bit pointer on a 32-bit platform.  The pointer value in f() is initialized (at least in the case where the compiler's not optimizing away things due to undefined behavior).  The problem is that a 64-bit store doesn't fit into a 32-bit pointer.

ASan is catching that when the load isn't optimized away, and I've verified that mainline SAFECode does as well.  It also appears that SAFECode finds the error when the noinline attribute is removed, but I think that's a bug in how we integrated SAFECode into Clang (the store should be optimized away by the LLVM optimizations, but it isn't).

Rafael, for now, if you want to get more pedantic behavior out of ASan and SAFECode, you might want to try reducing the optimization level.  However, your test case brings up a good point: should there be an option to clang that makes tools like ASan and SAFECode more or less pedantic (either by reducing optimization or making checks more picky)?  It might make a good question at our BoF session next week. :)

-- John T.



--kcc 

On Sat, Nov 3, 2012 at 5:38 AM, Eli Friedman <[hidden email]> wrote:
On Fri, Nov 2, 2012 at 6:27 PM, Rafael Espíndola
<[hidden email]> wrote:
> I just tried asan on an optimized  32 bit build of
> -------------------------------------
> #include <stdint.h>
> __attribute__((noinline))
>  void f(uint64_t *p) {
>   *p = 42;
> }
> int main() {
>   void *p;
>   f((uint64_t*)&p);
> }
> ------------------------------------
>
> and it correctly catches the invalid access. If I comment the
> attribute, the optimizers find and exploit the undefined behavior and
> asan fails to report it. Is this the expected behavior? Is this
> something that needs -fcatch-undefined-behavior instead?

For performance reasons, asan runs at the end of the optimization
pipeline, so it doesn't check loads which get removed by the IR
optimizers.

-Eli

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: should asan catch tihs?

Kostya Serebryany
Ah, my bad, I missed '&' in main. 
There are quite a few similar cases when asan will miss a bug in fully optimized code, while 
it will not miss it at lower opt levels. 

--kcc 

On Sat, Nov 3, 2012 at 5:22 PM, John Criswell <[hidden email]> wrote:
On 11/3/12 4:06 AM, Kostya Serebryany wrote:
Also note that this is not the kind of bug for which asan is good.
If we are dereferencing an uninitialized pointer, there is a high chance that the program will SEGV w/o any tool.
If we are unlucky and the garbage is accidentally equal to some valid address, asan will not catch it either. 
Valgrind (and work-in-progress MemorySanitizer) will catch this.

Kostya, I think you're misreading the test case.  He's storing a 64-bit value into a 32-bit pointer on a 32-bit platform.  The pointer value in f() is initialized (at least in the case where the compiler's not optimizing away things due to undefined behavior).  The problem is that a 64-bit store doesn't fit into a 32-bit pointer.

ASan is catching that when the load isn't optimized away, and I've verified that mainline SAFECode does as well.  It also appears that SAFECode finds the error when the noinline attribute is removed, but I think that's a bug in how we integrated SAFECode into Clang (the store should be optimized away by the LLVM optimizations, but it isn't).

Rafael, for now, if you want to get more pedantic behavior out of ASan and SAFECode, you might want to try reducing the optimization level.  However, your test case brings up a good point: should there be an option to clang that makes tools like ASan and SAFECode more or less pedantic (either by reducing optimization or making checks more picky)?  It might make a good question at our BoF session next week. :)

-- John T.




--kcc 

On Sat, Nov 3, 2012 at 5:38 AM, Eli Friedman <[hidden email]> wrote:
On Fri, Nov 2, 2012 at 6:27 PM, Rafael Espíndola
<[hidden email]> wrote:
> I just tried asan on an optimized  32 bit build of
> -------------------------------------
> #include <stdint.h>
> __attribute__((noinline))
>  void f(uint64_t *p) {
>   *p = 42;
> }
> int main() {
>   void *p;
>   f((uint64_t*)&p);
> }
> ------------------------------------
>
> and it correctly catches the invalid access. If I comment the
> attribute, the optimizers find and exploit the undefined behavior and
> asan fails to report it. Is this the expected behavior? Is this
> something that needs -fcatch-undefined-behavior instead?

For performance reasons, asan runs at the end of the optimization
pipeline, so it doesn't check loads which get removed by the IR
optimizers.

-Eli

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: should asan catch tihs?

Richard Smith-33
In reply to this post by Rafael Espíndola
On Fri, Nov 2, 2012 at 6:27 PM, Rafael Espíndola
<[hidden email]> wrote:

> I just tried asan on an optimized  32 bit build of
> -------------------------------------
> #include <stdint.h>
> __attribute__((noinline))
>  void f(uint64_t *p) {
>   *p = 42;
> }
> int main() {
>   void *p;
>   f((uint64_t*)&p);
> }
> ------------------------------------
>
> and it correctly catches the invalid access. If I comment the
> attribute, the optimizers find and exploit the undefined behavior and
> asan fails to report it. Is this the expected behavior? Is this
> something that needs -fcatch-undefined-behavior instead?

-fcatch-undefined-behavior (more specifically, -fsanitize=object-size)
already catches this if the function gets inlined:

$ clang -x c++ <(grep -v attribute testcase.cpp) -fsanitize=object-size -m32 -O3
$ ./a.out
<stdin>:4:3: fatal error: store to address 0xff97f8c8 with
insufficient space for an object of type 'uint64_t' (aka 'unsigned
long long')

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev