[PATCH] D42582: [lldb][PPC64] Fixed step-in stopping in the wrong line

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

I don't think I know enough about this, so I'll defer to Jim to comment on the design. I just have some comments about the implementation.

Also, it looks like at least the breakpoint-setting part should be easy to test in a freestanding environment: make a .s file which sets up the function, llvm-mc it, load it up in lldb, set a breakpoint, verify it got moved to the right offset. I've been playing around with a patch that should make this easier for you. I'll see if I can clean it up and upload today...



================
Comment at: include/lldb/Core/Architecture.h:63
+  //------------------------------------------------------------------
+  virtual void AdjustBreakpointAddress(Symbol *func, Address &addr) const {}
+
----------------
It looks like symbol is always going to be non-null here, right? Can we make this a reference? maybe even a const reference?


================
Comment at: source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp:57-60
+  // This code handles only ELF files
+  if (target->GetArchitecture().GetTriple().getObjectFormat() !=
+      llvm::Triple::ObjectFormatType::ELF)
+    return 0;
----------------
Should we move this check into the `Create` function? I know this plugin is kinda supposed to be OS-agnostic, but if we're going to be checking the object format anyway, we might as well do it early-on. We can revisit that once we have a non-elf customer who wants a ppc64 architecture plugin.


https://reviews.llvm.org/D42582