<div dir="rtl"><div dir="ltr">Alp, also look at <a href="http://reviews.llvm.org/D3686">http://reviews.llvm.org/D3686</a> </div><div dir="ltr"><br></div><div dir="ltr">There, TmpDir is in host conventions and the hardcoded '/' results in paths like</div>
<div dir="ltr"><br></div><div dir="ltr"> c:\my\dir/file.cpp</div><div dir="ltr"><br></div><div dir="ltr">I tried to add you as reviewer there but Phabricator does not know you.</div></div><div class="gmail_extra"><div dir="ltr">
<br><br><div class="gmail_quote">2014-05-09 17:38 GMT+03:00 Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex">
<div dir="rtl"><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div dir="ltr">The use case would be building Linux executable on Windows and debugging with Linux gdb - but the paths are all '\\'. </div><div dir="ltr">
<br></div><div dir="ltr">This problem should exist in trunk clang as only the last path seperator is hardcoded to '/' and the directory before follows host conventions.Try</div><div dir="ltr"><br></div><div dir="ltr">
<font face="courier new, monospace">clang -S -g -target i686-pc-windows-gnu a.c<br></font></div><div dir="ltr"><br></div><div dir="ltr">for some trivial program and look at DW_AT_comp_dir, it will be in host convention.</div>
<div dir="ltr"><br></div><div dir="ltr">The purpose of the patch is to make alll path seperators in a directory+filename consistent. It does not make sense to mix host conventions with hardcoded '/'. </div><div dir="ltr">
<br></div><div dir="ltr">We can fix the inconsistency either by always normalizing MCDwarfDirs (and elsewhere needed) to '/' or by following the host convention all the time. Since most of clang already follows host convention (by using llvm::sys::path) including most of the debug info I thought it would make sense to fix the few hardcoded path seperators but certainly we can normalize all debug info to '/'.</div>
<div dir="ltr"><br></div><div dir="ltr">Yaron</div><div dir="ltr"><br></div><div dir="ltr"><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">2014-05-09 15:30 GMT+03:00 Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span>:</div>
<div><div class="h5">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br>
On 09/05/2014 12:48, Yaron Keren wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Takumi,<br>
<br>
I wish DOS would have used '/' but now it's far too late. Even today it sometimes causes troubles building on Windows, problems with configure, make,...<br>
<br>
The current state in incosistent, where most of LLVM uses sys::path and thus '\\' (on WIndows) but some places use hardcodes '/'.<br>
<br>
This patch does not make the '\\' or '/' decision but rather centralizes the decision to llvm::sys::path, so if someday we would like to switch all to '/' as you like, only path.cpp would be modified.<br>
</blockquote>
<br></div>
Takumi is pointing out that the decision of what path scheme to use on the host is potentially different to the scheme that will be used by the debugger on the target system.<br>
<br>
With your change Windows binaries will always have Unix paths (unless the compiler itself happens to be running on Windows, in which case Unix binaries will have Windows backslashes) -- does that work in practice with the respective debuggers on those platforms?<br>
<br>
If so then I don't see a problem.<br>
<br>
Alp.<br>
<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Yaron<br>
<br>
<br>
<br>
2014-05-09 14:39 GMT+03:00 NAKAMURA Takumi <<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a> <mailto:<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a>>><u></u>:<div>
<br>
<br>
I prefer '/' could be used everywhere, unless it were illegal. I think<br>
'\\' makes the world worse.<br>
<br>
2014-05-09 19:53 GMT+09:00 Yaron Keren <<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a><br></div>
<mailto:<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>><u></u>>:<div><br>
> In the discussion linked, Reid mentioned that when<br>
cross-compiling aren't<br>
> the paths are valid on the host only, so shouldn't the host path<br>
seperator<br>
> be used? the target may not even have a file system.<br>
><br>
> The alternative is to have all path seperators as '/', but this<br>
disagrees<br>
> with path.cpp functionality and all its many clients.<br>
><br>
> Alp, is that whan you meant?<br>
><br>
><br>
> 2014-05-09 13:21 GMT+03:00 Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><br></div>
<mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>>:<div><div><br>
>><br>
>><br>
>> On 09/05/2014 10:32, Yaron Keren wrote:<br>
>>><br>
>>> Hi echristo, rnk, chapuni,<br>
>>><br>
>>> Earlier duscussion:<br>
>>> <a href="https://www.mail-archive.com/cfe-commits@cs.uiuc.edu/msg92891.html" target="_blank">https://www.mail-archive.com/<u></u>cfe-commits@cs.uiuc.edu/<u></u>msg92891.html</a><br>
>>><br>
>>> Most of LLVM code uses the llvm::sys::path::append function to<br>
>>> concatenate paths and will get the correct native slash. In<br>
some code<br>
>>> locations it seems that direct string concatenation was used,<br>
possibly<br>
>>> because the code writer knows there are no extra path<br>
sepeartors to deal<br>
>>> with, so string concat was more efficient than append.<br>
>>><br>
>>> In the locations below, the path seperator is hardcoded as<br>
'/'. Fixed by<br>
>>> exposing the native seperator from llvm::sys:;path and using it.<br>
>>><br>
>>> As an alternative, the append function could be used but it's<br>
more code<br>
>>> and less efficient.<br>
>>><br>
>>> <a href="http://reviews.llvm.org/D3687" target="_blank">http://reviews.llvm.org/D3687</a><br>
>>><br>
>>> Files:<br>
>>> include/llvm/Support/Path.h<br>
>>> lib/MC/MCDwarf.cpp<br>
>>> lib/Support/Path.cpp<br>
>>> lib/Support/SourceMgr.cpp<br>
>>><br>
>>> Index: include/llvm/Support/Path.h<br>
>>><br>
==============================<u></u>==============================<u></u>=======<br>
>>> --- include/llvm/Support/Path.h<br>
>>> +++ include/llvm/Support/Path.h<br>
>>> @@ -295,6 +295,11 @@<br>
>>> /// @result true if \a value is a path separator character<br>
on the host<br>
>>> OS<br>
>>> bool is_separator(char value);<br>
>>> +/// @brief Return the preferred separator for this platform.<br>
>>> +///<br>
>>> +/// @result const char* to the preferred separator,<br>
null-terminated.<br>
>>> +const char *get_separator();<br>
>>> +<br>
>>> /// @brief Get the typical temporary directory for the<br>
system, e.g.,<br>
>>> /// "/var/tmp" or "C:/TEMP"<br>
>>> ///<br>
>>> Index: lib/Support/Path.cpp<br>
>>><br>
==============================<u></u>==============================<u></u>=======<br>
>>> --- lib/Support/Path.cpp<br>
>>> +++ lib/Support/Path.cpp<br>
>>> @@ -569,6 +569,12 @@<br>
>>> }<br>
>>> }<br>
>>> +static const char preferred_separator_string[] = {<br>
>>> preferred_separator, '\0' };<br>
>>> +<br>
>>> +const char *get_separator() {<br>
>>> + return preferred_separator_string;<br>
>>> +}<br>
>>> +<br>
>>> void system_temp_directory(bool erasedOnReboot,<br>
SmallVectorImpl<char><br>
>>> &result) {<br>
>>> result.clear();<br>
>>> Index: lib/MC/MCDwarf.cpp<br>
>>><br>
==============================<u></u>==============================<u></u>=======<br>
>>> --- lib/MC/MCDwarf.cpp<br>
>>> +++ lib/MC/MCDwarf.cpp<br>
>>> @@ -689,7 +689,7 @@<br>
>>> const SmallVectorImpl<std::string> &MCDwarfDirs =<br>
>>> context.getMCDwarfDirs();<br>
>>> if (MCDwarfDirs.size() > 0) {<br>
>>> MCOS->EmitBytes(MCDwarfDirs[0]<u></u>);<br>
>>> - MCOS->EmitBytes("/");<br>
>>> + MCOS->EmitBytes(sys::path::<u></u>get_separator());<br>
>>> }<br>
>>> const SmallVectorImpl<MCDwarfFile> &MCDwarfFiles =<br>
>>> MCOS->getContext().<u></u>getMCDwarfFiles();<br>
>><br>
>><br>
>> I know there isn't much clarity yet on the issue but it's worth<br>
mentioning<br>
>> (in the commit message or a comment) the current state of<br>
affairs regarding<br>
>> paths in cross-compiled debug info.<br>
>><br>
>> Alp.<br>
>><br>
>><br>
>><br>
>>> Index: lib/Support/SourceMgr.cpp<br>
>>><br>
==============================<u></u>==============================<u></u>=======<br>
>>> --- lib/Support/SourceMgr.cpp<br>
>>> +++ lib/Support/SourceMgr.cpp<br>
>>> @@ -18,6 +18,7 @@<br>
>>> #include "llvm/ADT/Twine.h"<br>
>>> #include "llvm/Support/Locale.h"<br>
>>> #include "llvm/Support/MemoryBuffer.h"<br>
>>> +#include "llvm/Support/Path.h"<br>
>>> #include "llvm/Support/raw_ostream.h"<br>
>>> #include "llvm/Support/system_error.h"<br>
>>> using namespace llvm;<br>
>>> @@ -60,7 +61,7 @@<br>
>>> // If the file didn't exist directly, see if it's in an<br>
include<br>
>>> path.<br>
>>> for (unsigned i = 0, e = IncludeDirectories.size(); i != e &&<br>
>>> !NewBuf; ++i) {<br>
>>> - IncludedFile = IncludeDirectories[i] + "/" + Filename;<br>
>>> + IncludedFile = IncludeDirectories[i] +<br>
sys::path::get_separator() +<br>
>>> Filename;<br>
>>> MemoryBuffer::getFile(<u></u>IncludedFile.c_str(), NewBuf);<br>
>>> }<br>
>>><br>
>>><br>
>>> ______________________________<u></u>_________________<br>
>>> llvm-commits mailing list<br></div></div>
>>> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.<u></u>edu</a>><div>
<br>
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
>><br>
>><br>
>> --<br>
>> <a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
>> the browser experts<br>
>><br>
><br>
<br>
<br>
</div></blockquote><div><div>
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</div></div></blockquote></div></div></div><br></div>
</blockquote></div></div></div>