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