[Lldb-commits] [lldb] r199510 - Enable Linux distribution in vendor portion of host triple.

Greg Clayton gclayton at apple.com
Fri Jan 17 12:56:59 PST 2014


A few comments below...

On Jan 17, 2014, at 12:18 PM, Todd Fiala <tfiala at google.com> wrote:

> Author: tfiala
> Date: Fri Jan 17 14:18:59 2014
> New Revision: 199510
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=199510&view=rev
> Log:
> Enable Linux distribution in vendor portion of host triple.
> 
> This change does the following:
> 
> * enables building lldb-gdbserver on linux_x86-64 platforms.
> 
>  Note - it builds but it has several run-time issues where many gdb
>  remote protocol features are not properly implemented yet. I'm
>  working on these one at a time.
> 
> * lldb-gdbserver: does not enable the eLaunchFlagDebug launch flag on
>  Linux. Currently the POSIX launch routine will assert if that flag
>  is passed in, presumably because that launch mode is not yet
>  available.  This prevents lldb-gdbserver from asserting the moment
>  it launches the debuggee process.
> 
> * Adds ConstString& Host::GetDistributionId ()
> 
>  This method is defined to return an empty result on all platforms
>  except for Linux.  On Linux, it makes one attempt to execute
>  'lsb_release -i' (both /usr/bin/lsb_release, where it appears
>  on ubuntu, and /bin/lsb_release, where it appears on fedora
>  if the redhat-lsb package is installed).  If lsb_release is not
>  found in either of those locations, or if 'lsb_release -i' does
>  not return the first line starting with "Distributor ID:\t",
>  then the distribution id is empty.  The method will lower-case
>  the id and replace whitespace with underscores.

Can this be launched with "/usr/bin/env lsb_release"? Would that make it work on both systems with no differences?

> * Modify Host::GetArchitecture () so that linux replaces an unknown
>  vendor portion with the results of GetDistributionId () if that
>  is non-empty.  This shows up now in qHostInfo remote packet
>  responses and on the lldb host side.  Tested with ubuntu and
>  fedora (the latter both with the default of not having lsb_release
>  installed, and with having lsb_release installed via the redhat-lsb
>  package).
> 
>  Examples of triples on Linux after this change:
> 
>    # x86_64 Unbuntu 12.04 LTS:
>    x86_64-ubuntu-linux-gnu
> 
>    # x86_64 Fedora 20 Desktop with redhat-lsb package installed
>    x86_64-fedora-linux-gnu
> 
>    # x86_64 Fedora 20 Desktop without redhat-lsb-core installed
>    # (i.e. no /bin/lsb_release available)
>    # same as before the change
>    x86_64--linux-gnu
> 
>  Note I intend to have Android respond with:
> 
>    {arch}-android-linux
> 
>  when I get to implementing Android lldb-gdbserver support.

We need to be careful with making new target triples as these target triples must be consumable by clang and LLVM. I would avoid changing the triples in any way that doesn't play well with clang. A few possible work arounds:

1 - Modify ArchSpec to also be able to have a Distribution inside of it and then we use ArchSpec when we need to get the distro ID.
2 - Modify the llvm::Triple class to be able to include the distribution and deal with it there


