[PATCH] [ELF] Add NMAGIC/OMAGIC support

Rui Ueyama ruiu at google.com
Wed Jun 12 16:10:58 PDT 2013



================
Comment at: include/lld/ReaderWriter/ELFTargetInfo.h:42
@@ +41,3 @@
+             // PageAlign Data, Mark Text Segment/Data segment RW
+    OMAGIC   // Disallow shared libraries and dont align sections,
+             // Mark Text Segment/Data segment RW
----------------
Please add a prefix ("MAGIC_" or something) to enum constants. DEFAULT seems a bit confusing in particular.

================
Comment at: lib/ReaderWriter/ELF/Chunk.h:77
@@ -73,1 +76,3 @@
+  // Whats the contentType of the chunk ?
+  virtual int getContentType() { return UNKNOWN; }
   // Writer the chunk
----------------
In what situation it can be unknown? 

================
Comment at: lib/ReaderWriter/ELF/Reader.cpp:109
@@ -106,1 +108,3 @@
+      if (!_elfTargetInfo.allowLinkWithDynamicLibraries())
+        return llvm::make_error_code(llvm::errc::executable_format_error);
       auto f = createELF<DynamicFileCreateELFTraits>(
----------------
We need a better diagnostics here.

================
Comment at: lib/ReaderWriter/ELF/SegmentChunks.h:430
@@ +429,3 @@
+          (outputMagic == ELFTargetInfo::NMAGIC &&
+           (*si)->getContentType() == Section<ELFT>::DATA)) {
+        startOffset = llvm::RoundUpToAlignment(startOffset,
----------------
This condition appears three times. Let's make it a separate function.


http://llvm-reviews.chandlerc.com/D968



More information about the llvm-commits mailing list