[PATCH] D139092: [LLD][ELF] Cortex-M Security Extensions (CMSE) Support

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I was able to successfully link a trusted-firmware secure application example with this patch. I've only got one minor observation about symbol order. Hopefully easy to sort out. From there I should be able to approve.



================
Comment at: lld/ELF/Arch/ARM.cpp:1237
+  // Copy the secure gateway entry symbols to the import library symbol table.
+  for (auto &p : symtab.cmseSymMap) {
+    Defined *d = cast<Defined>(p.second.sym);
----------------
I noticed that for an example CMSE application that I tried, the symbols in the LLD import library were in reverse order of address. While there is no requirement for the symbols to be in ascending order it did look a bit strange.

The code here looks like it will be determined by the order that the StringMap iterator gives. Maps often have no defined order.

I recommend putting into a vector and sorting by address so we get a known address order. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139092