Re: [Qemu-devel] qom and debug

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Aug 30, 2016 at 01:37:41PM +0200, Cornelia Huck wrote:
> On Tue, 30 Aug 2016 14:15:14 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Tue, Aug 30, 2016 at 01:11:05PM +0200, Cornelia Huck wrote:
> > > On Tue, 30 Aug 2016 13:21:23 +0300
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > BTW downstreams are building with --disable-qom-cast-debug which drops
> > > > all QOM casts on data path - one way is to say we just make this the
> > > > default upstream as well. Another to say that we want to distinguish
> > > > fast path calls from slow path, this way we will be able to bring back
> > > > some of the checks.
> > > 
> > > I find CONFIG_QOM_CAST_DEBUG a bit inconsistent, btw:
> > > 
> > > - for object casts, we optimize away all checks and just return the
> > > object for !debug
> > > - for class casts, we optimize away only the caching and still keep the
> > > checking (why would we drop the caching if this can speed up things?)
> > > 
> > > We certainly want to have debug turned on during development to avoid
> > > nasty surprises later (otherwise, why even bother?), but it makes sense
> > > to turn it off for a release. (Is there an easy way to turn it off for
> > > the release, normal or stable, and keep it during the development
> > > cycle?)
> > 
> > I think the assumption was class casts are not on data path.
> > Ideally we'd keep it on for release too for non-datapath things,
> > to help improve security.
> 
> This would probably need some more fine-grained configuration.

We can start with Jason's patch :)

> For now, what about this completely untested patch that at least adds
> caching for the class->interfaces case if debug is off?

Fine with me.

> diff --git a/qom/object.c b/qom/object.c
> index 8166b7d..05f1fe4 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -696,12 +696,16 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
>                                                const char *func)
>  {
>      ObjectClass *ret;
> +    int i;
>  
>      trace_object_class_dynamic_cast_assert(class ? class->type->name : "(null)",
>                                             typename, file, line, func);
>  
> -#ifdef CONFIG_QOM_CAST_DEBUG
> -    int i;
> +#ifndef CONFIG_QOM_CAST_DEBUG
> +    if (!class || !class->interfaces) {
> +        return class;
> +    }
> +#endif
>  
>      for (i = 0; class && i < OBJECT_CLASS_CAST_CACHE; i++) {
>          if (class->class_cast_cache[i] == typename) {
> @@ -709,11 +713,6 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
>              goto out;
>          }
>      }
> -#else
> -    if (!class || !class->interfaces) {
> -        return class;
> -    }
> -#endif
>  
>      ret = object_class_dynamic_cast(class, typename);
>      if (!ret && class) {
> @@ -722,7 +721,6 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
>          abort();
>      }
>  
> -#ifdef CONFIG_QOM_CAST_DEBUG
>      if (class && ret == class) {
>          for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
>              class->class_cast_cache[i - 1] = class->class_cast_cache[i];
> @@ -730,7 +728,6 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
>          class->class_cast_cache[i - 1] = typename;
>      }
>  out:
> -#endif
>      return ret;
>  }
>