You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Weldon Washburn <we...@gmail.com> on 2007/03/12 18:51:45 UTC
[drlvm][runtime support] a question about an old JIRA -- H1922
Looking at just the procedure named, class_alloc_new_object(), the assert
definitely looks wrong. However, I think the patch ignores the intent of
the assert. I think the developer intended to somehow assert that the input
arg, a ptr to struct Class, is valid. In other words the original intent is
to do something like:
assert(c);
ManagedObject** hjlc = c->get_class_handle();
assert(hjlc);
ManagedObject* jlc = *hjlc;
assert(jlc != NULL);
assert(jlc->vt());
assert(jlc->vt()->clss == VM_Global_State::loader_env->JavaLangClass_Class);
assert(java_lang_Class_to_struct_Class(jlc) == clss); // else clss's
java.lang.Class had a bad "back" pointer
Note that I stole all the above code from the existing function called,
ManagedObject *struct_Class_to_java_lang_Class(Class *clss) {...}
Rather than cut/paste the body of struct_Class_to_java_lang_Class into
class_alloc_new_object(), how about simply calling
struct_Class_to_java_lang_Class()? The only unknown is if we can assume
the condition assert(!hythread_is_suspend_enabled() ); will be OK from the
body of class_alloc_new_object(). Thoughts?
--
Weldon Washburn
Intel Enterprise Solutions Software Division
Re: [drlvm][runtime support] a question about an old JIRA -- H1922
Posted by Weldon Washburn <we...@gmail.com>.
Thanks for testing this. I'll have to try committing again later. Right
now, I am focused on getting 3288 and 3289 successfully committed.
On 3/16/07, Pavel Pervov <pm...@gmail.com> wrote:
>
> No I do not. Smoke tests has just passed for me on Windows ia32.
>
> Does OPT fails due to added assertion?
>
> Pavel.
>
>
> On 3/15/07, Weldon Washburn <we...@gmail.com> wrote:
> >
> > hmm... the new patch failed smoke tests on both windows32 and linux32
> > rhel4.0 with "jitrino.OPT FAILED". I looked at the reports directory
> and
> > see that a wait for notify test has failed. Pavel, do you see the same
> > problem?
> >
> >
> >
> >
> > On 3/13/07, Pavel Pervov <pm...@gmail.com> wrote:
> > >
> > > Second line of class_alloc_new_object asserts
> > > '!hythread_is_suspend_enabled()'. So, yes, we can assume the
> condition.
> > >
> > > The line to add is something like
> > > 'assert(struct_Class_to_java_lang_Class());' to remove it in release.
> > >
> > > WBR,
> > > Pavel.
> > >
> > >
> > > On 3/12/07, Weldon Washburn <we...@gmail.com> wrote:
> > > >
> > > > Looking at just the procedure named, class_alloc_new_object(), the
> > > assert
> > > > definitely looks wrong. However, I think the patch ignores the
> intent
> > > of
> > > > the assert. I think the developer intended to somehow assert that
> the
> > > > input
> > > > arg, a ptr to struct Class, is valid. In other words the original
> > > intent
> > > > is
> > > > to do something like:
> > > >
> > > >
> > > > assert(c);
> > > >
> > > > ManagedObject** hjlc = c->get_class_handle();
> > > >
> > > > assert(hjlc);
> > > >
> > > > ManagedObject* jlc = *hjlc;
> > > >
> > > > assert(jlc != NULL);
> > > >
> > > > assert(jlc->vt());
> > > >
> > > > assert(jlc->vt()->clss ==
> > > > VM_Global_State::loader_env->JavaLangClass_Class);
> > > >
> > > > assert(java_lang_Class_to_struct_Class(jlc) == clss); // else clss's
> > > > java.lang.Class had a bad "back" pointer
> > > > Note that I stole all the above code from the existing function
> > called,
> > > >
> > > > ManagedObject *struct_Class_to_java_lang_Class(Class *clss) {...}
> > > >
> > > > Rather than cut/paste the body of struct_Class_to_java_lang_Class
> into
> > > > class_alloc_new_object(), how about simply calling
> > > > struct_Class_to_java_lang_Class()? The only unknown is if we can
> > assume
> > > > the condition assert(!hythread_is_suspend_enabled() ); will be OK
> > from
> > > > the
> > > > body of class_alloc_new_object(). Thoughts?
> > > >
> > > >
> > > > --
> > > > Weldon Washburn
> > > > Intel Enterprise Solutions Software Division
> > > >
> > >
> > >
> > >
> > > --
> > > Pavel Pervov,
> > > Intel Enterprise Solutions Software Division
> > >
> >
> >
> >
> > --
> > Weldon Washburn
> > Intel Enterprise Solutions Software Division
> >
>
>
>
> --
> Pavel Pervov,
> Intel Enterprise Solutions Software Division
>
--
Weldon Washburn
Intel Enterprise Solutions Software Division
Re: [drlvm][runtime support] a question about an old JIRA -- H1922
Posted by Pavel Pervov <pm...@gmail.com>.
No I do not. Smoke tests has just passed for me on Windows ia32.
Does OPT fails due to added assertion?
Pavel.
On 3/15/07, Weldon Washburn <we...@gmail.com> wrote:
>
> hmm... the new patch failed smoke tests on both windows32 and linux32
> rhel4.0 with "jitrino.OPT FAILED". I looked at the reports directory and
> see that a wait for notify test has failed. Pavel, do you see the same
> problem?
>
>
>
>
> On 3/13/07, Pavel Pervov <pm...@gmail.com> wrote:
> >
> > Second line of class_alloc_new_object asserts
> > '!hythread_is_suspend_enabled()'. So, yes, we can assume the condition.
> >
> > The line to add is something like
> > 'assert(struct_Class_to_java_lang_Class());' to remove it in release.
> >
> > WBR,
> > Pavel.
> >
> >
> > On 3/12/07, Weldon Washburn <we...@gmail.com> wrote:
> > >
> > > Looking at just the procedure named, class_alloc_new_object(), the
> > assert
> > > definitely looks wrong. However, I think the patch ignores the intent
> > of
> > > the assert. I think the developer intended to somehow assert that the
> > > input
> > > arg, a ptr to struct Class, is valid. In other words the original
> > intent
> > > is
> > > to do something like:
> > >
> > >
> > > assert(c);
> > >
> > > ManagedObject** hjlc = c->get_class_handle();
> > >
> > > assert(hjlc);
> > >
> > > ManagedObject* jlc = *hjlc;
> > >
> > > assert(jlc != NULL);
> > >
> > > assert(jlc->vt());
> > >
> > > assert(jlc->vt()->clss ==
> > > VM_Global_State::loader_env->JavaLangClass_Class);
> > >
> > > assert(java_lang_Class_to_struct_Class(jlc) == clss); // else clss's
> > > java.lang.Class had a bad "back" pointer
> > > Note that I stole all the above code from the existing function
> called,
> > >
> > > ManagedObject *struct_Class_to_java_lang_Class(Class *clss) {...}
> > >
> > > Rather than cut/paste the body of struct_Class_to_java_lang_Class into
> > > class_alloc_new_object(), how about simply calling
> > > struct_Class_to_java_lang_Class()? The only unknown is if we can
> assume
> > > the condition assert(!hythread_is_suspend_enabled() ); will be OK
> from
> > > the
> > > body of class_alloc_new_object(). Thoughts?
> > >
> > >
> > > --
> > > Weldon Washburn
> > > Intel Enterprise Solutions Software Division
> > >
> >
> >
> >
> > --
> > Pavel Pervov,
> > Intel Enterprise Solutions Software Division
> >
>
>
>
> --
> Weldon Washburn
> Intel Enterprise Solutions Software Division
>
--
Pavel Pervov,
Intel Enterprise Solutions Software Division
Re: [drlvm][runtime support] a question about an old JIRA -- H1922
Posted by Weldon Washburn <we...@gmail.com>.
hmm... the new patch failed smoke tests on both windows32 and linux32
rhel4.0 with "jitrino.OPT FAILED". I looked at the reports directory and
see that a wait for notify test has failed. Pavel, do you see the same
problem?
On 3/13/07, Pavel Pervov <pm...@gmail.com> wrote:
>
> Second line of class_alloc_new_object asserts
> '!hythread_is_suspend_enabled()'. So, yes, we can assume the condition.
>
> The line to add is something like
> 'assert(struct_Class_to_java_lang_Class());' to remove it in release.
>
> WBR,
> Pavel.
>
>
> On 3/12/07, Weldon Washburn <we...@gmail.com> wrote:
> >
> > Looking at just the procedure named, class_alloc_new_object(), the
> assert
> > definitely looks wrong. However, I think the patch ignores the intent
> of
> > the assert. I think the developer intended to somehow assert that the
> > input
> > arg, a ptr to struct Class, is valid. In other words the original
> intent
> > is
> > to do something like:
> >
> >
> > assert(c);
> >
> > ManagedObject** hjlc = c->get_class_handle();
> >
> > assert(hjlc);
> >
> > ManagedObject* jlc = *hjlc;
> >
> > assert(jlc != NULL);
> >
> > assert(jlc->vt());
> >
> > assert(jlc->vt()->clss ==
> > VM_Global_State::loader_env->JavaLangClass_Class);
> >
> > assert(java_lang_Class_to_struct_Class(jlc) == clss); // else clss's
> > java.lang.Class had a bad "back" pointer
> > Note that I stole all the above code from the existing function called,
> >
> > ManagedObject *struct_Class_to_java_lang_Class(Class *clss) {...}
> >
> > Rather than cut/paste the body of struct_Class_to_java_lang_Class into
> > class_alloc_new_object(), how about simply calling
> > struct_Class_to_java_lang_Class()? The only unknown is if we can assume
> > the condition assert(!hythread_is_suspend_enabled() ); will be OK from
> > the
> > body of class_alloc_new_object(). Thoughts?
> >
> >
> > --
> > Weldon Washburn
> > Intel Enterprise Solutions Software Division
> >
>
>
>
> --
> Pavel Pervov,
> Intel Enterprise Solutions Software Division
>
--
Weldon Washburn
Intel Enterprise Solutions Software Division
Re: [drlvm][runtime support] a question about an old JIRA -- H1922
Posted by Pavel Pervov <pm...@gmail.com>.
Second line of class_alloc_new_object asserts
'!hythread_is_suspend_enabled()'. So, yes, we can assume the condition.
The line to add is something like
'assert(struct_Class_to_java_lang_Class());' to remove it in release.
WBR,
Pavel.
On 3/12/07, Weldon Washburn <we...@gmail.com> wrote:
>
> Looking at just the procedure named, class_alloc_new_object(), the assert
> definitely looks wrong. However, I think the patch ignores the intent of
> the assert. I think the developer intended to somehow assert that the
> input
> arg, a ptr to struct Class, is valid. In other words the original intent
> is
> to do something like:
>
>
> assert(c);
>
> ManagedObject** hjlc = c->get_class_handle();
>
> assert(hjlc);
>
> ManagedObject* jlc = *hjlc;
>
> assert(jlc != NULL);
>
> assert(jlc->vt());
>
> assert(jlc->vt()->clss ==
> VM_Global_State::loader_env->JavaLangClass_Class);
>
> assert(java_lang_Class_to_struct_Class(jlc) == clss); // else clss's
> java.lang.Class had a bad "back" pointer
> Note that I stole all the above code from the existing function called,
>
> ManagedObject *struct_Class_to_java_lang_Class(Class *clss) {...}
>
> Rather than cut/paste the body of struct_Class_to_java_lang_Class into
> class_alloc_new_object(), how about simply calling
> struct_Class_to_java_lang_Class()? The only unknown is if we can assume
> the condition assert(!hythread_is_suspend_enabled() ); will be OK from
> the
> body of class_alloc_new_object(). Thoughts?
>
>
> --
> Weldon Washburn
> Intel Enterprise Solutions Software Division
>
--
Pavel Pervov,
Intel Enterprise Solutions Software Division