This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
- From: David Malcolm <dmalcolm at redhat dot com>
- To: David Edelsohn <dje dot gcc at gmail dot com>
- Cc: Bernd Schmidt <bschmidt at redhat dot com>, Jeffrey Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <richard dot guenther at gmail dot com>, Dodji Seketeli <dodji at redhat dot com>
- Date: Sat, 21 Nov 2015 15:00:02 -0500
- Subject: Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
- Authentication-results: sourceware.org; auth=none
- References: <CAGWvnynj92T8T_U8P7x1SPyQpiW=_1jtUWhMxSn+aiVo+Ffkbw at mail dot gmail dot com> <1447561939 dot 19594 dot 27 dot camel at surprise> <1447707000 dot 19594 dot 64 dot camel at surprise> <564A4BD6 dot 9030201 at redhat dot com> <1447773221 dot 19594 dot 84 dot camel at surprise> <564B46BA dot 3000107 at redhat dot com> <1447791147 dot 19594 dot 86 dot camel at surprise> <CAGWvnyn5kCECW+w-3B2k8KoTpK54M6Eh9dVKo2jMKRnCs2bGgg at mail dot gmail dot com>
On Sat, 2015-11-21 at 13:54 -0500, David Edelsohn wrote:
> On Tue, Nov 17, 2015 at 3:12 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> > On Tue, 2015-11-17 at 16:24 +0100, Bernd Schmidt wrote:
> >> On 11/17/2015 04:13 PM, David Malcolm wrote:
> >> > On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote:
> >> >>
> >> >> Should c_expr perhaps acquire a constructor so that this problem is
> >> >> avoided in the future? The whole thing seems somewhat error-prone.
> >> >
> >> > I agree that it's error prone, and the ctor approach is what I've been
> >> > trying for the C++ FE [1] but I suspect that touching that in the C FE
> >> > would be a much more invasive patch (unless we simply give it a default
> >> > ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?).
> >>
> >> The UNKNOWN_LOCATIONS pair would have been my approach, yes.
> >>
> >> > This case gains a pair of locals: start_loc and end_loc (so that we can
> >> > track the spelling range whilst retaining the "loc" used for the caret),
> >> > and I preferred to confine their scope to within the case, hence the
> >> > extra braced block. Omitting the braced block leads to:
> >> > ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label [-fpermissive]
> >> > case RID_OFFSETOF:
> >> > ^
> >> > ../../src/gcc/c/c-parser.c:7472:17: error: crosses initialization of âlocation_t end_locâ
> >> > location_t end_loc = c_parser_peek_token (parser)->get_finish ();
> >> > ^
> >> > etc.
> >>
> >> Hmm, odd, I tried placing just the location_t start_loc line into the
> >> switch and that appeared to compile fine. But I guess this is not a huge
> >> problem.
> >> >
> >> > Is the combination of the 3 patches OK for trunk? (assuming
> >> > bootstrap®rest; it's only the braced-init tweak that hasn't been).
> >>
> >> Yes.
> >
> > Thanks. I've committed the 3 patches to trunk as r230497, which should
> > fix the worst of the regressions caused by r230331 seen on AIX. I'll
> > continue to investigate as per the discussion above.
>
> Hi, David
>
> The new stret-1.m Objective C failure on AIX shows the same symptoms.
> Is there another fix needed for Objective C?
>
> #1 0x10016794 in _Z14linemap_lookupP9line_mapsj (set=0x70000000, line=991)
> at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991
> 991 linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));
I believe this one is fixed by the patch I posted here:
"[PATCH] Fix PR objc/68438 (uninitialized source ranges)"
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02536.html
(it runs cleanly under valgrind on x86_64 with that patch applied)