Message ID | d84f438f-1966-3a1b-efa5-cbcb7b103381@redhat.com |
---|---|
State | New |
Headers | show |
Series | [PR,middle-end/85598] make -Wprintf* pass use loop info for PHI's | expand |
On Thu, Jan 31, 2019 at 8:40 AM Aldy Hernandez <aldyh@redhat.com> wrote: > > Hi folks. > > The problem here is that Wprintf* uses the evrp_range_analyzer engine, > and doesn't understand that the x_5 != 256 conditional below should make > the RANGE [0, 255], not [0, 256]. > > <bb 3> [local count: 1063004407]: > # RANGE [0, 256] NONZERO 511 > # x_10 = PHI <0(2), x_5(3)> > snprintf (&temp, 4, "%%%02X", x_10); > # RANGE [1, 256] NONZERO 511 > x_5 = x_10 + 1; > if (x_5 != 256) > goto <bb 3>; [98.99%] > else > goto <bb 4>; [1.01%] > > This PR is handled quite easily in the ranger work, but alas there is > nothing for this release. Therefore, I'd like to dedicate as little > time as possible to this PR this time around. > > Various possible solutions are discussed in the PR. I think an easy way > is to lean on loop analysis to refine the PHI result. It turns out the > evrp engine already does this if SCEV data is available. > > As Richard mentioned in the PR, we should avoid code differences > dependent on -W* flags. I have put the loop init code in the pass > itself, but am open to moving it outside, perhaps early in the gate > function ??. > > I also took the liberty of calling loop_optimizer_init() with the > smallest subset of LOOPS_NORMAL which could still fix the problem > (LOOPS_HAVE_PREHEADERS). This avoids unnecessary changes to the CFG. > But really, I'm just guessing here. LOOPS_NORMAL would also work, > albeit creating forwarder blocks. > > Tested on x86-64 Linux. > > What do you think? As far as I understand fold_return_value is true/false independent of -W* flags (but dependent on -fprintf-return-value or so). This means that your patch avoids the risk of CFG changes dependent on -W* flags. This means you could safely use LOOPS_NORMAL (I'm not sure SCEV is happy with just pre-headers, unfortunately its exact requirements are not documented... - at least I see unconditional dereferences of loop->latch which rules out LOOPS_NO_CFG_MANIPULATION, likewise loop_preheader_edge is mentioned). That said, LOOPS_HAVE_PREHEADERS probably works well enough for both SCEV and niter analysis. Thus, the patch is OK if you adjust the comment a bit, it's perfectly safe to init loops conditional on optimization and running in the gate isn't wanted. I'm also not sure why this should go away with on-demand info -- working in the EVRP DOM walker exactly provides on-demand info (just the DOM style one relies on SCEV / niter analysis for any backedge work). Richard. > > Aldy
On 1/31/19 4:52 AM, Richard Biener wrote: > On Thu, Jan 31, 2019 at 8:40 AM Aldy Hernandez <aldyh@redhat.com> wrote: >> >> Hi folks. >> >> The problem here is that Wprintf* uses the evrp_range_analyzer engine, >> and doesn't understand that the x_5 != 256 conditional below should make >> the RANGE [0, 255], not [0, 256]. >> >> <bb 3> [local count: 1063004407]: >> # RANGE [0, 256] NONZERO 511 >> # x_10 = PHI <0(2), x_5(3)> >> snprintf (&temp, 4, "%%%02X", x_10); >> # RANGE [1, 256] NONZERO 511 >> x_5 = x_10 + 1; >> if (x_5 != 256) >> goto <bb 3>; [98.99%] >> else >> goto <bb 4>; [1.01%] >> >> This PR is handled quite easily in the ranger work, but alas there is >> nothing for this release. Therefore, I'd like to dedicate as little >> time as possible to this PR this time around. >> >> Various possible solutions are discussed in the PR. I think an easy way >> is to lean on loop analysis to refine the PHI result. It turns out the >> evrp engine already does this if SCEV data is available. >> >> As Richard mentioned in the PR, we should avoid code differences >> dependent on -W* flags. I have put the loop init code in the pass >> itself, but am open to moving it outside, perhaps early in the gate >> function ??. >> >> I also took the liberty of calling loop_optimizer_init() with the >> smallest subset of LOOPS_NORMAL which could still fix the problem >> (LOOPS_HAVE_PREHEADERS). This avoids unnecessary changes to the CFG. >> But really, I'm just guessing here. LOOPS_NORMAL would also work, >> albeit creating forwarder blocks. >> >> Tested on x86-64 Linux. >> >> What do you think? > > As far as I understand fold_return_value is true/false independent > of -W* flags (but dependent on -fprintf-return-value or so). This means > that your patch avoids the risk of CFG changes dependent on > -W* flags. This means you could safely use LOOPS_NORMAL But isn't my code inside of ::execute, which is still gated by fold_return_value AND all the -W* flags:? bool pass_sprintf_length::gate (function *) { return ((warn_format_overflow > 0 || warn_format_trunc > 0 || flag_printf_return_value) && (optimize > 0) == fold_return_value); } Thus, I think we need to move the loop initialization somewhere else ??. > (I'm not sure SCEV is happy with just pre-headers, unfortunately > its exact requirements are not documented... - at least I see > unconditional dereferences of loop->latch which rules out > LOOPS_NO_CFG_MANIPULATION, likewise loop_preheader_edge > is mentioned). > > That said, LOOPS_HAVE_PREHEADERS probably works well > enough for both SCEV and niter analysis. > > Thus, the patch is OK if you adjust the comment a bit, it's > perfectly safe to init loops conditional on optimization and > running in the gate isn't wanted. I'm also not sure why > this should go away with on-demand info -- working in > the EVRP DOM walker exactly provides on-demand info > (just the DOM style one relies on SCEV / niter analysis > for any backedge work). I believe there are backedge smarts in the ranger, at least for the simple cases. But I'll change the comment about the on-demand code curing everything, into some reminder to remove the SCEV stuff if/when we have a more precise range mechanism. I'd hate for this loop optimization requirement to survive past our need for it. Aldy
On Thu, Jan 31, 2019 at 09:30:13AM -0500, Aldy Hernandez wrote: > > > (LOOPS_HAVE_PREHEADERS). This avoids unnecessary changes to the CFG. > > > But really, I'm just guessing here. LOOPS_NORMAL would also work, > > > albeit creating forwarder blocks. > > > > > > Tested on x86-64 Linux. > > > > > > What do you think? > > > > As far as I understand fold_return_value is true/false independent > > of -W* flags (but dependent on -fprintf-return-value or so). This means > > that your patch avoids the risk of CFG changes dependent on > > -W* flags. This means you could safely use LOOPS_NORMAL > > But isn't my code inside of ::execute, which is still gated by > fold_return_value AND all the -W* flags:? > > bool > pass_sprintf_length::gate (function *) > { > return ((warn_format_overflow > 0 > || warn_format_trunc > 0 > || flag_printf_return_value) > && (optimize > 0) == fold_return_value); > } > > Thus, I think we need to move the loop initialization somewhere else ??. Then perhaps turn the gate into just return (optimize > 0) == fold_return_value; and in execute do what you're adding and guard the rest with the above condition? As -fprintf-return-value is on by default for C-family, it shouldn't change much. + adjust comment on the gate of course. Jakub
On Thu, Jan 31, 2019 at 3:45 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Jan 31, 2019 at 09:30:13AM -0500, Aldy Hernandez wrote: > > > > (LOOPS_HAVE_PREHEADERS). This avoids unnecessary changes to the CFG. > > > > But really, I'm just guessing here. LOOPS_NORMAL would also work, > > > > albeit creating forwarder blocks. > > > > > > > > Tested on x86-64 Linux. > > > > > > > > What do you think? > > > > > > As far as I understand fold_return_value is true/false independent > > > of -W* flags (but dependent on -fprintf-return-value or so). This means > > > that your patch avoids the risk of CFG changes dependent on > > > -W* flags. This means you could safely use LOOPS_NORMAL > > > > But isn't my code inside of ::execute, which is still gated by > > fold_return_value AND all the -W* flags:? > > > > bool > > pass_sprintf_length::gate (function *) > > { > > return ((warn_format_overflow > 0 > > || warn_format_trunc > 0 > > || flag_printf_return_value) > > && (optimize > 0) == fold_return_value); > > } > > > > Thus, I think we need to move the loop initialization somewhere else ??. > > Then perhaps turn the gate into just return (optimize > 0) == fold_return_value; > and in execute do what you're adding and guard the rest with the above > condition? As -fprintf-return-value is on by default for C-family, it shouldn't > change much. > + adjust comment on the gate of course. Don't you alreday have @@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun) init_target_to_host_charmap (); calculate_dominance_info (CDI_DOMINATORS); + bool optimizing_late = optimize > 0 && fold_return_value; + if (optimizing_late) + { + /* ?? We should avoid changing the CFG as much as possible. ... + loop_optimizer_init (LOOPS_HAVE_PREHEADERS); + scev_initialize (); + } so loops are only initialized if fold_return_value is true? Ah - but that's the pass parameter from params.def rather than the flag to enable the folding... So indeed, change it to include && flag_printf_return_value Richard. > Jakub
On Thu, Jan 31, 2019 at 03:56:12PM +0100, Richard Biener wrote: > Don't you alreday have > > @@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun) > init_target_to_host_charmap (); > > calculate_dominance_info (CDI_DOMINATORS); > + bool optimizing_late = optimize > 0 && fold_return_value; > + if (optimizing_late) > + { > + /* ?? We should avoid changing the CFG as much as possible. > ... > + loop_optimizer_init (LOOPS_HAVE_PREHEADERS); > + scev_initialize (); > + } > > so loops are only initialized if fold_return_value is true? Ah - but that's > the pass parameter from params.def rather than the flag to enable > the folding... So indeed, change it to include && flag_printf_return_value fold_return_value is not the same thing as flag_printf_return_value, the former is just a bool whether it is the -O0 or -O1+ version of the pass. So, optimizing_late doesn't make much sense, one can use optimize > 0 directly instead. If changing the above to && flag_printf_return_value then people will complain that they get the false positive warning with -Wall -fno-printf-return-value. Jakub
On Thu, Jan 31, 2019 at 4:02 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Jan 31, 2019 at 03:56:12PM +0100, Richard Biener wrote: > > Don't you alreday have > > > > @@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun) > > init_target_to_host_charmap (); > > > > calculate_dominance_info (CDI_DOMINATORS); > > + bool optimizing_late = optimize > 0 && fold_return_value; > > + if (optimizing_late) > > + { > > + /* ?? We should avoid changing the CFG as much as possible. > > ... > > + loop_optimizer_init (LOOPS_HAVE_PREHEADERS); > > + scev_initialize (); > > + } > > > > so loops are only initialized if fold_return_value is true? Ah - but that's > > the pass parameter from params.def rather than the flag to enable > > the folding... So indeed, change it to include && flag_printf_return_value > > fold_return_value is not the same thing as flag_printf_return_value, > the former is just a bool whether it is the -O0 or -O1+ version of the pass. > So, optimizing_late doesn't make much sense, one can use optimize > 0 > directly instead. > > If changing the above to && flag_printf_return_value then people will > complain that they get the false positive warning with -Wall > -fno-printf-return-value. Sure - too bad then. Another option would be to make SCEV / niter analysis "safe" to be called with LOOPS_AVOID_CFG_MANIPULATIONs (multiple latches, multiple entries into headers, etc.). Richard. > Jakub
On Thu, Jan 31, 2019 at 04:05:51PM +0100, Richard Biener wrote: > > On Thu, Jan 31, 2019 at 03:56:12PM +0100, Richard Biener wrote: > > > Don't you alreday have > > > > > > @@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun) > > > init_target_to_host_charmap (); > > > > > > calculate_dominance_info (CDI_DOMINATORS); > > > + bool optimizing_late = optimize > 0 && fold_return_value; > > > + if (optimizing_late) > > > + { > > > + /* ?? We should avoid changing the CFG as much as possible. > > > ... > > > + loop_optimizer_init (LOOPS_HAVE_PREHEADERS); > > > + scev_initialize (); > > > + } > > > > > > so loops are only initialized if fold_return_value is true? Ah - but that's > > > the pass parameter from params.def rather than the flag to enable > > > the folding... So indeed, change it to include && flag_printf_return_value > > > > fold_return_value is not the same thing as flag_printf_return_value, > > the former is just a bool whether it is the -O0 or -O1+ version of the pass. > > So, optimizing_late doesn't make much sense, one can use optimize > 0 > > directly instead. > > > > If changing the above to && flag_printf_return_value then people will > > complain that they get the false positive warning with -Wall > > -fno-printf-return-value. > > Sure - too bad then. Another option would be to make SCEV / niter analysis > "safe" to be called with LOOPS_AVOID_CFG_MANIPULATIONs (multiple > latches, multiple entries into headers, etc.). But then that isn't likely going to make it into GCC9. Do we care that much about -W* not affecting code generation when we have many -W* options that do affect it and aren't going to be fixed in GCC9 (and unlikely later on either - mainly the C++ caching related ones), plus it could affect code generation only if one uses -fno-printf-return-value explicitly? Jakub
On Thu, Jan 31, 2019 at 4:36 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Jan 31, 2019 at 04:05:51PM +0100, Richard Biener wrote: > > > On Thu, Jan 31, 2019 at 03:56:12PM +0100, Richard Biener wrote: > > > > Don't you alreday have > > > > > > > > @@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun) > > > > init_target_to_host_charmap (); > > > > > > > > calculate_dominance_info (CDI_DOMINATORS); > > > > + bool optimizing_late = optimize > 0 && fold_return_value; > > > > + if (optimizing_late) > > > > + { > > > > + /* ?? We should avoid changing the CFG as much as possible. > > > > ... > > > > + loop_optimizer_init (LOOPS_HAVE_PREHEADERS); > > > > + scev_initialize (); > > > > + } > > > > > > > > so loops are only initialized if fold_return_value is true? Ah - but that's > > > > the pass parameter from params.def rather than the flag to enable > > > > the folding... So indeed, change it to include && flag_printf_return_value > > > > > > fold_return_value is not the same thing as flag_printf_return_value, > > > the former is just a bool whether it is the -O0 or -O1+ version of the pass. > > > So, optimizing_late doesn't make much sense, one can use optimize > 0 > > > directly instead. > > > > > > If changing the above to && flag_printf_return_value then people will > > > complain that they get the false positive warning with -Wall > > > -fno-printf-return-value. > > > > Sure - too bad then. Another option would be to make SCEV / niter analysis > > "safe" to be called with LOOPS_AVOID_CFG_MANIPULATIONs (multiple > > latches, multiple entries into headers, etc.). > > But then that isn't likely going to make it into GCC9. > > Do we care that much about -W* not affecting code generation when we have > many -W* options that do affect it and aren't going to be fixed in GCC9 (and > unlikely later on either - mainly the C++ caching related ones), plus > it could affect code generation only if one uses -fno-printf-return-value > explicitly? We definitely shouldn't willingly add new instances. Why do we care about people getting false positives when they willingly disable -fprintf-return-value? Richard. > Jakub
On Fri, Feb 01, 2019 at 10:05:02AM +0100, Richard Biener wrote: > We definitely shouldn't willingly add new instances. Why do we care > about people getting false positives when they willingly disable > -fprintf-return-value? The -fprintf-return-value switch is about code transformations, perhaps somebody wants warnings but doesn't want the return value to be ignored or assumed to be such and such (e.g. because his C library doesn't do exactly what GCC is expecting it to do). Anyway, not that important to me, if you prefer adding "&& flag_printf_return_value" to Aldy's changes, let's go ahead with that. Jakub
On 1/31/19 9:56 AM, Richard Biener wrote: > On Thu, Jan 31, 2019 at 3:45 PM Jakub Jelinek <jakub@redhat.com> wrote: >> >> On Thu, Jan 31, 2019 at 09:30:13AM -0500, Aldy Hernandez wrote: >>>>> (LOOPS_HAVE_PREHEADERS). This avoids unnecessary changes to the CFG. >>>>> But really, I'm just guessing here. LOOPS_NORMAL would also work, >>>>> albeit creating forwarder blocks. >>>>> >>>>> Tested on x86-64 Linux. >>>>> >>>>> What do you think? >>>> >>>> As far as I understand fold_return_value is true/false independent >>>> of -W* flags (but dependent on -fprintf-return-value or so). This means >>>> that your patch avoids the risk of CFG changes dependent on >>>> -W* flags. This means you could safely use LOOPS_NORMAL >>> >>> But isn't my code inside of ::execute, which is still gated by >>> fold_return_value AND all the -W* flags:? >>> >>> bool >>> pass_sprintf_length::gate (function *) >>> { >>> return ((warn_format_overflow > 0 >>> || warn_format_trunc > 0 >>> || flag_printf_return_value) >>> && (optimize > 0) == fold_return_value); >>> } >>> >>> Thus, I think we need to move the loop initialization somewhere else ??. >> >> Then perhaps turn the gate into just return (optimize > 0) == fold_return_value; >> and in execute do what you're adding and guard the rest with the above >> condition? As -fprintf-return-value is on by default for C-family, it shouldn't >> change much. >> + adjust comment on the gate of course. > > Don't you alreday have > > @@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun) > init_target_to_host_charmap (); > > calculate_dominance_info (CDI_DOMINATORS); > + bool optimizing_late = optimize > 0 && fold_return_value; > + if (optimizing_late) > + { > + /* ?? We should avoid changing the CFG as much as possible. > ... > + loop_optimizer_init (LOOPS_HAVE_PREHEADERS); > + scev_initialize (); > + } > > so loops are only initialized if fold_return_value is true? Ah - but that's > the pass parameter from params.def rather than the flag to enable > the folding... So indeed, change it to include && flag_printf_return_value I believe the flag_printf_return_value change was the final agreed upon solution. Tested on x86-64 Linux. OK for trunk? gcc/ PR middle-end/85598 * gimple-ssa-sprintf.c (pass_sprintf_length::execute): Enable loop analysis for pass. diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index 8e6016fc42f..48580411eb9 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -65,6 +65,8 @@ along with GCC; see the file COPYING3. If not see #include "tree-ssa-propagate.h" #include "calls.h" #include "cfgloop.h" +#include "tree-scalar-evolution.h" +#include "tree-ssa-loop.h" #include "intl.h" #include "langhooks.h" @@ -4200,10 +4202,22 @@ pass_sprintf_length::execute (function *fun) init_target_to_host_charmap (); calculate_dominance_info (CDI_DOMINATORS); + bool optimizing_late = optimize > 0 && flag_printf_return_value; + if (optimizing_late) + { + loop_optimizer_init (LOOPS_NORMAL); + scev_initialize (); + } sprintf_dom_walker sprintf_dom_walker; sprintf_dom_walker.walk (ENTRY_BLOCK_PTR_FOR_FN (fun)); + if (optimizing_late) + { + scev_finalize (); + loop_optimizer_finalize (); + } + /* Clean up object size info. */ fini_object_sizes (); return 0; diff --git a/gcc/testsuite/gcc.dg/pr85598.c b/gcc/testsuite/gcc.dg/pr85598.c new file mode 100644 index 00000000000..c84b63f8084 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr85598.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wall" } */ + +typedef __SIZE_TYPE__ size_t; +extern void __chk_fail (void); +extern int snprintf (char *, size_t, const char *, ...); + +int main() +{ + char temp[100]; + unsigned int x; + char *str = temp; + for(x=0; x<256; ++x) { + snprintf(str, 4, "%%%02X", x); + } +}
On Fri, Feb 22, 2019 at 10:57 AM Aldy Hernandez <aldyh@redhat.com> wrote: > > On 1/31/19 9:56 AM, Richard Biener wrote: > > On Thu, Jan 31, 2019 at 3:45 PM Jakub Jelinek <jakub@redhat.com> wrote: > >> > >> On Thu, Jan 31, 2019 at 09:30:13AM -0500, Aldy Hernandez wrote: > >>>>> (LOOPS_HAVE_PREHEADERS). This avoids unnecessary changes to the CFG. > >>>>> But really, I'm just guessing here. LOOPS_NORMAL would also work, > >>>>> albeit creating forwarder blocks. > >>>>> > >>>>> Tested on x86-64 Linux. > >>>>> > >>>>> What do you think? > >>>> > >>>> As far as I understand fold_return_value is true/false independent > >>>> of -W* flags (but dependent on -fprintf-return-value or so). This means > >>>> that your patch avoids the risk of CFG changes dependent on > >>>> -W* flags. This means you could safely use LOOPS_NORMAL > >>> > >>> But isn't my code inside of ::execute, which is still gated by > >>> fold_return_value AND all the -W* flags:? > >>> > >>> bool > >>> pass_sprintf_length::gate (function *) > >>> { > >>> return ((warn_format_overflow > 0 > >>> || warn_format_trunc > 0 > >>> || flag_printf_return_value) > >>> && (optimize > 0) == fold_return_value); > >>> } > >>> > >>> Thus, I think we need to move the loop initialization somewhere else ??. > >> > >> Then perhaps turn the gate into just return (optimize > 0) == fold_return_value; > >> and in execute do what you're adding and guard the rest with the above > >> condition? As -fprintf-return-value is on by default for C-family, it shouldn't > >> change much. > >> + adjust comment on the gate of course. > > > > Don't you alreday have > > > > @@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun) > > init_target_to_host_charmap (); > > > > calculate_dominance_info (CDI_DOMINATORS); > > + bool optimizing_late = optimize > 0 && fold_return_value; > > + if (optimizing_late) > > + { > > + /* ?? We should avoid changing the CFG as much as possible. > > ... > > + loop_optimizer_init (LOOPS_HAVE_PREHEADERS); > > + scev_initialize (); > > + } > > > > so loops are only initialized if fold_return_value is true? Ah - but that's > > the pass parameter from params.def rather than the flag to enable > > the folding... So indeed, change it to include && flag_printf_return_value > > I believe the flag_printf_return_value change was the final agreed upon > solution. > > Tested on x86-64 Linux. > > OK for trunk? OK. Richard.
On Fri, Feb 22, 2019 at 01:24:38PM +0100, Richard Biener wrote: > > I believe the flag_printf_return_value change was the final agreed upon > > solution. > > > > Tested on x86-64 Linux. > > > > OK for trunk? > > OK. Just a nit, could you please s/optimizing_late/use_scev/ or something similar? With the flag_printf_* in there, it isn't really about optimizing late, but whether we do or don't want to use scev. Jakub
On 2/22/19 7:34 AM, Jakub Jelinek wrote: > On Fri, Feb 22, 2019 at 01:24:38PM +0100, Richard Biener wrote: >>> I believe the flag_printf_return_value change was the final agreed upon >>> solution. >>> >>> Tested on x86-64 Linux. >>> >>> OK for trunk? >> >> OK. > > Just a nit, could you please s/optimizing_late/use_scev/ or something > similar? With the flag_printf_* in there, it isn't really about optimizing > late, but whether we do or don't want to use scev. Done. Committed.
commit 189e32856dc4656931a66d4da0be81abb2eceb46 Author: Aldy Hernandez <aldyh@redhat.com> Date: Wed Jan 30 12:25:25 2019 +0100 PR middle-end/85598 * gimple-ssa-sprintf.c (pass_sprintf_length::execute): Build optimizer info when running late and optimizing. diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index 8e6016fc42f..973452fd209 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -65,6 +65,8 @@ along with GCC; see the file COPYING3. If not see #include "tree-ssa-propagate.h" #include "calls.h" #include "cfgloop.h" +#include "tree-scalar-evolution.h" +#include "tree-ssa-loop.h" #include "intl.h" #include "langhooks.h" @@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun) init_target_to_host_charmap (); calculate_dominance_info (CDI_DOMINATORS); + bool optimizing_late = optimize > 0 && fold_return_value; + if (optimizing_late) + { + /* ?? We should avoid changing the CFG as much as possible. + This is a warning pass, after all. Thus, use + LOOPS_HAVE_PREHEADERS instead of LOOPS_NORMAL. This avoids + extensive changes to the CFG, without the full hammer of + AVOID_CFG_MANIPULATIONS which would cripple SCEV. + + ?? Should we run this outside of the pass (early in the gate + function). + + FIXME: This should go away, when we have finer grained or + on-demand range information. */ + loop_optimizer_init (LOOPS_HAVE_PREHEADERS); + scev_initialize (); + } + sprintf_dom_walker sprintf_dom_walker; sprintf_dom_walker.walk (ENTRY_BLOCK_PTR_FOR_FN (fun)); + if (optimizing_late) + { + scev_finalize (); + loop_optimizer_finalize (); + } + /* Clean up object size info. */ fini_object_sizes (); return 0; diff --git a/gcc/testsuite/gcc.dg/pr85598.c b/gcc/testsuite/gcc.dg/pr85598.c new file mode 100644 index 00000000000..c84b63f8084 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr85598.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wall" } */ + +typedef __SIZE_TYPE__ size_t; +extern void __chk_fail (void); +extern int snprintf (char *, size_t, const char *, ...); + +int main() +{ + char temp[100]; + unsigned int x; + char *str = temp; + for(x=0; x<256; ++x) { + snprintf(str, 4, "%%%02X", x); + } +}