Skip to content

Commit

Permalink
8318682: SA decoding of scalar replaced objects is broken
Browse files Browse the repository at this point in the history
Reviewed-by: cjplummer, cslucas
  • Loading branch information
Tom Rodriguez committed Apr 30, 2024
1 parent a863ef5 commit b96b38c
Show file tree
Hide file tree
Showing 13 changed files with 423 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2024, 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 @@ -91,6 +91,9 @@ private Disassembler(long startPc, byte[] code) {
.get();

String arch = targetSysProps.getProperty("os.arch");
if (arch.equals("x86_64")) {
arch = "amd64";
}
String libname = "hsdis-" + arch + ext;

List<String> libs = List.of(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2024, 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 @@ -74,6 +74,22 @@ ScopeValue readObjectValue() {
return result;
}

ScopeValue readObjectMergeValue() {
int id = readInt();
if (Assert.ASSERTS_ENABLED) {
Assert.that(objectPool != null, "object pool does not exist");
for (Iterator itr = objectPool.iterator(); itr.hasNext();) {
ObjectValue ov = (ObjectValue) itr.next();
Assert.that(ov.id() != id, "should not be read twice");
}
}
ObjectValue result = new ObjectMergeValue(id);
// Cache the object since an object field could reference it.
objectPool.add(result);
result.readObject(this);
return result;
}

ScopeValue getCachedObject() {
int id = readInt();
Assert.that(objectPool != null, "object pool does not exist");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2024, 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
* 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.
*
*/

package sun.jvm.hotspot.code;

import java.io.PrintStream;

/** A placeholder value that has no concrete meaning other than helping constructing
* other values.
*/
public class MarkerValue extends ScopeValue {
public boolean isMarker() { return true; }

public void printOn(PrintStream tty) {
tty.print("marker");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public int immutableDataSize() {
public OopHandle getOopAt(int index) {
if (index == 0) return null;
if (Assert.ASSERTS_ENABLED) {
Assert.that(index > 0 && index <= getOopsLength(), "must be a valid non-zero index");
Assert.that(index > 0 && index <= getOopsLength(), "must be a valid non-zero index: " + index);
}
return oopsBegin().getOopHandleAt((index - 1) * VM.getVM().getOopSize());
}
Expand All @@ -208,7 +208,7 @@ public OopHandle getOopAt(int index) {
public Address getMetadataAt(int index) {
if (index == 0) return null;
if (Assert.ASSERTS_ENABLED) {
Assert.that(index > 0 && index <= getMetadataLength(), "must be a valid non-zero index");
Assert.that(index > 0 && index <= getMetadataLength(), "must be a valid non-zero index: " + index);
}
return metadataBegin().getAddressAt((index - 1) * VM.getVM().getOopSize());
}
Expand Down Expand Up @@ -298,6 +298,19 @@ public PCDesc getPCDescAt(Address pc) {
return null;
}

/**
* Attempt to decode all the debug info in this nmethod. This is intended purely for testing.
*/
public void decodeAllScopeDescs() {
for (Address p = scopesPCsBegin(); p.lessThan(scopesPCsEnd()); p = p.addOffsetTo(pcDescSize)) {
PCDesc pd = new PCDesc(p);
if (pd.getPCOffset() == -1) {
break;
}
ScopeDesc sd = new ScopeDesc(this, pd.getScopeDecodeOffset(), pd.getObjDecodeOffset(), pd.getReexecute());
}
}

/** ScopeDesc for an instruction */
public ScopeDesc getScopeDescAt(Address pc) {
PCDesc pd = getPCDescAt(pc);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright (c) 2024, 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
* 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.
*
*/

package sun.jvm.hotspot.code;

import java.io.PrintStream;
import java.util.ArrayList;
import java.util.List;

class ObjectMergeValue extends ObjectValue {

private ScopeValue selector;
private ScopeValue mergePointer;
private List<ScopeValue> possibleObjects;

public ObjectMergeValue(int id) {
super(id);
}

public boolean isObjectMerge() { return true; }

void readObject(DebugInfoReadStream stream) {
selector = ScopeValue.readFrom(stream);
mergePointer = ScopeValue.readFrom(stream);
possibleObjects = new ArrayList<>();
int length = stream.readInt();
for (int i = 0; i < length; i++) {
ScopeValue val = readFrom(stream);
possibleObjects.add(val);
}
}

@Override
public void printOn(PrintStream tty) {
tty.print("merge_obj[" + id + "]");
tty.print(" selector=\"");
selector.printOn(tty);
tty.print("\"");

tty.print(" merge_pointer=\"");
mergePointer.printOn(tty);
tty.print("\"");

tty.print(", candidate_objs=[" + ((ObjectValue) possibleObjects.get(0)).id);
for (int i = 1; i < possibleObjects.size(); i++) {
tty.print("," + ((ObjectValue) possibleObjects.get(i)).id);
}
tty.print("]");
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2009, 2024, 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 @@ -33,7 +33,8 @@
/** An ObjectValue describes an object eliminated by escape analysis. */

public class ObjectValue extends ScopeValue {
private int id;
protected final int id;
private boolean isRoot;
private ScopeValue klass;
private List<ScopeValue> fieldsValue;

Expand Down Expand Up @@ -61,8 +62,9 @@ public ObjectValue(int id) {
/** Serialization of debugging information */

void readObject(DebugInfoReadStream stream) {
isRoot = stream.readBoolean();
klass = readFrom(stream);
Assert.that(klass.isConstantOop(), "should be constant klass oop");
Assert.that(klass.isConstantOop(), "should be constant klass oop: " + klass);
int length = stream.readInt();
for (int i = 0; i < length; i++) {
ScopeValue val = readFrom(stream);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2009, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2024, 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.code;

import java.io.*;
import java.nio.charset.StandardCharsets;

import sun.jvm.hotspot.utilities.*;

Expand All @@ -51,13 +52,18 @@ public abstract class ScopeValue {
static final int CONSTANT_DOUBLE_CODE = 4;
static final int CONSTANT_OBJECT_CODE = 5;
static final int CONSTANT_OBJECT_ID_CODE = 6;
static final int AUTO_BOX_OBJECT_CODE = 7;
static final int MARKER_CODE = 8;
static final int OBJECT_MERGE_CODE = 9;

public boolean isLocation() { return false; }
public boolean isConstantInt() { return false; }
public boolean isConstantDouble() { return false; }
public boolean isConstantLong() { return false; }
public boolean isConstantOop() { return false; }
public boolean isObject() { return false; }
public boolean isMarker() { return false; }
public boolean isObjectMerge() { return false; }

public static ScopeValue readFrom(DebugInfoReadStream stream) {
switch (stream.readInt()) {
Expand All @@ -72,14 +78,29 @@ public static ScopeValue readFrom(DebugInfoReadStream stream) {
case CONSTANT_DOUBLE_CODE:
return new ConstantDoubleValue(stream);
case CONSTANT_OBJECT_CODE:
case AUTO_BOX_OBJECT_CODE:
// The C++ code handles these 2 cases separately because the autobox case needs
// some extra state during deoptimization. That's not required to display the
// information so treat it like a normal object value.
return stream.readObjectValue();
case CONSTANT_OBJECT_ID_CODE:
return stream.getCachedObject();
case MARKER_CODE:
return new MarkerValue();
case OBJECT_MERGE_CODE:
return stream.readObjectMergeValue();
default:
Assert.that(false, "should not reach here");
return null;
}
}

public abstract void printOn(PrintStream tty);

public String toString() {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
PrintStream ps = new PrintStream(baos, true, StandardCharsets.UTF_8);
printOn(ps);
return baos.toString(StandardCharsets.UTF_8);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2024, 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 @@ -317,13 +317,9 @@ public int getLineNumberFromBCI(int bci) {
return -1;
}

if (Assert.ASSERTS_ENABLED) {
Assert.that(0 <= bci && bci < getCodeSize(),
"illegal bci(" + bci + ") codeSize(" + getCodeSize() + ")");
}
int bestBCI = 0;
int bestLine = -1;
if (hasLineNumberTable()) {
if (0 <= bci && bci < getCodeSize() && hasLineNumberTable()) {
// The line numbers are a short array of 2-tuples [start_pc, line_number].
// Not necessarily sorted and not necessarily one-to-one.
CompressedLineNumberReadStream stream =
Expand Down
1 change: 1 addition & 0 deletions test/hotspot/jtreg/ProblemList-generational-zgc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ serviceability/sa/jmap-hprof/JMapHProfLargeHeapProc.java 8307393 generic-
serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java 8307393 generic-all
serviceability/sa/sadebugd/ClhsdbAttachToDebugServer.java 8307393 generic-all
serviceability/sa/sadebugd/ClhsdbTestConnectArgument.java 8307393 generic-all
serviceability/sa/ClhsdbTestAllocationMerge.java 8307393 generic-all
serviceability/sa/sadebugd/DebugdConnectTest.java 8307393 generic-all
serviceability/sa/sadebugd/DebugdUtils.java 8307393 generic-all
serviceability/sa/sadebugd/DisableRegistryTest.java 8307393 generic-all
Expand Down
1 change: 1 addition & 0 deletions test/hotspot/jtreg/serviceability/sa/ClhsdbLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ private String runCmd(List<String> commands,
oa.shouldNotMatch("^WARNING in native method:.*$");
// This will detect most SA failures, including during the attach.
oa.shouldNotMatch("^sun.jvm.hotspot.debugger.DebuggerException:.*$");
oa.shouldNotMatch("sun.jvm.hotspot.utilities.AssertionFailure");
// This will detect unexpected exceptions, like NPEs and asserts, that are caught
// by sun.jvm.hotspot.CommandProcessor.
oa.shouldNotMatch("^Error: .*$");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright (c) 2024, 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
* 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 java.util.List;
import jdk.test.lib.apps.LingeredApp;
import jtreg.SkippedException;

/**
* @test
* @bug 8318682
* @summary Test clhsdb that decoding of AllocationMerge objects in debug info works correctly
* @requires vm.hasSA
* @library /test/lib
* @run main/othervm ClhsdbTestAllocationMerge
*/

public class ClhsdbTestAllocationMerge {

public static void main(String[] args) throws Exception {
System.out.println("Starting ClhsdbTestDebugInfodDecode test");

LingeredApp theApp = null;
try {
ClhsdbLauncher test = new ClhsdbLauncher();

theApp = new LingeredAppWithAllocationMerge();
LingeredApp.startApp(theApp);
System.out.println("Started LingeredAppWithAllocationMerge with pid " + theApp.getPid());

List<String> cmds = List.of("jstack -v");

// sun.jvm.hotspot.utilities.AssertionFailure is caught by the harness so it's not
// necessary to include extra filters here.
test.run(theApp.getPid(), cmds, null, null);
} catch (SkippedException se) {
throw se;
} catch (Exception ex) {
throw new RuntimeException("Test ERROR " + ex, ex);
} finally {
LingeredApp.stopApp(theApp);
}
System.out.println("Test PASSED");
}
}
Loading

0 comments on commit b96b38c

Please sign in to comment.