[PATCH] D14140: [ELF2] SECTIONS command basic support

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 29 06:48:35 PDT 2015


ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:250
@@ +249,3 @@
+  InSec.InputFile = next();
+  mapBraces("(", ")", [this, &InSec]() {
+    StringRef Tok = next();
----------------
I wouldn't write this in this functional style. This can be written in a plain C style if you define skip(S) which returns true if the next token is S.

  expect("(");
  while (!skip(")") {
    StringRef Tok = next();
    if (Tok == "EXCLUDE_FILE") {
    ...

================
Comment at: ELF/Writer.cpp:461-465
@@ +460,7 @@
+template <class ELFT>
+bool Writer<ELFT>::hasBoundingSymbols(OutputSectionBase<ELFT> *OS) const {
+  if (!hasCustomSections())
+    return true;
+  return OS && Config->OutputSections.count(OS->getName()) == 0;
+}
+
----------------
Why don't you create __start/__stop symbols if the section is defined using a linker script? Is that a documented behavior?

================
Comment at: ELF/Writer.cpp:614
@@ -574,3 +613,3 @@
 
-  std::stable_sort(OutputSections.begin(), OutputSections.end(),
-                   compareOutputSections<ELFT>);
+  std::stable_sort(
+      OutputSections.begin(), OutputSections.end(),
----------------
I don't like to add more code to createSections() which is already too long. Move this lambda out of this function.


http://reviews.llvm.org/D14140





More information about the llvm-commits mailing list