[Lldb-commits] [lldb] [lldb][OpenBSD] Fixes for process handling (PR #122534)

[Pavel Labath via lldb-commits]

================
@@ -40,49 +40,63 @@ class ProcessLaunchInfo;
 static bool
 GetOpenBSDProcessArgs(const ProcessInstanceInfoMatch *match_info_ptr,
                       ProcessInstanceInfo &process_info) {
-  if (process_info.ProcessIDIsValid()) {
-    int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_ARGS,
-                  (int)process_info.GetProcessID()};
-
-    char arg_data[8192];
-    size_t arg_data_size = sizeof(arg_data);
-    if (::sysctl(mib, 4, arg_data, &arg_data_size, NULL, 0) == 0) {
-      DataExtractor data(arg_data, arg_data_size, endian::InlHostByteOrder(),
-                         sizeof(void *));
-      lldb::offset_t offset = 0;
-      const char *cstr;
-
-      cstr = data.GetCStr(&offset);
-      if (cstr) {
-        process_info.GetExecutableFile().SetFile(cstr, FileSpec::Style::native);
-
-        if (!(match_info_ptr == NULL ||
-              NameMatches(
-                  process_info.GetExecutableFile().GetFilename().GetCString(),
-                  match_info_ptr->GetNameMatchType(),
-                  match_info_ptr->GetProcessInfo().GetName())))
-          return false;
-
-        Args &proc_args = process_info.GetArguments();
-        while (1) {
-          const uint8_t *p = data.PeekData(offset, 1);
-          while ((p != NULL) && (*p == '\0') && offset < arg_data_size) {
-            ++offset;
-            p = data.PeekData(offset, 1);
-          }
-          if (p == NULL || offset >= arg_data_size)
-            return true;
-
-          cstr = data.GetCStr(&offset);
-          if (cstr)
-            proc_args.AppendArgument(llvm::StringRef(cstr));
-          else
-            return true;
-        }
-      }
-    }
+  if (!process_info.ProcessIDIsValid())
+    return false;
+
+  int pid = process_info.GetProcessID();
+
+  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_ARGV};
+  size_t kern_proc_args_size = 0;
+
+  // On OpenBSD, this will just fill ARG_MAX all the time
+  if (::sysctl(mib, 4, NULL, &kern_proc_args_size, NULL, 0) == -1)
+    return false;
+
+  std::string arg_data(kern_proc_args_size, 0);
+
+  if (::sysctl(mib, 4, (void *)arg_data.data(), &kern_proc_args_size, NULL,
+               0) == -1)
+    return false;
+
+  arg_data.resize(kern_proc_args_size);
+
+  // arg_data is a NULL terminated list of pointers, where the pointers
+  // point within arg_data to the location of the arg string
+  DataExtractor data(arg_data.data(), arg_data.length(),
----------------
labath wrote:

The DataExtractor is mainly useful for reading data of possibly-foreign endianness that you do not fully trust. Given that this is host code, the endianness will always match, and since the data comes from the OS, maybe we can trust it to be valid (?)

How do other applications read data from this interface? Is it something like this:
```
void *data = malloc(kern_proc_args_size);
sysctl(...);
for (const char **argv = data; *argv; ++argv)
  puts(*argv);
free(data);
```
If so, could we do something like that well (using llvm::make_scope_exit for freeing the memory)?

https://github.com/llvm/llvm-project/pull/122534