Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Rich Felker]

On Fri, Aug 12, 2011 at 04:45:58AM +0200, Luka Marčetić wrote:
> Hey.
> As I've explained in the recent e-mail to Alexander, I couldn't get
> alloc.c  to work with musl, so I switched to pthread_eintr, but ran
> into a problem there too.

So we're on the same page, is the latest version in git what you're
looking at? I'll take a look..

> Rich, help me out please with the question from IRC: What's up with
> the line from pthread_eintr.c looking like this:
>     pthread_cleanup_push(child_wait_vp, NULL);
> Both musl and glibc macros generate invalid code for this one, it
> ends with `do {;` in both cases iirc. Strange - what is it?
> Thank you,
> Luka

You might want to read this:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_cleanup_pop.html

There's even a sample implementation in the rationale.

Rich

cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

Hey.
As I've explained in the recent e-mail to Alexander, I couldn't get 
alloc.c  to work with musl, so I switched to pthread_eintr, but ran into 
a problem there too.
Rich, help me out please with the question from IRC: What's up with the 
line from pthread_eintr.c looking like this:
     pthread_cleanup_push(child_wait_vp, NULL);
Both musl and glibc macros generate invalid code for this one, it ends 
with `do {;` in both cases iirc. Strange - what is it?
Thank you,
Luka

P.S. Oh yes, priorities are pthread_eintr, and to start working on one 
of the other 2 tasks.

[-- Attachment #2: to-report --]
[-- Type: text/plain, Size: 209 bytes --]

 a.out           |binary
 alloc.c         |   53 ++++++++++++++++---------
 pthread_eintr.c |  117 +++++++++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 124 insertions(+), 46 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: alloc.c --]
[-- Type: text/x-csrc; name="alloc.c", Size: 15937 bytes --]

#include <stdio.h>
#include <unistd.h>
#include <setjmp.h>
#include <stdlib.h>
#include <string.h>
#include <fcntl.h>
#include <sys/wait.h>
#include <sys/mman.h>
#include <stdint.h> //PTRDIFF_MAX, SIZE_MAX
#include <signal.h>
#include "common/common.h"

/*
 * Copyright (c) 2011 Luka Marčetić<paxcoder@gmail.com>
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted.
 *
 * There's ABSOLUTELY NO WARRANTY, express or implied.
 */

/**
 ** \file
 ** Tests that blocks allocated by various functions do not overlap, that they 
 ** are successfully writable, and that they are freeable by free().
 ** In case of posix_memalign, it allso tests that blocks are correctly aligned,
 ** and in the case of calloc, that allocation of huge values results in failure
 ** tests: malloc, realloc, calloc, posix_memalign
 ** depends: sysconf, free, fork,wait, rand, setjmp,longjmp, qsort, fprintf
 **          memcmp,memset, open,mmap
 **/

jmp_buf env;
struct {
    size_t
        page,
        prog,
        blocks;
} mem;
struct block {
    char *ptr;
    size_t size;
};
struct s_err {
    unsigned int
        alloc:1,
        overlapping:1,
        notr:1,
        notw:1,
        notsame:1,
        misaligned:1,
        ov_size_t:1,
        ov_ptrdiff_t:1,
        free:1,
        mallocfree:1,
        rand:1,
        limit:1,
        no;
} *err;

#define TRY_FREE(PTR) { err->free = 0; free(PTR); err->free = 0; }
static int vm_limit(const size_t);
static size_t posix_memalign_alignment(size_t);
static size_t blocks_alloc(const void *, struct block *, size_t);
static size_t blocks_overlapping(struct block *, size_t);
static void blocks_sort(struct block *, size_t);
static int blocks_sort_compar(const void *, const void *);
static int block_misaligned(struct block, size_t);
static int blocks_misaligned(struct block *, size_t);
static int blocks_notrw(struct block *, size_t);
static int calloc_overflows(long long unsigned int);
static void bridge_sig_jmp(int);

