[PATCH] [ELF] Add NMAGIC/OMAGIC support

Shankar Kalpathi Easwaran shankarke at gmail.com
Wed Jun 12 18:21:24 PDT 2013


  Thanks for the review.


================
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
----------------
Rui Ueyama wrote:
> Please add a prefix ("MAGIC_" or something) to enum constants. DEFAULT seems a bit confusing in particular.
Ok

================
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
----------------
Rui Ueyama wrote:
> In what situation it can be unknown? 
I will make it pure virtual 

================
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>(
----------------
Rui Ueyama wrote:
> We need a better diagnostics here.
Will change to better error code. 

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


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



More information about the llvm-commits mailing list