You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucy.apache.org by "Marvin Humphrey (JIRA)" <ji...@apache.org> on 2009/12/25 01:53:29 UTC
[jira] Created: (LUCY-89) Tighten Clownfish method invocation type
checking
Tighten Clownfish method invocation type checking
-------------------------------------------------
Key: LUCY-89
URL: https://issues.apache.org/jira/browse/LUCY-89
Project: Lucy
Issue Type: Improvement
Components: Clownfish, Core, Perl bindings
Reporter: Marvin Humphrey
Assignee: Marvin Humphrey
Clownfish method invocation is implemented using C static inline functions.
The first argument to these functions is the object itself, which up till now
has been cast to "const void*". It is better to have the first argument match
the type of the invoker, so that the C compiler will issue warnings if you try
to invoke a method on an inappropriate object, e.g. "PriQ_Pop(hash)".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (LUCY-89) Tighten Clownfish method invocation type
checking
Posted by "Marvin Humphrey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Marvin Humphrey resolved LUCY-89.
---------------------------------
Resolution: Fixed
Committed as r894043 and r894044.
> Tighten Clownfish method invocation type checking
> -------------------------------------------------
>
> Key: LUCY-89
> URL: https://issues.apache.org/jira/browse/LUCY-89
> Project: Lucy
> Issue Type: Improvement
> Components: Clownfish, Core, Perl bindings
> Reporter: Marvin Humphrey
> Assignee: Marvin Humphrey
> Attachments: fallout.diff, tighten_types.diff
>
>
> Clownfish method invocation is implemented using C static inline functions.
> The first argument to these functions is the object itself, which up till now
> has been cast to "const void*". It is better to have the first argument match
> the type of the invoker, so that the C compiler will issue warnings if you try
> to invoke a method on an inappropriate object, e.g. "PriQ_Pop(hash)".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (LUCY-89) Tighten Clownfish method invocation type
checking
Posted by "Marvin Humphrey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Marvin Humphrey updated LUCY-89:
--------------------------------
Attachment: fallout.diff
The second patch, "fallout.diff", deals with the fallout from the first patch,
addressing all the new compiler warnings.
Since the C compiler can't tell that e.g. Hash inherits from Obj,
modifications such as these are necessary:
{code:none}
- Obj_Dec_RefCount(hash);
+ Hash_Dec_RefCount(hash);
{code}
I only found one instance of an inappropriate method invocation in the Lucy
code base, where a Hash method was being invoked on a ZombieCharBuf in a test
file:
{code}
- ASSERT_TRUE(batch, Hash_Equals(&foo, Hash_Fetch(hash, (Obj*)&forty)),
+ ASSERT_TRUE(batch, ZCB_Equals(&foo, Hash_Fetch(hash, (Obj*)&forty)),
{code}
This turns out to be harmless because both Hash and ZombieCharBuf inherit the
Equals() method from Obj, so it resides at the same offset in both VTables.
However, I think it's inevitable that without the improved type checking,
future contributors to Lucy would have have been bitten at some point. Since
function pointers with completely different signatures may reside at the same
offset in different VTables, the result would be severe: most often, a memory
error.
> Tighten Clownfish method invocation type checking
> -------------------------------------------------
>
> Key: LUCY-89
> URL: https://issues.apache.org/jira/browse/LUCY-89
> Project: Lucy
> Issue Type: Improvement
> Components: Clownfish, Core, Perl bindings
> Reporter: Marvin Humphrey
> Assignee: Marvin Humphrey
> Attachments: fallout.diff, tighten_types.diff
>
>
> Clownfish method invocation is implemented using C static inline functions.
> The first argument to these functions is the object itself, which up till now
> has been cast to "const void*". It is better to have the first argument match
> the type of the invoker, so that the C compiler will issue warnings if you try
> to invoke a method on an inappropriate object, e.g. "PriQ_Pop(hash)".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (LUCY-89) Tighten Clownfish method invocation type
checking
Posted by "Marvin Humphrey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Marvin Humphrey updated LUCY-89:
--------------------------------
Attachment: tighten_types.diff
The first patch, "tighten_types.diff", changes the Clownfish code generator to
avoid the cast to "const void*".
Before:
{code:none}
static CHY_INLINE lucy_Obj*
Lucy_PriQ_Pop(const void *vself)
{
lucy_PriorityQueue *const self = (lucy_PriorityQueue*)vself;
char *const method_address = *(char**)self + Lucy_PriQ_Pop_OFFSET;
const lucy_PriQ_pop_t method = *((lucy_PriQ_pop_t*)method_address);
return method(self);
}
{code}
After:
{code:none}
static CHY_INLINE lucy_Obj*
Lucy_PriQ_Pop(const lucy_PriorityQueue *self)
{
char *const method_address = *(char**)self + Lucy_PriQ_Pop_OFFSET;
const lucy_PriQ_pop_t method = *((lucy_PriQ_pop_t*)method_address);
return method((lucy_PriorityQueue*)self);
}
{code}
The "const" cast has been left in place, even though it is wrong quite a lot
-- whenever a method modifies the object. This is mildly dangerous, because
you can e.g. invoke CB_Cat_Str(charbuf, "foo", 3) on an constant CharBuf and
the compiler will not complain. However, in order to fix that problem, we
would need to audit all methods to determine which are safe to call on "const"
objects and modify their method signatures -- a cure which is worse than the
disease.
> Tighten Clownfish method invocation type checking
> -------------------------------------------------
>
> Key: LUCY-89
> URL: https://issues.apache.org/jira/browse/LUCY-89
> Project: Lucy
> Issue Type: Improvement
> Components: Clownfish, Core, Perl bindings
> Reporter: Marvin Humphrey
> Assignee: Marvin Humphrey
> Attachments: tighten_types.diff
>
>
> Clownfish method invocation is implemented using C static inline functions.
> The first argument to these functions is the object itself, which up till now
> has been cast to "const void*". It is better to have the first argument match
> the type of the invoker, so that the C compiler will issue warnings if you try
> to invoke a method on an inappropriate object, e.g. "PriQ_Pop(hash)".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.