On Thu, Jul 9, 2015 at 6:37 PM, Jeff Law <l...@redhat.com> wrote: > On 07/09/2015 09:41 AM, Jakub Jelinek wrote: >> >> On Thu, Jul 09, 2015 at 09:34:25AM -0600, Jeff Law wrote: >>> >>> On 07/09/2015 08:13 AM, Jakub Jelinek wrote: >>>> >>>> On Thu, Jul 09, 2015 at 03:56:35PM +0200, mliska wrote: >>>>> >>>>> --- >>>>> gcc/testsuite/g++.dg/ubsan/vptr-1.C | 2 +- >>>>> gcc/testsuite/g++.dg/ubsan/vptr-2.C | 2 +- >>>>> gcc/testsuite/g++.dg/ubsan/vptr-3.C | 2 +- >>>>> 3 files changed, 3 insertions(+), 3 deletions(-) >>>> >>>> >>>> I'd actually think it would be better to give up on the >>>> UBSAN_* internal calls in tail merging. Those internal pass >>>> arguments based on their gimple_location, so tail merging breaks >>>> them. >>> >>> So I think the larger question here is should differences in gimple >>> locations prevent tail merging? I'd tend to think not, which then begs >>> the >> >> >> Generally no. >> >>> question, are the UBSAN calls special enough to warrant an exception? >> >> >> ASAN internal calls too I suppose. And yes, I believe they are special >> enough to warrant an exception. The speciality is in them being lowered >> later on into a call that is passed as one argument a structure containing >> file:line into derived from that location, and for those calls that info >> is >> very important (by using -fsanitize=address or -fsanitize=undefined >> the user already says that he doesn't care that much about generated code >> quality, the extra overhead is already there). Another option would be >> for >> -fsanitize=address or undefined etc. to disable various optimization >> options >> (it does already disable some like non-null optimizations, because it is >> essential, but I believe there is no reason not to perform tail merging >> even with those options, as long as there are none of the problematic >> internal calls involved, or if the locus is the same. One could consider >> gimple_location as yet another parameter to those internal calls, not >> emitted directly just to avoid wasting compiler memory. > > I figured you'd say something along these lines :-) Essentially the > argument is the line numbers are absolutely core to what the sanitizers > provide by way their diagnostics. Getting them wrong because we tail merged > is likely to cause huge amounts of confusion. > > Martin -- if you could have the existence of ASAN or UBSAN calls inhibit > tail merging. I guess you could potentially check the location information > and still allow if the location information on those calls matches, but I > doubt that's worth the effort.
But the warning on the "bogus" line will still be warranted, so user goes and fixes it. Then tail-merge no longer applies and he gets the warning on the other warranted line. So in the end tail-merging caused him to fix _two_ bugs instead of just the single one he'd run into otherwise! -> I don't see any issue with tail-merging those. Richard. > Jeff >