On Wed, 28 Aug 2024 23:30:33 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> On Windows SA agent gets a class vtable from symbols, exported from jvm.dll
>> (it exports symbols like "??_7" + type + "@@6B@").
>> But symbol lookup function first requests WinDbg about the symbol.
>> Sometimes WinDbg routine IDebugSymbols::GetOffsetByName() returns offset for
>> both class and class pointer types. Returned offsets correspond to symbols
>> like "jvm!class_name::`vftable'".
>> The behavior is intermittent, I was not able to find what is the reason.
>> The fix adds workaround for the case - if GetOffsetByName succeeded, we
>> check if corresponding symbol contains requested one.
>> So it returns expected offset for non-vtable symbols like
>> "MaxJNILocalCapacity" (GetOffsetByName returns offset for
>> "jvm!MaxJNILocalCapacity"), but returns 0 for vtlb lookup.
>>
>> Additionally added check for results of
>> IDebugSymbols::SetImagePath/SetSymbolPath
>>
>> Testing: tier1,tier2,hs-tier5-svc
>
> Alex Menkov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> removed extra space
Changes look good.
I can't find any other place where we directly lookup a global symbol like
this. I did find this other reference to the lookup() API:
public String findSymbol(String symbol) {
Address addr = lookup(null, symbol);
if (addr == null && getOS().equals("win32")) {
// On win32 symbols are prefixed with the dll name. Do the user
// a favor and see if this is a symbol in jvm.dll or java.dll.
addr = lookup(null, "jvm!" + symbol);
if (addr == null) {
addr = lookup(null, "java!" + symbol);
}
}
...
So it seems isSharingEnabled() could be using this instead, although that is
not solving any of the problems we are seeing with non-native lookups. clhsdb
findsym uses this findSymbol() API.
new Command("findsym", "findsym name", false) {
public void doit(Tokens t) {
if (t.countTokens() != 1) {
usage();
} else {
String result =
VM.getVM().getDebugger().findSymbol(t.nextToken());
out.println(result == null ? "Symbol not found" : result);
}
}
},
We have one test that does a findsym of MaxJNILocalCapacity
(ClhsdbFindPC.java). I'm guessing this would fail if native lookups were
disabled. Note findsym was added less than 3 years ago.
Another thing I noticed is that isSharingEnabled() used to be the following:
public boolean isSharingEnabled() {
if (sharingEnabled == null) {
Flag flag = getCommandLineFlag("UseSharedSpaces");
sharingEnabled = (flag == null)? Boolean.FALSE :
(flag.getBool()? Boolean.TRUE: Boolean.FALSE);
}
return sharingEnabled.booleanValue();
}
So it used to use getCommandLineFlag(). It was modified to use lookup() as part
of https://bugs.openjdk.org/browse/JDK-8277481, which changed UseSharedSpaces
from a command line flag to just a regular global. This change was done 3 years
ago. getCommandLineFlag() seems to get the flag from VMStructs, which
references the following:
JVMFlag* JVMFlag::flags = flagTable;
size_t JVMFlag::numFlags = (sizeof(flagTable) / sizeof(JVMFlag));
So there is no dll or windbg symbol lookup here, just access to the VMStructs
database.
It seems then that theoretically the dll lookup should never be needed, but due
to the bug this PR is fixing, it is needed as a fallback when windbg lookup
fails to do the right thing. I wonder if there are also other types of symbols
we lookup that would only be found with the dll lookup.
-------------
Marked as reviewed by cjplummer (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/20684#pullrequestreview-2270180511
PR Comment: https://git.openjdk.org/jdk/pull/20684#issuecomment-2318965634
