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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus updated this revision to Diff 370935.
apoos-maximus added a comment.

added logic to report ptrace related error only if ptrace policy 
was the actual reason for failure. i.e. by reading the value of
/proc/sys/kernel/yama/ptrace_scope file if it exists, and setting the
ptrace error only if the value is not 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

Files:
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3118,6 +3118,46 @@
   return CreateTrace();
 }
 
+const char *fetchPtracePolicyIfApplicable(lldb::PlatformSP platform_sp) {
+  FileSpec filespec =
+  FileSpec(llvm::StringRef("/proc/sys/kernel/yama/ptrace_scope"));
+  char ptrace_scope = '\0';
+  char *status_message = NULL;
+
+  // check if /proc/sys/kernel/yama/ptrace_scope file exists
+  if (platform_sp->GetFileExists(filespec)) {
+Status fileOperationStatus = Status();
+mode_t perms = lldb::eFilePermissionsUserRW |
+   lldb::eFilePermissionsGroupRead |
+   lldb::eFilePermissionsWorldRead;
+lldb::user_id_t fd = platform_sp->OpenFile(
+filespec, File::eOpenOptionReadOnly, perms, fileOperationStatus);
+
+platform_sp->ReadFile(fd, 0, &ptrace_scope, sizeof(char),
+  fileOperationStatus);
+platform_sp->CloseFile(fd, fileOperationStatus);
+
+// return ptrace policy message only if ptrace_scope value is not zero
+// i.e. ptrace_scope is the reason behind attach fail
+if (ptrace_scope != '0') {
+  status_message =
+  "Could not attach to process. If your uid matches the uid of the "
+  "target process, \n"
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n"
+  "or try again as the root user. ";
+} else {
+  status_message = "Try again as the root user";
+}
+  }
+
+  // simply return try as Root user if file doesn't exist
+  else {
+status_message = "Try again as the root user";
+  }
+
+  return status_message;
+}
+
 Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
   auto state = eStateInvalid;
   auto process_sp = GetProcessSP();
@@ -3188,9 +3228,14 @@
 
   if (state != eStateStopped) {
 const char *exit_desc = process_sp->GetExitDescription();
-if (exit_desc)
-  error.SetErrorStringWithFormat("%s", exit_desc);
-else
+const char *ptrace_reason = "";
+if (exit_desc) {
+  // If we are on a linux flavoured system
+  if (platform_sp->GetSystemArchitecture().GetTriple().isOSLinux()) {
+ptrace_reason = fetchPtracePolicyIfApplicable(platform_sp);
+  }
+  error.SetErrorStringWithFormat("%s\n%s", exit_desc, ptrace_reason);
+} else
   error.SetErrorString(
   "process did not stop (no such process or permission problem?)");
 process_sp->Destroy(false);


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3118,6 +3118,46 @@
   return CreateTrace();
 }
 
+const char *fetchPtracePolicyIfApplicable(lldb::PlatformSP platform_sp) {
+  FileSpec filespec =
+  FileSpec(llvm::StringRef("/proc/sys/kernel/yama/ptrace_scope"));
+  char ptrace_scope = '\0';
+  char *status_message = NULL;
+
+  // check if /proc/sys/kernel/yama/ptrace_scope file exists
+  if (platform_sp->GetFileExists(filespec)) {
+Status fileOperationStatus = Status();
+mode_t perms = lldb::eFilePermissionsUserRW |
+   lldb::eFilePermissionsGroupRead |
+   lldb::eFilePermissionsWorldRead;
+lldb::user_id_t fd = platform_sp->OpenFile(
+filespec, File::eOpenOptionReadOnly, perms, fileOperationStatus);
+
+platform_sp->ReadFile(fd, 0, &ptrace_scope, sizeof(char),
+  fileOperationStatus);
+platform_sp->CloseFile(fd, fileOperationStatus);
+
+// return ptrace policy message only if ptrace_scope value is not zero
+// i.e. ptrace_scope is the reason behind attach fail
+if (ptrace_scope != '0') {
+  status_message =
+  "Could not attach to process. If your uid matches the uid of the "
+  "target process, \n"
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n"
+  "or try again as the root user. ";
+} else {
+  status_message = "Try again as the root user";
+}
+  }
+
+  // simply return try as Root user if file doesn't exist
+  else {
+status_message = "Try again as the root user";
+  }
+
+  return status_message;
+}
+
 Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
   auto state = eStateInvalid;
   auto process_sp = GetProcessSP();

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus updated this revision to Diff 370960.
apoos-maximus added a comment.

