[llvm-objcopy] Simplify --prefix-alloc-sections

[MaskRay]

Summary

Handle --prefix-alloc-sections after --rename-sections so that --prefix-alloc-sections code
does not have to check if renaming has been performed.

[jhenderson]

I agree that this code is simpler, but I'm slightly hesitant you might have impaired performance of the loop, since we now have to loop over all sections twice when using both --prefix-alloc-sections and --rename-sections switches. Admittedly, I suspect this is a rare case (I still don't know of a reasonable use-case for --prefix-alloc-sections), so maybe it's not worth worrying about. @rupprecht, I think you had some performance issues with llvm-objcopy in the past. Do you have any insight into whether my concern is justified? If not, LGTM.

[MaskRay]

PR41266 was filed for https://github.com/ClangBuiltLinux/linux/blob/master/drivers/firmware/efi/libstub/Makefile#L75
The performance should not matter.

[MaskRay]

Ping:)

[jhenderson]

LGTM, if @rupprecht is happy (or he doesn't get back in a reasonable amount of time).

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

static Error handleArgs(const CopyConfig &Config, Object &Obj,
   // us to avoid reporting the inappropriate errors about removing symbols
   // named in relocations.
   if (Error E = replaceAndRemoveSections(Config, Obj))
     return E;
 
   if (Error E = updateAndRemoveSymbols(Config, Obj))
     return E;
 
-  if (!Config.SectionsToRename.empty() || !Config.AllocSectionsPrefix.empty()) {
-    DenseSet<SectionBase *> PrefixedSections;
-    for (auto &Sec : Obj.sections()) {
+  if (!Config.SectionsToRename.empty()) {
+    for (SectionBase &Sec : Obj.sections()) {
       const auto Iter = Config.SectionsToRename.find(Sec.Name);
       if (Iter != Config.SectionsToRename.end()) {
         const SectionRename &SR = Iter->second;
         Sec.Name = SR.NewName;
         if (SR.NewFlags.hasValue())
           setSectionFlagsAndType(Sec, SR.NewFlags.getValue());
       }
+    }
+  }
 
-      // Add a prefix to allocated sections and their relocation sections. This
-      // should be done after renaming the section by Config.SectionToRename to
-      // imitate the GNU objcopy behavior.
-      if (!Config.AllocSectionsPrefix.empty()) {
-        if (Sec.Flags & SHF_ALLOC) {
-          Sec.Name = (Config.AllocSectionsPrefix + Sec.Name).str();
-          PrefixedSections.insert(&Sec);
-
-          // Rename relocation sections associated to the allocated sections.
-          // For example, if we rename .text to .prefix.text, we also rename
-          // .rel.text to .rel.prefix.text.
-          //
-          // Dynamic relocation sections (SHT_REL[A] with SHF_ALLOC) are handled
-          // above, e.g., .rela.plt is renamed to .prefix.rela.plt, not
-          // .rela.prefix.plt since GNU objcopy does so.
-        } else if (auto *RelocSec = dyn_cast<RelocationSectionBase>(&Sec)) {
-          auto *TargetSec = RelocSec->getSection();
-          if (TargetSec && (TargetSec->Flags & SHF_ALLOC)) {
-            StringRef prefix;
-            switch (Sec.Type) {
-            case SHT_REL:
-              prefix = ".rel";
-              break;
-            case SHT_RELA:
-              prefix = ".rela";
-              break;
-            default:
-              continue;
-            }
+    DenseSet<SectionBase *> PrefixedSections;
+    for (SectionBase &Sec : Obj.sections()) {
+      } else if (auto *RelocSec = dyn_cast<RelocationSectionBase>(&Sec)) {
+        const SectionBase *TargetSec = RelocSec->getSection();
+            llvm_unreachable("not a relocation section");
 
-            // If the relocation section comes *after* the target section, we
-            // don't add Config.AllocSectionsPrefix because we've already added
-            // the prefix to TargetSec->Name. Otherwise, if the relocation
-            // section comes *before* the target section, we add the prefix.
-            if (PrefixedSections.count(TargetSec)) {
-              Sec.Name = (prefix + TargetSec->Name).str();
-            } else {
-              const auto Iter = Config.SectionsToRename.find(TargetSec->Name);
-              if (Iter != Config.SectionsToRename.end()) {
-                // Both `--rename-section` and `--prefix-alloc-sections` are
-                // given but the target section is not yet renamed.
-                Sec.Name =
-                    (prefix + Config.AllocSectionsPrefix + Iter->second.NewName)
-                        .str();
-              } else {
-                Sec.Name =
-                    (prefix + Config.AllocSectionsPrefix + TargetSec->Name)
-                        .str();
-              }
-            }
-          }
+          if (PrefixedSections.count(TargetSec))
+          else
+                (prefix + Config.AllocSectionsPrefix + TargetSec->Name).str();
         }
       }
     }
   }
 
   if (!Config.SetSectionFlags.empty()) {
     for (auto &Sec : Obj.sections()) {
       const auto Iter = Config.SetSectionFlags.find(Sec.Name);