This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Fix build of Sanitizer-Test_Nolibc after D80648
ClosedPublic

Authored by arichardson on Aug 3 2020, 2:28 AM.

Details

Summary

Running ninja check-sanitizer fails for after that patch (commit
058f5f6fd813d1ee1480497394d6fd44e65ec62b) with the following error:

libRTSanitizerCommon.test.nolibc.x86_64.a(sanitizer_posix.cpp.o): In
function `__sanitizer::GetNamedMappingFd(char const*, unsigned long,
int*)':
..../llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp:358:
undefined reference to `fcntl'
clang-12: error: linker command failed with exit code 1 (use -v to see
invocation)

This patch works around the problem by only calling fcntl if O_CLOEXEC
is not defined.

Diff Detail

Event Timeline

arichardson created this revision.Aug 3 2020, 2:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2020, 2:28 AM
Herald added a subscriber: dberris. · View Herald Transcript
arichardson requested review of this revision.Aug 3 2020, 2:28 AM

This is only a workaround and a better solution would probably be an internal_fcntl() wrapper? Or maybe internal_set_cloexec() to avoid having to add a varargs syscall wrapper? However, I don't have time to work on that so I'd like to get the minimal compilation fix committed first since it's breaking ninja check-all for me.

plopresti accepted this revision.Aug 3 2020, 9:06 AM

This is pretty similar to my original patch, actually... But I ended up trying to minimize #ifdefs.

I do have a question, though. fcntl() and FD_CLOEXEC have been mandated by POSIX since the beginning of time (https://pubs.opengroup.org/onlinepubs/009695399/functions/fcntl.html). So how could it be an undefined symbol?

This revision is now accepted and ready to land.Aug 3 2020, 9:06 AM

This is pretty similar to my original patch, actually... But I ended up trying to minimize #ifdefs.

I do have a question, though. fcntl() and FD_CLOEXEC have been mandated by POSIX since the beginning of time (https://pubs.opengroup.org/onlinepubs/009695399/functions/fcntl.html). So how could it be an undefined symbol?

The nolibc version of the tests builds a sanitizer runtime that does not link against libc and reimplements the C system call wrappers inside the runtime (e.g. internal_open()/internal_unlink()), so unless fcntl is also provided by the compiler-rt runtime it will be undefined. The error only happens when building the sanitizer_nolibc_test_main test since it explicitly compiles with -nostdlib (all other tests end up using fcntl from libc).

Got it. Was never expecting to run in a non-POSIX-compliant environment.

internal_fcntl() seems like a lot of effort for this one-off need. Thanks
for the fix.