This is an archive of the discontinued LLVM Phabricator instance.

Differential D51538

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
Herald added subscribers: Restricted Project, krytarowski, kubamracek, srhines. · View Herald TranscriptAug 31 2018, 5:47 AM
srhines added a comment.Aug 31 2018, 10:36 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

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

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

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

I think this will work.

vitalybuka added inline comments.Aug 31 2018, 3:14 PM
lib/sanitizer_common/sanitizer_platform_limits_posix.cc
1044

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

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

Revision Contents

PathSize
lib/
sanitizer_common/
9 lines

Diff 163515

lib/sanitizer_common/sanitizer_platform_limits_posix.cc

Context not available.
CHECK_SIZE_AND_OFFSET(cmsghdr, cmsg_level); CHECK_SIZE_AND_OFFSET(cmsghdr, cmsg_level);
CHECK_SIZE_AND_OFFSET(cmsghdr, cmsg_type); CHECK_SIZE_AND_OFFSET(cmsghdr, cmsg_type);
#if SANITIZER_LINUX && (!defined(__ANDROID__) || __ANDROID_API__ >= 21) #ifndef __GLIBC_PREREQ
#define __GLIBC_PREREQ(x, y) 0
#endif
#if SANITIZER_LINUX && (!defined(__ANDROID__) || __ANDROID_API__ >= 21) && __GLIBC_PREREQ (2, 14)
srhinesUnsubmitted
Not Done
ReplyInline Actions

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.

srhines: I think this will break for Android, which doesn't define GLIBC_PREREQ. You need to put this…
vitalybukaUnsubmitted
Not Done
ReplyInline Actions

Please move __GLIBC_PREREQ back, under

#if SANITIZER_LINUX && (!defined(__ANDROID__) || __ANDROID_API__ >= 21)
vitalybuka: Please move __GLIBC_PREREQ back, under ``` #if SANITIZER_LINUX && (!defined(__ANDROID__) ||…
bcainAuthorUnsubmitted
Not Done
ReplyInline Actions

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
bcain: Oh, right, I think I understand. Does this make more sense? ``` #ifndef __GLIBC_PREREQ…
srhinesUnsubmitted
Not Done
ReplyInline Actions

I think this will work.

srhines: I think this will work.
vitalybukaUnsubmitted
Not Done
ReplyInline Actions

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))
vitalybuka: > Please move __GLIBC_PREREQ back, under Please ignore request above I think this should be…
srhinesUnsubmitted
Not Done
ReplyInline Actions

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

srhines: `defined(__ANDROID__)` is redundant in this case. You can just check for `__ANDROID_API__ >=…
CHECK_TYPE_SIZE(mmsghdr); CHECK_TYPE_SIZE(mmsghdr);
CHECK_SIZE_AND_OFFSET(mmsghdr, msg_hdr); CHECK_SIZE_AND_OFFSET(mmsghdr, msg_hdr);
CHECK_SIZE_AND_OFFSET(mmsghdr, msg_len); CHECK_SIZE_AND_OFFSET(mmsghdr, msg_len);
Context not available.
// Can't write checks for sa_handler and sa_sigaction due to them being // Can't write checks for sa_handler and sa_sigaction due to them being
// preprocessor macros. // preprocessor macros.
CHECK_STRUCT_SIZE_AND_OFFSET(sigaction, sa_mask); CHECK_STRUCT_SIZE_AND_OFFSET(sigaction, sa_mask);
#ifndef __GLIBC_PREREQ
#define __GLIBC_PREREQ(x, y) 0
#endif
#if !defined(__s390x__) || __GLIBC_PREREQ (2, 20) #if !defined(__s390x__) || __GLIBC_PREREQ (2, 20)
// On s390x glibc 2.19 and earlier sa_flags was unsigned long, and sa_resv // On s390x glibc 2.19 and earlier sa_flags was unsigned long, and sa_resv
// didn't exist. // didn't exist.
Context not available.