From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29465 invoked by alias); 14 Feb 2010 22:23:30 -0000 Received: (qmail 29457 invoked by uid 22791); 14 Feb 2010 22:23:29 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,HK_OBFDOM,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-ew0-f216.google.com (HELO mail-ew0-f216.google.com) (209.85.219.216) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 14 Feb 2010 22:23:23 +0000 Received: by ewy8 with SMTP id 8so4943514ewy.29 for ; Sun, 14 Feb 2010 14:23:20 -0800 (PST) Received: by 10.213.1.23 with SMTP id 23mr3061617ebd.98.1266186200250; Sun, 14 Feb 2010 14:23:20 -0800 (PST) Received: from ?192.168.2.99? (cpc2-cmbg8-0-0-cust61.cmbg.cable.ntl.com [82.6.108.62]) by mx.google.com with ESMTPS id 7sm7251185eyb.34.2010.02.14.14.23.18 (version=SSLv3 cipher=RC4-MD5); Sun, 14 Feb 2010 14:23:19 -0800 (PST) Message-ID: <4B787BF8.10200@gmail.com> Date: Sun, 14 Feb 2010 22:23:00 -0000 From: Dave Korn User-Agent: Thunderbird 2.0.0.17 (Windows/20080914) MIME-Version: 1.0 To: Dave Korn CC: GCC Java Subject: JVMTI events system and _jvmtiEnvironments uninitialised memory problem? [was Re: [PATCH PR42811,4.5 regression] java.lang.ExceptionInInitializerError in ecj1] References: <4B6C9B26.3010106@gmail.com> <4B6D0C67.5030500@gmail.com> In-Reply-To: <4B6D0C67.5030500@gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Mailing-List: contact java-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: java-owner@gcc.gnu.org X-SW-Source: 2010-02/txt/msg00002.txt.bz2 Good evening all, I've been trying to understand why my patch to try and fix PR42811 caused failures in the "events" test from jvmti.exp: On 06/02/2010 06:29, Dave Korn wrote: > On 05/02/2010 22:26, Dave Korn wrote: >> PR java/42811 >> * configure.ac (libgcj_spec_lgcj_override): Add undefined reference >> to arbitrary symbol in noncore library. >> (libgcj_spec_lgcj_bc_override): Likewise. I'm using configure flags: '--enable-libjava' '--enable-interpreter' '--disable-libgcj-debug' '--with-ecj-jar=/usr/share/java/ecj.jar', DW2 EH, native target i686-pc-cygwin, languages c/c++/java, graphite enabled but no LTO. Don't suppose those last two are relevant. > It caused a regression: > >> FAIL: events output > > For some reason, a couple of extra jvm event callbacks appeared: > >> $ diff -pu events.out .e.o >> --- events.out 2009-03-14 06:47:02.984375000 +0000 >> +++ .e.o 2010-02-06 06:04:50.218750000 +0000 >> @@ -4,9 +4,9 @@ created JVMTI environment #0 >> created JVMTI environment #1 >> created JVMTI environment #2 >> setting callbacks for envs >> -RequestedEvents: >> +RequestedEvents: VMInit,VMDeath, >> enable VM_INIT for env0, env1, env2 >> -RequestedEvents: VMInit, >> +RequestedEvents: VMInit,VMDeath, >> enable VM_DEATH for env1,env2 >> RequestedEvents: VMInit,VMDeath, >> enable THREAD_END for env2 Well, I've been tracking it down under the debugger (and it doesn't show up if you launch the program directly under the debugger, you have to launch it naturally and intercept it at an early breakpoint, which smells like a clue) and it looks like it's caused by use of uninitialised memory. Valgrind doesn't run on windows, which is the only platform where the sublib-enabled build currently works, so I don't have mechanical verification, but here's what appears to be the sequence of events: The testcase creates three VM environments, and tests enabling and disabling their VM event callbacks: > static void > do_enable_tests () > { > printf ("- enable tests -\n"); > JavaVM *vm = _Jv_GetJavaVM (); > jvmtiEnv *env[3]; > int i; > for (i = 0; i < 3; ++i) > { > vm->GetEnv (reinterpret_cast (&env[i]), JVMTI_VERSION_1_0); > printf ("created JVMTI environment #%d\n", i); > } > > jvmtiEventCallbacks callbacks; > memset (&callbacks, 0, sizeof (jvmtiEventCallbacks)); > > printf ("setting callbacks for envs\n"); > callbacks.VMInit = VMInitCB; > env[0]->SetEventCallbacks (&callbacks, sizeof (callbacks)); > callbacks.VMDeath = VMDeathCB; > env[1]->SetEventCallbacks (&callbacks, sizeof (callbacks)); > callbacks.ThreadEnd = ThreadEndCB; > env[2]->SetEventCallbacks (&callbacks, sizeof (callbacks)); > print_events (); At this point in the test, none of the callbacks should actually be enabled; even though some of the event callbacks have been set, the test hasn't called SetEventNotificationMode to enable them yet; but as the failure diff above shows, print_events(), which is where the "RequestedEvents:" trace comes from, thinks that some of the callbacks are indeed enabled. For reference, the contents of the environment struct look like this, from include/jvmti_md.h: > /* Contents of the jvmtiEnv; but only inside the implementation. */ > #define _CLASSPATH_JVMTIENV_CONTENTS \ > /* Event handlers registered via SetEventCallbacks */ \ > jvmtiEventCallbacks callbacks; \ > \ > /* Array of event thread for which to report event. */ \ > /* NULL means all threads. One for each callback. */ \ > jthread thread[EVENT_SLOTS]; \ > \ > /* Array of notification modes for callbacks. */ \ > /* One for each callback. */ \ > bool enabled[EVENT_SLOTS]; The vm->GetEnv() call that allocates the VMs passes through _Jv_JNI_GetEnv() and ends up in _Jv_GetJVMTIEnv() in libjava/jvmti.cc: > _Jv_JVMTIEnv * > _Jv_GetJVMTIEnv (void) > { > _Jv_JVMTIEnv *env > = (_Jv_JVMTIEnv *) _Jv_MallocUnchecked (sizeof (_Jv_JVMTIEnv)); > env->p = &_Jv_JVMTI_Interface; > struct jvmti_env_list *element > = (struct jvmti_env_list *) _Jv_MallocUnchecked (sizeof (struct jvmti_env_list)); > element->env = env; > element->next = NULL; > > _envListLock->writeLock ()->lock (); > if (_jvmtiEnvironments == NULL) > _jvmtiEnvironments = element; > else > { > struct jvmti_env_list *e; > for (e = _jvmtiEnvironments; e->next != NULL; e = e->next) > ; > e->next = element; > } > _envListLock->writeLock ()->unlock (); As far as I can see, that just mallocs a bunch of heap space, then immediately shoves it live onto the global list of environments - still full of uninitialised heap crud! Sure enough, that's also how it's returned to the calling code from do_enable_tests(), and although the calls to SetEventCallbacks() fully initialise the callbacks member, the thread[] and enabled[] arrays remain full of rubbish by the time we get to this loop in libjava/jvmti.cc#check_enabled_event(): > if (_jvmtiEnvironments != NULL) > { > _envListLock->readLock ()->lock (); > struct jvmti_env_list *e; > FOREACH_ENVIRONMENT (e) > { > char *addr > = reinterpret_cast (&e->env->callbacks) + offset; > void **callback = reinterpret_cast (addr); > if (e->env->enabled[index] && *callback != NULL) > { > *enabled = true; > _envListLock->readLock ()->unlock (); > return; > } > } > > _envListLock->readLock ()->unlock (); > } and as a result it believes various events are enabled even though we haven't yet called SetEventNotificationMode() to turn them on. I set a breakpoint in _Jv_GetJVMTIEnv(), just after the call to _Jv_MallocUnchecked(), and manually called memset to zero out the freshly-returned space, and it fixed the bug, so I'm just bootstrapping and testing a patch to add an explicit memset call there (will post separately), but I just wondered if there is supposed to be an assumption that the _Jv_Malloc* routines return zeroed memory, or if possibly there should be a _Jv_Calloc interface added? Or have I somehow missed some place that should have zeroed out the thread[] and enabled[] arrays? BTW, assuming this proposed patch passes testing and is agreed to be the right thing to do, I'll be resurrecting my proposed fix for PR42811(*). Heads up, and maybe a little pre-emptive "ping!" in advance :) cheers, DaveK -- (*) - http://gcc.gnu.org/ml/gcc-patches/2010-02/msg00261.html