[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 <troels@google.com>

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))
                     }
                 }

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)
+    }
+
 }

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<String, String>, 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")