[Lldb-commits] [PATCH] D106226: [lldb] Improve error message when "lldb attach" fails

Mon Jul 19 01:52:57 PDT 2021

DavidSpickett added a comment.

Thanks for looking at this, I've been tripped up by this myself.

> Is AppendMessage() really the right function to use in an error scenario ? or the additional message should just be appended to AppendErrorWithFormat() ?

It changes whether the output goes to stdout or stderr and whether we set the return status to failed. So I would merge it into one call to `AppendErrorWithFormat`.

> The error object is being returned by the Attach() call on line:399 so, should this message be handled by the Attach() function and not by DoExecute() ?

Go to https://lldb.llvm.org/python_reference/index.html, search for `SBTarget` and you'll see a few attach methods there. They call Target Attach so if you added it there they'd also get it. Which seems like a good thing to me. (but see my other comment about platforms)

================
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:416
+          "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n"
+          "or try again as the root user. \n");
     }
----------------
While this is useful for Linux/Unixish things, lldb also runs on Windows as well where this message isn't going to make a lot of sense. Adding a `if linux ...` at this level violates the separation of concerns though.
(yes an experienced user could ignore it but they'll probably at least wonder if lldb is confused)

Would it be possible to move this message further down into `Target::Attach` to a place where we know the target kind? (that would also answer your question about what level it should be at)

If you could return a status from that level with this message then you'd only see it where it makes sense.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106226/new/

https://reviews.llvm.org/D106226