libmd: Only define SHA256_Transform_c when using the ARM64 ifunc.

[jhb]

Summary

GCC 9 doesn't define a SHA256_Transform symbol when the stub just wraps
SHA256_Transform_c resulting in an undefined symbol for
_libmd_SHA256_Transform in libmd.so.

[jhb]

This is definitely a workaround, and I don't fully understand why the tail call messes up this way when using GCC.

[jrtc27]

I don't think I understand the problem. SHA256_Transform is static, it should never be visible outside the object.

[jhb]

sys/crypto/sha2/sha256c.c

397

This reference is not static however.

[jrtc27]

sys/crypto/sha2/sha256c.c

397

Oh I see. That's still a bit fragile, even if it happens to work. The correct way to fix that is to mark any static symbols referenced by inline assembly via strings as __used, which will ensure it gets emitted.

397

(Though this refactoring is perhaps *also* a better way to write the original code, not sure, it's shorter, though requires two places to know about the ifdef).

[jhb]

sys/crypto/sha2/sha256c.c

397

So is __used the better long term fix? I could maybe have a wrapper macro that only maps to __used when WEAK_REFS is defined (WEAK_REFS_USED or some such) and then tag all the static functions with weak references with that?

[jrtc27]

Was reminded of this revision as Adrian reported this exact symptom on IRC

sys/crypto/sha2/sha256c.c

397

__used is both necessary and sufficient. Whether or not you prefer the other refactoring is a separate issue.

[kevans]

clang has also seems to exhibit the same behavior at -O1 or -O2 WITH_ASAN, as reported by @koobs.

[koobs]

Thanks for tagging @kevans. The *SAN link failures and environment/build configuration are here: https://lists.freebsd.org/archives/freebsd-current/2021-September/000488.html

[andrew]

Was there a conclusion to this? I have a similar sha512 implementation so would like to include the fix there.

[jhb]

I haven't gone back and tried the __used variant, but I could just land this perhaps. I think I do slightly prefer this version with not having the _c variant for !ARM64.

[jrtc27]

I think that's ok. It's still technically wrong without __used, but it was like that before too, and in practice non-static functions like SHA256_Update can't inline the ifunc (and wouldn't want to inline the non-ifunc on non-arm64 due to its size and being called multiple times) so that symbol's unlikely to disappear in the assembly, even if there should be a __used on it (and any others that are static and used for __weak_reference, though I vaguely recall Transform being the only one).

[jrtc27]

For new code though please use __used from the start for any static function referenced by __weak_reference and then write the code however you like

[emaste]

I just encountered this again while trying to fix WITH_ASAN and WITH_UBSAN.
https://cirrus-ci.com/task/6650127776481280

--- all_subdir_lib ---
--- all_subdir_lib/libc ---
ld: error: /usr/obj/tmp/cirrus-ci-build/amd64.amd64/tmp/usr/lib/libmd.so: undefined reference to _libmd_SHA256_Transform [--no-allow-shlib-undefined]
cc: error: linker command failed with exit code 1 (use -v to see invocation)
*** [h_hash.full] Error code 1

(See D32805 and PR260099 for other ASAN/UBSAN changes needed.)