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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM.



================
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;
----------------
jrtc27 wrote:
> 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.
> what happens if another architecture allocates LOPROC+3 to a different kind of section?

This is probably unlikely. The logic is a bit complex now. So moving it outside can reduce indentation, which seems fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86309