[PATCH] D40029: [ELF] Only add contiguous isRelro sections to PT_GNU_RELRO PHDR

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I'm going to split this patch up into 3 at the suggestion of this message from [llvm-commits].

> I think I am fine with the overall direction of this change.
> 
> My main request is to split the patch in 2 or even better 3:
> 
> - Give an error if relro is not contiguous.
> - Relax handling of empty sections.
> - Name change when linker scripts are used.
> 
>> +  bool InRelroPhdr = false;
>>  +  bool IsRelroFinished = false;
>>  +  for (OutputSection *Sec : OutputSections) {
>>  +    if (!needsPtLoad(Sec))
>>  +      continue;
>>  +    if (isRelroSection(Sec)) {
>>  +      InRelroPhdr = true;
>>  +      if (!IsRelroFinished)
>>  +        RelRo->add(Sec);
>>  +      else if (Sec->Size)
>>  +        error("section: " + Sec->Name + " is not contiguous with other relro" +
>>  +              " sections");
>>  +    } else if (InRelroPhdr && Sec->Size) {
>>  +      InRelroPhdr = false;
>>  +      IsRelroFinished = true;
>>  +    }
>>  +  }
> 
> With this an empty relro section can still start the PT_GNU_RELRO. Is
> that intentional?
> 
> Cheers,
> Rafael

The first patch to output the error message is available at https://reviews.llvm.org/D40359. I've noticed that the size of the .dynamic section hasn't been set at the time createPhdrs() is called so I may need to think about the relaxation of the 0 size sections for a bit.


https://reviews.llvm.org/D40029