[lldb] Add support for a "global" lldbinit file

[labath]

Summary

This patch adds introduces a new kind of an lldbinit file. Unlike the
lldbinit in the home directory (useful for customizing lldb to the needs
of a particular user), or the cwd lldbinit file (useful for
project-specific settings), this file can be used to customize an entire
lldb installation to a particular environment.

The feature is enabled at build time, by setting the
LLDB_GLOBAL_INIT_DIRECTORY variable to a path to a directory which
should contain an "lldbinit" file. Lldb will then load the file at
startup, if it exists, and if automatic init loading has not been
disabled. Relative paths will be resolved (at runtime) relative to the
location of the lldb library (liblldb or LLDB.framework).

The system-wide lldbinit file will be loaded first, before any
$HOME/.lldbinit and $CWD/.lldbinit files are processed, so that those
can override any system-wide settings.

More information can be found on the RFC thread at
https://discourse.llvm.org/t/rfc-system-wide-lldbinit/59933.

[JDevlieghere]

This is in line with the existing ways of sourcing init files in LLDB and has all the things we discussed in the RFC. I left few nits but this all looks good to me. I'll hold off on accepting so it continues to show up in the other reviewer's queue.

lldb/include/lldb/Interpreter/CommandInterpreter.h

256

SourceInitFileSystem for consistency with the other two? Or maybe you were planning to change the other two in another patch?

lldb/source/API/SBCommandInterpreter.cpp

426–428

Nit: all the other functions should have a newline because that's what lldb-instr generates.

lldb/source/Interpreter/CommandInterpreter.cpp

2385–2393

Why not put the ifdef around the m_skip_lldbinit_files check?

[jingham]

LGTM. I agree with Jonas about keeping the naming consistent. I find it easier to spot the differentiating bits in a series of function names if they are at the beginning or the end, but that's a very mild preference.

[wallace]

lldb/include/lldb/Interpreter/CommandInterpreter.h

256

calling it SourceInitFileSystem is a bit confusing. What about using Global instead of System to avoid naming issues?

[clayborg]

This looks good to me. Just a few things to possibly think about:

• Maybe we would want addition system init files for different workflows and then we would start lldb with a new option like "lldb --workflow qemu" and it would load the system ".lldbinit-qemu" init file. That way we could have many supported workflows from one distribution. Here at Facebook we would have at least 3 that I know of: "fb-ios", "fb-android" and "fb-server". We would still have a system ".lldbinit" for global settings of course.
• We were currently doing stuff like this in python where it would auto import some of our facebook specific python modules when the debugger starts up, would that be useful as well? It wouldn't really be needed here because we could just "command script import" in the init files

[labath]

In D119831#3324830, @clayborg wrote:

This looks good to me. Just a few things to possibly think about:

• Maybe we would want addition system init files for different workflows and then we would start lldb with a new option like "lldb --workflow qemu" and it would load the system ".lldbinit-qemu" init file. That way we could have many supported workflows from one distribution. Here at Facebook we would have at least 3 that I know of: "fb-ios", "fb-android" and "fb-server". We would still have a system ".lldbinit" for global settings of course.

You are correctly guessing that this is motivated (at least partially, anyway) by the qemu patchset, but the idea there is to have a plain "lldb" just work. So, I don't think this would be interesting for us right now.

Nonetheless, I do find that idea intriguing. We would need to reconciliate this somehow with the existing concept of application-specific lldbinit files -- I think having both would be too much. Perhaps if we rename the app-specific files to "workflow-specific init files", and have the default workflow be derived from the application name... I don't think this would work right now, as we probably realpath everything, but if we avoided that then we could even allow using symlinks to automatically choose different workflows (ln -s lldb lldb.qemu; ./lldb.qemu could launch a "qemu" workflow).

(Of course, at that point, one could just make lldb.qemu a shell script which effectively does a exec lldb -S qemu.lldbinit "$@", and avoid all this extra infrastructure.)

