Re: malloca, freea are not thread-safe

2018-02-04 Thread Paul Eggert
Florian Weimer wrote: -fcheck-pointer-bounds in GCC doesn't really work. I agree that -fcheck-pointer-bounds is not suitable for production code. However, I am not as skeptical about its utility. I found a real bug in GNU Emacs with it, a bug that was not found with other checkers. So I

Re: malloca, freea are not thread-safe

2018-02-04 Thread Paul Eggert
Bruno Haible wrote: Hi Paul, By the way, why write "if __GNUC__ >= 5 && !defined __cplusplus && !defined __clang__" instead of "ifdef __CHKP__"? The latter is easier to read I couldn't find any documentation for this __CHKP__ macro. It's not documented in the GCC manual. I found

Re: malloca, freea are not thread-safe

2018-02-03 Thread Bruno Haible
Hello Florian, > -fcheck-pointer-bounds in GCC doesn't really work. The existing > implementation is barely a research prototype (for example, most string > functions are not protected by it) Thanks for the info. I was just setting up a build of gnulib with CC="gcc -mmpx

Re: malloca, freea are not thread-safe

2018-02-03 Thread Florian Weimer
On 02/02/2018 11:10 PM, Paul Eggert wrote: This can cause problems when -fcheck-pointer-bounds is in effect, since converting a pointer to uintptr_t and back means that GCC won't connect the resulting pointer to the original and this messes up bounds checking on the result.

Re: malloca, freea are not thread-safe

2018-02-02 Thread Bruno Haible
Hi Paul, > By the way, why write "if __GNUC__ >= 5 && !defined __cplusplus && > !defined __clang__" instead of "ifdef __CHKP__"? The latter is easier to > read I couldn't find any documentation for this __CHKP__ macro. > An advantage of the #ifdef __CHKP__ code I suggested is > that it

Re: malloca, freea are not thread-safe

2018-02-02 Thread Paul Eggert
On 02/02/2018 04:03 PM, Bruno Haible wrote: What do you mean by "cause problems" and "messes up bounds checking"? As far as I understand, it will disable bounds checking on the returned pointer and its derivatives, right? I'm operating from memory here (my work desktop doesn't have MPX, nor do

Re: malloca, freea are not thread-safe

2018-02-02 Thread Bruno Haible
Hi Paul, > This can cause problems when -fcheck-pointer-bounds is in effect, since > converting a pointer to uintptr_t and back means that GCC won't connect > the resulting pointer to the original and this messes up bounds checking > on the result. To be precise: What do you mean by "cause

Re: malloca, freea are not thread-safe

2018-02-02 Thread Paul Eggert
On 02/02/2018 03:41 PM, Bruno Haible wrote: Regarding the parentheses, I disagree: If we put parentheses they should be like this: size_t nplus = (n + sizeof (small_t)) + (2 * sa_alignment_max - 1); because we want n + sizeof (small_t) consecutive bytes in memory, and the other summand is

Re: malloca, freea are not thread-safe

2018-02-02 Thread Bruno Haible
Hi Paul, > > ! /* Type for holding very small pointer differences. */ > > ! typedef unsigned char small_t; > There should be a compile-time check guaranteeing that small_t is wide > enough. OK, if you like. Applied: 2018-02-02 Paul Eggert malloca: Add a

Re: malloca, freea are not thread-safe

2018-02-02 Thread Bruno Haible
Paul Eggert wrote: > > ! size_t nplus = n + sizeof (small_t) + 2 * sa_alignment_max - 1; > For expressions like these, it's a bit better to parenthesize the value > added to N, mostly because it makes it clearer to the reader that we're > just adding a constant. Also, on (admittedly-weird)

Re: malloca, freea are not thread-safe

2018-02-02 Thread Bruno Haible
Hi Paul, > > ! void > >freea (void *p) > >{ > > ! /* Determine whether p was a non-NULL pointer returned by mmalloca(). > > */ > > ! if ((uintptr_t) p & sa_alignment_max) > > This should be "((uintptr_t) p & (2 * sa_alignment_max - 1))", to make > it more likely that a runtime

Re: malloca, freea are not thread-safe

2018-02-02 Thread Paul Eggert
On 02/02/2018 10:35 AM, Bruno Haible wrote: Done: Thanks. Some comments, with a proposed patch attached: # define malloca(N) \ - ((N) < 4032 - sa_increment\ - ? (void *) ((char *) alloca ((N) + sa_increment) + sa_increment) \ + ((N) < 4032 - (2 *

Re: malloca, freea are not thread-safe

2018-02-02 Thread Bruno Haible
Hi Florian, > Could we please fix this issue along those lines? Thanks. Done: 2018-02-02 Bruno Haible malloca, xmalloca: Make multithread-safe. Reported by Florian Weimer . Implements an idea by Ondřej Bílka .

Re: malloca, freea are not thread-safe

2018-02-01 Thread Florian Weimer
On 01/11/2018 05:26 AM, Bruno Haible wrote: Another idea is to add some header (like the current implementation does), but instead of storing a marker only in the malloc case, store a different marker also in the alloca case. This should be done through a GCC statement expression. => Should work

Re: malloca, freea are not thread-safe

2018-01-10 Thread Bruno Haible
Florian Weimer wrote: > freea also causes valgrind warnings because it contains an out-of-bounds > access. This is very undesirable because it will cause programmers to > miss real bugs. As a mitigation, the 'malloca' module contains a file, malloca.valgrind, that tells valgrind to ignore

Re: malloca, freea are not thread-safe

2018-01-10 Thread Bruno Haible
Ondřej Bílka wrote: > This could be done faster without hash table by making alloca result > aligned to 2 * S and malloc ones not aligned to 2 * S by adding some padding. Nice trick. This can be done without violating the rules of how alloca() is used. A similar idea, that also consists in

Re: malloca, freea are not thread-safe

2018-01-10 Thread Ondřej Bílka
On Wed, Jan 10, 2018 at 03:02:17PM +0100, Florian Weimer wrote: > The mmalloca function used to implement malloca accesses a static > global array without synchronization: > > #define HASH_TABLE_SIZE 257 > static void * mmalloca_results[HASH_TABLE_SIZE]; > … > mmalloca (size_t n) > { > … > /*

malloca, freea are not thread-safe

2018-01-10 Thread Florian Weimer
The mmalloca function used to implement malloca accesses a static global array without synchronization: #define HASH_TABLE_SIZE 257 static void * mmalloca_results[HASH_TABLE_SIZE]; … mmalloca (size_t n) { … /* Enter p into the hash table. */ slot = (uintptr_t) p %