diff mbox

Fix for PR/62089 (enable missing Asan checks)

Message ID 53EB395D.2060604@samsung.com
State New
Headers show

Commit Message

Yury Gribov Aug. 13, 2014, 10:09 a.m. UTC 
Hi all,

GCC Asan may currently fail to generate checks for accesses to wide 
structure fields due to unfortunate intermix with code that handles 
bitfields. This patch fixes it and also adds a bunch of tests.

Bootstrapped, regtested and asan-bootstrapped on x64.

Ok to commit?

-Y

Comments

Jakub Jelinek Aug. 13, 2014, 10:13 a.m. UTC | #1 
On Wed, Aug 13, 2014 at 02:09:33PM +0400, Yury Gribov wrote:
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1690,9 +1690,7 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
>    int volatilep = 0, unsignedp = 0;
>    tree inner = get_inner_reference (t, &bitsize, &bitpos, &offset,
>  				    &mode, &unsignedp, &volatilep, false);
> -  if (((size_in_bytes & (size_in_bytes - 1)) == 0
> -       && (bitpos % (size_in_bytes * BITS_PER_UNIT)))
> -      || bitsize != size_in_bytes * BITS_PER_UNIT)
> +  if (bitpos % BITS_PER_UNIT || bitsize != size_in_bytes * BITS_PER_UNIT)
>      {
>        if (TREE_CODE (t) == COMPONENT_REF
>  	  && DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (t, 1)) != NULL_TREE)

I wonder if we just shouldn't handle COMPONENT_REFs with
DECL_BIT_FIELD_REPRESENTATIVE that way unconditionally.
So, do the if (TREE_CODE (t) == COMPONENT_REF ... before the other checks,
and
  else if (bitpos % BITS_PER_UNIT || bitsize != size_in_bytes * BITS_PER_UNIT)
    return;

	Jakub
diff mbox

Patch

commit 5a10147cef710e6f43365567615f37261d7e70c5
Author: Yury Gribov <y.gribov@samsung.com>
Date:   Mon Aug 11 15:09:45 2014 +0400

    2014-08-12  Yury Gribov  <y.gribov@samsung.com>
    
    gcc/
    	PR sanitizer/62089
    	* asan.c (instrument_derefs): Fix bitfield check.
    
    gcc/testsuite/
    	PR sanitizer/62089
    	* c-c++-common/asan/pr62089.c: New test.
    	* c-c++-common/asan/bitfield-1.c: New test.
    	* c-c++-common/asan/bitfield-2.c: New test.
    	* c-c++-common/asan/bitfield-3.c: New test.
    	* c-c++-common/asan/bitfield-4.c: New test.

diff --git a/gcc/asan.c b/gcc/asan.c
index 4e6f438..b38264b 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1690,9 +1690,7 @@  instrument_derefs (gimple_stmt_iterator *iter, tree t,
   int volatilep = 0, unsignedp = 0;
   tree inner = get_inner_reference (t, &bitsize, &bitpos, &offset,
 				    &mode, &unsignedp, &volatilep, false);
-  if (((size_in_bytes & (size_in_bytes - 1)) == 0
-       && (bitpos % (size_in_bytes * BITS_PER_UNIT)))
-      || bitsize != size_in_bytes * BITS_PER_UNIT)
+  if (bitpos % BITS_PER_UNIT || bitsize != size_in_bytes * BITS_PER_UNIT)
     {
       if (TREE_CODE (t) == COMPONENT_REF
 	  && DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (t, 1)) != NULL_TREE)
@@ -1704,8 +1702,6 @@  instrument_derefs (gimple_stmt_iterator *iter, tree t,
 	}
       return;
     }
-  if (bitpos % BITS_PER_UNIT)
-    return;
 
   if (TREE_CODE (inner) == VAR_DECL
       && offset == NULL_TREE
diff --git a/gcc/testsuite/c-c++-common/asan/bitfield-1.c b/gcc/testsuite/c-c++-common/asan/bitfield-1.c
new file mode 100644
index 0000000..b3f300c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/bitfield-1.c
@@ -0,0 +1,25 @@ 
+/* Check that Asan correctly instruments bitfields with non-round size.  */
+
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+struct A
+{
+  char base;
+  int : 4;
+  long x : 7;
+};
+
+int __attribute__ ((noinline, noclone))
+f (void *p) {
+  return ((struct A *)p)->x;
+}
+
+int
+main ()
+{
+  char a = 0;
+  return f (&a);
+}
+
+/* { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow" } */
diff --git a/gcc/testsuite/c-c++-common/asan/bitfield-2.c b/gcc/testsuite/c-c++-common/asan/bitfield-2.c
new file mode 100644
index 0000000..8ab0f80
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/bitfield-2.c
@@ -0,0 +1,25 @@ 
+/* Check that Asan correctly instruments bitfields with non-round offset.  */
+
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+struct A
+{
+  char base;
+  int : 7;
+  int x : 8;
+};
+
+int __attribute__ ((noinline, noclone))
+f (void *p) {
+  return ((struct A *)p)->x;
+}
+
+int
+main ()
+{
+  char a = 0;
+  return f (&a);
+}
+
+/* { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow" } */
diff --git a/gcc/testsuite/c-c++-common/asan/bitfield-3.c b/gcc/testsuite/c-c++-common/asan/bitfield-3.c
new file mode 100644
index 0000000..c590778
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/bitfield-3.c
@@ -0,0 +1,25 @@ 
+/* Check that Asan correctly instruments bitfields with round offset.  */
+
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+struct A
+{
+  char base;
+  int : 8;
+  int x : 8;
+};
+
+int __attribute__ ((noinline, noclone))
+f (void *p) {
+  return ((struct A *)p)->x;
+}
+
+int
+main ()
+{
+  char a = 0;
+  return f (&a);
+}
+
+/* { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow" } */
diff --git a/gcc/testsuite/c-c++-common/asan/bitfield-4.c b/gcc/testsuite/c-c++-common/asan/bitfield-4.c
new file mode 100644
index 0000000..94de9a4
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/bitfield-4.c
@@ -0,0 +1,25 @@ 
+/* Check that Asan correctly instruments bitfields with round offset.  */
+
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+struct A
+{
+  char base;
+  int : 0;
+  int x : 8;
+};
+
+int __attribute__ ((noinline, noclone))
+f (void *p) {
+  return ((struct A *)p)->x;
+}
+
+int
+main ()
+{
+  char a = 0;
+  return f (&a);
+}
+
+/* { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow" } */
diff --git a/gcc/testsuite/c-c++-common/asan/pr62089.c b/gcc/testsuite/c-c++-common/asan/pr62089.c
new file mode 100644
index 0000000..22b877b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pr62089.c
@@ -0,0 +1,37 @@ 
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#include <sanitizer/asan_interface.h>
+
+struct vfsmount {};
+struct dentry {};
+
+struct path {
+  struct vfsmount *mnt;
+  struct dentry *dentry;
+};
+
+struct fs_struct {
+  int users;
+  int lock;
+  int seq;
+  int umask;
+  int in_exec;
+  struct path root, pwd;
+};
+
+void __attribute__((noinline, noclone))
+copy_fs_struct(struct fs_struct *a, struct fs_struct *b) {
+  a->root = b->root;
+}
+
+struct fs_struct a, b;
+
+int
+main () {
+  __asan_poison_memory_region (&a.root, sizeof (a.root));
+  copy_fs_struct (&a, &b);
+  return 0;
+}
+
+/* { dg-output "ERROR: AddressSanitizer: use-after-poison" } */