On 11/02/2014 11:07 PM, Alan Modra wrote:
Really, it's a fiction that the size argument of obstack_blank_fast has a type.
Yes and no. It's a fiction in the sense that obstack_blank_fast is a macro, so its argument doesn't actually have a C type. But it's not a fiction in the sense that obstack_blank_fast can be documented as if it were a function, and if a program calls it with valid arguments that a function would have, it behaves like that function.
In this sense, obstack_blank_fast is like putchar. Although putchar is a macro and strictly speaking its argument doesn't have a C type, it is documented to take a value of type int, and if you call it according to its specification it behaves properly.
I've added: "Earlier versions of obstacks allowed you to use @code{obstack_blank} to shrink objects. This will no longer work."
But this doesn't suffice for obstack_blank_stack, which *can* shrink objects and where user code relies on this feature. E.g., the following code (taken from GCC's finish_regexp_representation) should work if the current object size fits into 'int', and should continue to work if we change its 'int' to 'size_t' and allocate objects larger than what 'int' can represent:
int length = obstack_object_size (&irp); obstack_blank_fast (&irp, -length);
There another gnulib issue too. AC_FUNC_OBSTACK is no longer correct. The system libc may support obstacks, but not the current version. It seems to me the easiest solution is to always include obstack.o in libgnu.a.
Thanks, I wrote a "harder" solution, which builds obstack.o only if the system version doesn't work with arbitrary size_t values. This is always true now but won't be once these fixes make their way into glibc.
Ianother small tweak: Don't use alignof.h if __IBM__ALIGNOF__ is defined (alignof.h just uses __alignof__ in that case), or when __alignof__ is defined (for oddball systems, or testing).
I'd rather insulate obstack from that internal detail of alignof. I gave a shot at doing this in a different way, which preserves the insulation better.
A couple more things. obstack_chunkfun and obstack_freefun should also return void. And the code should avoid casting function values from one calling convention to another: this sort-of-works with the old API on glibc's current platforms but is likely to cause problems with the transition to the new one.
I installed the attached patches to gnulib to try to implement the above; please let me know of any issues.
From 3d99d9fb66ceea0f93a6cfeb496f0471efa212d3 Mon Sep 17 00:00:00 2001 From: Paul Eggert <egg...@cs.ucla.edu> Date: Mon, 3 Nov 2014 16:34:52 -0800 Subject: [PATCH 1/4] obstack: port to platforms that #define __alignof__ * lib/obstack.c: Include <alignof.h> if !defined __alignof__, not if !_LIBC. We don't know of any platforms that #define __alignof__, but it might be useful in tests. Conversely, glibc assumes GCC. --- ChangeLog | 8 ++++++++ lib/obstack.c | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 6728893..274b794 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2014-11-03 Paul Eggert <egg...@cs.ucla.edu> + + obstack: port to platforms that #define __alignof__ + * lib/obstack.c: Include <alignof.h> if !defined __alignof__, + not if !_LIBC. We don't know of any platforms that #define + __alignof__, but it might be useful in tests. Conversely, + glibc assumes GCC. + 2014-11-03 Pádraig Brady <p...@draigbrady.com> linkat: don't unconditionally replace on GNU/Linux diff --git a/lib/obstack.c b/lib/obstack.c index ba7dff3..d763c57 100644 --- a/lib/obstack.c +++ b/lib/obstack.c @@ -48,7 +48,10 @@ #endif #ifndef _OBSTACK_ELIDE_CODE -# if !defined _LIBC && !defined __GNUC__ +/* If GCC, or if an oddball (testing?) host that #defines __alignof__, + use the already-supplied __alignof__. Otherwise, this must be Gnulib + (as glibc assumes GCC); defer to Gnulib's alignof_type. */ +# if !defined __GNUC__ && !defined __alignof__ # include <alignof.h> # define __alignof__(type) alignof_type (type) # endif -- 1.9.3
From 035a186cc20f21354720c51cc303b6c0d6017c5f Mon Sep 17 00:00:00 2001 From: Paul Eggert <egg...@cs.ucla.edu> Date: Mon, 3 Nov 2014 16:36:27 -0800 Subject: [PATCH 2/4] obstack: do not assume system-supplied obstack is size_t safe * m4/obstack.m4: New file. * modules/obstack (Files): Add it. --- ChangeLog | 4 ++++ m4/obstack.m4 | 35 +++++++++++++++++++++++++++++++++++ modules/obstack | 1 + 3 files changed, 40 insertions(+) create mode 100644 m4/obstack.m4 diff --git a/ChangeLog b/ChangeLog index 274b794..23accbe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2014-11-03 Paul Eggert <egg...@cs.ucla.edu> + obstack: do not assume system-supplied obstack is size_t safe + * m4/obstack.m4: New file. + * modules/obstack (Files): Add it. + obstack: port to platforms that #define __alignof__ * lib/obstack.c: Include <alignof.h> if !defined __alignof__, not if !_LIBC. We don't know of any platforms that #define diff --git a/m4/obstack.m4 b/m4/obstack.m4 new file mode 100644 index 0000000..bc5b4a4 --- /dev/null +++ b/m4/obstack.m4 @@ -0,0 +1,35 @@ +# See if we need to provide obstacks. + +dnl Copyright 1996-2014 Free Software Foundation, Inc. +dnl This file is free software; the Free Software Foundation +dnl gives unlimited permission to copy and/or distribute it, +dnl with or without modifications, as long as this notice is preserved. + +dnl This replaces Autoconf's AC_FUNC_OBSTACK. +dnl The Autoconf version should be marked obsolete at some point. + +AC_DEFUN([AC_FUNC_OBSTACK], + [AC_LIBSOURCES([obstack.h, obstack.c])dnl + AC_CACHE_CHECK([for obstacks that work with any size object], + [ac_cv_func_obstack], + [AC_LINK_IFELSE( + [AC_LANG_PROGRAM( + [[#include "obstack.h" + void *obstack_chunk_alloc (size_t n) { return 0; } + void obstack_chunk_free (void *p) { } + /* Check that an internal function returns size_t, not int. */ + size_t _obstack_memory_used (struct obstack *); + ]], + [[struct obstack mem; + obstack_init (&mem); + obstack_free (&mem, 0); + ]])], + [ac_cv_func_obstack=yes], + [ac_cv_func_obstack=no])]) + if test "$ac_cv_func_obstack" = yes; then + AC_DEFINE([HAVE_OBSTACK], 1, + [Define to 1 if the system has obstacks that work with any size object.]) + else + AC_LIBOBJ([obstack]) + fi +]) diff --git a/modules/obstack b/modules/obstack index 36dc444..e0b528e 100644 --- a/modules/obstack +++ b/modules/obstack @@ -4,6 +4,7 @@ Memory allocation, optimized for stack-like allocation patterns. Files: lib/obstack.h lib/obstack.c +m4/obstack.m4 Depends-on: alignof -- 1.9.3
From 488669cb5f25d4e9546fb8727f0ec117a6b828f2 Mon Sep 17 00:00:00 2001 From: Alan Modra <amo...@gmail.com> Date: Mon, 3 Nov 2014 17:32:27 -0800 Subject: [PATCH 3/4] obstack: fix macro return values * lib/obstack.h (obstack_next_free): Return void *. (obstack_1grow_fast, obstack_blank_fast): Return void. For __GNUC__ macros: (obstack_1grow, obstack_blank): Remove now unnecessary (void) 0. For !__GNUC__ macros: (obstack_make_room, obstack_grow, obstack_grow0) (obstack_ptr_grow_fast, obstack_int_grow_fast): Return void. --- ChangeLog | 11 +++++++++++ lib/obstack.h | 27 +++++++++++++++------------ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 23accbe..ac78e88 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2014-11-03 Alan Modra <amo...@gmail.com> + + obstack: fix macro return values + * lib/obstack.h (obstack_next_free): Return void *. + (obstack_1grow_fast, obstack_blank_fast): Return void. + For __GNUC__ macros: + (obstack_1grow, obstack_blank): Remove now unnecessary (void) 0. + For !__GNUC__ macros: + (obstack_make_room, obstack_grow, obstack_grow0) + (obstack_ptr_grow_fast, obstack_int_grow_fast): Return void. + 2014-11-03 Paul Eggert <egg...@cs.ucla.edu> obstack: do not assume system-supplied obstack is size_t safe diff --git a/lib/obstack.h b/lib/obstack.h index ba4de1d..61b824b 100644 --- a/lib/obstack.h +++ b/lib/obstack.h @@ -219,7 +219,7 @@ extern int obstack_exit_failure; /* Pointer to next byte not yet allocated in current chunk. */ -#define obstack_next_free(h) ((h)->next_free) +#define obstack_next_free(h) ((void *) (h)->next_free) /* Mask specifying low bits that should be clear in address of an object. */ @@ -252,9 +252,9 @@ extern int obstack_exit_failure; #define obstack_freefun(h, newfreefun) \ ((h)->freefun = (void (*)(void *, struct _obstack_chunk *))(newfreefun)) -#define obstack_1grow_fast(h, achar) (*((h)->next_free)++ = (achar)) +#define obstack_1grow_fast(h, achar) ((void) (*((h)->next_free)++ = (achar))) -#define obstack_blank_fast(h, n) ((h)->next_free += (n)) +#define obstack_blank_fast(h, n) ((void) ((h)->next_free += (n))) #define obstack_memory_used(h) _obstack_memory_used (h) @@ -322,8 +322,7 @@ extern int obstack_exit_failure; ({ struct obstack *__o = (OBSTACK); \ if (obstack_room (__o) < 1) \ _obstack_newchunk (__o, 1); \ - obstack_1grow_fast (__o, datum); \ - (void) 0; }) + obstack_1grow_fast (__o, datum); }) /* These assume that the obstack alignment is good enough for pointers or ints, and that the data added so far to the current object @@ -365,8 +364,7 @@ extern int obstack_exit_failure; _OBSTACK_SIZE_T __len = (length); \ if (obstack_room (__o) < __len) \ _obstack_newchunk (__o, __len); \ - obstack_blank_fast (__o, __len); \ - (void) 0; }) + obstack_blank_fast (__o, __len); }) # define obstack_alloc(OBSTACK, length) \ __extension__ \ @@ -435,14 +433,16 @@ extern int obstack_exit_failure; # define obstack_make_room(h, length) \ ((h)->temp.i = (length), \ ((obstack_room (h) < (h)->temp.i) \ - ? (_obstack_newchunk ((h), (h)->temp.i), 0) : 0)) + ? (_obstack_newchunk (h, (h)->temp.i), 0) : 0), \ + (void) 0) # define obstack_grow(h, where, length) \ ((h)->temp.i = (length), \ ((obstack_room (h) < (h)->temp.i) \ ? (_obstack_newchunk ((h), (h)->temp.i), 0) : 0), \ memcpy ((h)->next_free, where, (h)->temp.i), \ - (h)->next_free += (h)->temp.i) + (h)->next_free += (h)->temp.i, \ + (void) 0) # define obstack_grow0(h, where, length) \ ((h)->temp.i = (length), \ @@ -450,7 +450,8 @@ extern int obstack_exit_failure; ? (_obstack_newchunk ((h), (h)->temp.i + 1), 0) : 0), \ memcpy ((h)->next_free, where, (h)->temp.i), \ (h)->next_free += (h)->temp.i, \ - *((h)->next_free)++ = 0) + *((h)->next_free)++ = 0, \ + (void) 0) # define obstack_1grow(h, datum) \ (((obstack_room (h) < 1) \ @@ -468,10 +469,12 @@ extern int obstack_exit_failure; obstack_int_grow_fast (h, datum)) # define obstack_ptr_grow_fast(h, aptr) \ - (((const void **) ((h)->next_free += sizeof (void *)))[-1] = (aptr)) + (((const void **) ((h)->next_free += sizeof (void *)))[-1] = (aptr), \ + (void) 0) # define obstack_int_grow_fast(h, aint) \ - (((int *) ((h)->next_free += sizeof (int)))[-1] = (aint)) + (((int *) ((h)->next_free += sizeof (int)))[-1] = (aint), \ + (void) 0) # define obstack_blank(h, length) \ ((h)->temp.i = (length), \ -- 1.9.3
From ecfbf8c8668da0021015796361272e402efe01e4 Mon Sep 17 00:00:00 2001 From: Paul Eggert <egg...@cs.ucla.edu> Date: Tue, 4 Nov 2014 00:31:31 -0800 Subject: [PATCH 4/4] obstack: avoid potentially-nonportable function casts * lib/obstack.c (CALL_CHUNKFUN, CALL_FREEFUN): Remove, replacing with ... (call_chunkfun, call_freefun): New static functions. All uses changed. Avoid potentially-nonportable casts. (chunkfun_type, freefun_type): Remove typedefs; no longer used. (_obstack_begin_worker): Omit last two args, since they rely on potentially-nonportable casts. All callers changed. * lib/obstack.h (_OBSTACK_CAST): New macro. Use it everywhere the old API used a potentially-nonportable cast. The new API doesn't cast. (struct obstack): Use unions rather than requiring potentially-nonportable casts. (obstack_chunkfun, obstack_freefun): Return void. --- ChangeLog | 17 +++++++++++++++ lib/obstack.c | 69 +++++++++++++++++++++++++++-------------------------------- lib/obstack.h | 48 +++++++++++++++++++++++++---------------- 3 files changed, 79 insertions(+), 55 deletions(-) diff --git a/ChangeLog b/ChangeLog index ac78e88..3b3af67 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2014-11-04 Paul Eggert <egg...@cs.ucla.edu> + + obstack: avoid potentially-nonportable function casts + * lib/obstack.c (CALL_CHUNKFUN, CALL_FREEFUN): + Remove, replacing with ... + (call_chunkfun, call_freefun): New static functions. + All uses changed. Avoid potentially-nonportable casts. + (chunkfun_type, freefun_type): Remove typedefs; no longer used. + (_obstack_begin_worker): Omit last two args, since they + rely on potentially-nonportable casts. All callers changed. + * lib/obstack.h (_OBSTACK_CAST): New macro. + Use it everywhere the old API used a potentially-nonportable cast. + The new API doesn't cast. + (struct obstack): Use unions rather than requiring + potentially-nonportable casts. + (obstack_chunkfun, obstack_freefun): Return void. + 2014-11-03 Alan Modra <amo...@gmail.com> obstack: fix macro return values diff --git a/lib/obstack.c b/lib/obstack.c index d763c57..90555ed 100644 --- a/lib/obstack.c +++ b/lib/obstack.c @@ -76,41 +76,38 @@ MAX (sizeof (uintmax_t), \ sizeof (void *))) -/* Define a macro that either calls functions with the traditional malloc/free - calling interface, or calls functions with the mmalloc/mfree interface - (that adds an extra first argument), based on the state of use_extra_arg. - For free, do not use ?:, since some compilers, like the MIPS compilers, - do not allow (expr) ? void : void. */ - -# define CALL_CHUNKFUN(h, size) \ - (((h)->use_extra_arg) \ - ? (*(h)->chunkfun)((h)->extra_arg, (size)) \ - : (*(struct _obstack_chunk *(*)(size_t))(h)->chunkfun)((size))) - -# define CALL_FREEFUN(h, old_chunk) \ - do { \ - if ((h)->use_extra_arg) \ - (*(h)->freefun)((h)->extra_arg, (old_chunk)); \ - else \ - (*(void (*)(void *))(h)->freefun)((old_chunk)); \ - } while (0) +/* Call functions with either the traditional malloc/free calling + interface, or the mmalloc/mfree interface (that adds an extra first + argument), based on the value of use_extra_arg. */ + +static void * +call_chunkfun (struct obstack *h, size_t size) +{ + if (h->use_extra_arg) + return h->chunkfun.extra (h->extra_arg, size); + else + return h->chunkfun.plain (size); +} + +static void +call_freefun (struct obstack *h, void *old_chunk) +{ + if (h->use_extra_arg) + h->freefun.extra (h->extra_arg, old_chunk); + else + h->freefun.plain (old_chunk); +} /* Initialize an obstack H for use. Specify chunk size SIZE (0 means default). Objects start on multiples of ALIGNMENT (0 means use default). - CHUNKFUN is the function to use to allocate chunks, - and FREEFUN the function to free them. Return nonzero if successful, calls obstack_alloc_failed_handler if allocation fails. */ -typedef struct _obstack_chunk * (*chunkfun_type) (void *, size_t); -typedef void (*freefun_type) (void *, struct _obstack_chunk *); - static int _obstack_begin_worker (struct obstack *h, - _OBSTACK_SIZE_T size, _OBSTACK_SIZE_T alignment, - chunkfun_type chunkfun, freefun_type freefun) + _OBSTACK_SIZE_T size, _OBSTACK_SIZE_T alignment) { struct _obstack_chunk *chunk; /* points to new chunk */ @@ -133,12 +130,10 @@ _obstack_begin_worker (struct obstack *h, size = 4096 - extra; } - h->chunkfun = chunkfun; - h->freefun = freefun; h->chunk_size = size; h->alignment_mask = alignment - 1; - chunk = h->chunk = CALL_CHUNKFUN (h, h->chunk_size); + chunk = h->chunk = call_chunkfun (h, h->chunk_size); if (!chunk) (*obstack_alloc_failed_handler) (); h->next_free = h->object_base = __PTR_ALIGN ((char *) chunk, chunk->contents, @@ -157,10 +152,10 @@ _obstack_begin (struct obstack *h, void *(*chunkfun) (size_t), void (*freefun) (void *)) { + h->chunkfun.plain = chunkfun; + h->freefun.plain = freefun; h->use_extra_arg = 0; - return _obstack_begin_worker (h, size, alignment, - (chunkfun_type) chunkfun, - (freefun_type) freefun); + return _obstack_begin_worker (h, size, alignment); } int @@ -170,11 +165,11 @@ _obstack_begin_1 (struct obstack *h, void (*freefun) (void *, void *), void *arg) { + h->chunkfun.extra = chunkfun; + h->freefun.extra = freefun; h->extra_arg = arg; h->use_extra_arg = 1; - return _obstack_begin_worker (h, size, alignment, - (chunkfun_type) chunkfun, - (freefun_type) freefun); + return _obstack_begin_worker (h, size, alignment); } /* Allocate a new current chunk for the obstack *H @@ -202,7 +197,7 @@ _obstack_newchunk (struct obstack *h, _OBSTACK_SIZE_T length) /* Allocate and initialize the new chunk. */ if (obj_size <= sum1 && sum1 <= sum2) - new_chunk = CALL_CHUNKFUN (h, new_size); + new_chunk = call_chunkfun (h, new_size); if (!new_chunk) (*obstack_alloc_failed_handler)(); h->chunk = new_chunk; @@ -225,7 +220,7 @@ _obstack_newchunk (struct obstack *h, _OBSTACK_SIZE_T length) h->alignment_mask))) { new_chunk->prev = old_chunk->prev; - CALL_FREEFUN (h, old_chunk); + call_freefun (h, old_chunk); } h->object_base = object_base; @@ -276,7 +271,7 @@ _obstack_free (struct obstack *h, void *obj) while (lp != 0 && ((void *) lp >= obj || (void *) (lp)->limit < obj)) { plp = lp->prev; - CALL_FREEFUN (h, lp); + call_freefun (h, lp); lp = plp; /* If we switch chunks, we can't tell whether the new current chunk contains an empty object, so assume that it may. */ diff --git a/lib/obstack.h b/lib/obstack.h index 61b824b..7b6dee8 100644 --- a/lib/obstack.h +++ b/lib/obstack.h @@ -116,10 +116,12 @@ and "long" for these two types. */ # define _OBSTACK_SIZE_T unsigned int # define _CHUNK_SIZE_T unsigned long +# define _OBSTACK_CAST(type, expr) ((type) (expr)) #else /* Version 2 with sane types, especially for 64-bit hosts. */ # define _OBSTACK_SIZE_T size_t # define _CHUNK_SIZE_T size_t +# define _OBSTACK_CAST(type, expr) (expr) #endif /* If B is the base of an object addressed by P, return the result of @@ -167,11 +169,19 @@ struct obstack /* control current object in current chunk */ void *p; } temp; /* Temporary for some macros. */ _OBSTACK_SIZE_T alignment_mask; /* Mask of alignment for each object. */ - /* These prototypes vary based on 'use_extra_arg', and we use - casts to the prototypeless function type in all assignments, - but having prototypes here quiets -Wstrict-prototypes. */ - struct _obstack_chunk *(*chunkfun) (void *, size_t); - void (*freefun) (void *, struct _obstack_chunk *); + + /* These prototypes vary based on 'use_extra_arg'. */ + union + { + void *(*plain) (size_t); + void *(*extra) (void *, size_t); + } chunkfun; + union + { + void (*plain) (void *); + void (*extra) (void *, void *); + } freefun; + void *extra_arg; /* first arg for chunk alloc/dealloc funcs */ unsigned use_extra_arg : 1; /* chunk alloc/dealloc funcs take extra arg */ unsigned maybe_empty_object : 1; /* There is a possibility that the current @@ -189,11 +199,11 @@ extern void _obstack_newchunk (struct obstack *, _OBSTACK_SIZE_T); extern void _obstack_free (struct obstack *, void *); extern int _obstack_begin (struct obstack *, _OBSTACK_SIZE_T, _OBSTACK_SIZE_T, - void *(*)(size_t), void (*)(void *)); + void *(*) (size_t), void (*) (void *)); extern int _obstack_begin_1 (struct obstack *, _OBSTACK_SIZE_T, _OBSTACK_SIZE_T, - void *(*)(void *, size_t), - void (*)(void *, void *), void *); + void *(*) (void *, size_t), + void (*) (void *, void *), void *); extern _OBSTACK_SIZE_T _obstack_memory_used (struct obstack *) __attribute_pure__; @@ -228,29 +238,31 @@ extern int obstack_exit_failure; /* To prevent prototype warnings provide complete argument list. */ #define obstack_init(h) \ _obstack_begin ((h), 0, 0, \ - (void *(*)(size_t))obstack_chunk_alloc, \ - (void (*)(void *))obstack_chunk_free) + _OBSTACK_CAST (void *(*) (size_t), obstack_chunk_alloc), \ + _OBSTACK_CAST (void (*) (void *), obstack_chunk_free)) #define obstack_begin(h, size) \ _obstack_begin ((h), (size), 0, \ - (void *(*)(size_t))obstack_chunk_alloc, \ - (void (*)(void *))obstack_chunk_free) + _OBSTACK_CAST (void *(*) (size_t), obstack_chunk_alloc), \ + _OBSTACK_CAST (void (*) (void *), obstack_chunk_free)) #define obstack_specify_allocation(h, size, alignment, chunkfun, freefun) \ _obstack_begin ((h), (size), (alignment), \ - (void *(*)(size_t))(chunkfun), \ - (void (*)(void *))(freefun)) + _OBSTACK_CAST (void *(*) (size_t), chunkfun), \ + _OBSTACK_CAST (void (*) (void *), freefun)) #define obstack_specify_allocation_with_arg(h, size, alignment, chunkfun, freefun, arg) \ _obstack_begin_1 ((h), (size), (alignment), \ - (void *(*)(void *, size_t))(chunkfun), \ - (void (*)(void *, void *))(freefun), (arg)) + _OBSTACK_CAST (void *(*) (void *, size_t), chunkfun), \ + _OBSTACK_CAST (void (*) (void *, void *), freefun), arg) #define obstack_chunkfun(h, newchunkfun) \ - ((h)->chunkfun = (struct _obstack_chunk *(*)(void *, size_t))(newchunkfun)) + ((void) ((h)->chunkfun.extra = _OBSTACK_CAST (void *(*) (void *, size_t), \ + newchunkfun))) #define obstack_freefun(h, newfreefun) \ - ((h)->freefun = (void (*)(void *, struct _obstack_chunk *))(newfreefun)) + ((void) ((h)->freefun.extra = _OBSTACK_CAST (void (*) (void *, void *), \ + newfreefun))) #define obstack_1grow_fast(h, achar) ((void) (*((h)->next_free)++ = (achar))) -- 1.9.3