David, Thx for your patience. pls checkin Sun On Tue, Nov 30, 2010 at 3:07 AM, David Coakley <dcoak...@gmail.com> wrote: > Thanks, Doug. I had already implemented something else by the time I > saw your suggestion so I did not use it in the attached patch. > Instead I used some macros that check the size of the destination > buffer at compilation time. > > The handling of msg_str is the only difference in the revised patch. > I also added more explanation to the proposed log message in msg.txt. > > On Wed, Nov 24, 2010 at 1:49 PM, Sun Chan <sun.c...@gmail.com> wrote: >> 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
