Skip to main content

What's the Diff?

Posted by mlam on August 31, 2007 at 7:22 PM PDT

In a comment for a previous blog entry, I was asked ...

Hi Mark,

Although my question does not directly have to do with VM Inspector, I have a question regarding phoneME advanced MR2. This is regarding revision 5512 - Fix for CR 6554965: crash in pthread_getattr_np calling JNI AttachCurrentThread Reviewed by Chris.

Can you disclose what's the investigation on CR 6554965? I've seem cvm segfault occasionally without any further debug messages. However, I do not have a reproducible case. I would like to understand what is CR 6554965 is about and whether that applies to the problem I have experienced with the cvm.

Thanks,

Steven

This is how I get the answer for that ...

What Files were Modified?

First of all, let's get the revision info and a list of what files were changed for this revision. I'm assuming that you have already set yourself up with subversion and can access the phoneME repository. So here goes ...

From a command prompt (I'm assuming tcsh ... pick your favorite):

> setenv phoneME https://phoneme.dev.java.net/svn/phoneme/components
>
svn log -v -r5512 $phoneME/cdc/trunk

I basically asked for the verbose (-v) log for revision 5512 (-r5512) which Steven asked about, and I asked for it on the phoneME Advanced trunk code ($phoneME/cdc/trunk). This is what I get:

------------------------------------------------------------------------
r5512 | xyzzy | 2007-05-08 12:32:32 -0700 (Tue, 08 May 2007) | 4 lines
Changed paths:
   M /components/cdc/trunk/src/linux/javavm/runtime/threads_md.c

Fix for CR 6554965: crash in pthread_getattr_np calling JNI AttachCurrentThread
Reviewed by Chris


------------------------------------------------------------------------

The revision info says that only one file was modified: src/linux/javavm/runtime/threads_md.c

What's the Diff?

From the command prompt, type:

> svn diff -r5511:5512 $phoneME/cdc/trunk/src/linux/javavm/runtime/threads_md.c

And svn tells me ...

Index: threads_md.c
===================================================================
--- threads_md.c        (revision 5511)
+++ threads_md.c        (revision 5512)
@@ -129,7 +129,8 @@
LINUXcomputeStackTop(CVMThreadID *self)
{
     void *sp = &self;
-    if (pthread_self() == initial_thread_id) {
+    pthread_t myself = pthread_self();
+    if (myself == initial_thread_id) {
         self->stackTop = initial_stack_top;
#ifdef LINUX_WATCH_STACK_GROWTH
        self->stackBottom = initial_stack_bottom;
@@ -137,13 +138,12 @@
#endif
         return CVM_TRUE;
     } else if (pthreadGetAttr != NULL) {
-        pthread_t tid = POSIX_COOKIE(self);
         pthread_attr_t attr;
         int result;
         void *base;
         size_t size;
 
-        result = (*pthreadGetAttr)(tid, &attr);
+        result = (*pthreadGetAttr)(myself, &attr);
        if (result != 0) {
            return CVM_FALSE;
        }

If you aren't familiar with reading diffs, the lines that start with a '-' are lines that are removed from the old revision. The lines that start with '+' are added in the new revision.

If you would like to see the whole file for context, you can check out the file, or just cat it like this below:

> svn cat -r5511 $phoneME/cdc/trunk/src/linux/javavm/runtime/threads_md.c > threads_md.c.5511.c
> svn cat -r5512 $phoneME/cdc/trunk/src/linux/javavm/runtime/threads_md.c > threads_md.c.5512.c

Interpreting the Diff

In this case, from the diffs, I see that the change was made in LINUXcomputeStackTop(). LINUXcomputeStackTop() is responsible for computing the address of the top of the native stack for the current thread. This value is used for stack bounds checks that are done later during VM runtime execution.

-    if (pthread_self() == initial_thread_id) {
+    pthread_t myself = pthread_self();
+    if (myself == initial_thread_id) {

The first 3 lines of diffs show basically caches the return value of pthread_self() in a variable myself whereas the old code just uses it without caching. No significant difference there.

-        pthread_t tid = POSIX_COOKIE(self);

The next line of diff removes the fetching of tid. Hmmm ....

-        result = (*pthreadGetAttr)(tid, &attr);
+        result = (*pthreadGetAttr)(myself, &attr);

The last 2 lines of diffs shows that we're calling pthreadGetAttr() with the myself instead of tid. If you look in the entire file for LINUXcomputeStackTop(), you will see that pthreadGetAttr is a function pointer to pthread_getattr_np. We use it to get the attributes of the current thread. Those attributes are then later used to get the native stack info of the current thread via pthread_attr_getstack.

Both tid and myself are some token that represents the current thread. What happened is that under some circumstances, POSIX_COOKIE(self) will not return the token for the current thread because the CVMThreadID data structure has not been initialized yet. Getting the token directly from the pthreads lib ensures that we have a valid token.

The segfault that was reported was because of the use of a tid token that was not initialized yet.

What does CR 6554965 say?

Though I can't always fulfill a request like this to provide info on change requests that are internal to Sun (for various reasons), I will indulge this one. CR 6554965 says:

The code tries to use POSIX_COOKIE(self), but it hasn't been set yet.

And that is pretty much what we deduced from taking a look at the diffs for this revision change.

Final Thoughts

In case you really really want to know what a Sun change request says (regardless of what I've shown you above), and I am not available to give you the info, you have 2 options:

  1. You can query the Sun bug database at http://bugs.sun.com/bugdatabase. But for some reason, I can't seem to query any info on CR 6554965. Maybe the database is down at this moment. Or maybe the CR is a highly confidential one ... which I know is not the case here.

  2. If you are a Java licensee, depending on your support level (I'm not too clear on how these support level things work since I've never worked in Sun's Java Licensee Engineering myself), you can ask your JLE engineer to get you the info on the CR / bug.

But if the above 2 options won't work for you, you can always do what I showed you in this blog entry, and take a look at the code changes. Believe it or not, this is what I actually did before I even bothered to look up the bug database. Often times, the revision log will give you enough data.

One more thing: the problem that got you interested in CR 6554965 in the first place was a segfault. I know that Steven said that he wasn't able to reproduce it readily, but in case you have a segfault that you can reproduce and want to know a bit about how to debug it, read this forum post that I wrote a while ago. It will give you an idea about how to get more info about a segfault. One day, I may repost that in a blog (with some additional info), but for now, check out the forum post.

Hope this has been helpful to you.

Regards,

Mark


Tags: href="http://technorati.com/tag/CVM" rel="tag">CVM href="http://technorati.com/tag/Java" rel="tag">Java href="http://technorati.com/tag/J2ME" rel="tag">J2ME href="http://technorati.com/tag/JavaME" rel="tag">JavaME href="http://technorati.com/tag/phoneME" rel="tag">phoneME href="http://technorati.com/tag/phoneME+Advanced" rel="tag">phoneME
Advanced

Related Topics >>