> 
> Modified:
>    lldb/trunk/include/lldb/Host/Host.h
>    lldb/trunk/source/Host/common/Host.cpp
>    lldb/trunk/source/Host/linux/Host.cpp
>    lldb/trunk/tools/Makefile
>    lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp
> 
> Modified: lldb/trunk/include/lldb/Host/Host.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Host.h?rev=199510&r1=199509&r2=199510&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Host/Host.h (original)
> +++ lldb/trunk/include/lldb/Host/Host.h Fri Jan 17 14:18:59 2014
> @@ -202,6 +202,23 @@ public:
>     GetTargetTriple ();
> 
>     //------------------------------------------------------------------
> +    /// Gets the name of the distribution (i.e. distributor id).
> +    ///
> +    /// On Linux, this will return the equivalent of lsb_release -i.
> +    /// Android will return 'android'.  Other systems may return
> +    /// nothing.
> +    ///
> +    /// @return
> +    ///     A ConstString reference containing the OS distribution id.
> +    ///     The return string will be all lower case, with whitespace
> +    ///     replaced with underscores.  The return string will be
> +    ///     empty (result.AsCString() will return NULL) if the distribution
> +    ///     cannot be obtained.
> +    //------------------------------------------------------------------
> +    static const ConstString &
> +    GetDistributionId ();
> +
> +    //------------------------------------------------------------------
>     /// Get the process ID for the calling process.
>     ///
>     /// @return
> 
> Modified: lldb/trunk/source/Host/common/Host.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Host.cpp?rev=199510&r1=199509&r2=199510&view=diff
> ==============================================================================
> --- lldb/trunk/source/Host/common/Host.cpp (original)
> +++ lldb/trunk/source/Host/common/Host.cpp Fri Jan 17 14:18:59 2014
> @@ -368,7 +368,13 @@ Host::GetArchitecture (SystemDefaultArch
>         // If the OS is Linux, "unknown" in the vendor slot isn't what we want
>         // for the default triple.  It's probably an artifact of config.guess.
>         if (triple.getOS() == llvm::Triple::Linux && triple.getVendor() == llvm::Triple::UnknownVendor)
> -            triple.setVendorName("");
> +        {
> +            const char *const dist_c_str = GetDistributionId ().AsCString ();
> +            if (dist_c_str)
> +                triple.setVendorName (dist_c_str);
> +            else
> +                triple.setVendorName ("");
> +        }
> 
>         switch (triple.getArch())
>         {
> @@ -446,6 +452,19 @@ Host::GetTargetTriple()
>     return g_host_triple;
> }
> 
> +// See linux/Host.cpp for Linux-based implementations of this.
> +// Add your platform-specific implementation to the appropriate host file.
> +#if !defined(__linux__)
> +
> +const ConstString &
> +    Host::GetDistributionId ()
> +{
> +    static ConstString s_distribution_id;
> +    return s_distribution_id;
> +}
> +
> +#endif // #if !defined(__linux__)
> +
> lldb::pid_t
> Host::GetCurrentProcessID()
> {
> 
> Modified: lldb/trunk/source/Host/linux/Host.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/Host.cpp?rev=199510&r1=199509&r2=199510&view=diff
> ==============================================================================
> --- lldb/trunk/source/Host/linux/Host.cpp (original)
> +++ lldb/trunk/source/Host/linux/Host.cpp Fri Jan 17 14:18:59 2014
> @@ -20,6 +20,7 @@
> // Other libraries and framework includes
> // Project includes
> #include "lldb/Core/Error.h"
> +#include "lldb/Core/Log.h"
> #include "lldb/Target/Process.h"
> 
> #include "lldb/Host/Host.h"
> @@ -494,3 +495,114 @@ Host::GetEnvironment (StringList &env)
>         env.AppendString(env_entry);
>     return i;
> }
> +
> +const ConstString &
> +Host::GetDistributionId ()
> +{
> +    // Try to run 'lbs_release -i', and use that response
> +    // for the distribution id.
> +
> +    static bool s_evaluated;
> +    static ConstString s_distribution_id;
> +
> +    if (!s_evaluated)
> +    {
> +        s_evaluated = true;
> +
> +        Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_HOST));
> +        if (log)
> +            log->Printf ("attempting to determine Linux distribution...");
> +
> +        // check if the lsb_release command exists at one of the
> +        // following paths
> +        const char *const exe_paths[] = {
> +            "/bin/lsb_release",
> +            "/usr/bin/lsb_release"
> +        };
> +
> +        for (int exe_index = 0;
> +             exe_index < sizeof (exe_paths) / sizeof (exe_paths[0]);
> +             ++exe_index)
> +        {
> +            const char *const get_distribution_info_exe = exe_paths[exe_index];
> +            if (access (get_distribution_info_exe, F_OK))
> +            {
> +                // this exe doesn't exist, move on to next exe
> +                if (log)
> +                    log->Printf ("executable doesn't exist: %s",
> +                            get_distribution_info_exe);
> +                continue;
> +            }
> +
> +            // execute the distribution-retrieval command, read output
> +            std::string get_distribution_id_command (get_distribution_info_exe);
> +            get_distribution_id_command += " -i";
> +
> +            FILE *file = popen (get_distribution_id_command.c_str (), "r");
> +            if (!file)
> +            {
> +                if (log)
> +                    log->Printf (
> +                        "failed to run command: \"%s\", cannot retrieve "
> +                        "platform information",
> +                        get_distribution_id_command.c_str ());
> +                return s_distribution_id;
> +            }
> +
> +            // retrieve the distribution id string.
> +            char distribution_id[256] = { '\0' };
> +            if (fgets (distribution_id, sizeof (distribution_id) - 1, file)
> +                    != NULL)
> +            {
> +                if (log)
> +                    log->Printf ("distribution id command returned \"%s\"",
> +                            distribution_id);
> +
> +                const char *const distributor_id_key = "Distributor ID:\t";
> +                if (strstr (distribution_id, distributor_id_key))
> +                {
> +                    // strip newlines
> +                    std::string id_string (distribution_id +
> +                            strlen (distributor_id_key));
> +                    id_string.erase(
> +                        std::remove (
> +                            id_string.begin (),
> +                            id_string.end (),
> +                            '\n'),
> +                        id_string.end ());
> +
> +                    // lower case it and convert whitespace to underscores
> +                    std::transform (
> +                        id_string.begin(),
> +                        id_string.end (),
> +                        id_string.begin (),
> +                        [] (char ch)
> +                        { return tolower ( isspace (ch) ? '_' : ch ); });
> +
> +                    s_distribution_id.SetCString (id_string.c_str ());
> +                    if (log)
> +                        log->Printf ("distribion id set to \"%s\"",
> +                                s_distribution_id.GetCString ());
> +                }
> +                else
> +                {
> +                    if (log)
> +                        log->Printf ("failed to find \"%s\" field in \"%s\"",
> +                                distributor_id_key, distribution_id);
> +                }
> +            }
> +            else
> +            {
> +                if (log)
> +                    log->Printf (
> +                        "failed to retrieve distribution id, \"%s\" returned no"
> +                        " lines", get_distribution_id_command.c_str ());
> +            }
> +
> +            // clean up the file
> +            pclose(file);
> +        }
> +    }
> +
> +    return s_distribution_id;
> +}
> 
> Modified: lldb/trunk/tools/Makefile
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/Makefile?rev=199510&r1=199509&r2=199510&view=diff
> ==============================================================================
> --- lldb/trunk/tools/Makefile (original)
> +++ lldb/trunk/tools/Makefile Fri Jan 17 14:18:59 2014
> @@ -1,18 +1,27 @@
> ##===- source/Makefile -------------------------------------*- Makefile -*-===##
> -# 
> +#
> #                     The LLVM Compiler Infrastructure
> #
> # This file is distributed under the University of Illinois Open Source
> # License. See LICENSE.TXT for details.
> -# 
> +#
> ##===----------------------------------------------------------------------===##
> 
> LLDB_LEVEL := ..
> include $(LLDB_LEVEL)/../../Makefile.config
> 
> -# tfiala test commit: will include lldb-gdbserver for linux x86_64 soon.
> +# enable lldb-gdbserver for supported platforms
> +DIRS :=
> +ifneq (,$(strip $(filter $(HOST_OS), Linux)))
> +ifneq (,$(strip $(filter $(HOST_ARCH), x86_64)))
> +DIRS += lldb-gdbserver
> +else
> +endif
> +else
> +endif
> +
> ifneq ($(HOST_OS),MingW)
> -DIRS := driver lldb-platform
> +DIRS += driver lldb-platform
> endif
> 
> include $(LLDB_LEVEL)/Makefile
> 
> Modified: lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp?rev=199510&r1=199509&r2=199510&view=diff
> ==============================================================================
> --- lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp (original)
> +++ lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp Fri Jan 17 14:18:59 2014
> @@ -188,7 +188,7 @@ main (int argc, char *argv[])
>         display_usage(progname);
>         exit(255);
>     }
> -    
> +
>     const char *host_and_port = argv[0];
>     argc -= 1;
>     argv += 1;
> @@ -199,9 +199,15 @@ main (int argc, char *argv[])
>     {
>         // Launch the program specified on the command line
>         launch_info.SetArguments((const char **)argv, true);
> -        launch_info.GetFlags().Set(eLaunchFlagDebug | eLaunchFlagStopAtEntry);
> +
> +        unsigned int launch_flags = eLaunchFlagStopAtEntry;
> +#if !defined(__linux__)
> +        // linux doesn't yet handle eLaunchFlagDebug
> +        launch_flags |= eLaunchFlagDebug;
> +#endif
> +        launch_info.GetFlags ().Set (launch_flags);
>         error = Host::LaunchProcess (launch_info);
> -        
> +
>         if (error.Success())
>         {
>             printf ("Launched '%s' as process %" PRIu64 "...\n", argv[0], launch_info.GetProcessID());
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits




More information about the lldb-commits mailing list