[Lldb-commits] r236925 and r232437

Chaoren Lin chaorenl at google.com
Tue May 19 14:07:59 PDT 2015


Ah, sorry. This was a side effect from reusing FileSpec::Dump for
FileSpec::GetPath, since that was the original behavior of Dump.

http://reviews.llvm.org/D9862 should address this, I'm going to run a few
tests to check if anything relies on the trailing slash from Dump.

On Tue, May 19, 2015 at 7:38 AM, Colin Riley <colin at codeplay.com> wrote:

> r236925 (below) seems to have some side effects on windows when combined
> with r232437 (Make ModuleList::GetSharedModule to use module_search_paths
> parameter.)
>
> -    if (m_directory)
> -        path.append(m_directory.GetCString(), m_directory.GetCString() +
> m_directory.GetLength());
> -    if (m_filename)
> -        llvm::sys::path::append(path, m_filename.GetCString());
> +    StreamString stream;
> +    Dump(&stream);
> +    path.append(stream.GetString().begin(), stream.GetString().end());
>
>
> Now a slash will appear at the end of a filespec path which references
> just a directory. On windows, ::stat fails when passed a path with trailing
> slash.
>
> This now, with changes from r232437 which checks for the existence of a
> directory, means no modules can be loaded from the search paths when
> launching a target using the dyld.
>
> If checking the existence of a filespec which refers to just a directory
> is a legitamate operation, there are various options for a fix. I've not
> fixed it immediately in case your use cases require something different.
> Given this doesn't seem to have failed any tests we may need to add some.
>
> Colin
>
> On 09/05/2015 02:21, Chaoren Lin wrote:
>
>> Author: chaoren
>> Date: Fri May  8 20:21:32 2015
>> New Revision: 236925
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=236925&view=rev
>> Log:
>> Set path syntax for remote executable FileSpec.
>>
>> Reviewers: ovyalov, zturner
>>
>> Subscribers: lldb-commits
>>
>> Differential Revision: http://reviews.llvm.org/D9579
>>
>> Modified:
>>      lldb/trunk/include/lldb/Host/FileSpec.h
>>      lldb/trunk/source/Host/common/FileSpec.cpp
>>
>>  lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>>      lldb/trunk/source/Target/Target.cpp
>>
>> Modified: lldb/trunk/include/lldb/Host/FileSpec.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=236925&r1=236924&r2=236925&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Host/FileSpec.h (original)
>> +++ lldb/trunk/include/lldb/Host/FileSpec.h Fri May  8 20:21:32 2015
>> @@ -78,6 +78,8 @@ public:
>>       //------------------------------------------------------------------
>>       explicit FileSpec (const char *path, bool resolve_path, PathSyntax
>> syntax = ePathSyntaxHostNative);
>>   +    explicit FileSpec (const char *path, bool resolve_path, ArchSpec
>> arch);
>> +
>>       //------------------------------------------------------------------
>>       /// Copy constructor
>>       ///
>>
>> Modified: lldb/trunk/source/Host/common/FileSpec.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=236925&r1=236924&r2=236925&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/source/Host/common/FileSpec.cpp (original)
>> +++ lldb/trunk/source/Host/common/FileSpec.cpp Fri May  8 20:21:32 2015
>> @@ -27,6 +27,7 @@
>>   #include <pwd.h>
>>   #endif
>>   +#include "lldb/Core/ArchSpec.h"
>>   #include "lldb/Core/DataBufferHeap.h"
>>   #include "lldb/Core/DataBufferMemoryMap.h"
>>   #include "lldb/Core/RegularExpression.h"
>> @@ -201,6 +202,11 @@ FileSpec::FileSpec(const char *pathname,
>>           SetFile(pathname, resolve_path, syntax);
>>   }
>>   +FileSpec::FileSpec(const char *pathname, bool resolve_path, ArchSpec
>> arch) :
>> +    FileSpec(pathname, resolve_path, arch.GetTriple().isOSWindows() ?
>> ePathSyntaxWindows : ePathSyntaxPosix)
>> +{
>> +}
>> +
>>   //------------------------------------------------------------------
>>   // Copy constructor
>>   //------------------------------------------------------------------
>> @@ -605,11 +611,10 @@ FileSpec::RemoveBackupDots (const ConstS
>>   void
>>   FileSpec::Dump(Stream *s) const
>>   {
>> -    static ConstString g_slash_only ("/");
>>       if (s)
>>       {
>>           m_directory.Dump(s);
>> -        if (m_directory && m_directory != g_slash_only)
>> +        if (m_directory && m_directory.GetStringRef().back() != '/')
>>               s->PutChar('/');
>>           m_filename.Dump(s);
>>       }
>> @@ -810,10 +815,9 @@ FileSpec::GetPath(bool denormalize) cons
>>   void
>>   FileSpec::GetPath(llvm::SmallVectorImpl<char> &path, bool denormalize)
>> const
>>   {
>> -    if (m_directory)
>> -        path.append(m_directory.GetCString(), m_directory.GetCString() +
>> m_directory.GetLength());
>> -    if (m_filename)
>> -        llvm::sys::path::append(path, m_filename.GetCString());
>> +    StreamString stream;
>> +    Dump(&stream);
>> +    path.append(stream.GetString().begin(), stream.GetString().end());
>>       Normalize(path, m_syntax);
>>       if (denormalize && !path.empty())
>>           DeNormalize(path, m_syntax);
>>
>> Modified:
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=236925&r1=236924&r2=236925&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>> (original)
>> +++
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>> Fri May  8 20:21:32 2015
>> @@ -1312,7 +1312,7 @@ GDBRemoteCommunicationClient::SendArgume
>>       const char *arg = NULL;
>>       const Args &launch_args = launch_info.GetArguments();
>>       if (exe_file)
>> -        exe_path = exe_file.GetPath(false);
>> +        exe_path = exe_file.GetPath();
>>       else
>>       {
>>           arg = launch_args.GetArgumentAtIndex(0);
>> @@ -3744,8 +3744,8 @@ GDBRemoteCommunicationClient::GetModuleI
>>       packet.PutCString("qModuleInfo:");
>>       packet.PutCStringAsRawHex8(module_path.c_str());
>>       packet.PutCString(";");
>> -    const auto& tripple = arch_spec.GetTriple().getTriple();
>> -    packet.PutBytesAsRawHex8(tripple.c_str(), tripple.size());
>> +    const auto& triple = arch_spec.GetTriple().getTriple();
>> +    packet.PutBytesAsRawHex8(triple.c_str(), triple.size());
>>         StringExtractorGDBRemote response;
>>       if (SendPacketAndWaitForResponse (packet.GetData(),
>> packet.GetSize(), response, false) != PacketResult::Success)
>> @@ -3795,7 +3795,7 @@ GDBRemoteCommunicationClient::GetModuleI
>>               extractor.GetStringRef ().swap (value);
>>               extractor.SetFilePos (0);
>>               extractor.GetHexByteString (value);
>> -            module_spec.GetFileSpec () = FileSpec (value.c_str(), false);
>> +            module_spec.GetFileSpec() = FileSpec(value.c_str(), false,
>> arch_spec);
>>           }
>>       }
>>
>> Modified: lldb/trunk/source/Target/Target.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=236925&r1=236924&r2=236925&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/source/Target/Target.cpp (original)
>> +++ lldb/trunk/source/Target/Target.cpp Fri May  8 20:21:32 2015
>> @@ -2377,8 +2377,9 @@ Target::Install (ProcessLaunchInfo *laun
>>                                   if (is_main_executable) // TODO: add
>> setting for always installing main executable???
>>                                   {
>>                                       // Always install the main
>> executable
>> +                                    remote_file =
>> FileSpec(module_sp->GetFileSpec().GetFilename().AsCString(),
>> +                                                           false,
>> module_sp->GetArchitecture());
>>                                       remote_file.GetDirectory() =
>> platform_sp->GetWorkingDirectory();
>> -                                    remote_file.GetFilename() =
>> module_sp->GetFileSpec().GetFilename();
>>                                   }
>>                               }
>>                               if (remote_file)
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>
> --
> - Colin Riley
> Senior Director,
> Parallel/Graphics Debugger Systems
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150519/d16e6b22/attachment.html>


More information about the lldb-commits mailing list