Gate FrameType::isRawPointer on ENCODED_MASK and HotSpot

ddprof-lib/src/main/cpp/frame.h

@@ -79,11 +79,13 @@ class FrameType {
     return (FrameTypeId)((bci >> TYPE_SHIFT) & FRAME_TYPE_MASK);
   }
 
-  // Raw pointers only exist on HotSpot (see hotspotSupport.cpp fillFrameRaw).
-  // On other VMs an unencoded, VM-supplied bci can coincidentally have bit 30
-  // set; that must not be mistaken for our raw-pointer encoding.
+  // Raw pointers only exist on HotSpot (see hotspotSupport.cpp fillFrameRaw),
+  // and RAW_POINTER_MASK is only ever set by encode(..., rawPointer=true),
+  // which unconditionally also sets ENCODED_MASK. Requiring both HotSpot and
+  // ENCODED_MASK prevents a raw, VM-supplied ASGCT BCI from false-positiving
+  // just because it happens to have bit 30 set.
   static inline bool isRawPointer(int bci) {
-    return VM::isHotspot() && bci > 0 && (bci & RAW_POINTER_MASK) != 0;
+    return VM::isHotspot() && (bci > 0 && (bci & ENCODED_MASK) != 0 && (bci & RAW_POINTER_MASK) != 0);
   }
 };
 

ddprof-lib/src/test/cpp/frame_ut.cpp

@@ -7,14 +7,29 @@
 #include "../../main/cpp/frame.h"
 #include "../../main/cpp/gtest_crash_handler.h"
 
-// VMTestAccessor — friend of VM, lets tests toggle VM::isHotspot() since
-// isRawPointer() now gates on it (raw pointers only exist on HotSpot).
+// Test-only friend accessor for VM internals. It exists solely so these unit
+// tests can exercise the VM-specific raw-pointer path in FrameType and must
+// never be reused from production code.
 class VMTestAccessor {
 public:
     static bool getHotspot() { return VM::_hotspot; }
     static void setHotspot(bool v) { VM::_hotspot = v; }
 };
 
+class VMHotspotGuard {
+private:
+    bool _saved;
+
+public:
+    explicit VMHotspotGuard(bool hotspot) : _saved(VMTestAccessor::getHotspot()) {
+        VMTestAccessor::setHotspot(hotspot);
+    }
+
+    ~VMHotspotGuard() {
+        VMTestAccessor::setHotspot(_saved);
+    }
+};
+
 static constexpr char FRAME_TEST_NAME[] = "FrameTest";
 
 class GlobalSetup {
@@ -150,26 +165,22 @@ TEST(FrameTypeIsRawPointerTest, FalseForEncodedWithoutFlag) {
 TEST(FrameTypeIsRawPointerTest, TrueWhenBit30IsSetOnHotspot) {
     // Manually set the raw-pointer flag (bit 30) on an encoded value.
     // Raw pointers only exist on HotSpot, so isRawPointer() must be gated on it.
-    bool saved = VMTestAccessor::getHotspot();
-    VMTestAccessor::setHotspot(true);
+    VMHotspotGuard hotspot(true);
     int base = FrameType::encode(FRAME_JIT_COMPILED, 0);
     int withFlag = base | (1 << 30);
     EXPECT_TRUE(FrameType::isRawPointer(withFlag))
         << "isRawPointer() must be true when bit 30 is set, value is positive, and VM is HotSpot";
-    VMTestAccessor::setHotspot(saved);
 }
 
 TEST(FrameTypeIsRawPointerTest, FalseWhenBit30IsSetOnNonHotspot) {
     // Non-HotSpot VMs can coincidentally produce an unencoded bci with bit 30
     // set (e.g. OpenJ9's raw AsyncGetCallTrace output); that must not be
     // mistaken for HotSpot's raw-pointer encoding.
-    bool saved = VMTestAccessor::getHotspot();
-    VMTestAccessor::setHotspot(false);
+    VMHotspotGuard hotspot(false);
     int base = FrameType::encode(FRAME_JIT_COMPILED, 0);
     int withFlag = base | (1 << 30);
     EXPECT_FALSE(FrameType::isRawPointer(withFlag))
         << "isRawPointer() must be false when VM is not HotSpot, even with bit 30 set";
-    VMTestAccessor::setHotspot(saved);
 }
 
 TEST(FrameTypeIsRawPointerTest, FalseWithOnlyBit30SetOnNegative) {
@@ -179,3 +190,16 @@ TEST(FrameTypeIsRawPointerTest, FalseWithOnlyBit30SetOnNegative) {
     EXPECT_LT(negativeWithBit30, 0);
     EXPECT_FALSE(FrameType::isRawPointer(negativeWithBit30));
 }
+
+TEST(FrameTypeIsRawPointerTest, FalseForRawAsgctBciWithBit30SetButNoEncodedMarker) {
+    // Regression test: a raw, unencoded ASGCT BCI (bit 20 / ENCODED_MASK not set)
+    // that incidentally has bit 30 set must NOT be reported as a raw pointer.
+    // Such values occur on non-HotSpot VMs (e.g. J9), which never call encode()
+    // and therefore never set ENCODED_MASK; only encode(..., rawPointer=true)
+    // (HotSpot-only, per its own assert) is allowed to set RAW_POINTER_MASK.
+    int rawAsgctBciWithBit30 = 1 << 30;
+    VMHotspotGuard hotspot(true);
+    EXPECT_FALSE(FrameType::isRawPointer(rawAsgctBciWithBit30))
+        << "isRawPointer() must require ENCODED_MASK (bit 20) before trusting bit 30, "
+        << "otherwise raw ASGCT BCIs that never went through encode() can false-positive";
+}