Changed the error message to be more informative, and added things,
based on the previous review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

Files:
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3118,6 +3118,54 @@
   return CreateTrace();
 }
 
+const char *fetchPtracePolicyIfApplicable(lldb::PlatformSP platform_sp) {
+  FileSpec filespec =
+  FileSpec(llvm::StringRef("/proc/sys/kernel/yama/ptrace_scope"));
+  char ptrace_scope = '\0';
+  char *status_message = NULL;
+
+  // check if /proc/sys/kernel/yama/ptrace_scope file exists
+  if (platform_sp->GetFileExists(filespec)) {
+Status fileOperationStatus = Status();
+mode_t perms = lldb::eFilePermissionsUserRW |
+   lldb::eFilePermissionsGroupRead |
+   lldb::eFilePermissionsWorldRead;
+lldb::user_id_t fd = platform_sp->OpenFile(
+filespec, File::eOpenOptionReadOnly, perms, fileOperationStatus);
+
+platform_sp->ReadFile(fd, 0, &ptrace_scope, sizeof(char),
+  fileOperationStatus);
+platform_sp->CloseFile(fd, fileOperationStatus);
+
+// return ptrace policy message only if ptrace_scope value is not zero
+// i.e. ptrace_scope is the reason behind attach fail
+if (ptrace_scope != '0') {
+  status_message =
+  "Note that attaching might have failed due to the ptrace_scope "
+  "security policy \n"
+  "which restricts debuggers from attaching to other processes.\n"
+
+  "\nSee the ptrace_scope documentation for more information: \n"
+  "\thttps://www.kernel.org/doc/Documentation/security/Yama.txt \n"
+
+  "\nThe current ptrace_scope policy can be found here: \n"
+  "\t/proc/sys/kernel/yama/ptrace_scope\n"
+
+  "\nTo Enable the debugger to attach to the process, run:\n"
+  "\nsudo sysctl -w kernel.yama.ptrace_scope=0\n";
+} else {
+  status_message = "Try again as the root user";
+}
+  }
+
+  // simply return try as Root user if file doesn't exist
+  else {
+status_message = "Try again as the root user";
+  }
+
+  return status_message;
+}
+
 Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
   auto state = eStateInvalid;
   auto process_sp = GetProcessSP();
@@ -3188,9 +3236,14 @@
 
   if (state != eStateStopped) {
 const char *exit_desc = process_sp->GetExitDescription();
-if (exit_desc)
-  error.SetErrorStringWithFormat("%s", exit_desc);
-else
+const char *ptrace_reason = "";
+if (exit_desc) {
+  // If we are on a linux flavoured system
+  if (platform_sp->GetSystemArchitecture().GetTriple().isOSLinux()) {
+ptrace_reason = fetchPtracePolicyIfApplicable(platform_sp);
+  }
+  error.SetErrorStringWithFormat("%s\n%s", exit_desc, ptrace_reason);
+} else
   error.SetErrorString(
   "process did not stop (no such process or permission problem?)");
 process_sp->Destroy(false);


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3118,6 +3118,54 @@
   return CreateTrace();
 }
 
