From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13366 invoked by alias); 27 Sep 2014 09:53:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 13345 invoked by uid 89); 27 Sep 2014 09:53:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 X-HELO: DUB004-OMC3S14.hotmail.com Received: from dub004-omc3s14.hotmail.com (HELO DUB004-OMC3S14.hotmail.com) (157.55.2.23) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA256 encrypted) ESMTPS; Sat, 27 Sep 2014 09:53:07 +0000 Received: from DUB118-W10 ([157.55.2.8]) by DUB004-OMC3S14.hotmail.com with Microsoft SMTPSVC(7.5.7601.22724); Sat, 27 Sep 2014 02:53:03 -0700 X-TMN: [AbiEMakqkdkLKMt+qUi6MDgG1VPibra2] Message-ID: Content-Type: multipart/mixed; boundary="_c014c58c-1d78-443e-93a3-d791d864f378_" From: Bernd Edlinger To: "gcc-patches@gcc.gnu.org" Subject: RE: [PATCH] Fix PR preprocessor/58893 access to uninitialized memory Date: Sat, 27 Sep 2014 09:53:00 -0000 In-Reply-To: References: ,,<5425B50C.6060008@redhat.com>, MIME-Version: 1.0 X-SW-Source: 2014-09/txt/msg02453.txt.bz2 --_c014c58c-1d78-443e-93a3-d791d864f378_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-length: 6840 Hmm, original massage bounced, resent, without html. ________________________________ > From: bernd.edlinger@hotmail.de=20 > To: law@redhat.com; gcc-patches@gcc.gnu.org=20 > CC: joseph@codesourcery.com=20 > Subject: RE: [PATCH] Fix PR preprocessor/58893 access to uninitialized me= mory=20 > Date: Sat, 27 Sep 2014 11:42:29 +0200=20 >=20=20 >=20=20 >=20=20 > On Fri, 26 Sep 2014 12:48:44, Jeff Law wrote:=20 > >=20 > > On 09/26/14 06:21, Bernd Edlinger wrote:=20 > >>>>=20 > >>>>Hi,=20 > >>>>=20 > >>>>this patch fixes PR58893, which is an access to uninitialized=20=20 > memory, which may or may not crash in=20 > >>>>linemap_resolve_location, or just print error messages with bogus=20= =20 > location.=20 > >>>>=20 > >>>>When the first -include file is processed we have the case, where=20 > >>>>pfile->cur_token =3D=3D pfile->cur_run->base, this is directly called= =20 > >>>>by the front end. However in the case of the second -include file,=20 > >>>>this is called from _cpp_lex_token -> _cpp_get_fresh_line ->=20 > >>>>cpp_push_include, with pfile->cur_token !=3D pfile->cur_run->base,=20 > >>>>and pfile->cur_token[-1].src_loc and token not (yet) initialized.=20 > >>>>The problem is, when the include file cannot be found, we need=20 > >>>>src_loc to be initialized to some safe value: 0 means UNKNOWN_LOCATIO= N.=20 > >>>>=20 > >>>>Regarding the hunk in cpp_diagnostic, which is not directly involved= =20 > >>>>in this bug, but it is still obviously wrong:=20 > >>>>=20 > >>>>The line "src_loc =3D pfile->cur_run->prev->limit->src_loc"=20 > >>>>is probably unreachable, but will crash it is ever executed.=20 > >>>>=20 > >>>>see:=20 > >>>>=20 > >>>>_cpp_init_tokenrun (tokenrun *run, unsigned int count)=20 > >>>>{=20 > >>>>run->base =3D XNEWVEC (cpp_token, count);=20 > >>>>run->limit =3D run->base + count;=20 > >>>>run->next =3D NULL;=20 > >>>>}=20 > >>>>=20 > >>>>so, limit points at the end of the run.=20 > >>>>=20 > >>>>=20 > >>>>Boot-Strapped and Regression-tested on x86_64-linux-gnu=20 > >>>>Ok for trunk?=20 > >>>>=20 > >>>>=20 > >>>>Thanks=20 > >>>>Bernd.=20 > >>>>=20 > >>=20 > >>=20 > >>=20 > >> changelog-pr58893.txt=20 > >>=20 > >>=20 > >> 2014-09-26 Bernd Edlinger=20 > >>=20 > >> PR preprocessor/58893=20 > >> * errors.c (cpp_diagnostic): Fix possible out of bounds access.=20 > >> * files.c (_cpp_stack_include): Initialize src_loc for IT_CMDLINE.=20 > >>=20 > >>=20 > >> patch-pr58893.diff=20 > >>=20 > >>=20 > >> --- libcpp/errors.c 2014-01-02 23:24:45.000000000 +0100=20 > >> +++ libcpp/errors.c 2014-09-24 10:30:33.708048505 +0200=20 > >> @@ -48,10 +48,7 @@ cpp_diagnostic (cpp_reader * pfile, int=20 > >> current run -- that is invalid. */=20 > >> else if (pfile->cur_token =3D=3D pfile->cur_run->base)=20 > >> {=20 > >> - if (pfile->cur_run->prev !=3D NULL)=20 > >> - src_loc =3D pfile->cur_run->prev->limit->src_loc;=20 > >> - else=20 > >> - src_loc =3D 0;=20 > >> + src_loc =3D 0;=20 > >> }=20 > >> else=20 > >> {=20 > >> --- libcpp/files.c 2014-05-21 20:54:12.000000000 +0200=20 > >> +++ libcpp/files.c 2014-09-24 10:35:47.191117490 +0200=20 > >> @@ -991,6 +991,9 @@ _cpp_stack_include (cpp_reader *pfile, c=20 > >> _cpp_file *file;=20 > >> bool stacked;=20 > >>=20 > >> + if (type =3D=3D IT_CMDLINE && pfile->cur_token !=3D pfile->cur_run->= base)=20 > >> + pfile->cur_token[-1].src_loc =3D 0;=20 > > Comment before this change. Someone not familiar with this code is=20 > > going to have no idea why these two lines exist.=20 > >=20 >=20=20 > Ok, I added a comment now, do you like it?=20 >=20=20 > > Please try to include a testcase. If you're having trouble reproducing= =20 > > on the trunk, you could use MALLOC_PERTURB per c#8 in the bug report.=20 > > If there's a way to set environment variables in our testing framework= =20 > > that may be a reasonable way to test (if you need to do that, limit=20 > > testing to linux targets as we'll have a dependency on glibc features).= =20 > >=20 >=20=20 > For whatever reason, the first -include must end with a pragma=20 > as in the PR, and MALLOC_PERTURB_ must be set to something.=20 > Then we get an ICE, otherwise we get an error message without line number= .=20 > I tried to make this a valid test case, but that might be less trivial th= an=20 > it looks at first sight.=20 >=20=20 > I tried to set MALLOC_PERTURB_=3D123 globally, like this:=20 >=20=20 > MALLOC_PERTURB_=3D123 make -k check=20 >=20=20 > but then this happened:=20 >=20=20 > ....=20 > WARNING: program timed out.=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c, -O3 -fomit-frame-pointer=20= =20 > -funroll-all-loops -finline-functions -dumpbase dump1/dump-noaddr.c=20= =20 > -DMASK=3D1 -x c --param ggc-min-heapsize=3D1 -fdump-ipa-all -fdump-rtl-al= l=20=20 > -fdump-tree-all -fdump-noaddr=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c.000i.cgraph, -O3=20=20 > -fomit-frame-pointer -funroll-all-loops -finline-functions comparison=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c.003t.original, -O3=20=20 > -fomit-frame-pointer -funroll-all-loops -finline-functions comparison=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c.032t.profile_estimate, -O3=20= =20 > -fomit-frame-pointer -funroll-all-loops -finline-functions comparison=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c.253t.statistics, -O3=20=20 > -fomit-frame-pointer -funroll-all-loops -finline-functions comparison=20 > WARNING: program timed out.=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c, -O3 -g -dumpbase=20=20 > dump1/dump-noaddr.c -DMASK=3D1 -x c --param ggc-min-heapsize=3D1=20=20 > -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -fdump-noaddr=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c.000i.cgraph, -O3 -g comparis= on=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c.003t.original, -O3 -g compar= ison=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c.032t.profile_estimate, -O3=20= =20 > -g comparison=20 > FAIL: gcc.c-torture/unsorted/dump-noaddr.c.253t.statistics, -O3 -g=20=20= =20 > comparison=20 > ^Cgot a INT signal, interrupted by user=20 >=20=20 > Well I am afraid this test case alone takes hours, and would disrupt=20=20 > the whole test suite,=20 > so currently I think it would be the right thing to set MALLOC_PERTURB_= =3D123=20 > globally in the test suite, but this looks not like a small step for=20=20 > one man....=20 >=20=20 > Any Ideas what is wrong with that test case?=20 >=20=20 >=20=20 > Well, I added a test case, but it does not reliably fail without the=20=20 > patch, because setting=20 > MALLOC_PERTURB_ causes too much trouble at this time.=20 >=20=20 > I would propose to set MALLOC_PERTURB_ globally at a later time.=20 >=20=20 > Boot-Strapped & Regression-Tested on x86_64-linux-gnu.=20 > Ok for trunk?=20 >=20=20 >=20=20 > Thanks=20 > Bernd.=20 >=20=20 > > jeff=20 > >=20 =20=09=09=20=09=20=20=20=09=09=20=20= --_c014c58c-1d78-443e-93a3-d791d864f378_ Content-Type: text/plain Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="changelog-pr58893.txt" Content-length: 525 bGliY3BwOgoyMDE0LTA5LTI3ICBCZXJuZCBFZGxpbmdlciAgPGJlcm5kLmVk bGluZ2VyQGhvdG1haWwuZGU+CgoJUFIgcHJlcHJvY2Vzc29yLzU4ODkzCgkq IGVycm9ycy5jIChjcHBfZGlhZ25vc3RpYyk6IEZpeCBwb3NzaWJsZSBvdXQg b2YgYm91bmRzIGFjY2Vzcy4KCSogZmlsZXMuYyAoX2NwcF9zdGFja19pbmNs dWRlKTogSW5pdGlhbGl6ZSBzcmNfbG9jIGZvciBJVF9DTURMSU5FLgoKdGVz dHN1aXRlOgoyMDE0LTA5LTI3ICBCZXJuZCBFZGxpbmdlciAgPGJlcm5kLmVk bGluZ2VyQGhvdG1haWwuZGU+CgoJUFIgcHJlcHJvY2Vzc29yLzU4ODkzCgkq IGdjYy5kZy9wcjU4ODkzLmM6IE5ldyB0ZXN0IGNhc2UuCgkqIGdjYy5kZy9w cjU4ODkzLTAuaDogTmV3IGluY2x1ZGUuCg== --_c014c58c-1d78-443e-93a3-d791d864f378_ Content-Type: application/octet-stream Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="patch-pr58893.diff" Content-length: 3201 SW5kZXg6IGdjYy90ZXN0c3VpdGUvZ2NjLmRnL3ByNTg4OTMtMC5oCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIGdjYy90ZXN0c3VpdGUvZ2NjLmRnL3By NTg4OTMtMC5oCShSZXZpc2lvbiAwKQorKysgZ2NjL3Rlc3RzdWl0ZS9nY2Mu ZGcvcHI1ODg5My0wLmgJKEFyYmVpdHNrb3BpZSkKQEAgLTAsMCArMSBAQAor I3ByYWdtYSBHQ0MgdmlzaWJpbGl0eSBwdXNoKGhpZGRlbikKSW5kZXg6IGdj Yy90ZXN0c3VpdGUvZ2NjLmRnL3ByNTg4OTMuYwo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBnY2MvdGVzdHN1aXRlL2djYy5kZy9wcjU4ODkzLmMJKFJl dmlzaW9uIDApCisrKyBnY2MvdGVzdHN1aXRlL2djYy5kZy9wcjU4ODkzLmMJ KEFyYmVpdHNrb3BpZSkKQEAgLTAsMCArMSw1IEBACisvKiBQUiBwcmVwcm9j ZXNzb3IvNTg4OTMgKi8KKy8qIHsgZGctZG8gY29tcGlsZSB9ICovCisvKiB7 IGRnLW9wdGlvbnMgIi1pbmNsdWRlIHByNTg4OTMtMC5oIC1pbmNsdWRlIHBy NTg4OTMtMS5oIC1JJHtzcmNkaXJ9L2djYy5kZyIgfSAqLworLyogeyBkZy1l cnJvciAicHI1ODg5My0xLmg6IE5vIHN1Y2ggZmlsZSBvciBkaXJlY3Rvcnki ICIiIHsgdGFyZ2V0ICotKi0qIH0gMCB9ICovCisvKiB7IGRnLXBydW5lLW91 dHB1dCAiY29tcGlsYXRpb24gdGVybWluYXRlZCIgfSAqLwpJbmRleDogbGli Y3BwL2Vycm9ycy5jCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIGxpYmNw cC9lcnJvcnMuYwkoUmV2aXNpb24gMjE1NjQ0KQorKysgbGliY3BwL2Vycm9y cy5jCShBcmJlaXRza29waWUpCkBAIC00OCwxMCArNDgsNyBAQCBjcHBfZGlh Z25vc3RpYyAoY3BwX3JlYWRlciAqIHBmaWxlLCBpbnQgbGV2ZWwsIGludAog ICAgICBjdXJyZW50IHJ1biAtLSB0aGF0IGlzIGludmFsaWQuICAqLwogICBl bHNlIGlmIChwZmlsZS0+Y3VyX3Rva2VuID09IHBmaWxlLT5jdXJfcnVuLT5i YXNlKQogICAgIHsKLSAgICAgIGlmIChwZmlsZS0+Y3VyX3J1bi0+cHJldiAh PSBOVUxMKQotCXNyY19sb2MgPSBwZmlsZS0+Y3VyX3J1bi0+cHJldi0+bGlt aXQtPnNyY19sb2M7Ci0gICAgICBlbHNlCi0Jc3JjX2xvYyA9IDA7CisgICAg ICBzcmNfbG9jID0gMDsKICAgICB9CiAgIGVsc2UKICAgICB7CkluZGV4OiBs aWJjcHAvZmlsZXMuYwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBsaWJj cHAvZmlsZXMuYwkoUmV2aXNpb24gMjE1NjQ0KQorKysgbGliY3BwL2ZpbGVz LmMJKEFyYmVpdHNrb3BpZSkKQEAgLTk5MSw2ICs5OTEsMTggQEAgX2NwcF9z dGFja19pbmNsdWRlIChjcHBfcmVhZGVyICpwZmlsZSwgY29uc3QgY2hhcgog ICBfY3BwX2ZpbGUgKmZpbGU7CiAgIGJvb2wgc3RhY2tlZDsKIAorICAvKiBG b3IgLWluY2x1ZGUgY29tbWFuZC1saW5lIGZsYWdzIHdlIGhhdmUgdHlwZSA9 PSBJVF9DTURMSU5FLgorICAgICBXaGVuIHRoZSBmaXJzdCAtaW5jbHVkZSBm aWxlIGlzIHByb2Nlc3NlZCB3ZSBoYXZlIHRoZSBjYXNlLCB3aGVyZQorICAg ICBwZmlsZS0+Y3VyX3Rva2VuID09IHBmaWxlLT5jdXJfcnVuLT5iYXNlLCB3 ZSBhcmUgZGlyZWN0bHkgY2FsbGVkIHVwCisgICAgIGJ5IHRoZSBmcm9udCBl bmQuICBIb3dldmVyIGluIHRoZSBjYXNlIG9mIHRoZSBzZWNvbmQgLWluY2x1 ZGUgZmlsZSwKKyAgICAgd2UgYXJlIGNhbGxlZCBmcm9tIF9jcHBfbGV4X3Rv a2VuIC0+IF9jcHBfZ2V0X2ZyZXNoX2xpbmUgLT4KKyAgICAgY3BwX3B1c2hf aW5jbHVkZSwgd2l0aCBwZmlsZS0+Y3VyX3Rva2VuICE9IHBmaWxlLT5jdXJf cnVuLT5iYXNlLAorICAgICBhbmQgcGZpbGUtPmN1cl90b2tlblstMV0uc3Jj X2xvYyBub3QgKHlldCkgaW5pdGlhbGl6ZWQuCisgICAgIEhvd2V2ZXIsIHdo ZW4gdGhlIGluY2x1ZGUgZmlsZSBjYW5ub3QgYmUgZm91bmQsIHdlIG5lZWQg c3JjX2xvYyB0bworICAgICBiZSBpbml0aWFsaXplZCB0byBzb21lIHNhZmUg dmFsdWU6IDAgbWVhbnMgVU5LTk9XTl9MT0NBVElPTi4gICovCisgIGlmICh0 eXBlID09IElUX0NNRExJTkUgJiYgcGZpbGUtPmN1cl90b2tlbiAhPSBwZmls ZS0+Y3VyX3J1bi0+YmFzZSkKKyAgICBwZmlsZS0+Y3VyX3Rva2VuWy0xXS5z cmNfbG9jID0gMDsKKwogICBkaXIgPSBzZWFyY2hfcGF0aF9oZWFkIChwZmls ZSwgZm5hbWUsIGFuZ2xlX2JyYWNrZXRzLCB0eXBlKTsKICAgaWYgKCFkaXIp CiAgICAgcmV0dXJuIGZhbHNlOwo= --_c014c58c-1d78-443e-93a3-d791d864f378_--