int main()
{
    struct block *b;
    size_t       i, j, nr_blocks;
    int          ret, stat;
    const void   *fun[]      = { malloc,  realloc,  calloc,  posix_memalign };
    const char   *fun_name[] = {"malloc","realloc","calloc","posix_memalign"};
    const int    fd          = open("/dev/zero", O_RDWR);
    char         *s;
    
    mem.page   = sysconf(_SC_PAGE_SIZE);
    mem.blocks = 1000;                  //max. nr. of blocks to be generated
    mem.prog   = mem.blocks*mem.page;   //memory reserved for dynamic allocation
    b = malloc(mem.blocks*sizeof(struct block));
    err = mmap(NULL,sizeof(struct s_err),PROT_READ|PROT_WRITE, MAP_SHARED,fd,0);
    
    ret = 0;
    for (i=0; i<sizeof(fun)/sizeof(*fun); ++i) {
        memset(err, 0, sizeof(*err)); //all zeros = no errors
        if (!fork()){
            if (fun[i] == calloc) {
                if(calloc_overflows(SIZE_MAX))
                    err->ov_size_t = 1;
                if(calloc_overflows(PTRDIFF_MAX))
                    err->ov_ptrdiff_t = 1;
            }
            
            if (!vm_limit(mem.prog)) {
                err->limit = 1;
                free(b);
                return 0; //!!!
            }
            
            if (fun[i] == realloc) {
                nr_blocks = blocks_alloc(malloc, b, mem.blocks);
                if (nr_blocks > 5) {
                    //free some for subsequent realloc
                    err->mallocfree = 1;
                    for(j = nr_blocks; nr_blocks >= j-5; --nr_blocks)
                        free(b[nr_blocks-1].ptr);
                    err->mallocfree = 0;
                    nr_blocks = blocks_alloc(fun[i], b, nr_blocks);
                }
            }
            else
                nr_blocks = blocks_alloc(fun[i], b, mem.blocks);
            printf("%zu: %zu\n", i, nr_blocks); //---
            
            if (nr_blocks < 10) //an imposed minimum
                err->alloc = 1;
            else {
                if (fun[i]==posix_memalign && blocks_misaligned(b, nr_blocks))
                    err->misaligned = 1;
                //universal:
                if (blocks_overlapping(b, nr_blocks))
                    err->overlapping = 1;
                switch (blocks_notrw(b, nr_blocks)) {
                    case 1: err->notw    = 1; break;
                    case 2: err->notr    = 1; break;
                    case 3: err->notsame = 1; break;
                } 
            }
            for(j=0; j<nr_blocks; ++j)
                TRY_FREE(b[j].ptr);
            free(b);
            return 0;
        }
        
        wait(&stat);
        if (!WIFEXITED(stat) && !err->free) {
            ++ret;
            fprintf(stderr, "%s test quit unexpectedly!\n", fun_name[i]);
            if (WIFSIGNALED(stat))
                fprintf(stderr, "\tTerminating signal: %d\n", WTERMSIG(stat));
        }else {
            ret += (memcmp(err,(struct s_err []){{.free=0}},sizeof(*err)) != 0);
            if (err->rand)
                fprintf(stderr,
                       "An unlikely situation occurred while generating size"
                       " arguments for %s: rand()%%(%zu)+1 on average resulted"
                       " in values no greater than 10%% of the possible maximum"
                       " value. Subsequent errors may result from this.\n",
                       fun_name[i],
                       mem.page*10
                );
            if (err->limit)
                fprintf(stderr,
                        "Initial virtual memory allocation failed."
                        "%s tests will not run\n", fun_name[i]
                );
            if (err->alloc)
                fprintf(
                    stderr,
                    "%s failed to allocate sufficient amount of memory required"
                    " for the test\n",
                    fun_name[i]
                );
            if (err->overlapping)
               fprintf(stderr,"%s allocated blocks that overlap\n",fun_name[i]);
            if (err->notr)
               fprintf(stderr,"%s 'allocated' unreadable memory\n",fun_name[i]);
            if (err->notw)
               fprintf(stderr,"%s 'allocated' unwritable memory\n",fun_name[i]);
            if (err->notsame)
                fprintf(stderr,
                        "a byte read from %s-allocated memory does not"
                        " match the one previously writen there\n",
                        fun_name[i]
                );
            if (err->misaligned)
                fprintf(
                    stderr, "%s allocated at least one misaligned block\n",
                    fun_name[i]
                );
            if (err->mallocfree)
                fprintf(
                    stderr,
                    "%s can't be tested: free() failed to free memory allocated"
                    " by malloc(), which was required for the test to run\n",
                    fun_name[i]
                );
            if (err->free)
                fprintf(
                    stderr, "free() failed to free memory allocated by %s\n",
                    fun_name[i]
                );
            if (err->ov_size_t)
                fprintf(
                    stderr,
                    "%s tried to allocate more than SIZE_MAX bytes, which"
                    " means the block's size would not be representable"
                    " by a size_t type\n",
                    fun_name[i]
                );
            if (err->ov_ptrdiff_t)
                fprintf(
                    stderr,
                    "%s tried to allocate more than PTRDIFF_MAX bytes, which"
                    " would cause an overflow of ptrdiff_t when subtracting"
                    " two pointers from the opposite ends of the block\n",
                    fun_name[i]
                );
            if (err->no) {
                fprintf(stderr, "%s test set errno=%s\n",
                        fun_name[i], s = e_name(err->no));
                free(s);
            }
        }
    }
    
    free(b);
    return ret;
}

