Page MenuHomePhabricator

Add glibc_prereq to platform limits mmsghdr
AcceptedPublic

Authored by bcain on Aug 31 2018, 5:47 AM.

Details

Reviewers
vitalybuka
hans
Summary

sendmmsg requires glibc >= 2.14.

Fixes PR38589.

Diff Detail

Event Timeline

bcain created this revision.Aug 31 2018, 5:47 AM

Please upload patches with context, as it makes it easier to review. https://llvm.org/docs/Phabricator.html has instructions on how to do this (search for "context").

lib/sanitizer_common/sanitizer_platform_limits_posix.cc
1044 ↗(On Diff #163515)

I think this will break for Android, which doesn't define GLIBC_PREREQ. You need to put this check in with the !defined(__ANDROID__) part.

vitalybuka added inline comments.Aug 31 2018, 11:11 AM
lib/sanitizer_common/sanitizer_platform_limits_posix.cc
1044 ↗(On Diff #163515)

Please move __GLIBC_PREREQ back, under

#if SANITIZER_LINUX && (!defined(__ANDROID__) || __ANDROID_API__ >= 21)
bcain added inline comments.Aug 31 2018, 2:54 PM
lib/sanitizer_common/sanitizer_platform_limits_posix.cc
1044 ↗(On Diff #163515)

Oh, right, I think I understand. Does this make more sense?

#ifndef __GLIBC_PREREQ
#define __GLIBC_PREREQ(x, y) 0
#endif
 
#if SANITIZER_LINUX && (!defined(__ANDROID__) || __ANDROID_API__ >= 21) && (!defined(__GLIBC__) ||  __GLIBC_PREREQ (2, 14))
CHECK_TYPE_SIZE(mmsghdr);
CHECK_SIZE_AND_OFFSET(mmsghdr, msg_hdr);
CHECK_SIZE_AND_OFFSET(mmsghdr, msg_len);
#endif
srhines added inline comments.Aug 31 2018, 3:04 PM
lib/sanitizer_common/sanitizer_platform_limits_posix.cc
1044 ↗(On Diff #163515)

I think this will work.

vitalybuka added inline comments.Aug 31 2018, 3:14 PM
lib/sanitizer_common/sanitizer_platform_limits_posix.cc
1044 ↗(On Diff #163515)

Please move __GLIBC_PREREQ back, under

Please ignore request above

I think this should be

#if SANITIZER_LINUX && (__GLIBC_PREREQ (2, 14) || (defined(__ANDROID__) && __ANDROID_API__ >= 21))
srhines added inline comments.Aug 31 2018, 3:23 PM
lib/sanitizer_common/sanitizer_platform_limits_posix.cc
1044 ↗(On Diff #163515)

defined(__ANDROID__) is redundant in this case. You can just check for __ANDROID_API__ >= 21 now.

hans added a comment.Sep 4 2018, 2:20 AM

What's the status here? Will it be ready for the 7.0.0 release?

bcain updated this revision to Diff 163824.Sep 4 2018, 8:10 AM

Updated per review, test-release.sh build completes w/this patch on sles11.3.

vitalybuka accepted this revision.Sep 4 2018, 4:27 PM
This revision is now accepted and ready to land.Sep 4 2018, 4:27 PM