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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Maybe I can help a little with the error code approach/add on.

lldb communicates to lldb-server using gdb-remote packets, you can enable logging of these with `log enable gdb-remote packets`. If you're running on the same machine, lldb will start an lldb-server in gdbserver mode then connect to that. Usually if the target is a different machine you'll run lldb-server in "platform" mode there and the first thing lldb will ask it to do is spawn another lldb-server in gdb-server mode, then lldb will use that. (and of course you could manually do that step and connect to it with the `gdb-remote` command if you want)

Here's a log of me trying to attach to a process that GDB is already debugging:

  (lldb) process attach -p 833700
  <...>
  b-remote.async>  <  17> send packet: $vAttach;cb8a4#98
  b-remote.async>  <  54> read packet: $E01;4f7065726174696f6e206e6f74207065726d6974746564#83
  error: attach failed: Operation not permitted
  Try again as the root user

The first 90% is negotiating capabilities of the server/client. The important bit here is at the end.
We're asking to attach to 833700 in hex then we get a reply with an error code 1 and an error string encoded as hex chars.

The 1 is the ptrace errno and the string is the string description of that errno.

Here's me connecting to a process that doesn't exist:

  (lldb) log enable gdb-remote packets
  (lldb) process attach -p 123456
  <...>
  b-remote.async>  <  17> send packet: $vAttach;1e240#32
  b-remote.async>  <  70> read packet: $Eff;43616e6e6f74206765742070726f6365737320617263686974656374757265#d4
  error: attach failed: Cannot get process architecture
  Try again as the root user

Here you could argue the "Try again" doesn't help. Also us not just saying "no such process" doesn't help but you get the idea.

> The proper way to check this seems to be checking the errno after we call ptrace and then propagate the error all the way back to lldb from lldb-server.

So in fact we *are* doing this but there are some situations (I just found the one above) where either we never get to calling ptrace or the errno is overwritten by some other call we make.
(0xff == -1 which I think is `inconvertible_error_code` which is what you use when you just want to make a generic error with some string)

What you could do is just look for the "Operation not permitted" case, and only show the message then. By the time you get the status in the lldb it won't have `eErrorTypePosix` but if you already know in that specific scenario that it will be, you can just compare to the errno code.
(be wary of including linux specific headers though, there's probably an llvm header you could use to make it generic)

There might be path where we `return 1` for non ptrace reasons but it would narrow it down.



================
Comment at: lldb/source/Target/Target.cpp:3121
 
+const char *fetchPtracePolicyIfApplicable(lldb::PlatformSP platform_sp) {
+  FileSpec filespec =
----------------
Make this function static since it's only used in this file.


================
Comment at: lldb/source/Target/Target.cpp:3143
+    if (ptrace_scope != '0') {
+      status_message =
+          "Note that attaching might have failed due to the ptrace_scope "
----------------
I get warnings like this with clang-12:
```
/home/david.spickett/llvm-project/lldb/source/Target/Target.cpp:3144:11: warning: ISO C++11 does not allow conversion from string literal to 'char *' [-Wwritable-strings]
          "Note that attaching might have failed due to the ptrace_scope "
          ^
```

I think just making status_message `const char*` will fix that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226