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
