[LLD][COFF] Fix ARM64 EC chunks comparator

[aganea]

Fix string weak ordering relationship. Since we were only testing !aType before, comp(a, b) and comp(b, a) would both return true if both values were nullopt

This has been found by the Debug MS-STL.

[llvmbot]

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-platform-windows

Author: Alexandre Ganea (aganea)

Changes

Fix string weak ordering relationship. Since we were only testing !aType before, comp(a, b) and comp(b, a) would both return true.

---

Full diff: https://github.com/llvm/llvm-project/pull/75407.diff

1 Files Affected:

• (modified) lld/COFF/Writer.cpp (+2-1)

diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 89e0f5b2df7441..0d28091efef2b1 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1480,7 +1480,8 @@ void Writer::sortECChunks() {
       llvm::stable_sort(sec->chunks, [=](const Chunk *a, const Chunk *b) {
         std::optional<chpe_range_type> aType = a->getArm64ECRangeType(),
                                        bType = b->getArm64ECRangeType();
-        return !aType || (bType && *aType < *bType);
+        return (!aType ? 0 : (unsigned)*aType + 1) <
+               (!bType ? 0 : (unsigned)*bType + 1);
       });
   }
 }

[cjacek]

Good catch, thanks! C-style casts are generally discouraged in LLVM, in this case I think we could entirely avoid casts by tweaking the expression:

Suggested change

	return (!aType ? 0 : (unsigned)*aType + 1) <
	(!bType ? 0 : (unsigned)*bType + 1);
	return bType && (!aType || *aType < *bType);

It would be also nice to have a test. In this case, reproducing it depends on STL implementation detail, so a mistake like this may not be caught on some setups, but I was able to reproduce the problem on Linux+libstdc++ with: https://gist.github.com/cjacek/7cac304bb03583d90452425cc71101b3

[aganea]

Thanks for getting back! Since you already have a diff ready do you mind sending out the PR & landing it? I can close this in that case.

[cjacek]

Sure, I created #75495.