Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 08/01/2016 04:35 PM, Joseph Myers wrote: On Tue, 19 Jul 2016, Aldy Hernandez wrote: + // Do not warn on VLAs occurring in a loop, since VLAs are + // guaranteed to be cleaned up when they go out of scope. + // That is, there is a corresponding __builtin_stack_restore +

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On Tue, 19 Jul 2016, Aldy Hernandez wrote: > + // Do not warn on VLAs occurring in a loop, since VLAs are > + // guaranteed to be cleaned up when they go out of scope. > + // That is, there is a corresponding __builtin_stack_restore > + // at the end of the scope in which the

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/19/2016 01:47 PM, Jeff Law wrote: On 07/17/2016 09:52 AM, Manuel López-Ibáñez wrote: + if (is_vla) +gcc_assert (warn_vla_limit > 0); + if (!is_vla) +gcc_assert (warn_alloca_limit > 0); if-else ? Or perhaps: Shouldn't really matter, except perhaps in a -O0 compilation. Though

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/19/2016 01:54 PM, Jeff Law wrote: On 07/19/2016 05:03 AM, Aldy Hernandez wrote: (Same thing with alloca). There should be no warning for VLAs, and for alloca, the warning should say "use of variable-length array within a loop." The VRP dump suggests the range information is available

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 19 July 2016 at 18:47, Jeff Law wrote: > On 07/17/2016 09:52 AM, Manuel López-Ibáñez wrote: >> >> + if (is_vla) >> +gcc_assert (warn_vla_limit > 0); >> + if (!is_vla) >> +gcc_assert (warn_alloca_limit > 0); >> >> if-else ? Or perhaps: > > Shouldn't really matter,

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/19/2016 05:03 AM, Aldy Hernandez wrote: (Same thing with alloca). There should be no warning for VLAs, and for alloca, the warning should say "use of variable-length array within a loop." The VRP dump suggests the range information is available within the loop. Is the get_range_info()

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/17/2016 09:52 AM, Manuel López-Ibáñez wrote: + if (is_vla) +gcc_assert (warn_vla_limit > 0); + if (!is_vla) +gcc_assert (warn_alloca_limit > 0); if-else ? Or perhaps: Shouldn't really matter, except perhaps in a -O0 compilation. Though I think else-if makes it slightly

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/18/2016 11:14 PM, Martin Sebor wrote: How does this look? I think it's 99% there. You've addressed all of my comments so far -- thanks for that and for being so patient. I realize it would be a lot more efficient to get all the feedback (or as much of it as possible) up front.

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

How does this look? I think it's 99% there. You've addressed all of my comments so far -- thanks for that and for being so patient. I realize it would be a lot more efficient to get all the feedback (or as much of it as possible) up front. Unfortunately, some things don't get noticed until

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/16/2016 05:07 PM, Martin Sebor wrote: [Addressed all of Manu's suggestions as well.] Done. -Walloca and -Wvla warn on any use of alloca and VLAs accordingly, with or without optimization. I sorry() on the bounded cases. I think it's an improvement though I suspect we each have a

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/17/2016 11:52 AM, Manuel López-Ibáñez wrote: On 15/07/16 18:05, Aldy Hernandez wrote: +case OPT_Walloca_larger_than_: + if (!value) +inform (loc, "-Walloca-larger-than=0 is meaningless"); + break; + +case OPT_Wvla_larger_than_: + if (!value) +inform (loc,

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 15/07/16 18:05, Aldy Hernandez wrote: +case OPT_Walloca_larger_than_: + if (!value) + inform (loc, "-Walloca-larger-than=0 is meaningless"); + break; + +case OPT_Wvla_larger_than_: + if (!value) + inform (loc, "-Wvla-larger-than=0 is meaningless"); +

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

I forgot to ask, Aldy: Have you tried using LTO with the warning? I haven't managed to get it to warn even though I expected it to. The test I used is below. [...more testing...] After some more testing I discovered that LTO somehow seems to suppress this and other warnings (including the new

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

Done. -Walloca and -Wvla warn on any use of alloca and VLAs accordingly, with or without optimization. I sorry() on the bounded cases. I think it's an improvement though I suspect we each have a slightly different understanding of what the sorry message is meant to be used for. It's

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/11/2016 01:56 PM, Martin Sebor wrote: On 07/11/2016 09:40 AM, Aldy Hernandez wrote: On 07/11/2016 11:08 AM, Martin Sebor wrote: Hey, I'm all for different options. It would definitely be easier for me :). I wast trying really hard to use -Wvla= and keep the current -Wvla meaning the

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/10/2016 06:09 PM, Martin Sebor wrote: On 07/08/2016 05:48 AM, Aldy Hernandez wrote: [New thread now that I actually have a tested patch :)]. ... I have overhauled the options and added extensive documentation to invoke.texi explaining them. See the included testcases. I have tried to

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/11/2016 09:40 AM, Aldy Hernandez wrote: On 07/11/2016 11:08 AM, Martin Sebor wrote: Hey, I'm all for different options. It would definitely be easier for me :). I wast trying really hard to use -Wvla= and keep the current -Wvla meaning the same. Though I see your point about using

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/11/2016 11:08 AM, Martin Sebor wrote: Hey, I'm all for different options. It would definitely be easier for me :). I wast trying really hard to use -Wvla= and keep the current -Wvla meaning the same. Though I see your point about using -Wvla* but using different variables for

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

Hey, I'm all for different options. It would definitely be easier for me :). I wast trying really hard to use -Wvla= and keep the current -Wvla meaning the same. Though I see your point about using -Wvla* but using different variables for representing -Wvla and -Wvla=blah. The easiest thing

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/10/2016 04:09 PM, Martin Sebor wrote: On 07/08/2016 05:48 AM, Aldy Hernandez wrote: I've played with the patch a bit over the weekend and have a few comments and suggestions (I hope you won't regret encouraging me :) I like the consistency between -Walloca and -Wvla! (And despite the

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/08/2016 05:48 AM, Aldy Hernandez wrote: [New thread now that I actually have a tested patch :)]. I think detecting potentially problematic uses of alloca would be useful, especially when done in an intelligent way like in your patch (as opposed to simply diagnosing every call to the

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 11 July 2016 at 11:10, Aldy Hernandez wrote: > On 07/10/2016 07:41 PM, Manuel López-Ibáñez wrote: >>> >>> +Walloca >>> +LangEnabledBy(C ObjC C++ ObjC++) >>> +; in common.opt >>> + >>> +Walloca= >>> +LangEnabledBy(C ObjC C++ ObjC++) >>> +; in common.opt >>> + >> >> >> I'm not

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/10/2016 07:41 PM, Manuel López-Ibáñez wrote: +Walloca +LangEnabledBy(C ObjC C++ ObjC++) +; in common.opt + +Walloca= +LangEnabledBy(C ObjC C++ ObjC++) +; in common.opt + I'm not sure what you think the above means, but this is an invalid use of LangEnabledBy(). (It would be nice if the

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

+Walloca +LangEnabledBy(C ObjC C++ ObjC++) +; in common.opt + +Walloca= +LangEnabledBy(C ObjC C++ ObjC++) +; in common.opt + I'm not sure what you think the above means, but this is an invalid use of LangEnabledBy(). (It would be nice if the .awk scripts were able to catch it and give an

Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

On 07/08/2016 05:48 AM, Aldy Hernandez wrote: [New thread now that I actually have a tested patch :)]. ... I have overhauled the options and added extensive documentation to invoke.texi explaining them. See the included testcases. I have tried to add a testcase for everything the pass