/**
 ** Allocates all remaining virtual memory, leaving limit both for brk and mmap
 ** \limit a desired number of free bytes upon returning from the function
 ** \returns 1 on success, 0 on failure
 **/
static int vm_limit(const size_t limit) {
    const int fd = open("/dev/zero", O_RDWR);
    size_t i,j,last; 
    void *mmapd, *vp;
    void **brkd, **vpp;
    
    //allocate mmap area of size limit:
    mmapd = mmap(NULL, limit, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
    //allocate brk area of size limit:
    brkd = (vpp = malloc(mem.page));
    for(i=0;  vpp!=NULL && (i+=mem.page)<limit;  vpp = *vpp = malloc(mem.page));
    
    if (mmapd != MAP_FAILED && i>=limit) {
        //repeated mmaps of binary-search-determined sizes to fill everything:
        do {
            last = 0;
            for (i=j=SIZE_MAX/2+1;  i>0;  i/=2) {
                if ((vp=mmap(NULL, j,PROT_NONE,MAP_PRIVATE,fd,0)) == MAP_FAILED)
                    j -= i/2;
                else {
                    munmap(vp, (last=j));
                    j += i/2;
                }
            }
        }while(last && mmap(NULL, last,PROT_NONE,MAP_PRIVATE,fd,0)!=MAP_FAILED);
        
        //free that mmap area:
        munmap(mmapd, limit);
        //free that brk area:
        for(i=0; i<limit; i+=mem.page) {
            vpp = *brkd;
            free(brkd);
            brkd = vpp;
        }
    }
    
    return (mmapd != MAP_FAILED && i>=limit && !last);
}

/**
 ** (re)Allocates blocks of random sizes to fill memory, saves info about them
 ** NOTE: Function not stand-alone, requires initialized *err and mem structures
 ** \param fun a pointer to the function to call to (re)allocate the blocks
 ** \param b a pointer to a struct block array to be filled with blocks info
 ** \param n for realloc: a number of blocks that are allocated (<=mem.blocks)
 ** \returns the number of blocks successfully allocated
 **/
static size_t blocks_alloc(const void *fun, struct block *b, size_t n)
{
    long int rndm;
    size_t  i, size, nr_blocks;
    char *ptr;
    int error;
    
    nr_blocks = error = 0;
    for (i=0; !error &&  i<mem.blocks; ++i) {
        ptr = NULL;
        rndm = random(); 
        size = rndm % (mem.page*10) +1; //see error report for err->rand
        
        if (fun == malloc)
            ptr = malloc(size);
        else if (fun == calloc)
            ptr = calloc(1, size);
        else if (fun == posix_memalign)
            error=posix_memalign((void**)&ptr,posix_memalign_alignment(i),size);
        else if (fun == realloc) {
            if (i<n)
                ptr = realloc(b[nr_blocks].ptr, size);
            else
                ptr = malloc(size);
        }
        
        if (ptr != NULL && (fun != posix_memalign || error == 0)) {
            if (fun != realloc && rndm % 10) {
                TRY_FREE(ptr);
                --i;
            }else{
                b[nr_blocks].ptr  = ptr;
                b[nr_blocks].size = size;
                ++nr_blocks;
            }
        }else{
            if (fun != posix_memalign)
               error = errno;
            --i;
        }
    }
    if (error != ENOMEM) {
        if (i==mem.blocks)
            err->rand = 1;
        else
            err->no = (unsigned int)error;
        
        for (i=0; i<nr_blocks; ++i)
            TRY_FREE(b[i].ptr);
        nr_blocks = 0; //will set err->alloc in main()
    }
    return nr_blocks;
}
/**
 ** Generates a value to be used as a 2nd argument(alignment) to posix_memalign
 ** \param seed - required for the calculation (index of the block element)
 ** \returns a power of two value that is a multiple of sizeof(void *)
 **/
static size_t posix_memalign_alignment(size_t seed)
{
    do{
        ++seed;
    }while(seed%sizeof(void*) || (seed/sizeof(void*) & (seed/sizeof(void*)-1)));
    return seed;
}

/**
 ** Searches for overlapping blocks
 ** \param b a pointer to an array of type struct block
 ** \param n number of elements in the array to search through (will be sorted!)
 ** \returns an index to b of an element whose .ptr points to an address in the
 **          preceeding block's space[ptr, ptr+size], or 0 if none such is found
 **/
static size_t blocks_overlapping(struct block *b, size_t n)
{
    blocks_sort(b, n);
    for (size_t i=1; i<n; ++i)
        if(b[i].ptr < b[i-1].ptr+b[i-1].size)
            return i;
    return 0;
}
/**
 ** Sorts the array of type struct block
 ** \parm b a pointer to the array
 ** \param n the number of array elements to sort
 **/
static void blocks_sort(struct block *b, size_t n)
{
    qsort(b, n, sizeof(b[0]), blocks_sort_compar);
}
//A comparison function used by qsort() in blocks_sort()
static int blocks_sort_compar(const void *v1, const void *v2)
{
    struct block b1 = *(struct block *)v1,
                 b2 = *(struct block *)v2;
    if (b1.ptr < b2.ptr)
        return -1;
    return (b1.ptr > b2.ptr);
}

/**
 ** Check blocks for read/write-ability (+consistence)
 ** \param b a pointer to an array of type struct block
 ** \param n the number of array elements to check
 ** \returns
 **         0 - if all blocks are readable and writable
 **         1 - if at least one of the blocks isn't writable
 **         2 - if at least one of the blocks isn't readable
 **         3 - if a byte read didn't match a byte previously written
 **/
static int blocks_notrw(struct block *b, size_t n)
{
    size_t i;
    struct sigaction
        oldact,
        act = {.sa_handler = bridge_sig_jmp, .sa_flags = SA_NODEFER};
    int ret = 0;
    
    sigaction(SIGSEGV, &act, &oldact);
    for (i=0; !ret && i<n; ++i) {
        if(!setjmp(env)) {
            b[i].ptr[0] = '\r';
            b[i].ptr[b[i].size/2] = '\r';
            b[i].ptr[b[i].size-1] = '\r';
            if(!setjmp(env)) {
                if(b[i].ptr[0] != '\r'
                   || b[i].ptr[b[i].size/2] != '\r'
                   || b[i].ptr[b[i].size-1] != '\r'
                )
                    ret = 3;
            }else
                ret = 2;
        }else
            ret = 1;
    }
    sigaction(SIGSEGV, &oldact, NULL);
    return ret;
}


/**
 ** Check that a block (allocated by posix_memalign) is correctly aligned
 ** \param b1 a block to check (type struct block)
 ** \param alignment value of the 2nd argument that was passed to posix_memalign
 ** \returns 1 if the block is incorrectly aligned, otherwise 0
 **/
static int block_misaligned(struct block b1, size_t alignment)
{
    return ((size_t)b1.ptr % alignment != 0);
}
/** Does for an array what block_misaligned does for a single block
 ** \note the function assumes that posix_memalign_alignment() generated  the 
 ** alignment value, seeded with an index of each element. Likewise, it assumes
 ** that assuming b[0] is truly the first element of the array
 ** \param b a pointer to the first(!) element of an array of type struct block
 ** \param n the number of elements in the array to check
 ** \returns 1 if any of the calls to block_misaligned() returns 1, otherwise 0
 **/
static int blocks_misaligned(struct block *b, size_t n)
{
    for(size_t i=0; i<n; ++i)
        if (block_misaligned(b[i], posix_memalign_alignment(i)))
            return 1;
    return 0;
}

/**
 ** Calls calloc with arguments whose product is greater than limit, to see if
 ** it returns a non-NULL pointer(ie allocates space, in which case it is freed)
 ** \param limit when incremented by 1, a value for which allocation should fail
 ** \returns 1 if calloc returns a non-NULL (incorrect behavior), otherwise 0
 **/
static int calloc_overflows(long long unsigned int limit)
{
    void *vp;
    
    vp = calloc(8, limit/8 + 1);
    free(vp);
    return (vp != NULL);
}

//A bridge function for sigaction, that longjmp's and restores env
static void bridge_sig_jmp(int sig)
{
    longjmp(env, sig);
    return;
}

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: pthread_eintr.c --]
[-- Type: text/x-csrc; name="pthread_eintr.c", Size: 6631 bytes --]

