From patchwork Tue Jul 30 01:49:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 33850 Received: (qmail 51442 invoked by alias); 30 Jul 2019 01:49:16 -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 51429 invoked by uid 89); 30 Jul 2019 01:49:15 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=H*f:sk:878ssgz, H*i:sk:878ssgz X-HELO: mail-qk1-f193.google.com Return-Path: Subject: Re: [PATCH] nptl: Add compiler barrier in nptl/tst-pthread-getattr To: Florian Weimer Cc: Andreas Schwab , libc-alpha@sourceware.org References: <87blxms3r1.fsf@oldenburg2.str.redhat.com> <87v9vuqjke.fsf@oldenburg2.str.redhat.com> <878ssgzi87.fsf@oldenburg2.str.redhat.com> From: Carlos O'Donell Message-ID: <6ae0059a-9c61-5722-aba9-0ba5968d5874@redhat.com> Date: Mon, 29 Jul 2019 21:49:10 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <878ssgzi87.fsf@oldenburg2.str.redhat.com> On 7/29/19 4:41 PM, Florian Weimer wrote: > * Carlos O'Donell: > >>> tst-pthread-getattr.c: In function ‘allocate_and_test’: >>> tst-pthread-getattr.c:54:10: error: function returns address of local variable [-Werror=return-local-addr] >>> 54 | return mem; >>> | ^~~ >>> In file included from ../include/alloca.h:3, >>> from tst-pthread-getattr.c:26: >>> ../stdlib/alloca.h:35:23: note: declared here >>> 35 | # define alloca(size) __builtin_alloca (size) >>> | ^~~~~~~~~~~~~~~~~~~~~~~ >>> tst-pthread-getattr.c:51:9: note: in expansion of macro ‘alloca’ >>> 51 | mem = alloca ((size_t) (mem - target)); >>> | ^~~~~~ >>> >>> The address itself is used in a check in the caller, so using >>> uintptr_t instead is reasonable. >> >> Any reason why we wouldn't move the test into the function? > > I think printf might fault because of its own stack usage (which can > easily exceed half a page in some cases). Good point. I approve the original commit if you extend the comment to this: diff --git a/nptl/tst-pthread-getattr.c b/nptl/tst-pthread-getattr.c index a954778767..50a324cc68 100644 --- a/nptl/tst-pthread-getattr.c +++ b/nptl/tst-pthread-getattr.c @@ -41,8 +41,10 @@ static size_t pagesize; -/* Check if the page in which TARGET lies is accessible. This will segfault - if it fails. */ +/* Test that the page in which TARGET lies is accessible. This will + segfault if the write fails. This function has only half a page + of thread stack left and so should not do anything and immediately + return the address to which the stack reached. */ static volatile char * allocate_and_test (char *target) {