• We were currently doing stuff like this in python where it would auto import some of our facebook specific python modules when the debugger starts up, would that be useful as well? It wouldn't really be needed here because we could just "command script import" in the init files

Right now, it looks like all of our setup will be done in python as well, and our lldbinit file will probably consist of a single command script import --relative-to-command-file line. Going straight to python would probably make things easier for us, but this seemed to fit in better into the existing lldb feature set.

lldb/include/lldb/Interpreter/CommandInterpreter.h

256

Yeah, that was the reason why I changed the word order. We can just call it "global" instead.

lldb/source/Interpreter/CommandInterpreter.cpp

2385–2393

It just happened to fall out that way. I agree switching the order looks better.

[labath]

Use the term "global" and rename functions to make it more consistent with
existing code.

It's worth noting that (as a part of the consistency), I have changed the cmake
variable to point to a directory instead of a file. This means that the init
file name is currently hardcoded -- currently it's "lldbinit", but we could
change that into something more complex to support the workflow feature.

In particular, I note the fact that the file name does not include a leading
dot, as I believe is the practice for global configuration files.

[labath]

And add the extra blank line.

[JDevlieghere]

In D119831#3325798, @labath wrote:

It's worth noting that (as a part of the consistency), I have changed the cmake
variable to point to a directory instead of a file. This means that the init
file name is currently hardcoded -- currently it's "lldbinit", but we could
change that into something more complex to support the workflow feature.

In particular, I note the fact that the file name does not include a leading
dot, as I believe is the practice for global configuration files.

