[PATCH] D44004: [ELF] - Show data and assignment commands in the map file.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/LinkerScript.h:113
+  std::string CommandString;
+  // The offset value within the section before execution of assignment.
+  unsigned Offset;
----------------
ruiu wrote:
> ruiu wrote:
> > Please insert a blank line before a comment.
> What do you mean by "before execution of assignment"? Before you assign a command to an output section, it doesn't have any address.
I mean that if we have a section description:

```
0  .foo : {
1    *(.bar)
2    . += 0x123;
3  }
```

Then at line 2, we have an assignment to Dot.
We handle commands one by one and offset before we process 
("before execution of assignment (//command//)" in my wording) the assignment will
be equal to the size of .bar. That offset is saved to Offset member.
After execution of assignment command, offset will be different and `Size` below
stores the difference.

I rewrote the comment slightly.


================
Comment at: ELF/MapFile.cpp:154
+      if (auto *Cmd = dyn_cast<ByteCommand>(Base)) {
+        writeHeader(OS, OSec->Addr + Cmd->Offset, Cmd->Size, Cmd->Size);
+        OS << Indent1 << Cmd->CommandString << '\n';
----------------
ruiu wrote:
> You are passing `Cmd->Size` as an alignment which seems wrong to me.
Right, that was incorrect, changed to 1.


================
Comment at: ELF/ScriptParser.cpp:118-119
+  // Concatenates tokens in a [FirstTok, LastTok] into string with Prefix.
+  std::string buildCommandString(StringRef Prefix, size_t FirstTok,
+                                 size_t LastTok);
+
----------------
ruiu wrote:
> FirstTok and LastTok are not tokens but positions, so you should avoid "Tok".
Not sure what is wrong here honestly. 
I think `first` and `last` can be both adjectives and nouns. I meant that they are adjectives describing the
tokens and hence they are not positions here I think.
I renamed to `First`/`Last` though as such naming is also OK for me.


https://reviews.llvm.org/D44004