From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6635 invoked by alias); 27 Sep 2013 16:20:35 -0000 Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org Received: (qmail 6622 invoked by uid 89); 27 Sep 2013 16:20:35 -0000 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 27 Sep 2013 16:20:35 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.8 required=5.0 tests=AWL,BAYES_20,RDNS_NONE,SPAM_SUBJECT,SPF_HELO_FAIL autolearn=no version=3.3.2 X-HELO: relay1.mentorg.com Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1VPamM-0007Nm-09 from joseph_myers@mentor.com ; Fri, 27 Sep 2013 09:20:30 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Fri, 27 Sep 2013 09:20:27 -0700 Received: from digraph.polyomino.org.uk (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.2.247.3; Fri, 27 Sep 2013 17:20:24 +0100 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.76) (envelope-from ) id 1VPamE-0001sI-T2; Fri, 27 Sep 2013 16:20:22 +0000 Date: Fri, 27 Sep 2013 16:20:00 -0000 From: "Joseph S. Myers" To: Will Newton CC: , Subject: Re: [PATCH v2] ARM: Add pointer guard support. In-Reply-To: <5245A8FF.4000602@linaro.org> Message-ID: References: <5245A8FF.4000602@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2013-09/txt/msg00151.txt.bz2 On Fri, 27 Sep 2013, Will Newton wrote: > - The bit I don't like in this version of the patch is the test for !IS_IN_nscd, > but without it I get undefined references to __pointer_chk_guard_local. Please don't just state the symptom. Please give a more detailed analysis of the issue, explaining *why* those undefined references arise - what code gets used where in nscd, what semantics it is meant to have there, what the correct way is to access the relevant variable in that context, and what it is specifically about nscd that makes IS_IN_nscd (rather than some other conditional that would also cover other executables built with glibc) the logically correct conditional, and whether your logic applies also to --disable-shared builds of glibc when nscd would be statically linked. In general, neither this patch nor the previous iteration includes any rationale - you state what you do, but not anything about why that's a good thing to do in the first place, or what choices were made and the basis on which they were made. Is there some overall design document (past mailing list message, etc.) explaining "pointer mangling in glibc internal structures" and discussing what macros should be defined, with what semantics, and used where? If so, point to it in your submission. If not, your patch submission should include something of that explanation itself (with reference to other architectures where appropriate) - presumably you reverse-engineered this machinery yourself to write the patch, if there wasn't such an existing document, in which case you should write up the results of your reverse-engineering to benefit the reviewer and people implementing this for other architectures, rather than expecting people to repeat it again. (An alternative to putting it all in the patch submission is to create such detailed documentation for pointer mangling on the wiki, then point to the wiki page in the patch submission email. But any ARM-specific rationale should still go in the email.) You define a macro LDST_GLOBAL. This needs a comment explaining its semantics in detail, like the other nearby macros in sysdep.h. You can avoid such detailed comments on the mangling macros if they have architecture-independent semantics documented somewhere appropriate, but not for any new architecture-specific macro. -- Joseph S. Myers joseph@codesourcery.com