[PATCH] Initial commit for the llvm-dsymutil tool.

kledzik at apple.com kledzik at apple.com
Wed Nov 12 18:18:23 PST 2014


Regarding testing, if you are ok limiting the test to run on darwin, you could have .ll files which you compile and use the system linker to produce the final executable which then has its debug map processed.

================
Comment at: tools/dsymutil/DebugMap.h:39-41
@@ +38,5 @@
+public:
+  const std::vector<std::unique_ptr<DebugMapObject>> &getObjects() const {
+    return Objects;
+  }
+
----------------
This could be replaced by a begin()/end() which return an iterator that dereferences to a "const DebugMapObject &", to be nicer for clients.

================
Comment at: tools/dsymutil/DebugMap.h:48
@@ +47,3 @@
+class DebugMapObject {
+  std::string Filename;
+  llvm::object::OwningBinary<llvm::object::ObjectFile> File;
----------------
It looks like "Filename" is the same as ObjectFile getFileName().

The existing debug map tracks the source (ex: /path/foo.c) path as well as the object (ex: /path/foo.o) path.  Does this DebugMap need both? or just the object path? 


================
Comment at: tools/dsymutil/DebugMap.h:49
@@ +48,3 @@
+  std::string Filename;
+  llvm::object::OwningBinary<llvm::object::ObjectFile> File;
+
----------------
The current debug map also records the last-mod-time of the object file.  This allows whoever is processing the debug map to notice if the .o file has  changed out from under it.

================
Comment at: tools/dsymutil/DebugMap.h:52-56
@@ +51,7 @@
+  struct Symbol {
+    enum SymTy : uint8_t {
+      Unknown,
+      Variable,
+      Function,
+    };
+
----------------
Why is this needed?  

================
Comment at: tools/dsymutil/DebugMap.h:58-59
@@ +57,4 @@
+
+    uint64_t OrigAddress;
+    uint64_t Address;
+    SymTy Type;
----------------
Are these the address in the .o file and address in the final binary? Can the names be clearer.

What is the rule for tentative definitions which don't have an address in the .o file?  Maybe OrigAddress could be an llvm::Optional<>?

================
Comment at: tools/dsymutil/DebugMap.h:61
@@ +60,3 @@
+    SymTy Type;
+    bool Used;
+  };
----------------
What is "Used"? When would it be false?

================
Comment at: tools/dsymutil/DebugMap.h:70
@@ +69,3 @@
+
+  llvm::StringRef getName() const { return Filename; }
+
----------------
Can this be more specific (e.g.  getSourcePath() or getObjectFilePath()).

================
Comment at: tools/dsymutil/DebugMap.h:72-75
@@ +71,6 @@
+
+  void addSymbol(llvm::StringRef Name, Symbol::SymTy Type,
+                 uint64_t OrigAddress);
+
+  bool relocateSymbol(llvm::StringRef Name, uint64_t Address);
+
----------------
I'm trying to think how lld could build a DebugMapObject in memory.  lld would need to make an ObjectFile for each input, then for each atom in the output it would call addSymbol() and relocateSymbol()?  Can we add an optional final-address to addSymbol() so lld does not need to call relocateSymbol.


================
Comment at: tools/dsymutil/DebugMap.h:80
@@ +79,3 @@
+private:
+  void createSymbols();
+};
----------------
Hmm.  It looks like you create a Symbol for every entry in the .o file even if it is not in the final linked image?  Why not start with the STABS debug map and only create those symbols, then look for them in the original .o file.

Also, lld would not want createSymbols() be be called because it will be populating the map.

http://reviews.llvm.org/D6242






More information about the llvm-commits mailing list