[PATCH] D24267: [ELF] - Added support of --gdb-index command line option.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 9 11:55:57 PDT 2016


ruiu added a comment.

I'm still reading this patch and haven't got the whole picture yet. A few swing-by comments. Please do it incrementally next time. You don't need to make up a complete, working code from day one -- but you can incrementally build a feature in the public repository, and it is actually more appreciated way of doing it than dropping a complete large patch out of the blue.


================
Comment at: ELF/Options.td:89-90
@@ -88,1 +88,4 @@
 
+def gdb_index: F<"gdb-index">,
+  HelpText<"Generate .gdb_index section">;
+
----------------
Sort. Write it in one line.

================
Comment at: ELF/OutputSections.cpp:82
@@ +81,3 @@
+  // and 5 more fields with different kinds of offsets.
+  CuListOffset = 6 * OffsetTypeSize;
+
----------------
Is CuListOffset always 6*4=24? If so, I don't see a reason to make this a variable.


https://reviews.llvm.org/D24267





More information about the llvm-commits mailing list