[Lldb-commits] [PATCH] D5871: Add an OperatingSystem plugin to support goroutines

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We have the notion of hard coded section types. See lldb::SectionType in lldb-enumerations.h. You could always add your Go section to this section list and then find the section by section type:

  const SectionList *section_list = module_sp->GetSectionList();
  if (section_list)
  {
      SectionSP section_sp (section_list->FindSectionByType(eSectionTypeGoSymtab, true));
      if (section_sp)
      {

Then you would need to modify ObjectFileMachO and ObjectFileELF to recognize the section and set the section type correctly...


================
Comment at: source/Core/PluginManager.cpp:774-778
@@ -773,6 +773,7 @@
 {
-    OperatingSystemInstance() :
-        name(),
-        description(),
-        create_callback(NULL)
+    OperatingSystemInstance()
+        : name()
+        , description()
+        , create_callback(NULL)
+        , debugger_init_callback(NULL)
     {
----------------
Please leave this in the previous style. convert to using nullptr, and put spaces before the parens:

```
    OperatingSystemInstance() :
        name (),
        description (),
        create_callback (nullptr),
        debugger_init_callback (nullptr),
```

================
Comment at: source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp:153-162
@@ +152,12 @@
+
+bool
+ModuleContainsGoRuntime(Module *module)
+{
+    const Symbol *symbol = module->FindFirstSymbolWithNameAndType(ConstString("runtime.allg"), lldb::eSymbolTypeAny);
+    if (symbol == nullptr)
+        return nullptr;
+    symbol = module->FindFirstSymbolWithNameAndType(ConstString("runtime.allglen"), lldb::eSymbolTypeAny);
+
+    return symbol != nullptr;
+}
+}
----------------
Convert over to using eSectionTypeGoSymtab as mentioned in main comment?

================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2047-2050
@@ -2046,2 +2046,6 @@
                 thread_sp->SetStopInfo (StopInfoSP());
+                // If there's a memory thread backed by this thread, we need to use it to calcualte StopInfo.
+                ThreadSP memory_thread_sp = m_thread_list.FindThreadByProtocolID(thread_sp->GetProtocolID());
+                if (memory_thread_sp)
+                    thread_sp = memory_thread_sp;
 
----------------
I am confused as to why we need to do this. If you have a memory thread, it might be backed by a real thread, or it might not. If it is backed by a real thread, we should be asking the real thread why it really stopped and the memory thread will say that it stopped for the same reason the real thread stopped. If it isn't backed by a real thread, then the memory thread will say why it stopped. Can you elaborate on why you think you need to do this?

================
Comment at: source/Target/ThreadList.cpp:776-777
@@ -775,3 +775,4 @@
             {
-                if (m_threads[idx]->GetID() == tid)
+                ThreadSP backing_thread = m_threads[idx]->GetBackingThread();
+                if (m_threads[idx]->GetID() == tid || (backing_thread && backing_thread->GetID() == tid))
                 {
----------------
Why is this needed? Do you have a case where sometimes you have a real thread that isn't backed by memory thread, and later it does become backed by a memory thread or vice versa? 



Repository:
  rL LLVM

http://reviews.llvm.org/D5871