I think some existing code in open64 backend does similar thing. Thx for the suggestion. Sun
On Thu, Nov 25, 2010 at 4:11 AM, Gilmore, Doug <doug.gilm...@amd.com> wrote: > Arggh, I neglected to take off the extra pair of parenthesis on the > invocation of GEN_MSG. > > Anyway, you get the idea. > > Doug >> -----Original Message----- >> From: Gilmore, Doug [mailto:doug.gilm...@amd.com] >> Sent: Wednesday, November 24, 2010 12:03 PM >> To: David Coakley; Sun Chan >> Cc: open64-devel >> Subject: Re: [Open64-devel] patch for building Fortran frontend with >> FORTIFY_SOURCE checking >> >> Here is something to consider. Don't worry about compile time >> checking, >> just detect that there are problems at runtime, and avoid overflow. >> >> Change: >> char msg_str[30]; >> >> To: >> >> /* Use a large buffer here, since we really want to avoid generating >> an error message >> * when generating an error message. >> */ >> char msg_str[300]; >> #define GEN_MSG(...) do { if (snprintf(msg_str, sizeof, msg_str, >> __VA_ARGS__) >= sizeof msg_str) PRINTMSG (0, 3, Internal, 0, 3); } while(0); >> >> As an example, change: >> >> msg_str[0] = NULL_CHAR; >> strcpy(msg_str, vector_lvl_str[opt_flags.vector_lvl]); >> strcpy(&msg_str[7], "\n"); >> strcpy(&msg_str[8], task_lvl_str[opt_flags.task_lvl]); >> >> To: >> >> GEN_MSG(("%s\n%s" vector_lvl_str[opt_flags.vector_lvl], >> task_lvl_str[opt_flags.task_lvl])); >> >> and after the function add: >> >> /* GEN_MSG is context sensitive, so we remove it. >> */ >> #undef GEN_MSG >> >> >> Doug >> >> > -----Original Message----- >> > From: David Coakley [mailto:dcoak...@gmail.com] >> > Sent: Tuesday, November 23, 2010 9:57 PM >> > To: Sun Chan >> > Cc: open64-devel >> > Subject: Re: [Open64-devel] patch for building Fortran frontend with >> > FORTIFY_SOURCE checking >> > >> > I only claimed to have fixed the buffer problems detected by >> > FORTIFY_SOURCE. >> > >> > But I am willing to improve the code dealing with msg_str. Note that >> > the strings that are copied into msg_str are all constants and the >> > size is known at compile-time. Here is my proposed modification for >> > the first use of msg_str: >> > >> > OLD: >> > >> > msg_str[0] = NULL_CHAR; >> > strcpy(msg_str, vector_lvl_str[opt_flags.vector_lvl]); >> > strcpy(&msg_str[7], "\n"); >> > strcpy(&msg_str[8], task_lvl_str[opt_flags.task_lvl]); >> > >> > NEW: >> > >> > strncpy(msg_str, vector_lvl_str[opt_flags.vector_lvl], 7); >> > msg_str[7] = NEWLINE; >> > strncpy(&msg_str[8], task_lvl_str[opt_flags.task_lvl], 5); >> > msg_str[13] = NULL_CHAR; >> > >> > Optionally we could add a static assert that 13 < (sizeof(msg_str) / >> > sizeof(msg_str[0])), although the above is simple enough that the >> > FORTIFY_SOURCE checking will detect an overrun at compile-time. Thus >> > we can be sure that no overruns will happen at run-time for msg_str. >> > Is this satisfactory? >> > >> > On Mon, Nov 22, 2010 at 10:38 PM, Sun Chan <sun.c...@gmail.com> >> wrote: >> > > I don't see it that way, your fix just make the "buffer overflow" >> > > problem happen later in time, so it is not a fix that I consider >> > sound >> > > Sun >> > > >> > > On Tue, Nov 23, 2010 at 11:15 AM, David Coakley >> <dcoak...@gmail.com> >> > wrote: >> > >> Hi Sun, >> > >> >> > >> I think the changes that you are suggesting are an addition to the >> > >> changes in the submitted patch. If that is the case, then I would >> > >> rather make them as a separate commit as I have already tested the >> > >> patch and I know that it fixes the FORTIFY_SOURCE issue. >> > >> >> > >> On Mon, Nov 22, 2010 at 4:13 PM, Sun Chan <sun.c...@gmail.com> >> > wrote: >> > >>> a most economic way to do that is to use #define like SZ_STR >> > >>> then afterwards, assert that strlen(msg_str) < SZ_STR >> > >>> Sun >> > >>> >> > >>> On Tue, Nov 23, 2010 at 8:11 AM, Sun Chan <sun.c...@gmail.com> >> > wrote: >> > >>>> If we are fixing that, we might as well fix potential issues. >> E.g. >> > >>>> check for strlen and truncate, use alloca, ... or simply make it >> > twice >> > >>>> the size (although it still technically is potentially wrong) >> > >>>> Sun >> > >>>> >> > >>>> On Tue, Nov 23, 2010 at 8:04 AM, David Coakley >> > <dcoak...@gmail.com> wrote: >> > >>>>> Yes, I noticed that the msg_str buffer size seemed small. >> > However, I >> > >>>>> didn't see any immediate problems and there were no more >> > complaints >> > >>>>> from the FORTIFY_SOURCE checking so I left it as-is. >> > >>>>> >> > >>>>> On Mon, Nov 22, 2010 at 4:53 AM, Sun Chan <sun.c...@gmail.com> >> > wrote: >> > >>>>>> your msg_str could get out of bound too (not now, but some >> fixes >> > later on). >> > >>>>>> Sun >> > >>>>>> >> > >>>>>> On Mon, Nov 22, 2010 at 3:31 PM, David Coakley >> > <dcoak...@gmail.com> wrote: >> > >>>>>>> Recently I tried to build Open64 with gcc-4.5.1 and >> > FORTIFY_SOURCE >> > >>>>>>> checking turned on. The Fortran frontend would not run at >> all >> > because >> > >>>>>>> of a failing buffer overflow check that occurred during >> > command-line >> > >>>>>>> processing. Although this failure turned out to be a false >> > alarm, it >> > >>>>>>> was fairly easy to work around. And since the FORTIFY_SOURCE >> > checking >> > >>>>>>> did uncover some real problems, I thought it was worth making >> > source >> > >>>>>>> changes to work around the problem rather than turning the >> > checking >> > >>>>>>> off. >> > >>>>>>> >> > >>>>>>> The attached file msg.txt details the changes. >> > >>>>>>> >> > >>>>>>> Could a gatekeeper please review the patch? Thanks, >> > >>>>>>> >> > >>>>>>> -David Coakley / AMD Open Source Compiler Engineering >> > >>>>>>> >> > >>>>>>> ------------------------------------------------------------- >> -- >> > --------------- >> > >>>>>>> Beautiful is writing same markup. Internet Explorer 9 >> supports >> > >>>>>>> standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 >> & >> > L3. >> > >>>>>>> Spend less time writing and rewriting code and more time >> > creating great >> > >>>>>>> experiences on the web. Be a part of the beta today >> > >>>>>>> http://p.sf.net/sfu/msIE9-sfdev2dev >> > >>>>>>> _______________________________________________ >> > >>>>>>> Open64-devel mailing list >> > >>>>>>> Open64-devel@lists.sourceforge.net >> > >>>>>>> https://lists.sourceforge.net/lists/listinfo/open64-devel >> > >>>>>>> >> > >>>>>>> >> > >>>>>> >> > >>>>> >> > >>>> >> > >>> >> > >> >> > > >> > >> > --------------------------------------------------------------------- >> -- >> > ------- >> > Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! >> > Tap into the largest installed PC base & get more eyes on your game >> by >> > optimizing for Intel(R) Graphics Technology. Get started today with >> the >> > Intel(R) Software Partner Program. Five $500 cash prizes are up for >> > grabs. >> > http://p.sf.net/sfu/intelisp-dev2dev >> > _______________________________________________ >> > Open64-devel mailing list >> > Open64-devel@lists.sourceforge.net >> > https://lists.sourceforge.net/lists/listinfo/open64-devel >> >> >> >> ----------------------------------------------------------------------- >> ------- >> Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! >> Tap into the largest installed PC base & get more eyes on your game by >> optimizing for Intel(R) Graphics Technology. Get started today with the >> Intel(R) Software Partner Program. Five $500 cash prizes are up for >> grabs. >> http://p.sf.net/sfu/intelisp-dev2dev >> _______________________________________________ >> Open64-devel mailing list >> Open64-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/open64-devel > > > ------------------------------------------------------------------------------ Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel
