[qom,v1,1/1] qom/object.c: Split out object and class caches.

The object-cast and class-cast caches cannot be shared because class
caching is conditional on the target type not being an interface and
object caching is unconditional. Leads to a bug when a class cast
to an interface follows an object cast to the same interface type:

FooObject = FOO(obj);
FooClass = FOO_GET_CLASS(obj);

Where TYPE_FOO is an interface. The first (object) cast will be
successful and cache the casting result (i.e. TYPE_FOO will be cached).
The second (class) cast will then check the shared cast cache
and register a hit. The issue is, when a class cast hits in the cache
it just returns a pointer cast of the input class (i.e. the concrete
class).

When casting to an interface, the cast itself must return the
interface class, not the concrete class. The implementation of class
cast caching already ensures that the returned cast result is only
a pointer cast before caching. The object cast logic however does
not have this check.

Resolve by just splitting the object and class caches.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/qom/object.h |  3 ++-
 qom/object.c         | 13 +++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

Il 28/11/2013 05:27, Peter Crosthwaite ha scritto:
> The object-cast and class-cast caches cannot be shared because class
> caching is conditional on the target type not being an interface and
> object caching is unconditional. Leads to a bug when a class cast
> to an interface follows an object cast to the same interface type:
> 
> FooObject = FOO(obj);
> FooClass = FOO_GET_CLASS(obj);
> 
> Where TYPE_FOO is an interface. The first (object) cast will be
> successful and cache the casting result (i.e. TYPE_FOO will be cached).
> The second (class) cast will then check the shared cast cache
> and register a hit. The issue is, when a class cast hits in the cache
> it just returns a pointer cast of the input class (i.e. the concrete
> class).
> 
> When casting to an interface, the cast itself must return the
> interface class, not the concrete class. The implementation of class
> cast caching already ensures that the returned cast result is only
> a pointer cast before caching. The object cast logic however does
> not have this check.
> 
> Resolve by just splitting the object and class caches.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Cc: qemu-stable@nongnu.org
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Hi

On Wed, Dec 4, 2013 at 1:40 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 28/11/2013 05:27, Peter Crosthwaite ha scritto:
>> The object-cast and class-cast caches cannot be shared because class
>> caching is conditional on the target type not being an interface and
>> object caching is unconditional. Leads to a bug when a class cast
>> to an interface follows an object cast to the same interface type:
>>
>> FooObject = FOO(obj);
>> FooClass = FOO_GET_CLASS(obj);
>>
>> Where TYPE_FOO is an interface. The first (object) cast will be
>> successful and cache the casting result (i.e. TYPE_FOO will be cached).
>> The second (class) cast will then check the shared cast cache
>> and register a hit. The issue is, when a class cast hits in the cache
>> it just returns a pointer cast of the input class (i.e. the concrete
>> class).
>>
>> When casting to an interface, the cast itself must return the
>> interface class, not the concrete class. The implementation of class
>> cast caching already ensures that the returned cast result is only
>> a pointer cast before caching. The object cast logic however does
>> not have this check.
>>
>> Resolve by just splitting the object and class caches.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Cc: qemu-stable@nongnu.org

Nathan just tested some Microblaze ethernet use cases and this is
needed to get it working. Please apply to stable as critical bugfix.

Regards,
Peter

> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>

> -----Original Message-----
> From: peter.crosthwaite@petalogix.com
> [mailto:peter.crosthwaite@petalogix.com] On Behalf Of Peter Crosthwaite
> Sent: Tuesday, December 10, 2013 4:19 PM
> To: Paolo Bonzini
> Cc: Anthony Liguori; qemu-stable@nongnu.org; qemu-devel@nongnu.org
> Developers; Andreas Färber; Nathan Rossi
> Subject: Re: [Qemu-devel] [PATCH qom v1 1/1] qom/object.c: Split out
> object and class caches.
> 
> Hi
> 
> On Wed, Dec 4, 2013 at 1:40 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Il 28/11/2013 05:27, Peter Crosthwaite ha scritto:
> >> The object-cast and class-cast caches cannot be shared because class
> >> caching is conditional on the target type not being an interface and
> >> object caching is unconditional. Leads to a bug when a class cast
> >> to an interface follows an object cast to the same interface type:
> >>
> >> FooObject = FOO(obj);
> >> FooClass = FOO_GET_CLASS(obj);
> >>
> >> Where TYPE_FOO is an interface. The first (object) cast will be
> >> successful and cache the casting result (i.e. TYPE_FOO will be cached).
> >> The second (class) cast will then check the shared cast cache
> >> and register a hit. The issue is, when a class cast hits in the cache
> >> it just returns a pointer cast of the input class (i.e. the concrete
> >> class).
> >>
> >> When casting to an interface, the cast itself must return the
> >> interface class, not the concrete class. The implementation of class
> >> cast caching already ensures that the returned cast result is only
> >> a pointer cast before caching. The object cast logic however does
> >> not have this check.
> >>
> >> Resolve by just splitting the object and class caches.
> >>
> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >
> > Cc: qemu-stable@nongnu.org
> 
> Nathan just tested some Microblaze ethernet use cases and this is
> needed to get it working. Please apply to stable as critical bugfix.
> 
> Regards,
> Peter
> 
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> >

