[PATCH] D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections.

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi added inline comments.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:381
+  case ELFT_ELF32LE:
+    return object::compress<object::ELF32LE>(Contents, Section.Align,
+                                             Config.CompressDebugSections);
----------------
plotfi wrote:
> jakehehrlich wrote:
> > This is a *huge* blocker. Avoid  wherever possible manually matching ElfType with ELFT types. It is just error prone and gross. You can solve this by moving this to the SectionWriter as I outlined in my proposal.
> With your proposal, as far as I could tell it would be too late for the section writer to back out of writing the section. Right now, the compression only happens when it is attempted and it is actually smaller than previously. Also, I've seem several places where ElfTypes are matched to ELFT types so I wasn't aware that this would be a huge blocker. 
> 
> 
> The problem here is I'm just not sure how you get this behavior without attempting the compression in HandleArgs. 
I'm also curious, if we don't want to use ElfType then why not just call CreateWriter and HandleArgs with a template parameter instead of passing in Reader.getElfType() from ExecuteElfObjcopyOnBinary?

The way I wrote ::compress here is not any  different than now ::CreateWriter is written.

I'm thinking of something like this:

  


```
                                                                               
std::unique_ptr<Writer> Writer;                                                  
if (isa<ELFObjectFile<ELF32LE>>(Bin)) {                                          
  HandleArgs<ELF32LE>(Config, *Obj, Reader);                                     
  Writer = CreateWriter<ELF32LE>(Config, *Obj, Out);                             
} else if (isa<ELFObjectFile<ELF64LE>>(Bin)) {                                   
  HandleArgs<ELF64LE>(Config, *Obj, Reader);                                     
  Writer = CreateWriter<ELF64LE>(Config, *Obj, Out);                             
} else if (isa<ELFObjectFile<ELF32BE>>(Bin)) {                                   
  HandleArgs<ELF32BE>(Config, *Obj, Reader);                                     
  Writer = CreateWriter<ELF32BE>(Config, *Obj, Out);                             
} else if (isa<ELFObjectFile<ELF64BE>>(Bin)) {                                   
  HandleArgs<ELF64BE>(Config, *Obj, Reader);                                                                                                                             
  Writer = CreateWriter<ELF64BE>(Config, *Obj, Out);                             
} else {                                                                         
  llvm_unreachable("Invalid ELFType");                                           
}                                                                                
      
```




Repository:
  rL LLVM

https://reviews.llvm.org/D49678