Hmm, original massage bounced, resent, without html.
________________________________
> From: bernd.edlin...@hotmail.de
> To: l...@redhat.com; gcc-patches@gcc.gnu.org
> CC: jos...@codesourcery.com
> Subject: RE: [PATCH] Fix PR preprocessor/58893 access to uninitialized memory
> Date: Sat, 27 Sep 2014 11:42:29 +0200
>
>
>
> On Fri, 26 Sep 2014 12:48:44, Jeff Law wrote:
> >
> > On 09/26/14 06:21, Bernd Edlinger wrote:
> >>>>
> >>>>Hi,
> >>>>
> >>>>this patch fixes PR58893, which is an access to uninitialized
> memory, which may or may not crash in
> >>>>linemap_resolve_location, or just print error messages with bogus
> location.
> >>>>
> >>>>When the first -include file is processed we have the case, where
> >>>>pfile->cur_token == pfile->cur_run->base, this is directly called
> >>>>by the front end. However in the case of the second -include file,
> >>>>this is called from _cpp_lex_token -> _cpp_get_fresh_line ->
> >>>>cpp_push_include, with pfile->cur_token != pfile->cur_run->base,
> >>>>and pfile->cur_token[-1].src_loc and token not (yet) initialized.
> >>>>The problem is, when the include file cannot be found, we need
> >>>>src_loc to be initialized to some safe value: 0 means UNKNOWN_LOCATION.
> >>>>
> >>>>Regarding the hunk in cpp_diagnostic, which is not directly involved
> >>>>in this bug, but it is still obviously wrong:
> >>>>
> >>>>The line "src_loc = pfile->cur_run->prev->limit->src_loc"
> >>>>is probably unreachable, but will crash it is ever executed.
> >>>>
> >>>>see:
> >>>>
> >>>>_cpp_init_tokenrun (tokenrun *run, unsigned int count)
> >>>>{
> >>>>run->base = XNEWVEC (cpp_token, count);
> >>>>run->limit = run->base + count;
> >>>>run->next = NULL;
> >>>>}
> >>>>
> >>>>so, limit points at the end of the run.
> >>>>
> >>>>
> >>>>Boot-Strapped and Regression-tested on x86_64-linux-gnu
> >>>>Ok for trunk?
> >>>>
> >>>>
> >>>>Thanks
> >>>>Bernd.
> >>>>
> >>
> >>
> >>
> >> changelog-pr58893.txt
> >>
> >>
> >> 2014-09-26 Bernd Edlinger<bernd.edlin...@hotmail.de>
> >>
> >> PR preprocessor/58893
> >> * errors.c (cpp_diagnostic): Fix possible out of bounds access.
> >> * files.c (_cpp_stack_include): Initialize src_loc for IT_CMDLINE.
> >>
> >>
> >> patch-pr58893.diff
> >>
> >>
> >> --- libcpp/errors.c 2014-01-02 23:24:45.000000000 +0100
> >> +++ libcpp/errors.c 2014-09-24 10:30:33.708048505 +0200
> >> @@ -48,10 +48,7 @@ cpp_diagnostic (cpp_reader * pfile, int
> >> current run -- that is invalid. */
> >> else if (pfile->cur_token == pfile->cur_run->base)
> >> {
> >> - if (pfile->cur_run->prev != NULL)
> >> - src_loc = pfile->cur_run->prev->limit->src_loc;
> >> - else
> >> - src_loc = 0;
> >> + src_loc = 0;
> >> }
> >> else
> >> {
> >> --- libcpp/files.c 2014-05-21 20:54:12.000000000 +0200
> >> +++ libcpp/files.c 2014-09-24 10:35:47.191117490 +0200
> >> @@ -991,6 +991,9 @@ _cpp_stack_include (cpp_reader *pfile, c
> >> _cpp_file *file;
> >> bool stacked;
> >>
> >> + if (type == IT_CMDLINE && pfile->cur_token != pfile->cur_run->base)
> >> + pfile->cur_token[-1].src_loc = 0;
> > Comment before this change. Someone not familiar with this code is
> > going to have no idea why these two lines exist.
> >
>
> Ok, I added a comment now, do you like it?
>
> > Please try to include a testcase. If you're having trouble reproducing
> > on the trunk, you could use MALLOC_PERTURB per c#8 in the bug report.
> > If there's a way to set environment variables in our testing framework
> > that may be a reasonable way to test (if you need to do that, limit
> > testing to linux targets as we'll have a dependency on glibc features).
> >
>
> For whatever reason, the first -include must end with a pragma
> as in the PR, and MALLOC_PERTURB_ must be set to something.
> Then we get an ICE, otherwise we get an error message without line number.
> I tried to make this a valid test case, but that might be less trivial than
> it looks at first sight.
>
> I tried to set MALLOC_PERTURB_=123 globally, like this:
>
> MALLOC_PERTURB_=123 make -k check
>
> but then this happened:
>
> ....
> WARNING: program timed out.
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c, -O3 -fomit-frame-pointer
> -funroll-all-loops -finline-functions -dumpbase dump1/dump-noaddr.c
> -DMASK=1 -x c --param ggc-min-heapsize=1 -fdump-ipa-all -fdump-rtl-all
> -fdump-tree-all -fdump-noaddr
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c.000i.cgraph, -O3
> -fomit-frame-pointer -funroll-all-loops -finline-functions comparison
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c.003t.original, -O3
> -fomit-frame-pointer -funroll-all-loops -finline-functions comparison
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c.032t.profile_estimate, -O3
> -fomit-frame-pointer -funroll-all-loops -finline-functions comparison
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c.253t.statistics, -O3
> -fomit-frame-pointer -funroll-all-loops -finline-functions comparison
> WARNING: program timed out.
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c, -O3 -g -dumpbase
> dump1/dump-noaddr.c -DMASK=1 -x c --param ggc-min-heapsize=1
> -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -fdump-noaddr
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c.000i.cgraph, -O3 -g comparison
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c.003t.original, -O3 -g comparison
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c.032t.profile_estimate, -O3
> -g comparison
> FAIL: gcc.c-torture/unsorted/dump-noaddr.c.253t.statistics, -O3 -g
> comparison
> ^Cgot a INT signal, interrupted by user
>
> Well I am afraid this test case alone takes hours, and would disrupt
> the whole test suite,
> so currently I think it would be the right thing to set MALLOC_PERTURB_=123
> globally in the test suite, but this looks not like a small step for
> one man....
>
> Any Ideas what is wrong with that test case?
>
>
> Well, I added a test case, but it does not reliably fail without the
> patch, because setting
> MALLOC_PERTURB_ causes too much trouble at this time.
>
> I would propose to set MALLOC_PERTURB_ globally at a later time.
>
> Boot-Strapped & Regression-Tested on x86_64-linux-gnu.
> Ok for trunk?
>
>
> Thanks
> Bernd.
>
> > jeff
> >
libcpp: 2014-09-27 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR preprocessor/58893
* errors.c (cpp_diagnostic): Fix possible out of bounds access.
* files.c (_cpp_stack_include): Initialize src_loc for IT_CMDLINE.
testsuite:
2014-09-27 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR preprocessor/58893
* gcc.dg/pr58893.c: New test case.
* gcc.dg/pr58893-0.h: New include.
patch-pr58893.diff
Description: Binary data