#include <stdio.h>
#include <signal.h>
#include <unistd.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <sys/wait.h>
#include <signal.h>
#include <setjmp.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <time.h>
#include <sched.h>
#include "common/common.h"
#include <pthread.h>

/*
 * Copyright (c) 2011 Luka Marčetić<paxcoder@gmail.com>
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted.
 *
 * There's ABSOLUTELY NO WARRANTY, express or implied.
 */
 
/**
 ** \file
 ** Tests that EINTR isn't returned by interrupted pthread_* functions for which
 ** this is a specified behavior
 ** tests: pthread_create, 
 ** depends: open,pipe,close, mmap, sigaction, kill,waitpid,nanosleep, fprintf,
 ** setjmp, sched_yield
 **/

#define NANOSLEEP_MAX 100
unsigned char *exit_child; //a barrier for child processes

//signal handler that just returns
static void handler(int sig)
{
    ++sig; // -Wunused-parameter
    return;
}

//waits for *exit_child to be set, then returns
static void child_wait(void)
{
    while(!*exit_child)
        sched_yield();
    return;
}
static void 
child_wait_vp(void* foo)
{
    child_wait();
    ++foo; // -Wunused-parameter
    return;
}
/**
 ** Executes a function whose pointer was passed to it, before exitting
 ** \param fun a void pointer to the function to execute, or NULL for none
 **/
