Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 6 May 2014 23:07:55 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        David Chisnall <theraven@FreeBSD.org>
Cc:        src-committers <src-committers@FreeBSD.org>, Andrey Chernov <ache@FreeBSD.org>, svn-src-all@FreeBSD.org, Pedro Giffuni <pfg@FreeBSD.org>, Bruce Evans <brde@optusnet.com.au>, svn-src-head@FreeBSD.org, Warner Losh <imp@bsdimp.com>
Subject:   Re: svn commit: r265367 - head/lib/libc/regex
Message-ID:  <20140506222048.V3154@besplex.bde.org>
In-Reply-To: <0FCEDD3C-A512-4B83-A8C8-5A1B7A33AAF2@FreeBSD.org>
References:  <201405051641.s45GfFje086423@svn.freebsd.org> <5367CD77.40909@freebsd.org> <B11B5B25-8E05-4225-93D5-3A607332F19A@FreeBSD.org> <5367EB54.1080109@FreeBSD.org> <3C7CFFB7-5C84-4AC1-9A81-C718D184E87B@FreeBSD.org> <7D7A417E-17C3-4001-8E79-0B57636A70E1@gmail.com> <A4B5E0E8-93CB-4E80-9065-5D25A007B726@FreeBSD.org> <536807D8.9000005@freebsd.org> <9349EAA9-F92C-4170-A1C0-2200FD490E5F@FreeBSD.org> <5368162A.9000002@freebsd.org> <20140506135706.T1596@besplex.bde.org> <0FCEDD3C-A512-4B83-A8C8-5A1B7A33AAF2@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 6 May 2014, David Chisnall wrote:

