2014-06-11 12:55 GMT+04:00 Jakub Jelinek <ja...@redhat.com>: > On Wed, Jun 11, 2014 at 12:44:29PM +0400, Ilya Enkovich wrote: >> 2014-06-11 12:22 GMT+04:00 Jakub Jelinek <ja...@redhat.com>: >> > On Wed, Jun 11, 2014 at 12:14:14PM +0400, Ilya Enkovich wrote: >> >> This patch adds instrumented code support for strlen optimization. >> >> >> >> Bootstrapped and tested on linux-x86_64. >> >> >> >> Does it look OK? >> > >> > Ugh, this looks terribly ugly. So you are changing the meaning of >> > arguments >> > of every single builtin that has pointer arguments, so all spots that >> > work with those builtins have to take into account >> > gimple_call_with_bounds_p and do something different? >> > That is very error prone and maintainance nightmare. >> >> All such builtins are had to be handled anyway. It is possible to use >> new codes for instrumented builtins, but this would not change the >> handling code much. It is also possible to use something like >> gimple_call_nobnd_arg to use same arg numbers for both instrumented >> and not instrumented calls. This would still require additional >> support when we replace calls. > > That just brings the question why is all the instrumentation introduced so > early that it affects practically all GIMPLE passes? > > I mean, mudflap, also a bounded pointer implementation, could get away with > two passes and practically no changes to the rest of the compiler, asan > also has two passes and almost no changes to the rest, why does this need > to affect everything? If that is because you need to clone functions, > which you indeed need to do at IPA time, perhaps you can just clone the > functions but not add the instrumentation to the whole IL at that point, > just note you've created a bounded pointer clone, and then just shortly > before expansion in a new pass add the instrumentation, plus hook into > expansion that it is expanded properly?
Mudflap did not work with pointer bounds. It maintained a database of valid objects and checked each dereferenced pointer points to a valid object. Asan does similar thing but via shadow map. When we work with bounds it becomes more important which pointer is used for memory access and it forces early instrumentation before optimizations corrupt this information. Ilya > > Changes all over the compiler for a rarely used feature most GCC developers > will not be able to even test is going to cause substantial maintainance > issues. > > Jakub
