This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR64078
- From: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- To: Tom de Vries <Tom_deVries at mentor dot com>, Marek Polacek <polacek at redhat dot com>
- Cc: Jeff Law <law at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>, "H.J. Lu" <hjl dot tools at gmail dot com>
- Date: Mon, 29 Aug 2016 16:43:35 +0000
- Subject: Re: [PATCH] Fix PR64078
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=softfail (sender IP is 10.152.4.52) smtp.mailfrom=hotmail.de; mentor.com; dkim=none (message not signed) header.d=none;mentor.com; dmarc=none action=none header.from=hotmail.de;
- References: <DUB118-W242786F48D078983D0B3DAE4550@phx.gbl> <20150907100700.GC30849@redhat.com> <DUB118-W39447996258C266A1D63AFE4540@phx.gbl> <55EF3690.8030201@redhat.com> <DUB118-W441CDEC82B86B82325CA23E4520@phx.gbl> <55F050D5.3020408@redhat.com> <DUB118-W29D540E8FAA34840CA988CE4520@phx.gbl> <20150917150026.GC27588@redhat.com> <55FAECA8.3070909@redhat.com> <DUB118-W4175AF780B5B3FD98AA47E45A0@phx.gbl> <20150917180838.GE27588@redhat.com> <7fcd8db4-af35-a0fe-fc5a-f7999d0c5fca@mentor.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
Thanks!
Actually my patch missed to fix one combination: -m32 with -fpic
make check-gcc-c++ RUNTESTFLAGS="ubsan.exp=object-size-9.c --tool_opts
'-m32 -fpic'"
FAIL: c-c++-common/ubsan/object-size-9.c -O2 execution test
FAIL: c-c++-common/ubsan/object-size-9.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none execution test
The problem here is that the functions f2 and f3 access a stack-
based object out of bounds and that is inlined in main and
therefore smashes the return address of main in this case.
A possible fix could look like follows:
Index: object-size-9.c
===================================================================
--- object-size-9.c (revision 239794)
+++ object-size-9.c (working copy)
@@ -93,5 +93,9 @@
#endif
f4 (12);
f5 (12);
+#ifdef __cplusplus
+ /* Stack may be smashed by f2/f3 above. */
+ __builtin_exit (0);
+#endif
return 0;
}
Do you think that this should be fixed too?
Bernd.
On 08/29/16 09:59, Tom de Vries wrote:
> On 17/09/15 20:08, Marek Polacek wrote:
>> On Thu, Sep 17, 2015 at 08:06:48PM +0200, Bernd Edlinger wrote:
>>> Hi,
>>>
>>> On Thu, 17 Sep 2015 10:39:04, Jeff Law wrote:
>>>>
>>>> On 09/17/2015 09:00 AM, Marek Polacek wrote:
>>>>> On Wed, Sep 09, 2015 at 07:48:15PM +0200, Bernd Edlinger wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Wed, 9 Sep 2015 09:31:33, Jeff Law wrote:
>>>>>>> You could probably make the function static or change its
>>>>>>> visibility via
>>>>>>> a function attribute (there's a visibility attribute which can
>>>>>>> take the
>>>>>>> values default, hidden protected or internal). Default visibility
>>>>>>> essentially means the function can be overridden. I think
>>>>>>> changing it
>>>>>>> to "protected" might work. Note if we do that, we may need some
>>>>>>> kind of
>>>>>>> target selector on the test since not all targets support the
>>>>>>> various
>>>>>>> visibility attributes.
>>>>>>>
>>>>>>
>>>>>> Yes, it works both ways: static works, and __attribute__
>>>>>> ((visibility ("protected"))) works too:
>>>>>>
>>>>>> make check-gcc-c++ RUNTESTFLAGS="ubsan.exp=object-size-9.c
>>>>>> --target_board='unix{-fpic,-mcmodel=medium,-fpic\
>>>>>> -mcmodel=medium,-mcmodel=large,-fpic\ -mcmodel=large}'"
>>>>>>
>>>>>> has all tests passed, but..
>>>>>>
>>>>>> make check-gcc-c++ RUNTESTFLAGS="ubsan.exp=object-size-9.c
>>>>>> --target_board='unix{-fno-inline}'"
>>>>>>
>>>>>> still fails in the same way for all workarounds: inline, static,
>>>>>> and __attribute__ ((visibility ("protected"))).
>>>>>>
>>>>>> Maybe "static" would be preferable?
>>>>>
>>>>> Yeah, I'd go with static if that helps. I'd rather avoid playing games
>>>>> with visibility.
>>>> Static is certainly easier and doesn't rely on targets implementing all
>>>> the visibility capabilities. So static is probably the best approach.
>>>>
>>>
>>> That's fine for me too, so is the original patch OK for trunk with
>>> s/inline/static/ ?
>>
>> Yes.
>>
>
> Hi,
>
> I've backported this fix to the 5 branch.
>
> Thanks,
> - Tom
>