On Fri, Jul 16, 2021 at 12:42 AM Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > A number of access warnings as well as their supporting > infrastructure (compute_objsize et al.) are implemented in > builtins.{c,h} where they (mostly) operate on trees and run > just before RTL expansion. > > This setup may have made sense initially when the warnings were > very simple and didn't perform any CFG analysis, but it's becoming > a liability. The code has grown both in size and in complexity, > might need to examine the CFG to improve detection, and in some > cases might achieve a better S/R ratio if run earlier. Running > the warning code on trees is also slower because it doesn't > benefit from the SSA_NAME caching provided by the pointer_query > class. Finally, having the code there is also an impediment to > maintainability as warnings and builtin expansion are unrelated > to each other and contributors to one area shouldn't need to wade > through unrelated code (similar for patch reviewers). > > The attached change introduces a new warning pass and a couple of > new source and headers and, as the first step, moves the warning > code from builtins.{c,h} there. To keep the initial changes as > simple as possible the pass only runs a subset of existing > warnings: -Wfree-nonheap-object, -Wmismatched-dealloc, and > -Wmismatched-new-delete. The others (-Wstringop-overflow and > -Wstringop-overread) still run on the tree representation and > are still invoked from builtins.c or elsewhere. > > The changes have no functional impact either on codegen or on > warnings. I tested them on x86_64-linux. > > As the next step I plan to change the -Wstringop-overflow and > -Wstringop-overread code to run on the GIMPLE IL in the new pass > instead of on trees in builtins.c.
That's the maybe_warn_rdwr_sizes thing? + gimple *stmt = gsi_stmt (si); + if (!is_gimple_call (stmt)) + continue; + + check (as_a <gcall *>(stmt)); if (gcall *call = dyn_cast <gcall *> (gsi_stmt (si))) check (call); might be more C++-ish. The patch looks OK - I skimmed it as mostly moving things around plus adding a new pass. Thanks, Richard. > Martin > > PS The builtins.c diff produced by git diff was much bigger than > the changes justify. It seems that the code removal somehow > confused it. To make review easy I replaced it with a plain > unified diff of builtins.c that doesn't suffer from the problem.