Tested-by: Nathan Rossi <nathan.rossi@xilinx.com>

On Wed, Nov 27, 2013 at 08:27:33PM -0800, Peter Crosthwaite wrote:
> The object-cast and class-cast caches cannot be shared because class
> caching is conditional on the target type not being an interface and
> object caching is unconditional. Leads to a bug when a class cast
> to an interface follows an object cast to the same interface type:
> 
> FooObject = FOO(obj);
> FooClass = FOO_GET_CLASS(obj);
> 
> Where TYPE_FOO is an interface. The first (object) cast will be
> successful and cache the casting result (i.e. TYPE_FOO will be cached).
> The second (class) cast will then check the shared cast cache
> and register a hit. The issue is, when a class cast hits in the cache
> it just returns a pointer cast of the input class (i.e. the concrete
> class).
> 
> When casting to an interface, the cast itself must return the
> interface class, not the concrete class. The implementation of class
> cast caching already ensures that the returned cast result is only
> a pointer cast before caching. The object cast logic however does
> not have this check.
> 
> Resolve by just splitting the object and class caches.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>



> ---
> 
>  include/qom/object.h |  3 ++-
>  qom/object.c         | 13 +++++++------
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index a275db2..5f78847 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -358,7 +358,8 @@ struct ObjectClass
>      Type type;
>      GSList *interfaces;
>  
> -    const char *cast_cache[OBJECT_CLASS_CAST_CACHE];
> +    const char *object_cast_cache[OBJECT_CLASS_CAST_CACHE];
> +    const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
>  
>      ObjectUnparent *unparent;
>  };
> diff --git a/qom/object.c b/qom/object.c
> index fc19cf6..21b5a0b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -458,7 +458,7 @@ Object *object_dynamic_cast_assert(Object *obj, const char *typename,
>      Object *inst;
>  
>      for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) {
> -        if (obj->class->cast_cache[i] == typename) {
> +        if (obj->class->object_cast_cache[i] == typename) {
>              goto out;
>          }
>      }
> @@ -475,9 +475,10 @@ Object *object_dynamic_cast_assert(Object *obj, const char *typename,
>  
>      if (obj && obj == inst) {
>          for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
> -            obj->class->cast_cache[i - 1] = obj->class->cast_cache[i];
> +            obj->class->object_cast_cache[i - 1] =
> +                    obj->class->object_cast_cache[i];
>          }
> -        obj->class->cast_cache[i - 1] = typename;
> +        obj->class->object_cast_cache[i - 1] = typename;
>      }
>  
>  out:
> @@ -547,7 +548,7 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
>      int i;
>  
>      for (i = 0; class && i < OBJECT_CLASS_CAST_CACHE; i++) {
> -        if (class->cast_cache[i] == typename) {
> +        if (class->class_cast_cache[i] == typename) {
>              ret = class;
>              goto out;
>          }
> @@ -568,9 +569,9 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
>  #ifdef CONFIG_QOM_CAST_DEBUG
>      if (class && ret == class) {
>          for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
> -            class->cast_cache[i - 1] = class->cast_cache[i];
> +            class->class_cast_cache[i - 1] = class->class_cast_cache[i];
>          }
> -        class->cast_cache[i - 1] = typename;
> +        class->class_cast_cache[i - 1] = typename;
>      }
>  out:
>  #endif
> -- 
> 1.8.4.4
> 
>

Am 13.12.2013 02:36, schrieb Edgar E. Iglesias:
> On Wed, Nov 27, 2013 at 08:27:33PM -0800, Peter Crosthwaite wrote:
>> The object-cast and class-cast caches cannot be shared because class
>> caching is conditional on the target type not being an interface and
>> object caching is unconditional. Leads to a bug when a class cast
>> to an interface follows an object cast to the same interface type:
>>
>> FooObject = FOO(obj);
>> FooClass = FOO_GET_CLASS(obj);
>>
>> Where TYPE_FOO is an interface. The first (object) cast will be
>> successful and cache the casting result (i.e. TYPE_FOO will be cached).
>> The second (class) cast will then check the shared cast cache
>> and register a hit. The issue is, when a class cast hits in the cache
>> it just returns a pointer cast of the input class (i.e. the concrete
>> class).
>>
>> When casting to an interface, the cast itself must return the
>> interface class, not the concrete class. The implementation of class
>> cast caching already ensures that the returned cast result is only
>> a pointer cast before caching. The object cast logic however does
>> not have this check.
>>
>> Resolve by just splitting the object and class caches.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>

Thanks, applied to qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas