jfb added a comment. I pointed out this review to Jonathan Wakely, who posted it to the GCC list <https://gcc.gnu.org/ml/gcc/2019-01/msg00188.html>. Jakub replied with:
> The current modes (0-3) certainly must not be changed and must return a > constant, that is what huge amounts of code in the wild relies on. > > The reason to choose constants only was the desire to make `_FORTIFY_SOURCE` > cheap at runtime. For the dynamically computed expressions, the question > is how far it should go, how complex expressions it wants to build and how > much code and runtime can be spent on computing that. > > The rationale for `__builtin_dynamic_object_size` lists only very simple > cases, where the builtin is just called on result of malloc, so that is > indeed easy, the argument is already evaluated before the malloc call, so > you can just save it in a temporary and use later. Slightly more complex > is calloc, where you need to multiply two numbers (do you handle overflow > somehow, or not?). But in real world, it can be arbitrarily more complex, > there can be pointer arithmetics with constant or variable offsets, > there can be conditional adjustments of pointers or PHIs with multiple > "dynamic" expressions for the sizes (shall the dynamic expression evaluate > as max over expressions for different phi arguments (that is essentially > what is done for the constant `__builtin_object_size`, but for dynamic > expressions max needs to be computed at runtime, or shall it reconstruct > the conditional or remember it and compute `whatever ? val1 : val2`), > loops which adjust pointers, etc. and all that can be done many times in > between where the objects are allocated and where the builtin is used. @rsmith, what do you think and how do you want to proceed? We think something like what Erik implemented will catch things `_FORTIFY_SOURCE` currently cannot. We agree there are valid code generation complexity concerns, yet it seems like having a different spelling for the builtin helps mitigate those concerns. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56760/new/ https://reviews.llvm.org/D56760 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
