From e92873b822bf78de7cdce380b85c99150ae4f464 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 9 Oct 2026 14:54:03 -0400 Subject: [PATCH 01/10] Code coverage for Java Profiler --- .../native/config/ConfigurationPresets.kt | 46 ++++++++++++++ ddprof-lib/build.gradle.kts | 63 +++++++++++++++++-- ddprof-test/build.gradle.kts | 58 +++++++++++++++++ 3 files changed, 163 insertions(+), 4 deletions(-) diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt index ea4a9c7c62..05fadf59a9 100644 --- a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt @@ -54,6 +54,16 @@ object ConfigurationPresets { // reason: those configs instrument every memory access, so the measured // per-sample times say more about the sanitizer than about the sampler. val samplerPerf = project.hasProperty("enableSamplerPerf") + + // Opt-in gcov/llvm-cov instrumented build. Inactive by default -- only + // registered as an active configuration (and thus only gets its + // compile/link/assemble/gtest tasks created at all, see + // NativeBuildPlugin/GtestPlugin filtering on getActiveConfigurations) + // when -PenableCoverage is passed. This keeps coverage instrumentation + // completely out of the default build and out of CI runs that don't + // ask for it, mirroring how asan/tsan are gated off when unsupported. + val enableCoverage = project.hasProperty("enableCoverage") + extension.buildConfigurations.apply { register("release") { configureRelease(this, currentPlatform, currentArch, version) @@ -74,6 +84,9 @@ object ConfigurationPresets { register("fuzzer") { configureFuzzer(this, currentPlatform, currentArch, version, rootDir, compiler) } + register("coverage") { + configureCoverage(this, currentPlatform, currentArch, version, enableCoverage) + } } val activeConfigs = extension.getActiveConfigurations(currentPlatform, currentArch) @@ -406,4 +419,37 @@ object ConfigurationPresets { } } } + + /** + * gcov/llvm-cov-instrumented debug build (`--coverage` is understood by both + * gcc and clang and emits gcov-compatible .gcno/.gcda files, so one preset + * covers both toolchains/platforms without a separate clang source-based + * coverage format). Inactive unless the caller opts in via -PenableCoverage. + */ + fun configureCoverage( + config: BuildConfiguration, + platform: Platform, + architecture: Architecture, + version: String, + active: Boolean + ) { + config.platform.set(platform) + config.architecture.set(architecture) + config.active.set(active) + + when (platform) { + Platform.LINUX -> { + config.compilerArgs.set( + listOf("-O0", "-g", "-DDEBUG", "--coverage") + commonLinuxCompilerArgs(version) + ) + config.linkerArgs.set(commonLinuxLinkerArgs() + listOf("--coverage")) + } + Platform.MACOS -> { + config.compilerArgs.set( + commonMacosCompilerArgs(version) + listOf("-O0", "-g", "-DDEBUG", "--coverage") + ) + config.linkerArgs.set(listOf("--coverage")) + } + } + } } diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index a672ab0c4e..c08ba35a41 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -96,9 +96,15 @@ val copyExternalLibs by tasks.registering(Copy::class) { } // Create JAR tasks for each build configuration using nativeBuild extension utilities -// Uses afterEvaluate to discover configurations dynamically from NativeBuildExtension +// Uses afterEvaluate to discover configurations dynamically from NativeBuildExtension. +// `coverage` is excluded: nativeBuild.buildConfigurations.names lists every registered +// config regardless of its `active` flag (the same quirk already lets inactive +// asan/tsan show up here on platforms where they're unsupported), so without this +// filter a --coverage-instrumented classifier jar would get built and added to the +// published Maven artifact by every `publish` run, coverage build or not. afterEvaluate { - nativeBuild.buildConfigurations.names.forEach { name -> + val packagedConfigNames = nativeBuild.buildConfigurations.names.filterNot { it == "coverage" } + packagedConfigNames.forEach { name -> val capitalizedName = name.replaceFirstChar { it.uppercase() } val copyTask = tasks.register("copy${capitalizedName}Libs", Copy::class) { @@ -151,6 +157,54 @@ afterEvaluate { } } +// C++ coverage report (only meaningful, and only registered, alongside the opt-in +// `coverage` build configuration -- see ConfigurationPresets.configureCoverage). +// Requires gcovr (`pip install gcovr`), which reads the gcc/clang-compatible +// .gcno/.gcda files gtestCoverage produces directly, so one tool covers both +// toolchains without a separate lcov+genhtml install. +if (project.hasProperty("enableCoverage")) { + tasks.register("gtestCoverageReport") { + group = "verification" + description = "Generates an HTML C++ coverage report from the gtestCoverage run (requires gcovr)" + dependsOn("gtestCoverage") + + val reportDir = layout.buildDirectory.dir("reports/coverage/cpp") + val objDir = layout.buildDirectory.dir("obj/gtest/coverage") + val srcRoot = project.file("src/main/cpp") + + doLast { + val gcovExecutable = when { + PlatformUtils.currentPlatform == Platform.MACOS -> "xcrun llvm-cov gcov" + File(PlatformUtils.findCompiler(project)).name.contains("clang") -> "llvm-cov gcov" + else -> "gcov" + } + + val outDir = reportDir.get().asFile.also { it.mkdirs() } + val cmd = listOf( + "gcovr", + "--root", srcRoot.absolutePath, + "--gcov-executable", gcovExecutable, + "--html", "--html-details", + "-o", "${outDir.absolutePath}/index.html", + "--print-summary", + objDir.get().asFile.absolutePath + ) + project.logger.lifecycle("Running: ${cmd.joinToString(" ")}") + val process = ProcessBuilder(cmd) + .directory(project.projectDir) + .redirectErrorStream(true) + .start() + val output = process.inputStream.bufferedReader().readText() + val exitCode = process.waitFor() + println(output) + if (exitCode != 0) { + throw GradleException("gcovr failed (exit $exitCode) -- install it with 'pip install gcovr' if missing") + } + project.logger.lifecycle("Coverage report written to file://${outDir.absolutePath}/index.html") + } + } +} + // Add runBenchmarks task tasks.register("runBenchmarks") { dependsOn(":ddprof-lib:benchmarks:runBenchmark") @@ -199,9 +253,10 @@ publishing { groupId = "com.datadoghq" artifactId = "ddprof" - // Add artifacts from each build configuration + // Add artifacts from each build configuration (coverage excluded -- see the + // packagedConfigNames comment above) afterEvaluate { - nativeBuild.buildConfigurations.names.forEach { name -> + nativeBuild.buildConfigurations.names.filterNot { it == "coverage" }.forEach { name -> val capitalizedName = name.replaceFirstChar { it.uppercase() } artifact(tasks.named("assemble${capitalizedName}Jar")) } diff --git a/ddprof-test/build.gradle.kts b/ddprof-test/build.gradle.kts index cbbb573b57..fe6e2e26ef 100644 --- a/ddprof-test/build.gradle.kts +++ b/ddprof-test/build.gradle.kts @@ -1,13 +1,71 @@ import com.datadoghq.profiler.ProfilerTestExtension +import org.gradle.api.tasks.SourceSetContainer plugins { java `java-library` application + jacoco id("com.datadoghq.profiler-test") id("com.datadoghq.java-conventions") } +jacoco { + toolVersion = "0.8.12" +} + +// JaCoCo's plugin auto-attaches a JacocoTaskExtension (enabled by default) to every Test +// task in this project, including testAsan/testSlowAsan/testTsan/testSlowTsan. Restrict +// instrumentation to the plain debug config's tasks only -- asan/tsan already carry +// sanitizer instrumentation and don't need coverage data, and leaving JaCoCo enabled +// everywhere would multiply CI cost for no additional signal. +val coverageTaskNames: Set = if (project.hasProperty("disableJacoco")) emptySet() else setOf("testDebug", "testSlowDebug") + +// When jacocoTestReport is actually being requested, don't let a handful of +// known-flaky native-thread/CPU tests (already @RetryingTest-annotated; see +// NativeThreadTest, DynamicNativeThread, VtableReceiverFrameTest, ThreadEntryDetectionTest) +// block report generation -- testDebug still records and prints its failures, +// it just won't fail the build, so jacocoTestReport's dependency is satisfied +// and the exec data that was produced gets aggregated regardless. +val requestingCoverageReport = gradle.startParameter.taskNames.any { it.contains("jacocoTestReport") } + +tasks.withType().configureEach { + extensions.configure { + isEnabled = name in coverageTaskNames + } + if (name in coverageTaskNames && requestingCoverageReport) { + ignoreFailures = true + } +} + +// The jacoco plugin already registers a default "jacocoTestReport" task (wired to the +// disabled "test" task); reconfigure it to aggregate our actual coverage-producing tasks +// instead of registering a new one. +tasks.named("jacocoTestReport") { + group = "verification" + description = "Generates an aggregated HTML/XML coverage report from testDebug and testSlowDebug" + + val coverageTasks = tasks.matching { it.name in coverageTaskNames } + dependsOn(coverageTasks) + executionData.setFrom(coverageTasks.map { task -> + fileTree(task.project.layout.buildDirectory) { include("jacoco/${task.name}.exec") } + }) + + // The tests here exercise ddprof-lib's classes (the profiler itself), not this + // project's own trivial main sourceSet (just the UnwindingValidator app) -- so + // coverage must be measured against ddprof-lib, the actual target under test. + val libMainSourceSet = project(":ddprof-lib").extensions.getByType()["main"] + sourceDirectories.setFrom(libMainSourceSet.allSource.srcDirs) + classDirectories.setFrom(libMainSourceSet.output) + + reports { + html.required.set(true) + xml.required.set(true) + html.outputLocation.set(layout.buildDirectory.dir("reports/jacoco/jacocoTestReport/html")) + xml.outputLocation.set(layout.buildDirectory.file("reports/jacoco/jacocoTestReport/jacocoTestReport.xml")) + } +} + // Reference to native test helpers library directory val testNativeLibDir = project(":ddprof-test-native").layout.buildDirectory.dir("lib") From 6d08c413a94b9e68a6bb068827669784c9820df4 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 12 Oct 2026 16:23:33 +0200 Subject: [PATCH 02/10] Run Java integration tests against the coverage build and harden coverage tasks - Package the `coverage` native library jar and consumable configuration when -PenableCoverage is set, so :ddprof-test:testCoverage can resolve it (still excluded from publishing). - gtestCoverageReport now also depends on :ddprof-test:testCoverage and merges the gtest and Java-test .gcda data into one gcovr report; negative hit counts are warned about instead of failing the report. - Build the coverage config with -fprofile-update=atomic so concurrent profiler threads don't corrupt gcov counters, and link it on Linux with -Wl,-z,nodelete like the debug config. - Clear stale .gcda files before each gtestCoverage_ and testCoverage run, and derive the gcov tool from the configured compiler instead of the gcov on PATH. - Enable JaCoCo on testDebug/testSlowDebug only when jacocoTestReport is requested (drops -PdisableJacoco). When a coverage report is requested, test failures are deferred rather than ignored: the report is generated, then the build fails listing the failed test tasks. - Add NativeBuildConfig so debug-only tests also run against the coverage build. Co-Authored-By: Claude Opus 5.5 --- .../native/config/ConfigurationPresets.kt | 19 +++- ddprof-lib/build.gradle.kts | 58 +++++++--- ddprof-test/build.gradle.kts | 105 +++++++++++++++--- .../profiler/AbstractProfilerTest.java | 6 +- .../com/datadoghq/profiler/JVMAccessTest.java | 9 +- .../datadoghq/profiler/JavaProfilerTest.java | 12 +- .../datadoghq/profiler/NativeBuildConfig.java | 31 ++++++ .../profiler/UnwindFailuresTest.java | 8 +- 8 files changed, 193 insertions(+), 55 deletions(-) create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/NativeBuildConfig.java diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt index 05fadf59a9..183482a80e 100644 --- a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt @@ -1,3 +1,8 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + package com.datadoghq.native.config @@ -425,6 +430,11 @@ object ConfigurationPresets { * gcc and clang and emits gcov-compatible .gcno/.gcda files, so one preset * covers both toolchains/platforms without a separate clang source-based * coverage format). Inactive unless the caller opts in via -PenableCoverage. + * + * `-fprofile-update=atomic`: the default counter updates are plain + * read-modify-writes, so the profiler's concurrent threads (samplers, signal + * handlers, JVM threads) lose and corrupt counts -- a Linux run produced + * negative branch counts (GCC bug 68080) that gcovr refuses to parse. */ fun configureCoverage( config: BuildConfiguration, @@ -440,13 +450,16 @@ object ConfigurationPresets { when (platform) { Platform.LINUX -> { config.compilerArgs.set( - listOf("-O0", "-g", "-DDEBUG", "--coverage") + commonLinuxCompilerArgs(version) + listOf("-O0", "-g", "-DDEBUG", "--coverage", "-fprofile-update=atomic") + commonLinuxCompilerArgs(version) ) - config.linkerArgs.set(commonLinuxLinkerArgs() + listOf("--coverage")) + // -z,nodelete: same as configureDebug -- testCoverage loads this library + // into the same JVMs, so it is exposed to the same unmap-while-executing + // SIGSEGV on the DestroyJavaVM path. + config.linkerArgs.set(commonLinuxLinkerArgs() + listOf("-Wl,-z,nodelete", "--coverage")) } Platform.MACOS -> { config.compilerArgs.set( - commonMacosCompilerArgs(version) + listOf("-O0", "-g", "-DDEBUG", "--coverage") + commonMacosCompilerArgs(version) + listOf("-O0", "-g", "-DDEBUG", "--coverage", "-fprofile-update=atomic") ) config.linkerArgs.set(listOf("--coverage")) } diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index c08ba35a41..ad4b4624b3 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -97,13 +97,15 @@ val copyExternalLibs by tasks.registering(Copy::class) { // Create JAR tasks for each build configuration using nativeBuild extension utilities // Uses afterEvaluate to discover configurations dynamically from NativeBuildExtension. -// `coverage` is excluded: nativeBuild.buildConfigurations.names lists every registered -// config regardless of its `active` flag (the same quirk already lets inactive -// asan/tsan show up here on platforms where they're unsupported), so without this -// filter a --coverage-instrumented classifier jar would get built and added to the -// published Maven artifact by every `publish` run, coverage build or not. +// `coverage` is only packaged with -PenableCoverage: nativeBuild.buildConfigurations.names +// lists every registered config regardless of its `active` flag (the same quirk already +// lets inactive asan/tsan show up here on platforms where they're unsupported). With the +// flag, its jar and consumable configuration are what ddprof-test's testCoverage loads; +// it is still never published (see the publishing block below). afterEvaluate { - val packagedConfigNames = nativeBuild.buildConfigurations.names.filterNot { it == "coverage" } + val packagedConfigNames = nativeBuild.buildConfigurations.names.filter { + it != "coverage" || project.hasProperty("enableCoverage") + } packagedConfigNames.forEach { name -> val capitalizedName = name.replaceFirstChar { it.uppercase() } @@ -160,23 +162,46 @@ afterEvaluate { // C++ coverage report (only meaningful, and only registered, alongside the opt-in // `coverage` build configuration -- see ConfigurationPresets.configureCoverage). // Requires gcovr (`pip install gcovr`), which reads the gcc/clang-compatible -// .gcno/.gcda files gtestCoverage produces directly, so one tool covers both -// toolchains without a separate lcov+genhtml install. +// .gcno/.gcda files directly, so one tool covers both toolchains without a +// separate lcov+genhtml install. +// +// Two runs feed it: the gtest binaries (gtestCoverage, objects under +// obj/gtest/coverage) and the Java integration tests (:ddprof-test:testCoverage -- +// the testDebug suite run against the coverage-instrumented library, objects +// under obj/main/coverage). gcovr merges both into one per-source-file report. if (project.hasProperty("enableCoverage")) { + val gtestObjDir = layout.buildDirectory.dir("obj/gtest/coverage") + + // gcov adds to existing .gcda counts, and .gcda files left over from before a + // recompile no longer match the new .gcno stamps -- so every gtest binary must start + // from zero. Runs before each gtestCoverage_ task rather than in gtestCoverage + // itself, which is only an aggregate that executes after its test tasks. + val cleanGtestCoverageData = tasks.register("cleanGtestCoverageData") { + delete(fileTree(gtestObjDir) { include("**/*.gcda") }) + } + tasks.matching { it.name.startsWith("gtestCoverage_") }.configureEach { + dependsOn(cleanGtestCoverageData) + } + tasks.register("gtestCoverageReport") { group = "verification" - description = "Generates an HTML C++ coverage report from the gtestCoverage run (requires gcovr)" - dependsOn("gtestCoverage") + description = "Generates an HTML C++ coverage report from gtestCoverage and :ddprof-test:testCoverage (requires gcovr)" + dependsOn("gtestCoverage", ":ddprof-test:testCoverage") val reportDir = layout.buildDirectory.dir("reports/coverage/cpp") - val objDir = layout.buildDirectory.dir("obj/gtest/coverage") + val libObjDir = layout.buildDirectory.dir("obj/main/coverage") val srcRoot = project.file("src/main/cpp") doLast { + // The gcov tool must come from the same toolchain that wrote the .gcno files, + // so derive it from the compiler (e.g. /usr/bin/g++-13 -> /usr/bin/gcov-13, + // clang++-17 -> llvm-cov-17 gcov) instead of whatever `gcov` is on PATH. + val compiler = File(PlatformUtils.findCompiler(project)) + fun siblingOf(tool: String) = compiler.parentFile?.resolve(tool)?.path ?: tool val gcovExecutable = when { PlatformUtils.currentPlatform == Platform.MACOS -> "xcrun llvm-cov gcov" - File(PlatformUtils.findCompiler(project)).name.contains("clang") -> "llvm-cov gcov" - else -> "gcov" + compiler.name.contains("clang") -> siblingOf(compiler.name.replace(Regex("clang(\\+\\+)?"), "llvm-cov")) + " gcov" + else -> siblingOf(compiler.name.replace(Regex("[gc]\\+\\+"), "gcov")) } val outDir = reportDir.get().asFile.also { it.mkdirs() } @@ -187,7 +212,12 @@ if (project.hasProperty("enableCoverage")) { "--html", "--html-details", "-o", "${outDir.absolutePath}/index.html", "--print-summary", - objDir.get().asFile.absolutePath + // Safety net only: -fprofile-update=atomic (see configureCoverage) should keep + // counters consistent, but a stray negative count must not discard a + // ~20-minute run -- warn and ignore it instead of failing. + "--gcov-ignore-parse-errors=negative_hits.warn_once_per_file", + gtestObjDir.get().asFile.absolutePath, + libObjDir.get().asFile.absolutePath, ) project.logger.lifecycle("Running: ${cmd.joinToString(" ")}") val process = ProcessBuilder(cmd) diff --git a/ddprof-test/build.gradle.kts b/ddprof-test/build.gradle.kts index fe6e2e26ef..c94653d04c 100644 --- a/ddprof-test/build.gradle.kts +++ b/ddprof-test/build.gradle.kts @@ -1,5 +1,11 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + import com.datadoghq.profiler.ProfilerTestExtension import org.gradle.api.tasks.SourceSetContainer +import java.util.concurrent.ConcurrentHashMap plugins { java @@ -15,29 +21,90 @@ jacoco { } // JaCoCo's plugin auto-attaches a JacocoTaskExtension (enabled by default) to every Test -// task in this project, including testAsan/testSlowAsan/testTsan/testSlowTsan. Restrict -// instrumentation to the plain debug config's tasks only -- asan/tsan already carry -// sanitizer instrumentation and don't need coverage data, and leaving JaCoCo enabled -// everywhere would multiply CI cost for no additional signal. -val coverageTaskNames: Set = if (project.hasProperty("disableJacoco")) emptySet() else setOf("testDebug", "testSlowDebug") - -// When jacocoTestReport is actually being requested, don't let a handful of -// known-flaky native-thread/CPU tests (already @RetryingTest-annotated; see -// NativeThreadTest, DynamicNativeThread, VtableReceiverFrameTest, ThreadEntryDetectionTest) -// block report generation -- testDebug still records and prints its failures, -// it just won't fail the build, so jacocoTestReport's dependency is satisfied -// and the exec data that was produced gets aggregated regardless. +// task in this project. Instrument only when jacocoTestReport is actually requested, and +// then only the plain debug config's tasks: ordinary testDebug runs (CI included) keep +// running the profiler against unmodified bytecode -- JaCoCo's injected $jacocoInit +// methods and probe arrays would otherwise show up in the allocation and stack-sampling +// data those tests assert on -- and asan/tsan already carry sanitizer instrumentation. val requestingCoverageReport = gradle.startParameter.taskNames.any { it.contains("jacocoTestReport") } +val requestingCppCoverageReport = gradle.startParameter.taskNames.any { it.contains("gtestCoverageReport") } +val coverageTaskNames: Set = if (requestingCoverageReport) setOf("testDebug", "testSlowDebug") else emptySet() + +// When a coverage report is requested, a handful of known-flaky native-thread/CPU tests +// (already @RetryingTest-annotated; see NativeThreadTest, DynamicNativeThread, +// VtableReceiverFrameTest, ThreadEntryDetectionTest) must not keep the report from being +// generated. Their failures are deferred, not ignored: the test task records them and +// lets the report run, and the report task fails the build once the report is written. +val deferredTestFailures: MutableSet = ConcurrentHashMap.newKeySet() + +fun Task.deferFailuresToCoverageReport() { + val taskName = name + when (this) { + is Test -> { + ignoreFailures = true + addTestListener(object : TestListener { + override fun beforeSuite(suite: TestDescriptor) {} + override fun beforeTest(testDescriptor: TestDescriptor) {} + override fun afterTest(testDescriptor: TestDescriptor, result: TestResult) {} + override fun afterSuite(suite: TestDescriptor, result: TestResult) { + if (suite.parent == null && result.resultType == TestResult.ResultType.FAILURE) { + deferredTestFailures.add(taskName) + } + } + }) + } + is Exec -> { + isIgnoreExitValue = true + doLast { + if (executionResult.get().exitValue != 0) { + deferredTestFailures.add(taskName) + } + } + } + } +} + +fun failOnDeferredTestFailures(taskNames: Collection) { + val failed = taskNames.filter { it in deferredTestFailures } + if (failed.isNotEmpty()) { + throw GradleException("Coverage report generated, but tests failed in: ${failed.joinToString()}") + } +} tasks.withType().configureEach { extensions.configure { isEnabled = name in coverageTaskNames } - if (name in coverageTaskNames && requestingCoverageReport) { - ignoreFailures = true + // The failed tests' results are cached like a pass would be, so a rerun must not be + // UP-TO-DATE and silently skip the deferred failure. + if (name in coverageTaskNames) { + deferFailuresToCoverageReport() + outputs.upToDateWhen { false } } } +// testCoverage (created by the profiler-test plugin only with -PenableCoverage) runs the +// testDebug suite against ddprof-lib's gcov-instrumented `coverage` library; every test +// JVM that loads it writes .gcda counts next to the library's objects when it exits, and +// :ddprof-lib:gtestCoverageReport folds them into the C++ report. +val libCoverageObjDir = project(":ddprof-lib").layout.buildDirectory.dir("obj/main/coverage") + +tasks.matching { it.name == "testCoverage" }.configureEach { + // gcov adds to existing .gcda counts, so start each run from zero rather than + // accumulating counts from earlier runs (.gcno files, written at compile time, stay). + doFirst { + fileTree(libCoverageObjDir) { include("**/*.gcda") }.forEach { it.delete() } + } + if (requestingCppCoverageReport) { + deferFailuresToCoverageReport() + outputs.upToDateWhen { false } + } +} + +project(":ddprof-lib").tasks.matching { it.name == "gtestCoverageReport" }.configureEach { + doLast { failOnDeferredTestFailures(listOf("testCoverage")) } +} + // The jacoco plugin already registers a default "jacocoTestReport" task (wired to the // disabled "test" task); reconfigure it to aggregate our actual coverage-producing tasks // instead of registering a new one. @@ -47,9 +114,11 @@ tasks.named("jacocoTestReport") { val coverageTasks = tasks.matching { it.name in coverageTaskNames } dependsOn(coverageTasks) - executionData.setFrom(coverageTasks.map { task -> - fileTree(task.project.layout.buildDirectory) { include("jacoco/${task.name}.exec") } - }) + executionData.setFrom( + coverageTasks.map { task -> + fileTree(task.project.layout.buildDirectory) { include("jacoco/${task.name}.exec") } + }, + ) // The tests here exercise ddprof-lib's classes (the profiler itself), not this // project's own trivial main sourceSet (just the UnwindingValidator app) -- so @@ -58,6 +127,8 @@ tasks.named("jacocoTestReport") { sourceDirectories.setFrom(libMainSourceSet.allSource.srcDirs) classDirectories.setFrom(libMainSourceSet.output) + doLast { failOnDeferredTestFailures(coverageTaskNames) } + reports { html.required.set(true) xml.required.set(true) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java index b059bb655c..3399d93874 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java @@ -134,11 +134,11 @@ protected final boolean isAsan() { } /** - * Whether the tests run against the -O0 debug build of the native library, which unwinds - * several times slower than the release build. + * Whether the tests run against an -O0 debug build of the native library (debug or coverage), + * which unwinds several times slower than the release build. */ protected final boolean isDebugBuild() { - return "debug".equals(System.getProperty("ddprof_test.config")); + return NativeBuildConfig.isDebug(); } private static long getPid() { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java index f6b983f603..efd717af7a 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java @@ -27,8 +27,7 @@ static void setUp() { @Test void sanityInitailizationTest() throws Exception { - String config = System.getProperty("ddprof_test.config"); - assumeTrue("debug".equals(config)); + assumeTrue(NativeBuildConfig.isDebug()); AtomicBoolean initLibraryFound = new AtomicBoolean(false); AtomicBoolean initProfilerFound = new AtomicBoolean(false); @@ -66,8 +65,7 @@ void sanityInitailizationTest() throws Exception { */ @Test void agentOnLoadVMFlagDetectionTest() throws Exception { - String config = System.getProperty("ddprof_test.config"); - assumeTrue("debug".equals(config)); + assumeTrue(NativeBuildConfig.isDebug()); Path agentLib = LibraryLoader.resolveLibraryPath(System.getProperty("java.io.tmpdir")); @@ -104,8 +102,7 @@ void agentOnLoadVMFlagDetectionTest() throws Exception { @Test void jvmVersionTest() throws Exception { - String config = System.getProperty("ddprof_test.config"); - assumeTrue("debug".equals(config)); + assumeTrue(NativeBuildConfig.isDebug()); String javaVersion = System.getenv("JAVA_VERSION"); assumeTrue(javaVersion != null); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java index 2023c4757c..9edb26582a 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java @@ -26,8 +26,7 @@ public class JavaProfilerTest extends AbstractProcessProfilerTest { @Test void sanityInitailizationTest() throws Exception { - String config = System.getProperty("ddprof_test.config"); - assumeTrue("debug".equals(config)); + assumeTrue(NativeBuildConfig.isDebug()); AtomicInteger initFlag = new AtomicInteger(0); boolean val = launch("profiler", Collections.emptyList(), "", l -> { @@ -45,8 +44,7 @@ void sanityInitailizationTest() throws Exception { @Test void testJ9DefaultSanity() throws Exception { - String config = System.getProperty("ddprof_test.config"); - assumeTrue("debug".equals(config)); + assumeTrue(NativeBuildConfig.isDebug()); assumeFalse(Platform.isMac()); // crashy on mac assumeTrue(Platform.isJ9()); @@ -84,8 +82,7 @@ void testJ9DefaultSanity() throws Exception { @Test void testJ9ForceJvmtiSanity() throws Exception { - String config = System.getProperty("ddprof_test.config"); - assumeTrue("debug".equals(config)); + assumeTrue(NativeBuildConfig.isDebug()); assumeFalse(Platform.isMac()); // crashy on mac assumeTrue(Platform.isJ9()); @@ -137,8 +134,7 @@ void getInstanceFromVirtualThreadThrowsIOException() throws Exception { @Test void vmStackwalkerCrashRecoveryTest() throws Exception { assumeFalse(Platform.isJ9() || Platform.isZing()); // J9 and Zing do not support vmstructs - String config = System.getProperty("ddprof_test.config"); - assumeTrue("debug".equals(config)); + assumeTrue(NativeBuildConfig.isDebug()); Path jfr = Files.createTempFile("work", ".jfr"); jfr.toFile().deleteOnExit(); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/NativeBuildConfig.java b/ddprof-test/src/test/java/com/datadoghq/profiler/NativeBuildConfig.java new file mode 100644 index 0000000000..c0270b9e77 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/NativeBuildConfig.java @@ -0,0 +1,31 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler; + +/** + * Describes the native library build the tests run against, as passed by the test task in the + * {@code ddprof_test.config} system property (the build configuration name, e.g. {@code release}, + * {@code debug}, {@code asan}, {@code coverage}). + */ +public final class NativeBuildConfig { + private NativeBuildConfig() {} + + /** The build configuration name, or {@code null} when the property is not set. */ + public static String name() { + return System.getProperty("ddprof_test.config"); + } + + /** + * Whether the library is an unoptimized {@code -O0 -DDEBUG} build without a sanitizer: + * {@code debug}, or {@code coverage} (the debug build with gcov instrumentation added). + * Debug-only code paths (e.g. {@code datadog.UnwindFailure} recording) are compiled in, and + * unwinding is several times slower than in the release build. + */ + public static boolean isDebug() { + String config = name(); + return "debug".equals(config) || "coverage".equals(config); + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/UnwindFailuresTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/UnwindFailuresTest.java index b8bca75f8b..ae9d8539bf 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/UnwindFailuresTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/UnwindFailuresTest.java @@ -40,11 +40,11 @@ protected String getProfilerCommand() { @Test public void unwindFailureEventsHaveSaneSchemaWhenPresent() throws Exception { // UnwindFailures/UnwindStats are compiled out unless the native lib is built with - // -DDEBUG (debug/asan/tsan configs); on a release build datadog.UnwindFailure can + // -DDEBUG (debug/coverage/asan/tsan configs); on a release build datadog.UnwindFailure can // never appear, so there is nothing to check. - String config = System.getProperty("ddprof_test.config"); - assumeTrue(config != null && ("debug".equals(config) || config.endsWith("san")), - "datadog.UnwindFailure is only recorded in DEBUG-instrumented builds (debug/asan/tsan)"); + String config = NativeBuildConfig.name(); + assumeTrue(NativeBuildConfig.isDebug() || (config != null && config.endsWith("san")), + "datadog.UnwindFailure is only recorded in DEBUG-instrumented builds (debug/coverage/asan/tsan)"); // HotspotSupport::walkVM (and therefore UnwindFailures recording) is Hotspot-only. assumeFalse(Platform.isZing() || Platform.isJ9() || Platform.isGraal()); From e59c04ad8f93d6750675ff6f4eee578d9bca5963 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 13 Oct 2026 12:03:04 +0000 Subject: [PATCH 03/10] Generate Java/C++ coverage in one run --- ddprof-lib/build.gradle.kts | 34 +++++++++++------- ddprof-test/build.gradle.kts | 67 +++++++++++++++++++----------------- 2 files changed, 56 insertions(+), 45 deletions(-) diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index ad4b4624b3..aa3ff07bd3 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -159,16 +159,18 @@ afterEvaluate { } } -// C++ coverage report (only meaningful, and only registered, alongside the opt-in -// `coverage` build configuration -- see ConfigurationPresets.configureCoverage). -// Requires gcovr (`pip install gcovr`), which reads the gcc/clang-compatible -// .gcno/.gcda files directly, so one tool covers both toolchains without a -// separate lcov+genhtml install. +// Java + C++ coverage report (only meaningful, and only registered, alongside the +// opt-in `coverage` build configuration -- see ConfigurationPresets.configureCoverage). +// The Java half is :ddprof-test:jacocoTestReport (reports/coverage/java); this task +// itself writes the C++ half (reports/coverage/cpp). That needs gcovr +// (`pip install gcovr`), which reads the gcc/clang-compatible .gcno/.gcda files +// directly, so one tool covers both toolchains without a separate lcov+genhtml install. // -// Two runs feed it: the gtest binaries (gtestCoverage, objects under -// obj/gtest/coverage) and the Java integration tests (:ddprof-test:testCoverage -- -// the testDebug suite run against the coverage-instrumented library, objects -// under obj/main/coverage). gcovr merges both into one per-source-file report. +// Two runs feed the C++ report: the gtest binaries (gtestCoverage, objects under +// obj/gtest/coverage) and the Java integration tests (:ddprof-test:testCoverage and +// testSlowCoverage -- the same runs that produce the JaCoCo data, against the +// coverage-instrumented library, objects under obj/main/coverage). gcovr merges both +// into one per-source-file report. if (project.hasProperty("enableCoverage")) { val gtestObjDir = layout.buildDirectory.dir("obj/gtest/coverage") @@ -183,10 +185,10 @@ if (project.hasProperty("enableCoverage")) { dependsOn(cleanGtestCoverageData) } - tasks.register("gtestCoverageReport") { + tasks.register("coverageReport") { group = "verification" - description = "Generates an HTML C++ coverage report from gtestCoverage and :ddprof-test:testCoverage (requires gcovr)" - dependsOn("gtestCoverage", ":ddprof-test:testCoverage") + description = "Generates the Java (JaCoCo) and C++ (gcovr) coverage reports from gtestCoverage and :ddprof-test:testCoverage/testSlowCoverage (requires gcovr)" + dependsOn("gtestCoverage", ":ddprof-test:testCoverage", ":ddprof-test:testSlowCoverage", ":ddprof-test:jacocoTestReport") val reportDir = layout.buildDirectory.dir("reports/coverage/cpp") val libObjDir = layout.buildDirectory.dir("obj/main/coverage") @@ -216,6 +218,11 @@ if (project.hasProperty("enableCoverage")) { // counters consistent, but a stray negative count must not discard a // ~20-minute run -- warn and ignore it instead of failing. "--gcov-ignore-parse-errors=negative_hits.warn_once_per_file", + // gcovr flags any count above 2^32 as a GCC-68080 artifact, but the 64-bit atomic + // counters legitimately pass that on hot paths (e.g. threadLocal.h's isKeyValid() + // hit ~4.4e9 over testCoverage + testSlowCoverage). Real counts stay far below + // 1e15 (1e9 hits/s for ~11 days), while wrapped/garbage counts land near 2^64. + "--gcov-suspicious-hits-threshold=1000000000000000", gtestObjDir.get().asFile.absolutePath, libObjDir.get().asFile.absolutePath, ) @@ -230,7 +237,8 @@ if (project.hasProperty("enableCoverage")) { if (exitCode != 0) { throw GradleException("gcovr failed (exit $exitCode) -- install it with 'pip install gcovr' if missing") } - project.logger.lifecycle("Coverage report written to file://${outDir.absolutePath}/index.html") + project.logger.lifecycle("C++ coverage report written to file://${outDir.absolutePath}/index.html") + project.logger.lifecycle("Java coverage report written to file://${outDir.parentFile.absolutePath}/java/html/index.html") } } } diff --git a/ddprof-test/build.gradle.kts b/ddprof-test/build.gradle.kts index c94653d04c..ece208cefa 100644 --- a/ddprof-test/build.gradle.kts +++ b/ddprof-test/build.gradle.kts @@ -20,15 +20,19 @@ jacoco { toolVersion = "0.8.12" } +// :ddprof-lib:coverageReport produces both the Java (JaCoCo) and the C++ (gcovr) report +// from a single run of the Java test suites: testCoverage and testSlowCoverage (created +// by the profiler-test plugin only with -PenableCoverage) load ddprof-lib's +// gcov-instrumented `coverage` library *and* carry the JaCoCo agent. +// // JaCoCo's plugin auto-attaches a JacocoTaskExtension (enabled by default) to every Test -// task in this project. Instrument only when jacocoTestReport is actually requested, and -// then only the plain debug config's tasks: ordinary testDebug runs (CI included) keep -// running the profiler against unmodified bytecode -- JaCoCo's injected $jacocoInit -// methods and probe arrays would otherwise show up in the allocation and stack-sampling -// data those tests assert on -- and asan/tsan already carry sanitizer instrumentation. -val requestingCoverageReport = gradle.startParameter.taskNames.any { it.contains("jacocoTestReport") } -val requestingCppCoverageReport = gradle.startParameter.taskNames.any { it.contains("gtestCoverageReport") } -val coverageTaskNames: Set = if (requestingCoverageReport) setOf("testDebug", "testSlowDebug") else emptySet() +// task in this project. Instrument only the coverage config's tasks: ordinary testDebug +// runs (CI included) keep running the profiler against unmodified bytecode -- JaCoCo's +// injected $jacocoInit methods and probe arrays would otherwise show up in the allocation +// and stack-sampling data those tests assert on -- and asan/tsan already carry sanitizer +// instrumentation. +val coverageTaskNames: Set = setOf("testCoverage", "testSlowCoverage") +val requestingCoverageReport = gradle.startParameter.taskNames.any { it.contains("coverageReport") } // When a coverage report is requested, a handful of known-flaky native-thread/CPU tests // (already @RetryingTest-annotated; see NativeThreadTest, DynamicNativeThread, @@ -75,42 +79,39 @@ tasks.withType().configureEach { extensions.configure { isEnabled = name in coverageTaskNames } - // The failed tests' results are cached like a pass would be, so a rerun must not be - // UP-TO-DATE and silently skip the deferred failure. - if (name in coverageTaskNames) { - deferFailuresToCoverageReport() - outputs.upToDateWhen { false } - } } -// testCoverage (created by the profiler-test plugin only with -PenableCoverage) runs the -// testDebug suite against ddprof-lib's gcov-instrumented `coverage` library; every test -// JVM that loads it writes .gcda counts next to the library's objects when it exits, and -// :ddprof-lib:gtestCoverageReport folds them into the C++ report. +// Every test JVM that loads the coverage library writes .gcda counts next to the +// library's objects when it exits, and :ddprof-lib:coverageReport folds them into the +// C++ report. gcov adds to existing .gcda counts, so clear them once before the coverage +// test tasks run -- not in each task, which would wipe the counts the other one just +// wrote (.gcno files, written at compile time, stay). val libCoverageObjDir = project(":ddprof-lib").layout.buildDirectory.dir("obj/main/coverage") +val cleanLibCoverageData = tasks.register("cleanLibCoverageData") { + delete(fileTree(libCoverageObjDir) { include("**/*.gcda") }) +} -tasks.matching { it.name == "testCoverage" }.configureEach { - // gcov adds to existing .gcda counts, so start each run from zero rather than - // accumulating counts from earlier runs (.gcno files, written at compile time, stay). - doFirst { - fileTree(libCoverageObjDir) { include("**/*.gcda") }.forEach { it.delete() } - } - if (requestingCppCoverageReport) { +tasks.matching { it.name in coverageTaskNames }.configureEach { + dependsOn(cleanLibCoverageData) + // The failed tests' results are cached like a pass would be, so a rerun must not be + // UP-TO-DATE and silently skip the deferred failure (or the cleared .gcda counts). + if (requestingCoverageReport) { deferFailuresToCoverageReport() outputs.upToDateWhen { false } } } -project(":ddprof-lib").tasks.matching { it.name == "gtestCoverageReport" }.configureEach { - doLast { failOnDeferredTestFailures(listOf("testCoverage")) } +// Runs after coverageReport's own gcovr doLast, so both reports are written first. +project(":ddprof-lib").tasks.matching { it.name == "coverageReport" }.configureEach { + doLast { failOnDeferredTestFailures(coverageTaskNames) } } // The jacoco plugin already registers a default "jacocoTestReport" task (wired to the -// disabled "test" task); reconfigure it to aggregate our actual coverage-producing tasks +// disabled "test" task); reconfigure it as the Java half of :ddprof-lib:coverageReport // instead of registering a new one. tasks.named("jacocoTestReport") { group = "verification" - description = "Generates an aggregated HTML/XML coverage report from testDebug and testSlowDebug" + description = "Java half of :ddprof-lib:coverageReport (run that instead; requires -PenableCoverage)" val coverageTasks = tasks.matching { it.name in coverageTaskNames } dependsOn(coverageTasks) @@ -127,13 +128,15 @@ tasks.named("jacocoTestReport") { sourceDirectories.setFrom(libMainSourceSet.allSource.srcDirs) classDirectories.setFrom(libMainSourceSet.output) - doLast { failOnDeferredTestFailures(coverageTaskNames) } + // Test failures are checked by coverageReport, after both reports are written. + // Next to the C++ report (ddprof-lib/build/reports/coverage/cpp). + val javaReportDir = project(":ddprof-lib").layout.buildDirectory.dir("reports/coverage/java") reports { html.required.set(true) xml.required.set(true) - html.outputLocation.set(layout.buildDirectory.dir("reports/jacoco/jacocoTestReport/html")) - xml.outputLocation.set(layout.buildDirectory.file("reports/jacoco/jacocoTestReport/jacocoTestReport.xml")) + html.outputLocation.set(javaReportDir.map { it.dir("html") }) + xml.outputLocation.set(javaReportDir.map { it.file("jacoco.xml") }) } } From cc572d02ede7c40a437bf3ec1bba380ea9d69c22 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 14 Oct 2026 07:44:18 +0000 Subject: [PATCH 04/10] Fix musl --- .../datadoghq/profiler/ProfilerTestPlugin.kt | 20 ++++++++++++++++ ddprof-lib/build.gradle.kts | 8 ++++++- ddprof-test/build.gradle.kts | 24 ++++++++++++++++++- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt b/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt index b7fbae4632..c3faa324bf 100644 --- a/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt @@ -78,6 +78,23 @@ import javax.inject.Inject */ class ProfilerTestPlugin : Plugin { + companion object { + private const val ADDITIONAL_JVM_ARGS_EXTENSION = "additionalJvmArgs" + + /** + * Extra JVM args a build script can contribute to a musl Exec test task (e.g. + * ddprof-test's JaCoCo agent for the coverage tasks, which Gradle only attaches to + * Test tasks). Get-or-create, since a build script's configureEach on the task can + * run before the plugin's own configuration of it. + */ + @Suppress("UNCHECKED_CAST") + fun additionalJvmArgs(task: Exec): ListProperty = + task.extensions.findByName(ADDITIONAL_JVM_ARGS_EXTENSION) as ListProperty? + ?: task.project.objects.listProperty(String::class.java).also { + task.extensions.add(ADDITIONAL_JVM_ARGS_EXTENSION, it) + } + } + override fun apply(project: Project) { val extension = project.extensions.create( "profilerTest", @@ -362,6 +379,9 @@ class ProfilerTestPlugin : Plugin { } allArgs.addAll(testConfig.extraJvmArgs) allArgs.addAll(testConfig.configJvmArgs) + // Read here, not contributed via a doFirst of the build script's own: that + // would run before this one and its args would be overwritten. + allArgs.addAll(additionalJvmArgs(execTask).get()) // System properties testConfig.systemProperties.forEach { (key, value) -> diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index aa3ff07bd3..7274a4682e 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -238,7 +238,13 @@ if (project.hasProperty("enableCoverage")) { throw GradleException("gcovr failed (exit $exitCode) -- install it with 'pip install gcovr' if missing") } project.logger.lifecycle("C++ coverage report written to file://${outDir.absolutePath}/index.html") - project.logger.lifecycle("Java coverage report written to file://${outDir.parentFile.absolutePath}/java/html/index.html") + val javaReport = outDir.resolveSibling("java/html/index.html") + if (javaReport.isFile) { + project.logger.lifecycle("Java coverage report written to file://${javaReport.absolutePath}") + } else { + // jacocoTestReport is skipped when no test task wrote JaCoCo execution data. + project.logger.warn("No Java coverage report: no JaCoCo execution data was recorded") + } } } } diff --git a/ddprof-test/build.gradle.kts b/ddprof-test/build.gradle.kts index ece208cefa..0ed73239cf 100644 --- a/ddprof-test/build.gradle.kts +++ b/ddprof-test/build.gradle.kts @@ -4,6 +4,7 @@ */ import com.datadoghq.profiler.ProfilerTestExtension +import com.datadoghq.profiler.ProfilerTestPlugin import org.gradle.api.tasks.SourceSetContainer import java.util.concurrent.ConcurrentHashMap @@ -17,7 +18,7 @@ plugins { } jacoco { - toolVersion = "0.8.12" + toolVersion = "0.8.14" } // :ddprof-lib:coverageReport produces both the Java (JaCoCo) and the C++ (gcovr) report @@ -81,6 +82,27 @@ tasks.withType().configureEach { } } +// On musl the profiler-test plugin creates the test tasks as Exec tasks (running its own +// ProfilerTestRunner), which the jacoco plugin doesn't instrument -- attach the agent +// by hand so they write the same build/jacoco/.exec file a Test task would. +// The agent jar ships nested inside the org.jacoco.agent artifact (`jacocoAgent`). +val extractJacocoAgent = tasks.register("extractJacocoAgent") { + from(configurations.named("jacocoAgent").map { zipTree(it.singleFile) }) { + include("jacocoagent.jar") + } + into(layout.buildDirectory.dir("tmp/jacocoAgent")) +} + +tasks.withType().matching { it.name in coverageTaskNames }.configureEach { + dependsOn(extractJacocoAgent) + val agentJar = extractJacocoAgent.map { it.destinationDir.resolve("jacocoagent.jar") } + val execFile = layout.buildDirectory.file("jacoco/$name.exec") + outputs.file(execFile) + ProfilerTestPlugin.additionalJvmArgs(this).add( + provider { "-javaagent:${agentJar.get().absolutePath}=destfile=${execFile.get().asFile.absolutePath},append=false" }, + ) +} + // Every test JVM that loads the coverage library writes .gcda counts next to the // library's objects when it exits, and :ddprof-lib:coverageReport folds them into the // C++ report. gcov adds to existing .gcda counts, so clear them once before the coverage From e1cd10266f9d13b20f1942bdc7b571301ff3645d Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 14 Oct 2026 11:23:03 +0000 Subject: [PATCH 05/10] musl/c++ coverage --- ddprof-lib/build.gradle.kts | 20 +++++++++++++++----- utils/run-containers-tests.sh | 12 +++++++++--- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index 7274a4682e..316daacf78 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -227,15 +227,25 @@ if (project.hasProperty("enableCoverage")) { libObjDir.get().asFile.absolutePath, ) project.logger.lifecycle("Running: ${cmd.joinToString(" ")}") - val process = ProcessBuilder(cmd) - .directory(project.projectDir) - .redirectErrorStream(true) - .start() + // A missing gcovr surfaces here as a bare IOException, after the Java report has + // already been written -- turn it into the install hint instead. + val process = try { + ProcessBuilder(cmd) + .directory(project.projectDir) + .redirectErrorStream(true) + .start() + } catch (e: java.io.IOException) { + throw GradleException( + "gcovr not found -- install it with 'pip install gcovr' (Alpine/musl: 'apk add py3-pip && " + + "pip install --break-system-packages gcovr'; distro gcovr packages may be too old for the flags used here)", + e, + ) + } val output = process.inputStream.bufferedReader().readText() val exitCode = process.waitFor() println(output) if (exitCode != 0) { - throw GradleException("gcovr failed (exit $exitCode) -- install it with 'pip install gcovr' if missing") + throw GradleException("gcovr failed (exit $exitCode) -- see its output above") } project.logger.lifecycle("C++ coverage report written to file://${outDir.absolutePath}/index.html") val javaReport = outDir.resolveSibling("java/html/index.html") diff --git a/utils/run-containers-tests.sh b/utils/run-containers-tests.sh index fa2a1f9e24..c6c90af25c 100755 --- a/utils/run-containers-tests.sh +++ b/utils/run-containers-tests.sh @@ -1026,11 +1026,14 @@ FROM alpine:3.21 # - compiler-rt provides sanitizer runtimes (ASan, TSan) for clang # - llvm provides libFuzzer # - openssh-client for git clone over SSH +# - py3-pip + gcovr for :ddprof-lib:coverageReport (-PenableCoverage); Alpine +# doesn't package gcovr, so it comes from PyPI RUN apk update && \ apk add --no-cache \ curl wget bash make g++ clang git jq cmake coreutils \ gtest-dev gmock tar binutils musl-dbg linux-headers \ - compiler-rt llvm openssh-client gdb + compiler-rt llvm openssh-client gdb py3-pip && \ + pip install --no-cache-dir --break-system-packages gcovr # Set up Gradle cache directory ENV GRADLE_USER_HOME=/gradle-cache @@ -1075,13 +1078,16 @@ ENV DEBIAN_FRONTEND=noninteractive # - libclang-rt-dev for clang sanitizers and libFuzzer (x64 only) # - gnupg for LLVM apt key import # - openssh-client for git clone over SSH +# - python3-pip + gcovr for :ddprof-lib:coverageReport (-PenableCoverage); the +# distro's gcovr 5.0 predates flags the report task passes, so use PyPI's RUN apt-get update && \ apt-get install -y --no-install-recommends \ curl wget gnupg bash make g++ clang git jq cmake \ libgtest-dev libgmock-dev tar binutils libc6-dbg \ ca-certificates linux-libc-dev \ - libasan6 libtsan0 ${CLANG_RT_PKG} openssh-client build-essential gdb && \ - rm -rf /var/lib/apt/lists/* + libasan6 libtsan0 ${CLANG_RT_PKG} openssh-client build-essential gdb python3-pip && \ + rm -rf /var/lib/apt/lists/* && \ + pip3 install --no-cache-dir gcovr ${CLANG17_BLOCK} # Set up Gradle cache directory From f693672c5ea3281f4ce2901084a897bd87c2b1c5 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 16 Oct 2026 07:53:34 +0000 Subject: [PATCH 06/10] Eliminate jmethodID dependency for hotspot/cstack=vm --- .../datadoghq/native/gtest/GtestPlugin.kt.bak | 266 ++++++++++ .../native/util/PlatformUtils.kt.bak | 495 ++++++++++++++++++ .../datadoghq/profiler/ProfilerTestPlugin.kt | 15 +- ddprof-lib/src/main/cpp/arguments.cpp | 9 + ddprof-lib/src/main/cpp/arguments.h | 4 +- ddprof-lib/src/main/cpp/callTraceStorage.h | 4 + ddprof-lib/src/main/cpp/counters.h | 6 +- ddprof-lib/src/main/cpp/flightRecorder.cpp | 309 +++++++---- ddprof-lib/src/main/cpp/flightRecorder.h | 40 +- .../src/main/cpp/hotspot/hotspotStackFrame.h | 2 +- .../src/main/cpp/hotspot/hotspotSupport.cpp | 347 +++++------- .../src/main/cpp/hotspot/hotspotSupport.h | 101 +++- ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp | 13 + ddprof-lib/src/main/cpp/hotspot/vmStructs.h | 88 +++- .../src/main/cpp/hotspot/vmStructs.inline.h | 26 + ddprof-lib/src/main/cpp/jvmSupport.h | 5 +- ddprof-lib/src/main/cpp/jvmSupport.inline.h | 7 +- ddprof-lib/src/main/cpp/mutex.cpp | 4 + ddprof-lib/src/main/cpp/mutex.h | 2 + ddprof-lib/src/main/cpp/profiler.cpp | 93 ++++ ddprof-lib/src/main/cpp/profiler.h | 6 + ddprof-lib/src/main/cpp/vmEntry.cpp | 10 +- ddprof-lib/src/test/cpp/arguments_ut.cpp | 37 ++ .../src/test/cpp/hotspotMethodId_ut.cpp | 374 +++++++++---- ddprof-test/build.gradle.kts | 3 +- .../cpu/ContextClassLoaderUnloadTest.java | 301 +++++++++++ .../profiler/cpu/LoaderLocalWorker.java | 63 +++ .../ContendedWallclockSamplesTest.java | 2 + utils/run-containers-tests.sh | 4 + 29 files changed, 2189 insertions(+), 447 deletions(-) create mode 100644 build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestPlugin.kt.bak create mode 100644 build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt.bak create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/cpu/LoaderLocalWorker.java diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestPlugin.kt.bak b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestPlugin.kt.bak new file mode 100644 index 0000000000..409a705153 --- /dev/null +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestPlugin.kt.bak @@ -0,0 +1,266 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + + +package com.datadoghq.native.gtest + +import com.datadoghq.native.NativeBuildExtension +import com.datadoghq.native.model.BuildConfiguration +import com.datadoghq.native.model.Platform +import com.datadoghq.native.util.PlatformUtils +import org.gradle.api.Plugin +import org.gradle.api.Project +import java.io.File + +/** + * Gradle plugin for Google Test integration in C++ projects. + * + * This plugin automatically creates compilation, linking, and execution tasks for Google Test + * tests across multiple build configurations. It handles platform-specific differences (macOS, Linux) + * and integrates with the NativeBuildPlugin's BuildConfiguration model. + * + * For each test file in the test source directory, the plugin creates: + * - compileGtest{Config}_{TestName} - Compile all main sources + test file + * - linkGtest{Config}_{TestName} - Link test executable with gtest libraries + * - gtest{Config}_{TestName} - Execute the test + * + * Aggregation tasks: + * - gtest{Config} - Run all tests for a specific configuration (e.g., gtestDebug) + * - gtest - Run all tests across all configurations + * + * Usage: + * ```kotlin + * plugins { + * id("com.datadoghq.native-build") + * id("com.datadoghq.gtest") + * } + * + * gtest { + * testSourceDir.set(layout.projectDirectory.dir("src/test/cpp")) + * mainSourceDir.set(layout.projectDirectory.dir("src/main/cpp")) + * includes.from("src/main/cpp", "${javaHome}/include") + * } + * ``` + */ +class GtestPlugin : Plugin { + + override fun apply(project: Project) { + // Create extension + val extension = project.extensions.create("gtest", GtestExtension::class.java) + + // Register tasks after project evaluation + project.afterEvaluate { + // Check if testSourceDir is set + if (!extension.testSourceDir.isPresent) { + project.logger.warn("WARNING: gtest.testSourceDir not configured - skipping Google Test tasks") + return@afterEvaluate + } + + // Get configurations from NativeBuildExtension + val nativeBuildExtension = project.extensions.findByType(NativeBuildExtension::class.java) + if (nativeBuildExtension == null) { + project.logger.warn("WARNING: NativeBuildExtension not found - apply com.datadoghq.native-build plugin first") + return@afterEvaluate + } + + val activeConfigs = nativeBuildExtension.getActiveConfigurations( + PlatformUtils.currentPlatform, + PlatformUtils.currentArchitecture + ) + + if (activeConfigs.isEmpty()) { + project.logger.warn("WARNING: No active build configurations - skipping Google Test tasks") + return@afterEvaluate + } + + // Check if gtest is available + val hasGtest = PlatformUtils.isGtestAvailable() + if (!hasGtest) { + project.logger.warn("WARNING: Google Test not found - skipping native tests") + } + + // Create buildNativeLibs task (Linux only) + if (extension.buildNativeLibs.get()) { + createBuildNativeLibsTask(project, extension, hasGtest) + } + + // Create master aggregation task + val gtestAll = createMasterAggregationTask(project, hasGtest) + + // Create tasks for each active configuration + activeConfigs.forEach { config -> + createConfigTasks(project, extension, config, hasGtest, gtestAll) + } + } + } + + private fun createBuildNativeLibsTask(project: Project, extension: GtestExtension, hasGtest: Boolean) { + project.tasks.register("buildNativeLibs") { + group = "build" + description = "Build the native libs for the Google Tests" + + onlyIf { + hasGtest && + !PlatformUtils.isNativeSkipped(project) && + !project.hasProperty("skip-gtest") && + PlatformUtils.currentPlatform == Platform.LINUX && + extension.nativeLibsSourceDir.isPresent && + extension.nativeLibsOutputDir.isPresent + } + + val srcDir = if (extension.nativeLibsSourceDir.isPresent) { + extension.nativeLibsSourceDir.get().asFile + } else { + project.file("src/test/resources/native-libs") + } + val targetDir = if (extension.nativeLibsOutputDir.isPresent) { + extension.nativeLibsOutputDir.get().asFile + } else { + project.file("build/test/resources/native-libs") + } + + doLast { + if (!srcDir.exists()) { + project.logger.info("Native libs source directory does not exist: $srcDir") + return@doLast + } + + srcDir.listFiles()?.filter { it.isDirectory }?.forEach { dir -> + val libName = dir.name + val libDir = File("$targetDir/$libName") + val libSrcDir = File("$srcDir/$libName") + + // Use ProcessBuilder directly (Gradle 9 removed project.exec in task actions) + val process = ProcessBuilder("sh", "-c", """ + echo "Processing library: $libName @ $libSrcDir" + mkdir -p $libDir + cd $libSrcDir + make TARGET_DIR=$libDir + """.trimIndent()) + .inheritIO() + .start() + val exitCode = process.waitFor() + if (exitCode != 0) { + throw org.gradle.api.GradleException("Failed to build native lib: $libName (exit code: $exitCode)") + } + } + } + + inputs.files(project.fileTree(srcDir)) + outputs.dir(targetDir) + } + } + + private fun createMasterAggregationTask(project: Project, hasGtest: Boolean): org.gradle.api.tasks.TaskProvider<*> { + return project.tasks.register("gtest") { + group = "verification" + description = "Run all Google Tests for all build configurations of the library" + + onlyIf { + hasGtest && + !project.hasProperty("skip-tests") && + !PlatformUtils.isNativeSkipped(project) && + !project.hasProperty("skip-gtest") + } + } + } + + private fun createConfigTasks( + project: Project, + extension: GtestExtension, + config: BuildConfiguration, + hasGtest: Boolean, + gtestAll: org.gradle.api.tasks.TaskProvider<*> + ) { + // Find compiler and build include paths + val compiler = findCompiler(project) + val includeFiles = extension.includes.plus(project.files(getGtestIncludes(extension))) + + // Create per-config aggregation task (compile + link + run) + val gtestConfigTask = project.tasks.register("gtest${config.capitalizedName()}") { + group = "verification" + description = "Run all Google Tests for the ${config.name} build of the library" + } + + // Per-config build-only aggregation task (compile + link, no run). + // Useful in CI where binaries are executed directly without going + // through Gradle's logging infrastructure. + val buildGtestConfigTask = project.tasks.register("buildGtest${config.capitalizedName()}") { + group = "build" + description = "Compile and link all Google Tests for the ${config.name} build (no run)" + // The binaries load the helper DSOs at run time, and CI runs them + // straight from this task's output, so build the DSOs here too. + if (PlatformUtils.currentPlatform == Platform.LINUX && extension.buildNativeLibs.get()) { + project.tasks.findByName("buildNativeLibs")?.let { dependsOn(it) } + } + } + + // Compile all library sources ONCE for this config. Each test + // binary only compiles its own test file and links against these + // shared objects, reducing compilations from O(n_tests × n_sources) + // to O(n_sources + n_tests). + val sharedBuilder = GtestTaskBuilder(project, extension, config) + .withCompiler(compiler) + .withIncludes(includeFiles) + .onlyIfGtest(hasGtest) + val sharedCompilerArgs = sharedBuilder.sharedCompilerArgs() + val sharedLibCompileTask = project.tasks.register( + "compileGtestLibrary${config.capitalizedName()}", + com.datadoghq.native.tasks.NativeCompileTask::class.java + ) { + onlyIf { hasGtest && !sharedBuilder.skipConditions() } + group = "build" + description = "Compile shared library sources for ${config.name} gtest binaries" + + this.compiler.set(compiler) + this.compilerArgs.set(sharedCompilerArgs) + sources.from(project.fileTree(extension.mainSourceDir.get()) { include("**/*.cpp") }) + includes.from(includeFiles) + objectFileDir.set(project.file( + "${project.layout.buildDirectory.get()}/obj/gtest/${config.name}/lib" + )) + } + + // Discover and create tasks for each test file using builder + val testDir = extension.testSourceDir.get().asFile + if (!testDir.exists()) { + project.logger.info("Test source directory does not exist: $testDir") + return + } + + testDir.listFiles()?.filter { it.name.endsWith(".cpp") }?.forEach { testFile -> + val taskBundle = GtestTaskBuilder(project, extension, config) + .forTest(testFile) + .withCompiler(compiler) + .withIncludes(includeFiles) + .withSharedLibObjects(sharedLibCompileTask) + .onlyIfGtest(hasGtest) + .build() + + gtestConfigTask.configure { dependsOn(taskBundle) } + gtestAll.configure { dependsOn(taskBundle) } + // buildGtest depends on the link task, not the run task + buildGtestConfigTask.configure { + dependsOn("linkGtest${config.capitalizedName()}_${testFile.nameWithoutExtension}") + } + } + } + + private fun findCompiler(project: Project): String = PlatformUtils.findCompiler(project) + + private fun getGtestIncludes(extension: GtestExtension): List { + return when (PlatformUtils.currentPlatform) { + Platform.MACOS -> { + val gtestPath = if (extension.googleTestHome.isPresent) { + extension.googleTestHome.get().asFile.absolutePath + } else { + "/opt/homebrew/opt/googletest" + } + listOf(File("$gtestPath/include")) + } + Platform.LINUX -> emptyList() // System includes + } + } +} diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt.bak b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt.bak new file mode 100644 index 0000000000..2ce3889cee --- /dev/null +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt.bak @@ -0,0 +1,495 @@ + +package com.datadoghq.native.util + +import com.datadoghq.native.model.Architecture +import com.datadoghq.native.model.Platform +import org.gradle.api.GradleException +import org.gradle.api.Project +import java.io.File +import kotlin.io.path.createTempFile +import kotlin.io.path.deleteIfExists +import kotlin.io.path.writeText +import java.util.concurrent.TimeUnit + +object PlatformUtils { + val currentPlatform: Platform by lazy { Platform.current() } + val currentArchitecture: Architecture by lazy { Architecture.current() } + + fun isMusl(): Boolean { + if (currentPlatform != Platform.LINUX) { + return false + } + + // Check if running on musl libc by scanning /lib/ld-musl-*.so.1 + return File("/lib").listFiles()?.any { + it.name.startsWith("ld-musl-") && it.name.endsWith(".so.1") + } ?: false + } + + fun javaHome(): String { + return System.getenv("JAVA_HOME") + ?: System.getProperty("java.home") + ?: throw IllegalStateException("Neither JAVA_HOME environment variable nor java.home system property is set") + } + + /** + * Resolve JAVA_HOME for test execution, preferring JAVA_TEST_HOME if set. + * This allows running tests with a different JDK than the build JDK. + */ + fun testJavaHome(): String { + return System.getenv("JAVA_TEST_HOME") + ?: System.getenv("JAVA_HOME") + ?: System.getProperty("java.home") + ?: throw IllegalStateException("Neither JAVA_TEST_HOME, JAVA_HOME, nor java.home is set") + } + + /** + * Get the java executable path for test execution. + */ + fun testJavaExecutable(): String { + return "${testJavaHome()}/bin/java" + } + + fun jniIncludePaths(): List { + val javaHome = javaHome() + val platform = when (currentPlatform) { + Platform.LINUX -> "linux" + Platform.MACOS -> "darwin" + } + return listOf( + "$javaHome/include", + "$javaHome/include/$platform" + ) + } + + /** + * Google Test install locations probed by the gtest plugin (and by the coverage + * preflight check, since a missing gtest silently drops the gtest half of coverage). + */ + val gtestLocations: List + get() = when (currentPlatform) { + Platform.MACOS -> listOf( + "/opt/homebrew/opt/googletest", + "/usr/local/opt/googletest" + ) + Platform.LINUX -> listOf( + "/usr/include/gtest", + "/usr/local/include/gtest" + ) + } + + fun isGtestAvailable(): Boolean = gtestLocations.any { File(it).exists() } + + /** + * Check if a compiler is available and functional + */ + fun isCompilerAvailable(compiler: String): Boolean { + return try { + val process = ProcessBuilder(compiler, "--version") + .redirectErrorStream(true) + .start() + // Generous beyond the obvious: on a loaded macOS host the /usr/bin/clang++ Xcode + // shim can take several seconds to answer --version (Rosetta translation churn, + // oahd-helper at 100% CPU), and a 5s budget made this probe fail intermittently + // from invocations that accepted the same path minutes earlier. + process.waitFor(30, TimeUnit.SECONDS) + process.exitValue() == 0 + } catch (e: Exception) { + false + } + } + + /** + * Locate a library using compiler's -print-file-name. + * Uses the specified compiler, falling back to gcc if not available. + */ + fun locateLibrary(libName: String, compiler: String = "gcc"): String? { + if (currentPlatform != Platform.LINUX) { + return null + } + + // Try the specified compiler first + if (isCompilerAvailable(compiler)) { + try { + val process = ProcessBuilder(compiler, "-print-file-name=$libName.so") + .redirectErrorStream(true) + .start() + + val output = process.inputStream.bufferedReader().readText().trim() + process.waitFor() + + if (process.exitValue() == 0 && output != "$libName.so") { + return output + } + } catch (e: Exception) { + // Fall through to try gcc + } + } + + // If the specified compiler didn't find it, try gcc as fallback + if (compiler != "gcc" && isCompilerAvailable("gcc")) { + try { + val process = ProcessBuilder("gcc", "-print-file-name=$libName.so") + .redirectErrorStream(true) + .start() + + val output = process.inputStream.bufferedReader().readText().trim() + process.waitFor() + + if (process.exitValue() == 0 && output != "$libName.so") { + return output + } + } catch (e: Exception) { + // Fall through to return null + } + } + + return null + } + + fun locateLibasan(compiler: String = "gcc"): String? { + if (currentPlatform != Platform.LINUX) return null + // For clang, prefer the architecture-specific clang_rt.asan library over + // GCC's libasan. Using GCC's runtime alongside clang's libclang_rt.asan + // (which -fsanitize=address links for executables) causes "incompatible + // ASan runtimes" at startup. The clang runtime also includes UBSan symbols, + // so no separate -lubsan is needed. + if (compiler.contains("clang")) { + val archSuffix = when (currentArchitecture) { + Architecture.X64 -> "x86_64" + Architecture.ARM64 -> "aarch64" + } + val clangAsan = locateLibrary("libclang_rt.asan-$archSuffix", compiler) + if (clangAsan != null) return clangAsan + } + return locateLibrary("libasan", compiler) + } + + fun locateLibtsan(compiler: String = "gcc"): String? { + if (currentPlatform != Platform.LINUX) return null + // For clang, prefer the architecture-specific clang_rt.tsan library over + // GCC's libtsan. GCC 11's libtsan only handles 39-bit VMA on aarch64; + // clang's compiler-rt tsan handles both 39-bit and 48-bit VMA. + if (compiler.contains("clang")) { + val archSuffix = when (currentArchitecture) { + Architecture.X64 -> "x86_64" + Architecture.ARM64 -> "aarch64" + } + val clangTsan = locateLibrary("libclang_rt.tsan-$archSuffix", compiler) + if (clangTsan != null) return clangTsan + } + return locateLibrary("libtsan", compiler) + } + + fun checkFuzzerSupport(): Boolean { + return try { + val testFile = createTempFile("fuzzer_check", ".cpp") + val outFile = createTempFile("fuzzer_check", "") + // Remove the pre-created output file so the compiler writes its own binary + outFile.deleteIfExists() + try { + testFile.writeText("extern \"C\" int LLVMFuzzerTestOneInput(const char*, long) { return 0; }") + + // Link (not just compile) to catch missing libclang_rt.fuzzer_osx.a on Apple clang + val process = ProcessBuilder( + "clang++", + "-fsanitize=fuzzer", + testFile.toAbsolutePath().toString(), + "-o", + outFile.toAbsolutePath().toString() + ).redirectErrorStream(true).start() + + if (!process.waitFor(30, TimeUnit.SECONDS)) { + process.destroyForcibly() + process.waitFor(5, TimeUnit.SECONDS) + return false + } + process.exitValue() == 0 + } finally { + testFile.deleteIfExists() + outFile.deleteIfExists() + } + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + false + } catch (e: Exception) { + false + } + } + + fun hasAsan(compiler: String = "gcc"): Boolean { + return !isMusl() && locateLibasan(compiler) != null + } + + fun hasTsan(compiler: String = "gcc"): Boolean { + return !isMusl() && locateLibtsan(compiler) != null + } + + fun hasFuzzer(): Boolean { + return !isMusl() && checkFuzzerSupport() + } + + fun sharedLibExtension(): String = when (currentPlatform) { + Platform.LINUX -> "so" + Platform.MACOS -> "dylib" + } + + /** + * Architecture targeted by the JVM running the build. + * + * This is used as a Gradle task input on every platform. Native compiler options remain + * platform-specific: macOS accepts a portable {@code -arch} flag, whereas Linux cross + * compilation requires a configured cross compiler (and usually a sysroot). + */ + fun targetArchitecture(): String = currentArchitecture.toString() + + /** + * Returns true unless the given compiler/linker driver is GCC-family (`gcc`, `g++`, or a + * versioned variant like `g++-13`), which does not accept Apple Clang's {@code -arch} flag. + * Anything else (`clang++`, `c++`, `cc`, or a custom path) is treated as clang-like. + */ + private fun isClangLikeDriver(driver: String): Boolean { + val name = File(driver).name + return !Regex("""^(gcc|g\+\+)(-\d+(\.\d+)*)?$""").matches(name) + } + + /** + * Forces Apple Clang to produce objects for the JVM's architecture. + * + * The compiler process may run through Rosetta even when Gradle and the JVM run natively on + * Apple Silicon. Without an explicit target, Apple Clang follows the compiler process + * architecture instead, producing objects that cannot be linked with native dependencies for + * the JVM architecture. This is intentionally macOS-only: {@code -arch} is an Apple Clang + * option; Linux cross compilation must be configured with its own target compiler and sysroot. + * GCC (via {@code -Pnative.forceCompiler}) doesn't accept {@code -arch}, so the flag is + * skipped when {@code driver} resolves to a GCC-family executable. + */ + fun macosArchitectureArgs(driver: String): List { + return macosArchitectureArgsFor(currentPlatform, currentArchitecture, driver) + } + + /** + * Platform/architecture-parameterized implementation of [macosArchitectureArgs], split out so + * it can be unit tested independently of the host OS running the build. + */ + internal fun macosArchitectureArgsFor( + platform: Platform, + architecture: Architecture, + driver: String + ): List { + if (platform != Platform.MACOS || !isClangLikeDriver(driver)) { + return emptyList() + } + + val archFlag = when (architecture) { + Architecture.X64 -> "x86_64" + Architecture.ARM64 -> "arm64" + else -> throw GradleException( + "Unsupported macOS native build architecture: $architecture" + ) + } + return listOf("-arch", archFlag) + } + + /** + * Find Homebrew LLVM installation on macOS. + * Returns the LLVM installation path or null if not found. + */ + fun findHomebrewLLVM(): String? { + if (currentPlatform != Platform.MACOS) { + return null + } + + val possiblePaths = listOf( + "/opt/homebrew/opt/llvm", // Apple Silicon + "/usr/local/opt/llvm" // Intel Mac + ) + + for (path in possiblePaths) { + if (File(path).exists() && File("$path/bin/clang++").exists()) { + return path + } + } + + // Try using brew command + return try { + val process = ProcessBuilder("brew", "--prefix", "llvm") + .redirectErrorStream(true) + .start() + process.waitFor(5, TimeUnit.SECONDS) + if (process.exitValue() == 0) { + val brewPath = process.inputStream.bufferedReader().readText().trim() + if (File("$brewPath/bin/clang++").exists()) { + brewPath + } else { + null + } + } else { + null + } + } catch (e: Exception) { + null + } + } + + /** + * Find the clang resource directory within an LLVM installation. + * This is needed for locating libFuzzer on macOS with Homebrew LLVM. + */ + fun findClangResourceDir(llvmPath: String?): String? { + if (llvmPath == null) { + return null + } + + val clangLibDir = File("$llvmPath/lib/clang") + if (!clangLibDir.exists()) { + return null + } + + // Find the version directory (e.g., 18.1.8 or 19) + val versions = clangLibDir.listFiles() + ?.filter { it.isDirectory } + ?.sortedByDescending { it.name } + + return if (versions != null && versions.isNotEmpty()) { + "$llvmPath/lib/clang/${versions[0].name}" + } else { + null + } + } + + /** + * Find a compiler suitable for fuzzing. + * On macOS, prefers Homebrew LLVM's clang++ for libFuzzer support. + */ + fun findFuzzerCompiler(project: Project): String { + if (currentPlatform == Platform.MACOS) { + val homebrewLLVM = findHomebrewLLVM() + if (homebrewLLVM != null) { + return "$homebrewLLVM/bin/clang++" + } + } + return findCompiler(project) + } + + /** + * Detect the installed clang-format version. + * Returns null if clang-format is not available. + */ + fun clangFormatVersion(): String? { + return try { + val process = ProcessBuilder("clang-format", "--version").start() + process.waitFor(5, TimeUnit.SECONDS) + if (process.exitValue() == 0) { + val output = process.inputStream.bufferedReader().readText().trim() + val match = Regex("""clang-format version (\d+\.\d+\.\d+)""").find(output) + match?.groupValues?.get(1) + } else { + null + } + } catch (e: Exception) { + null + } + } + + /** + * Whether native compilation should be skipped for [project]. + * + * `-Pskip-native` (bare, no value) or `skip-native=true` (the form + * gradle.properties.template documents) skips it everywhere — the + * long-standing behavior, used when consuming a prebuilt shipped library + * via -Pwith-libs and never touching a compiler. `skip-native=false` skips + * nothing. + * + * `-Pskip-native=` skips it only for those + * projects. This lets a caller substitute the prebuilt *shipped* library + * (ddprof-lib) while still compiling something unrelated to its ABI, like + * ddprof-test-native's small JNI test helper, which several ddprof-test + * suites need on java.library.path regardless of which profiler binary + * is under test. + */ + fun isNativeSkipped(project: Project): Boolean { + if (!project.hasProperty("skip-native")) { + return false + } + val value = (project.property("skip-native") as? String)?.trim() + if (value.isNullOrEmpty() || value.equals("true", ignoreCase = true)) { + return true + } + if (value.equals("false", ignoreCase = true)) { + return false + } + return value.split(",").map { it.trim() }.contains(project.name) + } + + /** + * Find a C++ compiler, respecting -Pnative.forceCompiler property. + * Auto-detects clang++ or g++ if not specified. + */ + fun findCompiler(project: Project): String { + // Check for forced compiler override + val forcedCompiler = project.findProperty("native.forceCompiler") as? String + if (forcedCompiler != null) { + if (isCompilerAvailable(forcedCompiler)) { + project.logger.info("Using forced compiler: $forcedCompiler") + return forcedCompiler + } + throw GradleException( + "Forced compiler '$forcedCompiler' is not available. " + + "Verify the path or remove -Pnative.forceCompiler to auto-detect." + ) + } + + // Auto-detect: prefer clang++, then g++, then c++ + val compilers = listOf("clang++", "g++", "c++") + for (compiler in compilers) { + if (isCompilerAvailable(compiler)) { + project.logger.info("Auto-detected compiler: $compiler") + return compiler + } + } + + throw GradleException( + "No C++ compiler found. Please install clang++ or g++, " + + "or specify one with -Pnative.forceCompiler=/path/to/compiler" + ) + } + + /** + * Returns the major version of the test JVM (e.g. 8, 11, 17, 21, 25). + * Returns 0 if the version cannot be determined. + */ + fun testJvmMajorVersion(): Int { + val javaHome = testJavaHome() + return try { + val process = ProcessBuilder("$javaHome/bin/java", "-version") + .redirectErrorStream(true) + .start() + val output = process.inputStream.bufferedReader().readText() + process.waitFor(10, TimeUnit.SECONDS) + val match = Regex("""version "(?:1\.)?(\d+)""").find(output) + match?.groupValues?.get(1)?.toIntOrNull() ?: 0 + } catch (_: Exception) { + 0 + } + } + + /** + * Returns true if the test JVM (from JAVA_TEST_HOME or JAVA_HOME) is an OpenJ9/J9 JVM. + * Probes `java -version` stderr output for "J9" or "OpenJ9". + */ + fun isTestJvmJ9(): Boolean { + val javaHome = testJavaHome() + return try { + val process = ProcessBuilder("$javaHome/bin/java", "-version") + .redirectErrorStream(true) + .start() + val output = process.inputStream.bufferedReader().readText() + process.waitFor(10, TimeUnit.SECONDS) + output.contains("J9") || output.contains("OpenJ9") + } catch (_: Exception) { + false + } + } + +} diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt b/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt index c3faa324bf..12fd4a3c9d 100644 --- a/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt @@ -264,6 +264,12 @@ class ProfilerTestPlugin : Plugin { // Configure Java executable - bypasses toolchain system testTask.setExecutable(PlatformUtils.testJavaExecutable()) + // Run each test class in a fresh JVM: the profiler's native state + // (started engines, preloaded jmethodIDs, patched libraries, counters) + // outlives a single test and otherwise leaks into the next class. + // Gradle forks per class at most; methods of one class share a JVM. + testTask.setForkEvery(1) + // Environment variables (from testConfig which already includes DDPROF_TEST_DISABLE_RATE_LIMIT and CI) testConfig.environmentVariables.forEach { (key, value) -> testTask.environment(key, value) @@ -325,12 +331,9 @@ class ProfilerTestPlugin : Plugin { !PlatformUtils.isTestJvmJ9() } // The ASAN test JVM is pinned to -Xmx1024m (see ConfigurationPresets.kt) - // to keep the heap below ASan's shadow-memory region. Without forking, - // Gradle reuses one JVM for the whole suite, and per-test allocations - // (JFR recordings, JMC object models) accumulate until it OOMs late in - // the run even though every individual test passes. Restart periodically - // to bound cumulative heap growth. - testTask.setForkEvery(25) + // to keep the heap below ASan's shadow-memory region; forkEvery(1) + // above keeps per-test allocations (JFR recordings, JMC object models) + // from accumulating across the suite until it OOMs. } // TSan + JVM integration tests are incompatible: the profiler's signal // handlers (SIGPROF at 1ms) are TSan-instrumented; when a signal fires diff --git a/ddprof-lib/src/main/cpp/arguments.cpp b/ddprof-lib/src/main/cpp/arguments.cpp index 82db261aa5..a1ea23d1c5 100644 --- a/ddprof-lib/src/main/cpp/arguments.cpp +++ b/ddprof-lib/src/main/cpp/arguments.cpp @@ -161,6 +161,7 @@ Error Arguments::parse(const char *args) { char *args_copy = strcpy(_buf + EXTRA_BUF_SIZE, args); const char *msg = NULL; + bool force_jmethodID_set = false; for (char *arg = strtok(args_copy, ","); arg != NULL; arg = strtok(NULL, ",")) { @@ -302,6 +303,7 @@ Error Arguments::parse(const char *args) { CASE("fjmethodid") if (value != nullptr) { + force_jmethodID_set = true; if (strcmp(value, "false") == 0) { _force_jmethodID = false; } else if (strcmp(value, "true") == 0) { @@ -517,6 +519,13 @@ Error Arguments::parse(const char *args) { _event = EVENT_CPU; } + // Without an explicit fjmethodid, only the VMStructs walker (cstack=vm/vmx, + // and default, which resolves to vm where supported) can store a raw Method* + // for a frame without a jmethodID; every other mode needs them preloaded. + if (!force_jmethodID_set) { + _force_jmethodID = !(_cstack == CSTACK_VM || _cstack == CSTACK_DEFAULT); + } + if (VM::isOpenJ9()) { if (_cstack == CSTACK_FP) { // J9 is compiled without FP diff --git a/ddprof-lib/src/main/cpp/arguments.h b/ddprof-lib/src/main/cpp/arguments.h index d6222f9157..8c931f2ca2 100644 --- a/ddprof-lib/src/main/cpp/arguments.h +++ b/ddprof-lib/src/main/cpp/arguments.h @@ -273,7 +273,7 @@ class Arguments { bool _jvmtistacks; // Delegate CPU/wall stack walks to HotSpot JFR RequestStackTrace extension bool _nativesocket; long _nativesocket_interval; // initial sampling period in nanoseconds; 0 = engine default - bool _force_jmethodID; // Load all jmethodIDs, true by default + bool _force_jmethodID; // Load all jmethodIDs; defaults to false for cstack=vm/vmx/default, true otherwise Arguments(bool persistent = false) : _buf(NULL), @@ -324,7 +324,7 @@ class Arguments { _jvmtistacks(false), _nativesocket(false), _nativesocket_interval(0), - _force_jmethodID(true) {} + _force_jmethodID(false) {} ~Arguments(); diff --git a/ddprof-lib/src/main/cpp/callTraceStorage.h b/ddprof-lib/src/main/cpp/callTraceStorage.h index a091b54522..b05b90bae2 100644 --- a/ddprof-lib/src/main/cpp/callTraceStorage.h +++ b/ddprof-lib/src/main/cpp/callTraceStorage.h @@ -96,6 +96,10 @@ class CallTraceStorage { // Register a liveness checker (rare operation - uses simple lock) void registerLivenessChecker(LivenessChecker checker); + bool hasLivenessCheckers() { + SharedLockGuard lock(&_liveness_lock); + return !_liveness_checkers.empty(); + } // Clear liveness checkers (rare operation - uses simple lock) void clearLivenessCheckers(); diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index 0c55316899..ec6042440a 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -165,11 +165,15 @@ /* Every siglongjmp recovery, from any protected window, counted centrally \ * in Profiler::checkFault(). */ \ X(STACKWALK_LONGJMP_RECOVERED, "stackwalk_longjmp_recovered") \ + /* JFR chunks written from GarbageCollectionStart (Profiler::onGCStart()), \ + * and GCs that skipped it because a lock was taken. */ \ + X(GC_CHECKPOINTS, "gc_checkpoints") \ + X(GC_CHECKPOINTS_SKIPPED, "gc_checkpoints_skipped") \ /* Dump-time method resolution failures. The frame serializes as "unknown". */ \ X(METHOD_RESOLVE_FAULT_RECOVERED, "method_resolve_fault_recovered") \ /* Symbol length/body rejected during the same resolution: unreadable body, \ * empty (recycled slot), or over MAX_SYMBOL_LEN. A name that merely exceeds \ - * the fixed inline buffers in hotspotSupport.cpp's ResolvedNames still \ + * the fixed inline buffers in hotspotSupport.h's ResolvedMethod still \ * resolves via its malloc fallback and does not land here. The frame \ * serializes as "unknown". */ \ X(METHOD_RESOLVE_SYMBOL_UNREADABLE, "method_resolve_symbol_unreadable") \ diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 98455483d8..51d8535519 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -86,7 +86,11 @@ SharedLineNumberTable::~SharedLineNumberTable() { // whenever -XX:NativeMemoryTracking is enabled, because native memory tracking // adds a header to malloc'd block. if (_table != nullptr) { - VM::jvmti()->Deallocate((unsigned char *)_table); + if (_jvmti_allocated) { + VM::jvmti()->Deallocate((unsigned char *)_table); + } else { + free(_table); + } Counters::decrement(LINE_NUMBER_TABLES); // _size is the JVMTI entry count passed at construction (see // fillJavaMethodInfo), so the byte size matches the allocation. @@ -266,9 +270,6 @@ bool Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method, char* volatile &class_name = state._class_name; char* volatile &method_name = state._method_name; char* volatile &method_sig = state._method_signature; - u32 class_name_id = 0; - u32 method_name_id = 0; - u32 method_sig_id = 0; jint line_number_table_size = 0; jvmtiLineNumberEntry *line_number_table = NULL; @@ -385,95 +386,15 @@ bool Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method, entry = true; } - // maybe we should store the lookups below in initialisation-time - // constants... - if (has_prefix(class_name, - "Ljdk/internal/reflect/GeneratedConstructorAccessor")) { - class_name_id = _classes->lookupDuringDump( - "jdk/internal/reflect/GeneratedConstructorAccessor", - strlen("jdk/internal/reflect/GeneratedConstructorAccessor"), - Profiler::maxClassMapSize()); - method_name_id = - _symbols.lookup("Object " - "jdk.internal.reflect.GeneratedConstructorAccessor." - "newInstance(Object[])"); - method_sig_id = _symbols.lookup(method_sig); - } else if (has_prefix(class_name, - "Lsun/reflect/GeneratedConstructorAccessor")) { - class_name_id = _classes->lookupDuringDump( - "sun/reflect/GeneratedConstructorAccessor", - strlen("sun/reflect/GeneratedConstructorAccessor"), - Profiler::maxClassMapSize()); - method_name_id = _symbols.lookup( - "Object " - "sun.reflect.GeneratedConstructorAccessor.newInstance(Object[])"); - method_sig_id = _symbols.lookup(method_sig); - } else if (has_prefix(class_name, - "Ljdk/internal/reflect/GeneratedMethodAccessor")) { - class_name_id = _classes->lookupDuringDump( - "jdk/internal/reflect/GeneratedMethodAccessor", - strlen("jdk/internal/reflect/GeneratedMethodAccessor"), - Profiler::maxClassMapSize()); - method_name_id = - _symbols.lookup("Object " - "jdk.internal.reflect.GeneratedMethodAccessor." - "invoke(Object, Object[])"); - method_sig_id = _symbols.lookup(method_sig); - } else if (has_prefix(class_name, - "Lsun/reflect/GeneratedMethodAccessor")) { - class_name_id = _classes->lookupDuringDump( - "sun/reflect/GeneratedMethodAccessor", - strlen("sun/reflect/GeneratedMethodAccessor"), - Profiler::maxClassMapSize()); - method_name_id = _symbols.lookup( - "Object sun.reflect.GeneratedMethodAccessor.invoke(Object, " - "Object[])"); - method_sig_id = _symbols.lookup(method_sig); - } else if (has_prefix(class_name, "Ljava/lang/invoke/LambdaForm$")) { - const int lambdaFormPrefixLength = - strlen("Ljava/lang/invoke/LambdaForm$"); - // we want to normalise to java/lang/invoke/LambdaForm$MH, - // java/lang/invoke/LambdaForm$DMH, java/lang/invoke/LambdaForm$BMH, - if (has_prefix(class_name + lambdaFormPrefixLength, "MH")) { - class_name_id = _classes->lookupDuringDump( - "java/lang/invoke/LambdaForm$MH", - strlen("java/lang/invoke/LambdaForm$MH"), - Profiler::maxClassMapSize()); - } else if (has_prefix(class_name + lambdaFormPrefixLength, "BMH")) { - class_name_id = _classes->lookupDuringDump( - "java/lang/invoke/LambdaForm$BMH", - strlen("java/lang/invoke/LambdaForm$BMH"), - Profiler::maxClassMapSize()); - } else if (has_prefix(class_name + lambdaFormPrefixLength, "DMH")) { - class_name_id = _classes->lookupDuringDump( - "java/lang/invoke/LambdaForm$DMH", - strlen("java/lang/invoke/LambdaForm$DMH"), - Profiler::maxClassMapSize()); - } else { - // don't recognise the suffix, so don't normalise - class_name_id = _classes->lookupDuringDump( - normalized_class_name, normalized_class_name_len, - Profiler::maxClassMapSize()); - } - method_name_id = _symbols.lookup(method_name); - method_sig_id = _symbols.lookup(method_sig); - } else { - class_name_id = _classes->lookupDuringDump( - normalized_class_name, normalized_class_name_len, - Profiler::maxClassMapSize()); - method_name_id = _symbols.lookup(method_name); - method_sig_id = _symbols.lookup(method_sig); - } + fillJavaMethodNames(mi, normalized_class_name, normalized_class_name_len, + method_name, method_sig); } else { Counters::increment(JMETHODID_SKIPPED); - class_name_id = _classes->lookupDuringDump("", 0, Profiler::maxClassMapSize()); - method_name_id = _symbols.lookup(""); - method_sig_id = _symbols.lookup("()L;"); + mi->_class = _classes->lookupDuringDump("", 0, Profiler::maxClassMapSize()); + mi->_name = _symbols.lookup(""); + mi->_sig = _symbols.lookup("()L;"); } - mi->_class = class_name_id; - mi->_name = method_name_id; - mi->_sig = method_sig_id; mi->_type = FRAME_INTERPRETED; mi->_is_entry = entry; // Only touch mi->_line_number_table when a table was actually fetched @@ -501,6 +422,126 @@ bool Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method, return false; } +void Lookup::fillJavaMethodNames(MethodInfo *mi, const char *class_name, + size_t class_name_len, const char *method_name, + const char *method_sig) { + +// TEST_LOG("fillJavaMethodNames: %s, %s", method_name, method_sig); + + u32 class_name_id; + u32 method_name_id; + // maybe we should store the lookups below in initialisation-time + // constants... + if (has_prefix_n(class_name, class_name_len, + "jdk/internal/reflect/GeneratedConstructorAccessor")) { + class_name_id = _classes->lookupDuringDump( + "jdk/internal/reflect/GeneratedConstructorAccessor", + strlen("jdk/internal/reflect/GeneratedConstructorAccessor"), + Profiler::maxClassMapSize()); + method_name_id = + _symbols.lookup("Object " + "jdk.internal.reflect.GeneratedConstructorAccessor." + "newInstance(Object[])"); + } else if (has_prefix_n(class_name, class_name_len, + "sun/reflect/GeneratedConstructorAccessor")) { + class_name_id = _classes->lookupDuringDump( + "sun/reflect/GeneratedConstructorAccessor", + strlen("sun/reflect/GeneratedConstructorAccessor"), + Profiler::maxClassMapSize()); + method_name_id = _symbols.lookup( + "Object " + "sun.reflect.GeneratedConstructorAccessor.newInstance(Object[])"); + } else if (has_prefix_n(class_name, class_name_len, + "jdk/internal/reflect/GeneratedMethodAccessor")) { + class_name_id = _classes->lookupDuringDump( + "jdk/internal/reflect/GeneratedMethodAccessor", + strlen("jdk/internal/reflect/GeneratedMethodAccessor"), + Profiler::maxClassMapSize()); + method_name_id = + _symbols.lookup("Object " + "jdk.internal.reflect.GeneratedMethodAccessor." + "invoke(Object, Object[])"); + } else if (has_prefix_n(class_name, class_name_len, + "sun/reflect/GeneratedMethodAccessor")) { + class_name_id = _classes->lookupDuringDump( + "sun/reflect/GeneratedMethodAccessor", + strlen("sun/reflect/GeneratedMethodAccessor"), + Profiler::maxClassMapSize()); + method_name_id = _symbols.lookup( + "Object sun.reflect.GeneratedMethodAccessor.invoke(Object, " + "Object[])"); + } else if (has_prefix_n(class_name, class_name_len, + "java/lang/invoke/LambdaForm$")) { + const size_t lambdaFormPrefixLength = + strlen("java/lang/invoke/LambdaForm$"); + const char *suffix = class_name + lambdaFormPrefixLength; + const size_t suffix_len = class_name_len - lambdaFormPrefixLength; + // we want to normalise to java/lang/invoke/LambdaForm$MH, + // java/lang/invoke/LambdaForm$DMH, java/lang/invoke/LambdaForm$BMH, + if (has_prefix_n(suffix, suffix_len, "MH")) { + class_name_id = _classes->lookupDuringDump( + "java/lang/invoke/LambdaForm$MH", + strlen("java/lang/invoke/LambdaForm$MH"), + Profiler::maxClassMapSize()); + } else if (has_prefix_n(suffix, suffix_len, "BMH")) { + class_name_id = _classes->lookupDuringDump( + "java/lang/invoke/LambdaForm$BMH", + strlen("java/lang/invoke/LambdaForm$BMH"), + Profiler::maxClassMapSize()); + } else if (has_prefix_n(suffix, suffix_len, "DMH")) { + class_name_id = _classes->lookupDuringDump( + "java/lang/invoke/LambdaForm$DMH", + strlen("java/lang/invoke/LambdaForm$DMH"), + Profiler::maxClassMapSize()); + } else { + // don't recognise the suffix, so don't normalise + class_name_id = _classes->lookupDuringDump( + class_name, class_name_len, Profiler::maxClassMapSize()); + } + method_name_id = _symbols.lookup(method_name); + } else { + class_name_id = _classes->lookupDuringDump( + class_name, class_name_len, Profiler::maxClassMapSize()); + method_name_id = _symbols.lookup(method_name); + } + + mi->_class = class_name_id; + mi->_name = method_name_id; + mi->_sig = _symbols.lookup(method_sig); +} + +void Lookup::fillJavaMethodInfo(MethodInfo *mi, ResolvedMethod &resolved) { + const char *class_name = resolved.klassName(); + const char *method_name = resolved.methodName(); + const char *method_sig = resolved.methodSignature(); + + fillJavaMethodNames(mi, class_name, strlen(class_name), method_name, method_sig); + mi->_type = FRAME_INTERPRETED; + // Same entry-frame rules as the jmethodID path: Thread.run (or an override + // in a Thread subclass), or public static void main(String[]). + mi->_is_entry = resolved.isThreadRun() || + (strcmp(method_name, "main") == 0 && + strcmp(method_sig, "([Ljava/lang/String;)V") == 0); + + // A raw Method* key can be reused by a different method after its class is + // unloaded, so unlike the jmethodID path the table is refreshed on every + // fill rather than only the first time. + if (resolved.lineNumberTable() != nullptr) { + int size = resolved.lineNumberTableSize(); + // Built before ownership moves, so a std::bad_alloc here leaves the table + // with `resolved`, which frees it. + auto table = std::make_shared( + size, const_cast(resolved.lineNumberTable()), + false /* malloc'd */); + resolved.takeLineNumberTable(&size); + mi->_line_number_table = std::move(table); + Counters::increment(LINE_NUMBER_TABLES); + NativeMem::record(NM_LINE_TABLES, (long long)((size_t)size * sizeof(jvmtiLineNumberEntry))); + } else { + mi->_line_number_table = nullptr; + } +} + bool Lookup::resolveVTableReceiver(VMSymbol *sym, char *buf, size_t bufsize, u32 *out_class_id) { if (sym == nullptr || !SafeAccess::isReadable(sym)) { @@ -652,6 +693,7 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { jint bci = frame.bci; jmethodID method_id = frame.method_id; +// TEST_LOG("ResolveMethod: method_id = %zx", (void*)method_id); // HotSpot's VM stack walker uses this sentinel when it could not validate a // Method*. It is not a JNI/JVMTI jmethodID and must never reach // fillJavaMethodInfo(). Keep the frame structurally intact, but serialize it @@ -672,15 +714,16 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { return unknown_method; } - // Fast path, deliberately unprotected. For anything but a raw-pointer or - // BCI_VTABLE_RECEIVER frame, methodKey() reads no VM metadata (see its - // comment) and an already-marked row needs no symbolication -- there is + // Fast path, deliberately unprotected. For anything but a + // BCI_VTABLE_RECEIVER frame, methodKey() reads no VM metadata (raw-pointer + // frames are keyed by the Method* address itself) and an already-marked row + // needs no symbolication -- there is // nothing here that can fault, hence nothing to recover from. Worth // special-casing because it is the common case once a chunk is warm, and // arming the protection below is not free: initCurrentThreadSignalSafe() // blocks and unblocks signals and sigsetjmp(..., 1) reads the signal mask, // three syscalls on a loop that runs once per frame per trace. - if (!FrameType::isRawPointer(bci) && bci != BCI_VTABLE_RECEIVER) { + if (bci != BCI_VTABLE_RECEIVER) { MethodMap::iterator it = _method_map->find(methodKey(frame, method_id, bci, 0)); if (it != _method_map->end() && it->second._mark) { return &it->second; @@ -706,15 +749,15 @@ MethodInfo *Lookup::fillMethod(ASGCT_CallFrame &frame, jmethodID method_id, jint bci, ProfiledThread* const prof_thread) { assert(prof_thread != nullptr); - // Resolve native method - if (FrameType::isRawPointer(bci)) { - method_id = JVMSupport::resolve(frame.method); - if (method_id == nullptr) { - return unknownMethod(); - } - } - assert(method_id != nullptr && "Already filtered by caller"); + + // Raw Method*: symbolicate straight from VM metadata (no JNI/JVMTI) and key + // the MethodMap row by the Method* itself, which method_id already holds + // (ASGCT_CallFrame's method/method_id share storage). + // Declared before sigsetjmp() below so its landing pad can release it. + const bool raw_pointer = FrameType::isRawPointer(bci); + ResolvedMethod resolved; + // Reinstates the thread's previous landing pad on every exit from this frame, // including a std::bad_alloc thrown by one of the map or dictionary inserts // underneath. Leaving ours installed past the end of this frame would leave @@ -738,6 +781,9 @@ MethodInfo *Lookup::fillMethod(ASGCT_CallFrame &frame, jmethodID method_id, // state.release() may fault. Unfortunately, it faults outside of profiler // code where checkFault() can not absorb. state.release(); + // siglongjmp bypasses ~ResolvedMethod(); free a line number table that + // was not yet handed over to a MethodInfo. + resolved.release(); // A member, already filled above -- no map lookup, no allocation, and no // reliance on a local surviving siglongjmp (the value of a non-volatile // local assigned after sigsetjmp() is indeterminate here). @@ -816,6 +862,22 @@ MethodInfo *Lookup::fillMethod(ASGCT_CallFrame &frame, jmethodID method_id, mi->_sig = _symbols.lookup("()V"); mi->_type = FRAME_NATIVE; mi->_is_entry = false; + } else if (raw_pointer) { + // resolve() installs its own (nested) landing pad around the metadata + // walk; a stale Method* leaves this row unfilled, see below. + filled = JVMSupport::resolve(frame.method, resolved); + if (filled) { + fillJavaMethodInfo(mi, resolved); + } + } else if (_rec != nullptr && _rec->_vmstructs_only) { + // Inside a GC pause (Recording::checkpointChunk()): JVMTI is off limits, + // so read the jmethodID's Method* and resolve that like a raw frame. A + // jmethodID that can't be dereferenced leaves the row unfilled. + const VMMethod* method = VMMethod::fromJmethodID(method_id); + filled = method != nullptr && JVMSupport::resolve(method, resolved); + if (filled) { + fillJavaMethodInfo(mi, resolved); + } } else { filled = fillJavaMethodInfo(mi, method_id, first_time, state); } @@ -826,7 +888,8 @@ MethodInfo *Lookup::fillMethod(ASGCT_CallFrame &frame, jmethodID method_id, // unwind skips this statement (and everything below it) entirely. // 2. fillJavaMethodInfo() can return normally without touching mi at // all (PushLocalFrame failed, or the JVM isn't in JVMTI_PHASE_START/ - // JVMTI_PHASE_LIVE) -- control returns here just like the success + // JVMTI_PHASE_LIVE), or resolve() can reject a stale raw Method* + // -- control returns here just like the success // case, so ordering alone does NOT catch this one; its `filled` // return value does. // Marking (or allocating a key for) an unfilled row would leave it @@ -958,7 +1021,11 @@ off_t Recording::finishChunk(bool end_recording, bool do_cleanup) { // Note: this only guards against concurrent unloading that starts AFTER this // call. Classes already unloaded before finishChunk() was entered are not // present in the list and receive no protection here. - jvmtiError err = jvmti->GetLoadedClasses(&count, &classes); + // + // Skipped by checkpointChunk(): it runs inside a GC pause, where JVMTI may not + // be called and no class can be unloaded concurrently anyway. + jvmtiError err = _vmstructs_only ? JVMTI_ERROR_WRONG_PHASE + : jvmti->GetLoadedClasses(&count, &classes); flush(&_cpu_monitor_buf); @@ -1127,6 +1194,26 @@ void Recording::switchChunk(int fd) { flush(_buf); } +void Recording::checkpointChunk() { + _vmstructs_only = true; + _chunk_start = finishChunk(/*end_recording=*/false, /*do_cleanup=*/true); + _vmstructs_only = false; + + _start_time = _stop_time; + _start_ticks = _stop_ticks; + _bytes_written = 0; + + // Every chunk needs its own header, metadata and settings. Unlike + // switchChunk(), the file is not restarted, so the once-per-file info events + // stay in its first chunk -- writeJvmInfo() and writeSystemProperties() call + // JVMTI, which a GC callback may not -- and _recorded_lib_count carries on, + // so finishChunk() keeps writing only libraries loaded since. + writeHeader(_buf); + writeMetadata(_buf); + writeSettings(_buf, _args); + flush(_buf); +} + void Recording::cpuMonitorCycle() { if (!_cpu_monitor_enabled) return; @@ -2692,6 +2779,22 @@ Error FlightRecorder::dump(const char *filename, const int length) { return Error("No active recording"); } +bool FlightRecorder::tryCheckpointBegin() { + if (!_rec_lock.tryLock()) { + return false; + } + if (_rec == nullptr) { + _rec_lock.unlock(); + return false; + } + return true; +} + +void FlightRecorder::checkpointAndEnd() { + _rec->checkpointChunk(); + _rec_lock.unlock(); +} + void FlightRecorder::wallClockEpoch(int lock_index, WallClockEpochEvent *event) { OptionalSharedLockGuard locker(&_rec_lock); diff --git a/ddprof-lib/src/main/cpp/flightRecorder.h b/ddprof-lib/src/main/cpp/flightRecorder.h index bee39b415e..eefcb6ea3d 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.h +++ b/ddprof-lib/src/main/cpp/flightRecorder.h @@ -103,14 +103,18 @@ struct CpuTimes { class SharedLineNumberTable { public: int _size; - // The buffer jvmti->GetLineNumberTable() returned, held directly (not a - // copy) and freed via jvmti->Deallocate() (see ~SharedLineNumberTable()) - // so native-memory-tracking accounting stays correct. Per the JVMTI spec - // this array is a fresh, caller-owned allocation decoupled from the - // Method's lifetime, so holding onto it is safe across class unload. + // Either the buffer jvmti->GetLineNumberTable() returned, held directly (not + // a copy) and freed via jvmti->Deallocate() (see ~SharedLineNumberTable()) + // so native-memory-tracking accounting stays correct, or -- when + // _jvmti_allocated is false -- a malloc'd table decoded from VM metadata by + // HotspotSupport::resolve(), freed via free(). Either way the array is + // decoupled from the Method's lifetime, so holding onto it is safe across + // class unload. jvmtiLineNumberEntry* _table; + bool _jvmti_allocated; - SharedLineNumberTable(int size, jvmtiLineNumberEntry *ptr) : _size(size), _table(ptr) {} + SharedLineNumberTable(int size, jvmtiLineNumberEntry *ptr, bool jvmti_allocated = true) : + _size(size), _table(ptr), _jvmti_allocated(jvmti_allocated) {} ~SharedLineNumberTable(); }; @@ -307,6 +311,10 @@ class Recording { return value < 0 ? 0 : value > 1 ? 1 : value; } + // Set only for the duration of checkpointChunk(): finishChunk() skips the + // JVMTI class pinning, and Lookup resolves jmethodID frames through VMStructs. + bool _vmstructs_only = false; + public: Recording(int fd, Arguments &args); ~Recording(); @@ -316,6 +324,11 @@ class Recording { off_t finishChunk(bool end_recording, bool do_cleanup = false); void switchChunk(int fd); + // Finishes the current chunk in place and starts the next one in the same + // file, resolving every method from VM metadata alone (no JNI/JVMTI), so it + // can run inside the JVMTI GarbageCollectionStart callback -- while the + // classes the pending samples point at are still loaded. + void checkpointChunk(); void cpuMonitorCycle(); void appendRecording(const char *target_file, size_t size); @@ -450,6 +463,7 @@ class Recording { }; class ResolveMethodState; +class ResolvedMethod; class Lookup { public: Recording *_rec; @@ -492,6 +506,14 @@ class Lookup { // case. Every other path (including the "" stale-jmethodID // sentinel) fully populates mi and returns true. bool fillJavaMethodInfo(MethodInfo *mi, jmethodID method, bool first_time, ResolveMethodState& state); + // Fills mi from a raw Method* already resolved by HotspotSupport::resolve(), + // taking ownership of its line number table. No JNI/JVMTI. + void fillJavaMethodInfo(MethodInfo *mi, ResolvedMethod& resolved); + // Sets mi's class/name/signature ids, applying the synthetic-accessor and + // LambdaForm class name normalisation. class_name is an internal name + // ("java/lang/Object"), not necessarily NUL-terminated at class_name_len. + void fillJavaMethodNames(MethodInfo *mi, const char *class_name, size_t class_name_len, + const char *method_name, const char *method_sig); bool has_prefix(const char *str, const char *prefix) const { return strncmp(str, prefix, strlen(prefix)) == 0; } @@ -568,6 +590,12 @@ class FlightRecorder { Error start(Arguments &args, bool reset); void stop(); Error dump(const char *filename, const int length); + // Recording::checkpointChunk() on the active recording, split in two so the + // caller can do its own work in between. tryCheckpointBegin() never blocks: + // it returns true holding _rec_lock if there is a recording, and false + // holding nothing otherwise. checkpointAndEnd() writes and releases it. + bool tryCheckpointBegin(); + void checkpointAndEnd(); void wallClockEpoch(int lock_index, WallClockEpochEvent *event); void recordTraceRoot(int lock_index, int tid, TraceRootEvent *event); void recordQueueTime(int lock_index, int tid, QueueTimeEvent *event); diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotStackFrame.h b/ddprof-lib/src/main/cpp/hotspot/hotspotStackFrame.h index d33aedfb1f..cfce48bb57 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotStackFrame.h +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotStackFrame.h @@ -32,7 +32,7 @@ class HotspotStackFrame : public StackFrame { // anchor. Per the setjmp/longjmp rules (C11 7.13.2.1p3, inherited // by C++), a non-volatile automatic local modified in that window // has an indeterminate value after longjmp -- the same hazard - // ResolvedNames::_long_method_name (hotspotSupport.cpp) guards + // ResolvedMethod::_long_method_name (hotspotSupport.h) guards // against for an analogous fault-recovery readback. VMJavaFrameAnchor* volatile _anchor; const void* volatile _anchor_pc; diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index a811682936..ce9b7b4028 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -1453,99 +1453,23 @@ bool HotspotSupport::loadMethodIDsIfNeededImpl(jvmtiEnv *jvmti, JNIEnv *jni, jcl return JVMSupport::loadMethodIDsImpl(jvmti, jni, klass); } -// The three names resolve() needs, owned by resolve()'s frame. Each name has -// a fixed-size inline buffer for the common case, with a malloc'd fallback -// (up to MAX_SYMBOL_LEN) for names that don't fit -- see release() for why -// that fallback needs explicit cleanup on the crash-recovery path. -class ResolvedNames { - // Hard ceiling for the malloc fallback in setImpl(), independent of which - // field is being resolved. Not a JVM/class-file limit (Symbol::length() is - // a u2, so up to 65535 is legal) -- a name that would already have been rejected - // as too long for those fields' fixed buffers doesn't get an unbounded - // allocation just because it went through the malloc path instead. - static constexpr size_t MAX_SYMBOL_LEN = 64 * 1024; - - // Fixed-size fast-path buffers for the common case, sized generously above - // the realistic common-case length for each field (see the per-field - // comments below) but deliberately not so generous that the malloc - // fallback in setImpl() never engages -- these caps are meant to be hit - // occasionally so that path stays exercised. - // HotSpot's Symbol length field is a u2, so the VM permits up to 65535 bytes. - // A name/descriptor that overflows its fixed buffer falls back to malloc - // (see ResolvedNames::setImpl), up to MAX_SYMBOL_LEN below; beyond that it is - // rejected and the frame serializes as "unknown" -- METHOD_RESOLVE_SYMBOL_UNREADABLE - // makes that visible if it bites. Unlike the old stack-only design, the malloc - // fallback means this is no longer a leak-proof region: siglongjmp out of - // resolve() bypasses ~ResolvedNames(), so the fault-recovery path must call - // ResolvedNames::release() explicitly (see resolve()) to free any buffer - // allocated before the fault. - static constexpr size_t MAX_KLASS_NAME_LEN = 1024; // internal names; realistically < 256 - static constexpr size_t MAX_METHOD_NAME_LEN = 512; // realistically < 64 - // A descriptor can in principle exceed this (255 argument *slots*, each able to - // carry an arbitrarily long L...; type name), so this cap is a fidelity choice, - // not a proof: over-cap descriptors serialize as "unknown" rather than being - // truncated. METHOD_RESOLVE_SYMBOL_UNREADABLE makes it visible if that bites. - static constexpr size_t MAX_SIGNATURE_LEN = 1024; - - private: - // volatile: resolve() mutates these (via setMethodName/setMethodSignature/ - // setKlassName, called through readMethodNames()) between sigsetjmp() and a - // possible siglongjmp() out of a fault, then release() reads them back at - // the landing pad to decide what to free. Per the setjmp/longjmp rules - // (C11 7.13.2.1p3, inherited by C++), a non-volatile automatic local - // modified in that window has an indeterminate value after longjmp; - // volatile is what makes release()'s reads on the recovery path defined. - char* volatile _long_method_name; - char* volatile _long_method_signature; - char* volatile _long_klass_name; - - char _method_name[MAX_METHOD_NAME_LEN]; - char _method_signature[MAX_SIGNATURE_LEN]; - char _klass_name[MAX_KLASS_NAME_LEN]; - - bool setImpl(char* short_name, char* volatile& long_name, size_t short_limit, VMSymbol* sym); -public: - // Non-copyable - ResolvedNames(const ResolvedNames&) = delete; - ResolvedNames& operator=(const ResolvedNames&) = delete; - - ResolvedNames(); - ~ResolvedNames(); - void release(); - - bool setMethodName(VMSymbol* sym); - bool setMethodSignature(VMSymbol* sym); - bool setKlassName(VMSymbol* sym); - - const char* methodName() const { - return _long_method_name != nullptr ? _long_method_name : _method_name; - } - - const char* methodSignature() const { - return _long_method_signature != nullptr ? _long_method_signature : _method_signature; - } - - const char* klassName() const { - return _long_klass_name != nullptr ? _long_klass_name : _klass_name; - } -}; - -ResolvedNames::ResolvedNames() : +ResolvedMethod::ResolvedMethod() : _long_method_name(nullptr), _long_method_signature(nullptr), - _long_klass_name(nullptr) { + _long_klass_name(nullptr), + _line_number_table(nullptr), + _line_number_table_size(0), + _is_thread_run(false) { + _method_name[0] = '\0'; + _method_signature[0] = '\0'; + _klass_name[0] = '\0'; } -ResolvedNames::~ResolvedNames() { - release(); -} - -void ResolvedNames::release() { +void ResolvedMethod::release() { // Must be idempotent: resolve()'s fault-recovery path calls this explicitly - // (siglongjmp bypasses ~ResolvedNames()), and then the destructor runs it - // again on the same object when resolve() returns normally afterward. - // Nulling out each pointer after freeing makes the second call a no-op - // instead of a double free. + // (siglongjmp bypasses ~ResolvedMethod()), and the destructor runs it again + // on the same object later. Nulling out each pointer after freeing makes + // the second call a no-op instead of a double free. if (_long_method_name != nullptr) { free(_long_method_name); _long_method_name = nullptr; @@ -1558,9 +1482,14 @@ void ResolvedNames::release() { free(_long_klass_name); _long_klass_name = nullptr; } + if (_line_number_table != nullptr) { + free(_line_number_table); + _line_number_table = nullptr; + } + _line_number_table_size = 0; } -bool ResolvedNames::setImpl(char* short_name, char* volatile& long_name, size_t short_limit, VMSymbol* sym) { +bool ResolvedMethod::setName(char* short_name, char* volatile& long_name, size_t short_limit, VMSymbol* sym) { unsigned len = sym->length(); // raw u2 deref; PC stays inside this library // A method name, descriptor or class name is never empty; 0 means the Symbol // slot has been recycled. `>=` leaves room for the NUL. @@ -1585,49 +1514,105 @@ bool ResolvedNames::setImpl(char* short_name, char* volatile& long_name, size_t } } -bool ResolvedNames::setMethodName(VMSymbol* sym) { - return setImpl(_method_name, _long_method_name, MAX_METHOD_NAME_LEN, sym); +bool ResolvedMethod::setMethodName(VMSymbol* sym) { + return setName(_method_name, _long_method_name, MAX_METHOD_NAME_LEN, sym); +} +bool ResolvedMethod::setMethodSignature(VMSymbol* sym) { + return setName(_method_signature, _long_method_signature, MAX_SIGNATURE_LEN, sym); } -bool ResolvedNames::setMethodSignature(VMSymbol* sym) { - return setImpl(_method_signature, _long_method_signature, MAX_SIGNATURE_LEN, sym); + +bool ResolvedMethod::setKlassName(VMSymbol* sym) { + return setName(_klass_name, _long_klass_name, MAX_KLASS_NAME_LEN, sym); } -bool ResolvedNames::setKlassName(VMSymbol* sym) { - return setImpl(_klass_name, _long_klass_name, MAX_KLASS_NAME_LEN, sym); +// Compares a Symbol's contents with `expected` without touching the Symbol +// memory through libc (a fault inside memcmp() would have a pc outside this +// library and could not be recovered). +static bool symbolEquals(VMSymbol* sym, const char* expected, size_t expected_len) { + static constexpr size_t MAX_COMPARE_LEN = 64; + assert(expected_len <= MAX_COMPARE_LEN); + if (sym == nullptr || sym->length() != expected_len) { + return false; + } + char buf[MAX_COMPARE_LEN]; + return SafeAccess::safeCopy(buf, sym->body(), expected_len) && + memcmp(buf, expected, expected_len) == 0; } +// Whether `klass` is java.lang.Thread or one of its subclasses, by walking +// the Klass::_super chain. Unknown (no _super offset) is reported as false. +static bool isThreadSubclass(VMKlass* klass) { + static const char THREAD_CLASS[] = "java/lang/Thread"; + // Real hierarchies are far shallower; the cap only guards against a cycle + // in stale metadata. + static constexpr int MAX_DEPTH = 64; + for (int depth = 0; klass != nullptr && depth < MAX_DEPTH; depth++) { + if (symbolEquals(klass->name(), THREAD_CLASS, sizeof(THREAD_CLASS) - 1)) { + return true; + } + klass = klass->super_or_null(); + } + return false; +} + +// Decodes the method's compressed line number table into a malloc'd +// jvmtiLineNumberEntry array, the same shape JVMTI GetLineNumberTable() +// produces. Returns false only if the table is present but implausible (a +// bci outside the bytecode or too many entries), i.e. the metadata is stale; +// a method without a table returns true with no table attached. +static bool readLineNumberTable(VMConstMethod* const_method, + jvmtiLineNumberEntry* volatile& table, volatile int& table_size) { + const unsigned char* compressed = const_method->compressedLineNumberTable(); + if (compressed == nullptr) { + return true; + } + int code_size = const_method->codeSize(); + + // Pass 1: validate and count, so the second pass can fill an exactly-sized + // buffer. Every bci must index the bytecode (JVMS 4.7.12). + int count = 0; + VMLineNumberStream counter(compressed); + while (counter.readPair()) { + if (counter.bci() < 0 || counter.bci() >= code_size || + ++count > ResolvedMethod::MAX_LINE_NUMBER_TABLE_ENTRIES) { + return false; + } + } + if (count == 0) { + return true; + } + + jvmtiLineNumberEntry* entries = (jvmtiLineNumberEntry*)malloc((size_t)count * sizeof(jvmtiLineNumberEntry)); + if (entries == nullptr) { + return true; // keep the names; the frame just has no line numbers + } + // Owned by the ResolvedMethod from here on, so a fault in pass 2 still frees it. + table = entries; -// PHASE 1 -- the raw HotSpot metadata walk. MUST run with a jmp ctx installed: -// every step is a raw *(void**)(this + offset) whose target may have been freed -// by GC or class unloading since the sample was taken. Deliberately contains no -// JNI and no JVMTI, so the whole protected region stays inside this library, -// where Profiler::checkFault() can actually recover. It can allocate, via -// ResolvedNames::setImpl()'s malloc fallback for over-sized names -- that -// allocation is self-contained (glibc malloc doesn't call back into JNI/JVMTI), -// but it does mean a fault after the allocation needs explicit freeing, since -// siglongjmp out of this scope bypasses ~ResolvedNames() (see resolve()). -// -// Returns false if the metadata is unusable. On success either *out_id holds an -// already-valid jmethodID (and `names` is untouched), or *out_id is null and -// `names` has been filled in for the JNI lookup the caller does afterwards. -static bool readMethodNames(const void* method, VMMethod** out_vm_method, - jmethodID* out_id, ResolvedNames* names) { - *out_vm_method = nullptr; - *out_id = nullptr; + VMLineNumberStream reader(compressed); + for (int i = 0; i < count; i++) { + if (!reader.readPair()) { + return false; // stream changed under us between the passes + } + entries[i].start_location = (jlocation)reader.bci(); + entries[i].line_number = (jint)reader.line(); + } + table_size = count; + return true; +} +// The raw HotSpot metadata walk. MUST run with a jmp ctx installed: every step +// is a raw *(void**)(this + offset) whose target may have been freed by GC or +// class unloading since the sample was taken. Contains no JNI and no JVMTI, so +// the whole protected region stays inside this library, where +// Profiler::checkFault() can recover. Allocations it makes are owned by +// `result` as soon as they are made, so resolve()'s landing pad can free them. +bool HotspotSupport::readMethod(const void* method, ResolvedMethod* result) { VMMethod* vm_method = VMMethod::cast_or_null(method); if (vm_method == nullptr) { return false; } - *out_vm_method = vm_method; - - // May have been populated by following code or JMETHODID_NOT_WALKABLE - jmethodID method_id = vm_method->validatedId(); - if (isValidJMethodID(method_id)) { - *out_id = method_id; - return true; - } VMConstMethod* const_method = vm_method->constMethod_or_null(); if (const_method == nullptr) { @@ -1652,101 +1637,37 @@ static bool readMethodNames(const void* method, VMMethod** out_vm_method, return false; } - if (!names->setMethodName(name_sym) || !names->setMethodSignature(sig_sym) || !names->setKlassName(klass_sym)) { + if (!result->setMethodName(name_sym) || !result->setMethodSignature(sig_sym) || !result->setKlassName(klass_sym)) { Counters::increment(METHOD_RESOLVE_SYMBOL_UNREADABLE); return false; } - return true; -} - -// PHASE 2 -- the JNI/JVMTI lookup, reading only the buffers phase 1 filled. -// MUST run with crash protection *off*. Two reasons, both about siglongjmp -// unwinding frames it must not: -// - A fault inside libjvm.so is unrecoverable anyway (checkFault() only -// recovers PCs inside this library), so a landing pad buys nothing here. -// - FindClass() loads the class when it is not already loaded, which -// synchronously runs our own JVMTI ClassPrepare callback -> -// patchClassLoaderData(), which holds the JVM's ClassLoaderData mutex. That -// code *is* in this library, so with a pad installed a fault there would -// siglongjmp out of a JVMTI callback with a JVM lock held, an abandoned JNI -// local frame and unbalanced safepoint state -- trading a crash for a -// JVM-wide deadlock. -// vm_method->validatedId() below is safefetch-based, so it is safe unprotected. -// -// A plain, TU-local helper -- like readMethodNames() -- rather than a -// HotspotSupport member or friend: it never touches HotspotSupport's private -// state directly, so nothing about it, including ResolvedNames (a type -// defined entirely in this file with no header of its own), needs to be -// declared in hotspotSupport.h. The fallback needs the private -// HotspotSupport::loadMethodIDsIfNeededImpl(), so resolve() -- which does -// have access, being a member -- passes it in as a plain function pointer -// instead of this function calling it directly. -// -// Returns a jmethodID valid for as long as the declaring class stays loaded -// (the same ownership/lifetime jmethodIDs always have in this codebase -- no -// release call is needed), or nullptr if the method could not be found via -// JNI/JVMTI. -static jmethodID lookupMethodIdViaJni(VMMethod* vm_method, const ResolvedNames& names, - bool (*loadMethodIDsIfNeededImpl)(jvmtiEnv*, JNIEnv*, jclass, bool)) { - jmethodID method_id = nullptr; - const char* method_name = names.methodName(); - const char* method_signature = names.methodSignature(); - const char* klass_name = names.klassName(); - JNIEnv *jni = VM::jni(); - jclass clz = jni->FindClass(klass_name); - if (clz == nullptr) { - jni->ExceptionClear(); - } else { - method_id = jni->GetMethodID(clz, method_name, method_signature); - if (method_id == nullptr) { - jni->ExceptionClear(); - method_id = jni->GetStaticMethodID(clz, method_name, method_signature); - if (method_id == nullptr) { - jni->ExceptionClear(); - // JNI GetMethodID/GetStaticMethodID cannot look up because - // the JVM intentionally hides class initializers from JNI callers. - // Fall back to loadMethodIDsIfNeededImpl(), which covers all methods - // including and forces jmethodID slot allocation for them - // (going through this helper, rather than calling GetClassMethods - // directly, ensures the JDK-8062116 patchClassLoaderData() workaround - // is applied here too, same as every other jmethodID-preload path). - // After the call, re-read the ID directly from VM metadata. - if (strcmp(method_name, "") == 0) { - jvmtiEnv* jvmti = VM::jvmti(); - if (jvmti != nullptr) { - if (loadMethodIDsIfNeededImpl(jvmti, jni, clz, true /*load all*/)) { - jmethodID validated = vm_method->validatedId(); - if (isValidJMethodID(validated)) { - method_id = validated; - } - } - } - } - } - } - jni->DeleteLocalRef(clz); + if (strcmp(result->methodName(), "run") == 0 && strcmp(result->methodSignature(), "()V") == 0) { + result->_is_thread_run = isThreadSubclass(klass); } - return method_id; + if (!readLineNumberTable(const_method, result->_line_number_table, result->_line_number_table_size)) { + Counters::increment(LINE_NUMBER_TABLE_UNREADABLE); + return false; + } + return true; } -// This method only resolves methods that are loaded by system class loaders -jmethodID HotspotSupport::resolve(const void* method) { +bool HotspotSupport::resolve(const void* method, ResolvedMethod& result) { assert(VM::isHotspot()); NO_INJECTION_ASSERT(method != nullptr); // fillJavaFrame stores the sentinel without the raw flag, so this should - // never reach the raw-pointer resolve path. Map it to nullptr so the dump - // thread serializes it as the shared unknown method. + // never reach the raw-pointer resolve path. Report it as unresolved so the + // dump thread serializes it as the shared unknown method. if ((jmethodID)method == JMETHODID_NOT_WALKABLE) { - return nullptr; + return false; } // The Method* was captured at sample time; GC or class unloading may have // freed the metadata since, so every dereference below can fault. Install a // landing pad and report the method as unresolved instead of taking the JVM - // down mid-dump -- nullptr is already a first-class result for our caller - // (Lookup::resolveMethod serializes it as the shared unknown method). + // down mid-dump -- false is already a first-class result for our caller + // (Lookup::fillMethod serializes it as the shared unknown method). // // Runs on the JFR dump thread (Profiler::dump/stop), not in a signal handler. // acquireCurrent() rather than current() because JNI_OnUnload reaches @@ -1757,14 +1678,10 @@ jmethodID HotspotSupport::resolve(const void* method) { // rather than risk crashing. Reached only on an unprimed shutdown path or // when the thread-local pool is exhausted. Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); - return nullptr; + return false; } - ResolvedNames names; - VMMethod* vm_method = nullptr; - jmethodID existing_id = nullptr; - bool walked = false; - + bool resolved = false; { sigjmp_buf crash_protection_ctx; // Chained via JmpCtxScope: the dump thread can be interrupted by a sampling @@ -1781,25 +1698,19 @@ jmethodID HotspotSupport::resolve(const void* method) { // before touching anything that could fault again. SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); jmp_scope.restore(); - // siglongjmp bypasses ~ResolvedNames(), so a name that was already - // malloc'd (in setImpl()'s over-sized-name fallback) before the fault - // would otherwise leak. release() is idempotent, so it's safe that the - // destructor also runs it when resolve() returns below. - names.release(); + // siglongjmp bypasses ~ResolvedMethod() and skips the cleanup below; + // free whatever was allocated before the fault. + result.release(); Counters::increment(METHOD_RESOLVE_FAULT_RECOVERED); - return nullptr; + return false; } jmp_scope.install(&crash_protection_ctx); - walked = readMethodNames(method, &vm_method, &existing_id, &names); + resolved = readMethod(method, &result); } - // --- crash protection is off from here on; see lookupMethodIdViaJni() --- - if (!walked) { - return nullptr; - } - if (existing_id != nullptr) { - return existing_id; + if (!resolved) { + result.release(); } - return lookupMethodIdViaJni(vm_method, names, &HotspotSupport::loadMethodIDsIfNeededImpl); + return resolved; } diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h index 8588635dfc..4b52b3d541 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h @@ -19,6 +19,97 @@ #include class VMMethod; +class VMSymbol; + +// What HotspotSupport::resolve() extracts from a raw Method*: the holder's +// internal class name, the method name and signature, and the decoded line +// number table. Everything is copied out of the VM metadata, so the result +// stays valid after the class has been unloaded. +// +// Each name has a fixed-size inline buffer for the common case, with a +// malloc'd fallback (up to MAX_SYMBOL_LEN) for names that don't fit. The line +// number table is always malloc'd. resolve() can siglongjmp out of a fault +// after any of these allocations, bypassing ~ResolvedMethod(), which is why +// its fault-recovery path calls release() explicitly. +class ResolvedMethod { + // Hard ceiling for the malloc fallback in setName(). Not a JVM/class-file + // limit (Symbol::length() is a u2, so up to 65535 is legal). + static constexpr size_t MAX_SYMBOL_LEN = 64 * 1024; + + // Fast-path buffer sizes, deliberately small enough that the malloc + // fallback in setName() still gets exercised occasionally. + static constexpr size_t MAX_KLASS_NAME_LEN = 1024; // internal names; realistically < 256 + static constexpr size_t MAX_METHOD_NAME_LEN = 512; // realistically < 64 + static constexpr size_t MAX_SIGNATURE_LEN = 1024; + + // volatile: resolve() mutates these between sigsetjmp() and a possible + // siglongjmp() out of a fault, and release() reads them back at the landing + // pad. Per C11 7.13.2.1p3 a non-volatile automatic modified in that window + // has an indeterminate value after longjmp. + char* volatile _long_method_name; + char* volatile _long_method_signature; + char* volatile _long_klass_name; + jvmtiLineNumberEntry* volatile _line_number_table; + volatile int _line_number_table_size; + volatile bool _is_thread_run; + + char _method_name[MAX_METHOD_NAME_LEN]; + char _method_signature[MAX_SIGNATURE_LEN]; + char _klass_name[MAX_KLASS_NAME_LEN]; + + bool setName(char* short_name, char* volatile& long_name, size_t short_limit, VMSymbol* sym); + + friend class HotspotSupport; + +public: + // Upper bound on decoded line number table entries; mirrors the u2 + // code_length limit of JVMS 4.7.3. + static constexpr int MAX_LINE_NUMBER_TABLE_ENTRIES = 65535; + + ResolvedMethod(const ResolvedMethod&) = delete; + ResolvedMethod& operator=(const ResolvedMethod&) = delete; + + ResolvedMethod(); + ~ResolvedMethod() { release(); } + + // Frees everything still owned. Idempotent. + void release(); + + bool setMethodName(VMSymbol* sym); + bool setMethodSignature(VMSymbol* sym); + bool setKlassName(VMSymbol* sym); + + // Internal (slash-separated) class name, e.g. "java/lang/Thread". + const char* klassName() const { + return _long_klass_name != nullptr ? _long_klass_name : _klass_name; + } + + const char* methodName() const { + return _long_method_name != nullptr ? _long_method_name : _method_name; + } + + const char* methodSignature() const { + return _long_method_signature != nullptr ? _long_method_signature : _method_signature; + } + + // True for run()V declared by java.lang.Thread or one of its subclasses. + bool isThreadRun() const { return _is_thread_run; } + + // Decoded line number table, sorted by bci as HotSpot stores it, or + // nullptr if the method has none. Allocated with malloc(). + const jvmtiLineNumberEntry* lineNumberTable() const { return _line_number_table; } + int lineNumberTableSize() const { return _line_number_table_size; } + + // Hands ownership of the line number table to the caller, who must free() + // it. Returns nullptr if there is none. + jvmtiLineNumberEntry* takeLineNumberTable(int* size) { + jvmtiLineNumberEntry* table = _line_number_table; + *size = _line_number_table_size; + _line_number_table = nullptr; + _line_number_table_size = 0; + return table; + } +}; class HotspotSupport { friend class JVMSupport; @@ -49,6 +140,9 @@ class HotspotSupport { static bool loadMethodIDsIfNeededImpl(jvmtiEnv *jvmti, JNIEnv *jni, jclass klass, bool load_all); + // resolve()'s metadata walk; must run with crash protection installed. + static bool readMethod(const void* method, ResolvedMethod* result); + // Runs getJavaTraceAsync() under withUcontextFaultRecovery(), then layers // on its two post-processing steps: resolving frame types for the top // Java frame via fillFrameTypes(), and appending a synthetic "JVM @@ -164,8 +258,11 @@ class HotspotSupport { !(cstack == CSTACK_VM || cstack == CSTACK_DEFAULT); // Can only use Method* when cstack = vm } - // Resolve a method to a jmethodID at dumping time - static jmethodID resolve(const void* method); + // Resolve a raw Method* at dumping time into its class name, method name, + // signature and line number table, reading VM metadata through VMStructs + // only (no JNI/JVMTI). Returns false if the Method* is the + // JMETHODID_NOT_WALKABLE sentinel or its metadata is stale/unreadable. + static bool resolve(const void* method, ResolvedMethod& result); // Store a Java frame captured from HotSpot metadata. A null jmethodID // triggers the raw Method* fallback only when fjmethodid=false; with diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp index 0a04e46343..061846813e 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp @@ -931,6 +931,19 @@ bool VMMethod::check_jmethodID(jmethodID id) { return check_jmethodID_hotspot(id); } +const VMMethod* VMMethod::fromJmethodID(jmethodID id) { + if (id == nullptr || !_can_dereference_jmethod_id) { + return nullptr; + } + const void* method = SafeAccess::load((void**)id); + // Same empty-slot values check_jmethodID_hotspot() rejects + // (JNIMethodBlock::_free_method is 55). + if (method == nullptr || (size_t)method == 55) { + return nullptr; + } + return (const VMMethod*)method; +} + bool VMMethod::check_jmethodID_hotspot(jmethodID id) { if (VM::hotspot_version() > 25) { // In https://bugs.openjdk.org/browse/JDK-8268406 the jmethodids are completely reworked diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h index c61ccbc3b3..86a14c2121 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h @@ -217,12 +217,15 @@ typedef void* address; field(_constmethod_idnum_offset, offset, MATCH_SYMBOLS("_method_idnum")) \ field(_constmethod_name_index_offset, offset, MATCH_SYMBOLS("_name_index")) \ field(_constmethod_sig_index_offset, offset, MATCH_SYMBOLS("_signature_index")) \ + field(_constmethod_flags_offset, offset, MATCH_SYMBOLS("_flags", "_flags._flags")) \ + field(_constmethod_code_size_offset, offset, MATCH_SYMBOLS("_code_size")) \ type_end() \ type_begin(VMConstantPool, MATCH_SYMBOLS("ConstantPool")) \ field(_pool_holder_offset, offset, MATCH_SYMBOLS("_pool_holder")) \ type_end() \ type_begin(VMKlass, MATCH_SYMBOLS("Klass", "InstanceKlass")) \ field(_klass_name_offset, offset, MATCH_SYMBOLS("_name")) \ + field(_klass_super_offset, offset, MATCH_SYMBOLS("_super")) \ field(_class_loader_data_offset, offset, MATCH_SYMBOLS("_class_loader_data")) \ field(_methods_offset, offset, MATCH_SYMBOLS("_methods")) \ field(_jmethod_ids_offset, offset, MATCH_SYMBOLS("_methods_jmethod_ids")) \ @@ -328,8 +331,12 @@ typedef void* address; * - constant defines a constant of a class */ -#define DECLARE_INT_CONSTANTS_DO(constant, constant_with_version) \ - constant(frame, entry_frame_call_wrapper_offset) +// The has_linenumber_table bit of ConstMethod::_flags. JDK 21 moved the flags +// into ConstMethodFlags (and widened _flags from u2 to u4). +#define DECLARE_INT_CONSTANTS_DO(constant, constant_with_version) \ + constant(frame, entry_frame_call_wrapper_offset) \ + constant_with_version(ConstMethod, _has_linenumber_table, 8, 20) \ + constant_with_version(ConstMethodFlags, _misc_has_linenumber_table, 21, MAX_VERSION) #define DECLARE_LONG_CONSTANTS_DO(constant, constant_with_version) \ constant_with_version(markWord, klass_shift, 24, MAX_VERSION) \ @@ -495,6 +502,10 @@ class VMStructs { return _has_class_loader_data; } + static bool canDereferenceJmethodID() { + return _can_dereference_jmethod_id; + } + static bool hasNativeThreadId() { return _has_native_thread_id; } @@ -703,6 +714,13 @@ DECLARE(VMKlass) return VMSymbol::load_then_cast(at(_klass_name_offset)); } + // Super class, or nullptr for java.lang.Object, interfaces' roots, or when + // the VM does not export Klass::_super. + VMKlass* super_or_null() { + if (_klass_super_offset < 0) return nullptr; + return VMKlass::load_then_cast(at(_klass_super_offset)); + } + VMClassLoaderData* classLoaderData() { assert(_class_loader_data_offset >= 0); return VMClassLoaderData::load_then_cast(at(_class_loader_data_offset)); @@ -905,6 +923,11 @@ DECLARE(VMConstMethod) inline VMConstantPool* constants_or_null() const; inline VMSymbol* name() const; inline VMSymbol* signature() const; + inline u16 codeSize() const; + // Start of the compressed line number table, which HotSpot places right + // after the bytecodes, or nullptr if the method has none (or the VM does + // not export what is needed to locate it). + inline const unsigned char* compressedLineNumberTable() const; private: inline u16 nameIndex() const; inline u16 signatureIndex() const; @@ -931,6 +954,11 @@ DECLARE(VMMethod) inline VMNMethod* code(); static bool check_jmethodID(jmethodID id); + + // The Method* a jmethodID refers to, read straight from its JNI id slot -- + // no JNI/JVMTI. nullptr if this JVM's jmethodIDs can't be dereferenced + // (JDK-8268406) or the slot is empty/freed. + static const VMMethod* fromJmethodID(jmethodID id); DECLARE_END // Inline string comparison to avoid indirect call to strncmp @@ -1210,6 +1238,62 @@ class ScopeDesc : VMStructs { } }; +// Decoder for ConstMethod's compressed line number table, a mirror of +// HotSpot's CompressedLineNumberReadStream. Each entry is either a single byte +// holding (bci_delta << 3 | line_delta), or 0xFF followed by the two deltas as +// signed CompressedStream ints. A 0 byte terminates the table. +// +// Reads the raw stream without any safety checks: callers must run with crash +// protection installed and bound the number of entries they accept. +class VMLineNumberStream : VMStructs { + private: + const unsigned char* _stream; + int _bci; + int _line; + + // Same UNSIGNED5 decoding as ScopeDesc::readInt() (see JDK-8292758 for + // the _unsigned5_base shift). + unsigned int readUInt() { + unsigned char c = *_stream++; + unsigned int n = c - _unsigned5_base; + if (c >= 192) { + for (int shift = 6; ; shift += 6) { + c = *_stream++; + n += (c - _unsigned5_base) << shift; + if (c < 192 || shift >= 24) break; + } + } + return n; + } + + int readSignedInt() { + unsigned int n = readUInt(); + return (int)(n >> 1) ^ -(int)(n & 1); + } + + public: + explicit VMLineNumberStream(const unsigned char* table) : _stream(table), _bci(0), _line(0) {} + + // Advances to the next (bci, line) pair; returns false at the terminator. + bool readPair() { + unsigned char next = *_stream++; + if (next == 0) { + return false; + } + if (next == 0xFF) { + _bci += readSignedInt(); + _line += readSignedInt(); + } else { + _bci += next >> 3; + _line += next & 0x7; + } + return true; + } + + int bci() const { return _bci; } + int line() const { return _line; } +}; + class InterpreterFrame : VMStructs { public: enum { diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h index 043ab06722..eca40996fa 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h @@ -212,6 +212,32 @@ VMSymbol* VMConstMethod::signature() const { return cpool->symbolAt(sig_index); } +u16 VMConstMethod::codeSize() const { + assert(_constmethod_code_size_offset >= 0); + return *(u16*)at(_constmethod_code_size_offset); +} + +const unsigned char* VMConstMethod::compressedLineNumberTable() const { + if (_constmethod_flags_offset < 0 || _constmethod_code_size_offset < 0) { + return nullptr; + } + bool has_table; + if (_ConstMethodFlags__misc_has_linenumber_table != -1) { + // JDK 21+: u4 ConstMethodFlags::_flags + has_table = (*(u32*)at(_constmethod_flags_offset) & (u32)_ConstMethodFlags__misc_has_linenumber_table) != 0; + } else if (_ConstMethod__has_linenumber_table != -1) { + // JDK 8 - 20: u2 ConstMethod::_flags + has_table = (*(u16*)at(_constmethod_flags_offset) & (u16)_ConstMethod__has_linenumber_table) != 0; + } else { + return nullptr; + } + if (!has_table) { + return nullptr; + } + // ConstMethod::compressed_linenumber_table() == code_end() + return (const unsigned char*)at((int)type_size()) + codeSize(); +} + u16 VMConstMethod::nameIndex() const { assert(_constmethod_name_index_offset >= 0 && "Invalid name index"); return *(u16*)at(_constmethod_name_index_offset); diff --git a/ddprof-lib/src/main/cpp/jvmSupport.h b/ddprof-lib/src/main/cpp/jvmSupport.h index 49b8a9193f..3de2a52f88 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.h +++ b/ddprof-lib/src/main/cpp/jvmSupport.h @@ -22,6 +22,7 @@ enum StackRecovery { }; class HotspotSupport; +class ResolvedMethod; class JVMSupport { enum JMethodIDLoadStats { @@ -66,8 +67,8 @@ class JVMSupport { static void loadAllMethodIDsIfNeeded(jvmtiEnv *jvmti, JNIEnv *jni); static bool loadMethodIDsIfNeeded(jvmtiEnv *jvmti, JNIEnv *jni, jclass klass); - // Resolve method pointer to jmethodID - static inline jmethodID resolve(const void* method); + // Resolve a raw method pointer into names and line number table (HotSpot only) + static inline bool resolve(const void* method, ResolvedMethod& result); // If a class is hidden class static inline bool isHidden(jint modifiers); diff --git a/ddprof-lib/src/main/cpp/jvmSupport.inline.h b/ddprof-lib/src/main/cpp/jvmSupport.inline.h index bea61880ff..0092ef15dc 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.inline.h +++ b/ddprof-lib/src/main/cpp/jvmSupport.inline.h @@ -35,13 +35,12 @@ long long JVMSupport::runtimeStubsMemoryUsage() { } } -// Resolve method pointer to jmethodID -jmethodID JVMSupport::resolve(const void* method) { +bool JVMSupport::resolve(const void* method, ResolvedMethod& result) { if (VM::isHotspot()) { - return HotspotSupport::resolve(method); + return HotspotSupport::resolve(method, result); } else { assert(false && "Should not reach here"); - return nullptr; + return false; } } diff --git a/ddprof-lib/src/main/cpp/mutex.cpp b/ddprof-lib/src/main/cpp/mutex.cpp index 8b9c92b64d..137114331e 100644 --- a/ddprof-lib/src/main/cpp/mutex.cpp +++ b/ddprof-lib/src/main/cpp/mutex.cpp @@ -19,6 +19,10 @@ void Mutex::lock() { pthread_mutex_lock(&_mutex); } +bool Mutex::tryLock() { + return pthread_mutex_trylock(&_mutex) == 0; +} + void Mutex::unlock() { pthread_mutex_unlock(&_mutex); } diff --git a/ddprof-lib/src/main/cpp/mutex.h b/ddprof-lib/src/main/cpp/mutex.h index 7d017536da..f1419685fe 100644 --- a/ddprof-lib/src/main/cpp/mutex.h +++ b/ddprof-lib/src/main/cpp/mutex.h @@ -18,6 +18,8 @@ class Mutex { Mutex(); void lock(); + // Never blocks; for callers that must not wait (e.g. a JVMTI GC callback). + bool tryLock(); void unlock(); }; diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 119e08e74f..0186ce7f0e 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1784,6 +1784,24 @@ Error Profiler::start(Arguments &args, bool reset) { // Paired with drainInflight() on the stop side. _cpu_engine->enableEvents(true); + // onGCStart() may resolve frames from VM metadata only: raw Method* frames + // directly, jmethodID frames (every frame with cstack=fp/dwarf, and classes + // that still get one with cstack=vm) through the Method* their id slot + // holds. So the method/class structs must be known and jmethodIDs + // dereferenceable. Liveness tracking is excluded: processCallTraces() runs + // its checker under a lock that is held across JNI calls. + bool gc_checkpoint = VM::isHotspot() && VMStructs::hasMethodStructs() && + VMStructs::hasClassNames() && + VMStructs::canDereferenceJmethodID() && + !_call_trace_storage.hasLivenessCheckers(); + if (gc_checkpoint) { + // Never disabled again: reference chains share the event, and + // onGCStart() checks _gc_checkpoint first. + VM::jvmti()->SetEventNotificationMode( + JVMTI_ENABLE, JVMTI_EVENT_GARBAGE_COLLECTION_START, nullptr); + } + _gc_checkpoint.store(gc_checkpoint, std::memory_order_release); + _state.store(RUNNING, std::memory_order_release); _start_time = time(NULL); __atomic_add_fetch(&_epoch, 1, __ATOMIC_RELAXED); @@ -1807,6 +1825,7 @@ Error Profiler::stop() { if (state() != RUNNING) { return Error("Profiler is not active"); } + _gc_checkpoint.store(false, std::memory_order_release); // Order matters: disable engines first so the _enabled check inside signal // handlers will fail for any new signal delivered from now on. drain() then @@ -2096,6 +2115,80 @@ void Profiler::unlockAll() { _locks[i].unlock(); } +bool Profiler::tryLockAll() { + // Holders are signal handlers and JNI writers that keep a lock for a few + // microseconds; a short spin rides those out. + const int max_spins = 1000; + for (int i = 0; i < CONCURRENCY_LEVEL; i++) { + int spins = 0; + while (!_locks[i].tryLock()) { + if (++spins >= max_spins) { + while (--i >= 0) { + _locks[i].unlock(); + } + return false; + } + spinPause(); + } + } + return true; +} + +// Writes a JFR checkpoint -- finishes the current chunk -- at the start of +// every GC, so samples of methods a GC is about to unload are serialized while +// their metadata is still there to read, rather than as /unknown at +// the next dump. +// +// Runs in the JVMTI GarbageCollectionStart callback, with the VM stopped: no +// JNI, almost no JVMTI (Recording::checkpointChunk() resolves every method +// from VM metadata; start() enables this only where that works), and nothing +// may block, since a thread holding one of our locks may itself be parked for +// this pause. So every lock is try-locked, and a GC that finds one taken skips +// the checkpoint. Locks are taken in dump()'s order (_state_lock, _locks[], +// then the recording's), and all of them before rotating the dictionaries: a +// rotation must always be followed by the cpool write that consumes it. +void Profiler::onGCStart() { + if (!_gc_checkpoint.load(std::memory_order_acquire)) { + return; + } + if (!_state_lock.tryLock()) { + Counters::increment(GC_CHECKPOINTS_SKIPPED); + return; + } + bool written = false; + if (state() == RUNNING && tryLockAll()) { + SignalBlocker blocker; + if (_jfr.tryCheckpointBegin()) { + if (!_class_map.rotate()) { + reportDrainTimeout("Class map reused a standby buffer it could not clear"); + } + if (!_string_label_map.rotate()) { + reportDrainTimeout("String label map reused a standby buffer it could not clear"); + } + if (!_context_value_map.rotate()) { + reportDrainTimeout("Context value map reused a standby buffer it could not clear"); + } + _jfr.checkpointAndEnd(); + written = true; + __atomic_add_fetch(&_epoch, 1, __ATOMIC_SEQ_CST); + unlockAll(); + if (!_class_map.clearStandby()) { + reportDrainTimeout("Class map standby buffer not cleared"); + } + if (!_string_label_map.clearStandby()) { + reportDrainTimeout("String label map standby buffer not cleared"); + } + if (!_context_value_map.clearStandby()) { + reportDrainTimeout("Context value map standby buffer not cleared"); + } + } else { + unlockAll(); + } + } + _state_lock.unlock(); + Counters::increment(written ? GC_CHECKPOINTS : GC_CHECKPOINTS_SKIPPED); +} + void Profiler::switchThreadEvents(jvmtiEventMode mode) { if (_thread_events_state != mode) { jvmtiEnv *jvmti = VM::jvmti(); diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 6c85387d0a..4125ec618c 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -148,6 +148,8 @@ class alignas(alignof(SpinLock)) Profiler { int _safe_mode; CStack _cstack; bool _force_jmethodID; + // Whether onGCStart() writes a JFR checkpoint; see onGCStart(). + std::atomic _gc_checkpoint{false}; volatile jvmtiEventMode _thread_events_state; @@ -189,6 +191,8 @@ class alignas(alignof(SpinLock)) Profiler { Error checkJvmCapabilities(); void lockAll(); + // lockAll() that gives up, holding nothing, if any lock stays taken. + bool tryLockAll(); // Resets the dictionaries, the call trace storage and the counters for a // fresh recording; returns how many dictionaries were left unchanged because // their drain timed out. @@ -371,6 +375,8 @@ class alignas(alignof(SpinLock)) Profiler { Error start(Arguments &args, bool reset); Error stop(); Error dump(const char *path, const int length); + // JVMTI GarbageCollectionStart hook. See the definition. + void onGCStart(); Error checkState(); void logStats(); void switchThreadEvents(jvmtiEventMode mode); diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index acf8382322..7ed87ec503 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -456,6 +456,14 @@ void JNICALL ForwardedGarbageCollectionFinish(jvmtiEnv *jvmti_env) { ReferenceChainTracker::GarbageCollectionFinish(jvmti_env); } +// Same fan-out for GarbageCollectionStart: the reference-chain epoch counter +// and the profiler's JFR checkpoint (Profiler::onGCStart()). Each callee +// checks whether it is active. +void JNICALL ForwardedGarbageCollectionStart(jvmtiEnv *jvmti_env) { + ReferenceChainTracker::GarbageCollectionStart(jvmti_env); + Profiler::instance()->onGCStart(); +} + bool VM::initProfilerBridge(JavaVM *vm, bool attach) { TEST_LOG("VM::initProfilerBridge"); if (!initShared(vm)) { @@ -530,7 +538,7 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { callbacks.ThreadStart = Profiler::ThreadStart; callbacks.ThreadEnd = Profiler::ThreadEnd; callbacks.SampledObjectAlloc = ObjectSampler::SampledObjectAlloc; - callbacks.GarbageCollectionStart = ReferenceChainTracker::GarbageCollectionStart; + callbacks.GarbageCollectionStart = ForwardedGarbageCollectionStart; callbacks.GarbageCollectionFinish = ForwardedGarbageCollectionFinish; callbacks.NativeMethodBind = VMStructs::NativeMethodBind; _jvmti->SetEventCallbacks(&callbacks, sizeof(callbacks)); diff --git a/ddprof-lib/src/test/cpp/arguments_ut.cpp b/ddprof-lib/src/test/cpp/arguments_ut.cpp index 8ecefc13fc..bb596b0390 100644 --- a/ddprof-lib/src/test/cpp/arguments_ut.cpp +++ b/ddprof-lib/src/test/cpp/arguments_ut.cpp @@ -99,6 +99,43 @@ TEST_F(ArgumentsTest, CstackNoIsStillValid) { EXPECT_EQ(args._cstack, CSTACK_NO); } +// Without an explicit fjmethodid, jmethodIDs are forced only for cstack modes +// that cannot fall back to a raw Method* (anything but vm/vmx/default). +TEST_F(ArgumentsTest, FjmethodidDefaultsFromCstack) { + struct { const char* args; bool expected; } cases[] = { + {"wall=1ms", false}, + {"wall=1ms,cstack=vm", false}, + {"wall=1ms,cstack=vmx", false}, + {"wall=1ms,cstack=fp", true}, + {"wall=1ms,cstack=dwarf", true}, + {"wall=1ms,cstack=no", true}, + }; + for (const auto& c : cases) { + SCOPED_TRACE(c.args); + Arguments args; + EXPECT_FALSE(args.parse(c.args)); + EXPECT_EQ(args._force_jmethodID, c.expected); + } +} + +// An explicit fjmethodid wins over the cstack-derived default, wherever it +// appears relative to cstack. +TEST_F(ArgumentsTest, ExplicitFjmethodidOverridesCstackDefault) { + struct { const char* args; bool expected; } cases[] = { + {"cstack=vm,fjmethodid=true", true}, + {"fjmethodid=true,cstack=vmx", true}, + {"fjmethodid=true", true}, + {"cstack=fp,fjmethodid=false", false}, + {"fjmethodid=false,cstack=dwarf", false}, + }; + for (const auto& c : cases) { + SCOPED_TRACE(c.args); + Arguments args; + EXPECT_FALSE(args.parse(c.args)); + EXPECT_EQ(args._force_jmethodID, c.expected); + } +} + TEST_F(ArgumentsTest, InvalidWallsamplerValueIsAnError) { Arguments args; Error error = args.parse("wallsampler=x"); diff --git a/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp b/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp index f8501cf6e6..9b11860d6e 100644 --- a/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp +++ b/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp @@ -19,6 +19,8 @@ #include #include #include +#include +#include #ifdef __linux__ #include @@ -123,6 +125,53 @@ class VMStructsTestAccessor { } }; + // Scoped override for what resolve() needs to locate and decode a + // ConstMethod's line number table and to walk Klass::_super. + class LineTableLayout { + public: + LineTableLayout(int flags_offset, int code_size_offset, int has_table_mask, + unsigned char unsigned5_base, int klass_super_offset) { + _saved = _read(); + VMStructs::_constmethod_flags_offset = flags_offset; + VMStructs::_constmethod_code_size_offset = code_size_offset; + // Model JDK 21+ (u4 ConstMethodFlags); the u2 constant stays unset. + VMStructs::_ConstMethodFlags__misc_has_linenumber_table = has_table_mask; + VMStructs::_ConstMethod__has_linenumber_table = -1; + VMStructs::_unsigned5_base = unsigned5_base; + VMStructs::_klass_super_offset = klass_super_offset; + } + + ~LineTableLayout() { + VMStructs::_constmethod_flags_offset = _saved.flags_offset; + VMStructs::_constmethod_code_size_offset = _saved.code_size_offset; + VMStructs::_ConstMethodFlags__misc_has_linenumber_table = _saved.flags_mask; + VMStructs::_ConstMethod__has_linenumber_table = _saved.legacy_mask; + VMStructs::_unsigned5_base = _saved.unsigned5_base; + VMStructs::_klass_super_offset = _saved.klass_super_offset; + } + + private: + struct Saved { + int flags_offset; + int code_size_offset; + int flags_mask; + int legacy_mask; + unsigned char unsigned5_base; + int klass_super_offset; + } _saved; + + static Saved _read() { + return { + VMStructs::_constmethod_flags_offset, + VMStructs::_constmethod_code_size_offset, + VMStructs::_ConstMethodFlags__misc_has_linenumber_table, + VMStructs::_ConstMethod__has_linenumber_table, + VMStructs::_unsigned5_base, + VMStructs::_klass_super_offset, + }; + } + }; + private: struct SavedOffsets { int method_constmethod; @@ -327,12 +376,12 @@ TEST(HotspotMethodIdTest, IdReturnsValidIdForPopulatedSlot) { // --------------------------------------------------------------------------- // HotspotSupport::resolve() crash protection // -// resolve() turns a raw Method* captured at sample time into a jmethodID on the -// JFR dump thread. GC or class unloading can free that metadata in between, so +// resolve() turns a raw Method* captured at sample time into its class name, +// method name, signature and line number table on the JFR dump thread. GC or class unloading can free that metadata in between, so // every dereference in the walk (constMethod -> constants -> name/signature -> // holder -> klass->name, then Symbol length/body) can fault. resolve() installs // a sigsetjmp landing pad via JmpCtxScope so Profiler::checkFault() recovers and -// resolve() reports the method as unresolved (nullptr) instead of taking the JVM +// resolve() reports the method as unresolved (false) instead of taking the JVM // down mid-dump. // // checkFault() gates recovery on the faulting pc lying inside the profiler @@ -374,7 +423,7 @@ size_t kPageSize() { return OS::page_size; } // and consistent with the struct layouts below. constexpr VMStructsTestAccessor::TypeSizes RESOLVE_TYPE_SIZES = { /*method*/ sizeof(void*), - /*const_method*/ 2 * sizeof(void*), + /*const_method*/ 24, // == offsetof(ResolveFakes::ConstMethod, code) /*constant_pool*/ sizeof(void*), // base() = cpool + this, i.e. &symbols[0] /*klass*/ sizeof(void*), /*symbol*/ 8, @@ -413,7 +462,11 @@ struct ResolveFakes { uint16_t name_index; uint16_t sig_index; uint16_t idnum; - uint16_t pad; + uint16_t code_size; // RESOLVE_CODE_SIZE_OFFSET + uint32_t flags; // RESOLVE_FLAGS_OFFSET + uint32_t pad; + // Bytecodes, then the compressed line number table (code_end()). + unsigned char code[64]; } const_method; struct ConstantPool { void* holder; @@ -422,11 +475,12 @@ struct ResolveFakes { struct Klass { void* name_symbol; // klass_name offset 0 void* jmethod_ids; // stays null so VMMethod::id() reports "no cache" - } klass; + void* super_klass; // RESOLVE_KLASS_SUPER_OFFSET + } klass, super_klass; struct Symbol { uint16_t length; char body[64]; - } name_sym, sig_sym, klass_sym; + } name_sym, sig_sym, klass_sym, super_sym; // Wires the chain up so resolve() walks Method -> ConstMethod -> ConstantPool // -> {name, signature} Symbols and ConstantPool -> Klass -> class-name Symbol. @@ -446,74 +500,25 @@ struct ResolveFakes { s.length = (uint16_t)strlen(text); memcpy(s.body, text, s.length); } -}; - -// Minimal fake JavaVM/JNIEnv so HotspotSupport::resolve()'s unprotected JNI -// phase (lookupMethodIdViaJni) can run to completion without a live JVM. -// FindClass always reports success against an opaque, never-dereferenced -// jclass; GetMethodID/GetStaticMethodID record the name/signature they were -// called with and report "not found" -- the same real-world path resolve() -// takes for a method it can't find -- so nothing beyond that is exercised. -// Recording the arguments is what lets a test prove the malloc-fallback path -// copied the right bytes, rather than just checking resolve() didn't crash. -class ScopedFakeJni { -public: - ScopedFakeJni() : _saved_vm(VMTestAccessor::getVm()) { - _jni_tbl = JNINativeInterface_{}; - _jni_tbl.FindClass = &findClass; - _jni_tbl.ExceptionClear = &exceptionClear; - _jni_tbl.GetMethodID = &getMethodId; - _jni_tbl.GetStaticMethodID = &getMethodId; - _jni_tbl.DeleteLocalRef = &deleteLocalRef; - _jni_env.functions = &_jni_tbl; - - _vm_tbl = JNIInvokeInterface_{}; - _vm_tbl.GetEnv = &getEnv; - _vm.functions = &_vm_tbl; - - s_instance = this; - VMTestAccessor::setVm(reinterpret_cast(&_vm)); - } - ~ScopedFakeJni() { - VMTestAccessor::setVm(_saved_vm); - s_instance = nullptr; + // Marks the method as having a line number table and copies `table` + // (already compressed) right after `code_size` bytes of bytecode. + void setLineNumberTable(uint16_t code_size, const unsigned char* table, size_t len) { + const_method.code_size = code_size; + const_method.flags |= 1; + memcpy(const_method.code + code_size, table, len); } +}; - int getMethodIdCalls() const { return _get_method_id_calls; } - const std::string& lastMethodName() const { return _last_method_name; } - const std::string& lastMethodSignature() const { return _last_method_sig; } +// Must stay below the 1024-byte mark ResolveUsesMallocFallbackForOversizedButReadableName +// writes its long Symbol at. +static_assert(sizeof(ResolveFakes) <= 1024, "fakes overlap the long symbol"); +static_assert(offsetof(ResolveFakes::ConstMethod, code) == 24, "RESOLVE_TYPE_SIZES.const_method"); -private: - static jclass JNICALL findClass(JNIEnv*, const char*) { - return reinterpret_cast(&s_instance->_fake_class); - } - static jmethodID JNICALL getMethodId(JNIEnv*, jclass, const char* name, const char* sig) { - s_instance->_get_method_id_calls++; - s_instance->_last_method_name = name != nullptr ? name : ""; - s_instance->_last_method_sig = sig != nullptr ? sig : ""; - return nullptr; - } - static void JNICALL exceptionClear(JNIEnv*) {} - static void JNICALL deleteLocalRef(JNIEnv*, jobject) {} - static jint JNICALL getEnv(JavaVM*, void** penv, jint) { - *penv = reinterpret_cast(&s_instance->_jni_env); - return JNI_OK; - } +constexpr int RESOLVE_CODE_SIZE_OFFSET = offsetof(ResolveFakes::ConstMethod, code_size); +constexpr int RESOLVE_FLAGS_OFFSET = offsetof(ResolveFakes::ConstMethod, flags); +constexpr int RESOLVE_KLASS_SUPER_OFFSET = offsetof(ResolveFakes::Klass, super_klass); - static ScopedFakeJni* s_instance; - - JavaVM* _saved_vm; - JNINativeInterface_ _jni_tbl{}; - JNIEnv_ _jni_env{}; - JNIInvokeInterface_ _vm_tbl{}; - JavaVM_ _vm{}; - int _fake_class = 0; // address-only sentinel; never dereferenced - int _get_method_id_calls = 0; - std::string _last_method_name; - std::string _last_method_sig; -}; -ScopedFakeJni* ScopedFakeJni::s_instance = nullptr; } // namespace @@ -585,7 +590,8 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveRecoversFromFaultInsteadOfCrash // VMMethod::id() reads the same offset through SafeAccess first, so it // returns the sentinel rather than faulting; resolve() then falls through to // constMethod_or_null(), whose raw *(void**) deref is what actually faults. - EXPECT_EQ(nullptr, HotspotSupport::resolve(_region)); + ResolvedMethod result; + EXPECT_FALSE(HotspotSupport::resolve(_region, result)); EXPECT_EQ(before + 1, Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED)); // The landing pad must hand the previous (here: absent) context back. @@ -604,7 +610,8 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveRecoversRepeatedly) { long long before = Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED); for (int i = 0; i < 10; i++) { - EXPECT_EQ(nullptr, HotspotSupport::resolve(_region)); + ResolvedMethod result; + EXPECT_FALSE(HotspotSupport::resolve(_region, result)); EXPECT_FALSE(_pt->isProtected()); } EXPECT_EQ(before + 10, Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED)); @@ -635,7 +642,8 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveReturnsNullForUnreadableSymbolB long long before = Counters::getCounter(METHOD_RESOLVE_SYMBOL_UNREADABLE); long long faults_before = Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED); - EXPECT_EQ(nullptr, HotspotSupport::resolve(&f->method)); + ResolvedMethod result; + EXPECT_FALSE(HotspotSupport::resolve(&f->method, result)); EXPECT_EQ(before + 1, Counters::getCounter(METHOD_RESOLVE_SYMBOL_UNREADABLE)); // Rejected by the readability probe, not by recovering from a real fault. @@ -644,10 +652,10 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveReturnsNullForUnreadableSymbolB } // A method name longer than the fixed inline buffer (MAX_METHOD_NAME_LEN, -// 512 bytes) but fully readable must take ResolvedNames::setImpl()'s malloc -// fallback and still resolve successfully through to the JNI phase, with the -// full, correctly NUL-terminated name -- not get rejected the way a name -// this size would be by the *unreadable*-body case below. +// 512 bytes) but fully readable must take ResolvedMethod::setName()'s malloc +// fallback and still resolve successfully, with the full, correctly +// NUL-terminated name -- not get rejected the way a name this size would be +// by the *unreadable*-body case below. TEST_F(HotspotResolveCrashProtectionTest, ResolveUsesMallocFallbackForOversizedButReadableName) { HotspotMethodIdVMHotspotGuard hotspot; VMStructsTestAccessor offsets(RESOLVE_OFFSETS); @@ -669,34 +677,24 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveUsesMallocFallbackForOversizedB memcpy(long_symbol + RESOLVE_SYMBOL_OFFSETS.symbol_body, long_name.data(), long_name.size()); f->cpool.symbols[1] = (intptr_t)long_symbol; // name_index slot, see link() - ScopedFakeJni fake_jni; - long long unreadable_before = Counters::getCounter(METHOD_RESOLVE_SYMBOL_UNREADABLE); long long fault_before = Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED); - // The fake JNI's GetMethodID reports "not found", so resolve() itself - // still returns nullptr -- the point of this test is what it does on the - // way there, checked below. - EXPECT_EQ(nullptr, HotspotSupport::resolve(&f->method)); + ResolvedMethod result; + ASSERT_TRUE(HotspotSupport::resolve(&f->method, result)); - // Phase 1 accepted the name via the malloc fallback (no rejection, no - // fault) and reached phase 2, which called GetMethodID with the fully - // copied name -- proof the fallback copied all 600 bytes and - // NUL-terminated them correctly rather than truncating to the inline - // buffer or leaving it uninitialized. + // The fallback copied all 600 bytes and NUL-terminated them, rather than + // truncating to the inline buffer or leaving it uninitialized. + EXPECT_EQ(long_name, result.methodName()); + EXPECT_STREQ("()V", result.methodSignature()); + EXPECT_STREQ("java/lang/Object", result.klassName()); EXPECT_EQ(unreadable_before, Counters::getCounter(METHOD_RESOLVE_SYMBOL_UNREADABLE)); EXPECT_EQ(fault_before, Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED)); - // GetMethodID and GetStaticMethodID share this mock and both report "not - // found", so lookupMethodIdViaJni() falls through from one to the other -- - // two calls, both carrying the same (correct) captured name/signature. - EXPECT_EQ(2, fake_jni.getMethodIdCalls()); - EXPECT_EQ(long_name, fake_jni.lastMethodName()); - EXPECT_EQ("()V", fake_jni.lastMethodSignature()); EXPECT_FALSE(_pt->isProtected()); } // NOTE for reviewers: a companion test asserting the *hard* MAX_SYMBOL_LEN -// ceiling (`len >= MAX_SYMBOL_LEN` in ResolvedNames::setImpl) rejects a +// ceiling (`len >= MAX_SYMBOL_LEN` in ResolvedMethod::setName) rejects a // symbol is intentionally not included here. VMSymbol::length() returns // `unsigned short` (vmStructs.h), so the largest value it can ever produce is // 65535 -- which is already below MAX_SYMBOL_LEN (64 * 1024 = 65536). That @@ -727,7 +725,8 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveRejectsUnreadableSymbolAtMaxLen long long before = Counters::getCounter(METHOD_RESOLVE_SYMBOL_UNREADABLE); - EXPECT_EQ(nullptr, HotspotSupport::resolve(&f->method)); + ResolvedMethod result; + EXPECT_FALSE(HotspotSupport::resolve(&f->method, result)); EXPECT_EQ(before + 1, Counters::getCounter(METHOD_RESOLVE_SYMBOL_UNREADABLE)); EXPECT_FALSE(_pt->isProtected()); @@ -748,7 +747,8 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveReturnsNullForEmptySymbol) { long long before = Counters::getCounter(METHOD_RESOLVE_SYMBOL_UNREADABLE); - EXPECT_EQ(nullptr, HotspotSupport::resolve(&f->method)); + ResolvedMethod result; + EXPECT_FALSE(HotspotSupport::resolve(&f->method, result)); EXPECT_EQ(before + 1, Counters::getCounter(METHOD_RESOLVE_SYMBOL_UNREADABLE)); EXPECT_FALSE(_pt->isProtected()); @@ -760,10 +760,188 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveShortCircuitsSentinelWithoutPro HotspotMethodIdVMHotspotGuard hotspot; long long before = Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED); - EXPECT_EQ(nullptr, HotspotSupport::resolve((const void*)JMETHODID_NOT_WALKABLE)); + ResolvedMethod result; + EXPECT_FALSE(HotspotSupport::resolve((const void*)JMETHODID_NOT_WALKABLE, result)); EXPECT_EQ(before, Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED)); EXPECT_FALSE(_pt->isProtected()); } +// --------------------------------------------------------------------------- +// resolve(): names, line number table and Thread.run detection +// --------------------------------------------------------------------------- + +namespace { + +// Signed CompressedStream int, valid for small values only: single-byte +// UNSIGNED5 needs the zigzag-encoded value to stay below 191. +unsigned char encodeSmallSigned(int v, unsigned char base) { + unsigned int zigzag = ((unsigned int)v << 1) ^ (unsigned int)(v >> 31); + EXPECT_LT(zigzag, 191u); + return (unsigned char)(zigzag + base); +} + +// Compressed form of {(0,10), (4,11), (9,15), (12,13)}: escaped, inline, +// inline, escaped with a negative line delta. +std::vector sampleLineTable(unsigned char base) { + return { + 0xFF, encodeSmallSigned(0, base), encodeSmallSigned(10, base), + (4 << 3) | 1, + (5 << 3) | 4, + 0xFF, encodeSmallSigned(3, base), encodeSmallSigned(-2, base), + 0x00, + }; +} + +} // namespace + +TEST_F(HotspotResolveCrashProtectionTest, ResolveReturnsNamesWithoutLineNumberTable) { + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, 1, RESOLVE_KLASS_SUPER_OFFSET); + + ResolveFakes* f = new (_region) ResolveFakes{}; + f->link(); + ResolveFakes::setSymbol(f->name_sym, "compute"); + ResolveFakes::setSymbol(f->sig_sym, "(I)J"); + ResolveFakes::setSymbol(f->klass_sym, "com/example/Foo"); + f->const_method.code_size = 8; // has_linenumber_table bit stays clear + + ResolvedMethod result; + ASSERT_TRUE(HotspotSupport::resolve(&f->method, result)); + EXPECT_STREQ("compute", result.methodName()); + EXPECT_STREQ("(I)J", result.methodSignature()); + EXPECT_STREQ("com/example/Foo", result.klassName()); + EXPECT_EQ(nullptr, result.lineNumberTable()); + EXPECT_EQ(0, result.lineNumberTableSize()); + EXPECT_FALSE(result.isThreadRun()); + EXPECT_FALSE(_pt->isProtected()); +} + +// Both UNSIGNED5 flavours: base 0 (JDK < 20) and base 1 (JDK-8292758). +TEST_F(HotspotResolveCrashProtectionTest, ResolveDecodesLineNumberTable) { + for (int b : {0, 1}) { + SCOPED_TRACE(b); + const unsigned char base = (unsigned char)b; + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, base, RESOLVE_KLASS_SUPER_OFFSET); + + ResolveFakes* f = new (_region) ResolveFakes{}; + f->link(); + ResolveFakes::setSymbol(f->name_sym, "compute"); + ResolveFakes::setSymbol(f->sig_sym, "()V"); + ResolveFakes::setSymbol(f->klass_sym, "com/example/Foo"); + std::vector table = sampleLineTable(base); + f->setLineNumberTable(16, table.data(), table.size()); + + ResolvedMethod result; + ASSERT_TRUE(HotspotSupport::resolve(&f->method, result)); + ASSERT_EQ(4, result.lineNumberTableSize()); + const jvmtiLineNumberEntry* entries = result.lineNumberTable(); + ASSERT_NE(nullptr, entries); + const jlocation expected_bci[] = {0, 4, 9, 12}; + const jint expected_line[] = {10, 11, 15, 13}; + for (int i = 0; i < 4; i++) { + EXPECT_EQ(expected_bci[i], entries[i].start_location) << i; + EXPECT_EQ(expected_line[i], entries[i].line_number) << i; + } + + int size = 0; + jvmtiLineNumberEntry* owned = result.takeLineNumberTable(&size); + EXPECT_EQ(entries, owned); + EXPECT_EQ(4, size); + EXPECT_EQ(nullptr, result.lineNumberTable()); + free(owned); + } +} + +// A bci outside the bytecode means the metadata is not what it claims to be. +TEST_F(HotspotResolveCrashProtectionTest, ResolveRejectsLineNumberTableWithBciOutOfRange) { + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, 1, RESOLVE_KLASS_SUPER_OFFSET); + + ResolveFakes* f = new (_region) ResolveFakes{}; + f->link(); + ResolveFakes::setSymbol(f->name_sym, "compute"); + ResolveFakes::setSymbol(f->sig_sym, "()V"); + ResolveFakes::setSymbol(f->klass_sym, "com/example/Foo"); + std::vector table = sampleLineTable(1); + f->setLineNumberTable(12, table.data(), table.size()); // last bci is 12 + + long long before = Counters::getCounter(LINE_NUMBER_TABLE_UNREADABLE); + + ResolvedMethod result; + EXPECT_FALSE(HotspotSupport::resolve(&f->method, result)); + EXPECT_EQ(nullptr, result.lineNumberTable()); + EXPECT_EQ(before + 1, Counters::getCounter(LINE_NUMBER_TABLE_UNREADABLE)); + EXPECT_FALSE(_pt->isProtected()); +} + +// A table running off the readable page faults mid-decode; the landing pad +// must report failure (and free any partially built table). +TEST_F(HotspotResolveCrashProtectionTest, ResolveRecoversFromFaultInLineNumberTable) { + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, 1, RESOLVE_KLASS_SUPER_OFFSET); + + ResolveFakes* f = new (_region) ResolveFakes{}; + f->link(); + ResolveFakes::setSymbol(f->name_sym, "compute"); + ResolveFakes::setSymbol(f->sig_sym, "()V"); + ResolveFakes::setSymbol(f->klass_sym, "com/example/Foo"); + // code_size is a u2, so the table can start up to 64K past the bytecode + // base: point it into the guard page. + f->const_method.flags |= 1; + f->const_method.code_size = (uint16_t)(kPageSize() - 16); + + long long before = Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED); + + ResolvedMethod result; + EXPECT_FALSE(HotspotSupport::resolve(&f->method, result)); + EXPECT_EQ(before + 1, Counters::getCounter(METHOD_RESOLVE_FAULT_RECOVERED)); + EXPECT_FALSE(_pt->isProtected()); +} + +TEST_F(HotspotResolveCrashProtectionTest, ResolveDetectsRunOfThreadSubclass) { + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, 1, RESOLVE_KLASS_SUPER_OFFSET); + + ResolveFakes* f = new (_region) ResolveFakes{}; + f->link(); + ResolveFakes::setSymbol(f->name_sym, "run"); + ResolveFakes::setSymbol(f->sig_sym, "()V"); + ResolveFakes::setSymbol(f->klass_sym, "com/example/Worker"); + ResolveFakes::setSymbol(f->super_sym, "java/lang/Thread"); + f->super_klass.name_symbol = &f->super_sym; + f->klass.super_klass = &f->super_klass; + + { + ResolvedMethod result; + ASSERT_TRUE(HotspotSupport::resolve(&f->method, result)); + EXPECT_TRUE(result.isThreadRun()); + } + + // Same method on a class that does not extend Thread. + ResolveFakes::setSymbol(f->super_sym, "java/lang/Object"); + { + ResolvedMethod result; + ASSERT_TRUE(HotspotSupport::resolve(&f->method, result)); + EXPECT_FALSE(result.isThreadRun()); + } + EXPECT_FALSE(_pt->isProtected()); +} + #endif // __linux__ diff --git a/ddprof-test/build.gradle.kts b/ddprof-test/build.gradle.kts index 0ed73239cf..0ca3746bb5 100644 --- a/ddprof-test/build.gradle.kts +++ b/ddprof-test/build.gradle.kts @@ -114,7 +114,8 @@ val cleanLibCoverageData = tasks.register("cleanLibCoverageData") { } tasks.matching { it.name in coverageTaskNames }.configureEach { - dependsOn(cleanLibCoverageData) + // Registered by :ddprof-lib only with -PenableCoverage, as are these tasks. + dependsOn(":ddprof-lib:checkCoverageTools", cleanLibCoverageData) // The failed tests' results are cached like a pass would be, so a rerun must not be // UP-TO-DATE and silently skip the deferred failure (or the cleared .gcda counts). if (requestingCoverageReport) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java new file mode 100644 index 0000000000..d637199597 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java @@ -0,0 +1,301 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.cpu; + +import com.datadoghq.profiler.CStackAwareAbstractProfilerTest; +import com.datadoghq.profiler.JfrEvent; +import com.datadoghq.profiler.JfrEvents; +import com.datadoghq.profiler.JfrFrame; +import com.datadoghq.profiler.JfrStackTrace; +import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.junit.CStack; +import com.datadoghq.profiler.junit.RetryTest; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.TestTemplate; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.params.provider.ValueSource; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.ref.WeakReference; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Runs 20 CPU-bound threads, each with its own context class loader that defines a private + * copy of {@link LoaderLocalWorker}. Once every thread has exited, all 20 loaders (and so + * their classes) must be unloaded. Whether the recording is written before the unload or + * after it, every worker thread's stack traces must name its {@code LoaderLocalWorker} + * frames and contain no {@code unknown} method. + * + *

The loaders are not the system class loader, so their classes take the jmethodID path + * in every cstack mode. + */ +public class ContextClassLoaderUnloadTest extends CStackAwareAbstractProfilerTest { + + private static final String WORKER_CLASS = "com.datadoghq.profiler.cpu.LoaderLocalWorker"; + private static final String WORKER_SIMPLE_NAME = "LoaderLocalWorker"; + // Name the profiler gives a frame it could not resolve (flightRecorder.cpp). + private static final String UNKNOWN_METHOD = "unknown"; + private static final String THREAD_PREFIX = "ccl-worker-"; + private static final int THREADS = 20; + private static final long WORK_NANOS = TimeUnit.MILLISECONDS.toNanos(500); + private static final long UNLOAD_TIMEOUT_MS = 15_000; + + public ContextClassLoaderUnloadTest(@CStack String cstack) { + super(cstack); + } + + @Override + protected String getProfilerCommand() { + return "cpu=1ms"; + } + + /** + * Snapshot taken after the threads exit but while their loaders are still strongly held. + * The loaders must then unload. + */ + @RetryTest(3) + @TestTemplate + @ValueSource(strings = {"vm", "vmx", "fp", "dwarf"}) + @Timeout(60) + public void namesWorkerFramesBeforeLoadersUnload(@CStack String cstack) throws Exception { + Assumptions.assumeFalse(Platform.isJ9() || Platform.isZing()); + waitForProfilerReady(2000); + + List loaders = runWorkers(readClassBytes(WORKER_CLASS)); + + Path snapshot = Files.createTempFile("ccl-unload-" + cstack + "-", ".jfr"); + try { + dump(snapshot); + WorkerStacks stacks = WorkerStacks.collect(JfrEvents.load(snapshot, "datadog.ExecutionSample")); + System.out.println("[" + cstack + "] before unload: " + stacks); + stacks.assertResolved(); + } finally { + Files.deleteIfExists(snapshot); + } + + awaitUnload(weakRefs(loaders)); + } + + /** + * No dump until the loaders have unloaded, so every worker frame is resolved after its + * class is gone. + */ + @RetryTest(3) + @TestTemplate + @ValueSource(strings = {"vm", "vmx", "fp", "dwarf"}) + @Timeout(60) + public void namesWorkerFramesAfterLoadersUnload(@CStack String cstack) throws Exception { + Assumptions.assumeFalse(Platform.isJ9() || Platform.isZing()); + waitForProfilerReady(2000); + + List loaders = runWorkers(readClassBytes(WORKER_CLASS)); + awaitUnload(weakRefs(loaders)); + + stopProfiler(); + + WorkerStacks stacks = WorkerStacks.collect(verifyEvents("datadog.ExecutionSample")); + System.out.println("[" + cstack + "] after unload: " + stacks); + stacks.assertResolved(); + } + + /** Swaps the caller's strong references for weak ones; clears {@code loaders}. */ + private static List> weakRefs(List loaders) { + List> refs = new ArrayList<>(); + for (ClassLoader loader : loaders) { + refs.add(new WeakReference<>(loader)); + } + loaders.clear(); + return refs; + } + + /** Per-sample frame checks over the worker threads' samples in one recording. */ + private static final class WorkerStacks { + final Map samplesPerThread = new TreeMap<>(); + final Map threadsWithWorkerFrames = new TreeMap<>(); + final List unknownStacks = new ArrayList<>(); + final List unnamedStacks = new ArrayList<>(); + int samples; + + static WorkerStacks collect(JfrEvents events) { + WorkerStacks result = new WorkerStacks(); + for (JfrEvent sample : events) { + String thread = sample.getThreadName("eventThread"); + if (thread == null || !thread.startsWith(THREAD_PREFIX)) { + continue; + } + JfrStackTrace stackTrace = sample.getStackTrace(); + assertTrue(stackTrace != null && !stackTrace.isEmpty(), + "Worker sample on " + thread + " has no stack trace"); + result.samples++; + result.samplesPerThread.merge(thread, 1, Integer::sum); + boolean workerFrame = false; + boolean unknownFrame = false; + for (JfrFrame frame : stackTrace.frames()) { + String method = frame.methodName(); + String className = frame.className(); + if (UNKNOWN_METHOD.equals(method)) { + unknownFrame = true; + } else if (className != null && className.endsWith(WORKER_SIMPLE_NAME)) { + workerFrame = true; + } + } + if (unknownFrame && result.unknownStacks.size() < 3) { + result.unknownStacks.add(thread + ":\n" + sample.getStackTraceString()); + } + if (workerFrame) { + result.threadsWithWorkerFrames.merge(thread, 1, Integer::sum); + } else if (result.unnamedStacks.size() < 3) { + result.unnamedStacks.add(thread + ":\n" + sample.getStackTraceString()); + } + } + return result; + } + + void assertResolved() { + assertEquals(THREADS, samplesPerThread.size(), + "Expected CPU samples on all " + THREADS + " worker threads, got " + samplesPerThread.keySet()); + assertTrue(unknownStacks.isEmpty(), + "Worker stack traces contain " + UNKNOWN_METHOD + " frames, e.g.\n" + + String.join("\n---\n", unknownStacks)); + assertEquals(THREADS, threadsWithWorkerFrames.size(), + "Expected named " + WORKER_SIMPLE_NAME + " frames on all " + THREADS + + " worker threads, got " + threadsWithWorkerFrames.keySet() + + "; worker samples without them, e.g.\n" + String.join("\n---\n", unnamedStacks)); + } + + @Override + public String toString() { + return samples + " worker samples, named worker frames per thread " + threadsWithWorkerFrames + + ", stacks with " + UNKNOWN_METHOD + " frames " + (unknownStacks.isEmpty() ? 0 : ">0"); + } + } + + /** + * Starts the workers together, waits for them to exit, and returns their context class + * loaders. Once the caller drops the returned list, nothing keeps a loader reachable. + */ + private List runWorkers(byte[] workerBytes) throws Exception { + List loaders = new ArrayList<>(); + Thread[] threads = new Thread[THREADS]; + CountDownLatch start = new CountDownLatch(1); + AtomicReference failure = new AtomicReference<>(); + + for (int i = 0; i < THREADS; i++) { + ClassLoader loader = new ChildFirstLoader(WORKER_CLASS, workerBytes, getClass().getClassLoader()); + loaders.add(loader); + threads[i] = new Thread(() -> { + try { + start.await(); + // Load through the thread's own context loader, the way a container + // or plugin framework would. + ClassLoader ccl = Thread.currentThread().getContextClassLoader(); + Class workerClass = Class.forName(WORKER_CLASS, true, ccl); + if (workerClass.getClassLoader() != ccl) { + throw new AssertionError(WORKER_CLASS + " was not defined by the context loader"); + } + Runnable worker = (Runnable) workerClass.getConstructor(long.class).newInstance(WORK_NANOS); + worker.run(); + } catch (Throwable t) { + failure.compareAndSet(null, t); + } + }, THREAD_PREFIX + i); + threads[i].setContextClassLoader(loader); + } + + for (Thread thread : threads) { + thread.start(); + } + start.countDown(); + for (Thread thread : threads) { + thread.join(); + } + assertNull(failure.get(), () -> "Worker thread failed: " + failure.get()); + // An exited Thread still holds its context class loader; drop the threads too. + for (int i = 0; i < THREADS; i++) { + threads[i] = null; + } + return loaders; + } + + private static void awaitUnload(List> loaders) throws InterruptedException { + long deadline = System.currentTimeMillis() + UNLOAD_TIMEOUT_MS; + int alive = countAlive(loaders); + while (alive > 0 && System.currentTimeMillis() < deadline) { + System.gc(); + Thread.sleep(50); + alive = countAlive(loaders); + } + assertEquals(0, alive, alive + " of " + THREADS + + " context class loaders were still reachable " + UNLOAD_TIMEOUT_MS + "ms after their threads exited"); + } + + private static int countAlive(List> loaders) { + int alive = 0; + for (WeakReference ref : loaders) { + if (ref.get() != null) { + alive++; + } + } + return alive; + } + + private static byte[] readClassBytes(String className) throws IOException { + String resource = className.replace('.', '/') + ".class"; + try (InputStream in = ContextClassLoaderUnloadTest.class.getClassLoader().getResourceAsStream(resource)) { + assertNotNull(in, "Cannot find " + resource + " on the test classpath"); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[8192]; + for (int n; (n = in.read(buf)) != -1; ) { + out.write(buf, 0, n); + } + return out.toByteArray(); + } + } + + /** Defines its own copy of one class from bytes; delegates everything else to the parent. */ + private static final class ChildFirstLoader extends ClassLoader { + private final String ownedClass; + private final byte[] ownedBytes; + + ChildFirstLoader(String ownedClass, byte[] ownedBytes, ClassLoader parent) { + super(parent); + this.ownedClass = ownedClass; + this.ownedBytes = ownedBytes; + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (!ownedClass.equals(name)) { + return super.loadClass(name, resolve); + } + synchronized (getClassLoadingLock(name)) { + Class c = findLoadedClass(name); + if (c == null) { + c = defineClass(name, ownedBytes, 0, ownedBytes.length); + } + if (resolve) { + resolveClass(c); + } + return c; + } + } + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/LoaderLocalWorker.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/LoaderLocalWorker.java new file mode 100644 index 0000000000..c98f19f1c4 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/LoaderLocalWorker.java @@ -0,0 +1,63 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.cpu; + +/** + * CPU-bound workload for {@link ContextClassLoaderUnloadTest}. + * + *

This class is never run from the application class loader: each test thread's own + * context class loader defines a private copy from this class's bytes, so its frames + * belong to a class that is unloaded once that thread and its loader become unreachable. + * Do not reference it from the test by type -- only by name. + */ +public class LoaderLocalWorker implements Runnable { + // Written after each round so the JIT cannot drop the work as dead code. + public static volatile long sink; + + private final long durationNanos; + + public LoaderLocalWorker(long durationNanos) { + this.durationNanos = durationNanos; + } + + @Override + public void run() { + long deadline = System.nanoTime() + durationNanos; + long checksum = 0; + int limit = 50_000; + while (System.nanoTime() < deadline) { + checksum = mix(checksum, countPrimes(limit)); + // Vary the input so every round does fresh work. + limit = 50_000 + (int) (checksum & 0x3fff); + } + sink = checksum; + } + + // Sieve of Eratosthenes: the bulk of the CPU time, so most samples land here. + static int countPrimes(int limit) { + boolean[] composite = new boolean[limit + 1]; + int count = 0; + for (int i = 2; i <= limit; i++) { + if (!composite[i]) { + count++; + for (long j = (long) i * i; j <= limit; j += i) { + composite[(int) j] = true; + } + } + } + return count; + } + + // FNV-1a style fold of each round's result into a running checksum. + static long mix(long checksum, int value) { + long h = checksum ^ 0xcbf29ce484222325L; + for (int shift = 0; shift < 32; shift += 8) { + h ^= (value >>> shift) & 0xff; + h *= 0x100000001b3L; + } + return h; + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ContendedWallclockSamplesTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ContendedWallclockSamplesTest.java index b92ab864f0..5d44d80209 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ContendedWallclockSamplesTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ContendedWallclockSamplesTest.java @@ -81,6 +81,8 @@ public void test(@CStack String cstack) { String state = sample.getEnumName(THREAD_STATE); if ("CONTENDED".equals(state)) { String stackTrace = sample.getStackTraceString(); + System.out.println("StackTrace: "); + System.out.println(stackTrace); if (!stackTrace.endsWith(".GC_active()")) { // shortcut the assertions for sanitized runs // the samples are not that good, but it still makes sense to run this load under sanitizers diff --git a/utils/run-containers-tests.sh b/utils/run-containers-tests.sh index c6c90af25c..acafc70e0f 100755 --- a/utils/run-containers-tests.sh +++ b/utils/run-containers-tests.sh @@ -1254,6 +1254,10 @@ if $NEEDS_PRIVILEGED; then CONTAINER_CMD="$CONTAINER_CMD --privileged" fi CONTAINER_CMD="$CONTAINER_CMD $CONTAINER_PLATFORM" +# Podman caps containers at 2048 pids by default (Docker doesn't); thread-churn tests +# such as StartWithChurningThreadsTest exceed that, especially under the slower +# coverage build, and fail with "unable to create native thread". +CONTAINER_CMD="$CONTAINER_CMD --pids-limit=-1" CONTAINER_CMD="$CONTAINER_CMD -e LIBC=$LIBC" CONTAINER_CMD="$CONTAINER_CMD -e SANITIZER=$CONFIG" CONTAINER_CMD="$CONTAINER_CMD -e TEST_CONFIGURATION=$LIBC/${JDK_VERSION}-$CONFIG-$ARCH" From d80d1d693589e17e9c0fa066428969612ff05f0d Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 16 Oct 2026 08:45:08 +0000 Subject: [PATCH 07/10] Add performance counters for checkpoints --- ddprof-lib/src/main/cpp/counters.h | 7 ++++-- ddprof-lib/src/main/cpp/profiler.cpp | 12 +++++++--- ddprof-lib/src/main/cpp/profiler.h | 2 ++ ddprof-lib/src/main/cpp/samplerPerf.h | 3 ++- .../cpu/ContextClassLoaderUnloadTest.java | 23 +++++++++++++++++++ doc/reference/SamplerPerfCounters.md | 1 + 6 files changed, 42 insertions(+), 6 deletions(-) diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index ec6042440a..f7bfeda317 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -166,7 +166,8 @@ * in Profiler::checkFault(). */ \ X(STACKWALK_LONGJMP_RECOVERED, "stackwalk_longjmp_recovered") \ /* JFR chunks written from GarbageCollectionStart (Profiler::onGCStart()), \ - * and GCs that skipped it because a lock was taken. */ \ + * and GCs that skipped it because a lock was taken. The time it adds to \ + * GC pauses is sampler_ticks.gc_checkpoint, under -PenableSamplerPerf. */ \ X(GC_CHECKPOINTS, "gc_checkpoints") \ X(GC_CHECKPOINTS_SKIPPED, "gc_checkpoints_skipped") \ /* Dump-time method resolution failures. The frame serializes as "unknown". */ \ @@ -287,7 +288,9 @@ X(SAMPLER_TICKS_NATIVEMEM, "sampler_ticks.nativemem") \ X(SAMPLER_COUNT_NATIVEMEM, "sampler_count.nativemem") \ X(SAMPLER_TICKS_NATIVESOCKET, "sampler_ticks.nativesocket") \ - X(SAMPLER_COUNT_NATIVESOCKET, "sampler_count.nativesocket") + X(SAMPLER_COUNT_NATIVESOCKET, "sampler_count.nativesocket") \ + X(SAMPLER_TICKS_GC_CHECKPOINT, "sampler_ticks.gc_checkpoint") \ + X(SAMPLER_COUNT_GC_CHECKPOINT, "sampler_count.gc_checkpoint") #else #define DD_COUNTER_TABLE_SAMPLER_PERF(X) #endif diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 0186ce7f0e..6e5759b97b 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -2151,9 +2151,15 @@ void Profiler::onGCStart() { if (!_gc_checkpoint.load(std::memory_order_acquire)) { return; } + // Times everything this adds to the GC pause, skipped attempts included: + // their try-lock spins count too. + SAMPLER_PERF_PROBE(SP_GC_CHECKPOINT); + Counters::increment(tryGCCheckpoint() ? GC_CHECKPOINTS : GC_CHECKPOINTS_SKIPPED); +} + +bool Profiler::tryGCCheckpoint() { if (!_state_lock.tryLock()) { - Counters::increment(GC_CHECKPOINTS_SKIPPED); - return; + return false; } bool written = false; if (state() == RUNNING && tryLockAll()) { @@ -2186,7 +2192,7 @@ void Profiler::onGCStart() { } } _state_lock.unlock(); - Counters::increment(written ? GC_CHECKPOINTS : GC_CHECKPOINTS_SKIPPED); + return written; } void Profiler::switchThreadEvents(jvmtiEventMode mode) { diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 4125ec618c..3c51a196c1 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -193,6 +193,8 @@ class alignas(alignof(SpinLock)) Profiler { void lockAll(); // lockAll() that gives up, holding nothing, if any lock stays taken. bool tryLockAll(); + // onGCStart()'s checkpoint itself; false if it was skipped. + bool tryGCCheckpoint(); // Resets the dictionaries, the call trace storage and the counters for a // fresh recording; returns how many dictionaries were left unchanged because // their drain timed out. diff --git a/ddprof-lib/src/main/cpp/samplerPerf.h b/ddprof-lib/src/main/cpp/samplerPerf.h index c5ea6aa152..e8562459c7 100644 --- a/ddprof-lib/src/main/cpp/samplerPerf.h +++ b/ddprof-lib/src/main/cpp/samplerPerf.h @@ -120,7 +120,8 @@ X(ALLOC, "alloc") \ X(LIVENESS, "liveness") \ X(NATIVEMEM, "nativemem") \ - X(NATIVESOCKET, "nativesocket") + X(NATIVESOCKET, "nativesocket") \ + X(GC_CHECKPOINT, "gc_checkpoint") #define X_SAMPLER_ENUM(id, name) SP_##id, typedef enum SamplerId : int { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java index d637199597..270a083579 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java @@ -114,6 +114,29 @@ public void namesWorkerFramesAfterLoadersUnload(@CStack String cstack) throws Ex WorkerStacks stacks = WorkerStacks.collect(verifyEvents("datadog.ExecutionSample")); System.out.println("[" + cstack + "] after unload: " + stacks); stacks.assertResolved(); + assertGCCheckpointCounters(cstack); + } + + /** + * The GCs awaitUnload() forced must have written JFR checkpoints from + * GarbageCollectionStart. With -PenableSamplerPerf, they must also have been + * timed. Skipped on builds without counters. + */ + private void assertGCCheckpointCounters(String cstack) { + Map counters = profiler.getDebugCounters(); + Assumptions.assumeTrue(counters.containsKey("gc_checkpoints"), "Profiler built without counters"); + long checkpoints = counters.get("gc_checkpoints"); + long skipped = counters.getOrDefault("gc_checkpoints_skipped", 0L); + System.out.println("[" + cstack + "] GC checkpoints: " + checkpoints + " written, " + skipped + " skipped"); + assertTrue(checkpoints > 0, "No JFR checkpoint was written at GC start"); + + if (counters.containsKey("sampler_count.gc_checkpoint")) { + long timed = counters.get("sampler_count.gc_checkpoint"); + long timeNs = counters.getOrDefault("sampler_ticks.gc_checkpoint", 0L); + System.out.println("[" + cstack + "] GC checkpoint time: " + timeNs + " ns over " + timed + " GCs"); + assertEquals(checkpoints + skipped, timed, "Every GC-start checkpoint attempt should be timed"); + assertTrue(timeNs > 0, "GC checkpoint time was not recorded"); + } } /** Swaps the caller's strong references for weak ones; clears {@code loaders}. */ diff --git a/doc/reference/SamplerPerfCounters.md b/doc/reference/SamplerPerfCounters.md index a3af648f0c..bc95ceb580 100644 --- a/doc/reference/SamplerPerfCounters.md +++ b/doc/reference/SamplerPerfCounters.md @@ -54,6 +54,7 @@ and `JavaProfiler.getDebugCounters()`. The average is derived, not stored. | `liveness` | `LivenessTracker` | `track` | | `nativemem` | `MallocTracer` | `recordMalloc` | | `nativesocket` | `NativeSocketSampler` | `recordEvent` | +| `gc_checkpoint` | — (not a sampler: the JFR checkpoint written at GC start) | `Profiler::onGCStart` | Only one CPU engine and one wallclock engine is ever live in a run (`Profiler::selectCpuEngine` / `selectWallEngine`), so the variants share a slot; From 49825ffeb286a37666c871fb78dae458825b6481 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 16 Oct 2026 09:44:39 +0000 Subject: [PATCH 08/10] ResolveMethod instead of checkpoint --- .../src/main/cpp/callTraceHashTable.cpp | 17 +++ ddprof-lib/src/main/cpp/callTraceHashTable.h | 4 + ddprof-lib/src/main/cpp/callTraceStorage.cpp | 5 + ddprof-lib/src/main/cpp/callTraceStorage.h | 9 +- ddprof-lib/src/main/cpp/counters.h | 16 ++- ddprof-lib/src/main/cpp/flightRecorder.cpp | 134 ++++++++++-------- ddprof-lib/src/main/cpp/flightRecorder.h | 47 +++--- ddprof-lib/src/main/cpp/profiler.cpp | 82 ++++------- ddprof-lib/src/main/cpp/profiler.h | 15 +- ddprof-lib/src/main/cpp/samplerPerf.h | 2 +- ddprof-lib/src/main/cpp/vmEntry.cpp | 4 +- .../cpu/ContextClassLoaderUnloadTest.java | 37 ++--- doc/reference/SamplerPerfCounters.md | 2 +- 13 files changed, 209 insertions(+), 165 deletions(-) diff --git a/ddprof-lib/src/main/cpp/callTraceHashTable.cpp b/ddprof-lib/src/main/cpp/callTraceHashTable.cpp index 9e4e558062..b985af2e70 100644 --- a/ddprof-lib/src/main/cpp/callTraceHashTable.cpp +++ b/ddprof-lib/src/main/cpp/callTraceHashTable.cpp @@ -520,6 +520,23 @@ void CallTraceHashTable::collect(CallTraceSet &traces, std::function &visitor) { + for (LongHashTable *table = __atomic_load_n(&_table, __ATOMIC_ACQUIRE); + table != nullptr; table = table->prev()) { + u64 *keys = table->keys(); + CallTraceSample *values = table->values(); + u32 capacity = table->capacity(); + for (u32 slot = 0; slot < capacity; slot++) { + if (keys[slot] != 0) { + CallTrace *trace = values[slot].acquireTrace(); + if (trace != nullptr && trace != CallTraceSample::PREPARING) { + visitor(trace); + } + } + } + } +} + void CallTraceHashTable::putWithExistingId(CallTrace* source_trace, u64 weight) { // Trace preservation for standby tables (no contention with new puts) // This is safe because new put() operations go to the new active table diff --git a/ddprof-lib/src/main/cpp/callTraceHashTable.h b/ddprof-lib/src/main/cpp/callTraceHashTable.h index 8c1bae64d2..e71433f63f 100644 --- a/ddprof-lib/src/main/cpp/callTraceHashTable.h +++ b/ddprof-lib/src/main/cpp/callTraceHashTable.h @@ -161,6 +161,10 @@ class CallTraceHashTable { void clearAfterFailedDrain(); void collect(CallTraceSet &traces, std::function trace_hook = nullptr); + // Calls visitor on every published trace, like collect() but without + // gathering them (or the overflow trace) into a set. Same read-only + // contract as collect(). + void forEach(const std::function &visitor); u64 put(int num_frames, ASGCT_CallFrame *frames, bool truncated, u64 weight); void putWithExistingId(CallTrace* trace, u64 weight); // For standby tables with no contention diff --git a/ddprof-lib/src/main/cpp/callTraceStorage.cpp b/ddprof-lib/src/main/cpp/callTraceStorage.cpp index aadd96c1dc..3f22ffe169 100644 --- a/ddprof-lib/src/main/cpp/callTraceStorage.cpp +++ b/ddprof-lib/src/main/cpp/callTraceStorage.cpp @@ -172,6 +172,11 @@ u64 CallTraceStorage::put(int num_frames, ASGCT_CallFrame* frames, bool truncate * out and drained (see the class comment). * It is not designed to be called concurrently with itself. */ +void CallTraceStorage::forEachPendingTrace(const std::function &visitor) { + const_cast(__atomic_load_n(&_standby_storage, __ATOMIC_ACQUIRE))->forEach(visitor); + const_cast(__atomic_load_n(&_active_storage, __ATOMIC_ACQUIRE))->forEach(visitor); +} + void CallTraceStorage::processTraces(std::function processor) { // PHASE 1: Collect liveness information with simple lock (rare operation) { diff --git a/ddprof-lib/src/main/cpp/callTraceStorage.h b/ddprof-lib/src/main/cpp/callTraceStorage.h index b05b90bae2..a4c660f06a 100644 --- a/ddprof-lib/src/main/cpp/callTraceStorage.h +++ b/ddprof-lib/src/main/cpp/callTraceStorage.h @@ -96,10 +96,6 @@ class CallTraceStorage { // Register a liveness checker (rare operation - uses simple lock) void registerLivenessChecker(LivenessChecker checker); - bool hasLivenessCheckers() { - SharedLockGuard lock(&_liveness_lock); - return !_liveness_checkers.empty(); - } // Clear liveness checkers (rare operation - uses simple lock) void clearLivenessCheckers(); @@ -113,6 +109,11 @@ class CallTraceStorage { // Tolerates concurrent put() (see the class comment). void processTraces(std::function processor); + // Visits every trace the next processTraces() will hand out (the standby + // and active tables) without rotating or clearing anything. Requires put() + // and processTraces() to be excluded for the duration. + void forEachPendingTrace(const std::function &visitor); + #ifdef UNIT_TEST // The table put() currently targets. Compiled only into gtest binaries. CallTraceHashTable* activeTableForTest() { diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index f7bfeda317..c2bb4a8d99 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -165,11 +165,13 @@ /* Every siglongjmp recovery, from any protected window, counted centrally \ * in Profiler::checkFault(). */ \ X(STACKWALK_LONGJMP_RECOVERED, "stackwalk_longjmp_recovered") \ - /* JFR chunks written from GarbageCollectionStart (Profiler::onGCStart()), \ - * and GCs that skipped it because a lock was taken. The time it adds to \ - * GC pauses is sampler_ticks.gc_checkpoint, under -PenableSamplerPerf. */ \ - X(GC_CHECKPOINTS, "gc_checkpoints") \ - X(GC_CHECKPOINTS_SKIPPED, "gc_checkpoints_skipped") \ + /* GarbageCollectionStart passes that resolved pending samples' methods \ + * (Profiler::onGCStart()), GCs that skipped it because a lock was taken, \ + * and methods resolved. The time it adds to GC pauses is \ + * sampler_ticks.gc_resolve, under -PenableSamplerPerf. */ \ + X(GC_RESOLVE_PASSES, "gc_resolve_passes") \ + X(GC_RESOLVE_SKIPPED, "gc_resolve_skipped") \ + X(GC_RESOLVED_METHODS, "gc_resolved_methods") \ /* Dump-time method resolution failures. The frame serializes as "unknown". */ \ X(METHOD_RESOLVE_FAULT_RECOVERED, "method_resolve_fault_recovered") \ /* Symbol length/body rejected during the same resolution: unreadable body, \ @@ -289,8 +291,8 @@ X(SAMPLER_COUNT_NATIVEMEM, "sampler_count.nativemem") \ X(SAMPLER_TICKS_NATIVESOCKET, "sampler_ticks.nativesocket") \ X(SAMPLER_COUNT_NATIVESOCKET, "sampler_count.nativesocket") \ - X(SAMPLER_TICKS_GC_CHECKPOINT, "sampler_ticks.gc_checkpoint") \ - X(SAMPLER_COUNT_GC_CHECKPOINT, "sampler_count.gc_checkpoint") + X(SAMPLER_TICKS_GC_RESOLVE, "sampler_ticks.gc_resolve") \ + X(SAMPLER_COUNT_GC_RESOLVE, "sampler_count.gc_resolve") #else #define DD_COUNTER_TABLE_SAMPLER_PERF(X) #endif diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 51d8535519..054a8e6a80 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -510,22 +510,16 @@ void Lookup::fillJavaMethodNames(MethodInfo *mi, const char *class_name, mi->_sig = _symbols.lookup(method_sig); } -void Lookup::fillJavaMethodInfo(MethodInfo *mi, ResolvedMethod &resolved) { - const char *class_name = resolved.klassName(); - const char *method_name = resolved.methodName(); - const char *method_sig = resolved.methodSignature(); - - fillJavaMethodNames(mi, class_name, strlen(class_name), method_name, method_sig); - mi->_type = FRAME_INTERPRETED; - // Same entry-frame rules as the jmethodID path: Thread.run (or an override - // in a Thread subclass), or public static void main(String[]). - mi->_is_entry = resolved.isThreadRun() || - (strcmp(method_name, "main") == 0 && - strcmp(method_sig, "([Ljava/lang/String;)V") == 0); +// Same entry-frame rules as the jmethodID path: Thread.run (or an override in +// a Thread subclass), or public static void main(String[]). +static bool isEntryMethod(ResolvedMethod &resolved) { + return resolved.isThreadRun() || + (strcmp(resolved.methodName(), "main") == 0 && + strcmp(resolved.methodSignature(), "([Ljava/lang/String;)V") == 0); +} - // A raw Method* key can be reused by a different method after its class is - // unloaded, so unlike the jmethodID path the table is refreshed on every - // fill rather than only the first time. +// Moves resolved's line number table, if any, into mi. +static void takeLineNumberTable(MethodInfo *mi, ResolvedMethod &resolved) { if (resolved.lineNumberTable() != nullptr) { int size = resolved.lineNumberTableSize(); // Built before ownership moves, so a std::bad_alloc here leaves the table @@ -542,6 +536,48 @@ void Lookup::fillJavaMethodInfo(MethodInfo *mi, ResolvedMethod &resolved) { } } +void Lookup::fillJavaMethodInfo(MethodInfo *mi, ResolvedMethod &resolved) { + const char *class_name = resolved.klassName(); + fillJavaMethodNames(mi, class_name, strlen(class_name), resolved.methodName(), + resolved.methodSignature()); + mi->_type = FRAME_INTERPRETED; + mi->_is_entry = isEntryMethod(resolved); + // A raw Method* key can be reused by a different method after its class is + // unloaded, so unlike the jmethodID path the table is refreshed on every + // fill rather than only the first time. + takeLineNumberTable(mi, resolved); +} + +bool Lookup::resolveAtGC(MethodMap *method_map, const ASGCT_CallFrame &frame) { + jint bci = frame.bci; + jmethodID method_id = frame.method_id; + if (bci == BCI_ERROR || bci == BCI_NATIVE_FRAME || + bci == BCI_NATIVE_FRAME_REMOTE || bci == BCI_VTABLE_RECEIVER || + method_id == nullptr || method_id == JMETHODID_NOT_WALKABLE) { + return false; + } + unsigned long key = methodKey(frame, method_id, bci, 0); + MethodMap::iterator it = method_map->find(key); + if (it != method_map->end() && it->second._gc_resolved != nullptr) { + // Captured at an earlier GC since the last chunk. + return false; + } + // A jmethodID's Method* is read from its id slot: JVMTI is off limits here. + const void *method = FrameType::isRawPointer(bci) + ? frame.method + : (const void *)VMMethod::fromJmethodID(method_id); + ResolvedMethod resolved; + if (method == nullptr || !JVMSupport::resolve(method, resolved)) { + return false; + } + MethodInfo *mi = it != method_map->end() ? &it->second : &(*method_map)[key]; + mi->_gc_resolved = std::make_shared(GCResolvedMethod{ + resolved.klassName(), resolved.methodName(), resolved.methodSignature(), + isEntryMethod(resolved)}); + takeLineNumberTable(mi, resolved); + return true; +} + bool Lookup::resolveVTableReceiver(VMSymbol *sym, char *buf, size_t bufsize, u32 *out_class_id) { if (sym == nullptr || !SafeAccess::isReadable(sym)) { @@ -862,6 +898,16 @@ MethodInfo *Lookup::fillMethod(ASGCT_CallFrame &frame, jmethodID method_id, mi->_sig = _symbols.lookup("()V"); mi->_type = FRAME_NATIVE; mi->_is_entry = false; + } else if (mi->_gc_resolved != nullptr) { + // Captured at GC start while the class was still loaded; used as is, + // since the class may be gone by now. Consumed here: the next chunk + // resolves afresh. The line number table was set at capture. + const GCResolvedMethod &gc = *mi->_gc_resolved; + fillJavaMethodNames(mi, gc._class_name.c_str(), gc._class_name.size(), + gc._method_name.c_str(), gc._signature.c_str()); + mi->_type = FRAME_INTERPRETED; + mi->_is_entry = gc._is_entry; + mi->_gc_resolved.reset(); } else if (raw_pointer) { // resolve() installs its own (nested) landing pad around the metadata // walk; a stale Method* leaves this row unfilled, see below. @@ -869,15 +915,6 @@ MethodInfo *Lookup::fillMethod(ASGCT_CallFrame &frame, jmethodID method_id, if (filled) { fillJavaMethodInfo(mi, resolved); } - } else if (_rec != nullptr && _rec->_vmstructs_only) { - // Inside a GC pause (Recording::checkpointChunk()): JVMTI is off limits, - // so read the jmethodID's Method* and resolve that like a raw frame. A - // jmethodID that can't be dereferenced leaves the row unfilled. - const VMMethod* method = VMMethod::fromJmethodID(method_id); - filled = method != nullptr && JVMSupport::resolve(method, resolved); - if (filled) { - fillJavaMethodInfo(mi, resolved); - } } else { filled = fillJavaMethodInfo(mi, method_id, first_time, state); } @@ -1021,11 +1058,7 @@ off_t Recording::finishChunk(bool end_recording, bool do_cleanup) { // Note: this only guards against concurrent unloading that starts AFTER this // call. Classes already unloaded before finishChunk() was entered are not // present in the list and receive no protection here. - // - // Skipped by checkpointChunk(): it runs inside a GC pause, where JVMTI may not - // be called and no class can be unloaded concurrently anyway. - jvmtiError err = _vmstructs_only ? JVMTI_ERROR_WRONG_PHASE - : jvmti->GetLoadedClasses(&count, &classes); + jvmtiError err = jvmti->GetLoadedClasses(&count, &classes); flush(&_cpu_monitor_buf); @@ -1194,24 +1227,16 @@ void Recording::switchChunk(int fd) { flush(_buf); } -void Recording::checkpointChunk() { - _vmstructs_only = true; - _chunk_start = finishChunk(/*end_recording=*/false, /*do_cleanup=*/true); - _vmstructs_only = false; - - _start_time = _stop_time; - _start_ticks = _stop_ticks; - _bytes_written = 0; - - // Every chunk needs its own header, metadata and settings. Unlike - // switchChunk(), the file is not restarted, so the once-per-file info events - // stay in its first chunk -- writeJvmInfo() and writeSystemProperties() call - // JVMTI, which a GC callback may not -- and _recorded_lib_count carries on, - // so finishChunk() keeps writing only libraries loaded since. - writeHeader(_buf); - writeMetadata(_buf); - writeSettings(_buf, _args); - flush(_buf); +int Recording::resolvePendingMethods() { + int resolved = 0; + Profiler::instance()->forEachPendingTrace([&](const CallTrace *trace) { + for (int i = 0; i < trace->num_frames; i++) { + if (Lookup::resolveAtGC(&_method_map, trace->frames[i])) { + resolved++; + } + } + }); + return resolved; } void Recording::cpuMonitorCycle() { @@ -2779,22 +2804,17 @@ Error FlightRecorder::dump(const char *filename, const int length) { return Error("No active recording"); } -bool FlightRecorder::tryCheckpointBegin() { +bool FlightRecorder::tryResolvePendingMethods() { if (!_rec_lock.tryLock()) { return false; } - if (_rec == nullptr) { - _rec_lock.unlock(); - return false; + Recording* rec = _rec; + if (rec != nullptr) { + Counters::increment(GC_RESOLVED_METHODS, rec->resolvePendingMethods()); } - return true; -} - -void FlightRecorder::checkpointAndEnd() { - _rec->checkpointChunk(); _rec_lock.unlock(); + return rec != nullptr; } - void FlightRecorder::wallClockEpoch(int lock_index, WallClockEpochEvent *event) { OptionalSharedLockGuard locker(&_rec_lock); diff --git a/ddprof-lib/src/main/cpp/flightRecorder.h b/ddprof-lib/src/main/cpp/flightRecorder.h index eefcb6ea3d..b156974a44 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.h +++ b/ddprof-lib/src/main/cpp/flightRecorder.h @@ -9,6 +9,8 @@ #include #include +#include +#include #include #include #include @@ -118,6 +120,17 @@ class SharedLineNumberTable { ~SharedLineNumberTable(); }; +// A Java method's names, resolved at GC start (Lookup::resolveAtGC()) while +// its class was still loaded, for the next chunk to use even if the class has +// been unloaded by then. Plain strings, not dictionary ids: the ids are only +// assigned once that chunk is written. +struct GCResolvedMethod { + std::string _class_name; + std::string _method_name; + std::string _signature; + bool _is_entry; +}; + class MethodInfo { public: MethodInfo() @@ -135,6 +148,8 @@ class MethodInfo { jint _modifiers; std::shared_ptr _line_number_table; FrameTypeId _type; + // Set by Lookup::resolveAtGC(); consumed by the next fillMethod(). + std::shared_ptr _gc_resolved; inline jint getLineNumber(jint bci); inline bool isHidden(); @@ -311,10 +326,6 @@ class Recording { return value < 0 ? 0 : value > 1 ? 1 : value; } - // Set only for the duration of checkpointChunk(): finishChunk() skips the - // JVMTI class pinning, and Lookup resolves jmethodID frames through VMStructs. - bool _vmstructs_only = false; - public: Recording(int fd, Arguments &args); ~Recording(); @@ -324,11 +335,10 @@ class Recording { off_t finishChunk(bool end_recording, bool do_cleanup = false); void switchChunk(int fd); - // Finishes the current chunk in place and starts the next one in the same - // file, resolving every method from VM metadata alone (no JNI/JVMTI), so it - // can run inside the JVMTI GarbageCollectionStart callback -- while the - // classes the pending samples point at are still loaded. - void checkpointChunk(); + // Lookup::resolveAtGC() on every frame of every trace the next chunk will + // write. Caller holds Profiler::lockAll() (no trace writers) and _rec_lock. + // Returns the number of methods resolved. + int resolvePendingMethods(); void cpuMonitorCycle(); void appendRecording(const char *target_file, size_t size); @@ -551,8 +561,8 @@ class Lookup { // never dereferences it. So for every bci except BCI_VTABLE_RECEIVER -- whose // class_id the caller must resolve from a VMSymbol* first -- computing a key // touches no VM metadata and cannot fault. - unsigned long methodKey(const ASGCT_CallFrame &frame, jmethodID method_id, - jint bci, u32 vtable_class_id); + static unsigned long methodKey(const ASGCT_CallFrame &frame, jmethodID method_id, + jint bci, u32 vtable_class_id); // Resolves and fills in the MethodInfo for `frame`. This is the part that // reads VM metadata and may therefore fault; resolveMethod() wraps it in the // sigsetjmp/siglongjmp window. @@ -569,6 +579,12 @@ class Lookup { _symbols() {} MethodInfo *resolveMethod(ASGCT_CallFrame &frame); + // From the JVMTI GarbageCollectionStart callback (Recording:: + // resolvePendingMethods()): captures the names of a Java frame's method into + // its method_map row, reading VM metadata only -- no JNI/JVMTI. Returns true + // if it resolved one; false for non-Java frames, rows already captured, and + // metadata it could not read. + static bool resolveAtGC(MethodMap *method_map, const ASGCT_CallFrame &frame); u32 getPackage(const char *class_name); u32 getSymbol(const char *name); }; @@ -590,12 +606,9 @@ class FlightRecorder { Error start(Arguments &args, bool reset); void stop(); Error dump(const char *filename, const int length); - // Recording::checkpointChunk() on the active recording, split in two so the - // caller can do its own work in between. tryCheckpointBegin() never blocks: - // it returns true holding _rec_lock if there is a recording, and false - // holding nothing otherwise. checkpointAndEnd() writes and releases it. - bool tryCheckpointBegin(); - void checkpointAndEnd(); + // Recording::resolvePendingMethods() on the active recording, if _rec_lock + // is free. Never blocks. Returns false if it could not run. + bool tryResolvePendingMethods(); void wallClockEpoch(int lock_index, WallClockEpochEvent *event); void recordTraceRoot(int lock_index, int tid, TraceRootEvent *event); void recordQueueTime(int lock_index, int tid, QueueTimeEvent *event); diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 6e5759b97b..0fe2be9d96 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1788,19 +1788,17 @@ Error Profiler::start(Arguments &args, bool reset) { // directly, jmethodID frames (every frame with cstack=fp/dwarf, and classes // that still get one with cstack=vm) through the Method* their id slot // holds. So the method/class structs must be known and jmethodIDs - // dereferenceable. Liveness tracking is excluded: processCallTraces() runs - // its checker under a lock that is held across JNI calls. - bool gc_checkpoint = VM::isHotspot() && VMStructs::hasMethodStructs() && - VMStructs::hasClassNames() && - VMStructs::canDereferenceJmethodID() && - !_call_trace_storage.hasLivenessCheckers(); - if (gc_checkpoint) { + // dereferenceable. + bool gc_resolve = VM::isHotspot() && VMStructs::hasMethodStructs() && + VMStructs::hasClassNames() && + VMStructs::canDereferenceJmethodID(); + if (gc_resolve) { // Never disabled again: reference chains share the event, and - // onGCStart() checks _gc_checkpoint first. + // onGCStart() checks _gc_resolve first. VM::jvmti()->SetEventNotificationMode( JVMTI_ENABLE, JVMTI_EVENT_GARBAGE_COLLECTION_START, nullptr); } - _gc_checkpoint.store(gc_checkpoint, std::memory_order_release); + _gc_resolve.store(gc_resolve, std::memory_order_release); _state.store(RUNNING, std::memory_order_release); _start_time = time(NULL); @@ -1825,7 +1823,7 @@ Error Profiler::stop() { if (state() != RUNNING) { return Error("Profiler is not active"); } - _gc_checkpoint.store(false, std::memory_order_release); + _gc_resolve.store(false, std::memory_order_release); // Order matters: disable engines first so the _enabled check inside signal // handlers will fail for any new signal delivered from now on. drain() then @@ -2134,65 +2132,39 @@ bool Profiler::tryLockAll() { return true; } -// Writes a JFR checkpoint -- finishes the current chunk -- at the start of -// every GC, so samples of methods a GC is about to unload are serialized while -// their metadata is still there to read, rather than as /unknown at -// the next dump. +// At the start of every GC, resolves the methods of every pending sample -- +// the traces the next chunk will write -- while their classes are still +// loaded, and keeps the names on the method_map rows (Lookup::resolveAtGC()). +// Without that, a method whose class this GC unloads comes out as +// /unknown when the chunk is written. // // Runs in the JVMTI GarbageCollectionStart callback, with the VM stopped: no -// JNI, almost no JVMTI (Recording::checkpointChunk() resolves every method -// from VM metadata; start() enables this only where that works), and nothing -// may block, since a thread holding one of our locks may itself be parked for -// this pause. So every lock is try-locked, and a GC that finds one taken skips -// the checkpoint. Locks are taken in dump()'s order (_state_lock, _locks[], -// then the recording's), and all of them before rotating the dictionaries: a -// rotation must always be followed by the cpool write that consumes it. +// JNI, almost no JVMTI (resolution reads VM metadata only; start() enables +// this only where that works), and nothing may block, since a thread holding +// one of our locks may itself be parked for this pause. So every lock is +// try-locked, and a GC that finds one taken skips the pass. Locks are taken in +// dump()'s order: _state_lock, _locks[] (no trace writers), the recording's. void Profiler::onGCStart() { - if (!_gc_checkpoint.load(std::memory_order_acquire)) { + if (!_gc_resolve.load(std::memory_order_acquire)) { return; } // Times everything this adds to the GC pause, skipped attempts included: // their try-lock spins count too. - SAMPLER_PERF_PROBE(SP_GC_CHECKPOINT); - Counters::increment(tryGCCheckpoint() ? GC_CHECKPOINTS : GC_CHECKPOINTS_SKIPPED); + SAMPLER_PERF_PROBE(SP_GC_RESOLVE); + Counters::increment(tryResolveAtGC() ? GC_RESOLVE_PASSES : GC_RESOLVE_SKIPPED); } -bool Profiler::tryGCCheckpoint() { +bool Profiler::tryResolveAtGC() { if (!_state_lock.tryLock()) { return false; } - bool written = false; - if (state() == RUNNING && tryLockAll()) { - SignalBlocker blocker; - if (_jfr.tryCheckpointBegin()) { - if (!_class_map.rotate()) { - reportDrainTimeout("Class map reused a standby buffer it could not clear"); - } - if (!_string_label_map.rotate()) { - reportDrainTimeout("String label map reused a standby buffer it could not clear"); - } - if (!_context_value_map.rotate()) { - reportDrainTimeout("Context value map reused a standby buffer it could not clear"); - } - _jfr.checkpointAndEnd(); - written = true; - __atomic_add_fetch(&_epoch, 1, __ATOMIC_SEQ_CST); - unlockAll(); - if (!_class_map.clearStandby()) { - reportDrainTimeout("Class map standby buffer not cleared"); - } - if (!_string_label_map.clearStandby()) { - reportDrainTimeout("String label map standby buffer not cleared"); - } - if (!_context_value_map.clearStandby()) { - reportDrainTimeout("Context value map standby buffer not cleared"); - } - } else { - unlockAll(); - } + bool done = false; + if (state() == RUNNING && !_omit_stacktraces && tryLockAll()) { + done = _jfr.tryResolvePendingMethods(); + unlockAll(); } _state_lock.unlock(); - return written; + return done; } void Profiler::switchThreadEvents(jvmtiEventMode mode) { diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 3c51a196c1..98a0cbfb43 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -148,8 +148,8 @@ class alignas(alignof(SpinLock)) Profiler { int _safe_mode; CStack _cstack; bool _force_jmethodID; - // Whether onGCStart() writes a JFR checkpoint; see onGCStart(). - std::atomic _gc_checkpoint{false}; + // Whether onGCStart() resolves pending methods; see onGCStart(). + std::atomic _gc_resolve{false}; volatile jvmtiEventMode _thread_events_state; @@ -193,8 +193,8 @@ class alignas(alignof(SpinLock)) Profiler { void lockAll(); // lockAll() that gives up, holding nothing, if any lock stays taken. bool tryLockAll(); - // onGCStart()'s checkpoint itself; false if it was skipped. - bool tryGCCheckpoint(); + // onGCStart()'s resolution pass itself; false if it was skipped. + bool tryResolveAtGC(); // Resets the dictionaries, the call trace storage and the counters for a // fresh recording; returns how many dictionaries were left unchanged because // their drain timed out. @@ -349,6 +349,13 @@ class alignas(alignof(SpinLock)) Profiler { const char* cstack() const; int lookupClass(const char *key, size_t length); + // See CallTraceStorage::forEachPendingTrace(); caller holds lockAll(). + void forEachPendingTrace(const std::function &visitor) { + if (!_omit_stacktraces) { + _call_trace_storage.forEachPendingTrace(visitor); + } + } + void processCallTraces(std::function processor) { if (!_omit_stacktraces) { _call_trace_storage.processTraces(processor); diff --git a/ddprof-lib/src/main/cpp/samplerPerf.h b/ddprof-lib/src/main/cpp/samplerPerf.h index e8562459c7..ac5ac6aaf1 100644 --- a/ddprof-lib/src/main/cpp/samplerPerf.h +++ b/ddprof-lib/src/main/cpp/samplerPerf.h @@ -121,7 +121,7 @@ X(LIVENESS, "liveness") \ X(NATIVEMEM, "nativemem") \ X(NATIVESOCKET, "nativesocket") \ - X(GC_CHECKPOINT, "gc_checkpoint") + X(GC_RESOLVE, "gc_resolve") #define X_SAMPLER_ENUM(id, name) SP_##id, typedef enum SamplerId : int { diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index 7ed87ec503..6466153eb2 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -457,8 +457,8 @@ void JNICALL ForwardedGarbageCollectionFinish(jvmtiEnv *jvmti_env) { } // Same fan-out for GarbageCollectionStart: the reference-chain epoch counter -// and the profiler's JFR checkpoint (Profiler::onGCStart()). Each callee -// checks whether it is active. +// and the profiler's pending-method resolution (Profiler::onGCStart()). Each +// callee checks whether it is active. void JNICALL ForwardedGarbageCollectionStart(jvmtiEnv *jvmti_env) { ReferenceChainTracker::GarbageCollectionStart(jvmti_env); Profiler::instance()->onGCStart(); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java index 270a083579..ad47f3c765 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java @@ -114,28 +114,31 @@ public void namesWorkerFramesAfterLoadersUnload(@CStack String cstack) throws Ex WorkerStacks stacks = WorkerStacks.collect(verifyEvents("datadog.ExecutionSample")); System.out.println("[" + cstack + "] after unload: " + stacks); stacks.assertResolved(); - assertGCCheckpointCounters(cstack); + assertGCResolveCounters(cstack); } /** - * The GCs awaitUnload() forced must have written JFR checkpoints from - * GarbageCollectionStart. With -PenableSamplerPerf, they must also have been - * timed. Skipped on builds without counters. + * The GCs awaitUnload() forced must have resolved the pending samples' + * methods from GarbageCollectionStart. With -PenableSamplerPerf, every pass + * must also have been timed. Skipped on builds without counters. */ - private void assertGCCheckpointCounters(String cstack) { + private void assertGCResolveCounters(String cstack) { Map counters = profiler.getDebugCounters(); - Assumptions.assumeTrue(counters.containsKey("gc_checkpoints"), "Profiler built without counters"); - long checkpoints = counters.get("gc_checkpoints"); - long skipped = counters.getOrDefault("gc_checkpoints_skipped", 0L); - System.out.println("[" + cstack + "] GC checkpoints: " + checkpoints + " written, " + skipped + " skipped"); - assertTrue(checkpoints > 0, "No JFR checkpoint was written at GC start"); - - if (counters.containsKey("sampler_count.gc_checkpoint")) { - long timed = counters.get("sampler_count.gc_checkpoint"); - long timeNs = counters.getOrDefault("sampler_ticks.gc_checkpoint", 0L); - System.out.println("[" + cstack + "] GC checkpoint time: " + timeNs + " ns over " + timed + " GCs"); - assertEquals(checkpoints + skipped, timed, "Every GC-start checkpoint attempt should be timed"); - assertTrue(timeNs > 0, "GC checkpoint time was not recorded"); + Assumptions.assumeTrue(counters.containsKey("gc_resolve_passes"), "Profiler built without counters"); + long passes = counters.get("gc_resolve_passes"); + long skipped = counters.getOrDefault("gc_resolve_skipped", 0L); + long methods = counters.getOrDefault("gc_resolved_methods", 0L); + System.out.println("[" + cstack + "] GC resolve: " + passes + " passes, " + skipped + " skipped, " + + methods + " methods resolved"); + assertTrue(passes > 0, "No pending-method resolution ran at GC start"); + assertTrue(methods > 0, "No method was resolved at GC start"); + + if (counters.containsKey("sampler_count.gc_resolve")) { + long timed = counters.get("sampler_count.gc_resolve"); + long timeNs = counters.getOrDefault("sampler_ticks.gc_resolve", 0L); + System.out.println("[" + cstack + "] GC resolve time: " + timeNs + " ns over " + timed + " GCs"); + assertEquals(passes + skipped, timed, "Every GC-start resolution attempt should be timed"); + assertTrue(timeNs > 0, "GC resolve time was not recorded"); } } diff --git a/doc/reference/SamplerPerfCounters.md b/doc/reference/SamplerPerfCounters.md index bc95ceb580..bc67dd3003 100644 --- a/doc/reference/SamplerPerfCounters.md +++ b/doc/reference/SamplerPerfCounters.md @@ -54,7 +54,7 @@ and `JavaProfiler.getDebugCounters()`. The average is derived, not stored. | `liveness` | `LivenessTracker` | `track` | | `nativemem` | `MallocTracer` | `recordMalloc` | | `nativesocket` | `NativeSocketSampler` | `recordEvent` | -| `gc_checkpoint` | — (not a sampler: the JFR checkpoint written at GC start) | `Profiler::onGCStart` | +| `gc_resolve` | — (not a sampler: resolving pending samples' methods at GC start) | `Profiler::onGCStart` | Only one CPU engine and one wallclock engine is ever live in a run (`Profiler::selectCpuEngine` / `selectWallEngine`), so the variants share a slot; From ecbbf21a715d90fb541eb5a4b91313c039b74d15 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 16 Oct 2026 12:40:12 +0000 Subject: [PATCH 09/10] Reduce memory footprint --- ddprof-lib/src/main/cpp/counters.h | 7 +- ddprof-lib/src/main/cpp/flightRecorder.cpp | 159 +++++++++++++-- ddprof-lib/src/main/cpp/flightRecorder.h | 80 ++++++-- .../src/main/cpp/hotspot/hotspotSupport.cpp | 119 ++++++++++- .../src/main/cpp/hotspot/hotspotSupport.h | 24 +++ ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp | 7 + ddprof-lib/src/main/cpp/hotspot/vmStructs.h | 4 + ddprof-lib/src/main/cpp/jvmSupport.cpp | 1 + ddprof-lib/src/main/cpp/nativeMem.h | 1 + .../src/test/cpp/hotspotMethodId_ut.cpp | 189 ++++++++++++++++++ .../cpu/ContextClassLoaderUnloadTest.java | 22 ++ 11 files changed, 574 insertions(+), 39 deletions(-) diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index c2bb4a8d99..e42c6912c7 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -167,11 +167,16 @@ X(STACKWALK_LONGJMP_RECOVERED, "stackwalk_longjmp_recovered") \ /* GarbageCollectionStart passes that resolved pending samples' methods \ * (Profiler::onGCStart()), GCs that skipped it because a lock was taken, \ - * and methods resolved. The time it adds to GC pauses is \ + * and methods whose names a pass kept. The time it adds to GC pauses is \ * sampler_ticks.gc_resolve, under -PenableSamplerPerf. */ \ X(GC_RESOLVE_PASSES, "gc_resolve_passes") \ X(GC_RESOLVE_SKIPPED, "gc_resolve_skipped") \ X(GC_RESOLVED_METHODS, "gc_resolved_methods") \ + /* Method rows a chunk filled from names kept by an earlier resolution \ + * (CachedMethodNames), and kept names dropped because the row's raw \ + * Method* had been reused by another method. */ \ + X(METHOD_NAMES_CACHE_HITS, "method_names_cache_hits") \ + X(METHOD_NAMES_CACHE_STALE, "method_names_cache_stale") \ /* Dump-time method resolution failures. The frame serializes as "unknown". */ \ X(METHOD_RESOLVE_FAULT_RECOVERED, "method_resolve_fault_recovered") \ /* Symbol length/body rejected during the same resolution: unreadable body, \ diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 054a8e6a80..3c122b0332 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -254,6 +254,69 @@ void Lookup::cutArguments(char *func) { } } +size_t CachedMethodNames::size() const { + return offsetof(CachedMethodNames, _data) + _class_name_len + 1 + + _method_name_len + 1 + _signature_len + 1; +} + +CachedMethodNames *CachedMethodNames::create(const void *fingerprint, + const char *class_name, + size_t class_name_len, + const char *method_name, + const char *signature, + bool is_entry) { + size_t method_name_len = strlen(method_name); + size_t signature_len = strlen(signature); + if (class_name_len > USHRT_MAX || method_name_len > USHRT_MAX || + signature_len > USHRT_MAX) { + return nullptr; + } + size_t size = offsetof(CachedMethodNames, _data) + class_name_len + 1 + + method_name_len + 1 + signature_len + 1; + void *mem = malloc(size); + if (mem == nullptr) { + return nullptr; + } + NativeMem::recordAlloc(NM_METHOD_NAMES, mem, size); + CachedMethodNames *names = new (mem) CachedMethodNames(); + names->_fingerprint = fingerprint; + names->_class_name_len = (u16)class_name_len; + names->_method_name_len = (u16)method_name_len; + names->_signature_len = (u16)signature_len; + names->_is_entry = is_entry; + char *p = names->_data; + memcpy(p, class_name, class_name_len); + p[class_name_len] = '\0'; + p += class_name_len + 1; + memcpy(p, method_name, method_name_len + 1); + p += method_name_len + 1; + memcpy(p, signature, signature_len + 1); + return names; +} + +void CachedMethodNames::destroy(CachedMethodNames *names) { + if (names != nullptr) { + NativeMem::recordFreeBefore(NM_METHOD_NAMES, names, names->size()); + free(names); + } +} + +// Keeps mi's names for later chunks -- unless its class is never unloaded, +// which marks the row instead (see CachedMethodNames). fingerprint is nullptr +// for a jmethodID row. +static void keepNames(MethodInfo *mi, const void *class_loader_data, + const void *fingerprint, const char *class_name, + size_t class_name_len, const char *method_name, + const char *method_sig, bool is_entry) { + if (HotspotSupport::isPermanentLoaderData(class_loader_data)) { + mi->_never_unloaded = true; + mi->_cached_names.reset(); + return; + } + mi->_cached_names.reset(CachedMethodNames::create( + fingerprint, class_name, class_name_len, method_name, method_sig, is_entry)); +} + bool Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method, bool first_time, ResolveMethodState& state) { JNIEnv *jni = VM::jni(); @@ -388,6 +451,12 @@ bool Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method, fillJavaMethodNames(mi, normalized_class_name, normalized_class_name_len, method_name, method_sig); + // HotSpot never reuses a jmethodID; J9 frees them on class unload. + if (VM::isHotspot() && !mi->_never_unloaded) { + keepNames(mi, HotspotSupport::classLoaderDataOf(jni, method_class), + /*fingerprint=*/nullptr, normalized_class_name, + normalized_class_name_len, method_name, method_sig, entry); + } } else { Counters::increment(JMETHODID_SKIPPED); mi->_class = _classes->lookupDuringDump("", 0, Profiler::maxClassMapSize()); @@ -536,6 +605,42 @@ static void takeLineNumberTable(MethodInfo *mi, ResolvedMethod &resolved) { } } +static void keepNames(MethodInfo *mi, const void *fingerprint, + ResolvedMethod &resolved) { + const char *class_name = resolved.klassName(); + keepNames(mi, resolved.classLoaderData(), fingerprint, class_name, + strlen(class_name), resolved.methodName(), + resolved.methodSignature(), isEntryMethod(resolved)); +} + +const void *Lookup::methodFingerprint(const ASGCT_CallFrame &frame) { + return VMMethod::constMethodOf(frame.method); +} + +bool Lookup::useCachedNames(MethodInfo *mi, const ASGCT_CallFrame &frame) { + if (mi->_cached_names == nullptr) { + return false; + } + if (FrameType::isRawPointer(frame.bci)) { + // An unreadable fingerprint means the class is gone and nothing has taken + // its place, so the cached names are still the right ones for its samples. + const void *fingerprint = methodFingerprint(frame); + if (fingerprint != nullptr && fingerprint != mi->_cached_names->fingerprint()) { + // The Method* now belongs to a different method. + Counters::increment(METHOD_NAMES_CACHE_STALE); + mi->_cached_names.reset(); + return false; + } + } + const CachedMethodNames &names = *mi->_cached_names; + fillJavaMethodNames(mi, names.className(), names.classNameLength(), + names.methodName(), names.signature()); + mi->_type = FRAME_INTERPRETED; + mi->_is_entry = names.isEntry(); + Counters::increment(METHOD_NAMES_CACHE_HITS); + return true; +} + void Lookup::fillJavaMethodInfo(MethodInfo *mi, ResolvedMethod &resolved) { const char *class_name = resolved.klassName(); fillJavaMethodNames(mi, class_name, strlen(class_name), resolved.methodName(), @@ -558,24 +663,39 @@ bool Lookup::resolveAtGC(MethodMap *method_map, const ASGCT_CallFrame &frame) { } unsigned long key = methodKey(frame, method_id, bci, 0); MethodMap::iterator it = method_map->find(key); - if (it != method_map->end() && it->second._gc_resolved != nullptr) { - // Captured at an earlier GC since the last chunk. + if (it != method_map->end() && it->second._never_unloaded) { + // Resolvable from VM metadata whenever its chunk is written. + return false; + } + const bool raw_pointer = FrameType::isRawPointer(bci); + const void *fingerprint = nullptr; + if (raw_pointer) { + fingerprint = methodFingerprint(frame); + if (fingerprint == nullptr) { + // Nothing to resolve from (or to tell a reused Method* by). + return false; + } + } + if (it != method_map->end() && it->second._cached_names != nullptr && + it->second._cached_names->fingerprint() == fingerprint) { + // Kept names still valid (always, for a jmethodID). + return false; + } + // A jmethodID's Method* is read from its id slot, since JVMTI is off limits + // here. + const void *method = raw_pointer ? frame.method + : (const void *)VMMethod::fromJmethodID(method_id); + if (method == nullptr) { return false; } - // A jmethodID's Method* is read from its id slot: JVMTI is off limits here. - const void *method = FrameType::isRawPointer(bci) - ? frame.method - : (const void *)VMMethod::fromJmethodID(method_id); ResolvedMethod resolved; - if (method == nullptr || !JVMSupport::resolve(method, resolved)) { + if (!JVMSupport::resolve(method, resolved)) { return false; } MethodInfo *mi = it != method_map->end() ? &it->second : &(*method_map)[key]; - mi->_gc_resolved = std::make_shared(GCResolvedMethod{ - resolved.klassName(), resolved.methodName(), resolved.methodSignature(), - isEntryMethod(resolved)}); + keepNames(mi, fingerprint, resolved); takeLineNumberTable(mi, resolved); - return true; + return mi->_cached_names != nullptr; } bool Lookup::resolveVTableReceiver(VMSymbol *sym, char *buf, size_t bufsize, @@ -898,21 +1018,18 @@ MethodInfo *Lookup::fillMethod(ASGCT_CallFrame &frame, jmethodID method_id, mi->_sig = _symbols.lookup("()V"); mi->_type = FRAME_NATIVE; mi->_is_entry = false; - } else if (mi->_gc_resolved != nullptr) { - // Captured at GC start while the class was still loaded; used as is, - // since the class may be gone by now. Consumed here: the next chunk - // resolves afresh. The line number table was set at capture. - const GCResolvedMethod &gc = *mi->_gc_resolved; - fillJavaMethodNames(mi, gc._class_name.c_str(), gc._class_name.size(), - gc._method_name.c_str(), gc._signature.c_str()); - mi->_type = FRAME_INTERPRETED; - mi->_is_entry = gc._is_entry; - mi->_gc_resolved.reset(); + } else if (useCachedNames(mi, frame)) { + // Names kept from an earlier resolution of this same method, possibly + // before its class was unloaded. The line number table was kept too. } else if (raw_pointer) { // resolve() installs its own (nested) landing pad around the metadata // walk; a stale Method* leaves this row unfilled, see below. filled = JVMSupport::resolve(frame.method, resolved); if (filled) { + const void *fingerprint = methodFingerprint(frame); + if (fingerprint != nullptr && !mi->_never_unloaded) { + keepNames(mi, fingerprint, resolved); + } fillJavaMethodInfo(mi, resolved); } } else { diff --git a/ddprof-lib/src/main/cpp/flightRecorder.h b/ddprof-lib/src/main/cpp/flightRecorder.h index b156974a44..71ccee6448 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.h +++ b/ddprof-lib/src/main/cpp/flightRecorder.h @@ -120,26 +120,67 @@ class SharedLineNumberTable { ~SharedLineNumberTable(); }; -// A Java method's names, resolved at GC start (Lookup::resolveAtGC()) while -// its class was still loaded, for the next chunk to use even if the class has -// been unloaded by then. Plain strings, not dictionary ids: the ids are only -// assigned once that chunk is written. -struct GCResolvedMethod { - std::string _class_name; - std::string _method_name; - std::string _signature; +// A Java method's names, kept on its method_map row so that later chunks can +// still name it once its class is unloaded. Captured by the first successful +// resolution: at GC start (Lookup::resolveAtGC()) or when a chunk is written +// (fillMethod()). Plain strings, not dictionary ids: those are only valid +// within one chunk. +// +// Kept only for classes that can be unloaded. One defined by the boot, +// platform or application class loader never is, so its row is marked +// _never_unloaded instead: its names can always be read again. +// +// A jmethodID is never reused, so the names of a jmethodID row always stay +// valid. A raw Method* can be reused by a different method once its class is +// unloaded, so for those rows _fingerprint records which method the names +// belong to, and Lookup::useCachedNames() checks it first. +// +// One allocation: this header, then the three NUL-terminated strings. Counted +// under NM_METHOD_NAMES. +class CachedMethodNames { +public: + static CachedMethodNames *create(const void *fingerprint, const char *class_name, + size_t class_name_len, const char *method_name, + const char *signature, bool is_entry); + static void destroy(CachedMethodNames *names); + + struct Deleter { + void operator()(CachedMethodNames *names) const { destroy(names); } + }; + + const char *className() const { return _data; } + size_t classNameLength() const { return _class_name_len; } + const char *methodName() const { return _data + _class_name_len + 1; } + const char *signature() const { return methodName() + _method_name_len + 1; } + bool isEntry() const { return _is_entry; } + // Raw Method* rows: Lookup::methodFingerprint(). jmethodID rows: nullptr. + const void *fingerprint() const { return _fingerprint; } + +private: + CachedMethodNames() = default; + size_t size() const; + + const void *_fingerprint; + // Each a Symbol, whose length is a u2. + u16 _class_name_len; + u16 _method_name_len; + u16 _signature_len; bool _is_entry; + char _data[1]; }; class MethodInfo { public: MethodInfo() - : _mark(false), _is_entry(false), _referenced(false), _age(0), _key(0), _class(0), - _name(0), _sig(0), _modifiers(0), _line_number_table(nullptr), _type() {} + : _mark(false), _is_entry(false), _referenced(false), _never_unloaded(false), _age(0), + _key(0), _class(0), _name(0), _sig(0), _modifiers(0), _line_number_table(nullptr), + _type() {} bool _mark; bool _is_entry; bool _referenced; // Tracked during writeStackTraces() for cleanup + // The method's class can't be unloaded, so it gets no _cached_names. + bool _never_unloaded; int _age; // Consecutive chunks without reference (0 = recently used) u32 _key; u32 _class; @@ -148,8 +189,7 @@ class MethodInfo { jint _modifiers; std::shared_ptr _line_number_table; FrameTypeId _type; - // Set by Lookup::resolveAtGC(); consumed by the next fillMethod(). - std::shared_ptr _gc_resolved; + std::unique_ptr _cached_names; inline jint getLineNumber(jint bci); inline bool isHidden(); @@ -524,6 +564,13 @@ class Lookup { // ("java/lang/Object"), not necessarily NUL-terminated at class_name_len. void fillJavaMethodNames(MethodInfo *mi, const char *class_name, size_t class_name_len, const char *method_name, const char *method_sig); + // Fills mi from its cached names if they still belong to the method frame + // refers to; otherwise drops them. Returns whether it filled mi. + bool useCachedNames(MethodInfo *mi, const ASGCT_CallFrame &frame); + // Which method a raw Method* frame's key refers to right now: its + // ConstMethod*. A Method* reused by another method yields another + // ConstMethod*. nullptr if it can't be read -- the class is gone. + static const void *methodFingerprint(const ASGCT_CallFrame &frame); bool has_prefix(const char *str, const char *prefix) const { return strncmp(str, prefix, strlen(prefix)) == 0; } @@ -580,10 +627,11 @@ class Lookup { MethodInfo *resolveMethod(ASGCT_CallFrame &frame); // From the JVMTI GarbageCollectionStart callback (Recording:: - // resolvePendingMethods()): captures the names of a Java frame's method into - // its method_map row, reading VM metadata only -- no JNI/JVMTI. Returns true - // if it resolved one; false for non-Java frames, rows already captured, and - // metadata it could not read. + // resolvePendingMethods()): keeps the names of a Java frame's method on its + // method_map row, reading VM metadata only -- no JNI/JVMTI. Returns true if + // it kept them; false for non-Java frames, methods whose class is never + // unloaded, rows whose kept names are still valid, and metadata it could + // not read. static bool resolveAtGC(MethodMap *method_map, const ASGCT_CallFrame &frame); u32 getPackage(const char *class_name); u32 getSymbol(const char *name); diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index ce9b7b4028..7a24bd95a8 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include #include #include #include "asyncSampleMutex.h" @@ -1422,7 +1423,115 @@ static bool isHiddenClassBySignature(const char* signature) { return slash != nullptr && slash[1] >= '0' && slash[1] <= '9'; } +// ClassLoaderData of the boot, platform and application class loaders; see +// HotspotSupport::isPermanentLoaderData(). +enum { PERMANENT_BOOT, PERMANENT_PLATFORM, PERMANENT_APPLICATION, PERMANENT_COUNT }; +static std::atomic PERMANENT_LOADER_DATA[PERMANENT_COUNT]; + +static bool permanentLoaderDataComplete() { + return PERMANENT_LOADER_DATA[PERMANENT_APPLICATION].load(std::memory_order_acquire) != nullptr && + (JAVA_PLATFORM_CLASSLOADER == nullptr || // JDK 8 + PERMANENT_LOADER_DATA[PERMANENT_PLATFORM].load(std::memory_order_acquire) != nullptr); +} + +// fromJavaClass() needs java.lang.Class's klass field, part of hasClassNames(). +static bool canReadLoaderData() { + return VMStructs::class_loader_data_offset() >= 0 && VMStructs::hasClassNames(); +} + +const void* HotspotSupport::classLoaderDataOf(JNIEnv* jni, jclass klass) { + if (!canReadLoaderData()) { + return nullptr; + } + VMKlass* vmklass = VMKlass::fromJavaClass(jni, klass); + // As in readMethod(): the field's value, not a typed VMClassLoaderData*. + return vmklass != nullptr + ? *(const void* const*)((const char*)vmklass + VMStructs::class_loader_data_offset()) + : nullptr; +} + +void HotspotSupport::initPermanentLoaderData(jvmtiEnv* jvmti, JNIEnv* jni) { + if (!canReadLoaderData()) { + return; + } + initClassloaderInfo(jni); + if (PERMANENT_LOADER_DATA[PERMANENT_BOOT].load(std::memory_order_acquire) == nullptr) { + jclass object_class = jni->FindClass("java/lang/Object"); + if (object_class != nullptr) { + PERMANENT_LOADER_DATA[PERMANENT_BOOT].store(classLoaderDataOf(jni, object_class), + std::memory_order_release); + jni->DeleteLocalRef(object_class); + } else { + jni->ExceptionClear(); + } + } + if (permanentLoaderDataComplete()) { + return; + } + jint count = 0; + jclass* classes = nullptr; + if (jvmti->GetLoadedClasses(&count, &classes) != JVMTI_ERROR_NONE) { + return; + } + for (int i = 0; i < count; i++) { + if (!permanentLoaderDataComplete()) { + notePermanentLoaderData(jvmti, jni, classes[i]); + } + jni->DeleteLocalRef(classes[i]); + } + jvmti->Deallocate((unsigned char*)classes); +} + +void HotspotSupport::notePermanentLoaderData(jvmtiEnv* jvmti, JNIEnv* jni, jclass klass) { + if (!canReadLoaderData() || permanentLoaderDataComplete()) { + return; + } + jobject cl = nullptr; + if (jvmti->GetClassLoader(klass, &cl) != JVMTI_ERROR_NONE || cl == nullptr) { + return; // the boot loader's is taken from java.lang.Object + } + int slot = jni->IsSameObject(cl, JAVA_APPLICATION_CLASSLOADER) ? PERMANENT_APPLICATION + : JAVA_PLATFORM_CLASSLOADER != nullptr && + jni->IsSameObject(cl, JAVA_PLATFORM_CLASSLOADER) ? PERMANENT_PLATFORM + : PERMANENT_COUNT; + jni->DeleteLocalRef(cl); + if (slot == PERMANENT_COUNT || + PERMANENT_LOADER_DATA[slot].load(std::memory_order_acquire) != nullptr) { + return; + } + // A hidden or VM-anonymous class has a ClassLoaderData of its own, not + // its loader's. + char* signature = nullptr; + if (jvmti->GetClassSignature(klass, &signature, nullptr) != JVMTI_ERROR_NONE) { + return; + } + bool hidden = isHiddenClassBySignature(signature) || isLambdaClass(signature); + jvmti->Deallocate((unsigned char*)signature); + if (!hidden && !isHiddenClass(jvmti, klass)) { + PERMANENT_LOADER_DATA[slot].store(classLoaderDataOf(jni, klass), std::memory_order_release); + } +} + +#ifdef UNIT_TEST +void HotspotSupport::setPermanentLoaderDataForTest(const void* class_loader_data) { + PERMANENT_LOADER_DATA[PERMANENT_BOOT].store(class_loader_data, std::memory_order_release); +} +#endif + +bool HotspotSupport::isPermanentLoaderData(const void* class_loader_data) { + if (class_loader_data == nullptr) { + return false; + } + for (int i = 0; i < PERMANENT_COUNT; i++) { + if (PERMANENT_LOADER_DATA[i].load(std::memory_order_acquire) == class_loader_data) { + return true; + } + } + return false; +} + bool HotspotSupport::loadMethodIDsIfNeededImpl(jvmtiEnv *jvmti, JNIEnv *jni, jclass klass, bool load_all) { + notePermanentLoaderData(jvmti, jni, klass); if (!load_all) { jobject cl = nullptr; // Hidden/lambda classes can be unloaded, fallback to use jmethodIDs, so preload them. @@ -1459,7 +1568,8 @@ ResolvedMethod::ResolvedMethod() : _long_klass_name(nullptr), _line_number_table(nullptr), _line_number_table_size(0), - _is_thread_run(false) { + _is_thread_run(false), + _class_loader_data(nullptr) { _method_name[0] = '\0'; _method_signature[0] = '\0'; _klass_name[0] = '\0'; @@ -1645,6 +1755,13 @@ bool HotspotSupport::readMethod(const void* method, ResolvedMethod* result) { if (strcmp(result->methodName(), "run") == 0 && strcmp(result->methodSignature(), "()V") == 0) { result->_is_thread_run = isThreadSubclass(klass); } + if (VMStructs::class_loader_data_offset() >= 0) { + // Only ever compared, never dereferenced: read the field rather than go + // through classLoaderData(), whose typed cast needs ClassLoaderData's + // size from the JVM's exported types. + result->_class_loader_data = + *(const void* const*)((const char*)klass + VMStructs::class_loader_data_offset()); + } if (!readLineNumberTable(const_method, result->_line_number_table, result->_line_number_table_size)) { Counters::increment(LINE_NUMBER_TABLE_UNREADABLE); diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h index 4b52b3d541..2e4d1650c0 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h @@ -52,6 +52,7 @@ class ResolvedMethod { jvmtiLineNumberEntry* volatile _line_number_table; volatile int _line_number_table_size; volatile bool _is_thread_run; + const void* volatile _class_loader_data; char _method_name[MAX_METHOD_NAME_LEN]; char _method_signature[MAX_SIGNATURE_LEN]; @@ -95,6 +96,9 @@ class ResolvedMethod { // True for run()V declared by java.lang.Thread or one of its subclasses. bool isThreadRun() const { return _is_thread_run; } + // The declaring class's ClassLoaderData*, or nullptr if unknown. + const void* classLoaderData() const { return _class_loader_data; } + // Decoded line number table, sorted by bci as HotSpot stores it, or // nullptr if the method has none. Allocated with malloc(). const jvmtiLineNumberEntry* lineNumberTable() const { return _line_number_table; } @@ -157,6 +161,26 @@ class HotspotSupport { public: static void initClassloaderInfo(JNIEnv* jni); + // Classes defined by the boot, platform and application class loaders are + // never unloaded, so their method keys never go stale and their names can + // always be read again (see CachedMethodNames). These record those + // loaders' ClassLoaderData. A hidden class has a ClassLoaderData of its + // own, so it never matches. + // + // initPermanentLoaderData() runs at profiler start; classes loaded later + // fill in what it missed through notePermanentLoaderData() (ClassPrepare). + static void initPermanentLoaderData(jvmtiEnv* jvmti, JNIEnv* jni); + static void notePermanentLoaderData(jvmtiEnv* jvmti, JNIEnv* jni, jclass klass); + // Lock-free, no JNI/JVMTI: safe from a GC callback. + static bool isPermanentLoaderData(const void* class_loader_data); + // klass's ClassLoaderData*, or nullptr if this JVM's layout doesn't allow + // reading it. + static const void* classLoaderDataOf(JNIEnv* jni, jclass klass); +#ifdef UNIT_TEST + // Makes class_loader_data the boot loader's; nullptr clears it. + static void setPermanentLoaderDataForTest(const void* class_loader_data); +#endif + // Runs `work` under the ucontext-fault-recovery protocol walkJavaStack() // needs around getJavaTraceAsync(): installs a sigsetjmp/siglongjmp // crash-protection scope chained on prof_thread (JmpCtxScope, guards.h), diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp index 061846813e..1378529d9e 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp @@ -944,6 +944,13 @@ const VMMethod* VMMethod::fromJmethodID(jmethodID id) { return (const VMMethod*)method; } +const void* VMMethod::constMethodOf(const void* method) { + if (method == nullptr || _method_constmethod_offset < 0) { + return nullptr; + } + return SafeAccess::load((void**)((const char*)method + _method_constmethod_offset)); +} + bool VMMethod::check_jmethodID_hotspot(jmethodID id) { if (VM::hotspot_version() > 25) { // In https://bugs.openjdk.org/browse/JDK-8268406 the jmethodids are completely reworked diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h index 86a14c2121..aaf2a33ab5 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h @@ -959,6 +959,10 @@ DECLARE(VMMethod) // no JNI/JVMTI. nullptr if this JVM's jmethodIDs can't be dereferenced // (JDK-8268406) or the slot is empty/freed. static const VMMethod* fromJmethodID(jmethodID id); + + // The ConstMethod* of a raw Method*, read through SafeAccess; nullptr if + // unreadable or the offset is unknown. + static const void* constMethodOf(const void* method); DECLARE_END // Inline string comparison to avoid indirect call to strncmp diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index 4dc2be966e..fe94bac5ed 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -74,6 +74,7 @@ void JVMSupport::initExecution(Arguments& args, jvmtiEnv* jvmti, JNIEnv* jni) { bool load_all = true; if (VM::isHotspot()) { + HotspotSupport::initPermanentLoaderData(jvmti, jni); if (!HotspotSupport::shouldPreloadJmethodIDs(args)) { HotspotSupport::initClassloaderInfo(jni); load_all = false; diff --git a/ddprof-lib/src/main/cpp/nativeMem.h b/ddprof-lib/src/main/cpp/nativeMem.h index cbcb70844d..c11f135ea4 100644 --- a/ddprof-lib/src/main/cpp/nativeMem.h +++ b/ddprof-lib/src/main/cpp/nativeMem.h @@ -36,6 +36,7 @@ X(THREAD_INFO, "thread_info") \ X(JFR_BUFFERS, "jfr_buffers") \ X(METHOD_MAP, "method_map") \ + X(METHOD_NAMES, "method_names") \ X(LIVENESS, "liveness") \ X(WALLCLOCK, "wallclock") \ X(MISC, "misc") diff --git a/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp b/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp index 9b11860d6e..52d2c95a28 100644 --- a/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp +++ b/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp @@ -61,6 +61,17 @@ class VMStructsTestAccessor { ~VMStructsTestAccessor() { _restore(_saved); } + // Scoped override for Klass::_class_loader_data. + class ClassLoaderDataOffset { + public: + explicit ClassLoaderDataOffset(int offset) : _saved(VMStructs::_class_loader_data_offset) { + VMStructs::_class_loader_data_offset = offset; + } + ~ClassLoaderDataOffset() { VMStructs::_class_loader_data_offset = _saved; } + private: + int _saved; + }; + // Offsets beyond the id() set, needed to walk a fake Method* all the way to // its name/signature/class Symbols the way HotspotSupport::resolve() does. struct SymbolOffsets { @@ -476,6 +487,7 @@ struct ResolveFakes { void* name_symbol; // klass_name offset 0 void* jmethod_ids; // stays null so VMMethod::id() reports "no cache" void* super_klass; // RESOLVE_KLASS_SUPER_OFFSET + void* class_loader_data; // RESOLVE_KLASS_CLD_OFFSET } klass, super_klass; struct Symbol { uint16_t length; @@ -518,6 +530,7 @@ static_assert(offsetof(ResolveFakes::ConstMethod, code) == 24, "RESOLVE_TYPE_SIZ constexpr int RESOLVE_CODE_SIZE_OFFSET = offsetof(ResolveFakes::ConstMethod, code_size); constexpr int RESOLVE_FLAGS_OFFSET = offsetof(ResolveFakes::ConstMethod, flags); constexpr int RESOLVE_KLASS_SUPER_OFFSET = offsetof(ResolveFakes::Klass, super_klass); +constexpr int RESOLVE_KLASS_CLD_OFFSET = offsetof(ResolveFakes::Klass, class_loader_data); } // namespace @@ -944,4 +957,180 @@ TEST_F(HotspotResolveCrashProtectionTest, ResolveDetectsRunOfThreadSubclass) { EXPECT_FALSE(_pt->isProtected()); } +// --------------------------------------------------------------------------- +// CachedMethodNames: names kept on a method_map row across chunks, validated +// by the fingerprint of the method the key refers to (Lookup::useCachedNames). +// --------------------------------------------------------------------------- + +namespace { + +// A raw-Method* Java frame for `method`, as walkVM records it. +ASGCT_CallFrame rawFrame(void* method) { + ASGCT_CallFrame frame{}; + frame.bci = FrameType::encode(FRAME_INTERPRETED, 3, /*rawPointer=*/true); + frame.method = method; + return frame; +} + +void fillFoo(ResolveFakes* f, const char* method_name) { + f->link(); + ResolveFakes::setSymbol(f->name_sym, method_name); + ResolveFakes::setSymbol(f->sig_sym, "(I)J"); + ResolveFakes::setSymbol(f->klass_sym, "com/example/Foo"); + f->const_method.code_size = 8; +} + +} // namespace + +// The next chunk names the method from the kept names even though resolving +// it again would fail (here: its name Symbol is now empty, as after reuse of +// freed metadata), because the key still refers to the same ConstMethod. +TEST_F(HotspotResolveCrashProtectionTest, KeptNamesOutliveUnreadableMetadata) { + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, 1, RESOLVE_KLASS_SUPER_OFFSET); + ResolveFakes* f = new (_region) ResolveFakes{}; + fillFoo(f, "compute"); + ASGCT_CallFrame frame = rawFrame(&f->method); + + StringDictionary classes; + MethodMap methods; + MethodInfo* first; + { + Lookup chunk1(nullptr, &methods, &classes); + first = chunk1.resolveMethod(frame); + ASSERT_NE(&chunk1._unknown_method, first); + } + ASSERT_NE(nullptr, first->_cached_names); + EXPECT_STREQ("compute", first->_cached_names->methodName()); + EXPECT_STREQ("com/example/Foo", first->_cached_names->className()); + + first->_mark = false; // what writeMethods() does at the end of a chunk + f->name_sym.length = 0; + ResolvedMethod direct; + ASSERT_FALSE(HotspotSupport::resolve(&f->method, direct)); + + long long hits = Counters::getCounter(METHOD_NAMES_CACHE_HITS); + Lookup chunk2(nullptr, &methods, &classes); + MethodInfo* second = chunk2.resolveMethod(frame); + EXPECT_EQ(first, second); + EXPECT_TRUE(second->_mark); + ASSERT_NE(nullptr, second->_cached_names); + EXPECT_STREQ("compute", second->_cached_names->methodName()); + EXPECT_EQ(hits + 1, Counters::getCounter(METHOD_NAMES_CACHE_HITS)); +} + +// Once the key refers to a different method (the Method* now points at +// another ConstMethod), the kept names are dropped and the method resolved +// again. +TEST_F(HotspotResolveCrashProtectionTest, KeptNamesAreDroppedWhenTheKeyIsReused) { + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, 1, RESOLVE_KLASS_SUPER_OFFSET); + ResolveFakes* f = new (_region) ResolveFakes{}; + fillFoo(f, "compute"); + ResolveFakes* other = new ((char*)_region + 1024) ResolveFakes{}; + fillFoo(other, "replaced"); + ASGCT_CallFrame frame = rawFrame(&f->method); + + StringDictionary classes; + MethodMap methods; + MethodInfo* first; + { + Lookup chunk1(nullptr, &methods, &classes); + first = chunk1.resolveMethod(frame); + } + ASSERT_NE(nullptr, first->_cached_names); + EXPECT_STREQ("compute", first->_cached_names->methodName()); + + first->_mark = false; + f->method.const_method = &other->const_method; // same Method*, new method + + long long stale = Counters::getCounter(METHOD_NAMES_CACHE_STALE); + Lookup chunk2(nullptr, &methods, &classes); + MethodInfo* second = chunk2.resolveMethod(frame); + EXPECT_EQ(first, second); + ASSERT_NE(nullptr, second->_cached_names); + EXPECT_STREQ("replaced", second->_cached_names->methodName()); + EXPECT_EQ(&other->const_method, second->_cached_names->fingerprint()); + EXPECT_EQ(stale + 1, Counters::getCounter(METHOD_NAMES_CACHE_STALE)); +} + +// When the method's memory is gone altogether -- its fingerprint can't be read +// -- nothing can have replaced it, so the kept names still apply. +TEST_F(HotspotResolveCrashProtectionTest, KeptNamesApplyWhenTheMethodIsUnreadable) { + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, 1, RESOLVE_KLASS_SUPER_OFFSET); + ResolveFakes* f = new (_region) ResolveFakes{}; + fillFoo(f, "compute"); + ASGCT_CallFrame frame = rawFrame(&f->method); + + StringDictionary classes; + MethodMap methods; + MethodInfo* first; + { + Lookup chunk1(nullptr, &methods, &classes); + first = chunk1.resolveMethod(frame); + } + ASSERT_NE(nullptr, first->_cached_names); + first->_mark = false; + + ASSERT_EQ(0, mprotect(_region, kPageSize(), PROT_NONE)); + MethodInfo* second; + { + Lookup chunk2(nullptr, &methods, &classes); + second = chunk2.resolveMethod(frame); + } + ASSERT_EQ(0, mprotect(_region, kPageSize(), PROT_READ | PROT_WRITE)); + EXPECT_EQ(first, second); + EXPECT_TRUE(second->_mark); + ASSERT_NE(nullptr, second->_cached_names); + EXPECT_STREQ("compute", second->_cached_names->methodName()); + EXPECT_FALSE(_pt->isProtected()); +} + +// A method whose class belongs to a never-unloaded loader keeps no names: its +// row is marked instead, so neither later chunks nor GC passes try again. +TEST_F(HotspotResolveCrashProtectionTest, NoNamesKeptForClassesThatAreNeverUnloaded) { + HotspotMethodIdVMHotspotGuard hotspot; + VMStructsTestAccessor offsets(RESOLVE_OFFSETS); + VMStructsTestAccessor::SymbolLayout layout(RESOLVE_SYMBOL_OFFSETS, RESOLVE_TYPE_SIZES); + VMStructsTestAccessor::LineTableLayout lines(RESOLVE_FLAGS_OFFSET, RESOLVE_CODE_SIZE_OFFSET, + 1, 1, RESOLVE_KLASS_SUPER_OFFSET); + VMStructsTestAccessor::ClassLoaderDataOffset cld_offset(RESOLVE_KLASS_CLD_OFFSET); + ResolveFakes* f = new (_region) ResolveFakes{}; + fillFoo(f, "compute"); + static int boot_loader_data; + f->klass.class_loader_data = &boot_loader_data; + ASGCT_CallFrame frame = rawFrame(&f->method); + + StringDictionary classes; + MethodMap methods; + { + // Not yet known to be permanent: names are kept. + Lookup chunk(nullptr, &methods, &classes); + MethodInfo* info = chunk.resolveMethod(frame); + ASSERT_NE(&chunk._unknown_method, info); + EXPECT_NE(nullptr, info->_cached_names); + EXPECT_FALSE(info->_never_unloaded); + methods.clear(); + } + + HotspotSupport::setPermanentLoaderDataForTest(&boot_loader_data); + Lookup chunk(nullptr, &methods, &classes); + MethodInfo* info = chunk.resolveMethod(frame); + HotspotSupport::setPermanentLoaderDataForTest(nullptr); + ASSERT_NE(&chunk._unknown_method, info); + EXPECT_EQ(nullptr, info->_cached_names); + EXPECT_TRUE(info->_never_unloaded); + EXPECT_FALSE(Lookup::resolveAtGC(&methods, frame)); +} + #endif // __linux__ diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java index ad47f3c765..52caeffae8 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/ContextClassLoaderUnloadTest.java @@ -115,6 +115,24 @@ public void namesWorkerFramesAfterLoadersUnload(@CStack String cstack) throws Ex System.out.println("[" + cstack + "] after unload: " + stacks); stacks.assertResolved(); assertGCResolveCounters(cstack); + assertMethodNamesAccounted(cstack); + } + + /** The kept method names must show up in the native-memory accounting. */ + private void assertMethodNamesAccounted(String cstack) { + long maxBytes = 0; + long overheadBytes = 0; + for (JfrEvent counter : verifyEvents("datadog.ProfilerCounter", false)) { + String name = counter.getString(NAME); + if ("native_mem_max_bytes.method_names".equals(name)) { + maxBytes = Math.max(maxBytes, counter.getLong(COUNT, 0)); + } else if ("native_mem_chunk_overhead_bytes.method_names".equals(name)) { + overheadBytes = Math.max(overheadBytes, counter.getLong(COUNT, 0)); + } + } + System.out.println("[" + cstack + "] method_names native memory: " + maxBytes + + " bytes peak, " + overheadBytes + " bytes allocator overhead"); + assertTrue(maxBytes > 0, "Kept method names are not counted under native_mem_*.method_names"); } /** @@ -132,6 +150,10 @@ private void assertGCResolveCounters(String cstack) { + methods + " methods resolved"); assertTrue(passes > 0, "No pending-method resolution ran at GC start"); assertTrue(methods > 0, "No method was resolved at GC start"); + // The recording, written after the unload, named the workers from those + // kept names. + assertTrue(counters.getOrDefault("method_names_cache_hits", 0L) > 0, + "No method was named from the names kept at GC start"); if (counters.containsKey("sampler_count.gc_resolve")) { long timed = counters.get("sampler_count.gc_resolve"); From 689ae44d80f6e920333ef2f6df849e3141e5b23f Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 16 Oct 2026 16:10:16 +0200 Subject: [PATCH 10/10] Fix musl --- ddprof-lib/src/main/cpp/callTraceStorage.cpp | 2 ++ ddprof-lib/src/main/cpp/counters.h | 4 ++++ ddprof-lib/src/main/cpp/profiler.cpp | 7 +++++++ 3 files changed, 13 insertions(+) diff --git a/ddprof-lib/src/main/cpp/callTraceStorage.cpp b/ddprof-lib/src/main/cpp/callTraceStorage.cpp index 3f22ffe169..a660c42876 100644 --- a/ddprof-lib/src/main/cpp/callTraceStorage.cpp +++ b/ddprof-lib/src/main/cpp/callTraceStorage.cpp @@ -109,11 +109,13 @@ CallTrace* CallTraceStorage::getDroppedTrace() { void CallTraceStorage::registerLivenessChecker(LivenessChecker checker) { ExclusiveLockGuard lock(&_liveness_lock); _liveness_checkers.push_back(checker); + Counters::set(LIVENESS_CHECKERS, (long long)_liveness_checkers.size()); } void CallTraceStorage::clearLivenessCheckers() { ExclusiveLockGuard lock(&_liveness_lock); _liveness_checkers.clear(); + Counters::set(LIVENESS_CHECKERS, 0); } diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index e42c6912c7..df055814b4 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -198,6 +198,10 @@ * because no ProfiledThread could be allocated for the dump thread (OOM): \ * there is nowhere to publish a landing pad. Expected to stay at 0. */ \ X(METHOD_RESOLUTION_DROPPED_TLS, "method_resolution_dropped_tls") \ + /* Liveness checkers CallTraceStorage::processTraces() runs on every dump. \ + * Profiler::start() clears them and LivenessTracker::start() registers its \ + * own, so this is 1 with liveness tracking on and 0 otherwise. */ \ + X(LIVENESS_CHECKERS, "liveness_checkers") \ /* writeElement() guards against a corrupted/dangling JfrMetadata tree. \ * Root cause is still unconfirmed, so these counters are the durable \ * signal for spotting a recurrence. */ \ diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 0fe2be9d96..9843fa5b8e 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1663,6 +1663,13 @@ Error Profiler::start(Arguments &args, bool reset) { return error; } + // LivenessTracker::start() (via _alloc_engine->start() below) registers a + // checker on every start, and nothing unregisters it at stop: stop()'s final + // dump still needs it to preserve live traces. Drop the previous session's + // checker here, so repeated starts don't stack duplicates and a session + // without liveness tracking runs none. + _call_trace_storage.clearLivenessCheckers(); + // Must precede every signal-based engine's start() below: SamplerPerfProbe // (samplerPerf.h) calls OS::nanotime() from inside the signal handler, and // on macOS that can be the lazy, non-atomic first-call init of