Re: [C++] Add -fnull-this-pointer

2016-03-08 Thread Jan Hubicka
Hi, so I am not sure what is the consensus. Here are my two cents. I added the code motivated by looking into multiple inheritance code, where we often wind to if (this != null) foo = this + offset; else foo = NULL; which is redundant. I built libreoffice+LTO with and without this change

Re: [C++] Add -fnull-this-pointer

2016-03-07 Thread Richard Biener
On Mon, Mar 7, 2016 at 1:14 PM, Markus Trippelsdorf wrote: > On 2016.03.07 at 13:03 +0100, Jakub Jelinek wrote: >> On Mon, Mar 07, 2016 at 12:57:20PM +0100, Richard Biener wrote: >> > (no strong opinion about that - but for example the recent -flifetime-dse >> >

Re: [C++] Add -fnull-this-pointer

2016-03-07 Thread Markus Trippelsdorf
On 2016.03.07 at 13:03 +0100, Jakub Jelinek wrote: > On Mon, Mar 07, 2016 at 12:57:20PM +0100, Richard Biener wrote: > > (no strong opinion about that - but for example the recent -flifetime-dse > > strengthening fallout is similar) > > -flifetime-dse change is different, we default to

Re: [C++] Add -fnull-this-pointer

2016-03-07 Thread Richard Biener
On Mon, Mar 7, 2016 at 1:03 PM, Jakub Jelinek wrote: > On Mon, Mar 07, 2016 at 12:57:20PM +0100, Richard Biener wrote: >> > Honza, can you please repost the patch. Richard said on IRC that he may >> > reconsider his rejection after all. >> > >> > I've tested the patch on my

Re: [C++] Add -fnull-this-pointer

2016-03-07 Thread Jakub Jelinek
On Mon, Mar 07, 2016 at 12:57:20PM +0100, Richard Biener wrote: > > Honza, can you please repost the patch. Richard said on IRC that he may > > reconsider his rejection after all. > > > > I've tested the patch on my Gentoo test machine and it fixes segfaults > > in LLVM, QT, Chromium, Kdevelop. >

Re: [C++] Add -fnull-this-pointer

2016-03-07 Thread Richard Biener
On Mon, Mar 7, 2016 at 12:35 PM, Markus Trippelsdorf wrote: > On 2016.01.19 at 13:11 +0100, Jan Hubicka wrote: >> according to Trevor, the assumption about THIS pointer being non-NULL breaks >> several bigger C++ packages (definitly including Firefox, but I believe >>

Re: [C++] Add -fnull-this-pointer

2016-03-07 Thread Markus Trippelsdorf
On 2016.01.19 at 13:11 +0100, Jan Hubicka wrote: > according to Trevor, the assumption about THIS pointer being non-NULL breaks > several bigger C++ packages (definitly including Firefox, but I believe > kdevelop was mentioned, too). This patch makes the feature to be controlable > by a dedicated

[C++] Add -fnull-this-pointer

2016-01-19 Thread Jan Hubicka
Hi, according to Trevor, the assumption about THIS pointer being non-NULL breaks several bigger C++ packages (definitly including Firefox, but I believe kdevelop was mentioned, too). This patch makes the feature to be controlable by a dedicated flag. I am not sure about the default. We now have

Re: [C++] Add -fnull-this-pointer

2016-01-19 Thread Markus Trippelsdorf
On 2016.01.19 at 13:11 +0100, Jan Hubicka wrote: > according to Trevor, the assumption about THIS pointer being non-NULL breaks > several bigger C++ packages (definitly including Firefox, but I believe > kdevelop was mentioned, too). This patch makes the feature to be controlable > by a dedicated

Re: [C++] Add -fnull-this-pointer

2016-01-19 Thread Jan Hubicka
> On Jan 19, 2016, at 6:41 AM, Jakub Jelinek wrote: > > But then perhaps it will be better incentive for the projects to fix their > > cruft. With a specialized option they will keep broken code forever. > > Flags are forever. OK, in

Re: [C++] Add -fnull-this-pointer

2016-01-19 Thread Mike Stump
On Jan 19, 2016, at 6:41 AM, Jakub Jelinek wrote: > But then perhaps it will be better incentive for the projects to fix their > cruft. With a specialized option they will keep broken code forever. Flags are forever.

Re: [C++] Add -fnull-this-pointer

2016-01-19 Thread Trevor Saunders
On Tue, Jan 19, 2016 at 01:11:44PM +0100, Jan Hubicka wrote: > Hi, > according to Trevor, the assumption about THIS pointer being non-NULL breaks That was Markus, not me. > several bigger C++ packages (definitly including Firefox, but I believe > kdevelop was mentioned, too). This patch makes

Re: [C++] Add -fnull-this-pointer

2016-01-19 Thread Richard Biener
On Tue, Jan 19, 2016 at 2:18 PM, Trevor Saunders wrote: > On Tue, Jan 19, 2016 at 01:11:44PM +0100, Jan Hubicka wrote: >> Hi, >> according to Trevor, the assumption about THIS pointer being non-NULL breaks > > That was Markus, not me. > >> several bigger C++ packages

Re: [C++] Add -fnull-this-pointer

2016-01-19 Thread Markus Trippelsdorf
On 2016.01.19 at 15:23 +0100, Richard Biener wrote: > On Tue, Jan 19, 2016 at 2:18 PM, Trevor Saunders > wrote: > > On Tue, Jan 19, 2016 at 01:11:44PM +0100, Jan Hubicka wrote: > >> Hi, > >> according to Trevor, the assumption about THIS pointer being non-NULL > >> breaks

Re: [C++] Add -fnull-this-pointer

2016-01-19 Thread Jakub Jelinek
On Tue, Jan 19, 2016 at 03:37:02PM +0100, Markus Trippelsdorf wrote: > > Agreed. As we already have a flag that can be used as a workaround I don't > > see > > a reason to add another more specific one. That just makes it a > > lesser incentive > > for people to fix their code. > > Well,