From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: Emacs port to gcc -fcheck-pointer-bounds Date: Tue, 12 Dec 2017 15:35:01 -0800 Organization: UCLA Computer Science Department Message-ID: <1da23740-5960-9358-a46c-3b078428cb6c@cs.ucla.edu> References: <83indhwcx5.fsf@gnu.org> <83k1xwuwq3.fsf@gnu.org> <83efo2trwu.fsf@gnu.org> <83374hthe6.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------72E84601F825C3AA95D1E9AD" X-Trace: blaine.gmane.org 1513121717 14939 195.159.176.226 (12 Dec 2017 23:35:17 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 12 Dec 2017 23:35:17 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 Cc: pipcet@gmail.com, Emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Dec 13 00:35:12 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eOu4t-0003cE-Mj for ged-emacs-devel@m.gmane.org; Wed, 13 Dec 2017 00:35:11 +0100 Original-Received: from localhost ([::1]:32787 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eOu50-00021w-HR for ged-emacs-devel@m.gmane.org; Tue, 12 Dec 2017 18:35:18 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60724) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eOu4r-00021f-8M for Emacs-devel@gnu.org; Tue, 12 Dec 2017 18:35:10 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eOu4o-000481-4Q for Emacs-devel@gnu.org; Tue, 12 Dec 2017 18:35:09 -0500 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:42756) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eOu4n-000460-QA; Tue, 12 Dec 2017 18:35:06 -0500 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id B1AA616148E; Tue, 12 Dec 2017 15:35:03 -0800 (PST) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id jo8aEUgQBmTa; Tue, 12 Dec 2017 15:35:02 -0800 (PST) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 86C64161492; Tue, 12 Dec 2017 15:35:02 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id qXJ_k1nV2Fw8; Tue, 12 Dec 2017 15:35:02 -0800 (PST) Original-Received: from Penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 67722161442; Tue, 12 Dec 2017 15:35:02 -0800 (PST) In-Reply-To: <83374hthe6.fsf@gnu.org> Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:220978 Archived-At: This is a multi-part message in MIME format. --------------72E84601F825C3AA95D1E9AD Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 12/11/2017 07:26 AM, Eli Zaretskii wrote: > So we will have two different data types implementing a Lisp object, > one --with-wide-int and another without it? That sounds suboptimal to > me. I figured out a simple fix to get it to work --with-wide-int (see first attached patch), so I hope this issue is moot now, and boldly installed the patches into master, along with the attached followup patches. > Why not reserve the new representation to the -fcheck-pointer-bounds > builds only? Although we could change lisp.h to do that, I'd rather not. Having Lisp_Object be an opaque pointer type is a win even for non-developers who merely make minor changes to the C code and are not using -fcheck-pointer-bounds or --enable-check-lisp-object-type or anything like that. In practice it's error-prone when Lisp_Object is an integer type, and if we can easily avoid these errors we should. Here's another way to think about it. Our current practice of defaulting to --enable-check-lisp-object-type for developers is an outgrowth of the fact that integer Lisp_Objects are so error-prone. Unfortunately, this practice is dicey in its own right, as it means developers are dealing with different object code than non-developers. I would favor going back to the old practice of disabling --enable-check-lisp-object-type by default, even for developers, once we've shaken out the change that I just installed. That way, developers and non-developers will default to more-similar machine code. --------------72E84601F825C3AA95D1E9AD Content-Type: text/x-patch; name="0001-Port-fcheck-pointer-bounds-to-with-wide-int.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Port-fcheck-pointer-bounds-to-with-wide-int.patch" >From e921f97df9a11fd6f43ee040ba97c686c3fa62ee Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 12 Dec 2017 12:59:27 -0800 Subject: [PATCH 1/2] Port --fcheck-pointer-bounds to --with-wide-int * src/lisp (XSYMBOL) [__CHKP__ && !USE_LSB_TAG]: Bypass pointer bounds checking here, instead of failing the entire build. (make_lisp_symbol): Improve comment. --- src/lisp.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/lisp.h b/src/lisp.h index 56545b7094..eb31ba209a 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -902,12 +902,15 @@ INLINE struct Lisp_Symbol * { #if USE_LSB_TAG return lisp_h_XSYMBOL (a); -#elif defined __CHKP__ -# error "pointer-checking not supported with wide integers" #else eassert (SYMBOLP (a)); intptr_t i = (intptr_t) XUNTAG (a, Lisp_Symbol); void *p = (char *) lispsym + i; +# ifdef __CHKP__ + /* Bypass pointer checking. Although this could be improved it is + probably not worth the trouble. */ + p = __builtin___bnd_set_ptr_bounds (p, sizeof (struct Lisp_Symbol)); +# endif return p; #endif } @@ -916,9 +919,11 @@ INLINE Lisp_Object make_lisp_symbol (struct Lisp_Symbol *sym) { #ifdef __CHKP__ - /* Although this should use '__builtin___bnd_narrow_ptr_bounds (sym, - sym, sizeof *sym)', that would run afoul of GCC bug 83251 - . */ + /* Although '__builtin___bnd_narrow_ptr_bounds (sym, sym, sizeof *sym)' + should be more efficient, it runs afoul of GCC bug 83251 + . + Also, attempting to call __builtin___bnd_chk_ptr_bounds (sym, sizeof *sym) + here seems to trigger a GCC bug, as yet undiagnosed. */ char *addr = __builtin___bnd_set_ptr_bounds (sym, sizeof *sym); char *symoffset = addr - (intptr_t) lispsym; #else -- 2.14.3 --------------72E84601F825C3AA95D1E9AD Content-Type: text/x-patch; name="0002-Fix-recently-introduced-cast-typo.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Fix-recently-introduced-cast-typo.patch" >From 8e78d49765993bbbb93d42b0530f5ffaa4e759f4 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 12 Dec 2017 15:15:56 -0800 Subject: [PATCH 2/2] Fix recently-introduced cast typo * src/alloc.c (SDATA_OF_STRING): Put cast in right spot. This matters only if GC_CHECK_STRING_BYTES, which is sort of a coals-to-Newcastle situation if pointer bounds checking is also enabled. --- src/alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/alloc.c b/src/alloc.c index 9197ff12ef..fb0f948474 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -1727,7 +1727,7 @@ static EMACS_INT total_string_bytes; a pointer to the `u.data' member of its sdata structure; the structure starts at a constant offset in front of that. */ -#define SDATA_OF_STRING(S) ptr_bounds_init ((sdata *) ((S)->u.s.data \ +#define SDATA_OF_STRING(S) ((sdata *) ptr_bounds_init ((S)->u.s.data \ - SDATA_DATA_OFFSET)) -- 2.14.3 --------------72E84601F825C3AA95D1E9AD--