Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On December 3, 2020 5:07:28 PM GMT+01:00, Qing Zhao <qing.z...@oracle.com> > wrote: >> >> >>> On Dec 3, 2020, at 2:45 AM, Richard Biener >><richard.guent...@gmail.com> wrote: >>> >>> On Wed, Dec 2, 2020 at 4:36 PM Qing Zhao <qing.z...@oracle.com >><mailto:qing.z...@oracle.com>> wrote: >>>> >>>> >>>> >>>> On Dec 2, 2020, at 2:45 AM, Richard Biener >><richard.guent...@gmail.com> wrote: >>>> >>>> On Tue, Dec 1, 2020 at 8:49 PM Qing Zhao <qing.z...@oracle.com> >>wrote: >>>> >>>> >>>> Hi, Richard, >>>> >>>> Could you please comment on the following approach: >>>> >>>> Instead of adding the zero-initializer quite late at the pass >>“pass_expand”, we can add it as early as during gimplification. >>>> However, we will mark these new added zero-initializers as >>“artificial”. And passing this “artificial” information to >>>> “pass_early_warn_uninitialized” and “pass_late_warn_uninitialized”, >>in these two uninitialized variable analysis passes, >>>> (i.e., in tree-sea-uninit.c) We will update the checking on >>“ssa_undefined_value_p” to consider “artificial” zero-initializers. >>>> (i.e, if the def_stmt is marked with “artificial”, then it’s a >>undefined value). >>>> >>>> With such approach, we should be able to address all those >>conflicts. >>>> >>>> Do you see any obvious issue with this approach? >>>> >>>> >>>> Yes, DSE will happily elide an explicit zero-init following the >>>> artificial one leading to false uninit diagnostics. >>>> >>>> >>>> Indeed. This is a big issue. And other optimizations might also be >>impacted by the new zero-init, resulting changed behavior >>>> of uninitialized analysis in the later stage. >>> >>> I don't see how the issue can be resolved, you can't get both, uninit >>> warnings and no uninitialized memory. >>> People can compile twice, once without -fzero-init to get uninit >>> warnings and once with -fzero-init to get >>> the extra "security". >> >>So, for GCC, you think that it’s okay to get rid of the following >>requirement: >> >>C. The implementation needs to keep the current static warning on >>uninitialized >>variables untouched in order to avoid "forking the language”. >> >>Then, we can add explanation in the user documentation of the new >>-fzero-init and also >>that of the -Wuninitialized to inform users that -fzero-init will >>change the behavior of -Wuninitialized. >>In order to get the warnings, -fzero-init should not be added at the >>same time? >> >>With this requirement being eliminated, implementation will be much >>easier. >> >>We can add the new initialization during simplification phase. Then >>this new option will work >>for all languages. Is this reasonable? > > I think that's reasonable indeed. Eventually doing the init after the early > uninit pass is possible as well.
Sorry to be awkward, but I kind-of disagree. IIRC, clang was able to give uninit warnings while implementing the initialisation as expected, so I think this is a GCC restriction rather than a fundamental incompatibility. I don't think it's reasonable to expect people to read the documentation of -ffoo for Clang and separately read the documentation of -ffoo for GCC. They'll at best read the documentation for one and (rightly) expect the other compiler to behave in a compatible way. I'm also not sure people would build twice in practice. I remember the issue of forking the language was discussed at length on the Clang dev list at the time (but I haven't gone back and re-read the thread, so I'm relying on memory here). Not forking the language was an important goal/requirement of the option and I don't think we should drop it when implementing the option in GCC. IMO, if we want to define a dialect of C/C++ in which uninitialised uses are always well defined rather than UB, we should do that as a separate option. If we're implementing the Clang options, we should continue to treat uninitialised uses as UB that triggers the same warnings as if the option wasn't passed. So TBH I'd rather not add the option until it can be implemented in a way that is compatible with Clang. Thanks, Richard
