[PATCH] D133456: [V2] MC: make section classification a bit more thorough

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 8 21:51:08 PDT 2022


MaskRay added inline comments.


================
Comment at: llvm/lib/MC/MCContext.cpp:567
     Kind = SectionKind::getText();
-  else
+  else if (~Flags & ELF::SHF_WRITE)
     Kind = SectionKind::getReadOnly();
----------------
I think `!(... & ...)` is more common


================
Comment at: llvm/lib/MC/MCContext.cpp:576
+               .StartsWith(".bss.", SectionKind::getBSS())
+               .StartsWith(".gnu.linkonce.b.", SectionKind::getBSS())
+               .StartsWith(".llvm.linkonce.b.", SectionKind::getBSS())
----------------
Drop all `gnu.linkonce` and `llvm.linkonce`. linkonce is used before COMDAT era (<2000) and llvm never generates it.


================
Comment at: llvm/lib/MC/MCContext.cpp:580
+               .Case(".data1", SectionKind::getData())
+               .Case(".data.rel.ro", SectionKind::getReadOnlyWithRel())
+               .Case(".rodata", SectionKind::getReadOnly())
----------------
`.data.rel.ro.*` is possible.


================
Comment at: llvm/lib/MC/MCContext.cpp:583
+               .Case(".rodata1", SectionKind::getReadOnly())
+               .Case(".tbss", SectionKind::getThreadBSS())
+               .StartsWith(".tbss.", SectionKind::getThreadData())
----------------
Instead of a StringSwitch, consider

```
if (Name.consumes_front(".tbss") && (Name.empty() || Name[0] == '.'))
  ...
```
so that the many prefixes don't have to be repeated  for Case/StartsWith.


================
Comment at: llvm/lib/MC/MCContext.cpp:592
+               .StartsWith(".debug_", SectionKind::getMetadata())
+               .Default(SectionKind::getText());
 
----------------
Is default `SectionKind::getText()` instead of `SectionKind::getReadOnly()` desired? I expect that text is covered by a previous `if`: `Flags & ELF::SHF_EXECINSTR`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133456



More information about the llvm-commits mailing list