static void* thread(void *fun)
{
    if(fun != NULL) {
        pthread_cleanup_push(child_wait_vp, NULL);
    }
    return NULL;
}


int main()
{
    char            *s;
    pid_t           pid;
    pthread_t       tid;
    pthread_key_t   tkey;
    struct timespec ns = {.tv_nsec=0};
    unsigned int    i, nr_fun, failed;
    const int       fd = open("/dev/zero", O_RDWR);
    int             sig, ret[2]={0,0}, stat, pfd[2];
    const char      *function[] = {
        "pthread_create",
        "pthread_cancel",
        "pthread_once",
        "pthread_setspecific",
        "pthread_key_delete",
        "pthread_join",
        "pthread_atfork",
        "pthread_sigmask",
        "pthread_equal",
        "pthread_setschedprio",
        "pthread_setconcurrency",
        "pthread_create",
        "pthread_setconcurrency",
        "pthread_detach",
        "pthread_sigmask",
        "pthread_setspecific",
        "pthread_key_create",
        "pthread_rwlock_unlock",
        "pthread_kill",
    };
    struct {
        unsigned int
            kill:1,
            wait:1;
    } err, no_err = {.wait=0};
    struct sigaction
        act = {.sa_handler=handler, .sa_flags=SA_NODEFER};
    
    exit_child = mmap(NULL,1,PROT_READ|PROT_WRITE,MAP_SHARED,fd,0);
    
    
    failed = 0;
    for (i=0;  i < (nr_fun=sizeof(function)/sizeof(*function));  ++i) {
        memset(&err, 0, sizeof(err));
        pipe(pfd);
        *exit_child = 0;
        if(!(pid = fork())) {
            sigaction(SIGABRT, &act, NULL);
            sig = 0;
            switch (i) {
                case 1:
                    pthread_create(&tid, NULL, thread, child_wait);
                break;
                case 3:
                case 4:
                    pthread_key_create(&tkey, child_wait_vp);
                break;
            }
            close(pfd[0]);
            close(pfd[1]); //ready
            switch (i) {
                case 0:
                    while(sig != EINTR && !*exit_child)
                        sig = pthread_create(&tid, NULL, thread, NULL);
                break;
                case 1:
                    while(sig != EINTR && !*exit_child)
                        sig = pthread_cancel(tid);
                break;
                case 2:
                    sig = pthread_once((pthread_once_t []){ PTHREAD_ONCE_INIT },
                                       child_wait);
                break;
                case 3:
                    while(sig != EINTR && !*exit_child)
                        sig = pthread_setspecific(tkey, NULL);
                /*no break;*/
                case 4: 
                    pthread_key_delete(tkey);
                break;
                default:
                    fprintf(stdout, "BUG: test i=%d missing!\n", i);
                break;
            }
            return (sig == EINTR);
        }
        
        if (!pid) {
            fprintf(stdout, "BUG: child i=%d escaped!\n", i);
            exit(0); //exit/contain child
        }else if (pid == -1) {
            fprintf(stderr,
                "A call to fork() nr %d/%d yeilded -1 (errno=%s)!\n",
                i+1, nr_fun, (s = e_name(errno))
            );
            free(s);
            close (pfd[0]);
            close (pfd[1]);
            failed += 1;
        }else{
            close (pfd[1]);
            read(pfd[0], NULL, 1); //wait for the child to become ready
            close (pfd[0]);
            
            //Send signals in intervals of <NANOSLEEP_MAX,0> microseconds:
            ns.tv_nsec = NANOSLEEP_MAX;
            while(
                (ret[0] = kill(pid, SIGABRT)) == 0
                && (ret[1] = waitpid(pid, &stat, WNOHANG)) == 0
                && --ns.tv_nsec
            )
                nanosleep(&ns, NULL);
            *exit_child=1; //let the child loose from its loop (if any)
            
            //Process errors:
            s = e_name(errno);
            err.kill = (ret[0] != 0);
            err.wait = (ret[1] != 0 && ret[1] != pid);
            if (!(!memcmp(&err, &no_err, sizeof(err)))) {
                ++failed;
                if (err.kill)
                    fprintf(stderr,
                            "A call to kill() returned %d, errno=%s\n",
                            ret[0], s
                    );
                if (err.wait)
                    fprintf(stderr,
                            "A call to waitpid() returned %d, errno=%s\n",
                            ret[1], s
                    );
            }else if (WIFEXITED(stat)) {
                *ret = WEXITSTATUS(stat); //repurpose ret[0]
                if (*ret == 1) {
                    ++failed;
                    fprintf(stderr, "%s() returned EINTR\n", function[i]);
                }
                else if (*ret != 0)
                    fprintf(stdout, "BUG: child nr %d ended with status %d!\n",
                            i, *ret
                    );
            }else{
                fprintf(stderr, "%s()'s process crashed!\n", function[i]);
                if (WIFSIGNALED(stat))
                    fprintf(stderr,"\tTerminating signal: %d\n",WTERMSIG(stat));
            }
            free(s);
        }
    }
    return failed;
}

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Rich Felker]