> On 6 May 2014, at 05:46, Bruce Evans <brde@optusnet.com.au> wrote:
>
>> The standard behaviour is undefined.  It cannot be relied on.  From C99
>> (n869.txt):
>>
>> %        7.20.3.1  The calloc function
>> % %        Synopsis
>> % %        [#1]
>> % %                #include <stdlib.h>
>> %                void *calloc(size_t nmemb, size_t size);
>> % %        Description
>> % %        [#2] The calloc function allocates space  for  an  array  of
>> %        nmemb  objects,  each  of  whose size is size.  The space is
>> %        initialized to all bits zero.238)
>>
>> Oops, there is no object to begin with, so perhaps the behaviour is
>> defined after all.  This is unclear.
>
> You're missing off the next line:
>
>> =09=95 3  The calloc function returns either a null pointer or a pointer=
 to the allocated space.

It takes more that that to give defined behaviour.

There is a similar example for snprintf().  It is specified to return
a count in an int variable, but it is possible for the correct count
to be unrepresentable as an int.  The behaviour is then implicitly
undefined.  The function parameters are just invalid, and undefined
behaviour occurs because snprintf() just doesn't support invalid
parameters.

Here calloc() can sort of support invalid parameters by returning a
nondescript error for them.  The question is if it is required to do
this.

> Clarifications from WG14 have indicated that this means that calloc() *mu=
st* return either NULL or enough space for nmemb objects of size size.  The=
 text of the standard was not changed in C11 because it seemed to be the co=
nsensus of library authors that this is obvious from the existing text.  Se=
e the CERT report from my previous email - in 2002 it was regarded as a sec=
urity hole (and a lack of standards conformance) if your calloc did not do =
this and all known calloc implementations that did not were fixed.

It is not obvious.  C11 (n1570.pdf) also didn't change the wording for
snprintf().  It is not obvious that (because of 20+ year old design errors)
it has more undefined cases that might be expected.

> Now, you can argue that either:
>
> - In this case, we can statically prove that the multiplication won't ove=
rflow so we don't need a check, or
>
> - It is better to do the overflow check on the caller side and increase i=
-cache usage to save some memory zeroing.
>
> But please don't try to argue that it is permitted for calloc() to not co=
rrectly handle integer overflow.  It is both non-conformant and dangerous f=
or it to fail to do so.

calloc() is not even required to do the multiplication...

>> It is also unclear if objects
>> can have size too large to represent as a size_t

=2E.. A  silly implementation of calloc() could do sbrk() 1 element at a ti=
me.
This is a slow way of doing the multiplication as well as the allocation.
It might work to allocate an object(?) larger than SIZE_MAX.  Is calloc()
allowed to do this?

> That is implementation defined, however if sizeof(ptrdiff_t) <=3D sizeof(=
size_t) then they can not because you must be able to represent the differe=
nce between any two pointers as a ptrdiff_t[1].  If you want to be pedantic=
, _Static_assert(sizeof(ptrdiff_t) <=3D sizeof(size_t), "Unsupported platfo=
rm!") to make sure you catch it at compile time if this might change.

ptrdiff_t is much more broken as designed than size_t.   Apart from the
sign problem, it is not required to work useful unless the difference betwe=
en
the pointers is between -65535 and 65335 (the behaviour of pointer
subtraction is undefined unless the result is represntable as a ptrdiff_t,
and ptrdiff_t is not required to be any larger than 1's complement with 17
bits even if size_t is much larger).

The unclear point is if the implementation-defined result of sizeof() can
be different to the size of the object due to the latter being too large
to represent in a size_t.

> [1] This also means, on our platforms, that the maximum size of an object=
 must be one byte less than the total size of the address space, as C only =
defines pointer comparisons between valid pointers to the same object and a=
llows pointers to be one element past the end of an array.

I used to run into this problem on 16-bit systems where half of the
address space is the good buffer or heap size 32K.  C90 didn't have
PTRDIFF_MIN/MAX and pre-C90 had even less, so most implementations had
ptrdiff_t =3D int and the undefined behaviour from pointer subtraction
occurred in practice.

Bruce
From owner-svn-src-all@FreeBSD.ORG  Tue May  6 13:38:35 2014
Return-Path: <owner-svn-src-all@FreeBSD.ORG>
Delivered-To: svn-src-all@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by hub.freebsd.org (Postfix) with ESMTPS id 6B276CC0;
 Tue,  6 May 2014 13:38:35 +0000 (UTC)
Received: from svn.freebsd.org (svn.freebsd.org
 [IPv6:2001:1900:2254:2068::e6a:0])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (Client did not present a certificate)
 by mx1.freebsd.org (Postfix) with ESMTPS id 58C6EEF4;
 Tue,  6 May 2014 13:38:35 +0000 (UTC)
Received: from svn.freebsd.org ([127.0.1.70])
 by svn.freebsd.org (8.14.8/8.14.8) with ESMTP id s46DcZgu017479;
 Tue, 6 May 2014 13:38:35 GMT (envelope-from ian@svn.freebsd.org)
Received: (from ian@localhost)
 by svn.freebsd.org (8.14.8/8.14.8/Submit) id s46DcZJv017477;
 Tue, 6 May 2014 13:38:35 GMT (envelope-from ian@svn.freebsd.org)
Message-Id: <201405061338.s46DcZJv017477@svn.freebsd.org>
From: Ian Lepore <ian@FreeBSD.org>
Date: Tue, 6 May 2014 13:38:35 +0000 (UTC)
To: src-committers@freebsd.org, svn-src-all@freebsd.org,
 svn-src-head@freebsd.org
Subject: svn commit: r265440 - head/sys/arm/arm
X-SVN-Group: head
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-BeenThere: svn-src-all@freebsd.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: "SVN commit messages for the entire src tree \(except for &quot;
 user&quot; and &quot; projects&quot; \)" <svn-src-all.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/svn-src-all>,
 <mailto:svn-src-all-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-all/>;
List-Post: <mailto:svn-src-all@freebsd.org>
List-Help: <mailto:svn-src-all-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-all>,
 <mailto:svn-src-all-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 06 May 2014 13:38:35 -0000

Author: ian
Date: Tue May  6 13:38:34 2014
New Revision: 265440
URL: http://svnweb.freebsd.org/changeset/base/265440

Log:
  Move the pl310.enabled tunable to hw.pl310.enabled.  Clean up a few minor
  style(9) nits.  Use DEVMETHOD_END.

Modified:
  head/sys/arm/arm/pl310.c

Modified: head/sys/arm/arm/pl310.c
==============================================================================
--- head/sys/arm/arm/pl310.c	Tue May  6 12:39:23 2014	(r265439)
+++ head/sys/arm/arm/pl310.c	Tue May  6 13:38:34 2014	(r265440)
@@ -71,7 +71,7 @@ __FBSDID("$FreeBSD$");
 } while(0);
 
 static int pl310_enabled = 1;
-TUNABLE_INT("pl310.enabled", &pl310_enabled);
+TUNABLE_INT("hw.pl310.enabled", &pl310_enabled);
 
 static uint32_t g_l2cache_way_mask;
 
@@ -149,7 +149,8 @@ static __inline void
 pl310_wait_background_op(uint32_t off, uint32_t mask)
 {
 
-	while (pl310_read4(pl310_softc, off) & mask);
+	while (pl310_read4(pl310_softc, off) & mask)
+		continue;
 }
 
 
@@ -167,6 +168,7 @@ pl310_wait_background_op(uint32_t off, u
 static __inline void
 pl310_cache_sync(void)
 {
+
 	if ((pl310_softc == NULL) || !pl310_softc->sc_enabled)
 		return;
 
@@ -335,12 +337,13 @@ static int
 pl310_attach(device_t dev)
 {
 	struct pl310_softc *sc = device_get_softc(dev);
-	int rid = 0;
+	int rid;
 	uint32_t aux_value;
 	uint32_t ctrl_value;
 	uint32_t cache_id;
 
 	sc->sc_dev = dev;
+	rid = 0;
 	sc->sc_mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, 
 	    RF_ACTIVE);
 	if (sc->sc_mem_res == NULL)
@@ -446,7 +449,7 @@ pl310_attach(device_t dev)
 static device_method_t pl310_methods[] = {
 	DEVMETHOD(device_probe, pl310_probe),
 	DEVMETHOD(device_attach, pl310_attach),
-	{0, 0},
+	DEVMETHOD_END
 };
 
 static driver_t pl310_driver = {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140506222048.V3154>