From patchwork Mon Nov 20 15:51:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Liebler X-Patchwork-Id: 24369 Received: (qmail 97462 invoked by alias); 20 Nov 2017 15:51:49 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 97451 invoked by uid 89); 20 Nov 2017 15:51:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KB_WAM_FROM_NAME_SINGLEWORD, KHOP_DYNAMIC, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Subject: Re: [PATCH] Use libc_hidden_proto / _def for hidden wchar ifunc symbols. To: libc-alpha@sourceware.org References: <8f9ec268-7792-1b86-aee4-ec3aa5878108@linux.vnet.ibm.com> Cc: "H.J. Lu" From: Stefan Liebler Date: Mon, 20 Nov 2017 16:51:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: X-TM-AS-GCONF: 00 x-cbid: 17112015-0012-0000-0000-0000058F83FA X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17112015-0013-0000-0000-0000190A50C6 Message-Id: <709ddc70-da73-e352-03f6-3a9414a0a064@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-11-20_08:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=1 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1711200213 On 11/13/2017 06:30 PM, H.J. Lu wrote: > On Mon, Nov 13, 2017 at 9:11 AM, Stefan Liebler wrote: >> On 11/13/2017 02:58 PM, H.J. Lu wrote: >>> >>> On Mon, Nov 13, 2017 at 3:55 AM, Florian Weimer >>> wrote: >>>> >>>> On 11/13/2017 11:16 AM, Stefan Liebler wrote: >>>>> >>>>> >>>>> This patch adds the libc_hidden_proto / libc_hidden_def construct. >>>>> Then the __GI_* symbols are the default-ifunc-variants which can be >>>>> called >>>>> without PLT. >>>> >>>> >>>> >>>> attribute_hidden and *_hidden_{proto,def} conflict on some architectures. >>>> You need to remove attribute_hidden as part of this change. >>> >>> >>> That is true. On i686, a hidden IFUNC function inside libc.so must be >>> compiled >>> with -fPIC via PLT since EBX must be loaded with GOT first. This >>> isn't an issue >>> for x86-64 since PLT uses PC-relative addressing. In this case, we >>> should remove >>> hidden attribute, instead of using __GI_* symbols, if we sill want to use >>> IFUNC >>> inside libc.so. For the moment, I propose to remove the attribute_hidden in order to fix those internal IFUNC calls. Is the attached patch okay to commit? >> >> On s390/s390x, just removing attribute_hidden in wchar.h is fine, too. Then >> r12 will be setup with the GOT-pointer and IFUNC will be used inside >> libc.so. >> >> What was your original intention? Getting rid of "PLT" or "PLT and IFUNC" >> for calls inside libc.so? > > The original intention is to remove PLT. But it doesn't work for targets which > need to set up a special register for PLT which is required by IFUNC. > >>> >>> Now I have question, is there a way to apply attribute_hidden to a >>> function >>> depending on architecture? For example, we remove attribute_hidden >>> from __wcsnlen, __wcscat, __wcsncpy, __wcpncpy, __wcschrnul in headers >>> under include/. For x86, we mark them hidden in a header file under >>> sysdeps/x86? >> >> I don't know if duplicating the wchar.h file is such a good idea. >> > > Can we add > > #include > > to include/wchar.h and add a dummy sysdeps/generic/wcharP.h? Then > I can add sysdeps/x86_64/wcharP.h to hide __wcsnlen, __wcscat, __wcsncpy, > _wcpncpy, __wcschrnul > > @H.J. Lu: Can you propose a separate patch for architecture dependent wcharP.h files? Bye. Stefan commit 70c17e8f6a07236aaa37e1aab67703d3bc1e09a2 Author: Stefan Liebler Date: Mon Nov 20 12:26:24 2017 +0100 Remove attribute_hidden for wchar ifunc symbols. On s390 (31bit) various debug/tst-*chk* testcases are failing as the tests are ending with a segmentation fault. One test is e.g. calling wcsnrtombs in debug/tst-chk1.c:1549. The function wcsnrtombs itself calls __wcsnlen. This function is called via PLT! The PLT-stub itself loads the address from GOT (r12 is assumed to be the GOT-pointer). In this case the loaded address is zero and the following branch leads to the segmentation fault. Due to the attribute_hidden in commit 44af8a32c341672b5160fdc2839767e9a837ad26 "Mark internal wchar functions with attribute_hidden [BZ #18822]" for e.g. the __wcsnlen function, r12 is not loaded with the GOT-pointer in wcsnrtombs. On s390x (64bit), this __wcsnlen call is also using the PLT-stub. But it is not failing as the GOT-pointer is setup with larl-instruction by the PLT-stub itself. Note: On s390x/s390, __wcsnlen is an IFUNC symbol. On x86_64, __wcsnlen is also an IFUNC symbol and is called via PLT, too. Further IFUNC symbols on s390 which were marked as hidden by the mentioned commit are: __wcscat, __wcsncpy, __wcpncpy, __wcschrnul. This patch removes the attribute_hidden in wchar.h. Then the compiler setups e.g. r12 on s390 in order to call __wcsnlen via PLT. ChangeLog: * include/wchar.h (__wcsnlen, __wcscat, __wcsncpy, __wcpncpy, __wcschrnul): Remove attribute_hidden. diff --git a/include/wchar.h b/include/wchar.h index 24b2eaa..1db0ac8 100644 --- a/include/wchar.h +++ b/include/wchar.h @@ -152,9 +152,8 @@ extern int __wcsncasecmp (const wchar_t *__s1, const wchar_t *__s2, __attribute_pure__; extern size_t __wcslen (const wchar_t *__s) __attribute_pure__; extern size_t __wcsnlen (const wchar_t *__s, size_t __maxlen) - attribute_hidden __attribute_pure__; -extern wchar_t *__wcscat (wchar_t *dest, const wchar_t *src) - attribute_hidden; + __attribute_pure__; +extern wchar_t *__wcscat (wchar_t *dest, const wchar_t *src); extern wint_t __btowc (int __c) attribute_hidden; extern int __mbsinit (const __mbstate_t *__ps); extern size_t __mbrtowc (wchar_t *__restrict __pwc, @@ -182,11 +181,10 @@ extern size_t __wcsnrtombs (char *__restrict __dst, __mbstate_t *__restrict __ps) attribute_hidden; extern wchar_t *__wcsncpy (wchar_t *__restrict __dest, - const wchar_t *__restrict __src, size_t __n) - attribute_hidden; + const wchar_t *__restrict __src, size_t __n); extern wchar_t *__wcpcpy (wchar_t *__dest, const wchar_t *__src); extern wchar_t *__wcpncpy (wchar_t *__dest, const wchar_t *__src, - size_t __n) attribute_hidden; + size_t __n); extern wchar_t *__wmemcpy (wchar_t *__s1, const wchar_t *s2, size_t __n) attribute_hidden; extern wchar_t *__wmempcpy (wchar_t *__restrict __s1, @@ -195,7 +193,7 @@ extern wchar_t *__wmempcpy (wchar_t *__restrict __s1, extern wchar_t *__wmemmove (wchar_t *__s1, const wchar_t *__s2, size_t __n) attribute_hidden; extern wchar_t *__wcschrnul (const wchar_t *__s, wchar_t __wc) - attribute_hidden __attribute_pure__; + __attribute_pure__; extern wchar_t *__wmemset_chk (wchar_t *__s, wchar_t __c, size_t __n, size_t __ns) __THROW;