[PATCH] D65539: [llvm-objcopy][MachO] Implement a layout algorithm for executables

[Seiya Nuta via Phabricator via llvm-commits]

seiya added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:151
+        }
+        VMSize = std::max(VMSize, Sec.Addr + Sec.Size);
+      } else {
----------------
alexshap wrote:
> khm, maybe I'm missing smth, but why do we have Sec.Addr + Sec.Size here ?
> 
We need to recompute the VMSize in case a new section is added at the end of the segment. Note that in an object file, it seems that `Sec.Addr` is offset from the beginning of the first section data.

Why we need to use `std::max()` instead of simply assigning `Sec.Addr + Sec.Size` is that sections are not necessarily sorted by their vm addresses Here's an example:

```
Sections [
  Section {
    Index: 0
    Name: __text (5F 5F 74 65 78 74 00 00 00 00 00 00 00 00 00 00)
    Segment: __TEXT (5F 5F 54 45 58 54 00 00 00 00 00 00 00 00 00 00)
    Address: 0x0
    Size: 0xC
    Offset: 472
    ...
  }
  Section {
    Index: 1
    Name: __bss (5F 5F 62 73 73 00 00 00 00 00 00 00 00 00 00 00)
    Segment: __DATA (5F 5F 44 41 54 41 00 00 00 00 00 00 00 00 00 00)
    Address: 0x50
    Size: 0x4
    Offset: 0
    ...
  }
  Section {
    Index: 2
    Name: __eh_frame (5F 5F 65 68 5F 66 72 61 6D 65 00 00 00 00 00 00)
    Segment: __TEXT (5F 5F 54 45 58 54 00 00 00 00 00 00 00 00 00 00)
    Address: 0x10
    Size: 0x40
    Offset: 488
    ...
  }
]
Segment {
  Cmd: LC_SEGMENT_64
  Name: 
  Size: 312
  vmaddr: 0x0
  vmsize: 0x54
  ...
}
```

In this object file, `__bss` is located at the end of the segment so the vmsize should be `(__bss' vmaddr) + (__bss' vmsize)`, namely, 0x54. However, since  `__eh_frame` comes after the `__bss` in the section list, we need to determine the end of the segment by `std::max`.

FYI, this part comes from [[ https://github.com/llvm/llvm-project/blob/c01140ef1ff66869d4ea9465c3cd198d82d30cab/llvm/lib/MC/MachObjectWriter.cpp#L823 | lib/MC/MachObjectWriter.cpp ]].


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65539/new/

https://reviews.llvm.org/D65539