[PATCH] D87497: [llvm-objcopy][MachO] Fix --add-section

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added a subscriber: compnerd.
alexshap added a comment.

There were multiple issues with the test, I'll try to provide more details.

1. The input yaml was invalid, as by product the code path where a section should be added to one of the existing segments

wasn't covered (and the test didn't properly catch that)

1. The test contains a matrix of different configurations, roughly speaking [32-bit, 64-bit] * [Case1, Case2, Case3].

The format of the output for 64-bit is different from 32-bit (it contains the extra field "reserved3") + different offsets. 
Consequently, the checks (in the previous version) were partially incorrect.

I somehow find it inconvenient to have **interlacing** sequences of check-prefixes in this test.
So the intention here was to untangle it as follows: handle 32-bit / 64-bit input binaries separately, and for different cases factor out the common part.
Another important consideration is that the broken versions shared a bit more checks than the fixed ones do (as I mentioned above - the tool was always creating a new segment (in this test)). 
Alternatively, we can split the test into 2 (the input binaries just happened to share some names  / flags  - in general, they don't need to be coupled to this extent) 
or bring back the old structure  (even though I dislike it  I don't want to insist), or maybe somebody might have better ideas/suggestions.
P.S. By the way - it turns out that we already have a few pairs of 32-bit/64-bit tests.

Regarding the functional side - it's not urgent, I'll wait for @smeenai or @compnerd to take a look at it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87497