+const char *fetchPtracePolicyIfApplicable(lldb::PlatformSP platform_sp) {
+  FileSpec filespec =
+  FileSpec(llvm::StringRef("/proc/sys/kernel/yama/ptrace_scope"));
+  char ptrace_scope = '\0';
+  char *status_message = NULL;
+
+  // check if /proc/sys/kernel/yama/ptrace_scope file exists
+  if (platform_sp->GetFileExists(filespec)) {
+Status fileOperationStatus = Status();
+mode_t perms = lldb::eFilePermissionsUserRW |
+   lldb::eFilePermissionsGroupRead |
+   lldb::eFilePermissionsWorldRead;
+lldb::user_id_t fd = platform_sp->OpenFile(
+filespec, File::eOpenOptionReadOnly, perms, fileOperationStatus);
+
+platform_sp->ReadFile(fd, 0, &ptrace_scope, sizeof(char),
+  fileOperationStatus);
+platform_sp->CloseFile(fd, fileOperationStatus);
+
+// return ptrace policy message only if ptrace_scope value is not zero
+// i.e. ptrace_scope is the reason behind attach fail
+if (ptrace_scope != '0') {
+  status_message =
+  "Note that attaching might have failed due to the ptrace_scope "
+  "security policy \n"
+  "which restricts debuggers from attaching to other processes.\n"
+
+  "\nSee the ptrace_scope documentation for more information: \n"
+  "\thttps://www.kernel.org/doc/Documentation/security/Yama.txt \n"
+
+  "\nThe current ptrace_scope policy can

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus added a comment.

Hi, 
@DavidSpickett, @teemperor, @rupprecht

After our previous discussions, I have modified the patch.
Now :-

1. OS is detected in Target::Attach(), and reason for failure is analyzed 
accordingly.
2. ptrace_scope related error message is only emitted after checking :-
  - If /proc/sys/kernel/yama/ptrace_scope file exists
  - If it Exists weather it's value is non-zero.
  - If Either don't satisfy user is asked try again as the root user, as only 
possibly left (//I think..//) is that user is trying to debug a process, owned 
by the root user. In which case setting ptrace_policy to zero alone will not 
help.
3. Changed the Error Message to be more informative, and unique compared to 
gdb. I went with the kernel.org documentation as it's more generic to all linux 
OS es.

I have tested the above code by trying to debug a process on `localhost`, and 
also by debugging a remote process. It's works as expected.
I am yet to build this on  a windows machine, and test. As I don't own a Mac 
machine I'll need help from someone who does.
Only the Os detection scenario needs to be tested here, when attach fails i.e. 
process hasn't stopped.

Although this approach seems to work I am yet to implement the error-code 
approach suggested by @teemperor, I  have a few questions regarding the same :-

1. Will we have to introduce new Error Code particulary for the ptrace_conditon?
2. This logic will reside in Process::Attach() ?
3. Since Error code needs to be propagated from lldb-server to lldb, how are 
the two interacting (//I meant code wise ...//) :
  - In case of debugging a remote process : does local lldb, control the remote 
lldb through lldb-server ? have I understood it right ?
  - In this case lldb/lldb-server will have to get cross-compiled shipped to 
the remote host and than tested using the same lldb on localhost.
4. How are new Error Codes Introduced into the system ? And how are their 
behaviours modified, like `error.AsCString()`




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus added a comment.

Hi, 
@DavidSpickett, @teemperor, @rupprecht

After our previous discussions, I have modified the patch.
Now :-

1. OS is detected in Target::Attach(), and reason for failure is analyzed 
accordingly.
2. ptrace_scope related error message is only emitted after checking :-
  - If /proc/sys/kernel/yama/ptrace_scope file exists
  - If it Exists weather it's value is non-zero.
  - If Either don't satisfy user is asked try again as the root user, as only 
possibly left (//I think..//) is that user is trying to debug a process, owned 
by the root user. In which case setting ptrace_policy to zero alone will not 
help.
3. Changed the Error Message to be more informative, and unique compared to 
gdb. I went with the kernel.org documentation as it's more generic to all linux 
OS es.

I have tested the above code by trying to debug a process on `localhost`, and 
also by debugging a remote process. It's works as expected.
I am yet to build this on  a windows machine, and test. As I don't own a Mac 
machine I'll need help from someone who does.
Only the Os detection scenario needs to be tested here, when attach fails i.e. 
process hasn't stopped.

Although this approach seems to work I am yet to implement the error-code 
approach suggested by @teemperor, I  have a few questions regarding the same :-

1. Will we have to introduce new Error Code particulary for the ptrace_conditon?
2. This logic will reside in Process::Attach() ?
3. Since Error code needs to be propagated from lldb-server to lldb, how are 
the two interacting (//I meant code wise ...//) :
  - In case of debugging a remote process : does local lldb, control the remote 
lldb through lldb-server ? have I understood it right ?
  - In this case lldb/lldb-server will have to get cross-compiled shipped to 
the remote host and than tested using the same lldb on localhost.
4. How are new Error Codes Introduced into the system ? And how are their 
behaviours modified, like `error.AsCString()`




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus created this revision.
apoos-maximus requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

lldb attach operation fails when the ptrace() call fails on a traced
process. The error reporting in this scenario could be improved by
including what went wrong and what could be done by the user to resolve
the situation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106226

Files:
  lldb/source/Commands/CommandObjectProcess.cpp


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -409,6 +409,9 @@
   }
 } else {
   result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendMessage("Could not attach to process. If your uid matches 
the uid of the target process, \n" 
+   "check the setting of 
/proc/sys/kernel/yama/ptrace_scope, \n" 
+   "or try again as the root user. \n");
 }
 
 if (!result.Succeeded())


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -409,6 +409,9 @@
   }
 } else {
   result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendMessage("Could not attach to process. If your uid matches the uid of the target process, \n" 
+   "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n" 
+   "or try again as the root user. \n");
 }
 
 if (!result.Succeeded())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus added reviewers: jingham, rupprecht, davide.
apoos-maximus added a comment.

This patch is supposed to fix this Bug :  39166 

I compiled the code and used the binary with local/remote debugging scenarios. 
On attach failure this is the output it produces :-

  $ ./lldb -p 46503
  (lldb) process attach --pid 46503
  Could not attach to process. If your uid matches the uid of the target 
process, 
  check the setting of /proc/sys/kernel/yama/ptrace_scope, 
  or try again as the root user.
  error: attach failed: Operation not permitted
  (lldb) 

My queries :-

- Is AppendMessage() really the right function to use in an error scenario ? or 
the additional message should just be appended 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() ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus updated this revision to Diff 359584.
apoos-maximus added a comment.

changed the code-styling to comply with pre-merge checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

Files:
  lldb/source/Commands/CommandObjectProcess.cpp


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -409,6 +409,10 @@
   }
 } else {
   result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendMessage(
+  "Could not attach to process. If your uid matches the uid of the 
target process, \n" 
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n" 
+  "or try again as the root user. \n");
 }
 
 if (!result.Succeeded())


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -409,6 +409,10 @@
   }
 } else {
   result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendMessage(
+  "Could not attach to process. If your uid matches the uid of the target process, \n" 
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n" 
+  "or try again as the root user. \n");
 }
 
 if (!result.Succeeded())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus updated this revision to Diff 359585.
