diff mbox

PR78631 fix

Message ID CACysShj4UB8t7Fi3dySF+Fyhf5T7nf=n7+b8zEy1f9BicHhfSA@mail.gmail.com
State New
Headers show

Commit Message

Alexander Ivchenko Dec. 23, 2016, 11:02 a.m. UTC 
Hi Ilya,

Would that patch be OK to submit? (it is HJ's one with added testcase)

 char *

Comments

Ilya Enkovich Dec. 23, 2016, 6:48 p.m. UTC | #1 
Hi,

ChangeLog? Otherwise OK.

Ilya

2016-12-23 14:02 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
> Hi Ilya,
>
> Would that patch be OK to submit? (it is HJ's one with added testcase)
>
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
> b/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
> new file mode 100644
> index 0000000..1691348
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
> @@ -0,0 +1,23 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +/* Fix for PR78631 */
> +
> +#define SHOULDFAIL
> +
> +#include <stdio.h>
> +#include <string.h>
> +
> +char s[10];
> +char d[10];
> +__attribute__((noinline))
> +
> +char* foo(char* dst, char* src, size_t size) {
> +  return memcpy(dst, src, size);
> +}
> +int main() {
> +  char* r = foo(d, s, 11);
> +  printf("r = %p\n", r);
> +  return 0;
> +}
> diff --git a/libmpx/mpxwrap/libtool-version b/libmpx/mpxwrap/libtool-version
> index fab30fb..e241350 100644
> --- a/libmpx/mpxwrap/libtool-version
> +++ b/libmpx/mpxwrap/libtool-version
> @@ -3,4 +3,4 @@
>  # a separate file so that version updates don't involve re-running
>  # automake.
>  # CURRENT:REVISION:AGE
> -2:0:0
> +2:1:0
> diff --git a/libmpx/mpxwrap/mpx_wrappers.c b/libmpx/mpxwrap/mpx_wrappers.c
> index 171a780..aea0346 100644
> --- a/libmpx/mpxwrap/mpx_wrappers.c
> +++ b/libmpx/mpxwrap/mpx_wrappers.c
> @@ -30,14 +30,20 @@
>  #include <assert.h>
>  #include "mpxrt/mpxrt.h"
>
> -void *
> -__mpx_wrapper_malloc (size_t size)
> +/* Since internal MPX wrapper calls must avoid PLT which will clear bound
> +   registers, we make them static with an external alias.  */
> +#define EXTERN_ALIAS(f) \
> +  __typeof (f) __##f __attribute__((alias(#f)));
> +
> +static void *
> +mpx_wrapper_malloc (size_t size)
>  {
>    void *p = (void *)malloc (size);
>    if (!p) return __bnd_null_ptr_bounds (p);
>    return __bnd_set_ptr_bounds (p, size);
>  }
>
> +EXTERN_ALIAS (mpx_wrapper_malloc)
>
>  void *
>  __mpx_wrapper_mmap (void *addr, size_t length, int prot, int flags,
> @@ -52,7 +58,7 @@ void *
>  __mpx_wrapper_realloc (void *ptr, size_t n)
>  {
>    if (!ptr)
> -    return __mpx_wrapper_malloc (n);
> +    return mpx_wrapper_malloc (n);
>
>    /* We don't kwnow how much data is copied by realloc
>       and therefore may check only lower bounds.  */
> @@ -74,8 +80,8 @@ __mpx_wrapper_calloc (size_t n_elements, size_t element_size)
>    return __bnd_set_ptr_bounds (p, n_elements * element_size);
>  }
>
> -void *
> -__mpx_wrapper_memset (void *dstpp, int c, size_t len)
> +static void *
> +mpx_wrapper_memset (void *dstpp, int c, size_t len)
>  {
>    if (len > 0)
>      {
> @@ -85,10 +91,12 @@ __mpx_wrapper_memset (void *dstpp, int c, size_t len)
>    return dstpp;
>  }
>
> +EXTERN_ALIAS (mpx_wrapper_memset)
> +
>  void
>  __mpx_wrapper_bzero (void *dst, size_t len)
>  {
> -  __mpx_wrapper_memset (dst, 0, len);
> +  mpx_wrapper_memset (dst, 0, len);
>  }
>
>  /* The mpx_pointer type is used for getting bits
> @@ -484,8 +492,8 @@ move_bounds (void *dst, const void *src, size_t n)
>    return;
>  }
>
> -void *
> -__mpx_wrapper_memmove (void *dst, const void *src, size_t n)
> +static void *
> +mpx_wrapper_memmove (void *dst, const void *src, size_t n)
>  {
>    if (n == 0)
>      return dst;
> @@ -513,17 +521,20 @@ __mpx_wrapper_memmove (void *dst, const void
> *src, size_t n)
>    return dst;
>  }
>
> +EXTERN_ALIAS (mpx_wrapper_memmove)
>
> -void *
> -__mpx_wrapper_memcpy (void *dst, const void *src, size_t n)
> +static void *
> +mpx_wrapper_memcpy (void *dst, const void *src, size_t n)
>  {
> -  return __mpx_wrapper_memmove (dst, src, n);
> +  return mpx_wrapper_memmove (dst, src, n);
>  }
>
> +EXTERN_ALIAS (mpx_wrapper_memcpy)
> +
>  void *
>  __mpx_wrapper_mempcpy (void *dst, const void *src, size_t n)
>  {
> -  return (char *)__mpx_wrapper_memcpy (dst, src, n) + n;
> +  return (char *)mpx_wrapper_memcpy (dst, src, n) + n;
>  }
>
>  char *
Alexander Ivchenko Dec. 27, 2016, 3:36 p.m. UTC | #2 
Committed as r243942 with the ChangeLog entries


thanks,
Alexander

2016-12-23 21:48 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> Hi,
>
> ChangeLog? Otherwise OK.
>
> Ilya
>
> 2016-12-23 14:02 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>> Hi Ilya,
>>
>> Would that patch be OK to submit? (it is HJ's one with added testcase)
>>
>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
>> b/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
>> new file mode 100644
>> index 0000000..1691348
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
>> @@ -0,0 +1,23 @@
>> +/* { dg-do run } */
>> +/* { dg-shouldfail "bounds violation" } */
>> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
>> +
>> +/* Fix for PR78631 */
>> +
>> +#define SHOULDFAIL
>> +
>> +#include <stdio.h>
>> +#include <string.h>
>> +
>> +char s[10];
>> +char d[10];
>> +__attribute__((noinline))
>> +
>> +char* foo(char* dst, char* src, size_t size) {
>> +  return memcpy(dst, src, size);
>> +}
>> +int main() {
>> +  char* r = foo(d, s, 11);
>> +  printf("r = %p\n", r);
>> +  return 0;
>> +}
>> diff --git a/libmpx/mpxwrap/libtool-version b/libmpx/mpxwrap/libtool-version
>> index fab30fb..e241350 100644
>> --- a/libmpx/mpxwrap/libtool-version
>> +++ b/libmpx/mpxwrap/libtool-version
>> @@ -3,4 +3,4 @@
>>  # a separate file so that version updates don't involve re-running
>>  # automake.
>>  # CURRENT:REVISION:AGE
>> -2:0:0
>> +2:1:0
>> diff --git a/libmpx/mpxwrap/mpx_wrappers.c b/libmpx/mpxwrap/mpx_wrappers.c
>> index 171a780..aea0346 100644
>> --- a/libmpx/mpxwrap/mpx_wrappers.c
>> +++ b/libmpx/mpxwrap/mpx_wrappers.c
>> @@ -30,14 +30,20 @@
>>  #include <assert.h>
>>  #include "mpxrt/mpxrt.h"
>>
>> -void *
>> -__mpx_wrapper_malloc (size_t size)
>> +/* Since internal MPX wrapper calls must avoid PLT which will clear bound
>> +   registers, we make them static with an external alias.  */
>> +#define EXTERN_ALIAS(f) \
>> +  __typeof (f) __##f __attribute__((alias(#f)));
>> +
>> +static void *
>> +mpx_wrapper_malloc (size_t size)
>>  {
>>    void *p = (void *)malloc (size);
>>    if (!p) return __bnd_null_ptr_bounds (p);
>>    return __bnd_set_ptr_bounds (p, size);
>>  }
>>
>> +EXTERN_ALIAS (mpx_wrapper_malloc)
>>
>>  void *
>>  __mpx_wrapper_mmap (void *addr, size_t length, int prot, int flags,
>> @@ -52,7 +58,7 @@ void *
>>  __mpx_wrapper_realloc (void *ptr, size_t n)
>>  {
>>    if (!ptr)
>> -    return __mpx_wrapper_malloc (n);
>> +    return mpx_wrapper_malloc (n);
>>
>>    /* We don't kwnow how much data is copied by realloc
>>       and therefore may check only lower bounds.  */
>> @@ -74,8 +80,8 @@ __mpx_wrapper_calloc (size_t n_elements, size_t element_size)
>>    return __bnd_set_ptr_bounds (p, n_elements * element_size);
>>  }
>>
>> -void *
>> -__mpx_wrapper_memset (void *dstpp, int c, size_t len)
>> +static void *
>> +mpx_wrapper_memset (void *dstpp, int c, size_t len)
>>  {
>>    if (len > 0)
>>      {
>> @@ -85,10 +91,12 @@ __mpx_wrapper_memset (void *dstpp, int c, size_t len)
>>    return dstpp;
>>  }
>>
>> +EXTERN_ALIAS (mpx_wrapper_memset)
>> +
>>  void
>>  __mpx_wrapper_bzero (void *dst, size_t len)
>>  {
>> -  __mpx_wrapper_memset (dst, 0, len);
>> +  mpx_wrapper_memset (dst, 0, len);
>>  }
>>
>>  /* The mpx_pointer type is used for getting bits
>> @@ -484,8 +492,8 @@ move_bounds (void *dst, const void *src, size_t n)
>>    return;
>>  }
>>
>> -void *
>> -__mpx_wrapper_memmove (void *dst, const void *src, size_t n)
>> +static void *
>> +mpx_wrapper_memmove (void *dst, const void *src, size_t n)
>>  {
>>    if (n == 0)
>>      return dst;
>> @@ -513,17 +521,20 @@ __mpx_wrapper_memmove (void *dst, const void
>> *src, size_t n)
>>    return dst;
>>  }
>>
>> +EXTERN_ALIAS (mpx_wrapper_memmove)
>>
>> -void *
>> -__mpx_wrapper_memcpy (void *dst, const void *src, size_t n)
>> +static void *
>> +mpx_wrapper_memcpy (void *dst, const void *src, size_t n)
>>  {
>> -  return __mpx_wrapper_memmove (dst, src, n);
>> +  return mpx_wrapper_memmove (dst, src, n);
>>  }
>>
>> +EXTERN_ALIAS (mpx_wrapper_memcpy)
>> +
>>  void *
>>  __mpx_wrapper_mempcpy (void *dst, const void *src, size_t n)
>>  {
>> -  return (char *)__mpx_wrapper_memcpy (dst, src, n) + n;
>> +  return (char *)mpx_wrapper_memcpy (dst, src, n) + n;
>>  }
>>
>>  char *
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
b/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
new file mode 100644
index 0000000..1691348
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c
@@ -0,0 +1,23 @@ 
+/* { dg-do run } */
+/* { dg-shouldfail "bounds violation" } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
+
+/* Fix for PR78631 */
+
+#define SHOULDFAIL
+
+#include <stdio.h>
+#include <string.h>
+
+char s[10];
+char d[10];
+__attribute__((noinline))
+
+char* foo(char* dst, char* src, size_t size) {
+  return memcpy(dst, src, size);
+}
+int main() {
+  char* r = foo(d, s, 11);
+  printf("r = %p\n", r);
+  return 0;
+}
diff --git a/libmpx/mpxwrap/libtool-version b/libmpx/mpxwrap/libtool-version
index fab30fb..e241350 100644
--- a/libmpx/mpxwrap/libtool-version
+++ b/libmpx/mpxwrap/libtool-version
@@ -3,4 +3,4 @@ 
 # a separate file so that version updates don't involve re-running
 # automake.
 # CURRENT:REVISION:AGE
-2:0:0
+2:1:0
diff --git a/libmpx/mpxwrap/mpx_wrappers.c b/libmpx/mpxwrap/mpx_wrappers.c
index 171a780..aea0346 100644
--- a/libmpx/mpxwrap/mpx_wrappers.c
+++ b/libmpx/mpxwrap/mpx_wrappers.c
@@ -30,14 +30,20 @@ 
 #include <assert.h>
 #include "mpxrt/mpxrt.h"

-void *
-__mpx_wrapper_malloc (size_t size)
+/* Since internal MPX wrapper calls must avoid PLT which will clear bound
+   registers, we make them static with an external alias.  */
+#define EXTERN_ALIAS(f) \
+  __typeof (f) __##f __attribute__((alias(#f)));
+
+static void *
+mpx_wrapper_malloc (size_t size)
 {
   void *p = (void *)malloc (size);
   if (!p) return __bnd_null_ptr_bounds (p);
   return __bnd_set_ptr_bounds (p, size);
 }

+EXTERN_ALIAS (mpx_wrapper_malloc)

 void *
 __mpx_wrapper_mmap (void *addr, size_t length, int prot, int flags,
@@ -52,7 +58,7 @@  void *
 __mpx_wrapper_realloc (void *ptr, size_t n)
 {
   if (!ptr)
-    return __mpx_wrapper_malloc (n);
+    return mpx_wrapper_malloc (n);

   /* We don't kwnow how much data is copied by realloc
      and therefore may check only lower bounds.  */
@@ -74,8 +80,8 @@  __mpx_wrapper_calloc (size_t n_elements, size_t element_size)
   return __bnd_set_ptr_bounds (p, n_elements * element_size);
 }

-void *
-__mpx_wrapper_memset (void *dstpp, int c, size_t len)
+static void *
+mpx_wrapper_memset (void *dstpp, int c, size_t len)
 {
   if (len > 0)
     {
@@ -85,10 +91,12 @@  __mpx_wrapper_memset (void *dstpp, int c, size_t len)
   return dstpp;
 }

+EXTERN_ALIAS (mpx_wrapper_memset)
+
 void
 __mpx_wrapper_bzero (void *dst, size_t len)
 {
-  __mpx_wrapper_memset (dst, 0, len);
+  mpx_wrapper_memset (dst, 0, len);
 }

 /* The mpx_pointer type is used for getting bits
@@ -484,8 +492,8 @@  move_bounds (void *dst, const void *src, size_t n)
   return;
 }

-void *
-__mpx_wrapper_memmove (void *dst, const void *src, size_t n)
+static void *
+mpx_wrapper_memmove (void *dst, const void *src, size_t n)
 {
   if (n == 0)
     return dst;
@@ -513,17 +521,20 @@  __mpx_wrapper_memmove (void *dst, const void
*src, size_t n)
   return dst;
 }

+EXTERN_ALIAS (mpx_wrapper_memmove)

-void *
-__mpx_wrapper_memcpy (void *dst, const void *src, size_t n)
+static void *
+mpx_wrapper_memcpy (void *dst, const void *src, size_t n)
 {
-  return __mpx_wrapper_memmove (dst, src, n);
+  return mpx_wrapper_memmove (dst, src, n);
 }

+EXTERN_ALIAS (mpx_wrapper_memcpy)
+
 void *
 __mpx_wrapper_mempcpy (void *dst, const void *src, size_t n)
 {
-  return (char *)__mpx_wrapper_memcpy (dst, src, n) + n;
+  return (char *)mpx_wrapper_memcpy (dst, src, n) + n;
 }