This is an archive of the discontinued LLVM Phabricator instance.

Avoid O_CLOEXEC to allow building on older Linux (RHEL5)
ClosedPublic

Authored by plopresti on May 27 2020, 10:26 AM.

Details

Summary

See https://github.com/google/sanitizers/issues/1253.

Small patch to enable compilation on (ancient) Red Hat Enterprise Linux 5.

Diff Detail

Event Timeline

plopresti created this revision.May 27 2020, 10:26 AM

Vitaly, please review (and land if ok).

We don't call fcntl() anywhere else in the sanitizer run-time.
Can anything go wrong if we do?

vitalybuka added inline comments.May 27 2020, 1:31 PM
compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp
355

I guess idea was to avoid leaking fd as results of data race.
We use this function from random places, so race is quite possible.
Probably this the leak is not very critical however I'd prefer to we use O_CLOEXEC where possible.
Could you update the patch to use fcntl only when O_CLOEXEC is not available?

plopresti updated this revision to Diff 266654.May 27 2020, 1:47 PM

Use O_CLOEXEC if it exists.

Tried to keep this clean and to avoid duplicating code on the #if - #else path. Let me know if you see a better way to do it...

vitalybuka accepted this revision.May 29 2020, 2:06 AM
This revision is now accepted and ready to land.May 29 2020, 2:06 AM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: Restricted Project. · View Herald TranscriptMay 29 2020, 2:09 AM
arichardson added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp
358

This is causing a build failure for me:

libRTSanitizerCommon.test.nolibc.x86_64.a(sanitizer_posix.cpp.o): In function `__sanitizer::GetNamedMappingFd(char const*, unsigned long, int*)':
/local/scratch/alr48/cheri/upstream-llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp:358: undefined reference to `fcntl'

Should there be an internal_fcntl wrapper? Or maybe a internal_set_cloexec() to avoid having to add a varargs syscall wrapper?