On 6 mar 2014, at 11:33, Alan Bateman <[email protected]> wrote:
> On 06/03/2014 08:11, Staffan Larsen wrote: >> >> The JPLIS agent (libinstrument.so) compiles in code from libjava, >> specifically the file functions in canonicalize_md.c. This dependence has >> been there for a long time and will need to be changed to re-organize the >> JDK sources so that the code is organized by module. >> >> With this change the instrument library links the same method dynamically at >> runtime instead of statically at compile time. >> >> webrev: http://cr.openjdk.java.net/~sla/8034025/webrev.00/ >> bug: https://bugs.openjdk.java.net/browse/JDK-8034025 >> testing: the jdk_instrument target in jprt >> > It's good to fix this dependency as it was always odd to compile into this > libinstrument. > > I'm now sure about adding -ljava to LIBINSTRUMENT_LDFLAGS_SUFFIX as I would > this would need to be $(WIN_JAVA_LIB) on Windows. That is, I suspect > LDFLAGS_SUFFIX_<platform> needs to be used here. I’m not sure how these things work, but I’ve built and tested this on all platforms, so it seemed to work. I see that WIN_JAVA_LIB is already added on windows so perhaps that is why. I’ve changed the fix slightly to remove the -ljava from the windows LDFLAGS and verified that it builds and tests on all platforms. > The return from the JNI GetEnv isn't currently checked but for now I'll just > believe the comment that the thread calling Agent_OnLoad early in the startup > is already attached. As the JNIEnv parameter isn't used then it doesn't > matter of course but the temptation may be to use it with other JNI functions > that may not be usable early in the startup. This turned out to be a mistake. According to the JVMTI spec, the agent does not have access to the JNIEnv during Agent_OnLoad(). Since Canonicalize() does not use the value it didn’t matter what I sent to it. I would like to continue sending NULL to Canonicalize() as the JNIEnv since there is no way I can get a correct env in Agent_OnLoad. It would not be correct, but it would work. The other option is to add a Canonicalize_NoEnv() function to libjava that does not take a JNIEnv parameter. Thoughts on this? > As Canonicalize is not in FileSystemSupport.c then maybe it would be better > to remove the function prototype from the header file and just declare it as > an extern in appendBootClassPath where it is used. OK. webrev: http://cr.openjdk.java.net/~sla/8034025/webrev.01/ /Staffan > Otherwise thank for doing this. > > -Alan
