[coreboot-gerrit] Change in coreboot[master]: util/cbfstool/rmodule: Omit undefined extern symbols from reloc table

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58376 )

Change subject: util/cbfstool/rmodule: Omit undefined extern symbols from reloc table
......................................................................

util/cbfstool/rmodule: Omit undefined extern symbols from reloc table

When using `DECLARE_OPTIONAL_REGION`, it is assumed that
REGION_SIZE(name) == 0 if the region was not defined in the memlayout.
When using non-rmodule stages (i.e., bootblock, romstage, etc), this
assumption holds true, but breaks down in rmodule (i.e., ramstage)
stages.

The rmodule tool is not currently omitting undefined externals from the
relocation table. e.g.,

extern u8 _##name##_size[];

This means that when the rmodule loader runs, it will rewrite the symbol
from 0 (which is the default the linker assumed) to 0 + offset. This is
wrong since the symbol doesn't actually exist. Instead we need to omit
the relocation so it continues to keep the default value of 0.

BUG=b:179699789
TEST=Print out REGION_SIZE(cbfs_cache) in ramstage and verify it is set
to 0.
I also see the following printed by the rmodtool now:

DEBUG: Omitting relocation for undefined extern: _watchdog_tombstone_size
DEBUG: Omitting relocation for undefined extern: _watchdog_tombstone
DEBUG: Omitting relocation for undefined extern: _watchdog_tombstone
DEBUG: Omitting relocation for absolute symbol: _stack_size
DEBUG: Omitting relocation for absolute symbol: _program_size
DEBUG: Omitting relocation for absolute symbol: _cbmem_init_hooks_size
DEBUG: Omitting relocation for absolute symbol: _payload_preload_cache_size
DEBUG: Omitting relocation for absolute symbol: _payload_preload_cache
DEBUG: Omitting relocation for absolute symbol: _payload_preload_cache_size
DEBUG: Omitting relocation for absolute symbol: _payload_preload_cache
DEBUG: Omitting relocation for undefined extern: _cbfs_cache
DEBUG: Omitting relocation for undefined extern: _cbfs_cache_size

As you can see the _watchdog_tombstone will also be fixed by this CL.

Signed-off-by: Raul E Rangel 
Change-Id: Ib57e263fa9014da4f6854637000c1c8ad8eb351a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58376
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
Reviewed-by: Karthik Ramasubramanian 
---
M util/cbfstool/rmodule.c
1 file changed, 16 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved
  Karthik Ramasubramanian: Looks good to me, but someone else must approve

diff --git a/util/cbfstool/rmodule.c b/util/cbfstool/rmodule.c
index ce00510..02f1d90 100644
--- a/util/cbfstool/rmodule.c
+++ b/util/cbfstool/rmodule.c
@@ -155,6 +155,19 @@
    return 0;
 }

+static int relocation_for_weak_extern_symbols(struct rmod_context *ctx, Elf64_Rela *r)
+{
+	Elf64_Sym *s = &ctx->pelf.syms[ELF64_R_SYM(r->r_info)];
+
+	if (ELF64_ST_BIND(s->st_info) == STB_WEAK && ELF64_ST_TYPE(s->st_info) == STT_NOTYPE) {
+		DEBUG("Omitting relocation for undefined extern: %s\n",
+		      &ctx->strtab[s->st_name]);
+		return 1;
+	}
+
+	return 0;
+}
+
 /*
  * Relocation processing loops.
  */
@@ -197,6 +210,9 @@
    		if (relocation_for_absolute_symbol(ctx, r))
    			continue;

+			if (relocation_for_weak_extern_symbols(ctx, r))
+				continue;
+
    		/* Allow the provided filter to have precedence. */
    		if (f != NULL) {
    			filter_emit = f->filter(f, r);