This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Revert {send,sendm,recv,recvm}msg conformance changes



On 10/06/2016 11:13, Carlos O'Donell wrote:
> On 06/09/2016 05:09 PM, Adhemerval Zanella wrote:
>> After some discussion in libc-alpha about this POSIX compliance fix, I see
>> that GLIBC should indeed revert back to previous definition of msghdr and
>> cmsghdr and implementation of sendmsg, recvmsg, sendmmsg, recvmmsg due some
>> reasons:
>>
>>  * The possible issue where the syscalls wrapper add the compatibility
>>    layer is quite limited in scope and range.  And kernel current
>>    also add some limits to the values on the internal msghdr and
>>    cmsghdr fields:
>>
>>      - msghdr::msg_iovlen larger than UIO_MAXIOV (1024) returns
>>        EMSGSIZE.
>>      - msghdr::msg_controllen larger than INT_MAX returns ENOBUFS.
>>
>>  * There is a small performance hit for recvmsg/sendmsg/recmmsg which
>>    is neglectable, but it is a big hit for sendmmsg since now instead
>>    of calling the syscall for the packed structure, GLIBC is calling
>>    multiple sendmsg.  This defeat the very existence of the syscall.
>>
>>  * It currently breaks libsanitizer build on GCC [1] (I fixed on compiler-rt).
>>    However the fix is incomplete because it does add any runtime check
>>    since libsanitizer currently does not have any facility to intercept
>>    symbols with multiple version [2].
>>
>>    This, along with incorret dlsym/dlvsym return for versioned symbol due
>>    another bug [3], makes hard to interpose versioned symbols.
>>
>>    Also, current approach of fixing GCC PR#71445 leads to half-baked
>>    solutions without versioned symbol interposing.
>>
>> This patch basically reverts commits 2f0dc39029ae08, 222c2d7f4357d66,
>> af7f7c7ec8dea1.  I decided to not revert abf29edd4a3918 (Adjust
>> kernel-features.h defaults for recvmsg and sendmsg) mainly because it
>> does not really address the POSIX compliance original issue and also
>> adds some cleanups.
>>
>> Tested on x86, i386, s390, s390x, aarch64, and powerpc64le.
>>
>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71445
>> [2] https://github.com/google/sanitizers/issues/628
>> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=14932
> 
> Thank you for all the work you put into these patches.
> 
> While we ended up reverting them, I am wont to say "experience is knowing
> what not to do" and it is hard won.
> 
> While I am in favour of these patches, the subsequent problems make them
> difficult to justify until such problems are fixed.
> 
> Several of the problems are not specific to your changes either, they are
> general architectural problems with the tooling that will need to be fixed
> at some point as we version more and more of the implementation.
> 

I will push this soon and also reopen BZ#16919 with some notes why it was
reverted.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]