On Fri, Aug 12, 2011 at 04:45:58AM +0200, Luka Marčetić wrote:
> static void 
> child_wait_vp(void* foo)
> {
>     child_wait();
>     ++foo; // -Wunused-parameter
>     return;
> }

This is invalid C. You cannot do arithmetic on void pointers. I would
really recommend -Wno-unused-parameter, as unused parameters are NOT a
sign of bad code, but a fundamental part of using function pointers.

> /**
>  ** Executes a function whose pointer was passed to it, before exitting
>  ** \param fun a void pointer to the function to execute, or NULL for none
>  **/
> static void* thread(void *fun)
> {
>     if(fun != NULL) {
>         pthread_cleanup_push(child_wait_vp, NULL);
>     }
>     return NULL;
> }

pthread_cleanup_push and pthread_cleanup_pop must come in pairs that
would be valid if they were replaced by { and }. You can't use one
alone.

Rich

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Rich Felker]

On Fri, Aug 12, 2011 at 04:45:58AM +0200, Luka Marčetić wrote:
> Hey.
> As I've explained in the recent e-mail to Alexander, I couldn't get
> alloc.c  to work with musl, so I switched to pthread_eintr, but ran
> into a problem there too.

Please tell me what's not working for you. I tested the attached
alloc.c and it seems to work. I also examined the "maps" file in proc
after vm_limit, and it looks exactly right. I tried both with and
without ASLR enabled, no problem either way. My machines are 32-bit so
I have not tested on 64-bit. If it's failing for you, please tell me
some specifics about your test environment and show me a copy of the
/proc/self/maps after vm_limit.

