zturner requested changes to this revision.
zturner added a comment.
This revision now requires changes to proceed.
I know you are just following the existing pattern, but this whole function
appears broken in the presence of file systems that use backslash instead of
forward slash. Could you try to fix that? I've provided some suggestions
inline.
I also don't see any tests for this. We've been pretty lax about this
historically, but I want to start pushing back against CLs with no tests. If
you need help writing a test, let me know and we can figure something out. For
starters, what is the scenario you encountered this in?
================
Comment at: source/Host/common/Symbols.cpp:238-250
@@ -237,15 +237,15 @@
// Add /usr/lib/debug directory.
debug_file_search_paths.AppendIfUnique (FileSpec("/usr/lib/debug",
true));
std::string uuid_str;
const UUID &module_uuid = module_spec.GetUUID();
if (module_uuid.IsValid())
{
// Some debug files are stored in the .build-id directory like
this:
//
/usr/lib/debug/.build-id/ff/e7fe727889ad82bb153de2ad065b2189693315.debug
uuid_str = module_uuid.GetAsString("");
uuid_str.insert (2, 1, '/');
uuid_str = uuid_str + ".debug";
}
----------------
Can you do all of this only if the target is not Windows?
================
Comment at: source/Host/common/Symbols.cpp:263-270
@@ -266,6 +262,10 @@
files.push_back (dirname + "/" + symbol_filename);
files.push_back (dirname + "/.debug/" + symbol_filename);
files.push_back (dirname + "/.build-id/" + uuid_str);
- files.push_back (dirname + module_directory + "/" +
symbol_filename);
+
+ // Some debug files may stored in the module directory like this:
+ // /usr/lib/debug/usr/lib/library.so.debug
+ if (!file_dir.IsEmpty())
+ files.push_back (dirname + file_dir.AsCString() + "/" +
symbol_filename);
----------------
Don't use literal slashes anywhere. I know you're just copying the existing
code, which was already broken, but it seems easy enough to fix all this by
calling llvm::sys::path::append, which will do the right thing dependign on the
Host platform.
Since this code is in Host, it's ok to do this, since we're ultimately going to
pass this path directly through to the filesystem anyway, we already need to be
able to assume that we're dealing with a path that has the appropriate
separator for the platform anyway.
Repository:
rL LLVM
http://reviews.llvm.org/D13201
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
