Link in arm_none_eabi_gcc_support when building with Arm GCC [300471936]

Fixed

Internal Cleanup

Status Update

No update yet.

Description

tp...@google.com

created issue #1

Sep 14, 2023 04:58PM

When building Pigweed targets with our Arm GCC toolchain, you'll get warnings at linktime like,

INFO: From Linking pw_sync/mutex_facade_test:
/usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/bin/ld: /usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/lib/thumb/v7e-m+fp/hard/libc_nano.a(libc_a-closer.o): in function `_close_r':
closer.c:(.text._close_r+0xc): warning: _close is not implemented and will always fail
/usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/bin/ld: /usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/lib/thumb/v7e-m+fp/hard/libc_nano.a(libc_a-signalr.o): in function `_getpid_r':
signalr.c:(.text._getpid_r+0x0): warning: _getpid is not implemented and will always fail
/usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/bin/ld: /usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/lib/thumb/v7e-m+fp/hard/libc_nano.a(libc_a-signalr.o): in function `_kill_r':
signalr.c:(.text._kill_r+0xe): warning: _kill is not implemented and will always fail
/usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/bin/ld: /usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/lib/thumb/v7e-m+fp/hard/libc_nano.a(libc_a-lseekr.o): in function `_lseek_r':
lseekr.c:(.text._lseek_r+0x10): warning: _lseek is not implemented and will always fail
/usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/bin/ld: /usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/lib/thumb/v7e-m+fp/hard/libc_nano.a(libc_a-readr.o): in function `_read_r':
readr.c:(.text._read_r+0x10): warning: _read is not implemented and will always fail
/usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/bin/ld: /usr/local/google/home/tpudlik/.cache/bazel/_bazel_tpudlik/3002936d6b4062744699e89d7594ac11/external/gcc_arm_none_eabi_toolchain/bin/../lib/gcc/arm-none-eabi/12.2.1/../../../../arm-none-eabi/lib/thumb/v7e-m+fp/hard/libc_nano.a(libc_a-writer.o): in function `_write_r':
writer.c:(.text._write_r+0x10): warning: _write is not implemented and will always fail

The wrappers that will silence these warnings were implemented in https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/112311. They need to be added to the dependencies of any binary built with this toolchain.

In GN we do this through pw_build_LINK_DEPS, and in Bazel we should probably use the analogous link_extra_lib. But that's a new Bazel feature, so we need to upgrade to a Bazel 7 prerelease first (b/299671945).

Comments

tp...@google.com <tp...@google.com> #2Jan 26, 2024 10:54PM

I took a stab at using these wrappers, at first in a very minimal way, just adding them as a dependency to the Pigweed echo application at https://pigweed.googlesource.com/example/echo. I get a linker error:

/proc/self/cwd/external/pigweed/pw_toolchain/arm_gcc/newlib_os_interface_stubs.cc:90: undefined reference to `pw::toolchain::AttempedToInvokeUnsupportedNewlibOsInterfaceFunction()'

Full log.

At first glance, this looks like a bad interaction between the clever pw::toolchain::AttempedToInvokeUnsupportedNewlibOsInterfaceFunction (trying to link in kill_, for example, will produce this linker error) and the alwayslink option on the cc_library (added in http://pwrev.dev/180930, it causes the library to be linked with the --whole-archive option).

It turned out that my mistake was adding a dependency directly on the newlib_os_interface_stubs. When I switched the dependency to //pw_toolchain/arm_gcc:arm_none_eabi_gcc_support, everything compiled nicely.

ap...@google.com <ap...@google.com> #3Jan 26, 2024 11:01PM

Project: example/echo
Branch: main

commit 12839e4f6c07e9102c12d18116cfd6b5e8ae0b42
Author: Ted Pudlik <tpudlik@google.com>
Date: Fri Jan 26 22:58:51 2024

Add arm_none_eabi_gcc_support dependency

This silences some build warnings.

Test: Flashed and tested the application.

Bug: 300471936
Change-Id: I3025137aad3a9a55aefcec7fa9b9361f51e58a16
Reviewed-on:

https://pigweed-review.googlesource.com/c/example/echo/+/189411
Reviewed-by: Armando Montanez <amontanez@google.com>
Pigweed-Auto-Submit: Ted Pudlik <tpudlik@google.com>
Presubmit-Verified: CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed-service-accounts.iam.gserviceaccount.com>

M src/BUILD.bazel

https://pigweed-review.git.corp.google.com/189411

tp...@google.com <tp...@google.com> #4Jan 26, 2024 11:09PM

How to best close out this issue?

Ideally the Arm GCC toolchain would add a dependency on //pw_toolchain/arm_gcc:arm_none_eabi_gcc_support to any binary built with it. But I don't think this can be expressed in Bazel.

The trouble with extra_link_libs is that I don't know how to add the target to them if and only if the build is being done using this specific toolchain. We could add the target to //pw_build:default_link_extra_lib behind a select for @platforms//cpu:armv7e-m, but that's not quite right: there are a bunch of other Arm CPUs, but you might not be using Arm GCC when targeting them. So at the upstream Pigweed level this seems pretty brittle.

Maybe the best approach is to just document that if you're using our Arm GCC toolchain, you should depend on //pw_toolchain/arm_gcc:arm_none_eabi_gcc_support. We almost do (https://pigweed.dev/pw_toolchain/#arm-none-eabi-gcc-support), we just need to mention Bazel explicitly. Maybe that's all we need.

ap...@google.com <ap...@google.com> #5Jan 29, 2024 11:20PM

Project: pigweed/pigweed
Branch: main

commit 91ae9b642bfce2382970d4a97674f50479e2e2e9
Author: Ted Pudlik <tpudlik@google.com>
Date: Mon Jan 29 23:18:10 2024

docs: Update Bazel quickstart output sample

The linker warnings were removed by

http://pwrev.dev/189411.

Bug: 300471936
Change-Id: Ie1025bb6b3c32de769d5784a41cf8e7d7acde733
Reviewed-on:

https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/189412
Commit-Queue: Kayce Basques <kayce@google.com>
Reviewed-by: Kayce Basques <kayce@google.com>
Presubmit-Verified: CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com>

M docs/get_started/bazel.rst

https://pigweed-review.git.corp.google.com/189412

ap...@google.com <ap...@google.com> #6Jan 30, 2024 01:05AM

Marked as fixed.

Project: pigweed/pigweed
Branch: main

commit 0316a4d4ae164d4aadc9c43533a9ba484abfad2c
Author: Ted Pudlik <tpudlik@google.com>
Date: Tue Jan 30 01:03:57 2024

pw_toolchain: Small docs update

Provide some guidance on how to depend on arm_none_eabi_gcc_support when
building with Bazel.

Fixed:

b/300471936

Change-Id: I4864ab83962e2d199ee21d7b725fe00b4961df0e
Reviewed-on:

https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/189278
Commit-Queue: Ted Pudlik <tpudlik@google.com>
Reviewed-by: Kayce Basques <kayce@google.com>

M pw_toolchain/docs.rst

https://pigweed-review.git.corp.google.com/189278

Issue 300471936