Rich

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Solar Designer]

On Thu, Aug 11, 2011 at 10:47:25PM -0400, Rich Felker wrote:
> On Fri, Aug 12, 2011 at 04:45:58AM +0200, Luka Mar??eti?? wrote:
> > static void 
> > child_wait_vp(void* foo)
> > {
> >     child_wait();
> >     ++foo; // -Wunused-parameter
> >     return;
> > }
> 
> This is invalid C. You cannot do arithmetic on void pointers.

Right.  It's a GNU extension that we don't want to depend on.

> I would
> really recommend -Wno-unused-parameter, as unused parameters are NOT a
> sign of bad code, but a fundamental part of using function pointers.

Rather than disable the warning, Luka can do:

	(void) foo;

which is portable, unlike "++foo", and it silences the gcc warning too.

In some cases, unused parameters do in fact indicate programming errors
(such as a typo that resulted in another variable being used), so the
warning is somewhat useful.

Alexander

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Rich Felker]

On Fri, Aug 12, 2011 at 05:40:26PM +0200, Luka Marčetić wrote:
> >>Both musl and glibc macros generate invalid code for this one, it
> >>ends with `do {;` in both cases iirc. Strange - what is it?
> >You might want to read this:
> >http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_cleanup_pop.html
> >
> >There's even a sample implementation in the rationale.
> >
> >Rich
> 
> What I read was:
> 
> "The thread exits (that is, calls/pthread_exit/() <http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html>)."
> 
> Then I clicked the link(http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html <http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html>),
> hoping to find this:
> 
> "An implicit call to/pthread_exit/() is made when a thread other
> than the thread in which/main/() was first invoked returns from the
> start routine that was used to create it."
> 
> And when I did, I've overlooked '{', expecting an '}'. I regard this
> as slight inconsistency in the standard. At least it's missing the
> word "explicitly", but I'd reword it altogether hehe.
> Anyway, I'll just cast the void* to a function pointer and call it directly.

I don't follow what you're saying in this email...

Rich

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]

On 08/12/2011 04:41 AM, Rich Felker wrote:
> On Fri, Aug 12, 2011 at 04:45:58AM +0200, Luka Marčetić wrote:
>> alloc.c
> So we're on the same page, is the latest version in git what you're
> looking at? I'll take a look..

No, it was attached to the mail. Will commit though.

>> Both musl and glibc macros generate invalid code for this one, it
>> ends with `do {;` in both cases iirc. Strange - what is it?
> You might want to read this:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_cleanup_pop.html
>
> There's even a sample implementation in the rationale.
>
> Rich

What I read was:

"The thread exits (that is, calls/pthread_exit/() 
<http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html>)."

