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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Thanks for the updates. I think we're getting close now. I've made an alternative suggestion to how we can redirect symbols which might be more easily localised to the ARM.cpp file.



================
Comment at: lld/ELF/Arch/ARM.cpp:1055
+      warn("entry function '" + sym->getName() +
+           "' disappeared from secure code");
+  }
----------------
Recommend something like:
```
"entry function '" + sym->getName() +"' from CMSE import library is not present in secure application"
```


================
Comment at: lld/ELF/Relocations.cpp:1346
   const RelTy &rel = *i;
   uint32_t symIndex = rel.getSymbol(config->isMips64EL);
+  StringRef symName = sec->getFile<ELFT>()->getSymbol(symIndex).getName();
----------------
While I think this will work. I'm thinking that there could be a less intrusive way of achieving this. As I understand it when we create SG veneers we have input sections with `sym`, and `__acle_sym` defined to the same address. All relocations and references are to `sym`. 

In the current implementation, we redirect relocations from `sym` to `__acle_sym` and then it looks like we overwrite `sym` in finalize with the value of the SG Veneer.

Could we use a similar mechanism to `SymbolTable::wrap()` and essentially exchange `__acle_sym` with `sym` so that all references to `sym` go to `__acle_sym` we are then free to overwrite `sym`? 


================
Comment at: lld/ELF/SymbolTable.h:65
+  // Key is the <sym> name.
+  llvm::StringMap<std::pair<Symbol *, Symbol *>> cmseSymMap;
+
----------------
Can we use a struct with named values here rather than a pair for example:
```
struct EntryFunction {
  Symbol *acleSeSym;
  Symbol *sym;
};
```
I find nested pairs such as `second.first` and `second.second` hard to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139092