Re: [Qemu-devel] qom and debug

[Cornelia Huck]

On Tue, 30 Aug 2016 14:15:14 +0300
"Michael S. Tsirkin" <address@hidden> 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" <address@hidden> 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.

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

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;
 }