[PATCH] D106128: [lld-macho] Use DO_BIND_ADD_ADDR_IMM_SCALED for bind opcodes

[Vincent Lee via Phabricator via llvm-commits]

thevinster marked an inline comment as done.
thevinster added a comment.

In D106128#2890061 <https://reviews.llvm.org/D106128#2890061>, @int3 wrote:

> Fix looks good but can we figure out how it was happening in the first place? I would guess that we were running `optimizeOpcodes` on an empty vector, but I'm not sure how that would happen in the given test...

We assumed that the BIND_OPCODE_DONE would exist in the vector but it actually doesn't. It actually never gets stored in the vector and is just emitted after everything is optimized. Printing out the opcodes as well shows that BIND_OPCODE_DONE never existed. Now, why this wasn't caught in testing is that this pass happens on specific checks. Sooner or later, it will randomly encounter the an opcode of 0 (by random chance) and exit the loop. The size and contents of the vector are still unchanged so testing without ASAN continued to show correct results.



================
Comment at: lld/MachO/SyntheticSections.cpp:369
+  // expand it to the correct address.
+  for (i = 0; i < opcodes.size(); ++i) {
+    // It's unclear why the check needs to be less than BIND_IMMEDIATE_MASK,
----------------
int3 wrote:
> I think this can be a for-range loop
Done. Re-tested with ASAN. Everything looks good :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106128