[PATCH] D24294: [ELF] - Implemented --section-start, -Ttext, -Tdata, -Tbss options.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Config.h:67
@@ -66,2 +66,3 @@
   InputFile *FirstElf = nullptr;
+  llvm::StringMap<uint64_t> SectionStartList;
   llvm::StringRef DynamicLinker;
----------------
ruiu wrote:
> This is not a list but a map.
Done.

================
Comment at: ELF/Driver.cpp:375
@@ -374,1 +374,3 @@
 
+static uint64_t readSectionAddress(StringRef S) {
+  uint64_t VA = 0;
----------------
ruiu wrote:
> read -> parse to be consistent with `parseEmulation`.
Done.

================
Comment at: ELF/Driver.cpp:380
@@ +379,3 @@
+  if (S.getAsInteger(16, VA))
+    error("invalid argument for --section-start/-Tbss/-Tdata/-Ttext");
+  return VA;
----------------
ruiu wrote:
>   -Ttext: " + S);
Done. I did not do this initially because it does not catch everything. Ex:

--section-start ABCDF

ABCDF does not contain '=', and S will be empty, resulting message " ... -Ttext: ".
Was not sure if that worth to add initial expression as argument or do something for that case.

================
Comment at: ELF/Driver.cpp:384-385
@@ +383,4 @@
+
+static llvm::StringMap<uint64_t> getSectionStartList(opt::InputArgList &Args) {
+  llvm::StringMap<uint64_t> Ret;
+  for (auto *Arg : Args.filtered(OPT_section_start)) {
----------------
ruiu wrote:
> Remove `llvm::`.
Done.

================
Comment at: ELF/Driver.cpp:393-398
@@ +392,8 @@
+
+  if (Args.hasArg(OPT_Ttext))
+    Ret[".text"] = readSectionAddress(getString(Args, OPT_Ttext));
+  if (Args.hasArg(OPT_Tdata))
+    Ret[".data"] = readSectionAddress(getString(Args, OPT_Tdata));
+  if (Args.hasArg(OPT_Tbss))
+    Ret[".bss"] = readSectionAddress(getString(Args, OPT_Tbss));
+  return Ret;
----------------
ruiu wrote:
>   if (auto *Arg = Args.getLastArg(...))
>     Ret[...] = readSectionAddress(Arg->getValue());
Done.


https://reviews.llvm.org/D24294