Re: PR78631 fix
Hi,
ChangeLog? Otherwise OK.
Ilya
2016-12-23 14:02 GMT+03:00 Alexander Ivchenko :
> 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 000..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
> +#include
> +
> +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
> #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 *
Re: PR78631 fix
Committed as r243942 with the ChangeLog entries
thanks,
Alexander
2016-12-23 21:48 GMT+03:00 Ilya Enkovich :
> Hi,
>
> ChangeLog? Otherwise OK.
>
> Ilya
>
> 2016-12-23 14:02 GMT+03:00 Alexander Ivchenko :
>> 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 000..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
>> +#include
>> +
>> +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
>> #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 *
Re: PR78631 fix
On Tue, Dec 27, 2016 at 06:36:11PM +0300, Alexander Ivchenko wrote:
> Committed as r243942 with the ChangeLog entries
Unfortunately it fails if assembler has mpx support, but hw doesn't support
it.
The following patch should fix that. Tested on x86_64-linux, ok for trunk?
2016-12-29 Jakub Jelinek
* gcc.target/i386/mpx/memcpy-1.c: Include mpx-check.h.
(main): Renamed to ...
(mpx_test): ... this. Add argc and argv arguments.
--- gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c.jj 2016-12-28
13:14:24.0 +0100
+++ gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c2016-12-29
16:07:11.135200098 +0100
@@ -8,6 +8,7 @@
#include
#include
+#include "mpx-check.h"
char s[10];
char d[10];
@@ -16,7 +17,7 @@ __attribute__((noinline))
char* foo(char* dst, char* src, size_t size) {
return memcpy(dst, src, size);
}
-int main() {
+int mpx_test(int argc, const char **argv) {
char* r = foo(d, s, 11);
printf("r = %p\n", r);
return 0;
Jakub
Re: PR78631 fix
On Thu, Dec 29, 2016 at 4:26 PM, Jakub Jelinek wrote:
> On Tue, Dec 27, 2016 at 06:36:11PM +0300, Alexander Ivchenko wrote:
>> Committed as r243942 with the ChangeLog entries
>
> Unfortunately it fails if assembler has mpx support, but hw doesn't support
> it.
>
> The following patch should fix that. Tested on x86_64-linux, ok for trunk?
>
> 2016-12-29 Jakub Jelinek
>
> * gcc.target/i386/mpx/memcpy-1.c: Include mpx-check.h.
> (main): Renamed to ...
> (mpx_test): ... this. Add argc and argv arguments.
OK.
Thanks,
Uros.
> --- gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c.jj 2016-12-28
> 13:14:24.0 +0100
> +++ gcc/testsuite/gcc.target/i386/mpx/memcpy-1.c2016-12-29
> 16:07:11.135200098 +0100
> @@ -8,6 +8,7 @@
>
> #include
> #include
> +#include "mpx-check.h"
>
> char s[10];
> char d[10];
> @@ -16,7 +17,7 @@ __attribute__((noinline))
> char* foo(char* dst, char* src, size_t size) {
>return memcpy(dst, src, size);
> }
> -int main() {
> +int mpx_test(int argc, const char **argv) {
>char* r = foo(d, s, 11);
>printf("r = %p\n", r);
>return 0;
>
>
> Jakub
