Re: __builtin_dynamic_object_size

[Richard Smith <richard at metafoo dot co dot uk>]

[Please CC me; I'm not subscribed]

On Thu, 24 Jan 2019 11:59 at Richard Biener wrote:
> On Wed, Jan 23, 2019 at 12:33 PM Jakub Jelinek <jakub@redhat.com> wrote:
> > On Wed, Jan 23, 2019 at 10:40:43AM +0000, Jonathan Wakely wrote:
> > > There's a patch to add __builtin_dynamic_object_size to clang:
> > > https://reviews.llvm.org/D56760
> > >
> > > It was suggested that this could be done via a new flag bit for
> > > __builtin_object_size, but only if GCC would support that too
> > > (otherwise it would be done as a separate builtin).
> > >
> > > Is there any interest in adding that as an option to __builtin_object_size?
> > >
> > > I know Jakub is concerned about arbitrarily complex expressions, when
> > > __builtin_object_size is supposed to always be efficient and always
> > > evaluate at compile time (which would imply the dynamic behaviour
> > > should be a separate builtin, if it exists at all).
> >
> > 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.
>
> I wouldn't overload _bos but only use a new builtin.

Clang provides another useful attribute in this space:
__attribute__((pass_object_size(N))), when applied to a pointer
parameter to a function, causes the compiler to evaluate
__builtin_object_size in the caller, and pass the result to the callee
(see https://clang.llvm.org/docs/AttributeReference.html#pass-object-size).
This allows many of the _FORTIFY_SOURCE checks to be implemented
without forcibly inlining. One reason we'd like to see this added as a
flag bit rather than as a separate builtin is that it would then also
naturally extend to the pass_object_size attribute. But we certainly
don't want to conflict with any potential future GCC extension.

If the GCC devs aren't interested in adding similar functionality,
could we reserve a bit for this behavior? Eg, if GCC would promise to
never use the values 4-7 for any other purpose, I expect that'd work
for us, but I'd certainly understand if you don't want to guarantee
that.

> > 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.
>
> Which means I'd like to see a thorough specification of the builtin.

How far you go is a quality of implementation issue, just like with
all the existing __builtin_object_size modes -- you're always
permitted to return a conservatively-correct answer, for any mode.
(Ignoring the new flag bit would be a correct implementation, for
example.) But the intent is for this to be used in cases where users
want the security benefits more than they want a modicum of
performance.

> If it is allowed to return "failure" in any event then of what use is
> the builtin in practice?

The same question can be asked of the existing builtin, which has the
same property that it is allowed to return "failure" in any event.
That's already useful in practice, and it's hopefully also apparent
that catching just the easy non-constant cases (eg, a parameter passed
to malloc also gets used as the object size of the returned pointer)
would also be useful in practice.