From patchwork Mon Oct 5 21:12:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 40664 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 72D8B3857C67; Mon, 5 Oct 2020 21:12:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 72D8B3857C67 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1601932364; bh=wP7VDTQ7DUtNogUSicueM9gtW8ZbvWMC7Je1FazRodg=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=rM9ZB0r9b+aRcz6ox3hango+J8VoGloHVUQ+KRAzfQT4GGD3AoucVnVgUVMhEqsdA ZGTRhNlm9PYso4fISby+8pmjPbBFlQoznGJFGv/1budb8M32OFlkucFCVEt/p5wk1K Z7J/C1ZfX3Et/2a9wuPbFKeoFMec4eBPqECokFgw= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x842.google.com (mail-qt1-x842.google.com [IPv6:2607:f8b0:4864:20::842]) by sourceware.org (Postfix) with ESMTPS id C6FB33857C4E for ; Mon, 5 Oct 2020 21:12:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C6FB33857C4E Received: by mail-qt1-x842.google.com with SMTP id y11so10928191qtn.9 for ; Mon, 05 Oct 2020 14:12:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=wP7VDTQ7DUtNogUSicueM9gtW8ZbvWMC7Je1FazRodg=; b=EpoicpYHysVuSWtc4gk1TwdQzj7Gq8fGdiu46hFJBNAd3uz5tvEmYPWn/vrEO5bmFd R/zHtde8lXbJB95YRDqybjuxLpn3LZhjZqtrpw0aCp12N88P/8cR3ECjkQqmnV+w/8vU grJx246SvU6NkSGGDSmonYAY6O7QNik+gRKr7umRcnlq9OZzIeDS7DpfxRr+RWW2TpVw Fc9M9p2OMW3C98nH4QPCcihbrcA0FFzxfIj9t/xISmfcmZXlRgRI3LH73GWIDMLg9rUb sbm69s+ZMl76BAt78sV1Dzw5EgRm6ecAqx1BwP877uH439hLGgoVXbIdxM2MyzBMDjvo 5MZg== X-Gm-Message-State: AOAM531D14yuM2TKe5y4uQ9ggAEf4kmzB+P+8tXBNbAzS/gEU3L/M4hp oi9ULHCL7mgvCod3eamdXGI6D9DPQ0PyYw== X-Google-Smtp-Source: ABdhPJzPWbKRKugbj0X/EvzXjB2ZjoEYv7afNbbFfL17QBP9V8TLGw/obm2RnirLLMaco/vASl3DnA== X-Received: by 2002:ac8:70c3:: with SMTP id g3mr1917483qtp.253.1601932359684; Mon, 05 Oct 2020 14:12:39 -0700 (PDT) Received: from localhost.localdomain ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id j9sm794875qtk.89.2020.10.05.14.12.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Oct 2020 14:12:39 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH] posix: Fix -Warray-bounds instances building timer_create [BZ #26687] Date: Mon, 5 Oct 2020 18:12:33 -0300 Message-Id: <20201005211234.2240007-1-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Spam-Status: No, score=-13.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" GCC 11 -Warray-bounds triggers invalid warnings when building Linux timer_create.c: ../sysdeps/unix/sysv/linux/timer_create.c: In function ‘__timer_create_new’: ../sysdeps/unix/sysv/linux/timer_create.c:83:17: warning: array subscript ‘struct timer[0]’ is partly outside array bounds of ‘unsigned char[8]’ [-Warray-bounds] 83 | newp->sigev_notify = (evp != NULL | ^~ ../sysdeps/unix/sysv/linux/timer_create.c:59:47: note: referencing an object of size 8 allocated by ‘malloc’ 59 | struct timer *newp = (struct timer *) malloc (offsetof (struct timer, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 60 | thrfunc)); | ~~~~~~~~~ The struct allocated for !SIGEV_THREAD timers only requires two 'int' fields (sigev_notify and ktimerid) and the offsetof trick tries minimize the memory usage by only allocation the required size. However, although the resulting size is suffice for !SIGEV_THREAD time, accesing the partially allocated object is error-prone and UB. This patch fixes both issues by embedding the information whether the timer if a SIGEV_THREAD in the returned 'timer_t'. For !SIGEV_THREAD, the resulting 'timer_t' is the returned kernel timer identifer (kernel_timer_t), while for SIGEV_THREAD it uses the fact malloc returns at least _Alignof (max_align_t) pointers plus that valid kernel_timer_t are always positive to set MSB bit of the returned 'timer_t' to indicate the timer handles a SIGEV_THREAD. It allows to remove the memory allocation for !SIGEV_THREAD and also remove the 'sigev_notify' field from 'struct timer'. Checked on x86_64-linux-gnu and i686-linux-gnu. --- sysdeps/unix/sysv/linux/kernel-posix-timers.h | 46 +++++++++--- sysdeps/unix/sysv/linux/timer_create.c | 74 ++++++------------- sysdeps/unix/sysv/linux/timer_delete.c | 15 ++-- sysdeps/unix/sysv/linux/timer_getoverr.c | 8 +- sysdeps/unix/sysv/linux/timer_gettime.c | 6 +- sysdeps/unix/sysv/linux/timer_settime.c | 8 +- .../unix/sysv/linux/x86_64/timer_gettime.c | 4 +- .../unix/sysv/linux/x86_64/timer_settime.c | 5 +- 8 files changed, 77 insertions(+), 89 deletions(-) diff --git a/sysdeps/unix/sysv/linux/kernel-posix-timers.h b/sysdeps/unix/sysv/linux/kernel-posix-timers.h index 4f8d97d3de..2a5ee3d0fb 100644 --- a/sysdeps/unix/sysv/linux/kernel-posix-timers.h +++ b/sysdeps/unix/sysv/linux/kernel-posix-timers.h @@ -43,21 +43,11 @@ extern pthread_mutex_t __active_timer_sigev_thread_lock attribute_hidden; /* Type of timers in the kernel. */ typedef int kernel_timer_t; - -/* Internal representation of timer. */ +/* Internal representation of SIGEV_THREAD timer. */ struct timer { - /* Notification mechanism. */ - int sigev_notify; - - /* Timer ID returned by the kernel. */ kernel_timer_t ktimerid; - /* All new elements must be added after ktimerid. And if the thrfunc - element is not the third element anymore the memory allocation in - timer_create needs to be changed. */ - - /* Parameters for the thread to be started for SIGEV_THREAD. */ void (*thrfunc) (sigval_t); sigval_t sival; pthread_attr_t attr; @@ -65,3 +55,37 @@ struct timer /* Next element in list of active SIGEV_THREAD timers. */ struct timer *next; }; + + +/* For !SIGEV_THREAD, the resulting 'timer_t' is the returned kernel timer + identifer (kernel_timer_t), while for SIGEV_THREAD it uses the fact malloc + returns at least _Alignof (max_align_t) pointers plus that valid + kernel_timer_t are always positive to set MSB bit of the returned 'timer_t' + to indicate the timer handles a SIGEV_THREAD. */ + +static inline timer_t +timer_to_timerid (struct timer *ptr) +{ + return (timer_t) (INTPTR_MIN | (uintptr_t) ptr >> 1); +} + +static inline bool +timer_is_sigev_thread (timer_t timerid) +{ + return (intptr_t) timerid < 0; +} + +static inline struct timer * +timerid_to_timer (timer_t timerid) +{ + return (struct timer *)((uintptr_t) timerid << 1); +} + +static inline kernel_timer_t +timerid_to_kernel_timer (timer_t timerid) +{ + if (timer_is_sigev_thread (timerid)) + return timerid_to_timer (timerid)->ktimerid; + else + return (kernel_timer_t) ((uintptr_t) timerid); +} diff --git a/sysdeps/unix/sysv/linux/timer_create.c b/sysdeps/unix/sysv/linux/timer_create.c index 370c99a517..69b4e667fa 100644 --- a/sysdeps/unix/sysv/linux/timer_create.c +++ b/sysdeps/unix/sysv/linux/timer_create.c @@ -52,16 +52,6 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid) { struct sigevent local_evp; - /* We avoid allocating too much memory by basically - using struct timer as a derived class with the - first two elements being in the superclass. We only - need these two elements here. */ - struct timer *newp = (struct timer *) malloc (offsetof (struct timer, - thrfunc)); - if (newp == NULL) - /* No more memory. */ - return -1; - if (evp == NULL) { /* The kernel has to pass up the timer ID which is a @@ -69,31 +59,17 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid) the kernel to determine it. */ local_evp.sigev_notify = SIGEV_SIGNAL; local_evp.sigev_signo = SIGALRM; - local_evp.sigev_value.sival_ptr = newp; + local_evp.sigev_value.sival_ptr = NULL; evp = &local_evp; } kernel_timer_t ktimerid; - int retval = INLINE_SYSCALL (timer_create, 3, syscall_clockid, evp, - &ktimerid); - - if (retval != -1) - { - newp->sigev_notify = (evp != NULL - ? evp->sigev_notify : SIGEV_SIGNAL); - newp->ktimerid = ktimerid; - - *timerid = (timer_t) newp; - } - else - { - /* Cannot allocate the timer, fail. */ - free (newp); - retval = -1; - } + if (INLINE_SYSCALL_CALL (timer_create, syscall_clockid, evp, + &ktimerid) == -1) + return -1; - return retval; + *timerid = (timer_t) ((intptr_t) ktimerid); } else { @@ -106,20 +82,18 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid) return -1; } - struct timer *newp; - newp = (struct timer *) malloc (sizeof (struct timer)); + struct timer *newp = malloc (sizeof (struct timer)); if (newp == NULL) return -1; /* Copy the thread parameters the user provided. */ newp->sival = evp->sigev_value; newp->thrfunc = evp->sigev_notify_function; - newp->sigev_notify = SIGEV_THREAD; /* We cannot simply copy the thread attributes since the implementation might keep internal information for each instance. */ - (void) pthread_attr_init (&newp->attr); + pthread_attr_init (&newp->attr); if (evp->sigev_notify_attributes != NULL) { struct pthread_attr *nattr; @@ -137,8 +111,7 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid) } /* In any case set the detach flag. */ - (void) pthread_attr_setdetachstate (&newp->attr, - PTHREAD_CREATE_DETACHED); + pthread_attr_setdetachstate (&newp->attr, PTHREAD_CREATE_DETACHED); /* Create the event structure for the kernel timer. */ struct sigevent sev = @@ -149,27 +122,24 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid) /* Create the timer. */ int res; - res = INTERNAL_SYSCALL_CALL (timer_create, - syscall_clockid, &sev, &newp->ktimerid); - if (! INTERNAL_SYSCALL_ERROR_P (res)) + res = INTERNAL_SYSCALL_CALL (timer_create, syscall_clockid, &sev, + &newp->ktimerid); + if (res == -1) { - /* Add to the queue of active timers with thread - delivery. */ - pthread_mutex_lock (&__active_timer_sigev_thread_lock); - newp->next = __active_timer_sigev_thread; - __active_timer_sigev_thread = newp; - pthread_mutex_unlock (&__active_timer_sigev_thread_lock); - - *timerid = (timer_t) newp; - return 0; + free (newp); + __set_errno (INTERNAL_SYSCALL_ERRNO (res)); + return -1; } - /* Free the resources. */ - free (newp); - - __set_errno (INTERNAL_SYSCALL_ERRNO (res)); + /* Add to the queue of active timers with thread delivery. */ + pthread_mutex_lock (&__active_timer_sigev_thread_lock); + newp->next = __active_timer_sigev_thread; + __active_timer_sigev_thread = newp; + pthread_mutex_unlock (&__active_timer_sigev_thread_lock); - return -1; + *timerid = timer_to_timerid (newp); } } + + return 0; } diff --git a/sysdeps/unix/sysv/linux/timer_delete.c b/sysdeps/unix/sysv/linux/timer_delete.c index c240c06139..c8a5f101b4 100644 --- a/sysdeps/unix/sysv/linux/timer_delete.c +++ b/sysdeps/unix/sysv/linux/timer_delete.c @@ -32,15 +32,15 @@ int timer_delete (timer_t timerid) { #undef timer_delete - struct timer *kt = (struct timer *) timerid; - - /* Delete the kernel timer object. */ - int res = INLINE_SYSCALL (timer_delete, 1, kt->ktimerid); + kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid); + int res = INLINE_SYSCALL_CALL (timer_delete, ktimerid); if (res == 0) { - if (kt->sigev_notify == SIGEV_THREAD) + if (timer_is_sigev_thread (timerid)) { + struct timer *kt = timerid_to_timer (timerid); + /* Remove the timer from the list. */ pthread_mutex_lock (&__active_timer_sigev_thread_lock); if (__active_timer_sigev_thread == kt) @@ -58,10 +58,9 @@ timer_delete (timer_t timerid) prevp = prevp->next; } pthread_mutex_unlock (&__active_timer_sigev_thread_lock); - } - /* Free the memory. */ - (void) free (kt); + free (kt); + } return 0; } diff --git a/sysdeps/unix/sysv/linux/timer_getoverr.c b/sysdeps/unix/sysv/linux/timer_getoverr.c index 81b9723f01..7862d162b9 100644 --- a/sysdeps/unix/sysv/linux/timer_getoverr.c +++ b/sysdeps/unix/sysv/linux/timer_getoverr.c @@ -31,10 +31,6 @@ int timer_getoverrun (timer_t timerid) { #undef timer_getoverrun - struct timer *kt = (struct timer *) timerid; - - /* Get the information from the kernel. */ - int res = INLINE_SYSCALL (timer_getoverrun, 1, kt->ktimerid); - - return res; + kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid); + return INLINE_SYSCALL_CALL (timer_getoverrun, ktimerid); } diff --git a/sysdeps/unix/sysv/linux/timer_gettime.c b/sysdeps/unix/sysv/linux/timer_gettime.c index 5d31c7c864..07b6309ba9 100644 --- a/sysdeps/unix/sysv/linux/timer_gettime.c +++ b/sysdeps/unix/sysv/linux/timer_gettime.c @@ -26,18 +26,18 @@ int __timer_gettime64 (timer_t timerid, struct __itimerspec64 *value) { - struct timer *kt = (struct timer *) timerid; + kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid); #ifndef __NR_timer_gettime64 # define __NR_timer_gettime64 __NR_timer_gettime #endif - int ret = INLINE_SYSCALL_CALL (timer_gettime64, kt->ktimerid, value); + int ret = INLINE_SYSCALL_CALL (timer_gettime64, ktimerid, value); #ifndef __ASSUME_TIME64_SYSCALLS if (ret == 0 || errno != ENOSYS) return ret; struct itimerspec its32; - ret = INLINE_SYSCALL_CALL (timer_gettime, kt->ktimerid, &its32); + ret = INLINE_SYSCALL_CALL (timer_gettime, ktimerid, &its32); if (ret == 0) { value->it_interval = valid_timespec_to_timespec64 (its32.it_interval); diff --git a/sysdeps/unix/sysv/linux/timer_settime.c b/sysdeps/unix/sysv/linux/timer_settime.c index c97a9e92ef..01c6257938 100644 --- a/sysdeps/unix/sysv/linux/timer_settime.c +++ b/sysdeps/unix/sysv/linux/timer_settime.c @@ -28,17 +28,17 @@ __timer_settime64 (timer_t timerid, int flags, const struct __itimerspec64 *value, struct __itimerspec64 *ovalue) { - struct timer *kt = (struct timer *) timerid; + kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid); #ifdef __ASSUME_TIME64_SYSCALLS # ifndef __NR_timer_settime64 # define __NR_timer_settime64 __NR_timer_settime # endif - return INLINE_SYSCALL_CALL (timer_settime64, kt->ktimerid, flags, value, + return INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, value, ovalue); #else # ifdef __NR_timer_settime64 - int ret = INLINE_SYSCALL_CALL (timer_settime64, kt->ktimerid, flags, value, + int ret = INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, value, ovalue); if (ret == 0 || errno != ENOSYS) return ret; @@ -55,7 +55,7 @@ __timer_settime64 (timer_t timerid, int flags, its32.it_interval = valid_timespec64_to_timespec (value->it_interval); its32.it_value = valid_timespec64_to_timespec (value->it_value); - int retval = INLINE_SYSCALL_CALL (timer_settime, kt->ktimerid, flags, + int retval = INLINE_SYSCALL_CALL (timer_settime, ktimerid, flags, &its32, ovalue ? &oits32 : NULL); if (retval == 0 && ovalue) { diff --git a/sysdeps/unix/sysv/linux/x86_64/timer_gettime.c b/sysdeps/unix/sysv/linux/x86_64/timer_gettime.c index 89002c7243..0e4213d7aa 100644 --- a/sysdeps/unix/sysv/linux/x86_64/timer_gettime.c +++ b/sysdeps/unix/sysv/linux/x86_64/timer_gettime.c @@ -24,9 +24,9 @@ int __timer_gettime_new (timer_t timerid, struct itimerspec *value) { - struct timer *kt = (struct timer *) timerid; + kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid); - return INLINE_SYSCALL_CALL (timer_gettime, kt->ktimerid, value); + return INLINE_SYSCALL_CALL (timer_gettime, ktimerid, value); } versioned_symbol (librt, __timer_gettime_new, timer_gettime, GLIBC_2_3_3); diff --git a/sysdeps/unix/sysv/linux/x86_64/timer_settime.c b/sysdeps/unix/sysv/linux/x86_64/timer_settime.c index 7af417459e..0636f7316d 100644 --- a/sysdeps/unix/sysv/linux/x86_64/timer_settime.c +++ b/sysdeps/unix/sysv/linux/x86_64/timer_settime.c @@ -25,10 +25,9 @@ int __timer_settime_new (timer_t timerid, int flags, const struct itimerspec *value, struct itimerspec *ovalue) { - struct timer *kt = (struct timer *) timerid; + kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid); - return INLINE_SYSCALL_CALL (timer_settime, kt->ktimerid, flags, value, - ovalue); + return INLINE_SYSCALL_CALL (timer_settime, ktimerid, flags, value, ovalue); } versioned_symbol (librt, __timer_settime_new, timer_settime, GLIBC_2_3_3);