[PATCH] [llvm-symbolizer] Introduce the -dsym-hint option.

[Alexey Samsonov]

================
Comment at: test/tools/llvm-symbolizer/Inputs/dsym-test.c:2
@@ +1,3 @@
+int main() {
+  return 0;
+}
----------------
Do you need to add lit.local.cfg to this directory to exclude .c files from testsuite?

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.cpp:101
@@ -99,3 +100,3 @@
   SymbolDesc SD = { Address, Address };
-  SymbolMapTy::const_iterator it = M.upper_bound(SD);
-  if (it == M.begin())
+  auto symbol_iterator = SymbolMap.upper_bound(SD);
+  if (symbol_iterator == SymbolMap.begin())
----------------
SymbolIterator

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.cpp:327
@@ +326,3 @@
+  ErrorOr<MachO::uuid_command> bin_uuid_or_error = Obj->findAndGetUuidCommand();
+  if (error(dbg_uuid_or_error.getError()) ||
+      error(bin_uuid_or_error.getError()))
----------------
Is it OK for a binary to not have UUID command? I think that if the object is well-formed, but doesn't have UUID command, you shouldn't use error() wrapper, as it prints error messages to stderr.

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.cpp:334
@@ +333,3 @@
+
+void LLVMSymbolizer::populateDsymPaths(const std::string &ExePath,
+                                       std::vector<std::string> &OutPaths) {
----------------
Probably it's ok to even sink this function down to lookUpDsymFile (I don't see how it can be generally useful).

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.cpp:337
@@ +336,3 @@
+  StringRef Filename = sys::path::filename(ExePath);
+  const std::string &resource_path =
+      getDarwinDWARFResourceForPath(ExePath, Filename);
----------------
ResourcePath (or just get rid of local variable).

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.cpp:347
@@ +346,3 @@
+    const MachOObjectFile *MachExeObj, const std::string &ArchName) {
+  ObjectFile *result = nullptr;
+  // On Darwin we may find DWARF in separate object file in
----------------
This variable is not used.

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.cpp:383
@@ +382,3 @@
+    Binary *Bin = B.getBinary().get();
+    Obj = getObjectFileFromBinary(Bin, ArchName);
+    addOwningBinary(std::move(B));
----------------
Please check that the code works if Obj is nullptr

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.cpp:386
@@ -309,12 +385,3 @@
     if (Bin->isMachO() || Bin->isMachOUniversalBinary()) {
-      // On Darwin we may find DWARF in separate object file in
-      // resource directory.
-      const std::string &ResourcePath =
-          getDarwinDWARFResourceForPath(Path);
-      BinaryOrErr = createBinary(ResourcePath);
-      std::error_code EC = BinaryOrErr.getError();
-      if (EC != errc::no_such_file_or_directory && !error(EC)) {
-        OwningBinary<Binary> B = std::move(BinaryOrErr.get());
-        DbgBin = B.getBinary().get();
-        addOwningBinary(std::move(B));
-      }
+      const MachOObjectFile *MachObj = dyn_cast<const MachOObjectFile>(Obj);
+      if (MachObj)
----------------
  if (auto MachObj = dyn_cast<const MachOObjectFile>(Obj))
    DbgObj = ...;

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.h:95
@@ +94,3 @@
+      ObjectFileForArch;
+  std::map<std::pair<std::string, std::string>, ObjectPair>
+      ObjectPairForPathArch;
----------------
Thanks for the typedef cleanup. You may want to commit it separately as a cleanup change before the main patch.

================
Comment at: tools/llvm-symbolizer/LLVMSymbolize.h:130
@@ -124,5 +129,3 @@
   };
-  typedef std::map<SymbolDesc, StringRef> SymbolMapTy;
-  SymbolMapTy Functions;
-  SymbolMapTy Objects;
+  std::map<SymbolDesc, StringRef> Functions, Objects;
 };
----------------
Please define each member on a separate line.

================
Comment at: tools/llvm-symbolizer/llvm-symbolizer.cpp:132
@@ +131,3 @@
+    } else {
+      errs() << "Warning: dSYM hint must have the '.dSYM' extension.\n";
+    }
----------------
You may want to print the invalid flag value here.

http://reviews.llvm.org/D5705