From c6f63a6aa693e33ecd7e5e205a70407dffa7ed98 Mon Sep 17 00:00:00 2001 From: Troels Bjerre Lund Date: Fri, 20 May 2022 09:43:55 +0000 Subject: [PATCH] [K/N] Fix LLDB line number bug The lowering that inserts explicit returns into returnable blocks uses the character offset of the returnable block for the newly created return statement. This is wrong, but only noticeable when the callee is an inlined function in another file, where the statements of the block belong to callee rather than the caller. The fix takes the offset of the last statement in the block. In the case the block does not contain any statements, it falls back to using the offset of the block itself. The test does not impose any specific stepping order, since this would be too fragile. It only tests that LLDB does not step to empty (or fully //-commented) lines. Co-authored-by: Troels Lund --- .../konan/lower/ReturnsInsertionLowering.kt | 4 +- .../kotlin/native/test/debugger/LldbTests.kt | 35 +++++++++ .../kotlin/native/test/debugger/Matchers.kt | 78 ++++++++++++++----- 3 files changed, 97 insertions(+), 20 deletions(-) diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/ReturnsInsertionLowering.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/ReturnsInsertionLowering.kt index a2bf9e884a4..5d4cff3fef8 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/ReturnsInsertionLowering.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/ReturnsInsertionLowering.kt @@ -54,8 +54,8 @@ internal class ReturnsInsertionLowering(val context: Context) : FileLoweringPass expression.acceptChildrenVoid(this) if (expression !is IrReturnableBlock) return if (expression.inlineFunctionSymbol?.owner?.returnType == context.irBuiltIns.unitType) { - val irBuilder = context.createIrBuilder(expression.symbol, expression.endOffset, expression.endOffset) - irBuilder.run { + val offset = (expression.statements.lastOrNull() ?: expression).endOffset + context.createIrBuilder(expression.symbol, offset, offset).run { expression.statements += irReturn(irGetObject(symbols.unit)) } } diff --git a/kotlin-native/backend.native/tests/debugger/src/test/kotlin/org/jetbrains/kotlin/native/test/debugger/LldbTests.kt b/kotlin-native/backend.native/tests/debugger/src/test/kotlin/org/jetbrains/kotlin/native/test/debugger/LldbTests.kt index 33cbbbd0594..e2ecf2a83a8 100644 --- a/kotlin-native/backend.native/tests/debugger/src/test/kotlin/org/jetbrains/kotlin/native/test/debugger/LldbTests.kt +++ b/kotlin-native/backend.native/tests/debugger/src/test/kotlin/org/jetbrains/kotlin/native/test/debugger/LldbTests.kt @@ -5,6 +5,7 @@ import org.jetbrains.kotlin.native.test.debugger.lldbCommandRunOrContinue import org.jetbrains.kotlin.native.test.debugger.lldbComplexTest +import org.jetbrains.kotlin.native.test.debugger.lldbCheckLineNumbers import org.jetbrains.kotlin.native.test.debugger.lldbTest import org.junit.Test @@ -402,4 +403,38 @@ class LldbTests { > q """.trimIndent().lldb(binary) } + + @Test + fun `lldb line numbers are valid in source`() { + // Whitespace is important, since the character offsets are what defines source lines + lldbCheckLineNumbers(mapOf( + "main.kt" to """ + + fun main() { + + inliner { + + println("1") + + } + + inliner { + + println("2") + + } + + } + +""", "inliner.kt" to """ + ${" ".repeat(1000)} + inline fun inliner(block: ()->Unit) { + + block() + + } + +"""), "main.kt:2", 15) + } + } \ No newline at end of file diff --git a/kotlin-native/backend.native/tests/debugger/src/test/kotlin/org/jetbrains/kotlin/native/test/debugger/Matchers.kt b/kotlin-native/backend.native/tests/debugger/src/test/kotlin/org/jetbrains/kotlin/native/test/debugger/Matchers.kt index 0e91efdc97e..c5f8c81efcc 100644 --- a/kotlin-native/backend.native/tests/debugger/src/test/kotlin/org/jetbrains/kotlin/native/test/debugger/Matchers.kt +++ b/kotlin-native/backend.native/tests/debugger/src/test/kotlin/org/jetbrains/kotlin/native/test/debugger/Matchers.kt @@ -6,9 +6,7 @@ package org.jetbrains.kotlin.native.test.debugger import org.intellij.lang.annotations.Language -import org.jetbrains.kotlin.konan.target.hostTargetSuffix import org.junit.Assert.fail -import java.io.IOException import java.nio.file.Files import java.nio.file.Path @@ -59,25 +57,11 @@ import java.nio.file.Path * Current executable set to [..]program.kexe[..] */ fun lldbTest(@Language("kotlin") programText: String, lldbSession: String) { - if (!haveLldb) { - println("Skipping test: no LLDB") + lldbReasonToAbort()?.let { + println(it) return } - if (!targetIsHost() && !simulatorTestEnabled()) { - println("simulator tests disabled, check 'kotlin.native.test.debugger.simulator.enabled' property") - return - } - - if (!isOsxDevToolsEnabled) { - println("""Development tools aren't available. - |Please consider to execute: - | ${DistProperties.devToolsSecurity} -enable - |or - | csrutil disable - |to run lldb tests""".trimMargin()) - return - } val lldbSessionSpec = LldbSessionSpecification.parse(lldbSession) val tmpdir = Files.createTempDirectory("debugger_test") @@ -92,6 +76,64 @@ fun lldbTest(@Language("kotlin") programText: String, lldbSession: String) { lldbSessionSpec.match(result) } +fun lldbReasonToAbort() = when { + !haveLldb -> + "Skipping test: no LLDB" + !targetIsHost() && !simulatorTestEnabled() -> + "simulator tests disabled, check 'kotlin.native.test.debugger.simulator.enabled' property" + !isOsxDevToolsEnabled -> + """Development tools aren't available. + |Please consider to execute: + | ${DistProperties.devToolsSecurity} -enable + |or + | csrutil disable + |to run lldb tests""".trimMargin() + else -> null +} + +/** + * Another integration test for debug info. + * + * It works by compiling a given set of [src] files with debug info, then + * launching lldb, running to the given [breakpoint] and "step in" [steps] times. + * It then checks that none of the reached break points correspond to blank + * lines in the given source files. + */ +fun lldbCheckLineNumbers(src: Map, breakpoint: String, steps: Int) { + lldbReasonToAbort()?.let { + println(it) + return + } + + val tmpdir = Files.createTempDirectory("debugger_test") + tmpdir.toFile().deleteOnExit() + + val source = src.map { (filename, content) -> + val path = tmpdir.resolve(filename) + Files.write(path, content.trimIndent().toByteArray()) + path + }.toTypedArray() + + val output = tmpdir.resolve("program.kexe") + val driver = ToolDriver() + driver.compile(output, source, "-g") + + val commands = listOf("b ${breakpoint}", "r") + (1..steps).map { "s" } + listOf("q") + val result = driver.runLldb(output, commands) + + val noCodeLine = Regex("^\\s*(//.*)?$") + val validSourceBreaks = src.flatMap { (filename, content) -> + content.lines().withIndex() + .filterNot { noCodeLine.matches(it.value) } + .map{ "$filename:${it.index}"} + }.toSet() + + Regex("(${src.keys.joinToString("|")}):\\d+").findAll(result).forEach { + check(it.value in validSourceBreaks, { "${it.value} is not a meaningful debug stop" }) + } +} + + private val isOsxDevToolsEnabled: Boolean by lazy { //TODO: add OSX checks. val rawStatus = subprocess(DistProperties.devToolsSecurity, "-status")