[coreboot-gerrit] [S] Change in coreboot[main]: cbfstool: Skip relocation entries pointing to undefined symbol

[Jérémy Compostella (Code Review)]

Attention is currently required from: Julius Werner.

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/77699?usp=email

to look at the new patch set (#32).

Change subject: cbfstool: Skip relocation entries pointing to undefined symbol
......................................................................

cbfstool: Skip relocation entries pointing to undefined symbol

The linker can make relocation entries of a symbol which has a value
of zero point to the undefined symbol entry.  It is permitted since
when the symbol value is zero as the documentation of the relocation
entry `r_info' field states:

"If the index is STN_UNDEF, the undefined symbol index, the relocation
 uses 0 as the symbol value."

The ELF binary does not really have any missing symbols.  It is an
optimization as the symbol points to the undefined symbol because its
value is zero.

A typical way to hit this cbfstool limitation is to define an empty
region using the REGION macro in the linker script.  Here is an
example if we assume `CONFIG_MY_REGION' is set to 0:

.car.data {
            [...]
        REGION(my_region, CONFIG_MY_REGION_SIZE)
        [...]
    }

A region is defined as follow:

#define REGION_SIZE(name) ((size_t)_##name##_size)

#define DECLARE_REGION(name)	\
            extern u8 _##name[];	\
            extern u8 _e##name[];	\
            extern u8 _##name##_size[];

So the size of the region is actually the address of the
`_##name##_size' symbol.  Therefore, the `_my_region_size' symbol
address is zero and the linker can make the relocation entry of this
symbol point to the undefined symbol index.

In such a situation, cbfstool hits a segmentation fault when it
attempts to relocate the symbol in `parse_elf_to_xip_stage()'
function.  We resolves this issue by making cbfstool skips relocation
entries pointing to the undefined symbol similarly to the way it skips
relocation relative to absolute symbols.  A symbol which value is zero
can be considered an absolute symbol and therefore should not be
relocated.

Of course, we could argue that we could just prevent the declaration
of an empty region as illustrated in the following example:

.car.data {
            [...]
        #if CONFIG_MY_REGION_SIZE > 0
            REGION(my_region, CONFIG_MY_REGION_SIZE)
        #endif
        [...]
    }

However, this is not a satisfying solution because:

1. It requires to add unnecessary code in the linker script as an empty
   region is a valid declaration.  Such a workaround requires the code
   using it to mark the region symbols as weak symbols to handle the
   situation where the region is not defined.

2. There could be other situations which have yet to be uncovered which
   would lead the same cbfstool crash.

3. A binary with an empty region is a valid ELF file and cbfstool
   should not crash when it is asked to create an eXecute-In-Place stage
   out of it.

Change-Id: I2803fd3e96e7ff7a0b22d72d50bfbce7acaeb941
Signed-off-by: Jeremy Compostella 
---
M util/cbfstool/rmodule.c
1 file changed, 16 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/77699/32