[PATCH] D12788: [elf2] Combine adjacent compatible OutputSections in PT_LOADs.

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 14 06:45:18 PDT 2015


git-clang-format the patch

+  ProgramHeader(uintX_t p_type, uintX_t p_flags) : Closed(false) {

Used

bool Closed = false;

   std::unique_ptr<llvm::FileOutputBuffer> Buffer;
+
   llvm::SpecificBumpPtrAllocator<OutputSection<ELFT>> CAlloc;

Why?

+  void setFromSection(OutputSectionBase<Is64Bits> &Sec) {

Set what from section?

+  // Create a PHDR for the first output section which covers the file header.
+  PHDRs.push_back(new (PAlloc) ProgramHeader<ELFT::Is64Bits>(
+      PT_LOAD, convertSectionFlagsToPHDRFlags(OutputSections[0]->getFlags())));

This means that the file and program headers are now executable. While
that is the common default for elf linkers, it is undesirable in
security sensitive contexts. For now, just keep creating the first
PT_LOAD with just PF_R. We can change that (possible with an option)
on another patch.

+    if (Sec->getSize()) {

All sh_alloc segments come before all others, so it might be
possible/cleaner to write this with

if (outputSectionHasPHDR<ELFT>(Sec)) {

And just close the last program header out of the loop, no?

I noticed that this patch doesn't change compSec. This means that we
might alternate from, for example, PF_R to PF_R|PF_W back to PF_R and
create more segments than necessary. Changing this on another patch is
preferable to changing it on this one. I just wanted to point it out.

Cheers,
Rafael


On 11 September 2015 at 20:22, Michael Spencer <bigcheesegs at gmail.com> wrote:
> Bigcheese updated this revision to Diff 34608.
> Bigcheese added a comment.
>
> Rebase and fix compile issues on non-msvc.
>
>
> http://reviews.llvm.org/D12788
>
> Files:
>   ELF/Writer.cpp
>   test/elf2/basic.s
>   test/elf2/basic32.s
>   test/elf2/basic32be.s
>   test/elf2/basic64be.s
>   test/elf2/shared.s
>


More information about the llvm-commits mailing list