From d23b78ce50d04e97cc23f997fab8c0ce5faa8726 Mon Sep 17 00:00:00 2001
From: Igor Kulaychuk <igor.kulaychuk@gmail.com>
Date: Mon, 19 Dec 2016 14:03:59 +0300
Subject: [PATCH] [Linux][GDB-JIT] Fix bugs in gdbjit that break lldb stepping
 (#8637)

* Fix .text and .thunk symbols overlapping

When current method calls itself, a __thunk* symbol might be generated with the same address
as the method symbol in .text section. Avoid generating such __thunk* symbol.

* Do not create DWARF line table entries with the same address

* For each HiddenLine assign a zero line number in DWARF

Allow LLDB to to skip HiddenLines when stepping.

* Fix __thunk symbols containing garbage

Fix a bug when __thunk* symbols of previously compiled methods cause
generation of __thunk* symbols for currently compiled method without
filling symbol info.

* Fix missing check for the end of list of compiled methods

* Remove unnecessary check for zero prevLine in gdbjit
---
 src/vm/gdbjit.cpp | 44 ++++++++++++++++++++++++++++++++------------
 src/vm/gdbjit.h   |  2 +-
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/vm/gdbjit.cpp b/src/vm/gdbjit.cpp
index 96f8f8f93dfe..8e728839d664 100644
--- a/src/vm/gdbjit.cpp
+++ b/src/vm/gdbjit.cpp
@@ -1648,13 +1648,11 @@ void NotifyGdb::MethodCompiled(MethodDesc* MethodDescPtr)
     CodeHeader* pCH = (CodeHeader*)pCode - 1;
     CalledMethod* pCalledMethods = reinterpret_cast<CalledMethod*>(pCH->GetCalledMethods());
     /* Collect addresses of thunks called by method */
-    if (!CollectCalledMethods(pCalledMethods))
+    if (!CollectCalledMethods(pCalledMethods, (TADDR)MethodDescPtr->GetNativeCode()))
     {
         return;
     }
     pCH->SetCalledMethods(NULL);
-    if (!codeAddrs.Contains((TADDR)pCode))
-        codeAddrs.Add((TADDR)pCode);
 
     MetaSig sig(MethodDescPtr);
     int nArgsCount = sig.NumFixedArgs();
@@ -2102,18 +2100,17 @@ static void fixLineMapping(SymbolsInfo* lines, unsigned nlines)
     int prevLine = 0;
     for (int i = 0; i < nlines; ++i)
     {
+        if (lines[i].lineNumber == HiddenLine)
+            continue;
         if (lines[i].ilOffset == ICorDebugInfo::PROLOG) // will be fixed in next step
         {
             prevLine = 0;
         }
         else
         {
-            if (lines[i].lineNumber == 0 || lines[i].lineNumber == HiddenLine)
+            if (lines[i].lineNumber == 0)
             {
-                if (prevLine != 0)
-                {
-                    lines[i].lineNumber = prevLine;
-                }
+                lines[i].lineNumber = prevLine;
             }
             else
             {
@@ -2125,11 +2122,23 @@ static void fixLineMapping(SymbolsInfo* lines, unsigned nlines)
     prevLine = lines[nlines - 1].lineNumber;
     for (int i = nlines - 1; i >= 0; --i)
     {
-        if (lines[i].lineNumber == 0 || lines[i].lineNumber == HiddenLine)
+        if (lines[i].lineNumber == HiddenLine)
+            continue;
+        if (lines[i].lineNumber == 0)
             lines[i].lineNumber = prevLine;
         else
             prevLine = lines[i].lineNumber;
     }
+    // Skip HiddenLines
+    for (int i = 0; i < nlines; ++i)
+    {
+        if (lines[i].lineNumber == HiddenLine)
+        {
+            lines[i].lineNumber = 0;
+            if (i + 1 < nlines && lines[i + 1].ilOffset == ICorDebugInfo::NO_MAPPING)
+                lines[i + 1].lineNumber = 0;
+        }
+    }
 }
 
 /* Build program for DWARF source line section */
@@ -2166,6 +2175,13 @@ bool NotifyGdb::BuildLineProg(MemBuf& buf, PCODE startAddr, TADDR codeSize, Symb
             prevFile = lines[i].fileIndex;
         }
 
+        // GCC don't use the is_prologue_end flag to mark the first instruction after the prologue.
+        // Instead of it it is issueing a line table entry for the first instruction of the prologue
+        // and one for the first instruction after the prologue.
+        // We do not want to confuse the debugger so we have to avoid adding a line in such case.
+        if (i > 0 && lines[i - 1].nativeOffset == lines[i].nativeOffset)
+            continue;
+
         IssueSetAddress(ptr, startAddr + lines[i].nativeOffset);
 
         if (lines[i].lineNumber != prevLine) {
@@ -2369,10 +2385,13 @@ bool NotifyGdb::BuildDebugPub(MemBuf& buf, const char* name, uint32_t size, uint
 }
 
 /* Store addresses and names of the called methods into symbol table */
-bool NotifyGdb::CollectCalledMethods(CalledMethod* pCalledMethods)
+bool NotifyGdb::CollectCalledMethods(CalledMethod* pCalledMethods, TADDR nativeCode)
 {
     AddrSet tmpCodeAddrs;
 
+    if (!codeAddrs.Contains(nativeCode))
+        codeAddrs.Add(nativeCode);
+
     CalledMethod* pList = pCalledMethods;
 
      /* count called methods */
@@ -2389,7 +2408,8 @@ bool NotifyGdb::CollectCalledMethods(CalledMethod* pCalledMethods)
     SymbolNames = new (nothrow) Elf_Symbol[SymbolCount];
 
     pList = pCalledMethods;
-    for (int i = 1 + method.GetCount(); i < SymbolCount;)
+    int i = 1 + method.GetCount();
+    while (i < SymbolCount && pList != NULL)
     {
         TADDR callAddr = (TADDR)pList->GetCallAddr();
         if (!codeAddrs.Contains(callAddr))
@@ -2408,7 +2428,7 @@ bool NotifyGdb::CollectCalledMethods(CalledMethod* pCalledMethods)
         pList = pList->GetNext();
         delete ptr;
     }
-
+    SymbolCount = i;
     return true;
 }
 
diff --git a/src/vm/gdbjit.h b/src/vm/gdbjit.h
index 7d440b7e93c2..3160eccf57e6 100644
--- a/src/vm/gdbjit.h
+++ b/src/vm/gdbjit.h
@@ -401,7 +401,7 @@ class NotifyGdb
     static void IssueSimpleCommand(char*& ptr, uint8_t command);
     static void IssueParamCommand(char*& ptr, uint8_t command, char* param, int param_len);
     static void SplitPathname(const char* path, const char*& pathName, const char*& fileName);
-    static bool CollectCalledMethods(CalledMethod* pCM);
+    static bool CollectCalledMethods(CalledMethod* pCM, TADDR nativeCode);
 #ifdef _DEBUG
     static void DumpElf(const char* methodName, const MemBuf& buf);
 #endif