Specifying a directory instead of a file makes sense. Why omit the leading dot though? I can think of a few reasons (you can specify a subdir unlike home, it's probably not something the user modifies unlike the one in home and cwd), but still it seems needlessly confusing to me.

[labath]

In D119831#3329894, @JDevlieghere wrote:

Specifying a directory instead of a file makes sense. Why omit the leading dot though? I can think of a few reasons (you can specify a subdir unlike home, it's probably not something the user modifies unlike the one in home and cwd), but still it seems needlessly confusing to me.

I did it mainly because (I believe) that is common practice (so, *not* doing it could be confusing), and I believe the reason is precisely the one you mention -- as it's going to be in a global directory, you don't need it to be hidden (quite the opposite).

If we take bash as an example, here are some of the files it accesses:
~/.terminfo vs /etc/terminfo (a directory) -- done through the terminfo library, so which is used by lldb as well
~/.bashrc vs /etc/bash/bashrc
~/.dir_colors vs /etc/DIR_COLORS -- this isn't hardcoded in bash, but accessed from /etc/bash/bashrc
~/.inputrc vs /etc/inputrc
~/.bash_profile vs /etc/profile

[JDevlieghere]

In D119831#3329959, @labath wrote:

In D119831#3329894, @JDevlieghere wrote:

Specifying a directory instead of a file makes sense. Why omit the leading dot though? I can think of a few reasons (you can specify a subdir unlike home, it's probably not something the user modifies unlike the one in home and cwd), but still it seems needlessly confusing to me.

I did it mainly because (I believe) that is common practice (so, *not* doing it could be confusing), and I believe the reason is precisely the one you mention -- as it's going to be in a global directory, you don't need it to be hidden (quite the opposite).

If we take bash as an example, here are some of the files it accesses:
~/.terminfo vs /etc/terminfo (a directory) -- done through the terminfo library, so which is used by lldb as well
~/.bashrc vs /etc/bash/bashrc
~/.dir_colors vs /etc/DIR_COLORS -- this isn't hardcoded in bash, but accessed from /etc/bash/bashrc
~/.inputrc vs /etc/inputrc
~/.bash_profile vs /etc/profile

Good point, I was thinking about it too narrowly, strictly from an LLDB perspective.

LGTM!

lldb/cmake/modules/LLDBConfig.cmake

 option(LLDB_USE_SYSTEM_SIX "Use six.py shipped with system and do not install a copy of it" OFF)
 option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON)
 option(LLDB_BUILD_FRAMEWORK "Build LLDB.framework (Darwin only)" OFF)
 option(LLDB_NO_INSTALL_DEFAULT_RPATH "Disable default RPATH settings in binaries" OFF)
 option(LLDB_USE_SYSTEM_DEBUGSERVER "Use the system's debugserver for testing (Darwin only)." OFF)
 option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing lldb." OFF)
 option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing lldb." OFF)
 
+set(LLDB_GLOBAL_INIT_DIRECTORY "" CACHE STRING
+  "Path to the global lldbinit directory. Relative paths are resolved relative to the
+  directory containing the LLDB library.")
+
 if (LLDB_USE_SYSTEM_DEBUGSERVER)
   # The custom target for the system debugserver has no install target, so we
   # need to remove it from the LLVM_DISTRIBUTION_COMPONENTS list.
   if (LLVM_DISTRIBUTION_COMPONENTS)
     list(REMOVE_ITEM LLVM_DISTRIBUTION_COMPONENTS debugserver)
     set(LLVM_DISTRIBUTION_COMPONENTS ${LLVM_DISTRIBUTION_COMPONENTS} CACHE STRING "" FORCE)
   endif()
 endif()

lldb/include/lldb/API/SBCommandInterpreter.h

public:
   ///
   /// \return
   ///     A lldb::SBCommand representing the newly created command.
   lldb::SBCommand AddCommand(const char *name,
                              lldb::SBCommandPluginInterface *impl,
                              const char *help, const char *syntax,
                              const char *auto_repeat_command);
 
+  void SourceInitFileInGlobalDirectory(lldb::SBCommandReturnObject &result);
+
   void SourceInitFileInHomeDirectory(lldb::SBCommandReturnObject &result);
   void SourceInitFileInHomeDirectory(lldb::SBCommandReturnObject &result,
                                      bool is_repl);
 
   void
   SourceInitFileInCurrentWorkingDirectory(lldb::SBCommandReturnObject &result);
 
   lldb::ReturnStatus HandleCommand(const char *command_line,

lldb/include/lldb/Host/Config.h.cmake

 #cmakedefine01 LLDB_ENABLE_FBSDVMCORE
 
 #cmakedefine01 LLDB_EMBED_PYTHON_HOME
 
 #cmakedefine LLDB_PYTHON_HOME R"(${LLDB_PYTHON_HOME})"
 
 #define LLDB_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}"
 
+#cmakedefine LLDB_GLOBAL_INIT_DIRECTORY R"(${LLDB_GLOBAL_INIT_DIRECTORY})"
+
 #endif // #ifndef LLDB_HOST_CONFIG_H

lldb/include/lldb/Interpreter/CommandInterpreter.h

public:
   static ConstString &GetStaticBroadcasterClass();
 
   ConstString &GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
 
   void SourceInitFileCwd(CommandReturnObject &result);
   void SourceInitFileHome(CommandReturnObject &result, bool is_repl);
+  void SourceInitFileGlobal(CommandReturnObject &result);
 
   bool AddCommand(llvm::StringRef name, const lldb::CommandObjectSP &cmd_sp,
                   bool can_replace);
 
   Status AddUserCommand(llvm::StringRef name,
                         const lldb::CommandObjectSP &cmd_sp, bool can_replace);
 
   lldb::CommandObjectSP GetCommandSPExact(llvm::StringRef cmd,

lldb/source/API/SBCommandInterpreter.cpp

CommandInterpreter &SBCommandInterpreter::ref() {
   return *m_opaque_ptr;
 }
 
 void SBCommandInterpreter::reset(
     lldb_private::CommandInterpreter *interpreter) {
   m_opaque_ptr = interpreter;
 }
 
+void SBCommandInterpreter::SourceInitFileInGlobalDirectory(
+    SBCommandReturnObject &result) {
+  LLDB_INSTRUMENT_VA(this, result);
+
+  result.Clear();
+  if (IsValid()) {
+    TargetSP target_sp(m_opaque_ptr->GetDebugger().GetSelectedTarget());
+    std::unique_lock<std::recursive_mutex> lock;
+    if (target_sp)
+      lock = std::unique_lock<std::recursive_mutex>(target_sp->GetAPIMutex());
+    m_opaque_ptr->SourceInitFileGlobal(result.ref());
+  } else {
+    result->AppendError("SBCommandInterpreter is not valid");
+  }
+}
+
 void SBCommandInterpreter::SourceInitFileInHomeDirectory(
     SBCommandReturnObject &result) {
   LLDB_INSTRUMENT_VA(this, result);
 
   SourceInitFileInHomeDirectory(result, /*is_repl=*/false);
 }
 
 void SBCommandInterpreter::SourceInitFileInHomeDirectory(

lldb/source/API/SBDebugger.cpp

SBDebugger SBDebugger::Create(bool source_init_files,
 
   debugger.reset(Debugger::CreateInstance(callback, baton));
 
   SBCommandInterpreter interp = debugger.GetCommandInterpreter();
   if (source_init_files) {
     interp.get()->SkipLLDBInitFiles(false);
     interp.get()->SkipAppInitFiles(false);
     SBCommandReturnObject result;
+    interp.SourceInitFileInGlobalDirectory(result);
     interp.SourceInitFileInHomeDirectory(result, false);
   } else {
     interp.get()->SkipLLDBInitFiles(true);
     interp.get()->SkipAppInitFiles(true);
   }
   return debugger;
 }
 

lldb/source/Interpreter/CommandInterpreter.cpp

if (!m_skip_app_init_files) {
     GetHomeInitFile(program_init_file, program_name);
     if (FileSystem::Instance().Exists(program_init_file))
       init_file = program_init_file;
   }
 
   SourceInitFile(FileSpec(init_file.str()), result);
 }
 
+void CommandInterpreter::SourceInitFileGlobal(CommandReturnObject &result) {
+#ifdef LLDB_GLOBAL_INIT_DIRECTORY
+  if (!m_skip_lldbinit_files) {
+    FileSpec init_file(LLDB_GLOBAL_INIT_DIRECTORY);
+    if (init_file)
+      init_file.MakeAbsolute(HostInfo::GetShlibDir());
+
+    init_file.AppendPathComponent("lldbinit");
+    SourceInitFile(init_file, result);
+    return;
+  }
+#endif
+  result.SetStatus(eReturnStatusSuccessFinishNoResult);
+}
+
 const char *CommandInterpreter::GetCommandPrefix() {
   const char *prefix = GetDebugger().GetIOHandlerCommandPrefix();
   return prefix == nullptr ? "" : prefix;
 }
 
 PlatformSP CommandInterpreter::GetPlatform(bool prefer_target_platform) {
   PlatformSP platform_sp;
   if (prefer_target_platform) {

lldb/tools/driver/Driver.cpp

#endif
   if ((isatty(STDIN_FILENO) != 0) &&
       ::ioctl(STDIN_FILENO, TIOCGWINSZ, &window_size) == 0) {
     if (window_size.ws_col > 0)
       m_debugger.SetTerminalWidth(window_size.ws_col);
   }
 
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
 
-  // Before we handle any options from the command line, we parse the
-  // REPL init file or the default file in the user's home directory.
+  // Process lldbinit files before handling any options from the command line.
   SBCommandReturnObject result;
+  sb_interpreter.SourceInitFileInGlobalDirectory(result);
+  if (m_option_data.m_debug_mode) {
+    result.PutError(m_debugger.GetErrorFile());
+    result.PutOutput(m_debugger.GetOutputFile());
+  }
+
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
   if (m_option_data.m_debug_mode) {
     result.PutError(m_debugger.GetErrorFile());
     result.PutOutput(m_debugger.GetOutputFile());
   }
 
   // Source the local .lldbinit file if it exists and we're allowed to source.
   // Here we want to always print the return object because it contains the