[PATCH] ld: ensure build-id is placed near ELF headers

[Nick Clifton]

Hi Andrew,

   [Sorry for the delay in replying - I have been on PTO...]

> Second, I've read the link that Fangrui Song posted[1], which is yet
> another voice arguing for r/o content to be placed before the text
> content.  However, the initial suggestion is that this would be opt in,
> which would still leave GDB's build-id lookup broken by default when
> --rosegment was used, which is less than ideal.

Just as a matter of interest, have you checked lld's current behaviour ?
(Ideally with and without --rosegment and/or -z separate-code).

> Third, I've read up on bug PR ld/30907[2] which seems to be the
> motivator for the addition of --rosegment.  The initial complaint
> appears to be about file size rather than the number of segments.  It's
> just that reducing the number of segments helps reduce the file size.

Right.

> The description of --rosegment does mention creating a single r/o
> segment, which is fine, but, I think, we should keep the original goal
> (file size) in mind.

I totally agree.

> Now, when looking at this, I did wonder, is that initial alignment still
> needed?  I guess that it made sense when we have actual data content
> before the text content.  But if all we have before the text is the ELF
> headers, and if (apparently) we are OK having those headers mapped into
> the r/x segment, then surely we could drop the alignment?

That makes sense.


> The patch below is an RFC rather than a full patch, I've not update the
> commit message to describe the changes to the alignment code yet, but I
> wanted to share this so we could discuss it.

I think that the patch looks OK, apart from one possible typo...

> @@ -800,9 +804,12 @@ EOF
>     if test -z "${ALL_TEXT_BEFORE_RO}"; then
>       test -n "${SEPARATE_CODE}" || emit_early_ro
>       test -n "${NON_ALLOC_DYN}${SEPARATE_CODE}" || emit_dyn
> +
> +    if test -n "${SEPARATE_CODE}" -o -n "${NON_ALLOC_DYN}${SEPARATE_CODE}"; then
> +      align_text
> +    fi
>     fi
> -
> -  align_text
> +
>     emit_text
>   

The intent of this change is to only invoke the align_text operation if some
data has been emitted before the start of the emit_text operation, yes ?

But emit_early_ro and emit_dyn are only invoked if the corresponding test
*fails*.  So for example:

   test -n "${SEPARATE_CODE}" || emit_early_ro

translates to:

   if ${SEPARATE_CODE} is not empty do nothing, otherwise invoke emit_early_ro

So the test to invoke align_text ought to be:

    if test -z "${SEPARATE_CODE}" -o -z "${NON_ALLOC_DYN}${SEPARATE_CODE}"; then
      align_text
    fi

ie align_text is called only if either of the previous tests encountered
empty strings.

Does this sound right to you ?  (I confess that I find the bash shell's conditional
syntax very confusing.  I have been wondering if there is an easier way to code these
operations).

Cheers
   Nick