apoos-maximus added a comment.

realigned patch against main branch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

Files:
  lldb/source/Commands/CommandObjectProcess.cpp


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -409,6 +409,10 @@
   }
 } else {
   result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendMessage(
+  "Could not attach to process. If your uid matches the uid of the 
target process, \n" 
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n" 
+  "or try again as the root user. \n");
 }
 
 if (!result.Succeeded())


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -409,6 +409,10 @@
   }
 } else {
   result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendMessage(
+  "Could not attach to process. If your uid matches the uid of the target process, \n" 
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n" 
+  "or try again as the root user. \n");
 }
 
 if (!result.Succeeded())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus updated this revision to Diff 359586.
apoos-maximus added a comment.

reformatted code to comply with pre-merge checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

Files:
  lldb/source/Commands/CommandObjectProcess.cpp


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -409,6 +409,11 @@
   }
 } else {
   result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendMessage(
+  "Could not attach to process. If your uid matches the uid of the "
+  "target process, \n"
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n"
+  "or try again as the root user. \n");
 }
 
 if (!result.Succeeded())


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -409,6 +409,11 @@
   }
 } else {
   result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendMessage(
+  "Could not attach to process. If your uid matches the uid of the "
+  "target process, \n"
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n"
+  "or try again as the root user. \n");
 }
 
 if (!result.Succeeded())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus added a comment.

In D106226#2886627 , @DavidSpickett 
wrote:

