[PATCH] D86309: [ELF] Handle SHT_RISCV_ATTRIBUTES similarly to SHT_ARM_ATTRIBUTES

Jessica Clarke via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 5 07:08:14 PDT 2020


jrtc27 marked an inline comment as done.
jrtc27 added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:869
 
-  switch (sec.sh_type) {
-  case SHT_ARM_ATTRIBUTES: {
-    if (config->emachine != EM_ARM)
-      break;
+  if (config->emachine == EM_ARM && sec.sh_type == SHT_ARM_ATTRIBUTES) {
     ARMAttributeParser attributes;
----------------
MaskRay wrote:
> Keeping the switch may be fine.
> 
> ```
> case SHT_ARM_ATTRIBUTES:
>   assert(SHT_ARM_ATTRIBUTES == SHT_RISCV_ATTRIBUTES);
>   if (EM_ARM) {
>   } else if (EM_RISCV) {
>   } else
>     break;
> ```
> 
> Adding `assert(SHT_ARM_ATTRIBUTES == SHT_RISCV_ATTRIBUTES);` should be sufficiently clear.
I considered that but it just feels too dirty... and what happens if another architecture allocates LOPROC+3 to a different kind of section? Then you have completely unrelated code under SHT_ARM_ATTRIBUTES.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86309



More information about the llvm-commits mailing list