Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8263670: pmap and pstack in jhsdb do not work on debug server #3027

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -443,16 +443,12 @@ private void setupVM() {
db.getJShortType().getSize());
}

if (!isServer) {
// Do not initialize the VM on the server (unnecessary, since it's
// instantiated on the client)
try {
VM.initialize(db, debugger);
} catch (DebuggerException e) {
throw (e);
} catch (Exception e) {
throw new DebuggerException(e);
}
try {
VM.initialize(db, debugger);
} catch (DebuggerException e) {
throw (e);
} catch (Exception e) {
throw new DebuggerException(e);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2009, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -25,6 +25,7 @@
package sun.jvm.hotspot.debugger.remote;

import java.rmi.*;
import java.util.*;

import sun.jvm.hotspot.debugger.*;

Expand Down Expand Up @@ -76,4 +77,8 @@ public boolean areThreadsEqual(long addrOrId1, boolean isAddress1,
long addrOrId2, boolean isAddress2) throws RemoteException;
public int getThreadHashCode(long addrOrId, boolean isAddress) throws RemoteException;
public long[] getThreadIntegerRegisterSet(long addrOrId, boolean isAddress) throws RemoteException;

public default String execCommandOnServer(String command, Map<String, Object> options) throws RemoteException {
throw new DebuggerException("Command execution is not supported");
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -415,4 +415,12 @@ public ReadResult readBytesFromProcess(long address, long numBytes) {
public void writeBytesToProcess(long a, long b, byte[] c) {
throw new DebuggerException("Unimplemented!");
}

public String execCommandOnServer(String command, Map<String, Object> options) {
try {
return remoteDebugger.execCommandOnServer(command, options);
} catch (RemoteException e) {
throw new DebuggerException(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -24,18 +24,21 @@

package sun.jvm.hotspot.debugger.remote;

import java.io.*;
import java.rmi.*;
import java.rmi.server.*;
import java.util.*;

import sun.jvm.hotspot.debugger.*;
import sun.jvm.hotspot.tools.*;

/** The implementation of the RemoteDebugger interface. This
delegates to a local debugger */

public class RemoteDebuggerServer extends UnicastRemoteObject
implements RemoteDebugger {

private transient Debugger debugger;
private transient JVMDebugger debugger;

/** This is the required no-arg constructor */
public RemoteDebuggerServer() throws RemoteException {
Expand All @@ -44,14 +47,14 @@ public RemoteDebuggerServer() throws RemoteException {

/** This is the constructor used on the machine where the debuggee
process lies that accepts an RMI connector port */
public RemoteDebuggerServer(Debugger debugger, int port) throws RemoteException {
public RemoteDebuggerServer(JVMDebugger debugger, int port) throws RemoteException {
super(port);
this.debugger = debugger;
}

/** This is the constructor used on the machine where the debuggee
process lies */
public RemoteDebuggerServer(Debugger debugger) throws RemoteException {
public RemoteDebuggerServer(JVMDebugger debugger) throws RemoteException {
this(debugger, 0);
}

Expand Down Expand Up @@ -175,4 +178,23 @@ private ThreadProxy getThreadProxy(long addrOrId, boolean isAddress) throws Debu
return debugger.getThreadForThreadId(addrOrId);
}
}

@Override
public String execCommandOnServer(String command, Map<String, Object> options) throws RemoteException {
ByteArrayOutputStream bout = new ByteArrayOutputStream();
try (var out = new PrintStream(bout)) {
if (command.equals("pmap")) {
(new PMap(debugger)).run(out, debugger);
} else if (command.equals("pstack")) {
PStack pstack = new PStack(debugger);
pstack.setVerbose(false);
pstack.setConcurrentLocks((boolean)options.get("concurrentLocks"));
pstack.run(out, debugger);
} else {
throw new DebuggerException(command + " is not supported in this debugger");
}
}

return bout.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import sun.jvm.hotspot.*;
import sun.jvm.hotspot.debugger.*;
import sun.jvm.hotspot.debugger.cdbg.*;
import sun.jvm.hotspot.debugger.remote.*;
import sun.jvm.hotspot.utilities.PlatformInfo;

public class PMap extends Tool {
Expand Down Expand Up @@ -77,7 +78,7 @@ public void run(PrintStream out, Debugger dbg) {
}
} else {
if (getDebugeeType() == DEBUGEE_REMOTE) {
out.println("remote configuration is not yet implemented");
out.print(((RemoteDebuggerClient)dbg).execCommandOnServer("pmap", null));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Indent is incorrect now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I fixed it in new commit.

} else {
out.println("not yet implemented (debugger does not support CDebugger)!");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import sun.jvm.hotspot.interpreter.*;
import sun.jvm.hotspot.debugger.*;
import sun.jvm.hotspot.debugger.cdbg.*;
import sun.jvm.hotspot.debugger.remote.*;
import sun.jvm.hotspot.oops.*;
import sun.jvm.hotspot.runtime.*;
import sun.jvm.hotspot.utilities.PlatformInfo;
Expand Down Expand Up @@ -193,7 +194,7 @@ public void run(PrintStream out, Debugger dbg) {
} // for threads
} else {
if (getDebugeeType() == DEBUGEE_REMOTE) {
out.println("remote configuration is not yet implemented");
out.print(((RemoteDebuggerClient)dbg).execCommandOnServer("pstack", Map.of("concurrentLocks", concurrentLocks)));
} else {
out.println("not yet implemented (debugger does not support CDebugger)!");
}
Expand Down Expand Up @@ -285,4 +286,12 @@ private String[] getJavaNames(ThreadProxy th, Address fp) {
System.arraycopy(names.toArray(), 0, res, 0, res.length);
return res;
}

public void setVerbose(boolean verbose) {
this.verbose = verbose;
}

public void setConcurrentLocks(boolean concurrentLocks) {
this.concurrentLocks = concurrentLocks;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021 NTT DATA.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

import jdk.test.lib.JDKToolLauncher;
import jdk.test.lib.apps.LingeredApp;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.SA.SATestUtils;

import jtreg.SkippedException;

/**
* @test
* @bug 8263670
* @requires vm.hasSA
* @requires os.family != "windows"
Copy link
Contributor

@plummercj plummercj Mar 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You exclude windows but you check for ".dll" below.

* @library /test/lib
* @run main/othervm PmapOnDebugdTest
*/

public class PmapOnDebugdTest {

public static void main(String[] args) throws Exception {
SATestUtils.skipIfCannotAttach(); // throws SkippedException if attach not expected to work.

if (SATestUtils.needsPrivileges()) {
// This tests has issues if you try adding privileges on OSX. The debugd process cannot
// be killed if you do this (because it is a root process and the test is not), so the destroy()
// call fails to do anything, and then waitFor() will time out. If you try to manually kill it with
// a "sudo kill" command, that seems to work, but then leaves the LingeredApp it was
// attached to in a stuck state for some unknown reason, causing the stopApp() call
// to timeout. For that reason we don't run this test when privileges are needed. Note
// it does appear to run fine as root, so we still allow it to run on OSX when privileges
// are not required.
throw new SkippedException("Cannot run this test on OSX if adding privileges is required.");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pmap and pstack are not even supported on OSX with a live process. The automated github testing did not catch this because it passed due to the above check:

TEST RESULT: Passed. Skipped: jtreg.SkippedException: Cannot run this test on OSX if adding privileges is required.

If it passed the check, it would have failed below to find a .dylib. You could change the code below to something like what is in ClhsdbPmap.java:

            if (!withCore && Platform.isOSX()) {
                expStrMap.put("pmap", List.of("Not available for Mac OS X processes"));
            } else {
                expStrMap.put("pmap", List.of("jvm", "java", "jli", "jimage"));
            }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this code and the giant comment block will now be in 3 places. Maybe it should be moved to SATestUtils. Maybe something like validateSADebugDPrivileges().


LingeredApp theApp = null;
DebugdUtils debugd = null;
try {
theApp = LingeredApp.startApp();
System.out.println("Started LingeredApp with pid " + theApp.getPid());
debugd = new DebugdUtils(null);
debugd.attach(theApp.getPid());

JDKToolLauncher jhsdbLauncher = JDKToolLauncher.createUsingTestJDK("jhsdb");
jhsdbLauncher.addToolArg("jmap");
jhsdbLauncher.addToolArg("--connect");
jhsdbLauncher.addToolArg("localhost");

Process jhsdb = (SATestUtils.createProcessBuilder(jhsdbLauncher)).start();
OutputAnalyzer out = new OutputAnalyzer(jhsdb);

jhsdb.waitFor();
System.out.println(out.getStdout());
System.err.println(out.getStderr());

out.stderrShouldBeEmptyIgnoreDeprecatedWarnings();
out.shouldMatch("^0x[0-9a-f]+.+jvm\\.(dll|dylib|so)$"); // Find libjvm from output
out.shouldHaveExitValue(0);

// This will detect most SA failures, including during the attach.
out.shouldNotMatch("^sun.jvm.hotspot.debugger.DebuggerException:.*$");
} catch (SkippedException se) {
throw se;
} catch (Exception ex) {
throw new RuntimeException("Test ERROR " + ex, ex);
} finally {
if (debugd != null) {
debugd.detach();
}
LingeredApp.stopApp(theApp);
}
System.out.println("Test PASSED");
}
}