[PATCH] D26012: Don't create a dummy ELF to process a binary file

[Rafael Espíndola via llvm-commits]

On 26 October 2016 at 17:24, Rui Ueyama <ruiu at google.com> wrote:
> ruiu added inline comments.
>
>
> ================
> Comment at: ELF/Config.h:72
>  struct Configuration {
>    InputFile *FirstElf = nullptr;
> +  uint8_t OSABI = 0;
> ----------------
> Do we still need this `FirstELF`?


It is still used by, for example, isMipsR6. We should remove it at
some point and this is a step in the right direction, but it is not
there yet.

>
> ================
> Comment at: ELF/InputFiles.cpp:797-800
> +  if (!Config->FirstElf) {
> +    Config->FirstElf = Obj;
> +    Config->OSABI = Obj->getOSABI();
> +  }
> ----------------
> Do you think you can move the logic to inferMachineType in Driver.cpp?

Not directly. The problem is that we don't set the OSABI if -m is passed.

Ed, should "-m elf_amd64_fbsd" set OSABI in the same way it sets the
machine type?


> ================
> Comment at: ELF/InputSection.h:239-240
>  public:
>    InputSection(uintX_t Flags, uint32_t Type, uintX_t Addralign,
>                 ArrayRef<uint8_t> Data);
> +  InputSection(uintX_t Flags, uint32_t Type, uintX_t Addralign,
> ----------------
> If you add a new constructor, can you remove this instead?

Yes.

>
> ================
> Comment at: ELF/SymbolTable.h:77-78
>    Symbol *addRegular(StringRef Name, uint8_t Binding, uint8_t StOther);
> +  Symbol *addRegular(StringRef Name, InputSectionBase<ELFT> *Section,
> +                     uint8_t Binding, uint8_t Type, uintX_t Value);
>    Symbol *addSynthetic(StringRef N, OutputSectionBase<ELFT> *Section,
> ----------------
> Is there any way to reduce the number of overloaded functions here? I want to avoid having a bunch of functions that do basically the same thing.

I can inline the simpler ones.

Uploading a new patch in a sec.

Cheers,
Rafael