[PATCH] D67256: [yaml2obj] Simplify p_filesz/p_memsz computing

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: grimar, jakehehrlich, jhenderson.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This fixes a bug as well. When "FileSize:" (p_filesz) is specified and
different from the actual value, the following code probably should not
use PHeader.p_filesz:

  if (SHeader->sh_offset == PHeader.p_offset + PHeader.p_filesz)
    PHeader.p_memsz += SHeader->sh_size;


Repository:
  rL LLVM

https://reviews.llvm.org/D67256

Files:
  lib/ObjectYAML/ELFEmitter.cpp
  test/tools/yaml2obj/program-header-size-offset.yaml


Index: test/tools/yaml2obj/program-header-size-offset.yaml
===================================================================
--- test/tools/yaml2obj/program-header-size-offset.yaml
+++ test/tools/yaml2obj/program-header-size-offset.yaml
@@ -11,7 +11,7 @@
 
 # CHECK:    Offset: 0x2000
 # CHECK:    FileSize: 6
-# CHECK:    MemSize: 6
+# CHECK:    MemSize: 4
 
 # CHECK:    Offset: 0x2000
 # CHECK:    FileSize: 4
Index: lib/ObjectYAML/ELFEmitter.cpp
===================================================================
--- lib/ObjectYAML/ELFEmitter.cpp
+++ lib/ObjectYAML/ELFEmitter.cpp
@@ -624,36 +624,25 @@
         PHeader.p_offset = std::min(PHeader.p_offset, SHeader->sh_offset);
     }
 
-    // Find the maximum offset of the end of a section in order to set p_filesz,
-    // if not set explicitly.
-    if (YamlPhdr.FileSize) {
-      PHeader.p_filesz = *YamlPhdr.FileSize;
-    } else {
-      PHeader.p_filesz = 0;
-      for (Elf_Shdr *SHeader : Sections) {
-        uint64_t EndOfSection;
-        if (SHeader->sh_type == llvm::ELF::SHT_NOBITS)
-          EndOfSection = SHeader->sh_offset;
-        else
-          EndOfSection = SHeader->sh_offset + SHeader->sh_size;
-        uint64_t EndOfSegment = PHeader.p_offset + PHeader.p_filesz;
-        EndOfSegment = std::max(EndOfSegment, EndOfSection);
-        PHeader.p_filesz = EndOfSegment - PHeader.p_offset;
-      }
-    }
+    // Find the maximum offset of the end of a section in order to set p_filesz
+    // and p_memsz. When setting p_filesz, sh_size is seen as 0 for SHT_NOBITS
+    // sections.
+    uint64_t FileEnd = PHeader.p_offset, MemEnd = PHeader.p_offset;
+    for (Elf_Shdr *SHeader : Sections) {
+      uint64_t End = SHeader->sh_offset + SHeader->sh_size;
+      MemEnd = std::max(MemEnd, End);
 
-    // If not set explicitly, find the memory size by adding the size of
-    // sections at the end of the segment. These should be empty (size of zero)
-    // and NOBITS sections.
-    if (YamlPhdr.MemSize) {
-      PHeader.p_memsz = *YamlPhdr.MemSize;
-    } else {
-      PHeader.p_memsz = PHeader.p_filesz;
-      for (Elf_Shdr *SHeader : Sections)
-        if (SHeader->sh_offset == PHeader.p_offset + PHeader.p_filesz)
-          PHeader.p_memsz += SHeader->sh_size;
+      if (SHeader->sh_type == llvm::ELF::SHT_NOBITS)
+        End = SHeader->sh_offset;
+      FileEnd = std::max(FileEnd, End);
     }
 
+    // Set the file size and the memory size if not set explicitly.
+    PHeader.p_filesz = YamlPhdr.FileSize ? uint64_t(*YamlPhdr.FileSize)
+                                         : FileEnd - PHeader.p_offset;
+    PHeader.p_memsz = YamlPhdr.MemSize ? uint64_t(*YamlPhdr.MemSize)
+                                       : MemEnd - PHeader.p_offset;
+
     // Set the alignment of the segment to be the same as the maximum alignment
     // of the sections with the same offset so that by default the segment
     // has a valid and sensible alignment.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67256.219031.patch
Type: text/x-patch
Size: 2956 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190906/a06ff71c/attachment.bin>