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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
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);
----------------
peter.smith wrote:
> 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. 
Seems that we can use a `SmallMapVector<Key, Value, 0>`


================
Comment at: lld/ELF/MarkLive.cpp:233
     markSymbol(symtab.find(s));
+  for (auto &p : symtab.cmseSymMap) {
+    StringRef symName = p.first();
----------------
`for (auto [symName, _] : symtab.cmseSymMap)`


================
Comment at: lld/test/ELF/arm-cmse-diagnostics.s:96
+
+// ERR_SYMATTR-NOT: __acle_se_valid_1
+// ERR_SYMATTR-NOT: __acle_se_valid_2
----------------
Just needs one pattern


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139092