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

Wed Mar 8 09:24:52 PST 2023

amilendra marked 2 inline comments as done.
amilendra added inline comments.

================
Comment at: lld/ELF/Arch/ARM.cpp:1022
+      continue;
+    if (!config->armCMSESupport)
+      error("cmse special symbol '" + acleSeSym->getName() +
----------------
MaskRay wrote:
> Printing an error for every symbol is verbose. I think `armCMSESupport` should be removed.
> 
> In `updateSupportedARMFeatures`, report a single error if not v8-M and perhaps `config->cmseImplib = false;` to disable `processArmCmseSymbol`.
Moving the error to `updateSupportedARMFeatures` will cause the link to silently continue when build attributes are not found. I've changed in a way that `processArmCmseSymbol` reports the error once and then resetting `config->cmseImplib` so that further processing in `processArmCmseSymbol` is not done. Is this okay?

================
Comment at: lld/test/ELF/arm-cmse-cmdline-options.s:1
+# REQUIRES: arm
+## Test diagnostics when --in-implib/--out-implib are used without --cmse-implib
----------------
MaskRay wrote:
> There are still too many test files. Consider grouping this negative test with others together.
I've consolidated all diagnostics for the arm architecture to a single test file.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139092