[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:1082
+  // If this is an Arm CMSE secure app, replace references to entry symbol <sym>
+  // with its corresponding special symbol ; __acle_se_<sym>.
+  parallelForEach(ctx.objectFiles, [&](InputFile *file) {
----------------
Misuse of `;`?


================
Comment at: lld/ELF/Arch/ARM.cpp:1109
+    addSGVeneer(cast<Defined>(p.second.acleSeSym), cast<Defined>(p.second.sym));
+  for (auto &p : symtab.cmseImportLib) {
+    Defined *sym = p.second;
----------------
`auto [_, sym]` avoids `Defined *sym = p.second;`

ditto below


================
Comment at: lld/ELF/Arch/ARM.cpp:1129
+void ArmCmseSGSection::addSGVeneer(Symbol *acleSeSym, Symbol *sym) {
+  entries.emplace_back(std::make_pair(acleSeSym, sym));
+  if (symtab.cmseImportLib.count(sym->getName()))
----------------



================
Comment at: lld/ELF/Arch/ARM.cpp:1139
+  ArmCmseSGVeneer *ss = nullptr;
+  if (symtab.cmseImportLib.find(sym->getName()) == symtab.cmseImportLib.end()) {
+    ss = make<ArmCmseSGVeneer>(sym, acleSeSym);
----------------
Use `count` instead of `find(...) ==/!= end()`


================
Comment at: lld/ELF/Arch/ARM.cpp:1141
+    ss = make<ArmCmseSGVeneer>(sym, acleSeSym);
+    newEntries += 1;
+  } else {
----------------
`++newEntries`


================
Comment at: lld/ELF/Arch/ARM.cpp:1151
+void ArmCmseSGSection::writeTo(uint8_t *buf) {
+  for (auto *s : sgSections)
+    s->writeTo(buf + s->outSecOff - getVA());
----------------



================
Comment at: lld/ELF/Arch/ARM.cpp:1188
+
+  for (auto *s : sgSections) {
+    if (!s->getAddr().has_value())
----------------
`ArmCmseSGVeneer *`. ditto everywhere


================
Comment at: lld/test/ELF/arm-cmse-diagnostics.s:142
+
+    /// Symbol not absolute.
+    .global entry_not_absolute
----------------
These comments are indented while the majority of comments don't. Consider removing indentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139092