[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

[JF Bastien via Phabricator via cfe-commits]

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