[PATCH] D24830: [ELF] - Linkerscript: implemented BYTE/SHORT/QUAD commands.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:407
@@ -402,3 +406,3 @@
 template <class ELFT> void LinkerScript<ELFT>::process(BaseCommand &Base) {
   if (auto *AssignCmd = dyn_cast<SymbolAssignment>(&Base)) {
     if (AssignCmd->Name == ".") {
----------------
This

================
Comment at: ELF/LinkerScript.cpp:417
@@ -412,1 +416,3 @@
   }
+  if (auto *DataCmd = dyn_cast<BytesDataCommand>(&Base)) {
+    DataCmd->Offset = Dot - CurOutSec->getVA();
----------------
this and 

================
Comment at: ELF/LinkerScript.cpp:424
@@ -413,2 +423,3 @@
+
   auto &ICmd = cast<InputSectionDescription>(Base);
   for (InputSectionData *ID : ICmd.Sections) {
----------------
this needs comments.

================
Comment at: ELF/LinkerScript.cpp:657
@@ +656,3 @@
+    if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base.get())) {
+      if (Cmd->Name == Name) {
+        for (const std::unique_ptr<BaseCommand> &Base2 : Cmd->Commands)
----------------
Don't we have a function to find an output section by name?

================
Comment at: ELF/LinkerScript.cpp:660-663
@@ +659,6 @@
+          if (auto *DataCmd = dyn_cast<BytesDataCommand>(Base2.get()))
+            // Documentation says that if the object file format of the output
+            // file has an explicit endianness, the value will be stored in that
+            // endianness.
+            memcpy(Buf + DataCmd->Offset, &DataCmd->Data, DataCmd->getSize());
+        return;
----------------
This code seems to write the bytes in the host endianess instead of the target endianness.

================
Comment at: ELF/LinkerScript.cpp:1431-1436
@@ +1430,8 @@
+BytesDataCommand *ScriptParser::readBytesDataCommand(StringRef Tok) {
+  unsigned Type = StringSwitch<unsigned>(Tok)
+                      .Case("BYTE", BytesDataCommand::Byte)
+                      .Case("SHORT", BytesDataCommand::Short)
+                      .Case("LONG", BytesDataCommand::Long)
+                      .Case("QUAD", BytesDataCommand::Quad)
+                      .Default(-1);
+  if (Type == (unsigned)-1)
----------------
I don't think we need these enums. Can't this be

  int Size = StringSwitch(Tok)
    .Case("BYTE", 1)
    .Case("SHORT", 2)
    ...

?

================
Comment at: ELF/LinkerScript.h:47-51
@@ -46,6 +46,7 @@
 enum SectionsCommandKind {
   AssignmentKind,
   OutputSectionKind,
   InputSectionKind,
-  AssertKind
+  AssertKind,
+  BytesDataKind
 };
----------------
It's time to add comments for each command.

  AssignmentKind, // . = expr or <sym> = expr
  OutputSectionKind,
  InputSectionKind,
  AssertKind, // ASSERT(expr)
  BytesDataKind, // BYTE(expr), SHORT(expr), LONG(expr) or QUAD(expr)

================
Comment at: ELF/OutputSections.cpp:1008
@@ -1007,2 +1007,3 @@
   }
+  Script<ELFT>::X->writeDataBytes(this->Name, Buf);
 }
----------------
It needs a comment.

  // Linker scripts may have BYTE()-family commands with which you
  // can write arbitrary bytes to the output. Process them if any.


https://reviews.llvm.org/D24830