MachO::Binary::extend_section fix sorting order

If there are empty sections and non-empty section at the same offset, we
want to shift only empty sections.

The expected behavior is that we sort sections in ascending order, and
then `sections_to_shift` is trimmed, such that the last element becomes
`section`.

Previously, we were sorting in descending order, while keeping empty
sections in the same order. Due to this we were doing trimming
incorrectly.

Rework macho/test_builder.py::test_extend_section

The previous implementation of the test was expecting unsupported
behavior from LIEF.

LIEF currently does not support arbitrary calls to add_section and
extend_section and keeping the layout of the binary correct.
Currently it is responsibility of a user to ensure that the final layout
of the binary is well-formed.

This patch splits test into two cases:
1. Repeated calls to add_section followed by extend_section using
   different alignment values.
2. Multiple calls to add_section and then multiple calls to
   extend_section using the same alignment values.

src/MachO/Binary.cpp

@@ -1259,30 +1259,29 @@ bool Binary::extend_section(Section& section, size_t size) {
     return false;
   }
 
-  // Select sections that we need to shift.
+  // Note: if we are extending an empty section, then there may be many zero-sized sections at
+  // this offset `section.offset()`, and one non-empty section.
+
+  // Select sections that we need to shift to the left.
+  // Sections that come after the current one are not considered for shifting.
+  // Note: we compare sections by end_offset to exclude non-empty section that starts at the current offset.
   sections_cache_t sections_to_shift;
   for (Section& s : sections()) {
-    if (s.offset() > section.offset() || s.offset() == 0) {
+    if (s.offset() == 0 || s.offset() + s.size() > section.offset() + section.size()) {
       continue;
     }
     sections_to_shift.push_back(&s);
   }
   assert(!sections_to_shift.empty());
 
-  // Stable-sort by end_offset, preserving original order.
+  // Stable-sort by offset in ascending order as well as preserving original order.
   std::stable_sort(sections_to_shift.begin(), sections_to_shift.end(),
-            [](const Section* a, const Section* b) {
-              return a->offset() + a->size() > b->offset() + b->size();
-            });
+            [](const Section* a, const Section* b) { return a->offset() < b->offset(); });
 
-  // If we are extending an empty section, then there may be many sections at this offset,
-  // we do not want to shift sections which were added after the current one.
-  // Such that we preserve order of sections in which they were added to the binary.
-  if (section.size() == 0) {
-    auto it = std::find(sections_to_shift.begin(), sections_to_shift.end(), &section);
-    assert(it != sections_to_shift.end());
-    sections_to_shift.erase(std::next(it), sections_to_shift.end());
-  }
+  // We do not want to shift empty sections that were added after the current one.
+  auto it = std::find(sections_to_shift.begin(), sections_to_shift.end(), &section);
+  assert(it != sections_to_shift.end());
+  sections_to_shift.erase(std::next(it), sections_to_shift.end());
 
   // Find maximum alignment
   auto it_maxa = std::max_element(sections_to_shift.begin(), sections_to_shift.end(),

tests/macho/test_builder.py

@@ -187,40 +187,46 @@ def test_add_section_id(tmp_path):
         print(stdout)
         assert re.search(r'uid=', stdout) is not None
 
-def test_extend_section(tmp_path):
+def test_extend_section_1(tmp_path):
+    """ This test calls add_section followed by extend_section repeatedly.
+    """
     bin_path = pathlib.Path(get_sample("MachO/MachO64_x86-64_binary_id.bin"))
     original = lief.MachO.parse(bin_path.as_posix()).at(0)
     output = f"{tmp_path}/test_extend_section.bin"
 
     text_segment = original.get_segment("__TEXT")
-    def make_section(name, alignment):
-        section = lief.MachO.Section(f"__lief_{alignment}")
-        section.alignment = alignment
-        return section
-
-    sections = [
-        make_section("__lief_8", 8),
-        make_section("__lief_7", 7),
-        make_section("__lief_6", 6),
-        make_section("__lief_5", 5),
-        make_section("__lief_4", 4),
-        make_section("__lief_3", 3),
-        make_section("__lief_2", 2),
-        make_section("__lief_1", 1),
-        make_section("__lief_0", 0),
-        make_section("__lief_00", 0),
-    ]
-    sections = [original.add_section(text_segment, s) for s in sections]
 
-    checked, err = lief.MachO.check_layout(original)
-    assert checked, err
-
-    for i, section in enumerate(sections):
+    for i in range(9, -1, -1):
+        section = lief.MachO.Section(f"__lief_{i}")
+        section.alignment = i
+        section = original.add_section(text_segment, section)
         assert original.extend_section(section, 1 << section.alignment)
 
-    checked, err = lief.MachO.check_layout(original)
+    original.write(output)
+    new = lief.MachO.parse(output).at(0)
+
+    checked, err = lief.MachO.check_layout(new)
     assert checked, err
 
+def test_extend_section_2(tmp_path):
+    """ This test makes multiple calls to add_section, and then it
+        extends each added section using extend_section.
+    """
+    bin_path = pathlib.Path(get_sample("MachO/MachO64_x86-64_binary_id.bin"))
+    original = lief.MachO.parse(bin_path.as_posix()).at(0)
+    output = f"{tmp_path}/test_extend_section.bin"
+
+    text_segment = original.get_segment("__TEXT")
+
+    sections = []
+    for i in range(3):
+        section = lief.MachO.Section(f"__lief_{i}")
+        section.alignment = 2 # 2^2 == 4 bytes
+        sections.append(original.add_section(text_segment, section))
+
+    for section in sections:
+        assert original.extend_section(section, 1000)
+
     original.write(output)
     new = lief.MachO.parse(output).at(0)