CHERIoT-Platform / llvm-project

Fork of LLVM adding CHERIoT, based on the CHERI LLVM fork
4 stars 6 forks source link

clang appears to emit calls to c++ memcpy from c code when processing __builtin_memcpy #31

Closed sleffler closed 4 days ago

sleffler commented 5 months ago

Opentitan again, building the boot rom which is c & assembler (no c++). I get undefined symbols; e.g.

ld.lld: error: undefined symbol: __library_export_libcalls__Z6memcpyPvPKvj
>>> referenced by flash_ctrl_testutils.c
>>>               flash_ctrl_testutils.o:(__library_import_libcalls__Z6memcpyPvPKvj) in archive bazel-out/k8-fastbuild/bin/external/lowrisc_opentitan/sw/device/lib/testing/libflash_ctrl_testutils.a

Tracked this down to structure copies and similar operations. One example workaround is this (might be non-opentitan code as we incorporate opentitan in a more complex environment):

-  const char kTarMagic[] = "ustar";
+//  const char kTarMagic[] = "ustar";
+  const char* kTarMagic = "ustar";

but there are many others (e.g. struct's passed by value as params). I also saw an explicit __builtin_memcpy converted to a call to the c++ memcpy.

This is a sample cmd line:

toolchains/cheri_llvm/wrappers/clang -MD -MF bazel-out/k8-fastbuild/bin/sw/device/lib/_objs/spi_flash/spi_flash.d '-frandom-seed=bazel-out/k8-fastbuild/bin/sw/device/lib/_objs/spi_flash/spi_flash.o' '-DBAZEL_CURRENT_REPOSITORY=""' -iquote . -iquote bazel-out/k8-fastbuild/bin -iquote external/lowrisc_opentitan -iquote bazel-out/k8-fastbuild/bin/external/lowrisc_opentitan -Ibazel-out/k8-fastbuild/bin/external/lowrisc_opentitan/hw/ip/flash_ctrl/data -Ibazel-out/k8-fastbuild/bin/external/lowrisc_opentitan/hw/top_earlgrey/ip/flash_ctrl/data/autogen -Ibazel-out/k8-fastbuild/bin/external/lowrisc_opentitan/hw/ip/uart/data -Ibazel-out/k8-fastbuild/bin/external/lowrisc_opentitan/hw/ip/otp_ctrl/data -Ibazel-out/k8-fastbuild/bin/external/lowrisc_opentitan/hw/ip/spi_host/data -nostdinc -isystemexternal/cheriot-llvm/lib/clang/13.0.0/include -isystemexternal/cheriot-llvm/riscv32-unknown-elf/include '-march=rv32imcxcheri' '-mcpu=cheriot' '-mabi=cheriot' '-mcmodel=medany' -mlittle-endian '--target=riscv32-unknown-elf' -mxcheri-rvc -mrelax '-DCHERI=1' '-Werror=date-time' -ffunction-sections -fdata-sections -Os -g -ffreestanding -fno-common '' '-std=gnu11' -S sw/device/lib/spi_flash.c -o spi_flash.s

Tried adding -fno-built-memcpy but it seemed to have no effect. Maybe I'm missing a compiler flag?

davidchisnall commented 5 months ago

This is not a C++ memcpy, this is a memcpy with the CHERIoT libcall calling convention. We mangle all libcall calling convention functions to avoid problems where people accidentally miss type signature changes.

Freestanding C implementations are required to provide memcpy (and memcmp) and so LLVM does not have a mode to disable requiring them. For bare metal code, the best thing to do is have an early init phase set provide the sentry for the memcpy function in the compiler-generated export table entry.

sleffler commented 5 months ago

On Mon, Jun 24, 2024 at 12:08 AM David Chisnall @.***> wrote:

This is not a C++ memcpy, this is a memcpy with the CHERIoT libcall calling convention. We mangle all libcall calling convention functions to avoid problems where people accidentally miss type signature changes.

Ok. Don't recall seeing this documented anywhere.

Freestanding C implementations are required to provide memcpy (and memcmp)

and so LLVM does not have a mode to disable requiring them. For bare metal code, the best thing to do is have an early init phase set provide the sentry for the memcpy function in the compiler-generated export table entry.

Now that I understand why there's a mangled symbol, sdk/lib/freestanding https://github.com/microsoft/cheriot-rtos/tree/main/sdk/lib/freestanding makes more sense; esp the README. TBH I expected this to be part of a toolchain, but this gives me something to work with, thanks.

davidchisnall commented 5 months ago

Ok. Don't recall seeing this documented anywhere.

It's in §5.3 of the CHERIoT Architecture doc (Chapter 5 is the ABI description). It's not documented anywhere else because the only people who should care about it are folks implementing compilers or loaders.

TBH I expected this to be part of a toolchain

For a system where the ISA, programmer model, ABI, and RTOS are co-designed, it's not always clear where the best home for something is. We currently don't provide any headers with the compiler and expect them to come from the RTOS in all cases.

If you need to run code using the CHERIoT ABI from ROM, then you probably need to run a loader-style step early on that embeds the capabilities in the code for the ROM. We haven't done that because it breaks the strong provenance chain for all capabilities in the system are derived from the ones provided at boot (which lets us to layered boot things where each step in the chain may be arbitrarily restricted by a prior step).

sleffler commented 5 months ago

On Mon, Jun 24, 2024 at 9:07 AM David Chisnall @.***> wrote:

Ok. Don't recall seeing this documented anywhere.

It's in §5.3 of the CHERIoT Architecture doc (Chapter 5 is the ABI description). It's not documented anywhere else because the only people who should care about it are folks implementing compilers or loaders.

So it is, thank you (I tend to read the code since docs frequently get out of date/sync).

TBH I expected this to be part of a toolchain

For a system where the ISA, programmer model, ABI, and RTOS are co-designed, it's not always clear where the best home for something is. We currently don't provide any headers with the compiler and expect them to come from the RTOS in all cases.

I'm essentially programming against a different RTOS. The problem appears to be a mismatch in expectations as OpenTitan assumes the toolchain provides includes+libs for baremetal operation.

If you need to run code using the CHERIoT ABI from ROM, then you probably

need to run a loader-style step early on that embeds the capabilities in the code for the ROM. We haven't done that because it breaks the strong provenance chain for all capabilities in the system are derived from the ones provided at boot (which lets us to layered boot things where each step in the chain may be arbitrarily restricted by a prior step).

I'm not disagreeing about provenance; this was meant to be a quick path to booting the cheriot loader that preserved OT's functionality (live & learn). We've discussed restructuring the boot process in a follow-on effort.

resistor commented 5 days ago

Seems like this is resolved? Can we close it?

davidchisnall commented 4 days ago

I believe this is fixed now by the baremetal ABI.