Then I clicked the 
link(http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html 
<http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html>), 
hoping to find this:

"An implicit call to/pthread_exit/() is made when a thread other than 
the thread in which/main/() was first invoked returns from the start 
routine that was used to create it."

And when I did, I've overlooked '{', expecting an '}'. I regard this as 
slight inconsistency in the standard. At least it's missing the word 
"explicitly", but I'd reword it altogether hehe.
Anyway, I'll just cast the void* to a function pointer and call it directly.
Thanks.
Luka


[-- Attachment #2: Type: text/html, Size: 3703 bytes --]

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Luka Marčetić]

On 08/12/2011 05:13 AM, Solar Designer wrote:
> Rather than disable the warning, Luka can do:
>
> 	(void) foo;
>
> which is portable, unlike "++foo", and it silences the gcc warning too

is this situation is why casting to void is allowed? I notice there's no 
"statement with no effect" warning.
I wasn't aware of this, so thanks.
Luka

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Rich Felker]

On Sat, Aug 13, 2011 at 08:49:43PM +0200, Luka Marčetić wrote:
> pthread_exit is allegedly called upon main thread function return,
> and it is specified to pop and execute pushed functions. I expected

pthread_exit is called implicitly upon the return from the start
function of any thread *except* the main thread. pthread_exit does pop
and execute cleanup handlers when it's called, but if it's called
implcitly due to return from the start function, there cannot be any
cleanup handlers left because execution has already left the scope of
any cleanup handler that may have been installed.

> this to happen. I was blaming the spec for not being more specific,
> and saying that pthread_exit should be explicitly called. But in
> reality, perhaps even an implicit call should work, in which case
> the spec isn't to blame, but both implementations instead?

Could you please explain what effect you're trying to achieve? Cleanup
handlers are rather useless unless you're using pthread_cancel.

Rich

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Luka Marčetić]

On 08/12/2011 05:39 PM, Rich Felker wrote:
> On Fri, Aug 12, 2011 at 05:40:26PM +0200, Luka Marčetić wrote:
>>>> Both musl and glibc macros generate invalid code for this one, it
>>>> ends with `do {;` in both cases iirc. Strange - what is it?
>>> You might want to read this:
>>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_cleanup_pop.html
>>>
>>> There's even a sample implementation in the rationale.
>>>
>>> Rich
>> What I read was:
>>
>> "The thread exits (that is, calls/pthread_exit/()<http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html>)."
>>
>> Then I clicked the link(http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html<http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html>),
>> hoping to find this:
>>
>> "An implicit call to/pthread_exit/() is made when a thread other
>> than the thread in which/main/() was first invoked returns from the
>> start routine that was used to create it."
>>
>> And when I did, I've overlooked '{', expecting an '}'. I regard this
>> as slight inconsistency in the standard. At least it's missing the
>> word "explicitly", but I'd reword it altogether hehe.
>> Anyway, I'll just cast the void* to a function pointer and call it directly.
> I don't follow what you're saying in this email...
>
> Rich

pthread_exit is allegedly called upon main thread function return, and 
it is specified to pop and execute pushed functions. I expected this to 
happen. I was blaming the spec for not being more specific, and saying 
that pthread_exit should be explicitly called. But in reality, perhaps 
even an implicit call should work, in which case the spec isn't to 
blame, but both implementations instead?
Luka

Re: cluts daily reports 8/12 - continuing pthread_eintr, still stuck with alloc

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]

On 08/13/2011 08:43 PM, Rich Felker wrote:
> Could you please explain what effect you're trying to achieve? Cleanup
> handlers are rather useless unless you're using pthread_cancel.
>
> Rich

I'm testing pthread_cancel of course. However, I have since* re-read the 
spec, and noticed it wouldn't block pthread_cancel anyway:
"The cancellation processing in the target thread shall run 
asynchronously with respect to the calling thread returning 
from/pthread_cancel/()."
So I'm looping it (will post the fresh pthread_eintr in a sec).
Luka

*The last mail is an old one, I just wasn't able to send it until now

[-- Attachment #2: Type: text/html, Size: 1639 bytes --]