> 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`.

Thanks a lot for passing on this knowledge.

>> 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)



> 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.

Looks like target related information is available in `TargetProperties` class 
whom `Target` class is inheriting so perhaps a decision based on target-type 
could be made there. I will explore further into this, as in which property(s) 
can be used and how and have an update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus added a comment.

In D106226#2891107 , @teemperor wrote:

> Congrats on getting started on your first patch! I improving this error 
> message really seems like a good idea.

Thanks  : ) !

> From what I can see the error message here is identical to GDB's which is a 
> different project with an incompatible license. No idea if this is large 
> enough of a copy to bring us into the realm of copyright (not a lawyer), but 
> I think formulating our own (maybe even better?) error message would anyway 
> be a good idea. What about something along those lines:
>
>   error: attach failed:  (This 
> line is just the normal LLDB attach error)
>   Note that attaching might have failed due to the ptrace_scope security 
> policy
>   which restricts debuggers from attaching to other processes. See
>   the ptrace_scope documentation for more information:
> https://www.kernel.org/doc/Documentation/security/Yama.txt
>   The current ptrace_scope policy can be found here:
> /proc/sys/kernel/yama/ptrace_scope
>
> (Not sure how I feel about linking to some internet URL, but I couldn't find 
> any man page for Yama/ptrace_scope)

Agreed ! Having something of our own is a good idea. I will change it to what 
you and @rupprecht are suggesting !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus added a comment.

In D106226#2891107 , @teemperor wrote:

> Also I wonder how we could make sure we emit this diagnostic in cases where 
> the ptrace_scope is actually the reason for the failed attach. 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. From the lldb 
> side I don't think we have any way of knowing why the attach actually failed 
> so we would emit this error speculatively which doesn't seem ideal.



In D106226#2891565 , @rupprecht wrote:

> Can we have LLDB read the value of /proc/sys/kernel/yama/ptrace_scope, and 
> only print the error if the file exists and is not 0?

Reading the value of `/proc/sys/kernel/yama/ptrace_scope` is possible even when 
running as a non-root user, so user permissions would not pose a problem here.

  [apoos_maximus@host]$ sysctl kernel.yama.ptrace_scope   
  kernel.yama.ptrace_scope = 1
  [apoos_maximus@host]$ cat /proc/sys/kernel/yama/ptrace_scope   
  1

I will check into the ptrace-API to see if there is a direct way for this, else 
look into some other kernel call through which we can read this value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[APOORV SACHAN via Phabricator via lldb-commits]

apoos-maximus updated this revision to Diff 370769.
apoos-maximus added a comment.

moved the error message building logic to Target::Attach()
where the ptrace policy related mesasge gets appended only
if the debugee process is running on LinuxOS


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

Files:
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3118,6 +3118,16 @@
   return CreateTrace();
 }
 
+const char *fetchPtracePolicyIfApplicable(lldb::PlatformSP platform_sp) {
+  Platform *platform = platform_sp.get();
+
+  // read value of /proc/sys/kernel/yama/ptrace_scope
+  return "Could not attach to process. If your uid matches the uid of the "
+ "target process, \n"
+ "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n"
+ "or try again as the root user. \n";
+}
+
 Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
   auto state = eStateInvalid;
   auto process_sp = GetProcessSP();
@@ -3188,9 +3198,14 @@
 
   if (state != eStateStopped) {
 const char *exit_desc = process_sp->GetExitDescription();
-if (exit_desc)
-  error.SetErrorStringWithFormat("%s", exit_desc);
-else
+const char *ptrace_reason = "";
+if (exit_desc) {
+  // If we are on a linux flavoured system
+  if (platform_sp->GetSystemArchitecture().GetTriple().isOSLinux()) {
+ptrace_reason = fetchPtracePolicyIfApplicable(platform_sp);
+  }
+  error.SetErrorStringWithFormat("%s\n%s", exit_desc, ptrace_reason);
+} else
   error.SetErrorString(
   "process did not stop (no such process or permission problem?)");
 process_sp->Destroy(false);
Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -410,7 +410,7 @@
 "no error returned from Target::Attach, and target has no 
process");
   }
 } else {
-  result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendErrorWithFormat("attach failed: %s", error.AsCString());
 }
 
 if (!result.Succeeded())


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3118,6 +3118,16 @@
   return CreateTrace();
 }
 
+const char *fetchPtracePolicyIfApplicable(lldb::PlatformSP platform_sp) {
+  Platform *platform = platform_sp.get();
+
+  // read value of /proc/sys/kernel/yama/ptrace_scope
+  return "Could not attach to process. If your uid matches the uid of the "
+ "target process, \n"
+ "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n"
+ "or try again as the root user. \n";
+}
+
 Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
   auto state = eStateInvalid;
   auto process_sp = GetProcessSP();
@@ -3188,9 +3198,14 @@
 
   if (state != eStateStopped) {
 const char *exit_desc = process_sp->GetExitDescription();
-if (exit_desc)
-  error.SetErrorStringWithFormat("%s", exit_desc);
-else
+const char *ptrace_reason = "";
+if (exit_desc) {
+  // If we are on a linux flavoured system
+  if (platform_sp->GetSystemArchitecture().GetTriple().isOSLinux()) {
+ptrace_reason = fetchPtracePolicyIfApplicable(platform_sp);
+  }
+  error.SetErrorStringWithFormat("%s\n%s", exit_desc, ptrace_reason);
+} else
   error.SetErrorString(
   "process did not stop (no such process or permission problem?)");
 process_sp->Destroy(false);
Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -410,7 +410,7 @@
 "no error returned from Target::Attach, and target has no process");
   }
 } else {
-  result.AppendErrorWithFormat("attach failed: %s\n", error.AsCString());
+  result.AppendErrorWithFormat("attach failed: %s", error.AsCString());
 }
 
 if (!result.Succeeded())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits