RFR: JDK-8306441: Two phase segmented heap dump [v21]

[Yi Yang]

On Tue, 1 Aug 2023 17:48:30 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> test/hotspot/jtreg/serviceability/HeapDump/IntegrityHeapDumpTest.java line 84:
>> 
>>> 82:                 .addToolArg(heapDumpFile.getAbsolutePath());
>>> 83: 
>>> 84:         ProcessBuilder processBuilder = new ProcessBuilder(launcher.getCommand());
>> 
>> Can you explain this change? This will result in the test not being run with sudo in cases where sudo is necessary. Before we get here we already checked if we need sudo (not running as root) and verified that passwordless sudo is supported. Otherwise the test is skipped and would never get here. So if sudo is required, the test will fail to attach to the debuggee. Note you might not see this issue if you have DevSecurityMode enabled. See [JDK-8313357](https://bugs.openjdk.org/browse/JDK-8313357)
>
> I see now that this is a new test, and not an SA test, so probably should never have been using SATestUtils in the first place (I'm curious as to how that ended up happening). You should also remove the import of SATestUtils.

> Can you explain this change? This will result in the test not being run with sudo in cases where sudo is necessary. Before we get here we already checked if we need sudo (not running as root) and verified that passwordless sudo is supported. Otherwise the test is skipped and would never get here. So if sudo is required, the test will fail to attach to the debuggee. Note you might not see this issue if you have DevSecurityMode enabled. See [JDK-8313357](https://bugs.openjdk.org/browse/JDK-8313357)

I followed what DuplicateArrayClassesTest.java. The reason seems that `SATestUtils.createProcessBuilder` does not touch any verification of whether passwordless sudo is supported, we need to use SATestUtils.skipIfCannotAttach explicitly.


public class SATestUtils {
    public static ProcessBuilder createProcessBuilder(JDKToolLauncher launcher) {
        List<String> cmdStringList = Arrays.asList(launcher.getCommand());
        if (needsPrivileges()) {
            cmdStringList = addPrivileges(cmdStringList);
        }
        return new ProcessBuilder(cmdStringList);
    }
    public static boolean needsPrivileges() {
        return Platform.isOSX() && !Platform.isRoot();
    }
    private static List<String> addPrivileges(List<String> cmdStringList) {
        if (!Platform.isOSX()) {
            throw new RuntimeException("Can only add privileges on OSX.");
        }

        System.out.println("Adding 'sudo -E -n' to the command.");
        List<String> outStringList = new ArrayList<String>();
        outStringList.add("sudo");
        outStringList.add("-E"); // Preserve existing environment variables.
        outStringList.add("-n"); // non-interactive. Don't prompt for password. Must be cached or not required.
        outStringList.addAll(cmdStringList);
        return outStringList;
    }


All occurrences of skipIfCannotAttach are currently under `test/hotspot/jtreg/serviceability/sa or jhsdb`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13667#discussion_r1281477905