* [PATCH] MPX: fix PR middle-end/79753
@ 2017-03-10 13:15 Martin Liška
2017-03-14 22:27 ` Ilya Enkovich
0 siblings, 1 reply; 6+ messages in thread
From: Martin Liška @ 2017-03-10 13:15 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 269 bytes --]
Hello.
Currently, __builtin_ia32_bndret is used for all functions that have non-void
return type. I think the right fix is to return bounds just for a bounded type.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Martin
[-- Attachment #2: 0001-MPX-fix-PR-middle-end-79753.patch --]
[-- Type: text/x-patch, Size: 2850 bytes --]
From f4ad5003a42ca95187305a9b201656c7da2aaa01 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 9 Mar 2017 15:42:49 +0100
Subject: [PATCH] MPX: fix PR middle-end/79753
gcc/ChangeLog:
2017-03-09 Martin Liska <mliska@suse.cz>
PR middle-end/79753
* tree-chkp.c (chkp_build_returned_bound): Do not build
returned bounds for a LHS that's not a BOUNDED_P type.
gcc/testsuite/ChangeLog:
2017-03-10 Martin Liska <mliska@suse.cz>
* gcc.target/i386/mpx/pr79753.c: New test.
---
gcc/testsuite/gcc.target/i386/mpx/pr79753.c | 14 ++++++++++++++
gcc/tree-chkp.c | 11 ++++++-----
2 files changed, 20 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/mpx/pr79753.c
diff --git a/gcc/testsuite/gcc.target/i386/mpx/pr79753.c b/gcc/testsuite/gcc.target/i386/mpx/pr79753.c
new file mode 100644
index 00000000000..9b7bc52e1ed
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/pr79753.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx -O2" } */
+
+int
+foo (void)
+{
+ return 0;
+}
+
+void
+bar (int **p)
+{
+ *p = (int *) (__UINTPTR_TYPE__) foo ();
+}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index acd57eac5ef..c057b977342 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -2218,6 +2218,7 @@ chkp_build_returned_bound (gcall *call)
gimple *stmt;
tree fndecl = gimple_call_fndecl (call);
unsigned int retflags;
+ tree lhs = gimple_call_lhs (call);
/* To avoid fixing alloca expands in targets we handle
it separately. */
@@ -2227,9 +2228,8 @@ chkp_build_returned_bound (gcall *call)
|| DECL_FUNCTION_CODE (fndecl) == BUILT_IN_ALLOCA_WITH_ALIGN))
{
tree size = gimple_call_arg (call, 0);
- tree lb = gimple_call_lhs (call);
gimple_stmt_iterator iter = gsi_for_stmt (call);
- bounds = chkp_make_bounds (lb, size, &iter, true);
+ bounds = chkp_make_bounds (lhs, size, &iter, true);
}
/* We know bounds returned by set_bounds builtin call. */
else if (fndecl
@@ -2282,9 +2282,10 @@ chkp_build_returned_bound (gcall *call)
bounds = chkp_find_bounds (gimple_call_arg (call, argno), &iter);
}
- else if (chkp_call_returns_bounds_p (call))
+ else if (chkp_call_returns_bounds_p (call)
+ && BOUNDED_P (lhs))
{
- gcc_assert (TREE_CODE (gimple_call_lhs (call)) == SSA_NAME);
+ gcc_assert (TREE_CODE (lhs) == SSA_NAME);
/* In general case build checker builtin call to
obtain returned bounds. */
@@ -2311,7 +2312,7 @@ chkp_build_returned_bound (gcall *call)
print_gimple_stmt (dump_file, call, 0, TDF_VOPS|TDF_MEMSYMS);
}
- bounds = chkp_maybe_copy_and_register_bounds (gimple_call_lhs (call), bounds);
+ bounds = chkp_maybe_copy_and_register_bounds (lhs, bounds);
return bounds;
}
--
2.11.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MPX: fix PR middle-end/79753
2017-03-10 13:15 [PATCH] MPX: fix PR middle-end/79753 Martin Liška
@ 2017-03-14 22:27 ` Ilya Enkovich
2017-03-15 10:15 ` Martin Liška
0 siblings, 1 reply; 6+ messages in thread
From: Ilya Enkovich @ 2017-03-14 22:27 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches
2017-03-10 16:15 GMT+03:00 Martin Liška <mliska@suse.cz>:
> Hello.
>
> Currently, __builtin_ia32_bndret is used for all functions that have non-void
> return type. I think the right fix is to return bounds just for a bounded type.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Martin
Hi,
The fix makes sense and allows you to pass the test but it seems like we
still have the issue in inlining which can result in bndret call with wrong arg.
Do you avoid all such cases by this fix? Can we still have similar problem
if cast function with integer return type to a function with pointer return type
and the inline the call (not sure if we can inline such calls)?
I think this fix still should go to trunk anyway.
Thanks,
Ilya
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MPX: fix PR middle-end/79753
2017-03-14 22:27 ` Ilya Enkovich
@ 2017-03-15 10:15 ` Martin Liška
2017-03-15 17:09 ` Jeff Law
2017-03-15 19:00 ` Ilya Enkovich
0 siblings, 2 replies; 6+ messages in thread
From: Martin Liška @ 2017-03-15 10:15 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: GCC Patches
On 03/14/2017 11:27 PM, Ilya Enkovich wrote:
> 2017-03-10 16:15 GMT+03:00 Martin Liška <mliska@suse.cz>:
>> Hello.
>>
>> Currently, __builtin_ia32_bndret is used for all functions that have non-void
>> return type. I think the right fix is to return bounds just for a bounded type.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>> Martin
>
> Hi,
>
> The fix makes sense and allows you to pass the test but it seems like we
> still have the issue in inlining which can result in bndret call with wrong arg.
Hi.
The test I added does probably what you mean: a return value of integer is casted
to a pointer type. Or?
> Do you avoid all such cases by this fix? Can we still have similar problem
> if cast function with integer return type to a function with pointer return type
> and the inline the call (not sure if we can inline such calls)?
>
> I think this fix still should go to trunk anyway.
Good, may I consider this as patch approval?
Thanks,
Martin
>
> Thanks,
> Ilya
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MPX: fix PR middle-end/79753
2017-03-15 10:15 ` Martin Liška
@ 2017-03-15 17:09 ` Jeff Law
2017-03-15 19:01 ` Ilya Enkovich
2017-03-15 19:00 ` Ilya Enkovich
1 sibling, 1 reply; 6+ messages in thread
From: Jeff Law @ 2017-03-15 17:09 UTC (permalink / raw)
To: Martin Liška, Ilya Enkovich; +Cc: GCC Patches
On 03/15/2017 04:15 AM, Martin Liška wrote:
> On 03/14/2017 11:27 PM, Ilya Enkovich wrote:
>> 2017-03-10 16:15 GMT+03:00 Martin Liška <mliska@suse.cz>:
>>> Hello.
>>>
>>> Currently, __builtin_ia32_bndret is used for all functions that have
>>> non-void
>>> return type. I think the right fix is to return bounds just for a
>>> bounded type.
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Ready to be installed?
>>> Martin
>>
>> Hi,
>>
>> The fix makes sense and allows you to pass the test but it seems like we
>> still have the issue in inlining which can result in bndret call with
>> wrong arg.
>
> Hi.
>
> The test I added does probably what you mean: a return value of integer
> is casted
> to a pointer type. Or?
>
>> Do you avoid all such cases by this fix? Can we still have similar
>> problem
>> if cast function with integer return type to a function with pointer
>> return type
>> and the inline the call (not sure if we can inline such calls)?
>>
>> I think this fix still should go to trunk anyway.
>
> Good, may I consider this as patch approval?
Yes. Ilya knows this stuff better than anyone, even if he's not listed
as an official maintainer.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MPX: fix PR middle-end/79753
2017-03-15 10:15 ` Martin Liška
2017-03-15 17:09 ` Jeff Law
@ 2017-03-15 19:00 ` Ilya Enkovich
1 sibling, 0 replies; 6+ messages in thread
From: Ilya Enkovich @ 2017-03-15 19:00 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches
2017-03-15 13:15 GMT+03:00 Martin Liška <mliska@suse.cz>:
> On 03/14/2017 11:27 PM, Ilya Enkovich wrote:
>>
>> 2017-03-10 16:15 GMT+03:00 Martin Liška <mliska@suse.cz>:
>>>
>>> Hello.
>>>
>>> Currently, __builtin_ia32_bndret is used for all functions that have
>>> non-void
>>> return type. I think the right fix is to return bounds just for a bounded
>>> type.
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Ready to be installed?
>>> Martin
>>
>>
>> Hi,
>>
>> The fix makes sense and allows you to pass the test but it seems like we
>> still have the issue in inlining which can result in bndret call with
>> wrong arg.
>
>
> Hi.
>
> The test I added does probably what you mean: a return value of integer is
> casted
> to a pointer type. Or?
I suspect problem may still exist when you cast function, not just
returned value.
Inline is supposed to always remove retbnd corresponding to expanded call.
You avoid the problem in the testcase by not producing retbnd call. But in
case of calling a function casted to another function type we might
still hit this
issue in inline. I'll try to make a test.
Thanks,
Ilya
>
>> Do you avoid all such cases by this fix? Can we still have similar problem
>> if cast function with integer return type to a function with pointer
>> return type
>> and the inline the call (not sure if we can inline such calls)?
>>
>> I think this fix still should go to trunk anyway.
>
>
> Good, may I consider this as patch approval?
> Thanks,
> Martin
>
>>
>> Thanks,
>> Ilya
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MPX: fix PR middle-end/79753
2017-03-15 17:09 ` Jeff Law
@ 2017-03-15 19:01 ` Ilya Enkovich
0 siblings, 0 replies; 6+ messages in thread
From: Ilya Enkovich @ 2017-03-15 19:01 UTC (permalink / raw)
To: Jeff Law; +Cc: Martin Liška, GCC Patches
2017-03-15 20:09 GMT+03:00 Jeff Law <law@redhat.com>:
> On 03/15/2017 04:15 AM, Martin Liška wrote:
>>
>> On 03/14/2017 11:27 PM, Ilya Enkovich wrote:
>>>
>>> 2017-03-10 16:15 GMT+03:00 Martin Liška <mliska@suse.cz>:
>>>>
>>>> Hello.
>>>>
>>>> Currently, __builtin_ia32_bndret is used for all functions that have
>>>> non-void
>>>> return type. I think the right fix is to return bounds just for a
>>>> bounded type.
>>>>
>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>
>>>> Ready to be installed?
>>>> Martin
>>>
>>>
>>> Hi,
>>>
>>> The fix makes sense and allows you to pass the test but it seems like we
>>> still have the issue in inlining which can result in bndret call with
>>> wrong arg.
>>
>>
>> Hi.
>>
>> The test I added does probably what you mean: a return value of integer
>> is casted
>> to a pointer type. Or?
>>
>>> Do you avoid all such cases by this fix? Can we still have similar
>>> problem
>>> if cast function with integer return type to a function with pointer
>>> return type
>>> and the inline the call (not sure if we can inline such calls)?
>>>
>>> I think this fix still should go to trunk anyway.
>>
>>
>> Good, may I consider this as patch approval?
>
> Yes. Ilya knows this stuff better than anyone, even if he's not listed as
> an official maintainer.
I'm still in the list :)
Ilya
>
> jeff
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-15 19:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-10 13:15 [PATCH] MPX: fix PR middle-end/79753 Martin Liška
2017-03-14 22:27 ` Ilya Enkovich
2017-03-15 10:15 ` Martin Liška
2017-03-15 17:09 ` Jeff Law
2017-03-15 19:01 ` Ilya Enkovich
2017-03-15 19:00 ` Ilya Enkovich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).