<div dir="ltr">Yep - on it.<div><br></div><div>I'll adjust.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jan 17, 2014 at 12:56 PM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">A few comments below...<br>
<div><div class="h5"><br>
On Jan 17, 2014, at 12:18 PM, Todd Fiala <<a href="mailto:tfiala@google.com">tfiala@google.com</a>> wrote:<br>
<br>
> Author: tfiala<br>
> Date: Fri Jan 17 14:18:59 2014<br>
> New Revision: 199510<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=199510&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=199510&view=rev</a><br>
> Log:<br>
> Enable Linux distribution in vendor portion of host triple.<br>
><br>
> This change does the following:<br>
><br>
> * enables building lldb-gdbserver on linux_x86-64 platforms.<br>
><br>
> Note - it builds but it has several run-time issues where many gdb<br>
> remote protocol features are not properly implemented yet. I'm<br>
> working on these one at a time.<br>
><br>
> * lldb-gdbserver: does not enable the eLaunchFlagDebug launch flag on<br>
> Linux. Currently the POSIX launch routine will assert if that flag<br>
> is passed in, presumably because that launch mode is not yet<br>
> available. This prevents lldb-gdbserver from asserting the moment<br>
> it launches the debuggee process.<br>
><br>
> * Adds ConstString& Host::GetDistributionId ()<br>
><br>
> This method is defined to return an empty result on all platforms<br>
> except for Linux. On Linux, it makes one attempt to execute<br>
> 'lsb_release -i' (both /usr/bin/lsb_release, where it appears<br>
> on ubuntu, and /bin/lsb_release, where it appears on fedora<br>
> if the redhat-lsb package is installed). If lsb_release is not<br>
> found in either of those locations, or if 'lsb_release -i' does<br>
> not return the first line starting with "Distributor ID:\t",<br>
> then the distribution id is empty. The method will lower-case<br>
> the id and replace whitespace with underscores.<br>
<br>
</div></div>Can this be launched with "/usr/bin/env lsb_release"? Would that make it work on both systems with no differences?<br>
<div class="im"><br>
> * Modify Host::GetArchitecture () so that linux replaces an unknown<br>
> vendor portion with the results of GetDistributionId () if that<br>
> is non-empty. This shows up now in qHostInfo remote packet<br>
> responses and on the lldb host side. Tested with ubuntu and<br>
> fedora (the latter both with the default of not having lsb_release<br>
> installed, and with having lsb_release installed via the redhat-lsb<br>
> package).<br>
><br>
> Examples of triples on Linux after this change:<br>
><br>
> # x86_64 Unbuntu 12.04 LTS:<br>
> x86_64-ubuntu-linux-gnu<br>
><br>
> # x86_64 Fedora 20 Desktop with redhat-lsb package installed<br>
> x86_64-fedora-linux-gnu<br>
><br>
> # x86_64 Fedora 20 Desktop without redhat-lsb-core installed<br>
> # (i.e. no /bin/lsb_release available)<br>
> # same as before the change<br>
> x86_64--linux-gnu<br>
><br>
> Note I intend to have Android respond with:<br>
><br>
> {arch}-android-linux<br>
><br>
> when I get to implementing Android lldb-gdbserver support.<br>
<br>
</div>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:<br>
<br>
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.<br>
2 - Modify the llvm::Triple class to be able to include the distribution and deal with it there<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> Modified:<br>
> lldb/trunk/include/lldb/Host/Host.h<br>
> lldb/trunk/source/Host/common/Host.cpp<br>
> lldb/trunk/source/Host/linux/Host.cpp<br>
> lldb/trunk/tools/Makefile<br>
> lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp<br>
><br>
> Modified: lldb/trunk/include/lldb/Host/Host.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Host.h?rev=199510&r1=199509&r2=199510&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Host.h?rev=199510&r1=199509&r2=199510&view=diff</a><br>
> ==============================================================================<br>
> --- lldb/trunk/include/lldb/Host/Host.h (original)<br>
> +++ lldb/trunk/include/lldb/Host/Host.h Fri Jan 17 14:18:59 2014<br>
> @@ -202,6 +202,23 @@ public:<br>
> GetTargetTriple ();<br>
><br>
> //------------------------------------------------------------------<br>
> + /// Gets the name of the distribution (i.e. distributor id).<br>
> + ///<br>
> + /// On Linux, this will return the equivalent of lsb_release -i.<br>
> + /// Android will return 'android'. Other systems may return<br>
> + /// nothing.<br>
> + ///<br>
> + /// @return<br>
> + /// A ConstString reference containing the OS distribution id.<br>
> + /// The return string will be all lower case, with whitespace<br>
> + /// replaced with underscores. The return string will be<br>
> + /// empty (result.AsCString() will return NULL) if the distribution<br>
> + /// cannot be obtained.<br>
> + //------------------------------------------------------------------<br>
> + static const ConstString &<br>
> + GetDistributionId ();<br>
> +<br>
> + //------------------------------------------------------------------<br>
> /// Get the process ID for the calling process.<br>
> ///<br>
> /// @return<br>
><br>
> Modified: lldb/trunk/source/Host/common/Host.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Host.cpp?rev=199510&r1=199509&r2=199510&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Host.cpp?rev=199510&r1=199509&r2=199510&view=diff</a><br>
> ==============================================================================<br>
> --- lldb/trunk/source/Host/common/Host.cpp (original)<br>
> +++ lldb/trunk/source/Host/common/Host.cpp Fri Jan 17 14:18:59 2014<br>
> @@ -368,7 +368,13 @@ Host::GetArchitecture (SystemDefaultArch<br>
> // If the OS is Linux, "unknown" in the vendor slot isn't what we want<br>
> // for the default triple. It's probably an artifact of config.guess.<br>
> if (triple.getOS() == llvm::Triple::Linux && triple.getVendor() == llvm::Triple::UnknownVendor)<br>
> - triple.setVendorName("");<br>
> + {<br>
> + const char *const dist_c_str = GetDistributionId ().AsCString ();<br>
> + if (dist_c_str)<br>
> + triple.setVendorName (dist_c_str);<br>
> + else<br>
> + triple.setVendorName ("");<br>
> + }<br>
><br>
> switch (triple.getArch())<br>
> {<br>
> @@ -446,6 +452,19 @@ Host::GetTargetTriple()<br>
> return g_host_triple;<br>
> }<br>
><br>
> +// See linux/Host.cpp for Linux-based implementations of this.<br>
> +// Add your platform-specific implementation to the appropriate host file.<br>
> +#if !defined(__linux__)<br>
> +<br>
> +const ConstString &<br>
> + Host::GetDistributionId ()<br>
> +{<br>
> + static ConstString s_distribution_id;<br>
> + return s_distribution_id;<br>
> +}<br>
> +<br>
> +#endif // #if !defined(__linux__)<br>
> +<br>
> lldb::pid_t<br>
> Host::GetCurrentProcessID()<br>
> {<br>
><br>
> Modified: lldb/trunk/source/Host/linux/Host.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/Host.cpp?rev=199510&r1=199509&r2=199510&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/Host.cpp?rev=199510&r1=199509&r2=199510&view=diff</a><br>
> ==============================================================================<br>
> --- lldb/trunk/source/Host/linux/Host.cpp (original)<br>
> +++ lldb/trunk/source/Host/linux/Host.cpp Fri Jan 17 14:18:59 2014<br>
> @@ -20,6 +20,7 @@<br>
> // Other libraries and framework includes<br>
> // Project includes<br>
> #include "lldb/Core/Error.h"<br>
> +#include "lldb/Core/Log.h"<br>
> #include "lldb/Target/Process.h"<br>
><br>
> #include "lldb/Host/Host.h"<br>
> @@ -494,3 +495,114 @@ Host::GetEnvironment (StringList &env)<br>
> env.AppendString(env_entry);<br>
> return i;<br>
> }<br>
> +<br>
> +const ConstString &<br>
> +Host::GetDistributionId ()<br>
> +{<br>
> + // Try to run 'lbs_release -i', and use that response<br>
> + // for the distribution id.<br>
> +<br>
> + static bool s_evaluated;<br>
> + static ConstString s_distribution_id;<br>
> +<br>
> + if (!s_evaluated)<br>
> + {<br>
> + s_evaluated = true;<br>
> +<br>
> + Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_HOST));<br>
> + if (log)<br>
> + log->Printf ("attempting to determine Linux distribution...");<br>
> +<br>
> + // check if the lsb_release command exists at one of the<br>
> + // following paths<br>
> + const char *const exe_paths[] = {<br>
> + "/bin/lsb_release",<br>
> + "/usr/bin/lsb_release"<br>
> + };<br>
> +<br>
> + for (int exe_index = 0;<br>
> + exe_index < sizeof (exe_paths) / sizeof (exe_paths[0]);<br>
> + ++exe_index)<br>
> + {<br>
> + const char *const get_distribution_info_exe = exe_paths[exe_index];<br>
> + if (access (get_distribution_info_exe, F_OK))<br>
> + {<br>
> + // this exe doesn't exist, move on to next exe<br>
> + if (log)<br>
> + log->Printf ("executable doesn't exist: %s",<br>
> + get_distribution_info_exe);<br>
> + continue;<br>
> + }<br>
> +<br>
> + // execute the distribution-retrieval command, read output<br>
> + std::string get_distribution_id_command (get_distribution_info_exe);<br>
> + get_distribution_id_command += " -i";<br>
> +<br>
> + FILE *file = popen (get_distribution_id_command.c_str (), "r");<br>
> + if (!file)<br>
> + {<br>
> + if (log)<br>
> + log->Printf (<br>
> + "failed to run command: \"%s\", cannot retrieve "<br>
> + "platform information",<br>
> + get_distribution_id_command.c_str ());<br>
> + return s_distribution_id;<br>
> + }<br>
> +<br>
> + // retrieve the distribution id string.<br>
> + char distribution_id[256] = { '\0' };<br>
> + if (fgets (distribution_id, sizeof (distribution_id) - 1, file)<br>
> + != NULL)<br>
> + {<br>
> + if (log)<br>
> + log->Printf ("distribution id command returned \"%s\"",<br>
> + distribution_id);<br>
> +<br>
> + const char *const distributor_id_key = "Distributor ID:\t";<br>
> + if (strstr (distribution_id, distributor_id_key))<br>
> + {<br>
> + // strip newlines<br>
> + std::string id_string (distribution_id +<br>
> + strlen (distributor_id_key));<br>
> + id_string.erase(<br>
> + std::remove (<br>
> + id_string.begin (),<br>
> + id_string.end (),<br>
> + '\n'),<br>
> + id_string.end ());<br>
> +<br>
> + // lower case it and convert whitespace to underscores<br>
> + std::transform (<br>
> + id_string.begin(),<br>
> + id_string.end (),<br>
> + id_string.begin (),<br>
> + [] (char ch)<br>
> + { return tolower ( isspace (ch) ? '_' : ch ); });<br>
> +<br>
> + s_distribution_id.SetCString (id_string.c_str ());<br>
> + if (log)<br>
> + log->Printf ("distribion id set to \"%s\"",<br>
> + s_distribution_id.GetCString ());<br>
> + }<br>
> + else<br>
> + {<br>
> + if (log)<br>
> + log->Printf ("failed to find \"%s\" field in \"%s\"",<br>
> + distributor_id_key, distribution_id);<br>
> + }<br>
> + }<br>
> + else<br>
> + {<br>
> + if (log)<br>
> + log->Printf (<br>
> + "failed to retrieve distribution id, \"%s\" returned no"<br>
> + " lines", get_distribution_id_command.c_str ());<br>
> + }<br>
> +<br>
> + // clean up the file<br>
> + pclose(file);<br>
> + }<br>
> + }<br>
> +<br>
> + return s_distribution_id;<br>
> +}<br>
><br>
> Modified: lldb/trunk/tools/Makefile<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/Makefile?rev=199510&r1=199509&r2=199510&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/Makefile?rev=199510&r1=199509&r2=199510&view=diff</a><br>
> ==============================================================================<br>
> --- lldb/trunk/tools/Makefile (original)<br>
> +++ lldb/trunk/tools/Makefile Fri Jan 17 14:18:59 2014<br>
> @@ -1,18 +1,27 @@<br>
> ##===- source/Makefile -------------------------------------*- Makefile -*-===##<br>
> -#<br>
> +#<br>
> # The LLVM Compiler Infrastructure<br>
> #<br>
> # This file is distributed under the University of Illinois Open Source<br>
> # License. See LICENSE.TXT for details.<br>
> -#<br>
> +#<br>
> ##===----------------------------------------------------------------------===##<br>
><br>
> LLDB_LEVEL := ..<br>
> include $(LLDB_LEVEL)/../../Makefile.config<br>
><br>
> -# tfiala test commit: will include lldb-gdbserver for linux x86_64 soon.<br>
> +# enable lldb-gdbserver for supported platforms<br>
> +DIRS :=<br>
> +ifneq (,$(strip $(filter $(HOST_OS), Linux)))<br>
> +ifneq (,$(strip $(filter $(HOST_ARCH), x86_64)))<br>
> +DIRS += lldb-gdbserver<br>
> +else<br>
> +endif<br>
> +else<br>
> +endif<br>
> +<br>
> ifneq ($(HOST_OS),MingW)<br>
> -DIRS := driver lldb-platform<br>
> +DIRS += driver lldb-platform<br>
> endif<br>
><br>
> include $(LLDB_LEVEL)/Makefile<br>
><br>
> Modified: lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp?rev=199510&r1=199509&r2=199510&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp?rev=199510&r1=199509&r2=199510&view=diff</a><br>
> ==============================================================================<br>
> --- lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp (original)<br>
> +++ lldb/trunk/tools/lldb-gdbserver/lldb-gdbserver.cpp Fri Jan 17 14:18:59 2014<br>
> @@ -188,7 +188,7 @@ main (int argc, char *argv[])<br>
> display_usage(progname);<br>
> exit(255);<br>
> }<br>
> -<br>
> +<br>
> const char *host_and_port = argv[0];<br>
> argc -= 1;<br>
> argv += 1;<br>
> @@ -199,9 +199,15 @@ main (int argc, char *argv[])<br>
> {<br>
> // Launch the program specified on the command line<br>
> launch_info.SetArguments((const char **)argv, true);<br>
> - launch_info.GetFlags().Set(eLaunchFlagDebug | eLaunchFlagStopAtEntry);<br>
> +<br>
> + unsigned int launch_flags = eLaunchFlagStopAtEntry;<br>
> +#if !defined(__linux__)<br>
> + // linux doesn't yet handle eLaunchFlagDebug<br>
> + launch_flags |= eLaunchFlagDebug;<br>
> +#endif<br>
> + launch_info.GetFlags ().Set (launch_flags);<br>
> error = Host::LaunchProcess (launch_info);<br>
> -<br>
> +<br>
> if (error.Success())<br>
> {<br>
> printf ("Launched '%s' as process %" PRIu64 "...\n", argv[0], launch_info.GetProcessID());<br>
><br>
><br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><table cellspacing="0" cellpadding="0" style="color:rgb(136,136,136);font-family:'Times New Roman'"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small">
<td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Todd Fiala |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td>
<td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tfiala@google.com" style="color:rgb(17,85,204)" target="_blank"><span style="background-color:rgb(255,255,204);color:rgb(34,34,34);background-repeat:initial initial">tfiala@google.com</span></a> |</td>
<td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"><font color="#1155cc"> <a>650-943-3180</a></font></td></tr></tbody></table><br></div>
</div>