fix(romlib): wrap indirectly included functions
The problem that this resolves is a bit involved; the following
must be met at the same time for some function <to_be_wrapped>:
* to_be_wrapped must be specified as part of the romlib
* to_be_wrapped must _not_ be referenced by any translation unit
in TF-A
* to_be_wrapped must be referenced by a translation unit in a
dependent library, mbedtls for example.
Under these circumstances, to_be_wrapped will not be wrapped, and
will instead reference its original definition while simultaneously
residing in romlib.
This is a side effect of two issues with romlib prior to this patch:
1 to_be_wrapped is expected to wrap by duplicating its definition.
This causes any condition that links against both the base and
wrapper functions to be a link error (duplicate symbol definition).
2 to_be_wrapped is in its own translation unit
This causes the wrappers to be used by TF-A in an as needed.
The duplicate function definitions can be worked around using the
linker's `--wrap` flag, which redirects all references to a symbol
to resolve to `__wrap_<symbol>` and the original symbol to be
available as `__real_<symbol>`. Most of the changes handle creating
this arguments and passing them to the linker.
Further, once you use the linker's wrap, you will encounter another
issue: if TF-A does not use a function, its wrapper is not present.
This causes link issues when a library and not TF-A uses the wrapper.
Note that this issue would have been resolved previously by ignoring
the wrapper and using the base definition.
This further issue is worked around by concatenating the assembly for
all of the wrappers into a single translation unit. It's possible to
work around this issue in a few other ways, including reordering the
libraries passed to the linker to place libwrapper.a last or grouping
the libraries so that symbols from later libraries may be resolved
with prior libraries.
I chose the translation unit concatenation approach as it revealed
that a jumptable has duplicate symbols within it.
Change-Id: Ie57b5ae69bde2fc8705bdc7a93fae3ddb5341ed9
Signed-off-by: Jimmy Brisson <jimmy.brisson@arm.com>
diff --git a/lib/romlib/Makefile b/lib/romlib/Makefile
index 9ade331..8c8a110 100644
--- a/lib/romlib/Makefile
+++ b/lib/romlib/Makefile
@@ -44,7 +44,7 @@
.PHONY: all clean distclean
-all: $(BUILD_DIR)/romlib.bin $(LIB_DIR)/libwrappers.a
+all: $(BUILD_DIR)/romlib.bin $(BUILD_DIR)/romlib.ldflags $(LIB_DIR)/libwrappers.a
%.o: %.s
$(s)echo " AS $@"
@@ -88,6 +88,10 @@
$(s)echo " TBL $@"
$(q)$(ROMLIB_GEN) gentbl --output $@ --bti=$(ENABLE_BTI) $<
+$(BUILD_DIR)/romlib.ldflags: ../../$(PLAT_DIR)/jmptbl.i
+ $(s)echo " LDFLAGS $@"
+ $(q)$(ROMLIB_GEN) link-flags $< > $@
+
clean:
$(q)rm -f $(BUILD_DIR)/*
diff --git a/lib/romlib/romlib_generator.py b/lib/romlib/romlib_generator.py
index 0682dd4..8d2e88d 100755
--- a/lib/romlib/romlib_generator.py
+++ b/lib/romlib/romlib_generator.py
@@ -182,6 +182,22 @@
template_name = "jmptbl_entry_" + item["type"] + bti + ".S"
output_file.write(self.build_template(template_name, item, True))
+class LinkArgs(RomlibApplication):
+ """ Generates the link arguments to wrap functions. """
+
+ def __init__(self, prog):
+ RomlibApplication.__init__(self, prog)
+ self.args.add_argument("file", help="Input file")
+
+ def main(self):
+ index_file_parser = IndexFileParser()
+ index_file_parser.parse(self.config.file)
+
+ fns = [item["function_name"] for item in index_file_parser.items
+ if not item["patch"] and item["type"] != "reserved"]
+
+ print(" ".join("-Wl,--wrap " + f for f in fns))
+
class WrapperGenerator(RomlibApplication):
"""
Generates a wrapper function for each entry in the index file except for the ones that contain
@@ -214,21 +230,19 @@
if item["type"] == "reserved" or item["patch"]:
continue
- asm = self.config.b + "/" + item["function_name"] + ".s"
- if self.config.list:
- # Only listing files
- files.append(asm)
- else:
- with open(asm, "w") as asm_file:
- # The jump instruction is 4 bytes but BTI requires and extra instruction so
- # this makes it 8 bytes per entry.
- function_offset = item_index * (8 if self.config.bti else 4)
+ if not self.config.list:
+ # The jump instruction is 4 bytes but BTI requires and extra instruction so
+ # this makes it 8 bytes per entry.
+ function_offset = item_index * (8 if self.config.bti else 4)
- item["function_offset"] = function_offset
- asm_file.write(self.build_template("wrapper" + bti + ".S", item))
+ item["function_offset"] = function_offset
+ files.append(self.build_template("wrapper" + bti + ".S", item))
if self.config.list:
- print(" ".join(files))
+ print(self.config.b + "/wrappers.s")
+ else:
+ with open(self.config.b + "/wrappers.s", "w") as asm_file:
+ asm_file.write("\n".join(files))
class VariableGenerator(RomlibApplication):
""" Generates the jump table global variable with the absolute address in ROM. """
@@ -258,7 +272,8 @@
if __name__ == "__main__":
APPS = {"genvar": VariableGenerator, "pre": IndexPreprocessor,
- "gentbl": TableGenerator, "genwrappers": WrapperGenerator}
+ "gentbl": TableGenerator, "genwrappers": WrapperGenerator,
+ "link-flags": LinkArgs}
if len(sys.argv) < 2 or sys.argv[1] not in APPS:
print("usage: romlib_generator.py [%s] [args]" % "|".join(APPS.keys()), file=sys.stderr)
diff --git a/lib/romlib/templates/wrapper.S b/lib/romlib/templates/wrapper.S
index 734a68a..576474a 100644
--- a/lib/romlib/templates/wrapper.S
+++ b/lib/romlib/templates/wrapper.S
@@ -3,8 +3,9 @@
*
* SPDX-License-Identifier: BSD-3-Clause
*/
- .globl ${function_name}
-${function_name}:
+ .section .text.__wrap_${function_name}
+ .globl __wrap_${function_name}
+__wrap_${function_name}:
ldr x17, =jmptbl
mov x16, #${function_offset}
ldr x17, [x17]
diff --git a/lib/romlib/templates/wrapper_bti.S b/lib/romlib/templates/wrapper_bti.S
index ba9b11c..0dc316c 100644
--- a/lib/romlib/templates/wrapper_bti.S
+++ b/lib/romlib/templates/wrapper_bti.S
@@ -3,8 +3,9 @@
*
* SPDX-License-Identifier: BSD-3-Clause
*/
- .globl ${function_name}
-${function_name}:
+ .section .text.__wrap_${function_name}
+ .globl __wrap_${function_name}
+__wrap_${function_name}:
bti jc
ldr x17, =jmptbl
mov x16, #${function_offset}