* [musl] fix various warnings/theoretical UB
[not found] <1688401586.hkqjuyrd3s.none.ref@localhost>
@ 2023-07-03 17:55 ` Alex Xu (Hello71)
2023-07-03 19:59 ` Rich Felker
2023-11-01 23:44 ` [musl] [v2] " Alex Xu (Hello71)
0 siblings, 2 replies; 6+ messages in thread
From: Alex Xu (Hello71) @ 2023-07-03 17:55 UTC (permalink / raw)
To: musl
[-- Attachment #1: Type: text/plain, Size: 23 bytes --]
See attached patches.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-volatile-static-static-volatile.patch --]
[-- Type: text/x-patch; name=0001-volatile-static-static-volatile.patch, Size: 909 bytes --]
From 978f2cded65ce73450277d3fde48f038b339d5f9 Mon Sep 17 00:00:00 2001
From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
Date: Sun, 2 Jul 2023 20:28:23 -0400
Subject: [PATCH 1/4] volatile static -> static volatile
C11 6.11.5p1:
> The placement of a storage-class specifier other than at the
> beginning of the declaration specifiers in a declaration is an
> obsolescent feature.
gcc also warns about this.
---
src/time/timer_create.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/time/timer_create.c b/src/time/timer_create.c
index cd32c945..9216b3ab 100644
--- a/src/time/timer_create.c
+++ b/src/time/timer_create.c
@@ -61,7 +61,7 @@ static void *start(void *arg)
int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict res)
{
- volatile static int init = 0;
+ static volatile int init = 0;
pthread_t td;
pthread_attr_t attr;
int r;
--
2.41.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-__year_to_secs-fix-dangling-pointer.patch --]
[-- Type: text/x-patch; name=0002-__year_to_secs-fix-dangling-pointer.patch, Size: 1056 bytes --]
From b98f243e7921ddff6978ee9b0ce9f08efaa17951 Mon Sep 17 00:00:00 2001
From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
Date: Sun, 2 Jul 2023 20:29:41 -0400
Subject: [PATCH 2/4] __year_to_secs: fix dangling pointer
C11 6.5.2.5p5:
> If the compound literal occurs outside the body of a function, the
> object has static storage duration; otherwise, it has automatic
> storage duration associated with the enclosing block.
gcc also warns about this.
---
src/time/__year_to_secs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/time/__year_to_secs.c b/src/time/__year_to_secs.c
index 2824ec6d..d215880a 100644
--- a/src/time/__year_to_secs.c
+++ b/src/time/__year_to_secs.c
@@ -10,9 +10,9 @@ long long __year_to_secs(long long year, int *is_leap)
return 31536000*(y-70) + 86400*leaps;
}
- int cycles, centuries, leaps, rem;
+ int cycles, centuries, leaps, rem, tmp;
- if (!is_leap) is_leap = &(int){0};
+ if (!is_leap) is_leap = &tmp;
cycles = (year-100) / 400;
rem = (year-100) % 400;
if (rem < 0) {
--
2.41.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-fix-mismatched-VLA-parameter-types.patch --]
[-- Type: text/x-patch; name=0003-fix-mismatched-VLA-parameter-types.patch, Size: 1130 bytes --]
From a30c4ab397af040d10d978d97dd4a6835d4b99a8 Mon Sep 17 00:00:00 2001
From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
Date: Sun, 2 Jul 2023 20:54:45 -0400
Subject: [PATCH 3/4] fix mismatched VLA parameter types
gcc warns about this, and it's probably technically UB
---
src/internal/procfdname.c | 2 +-
src/prng/seed48.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/internal/procfdname.c b/src/internal/procfdname.c
index fd7306ab..bfa3e7e5 100644
--- a/src/internal/procfdname.c
+++ b/src/internal/procfdname.c
@@ -1,6 +1,6 @@
#include "syscall.h"
-void __procfdname(char *buf, unsigned fd)
+void __procfdname(char buf[static 15+3*sizeof(int)], unsigned fd)
{
unsigned i, j;
for (i=0; (buf[i] = "/proc/self/fd/"[i]); i++);
diff --git a/src/prng/seed48.c b/src/prng/seed48.c
index bce7b339..7b789086 100644
--- a/src/prng/seed48.c
+++ b/src/prng/seed48.c
@@ -2,7 +2,7 @@
#include <string.h>
#include "rand48.h"
-unsigned short *seed48(unsigned short *s)
+unsigned short *seed48(unsigned short s[3])
{
static unsigned short p[3];
memcpy(p, __seed48, sizeof p);
--
2.41.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [musl] fix various warnings/theoretical UB
2023-07-03 17:55 ` [musl] fix various warnings/theoretical UB Alex Xu (Hello71)
@ 2023-07-03 19:59 ` Rich Felker
2023-07-03 21:23 ` Jens Gustedt
2023-07-03 22:30 ` Alex Xu (Hello71)
2023-11-01 23:44 ` [musl] [v2] " Alex Xu (Hello71)
1 sibling, 2 replies; 6+ messages in thread
From: Rich Felker @ 2023-07-03 19:59 UTC (permalink / raw)
To: Alex Xu (Hello71); +Cc: musl
On Mon, Jul 03, 2023 at 01:55:57PM -0400, Alex Xu (Hello71) wrote:
> See attached patches.
> From 978f2cded65ce73450277d3fde48f038b339d5f9 Mon Sep 17 00:00:00 2001
> From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
> Date: Sun, 2 Jul 2023 20:28:23 -0400
> Subject: [PATCH 1/4] volatile static -> static volatile
>
> C11 6.11.5p1:
>
> > The placement of a storage-class specifier other than at the
> > beginning of the declaration specifiers in a declaration is an
> > obsolescent feature.
>
> gcc also warns about this.
> ---
> src/time/timer_create.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/time/timer_create.c b/src/time/timer_create.c
> index cd32c945..9216b3ab 100644
> --- a/src/time/timer_create.c
> +++ b/src/time/timer_create.c
> @@ -61,7 +61,7 @@ static void *start(void *arg)
>
> int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict res)
> {
> - volatile static int init = 0;
> + static volatile int init = 0;
> pthread_t td;
> pthread_attr_t attr;
> int r;
> --
> 2.41.0
No objection to this change. It's contrary to usual style. I would say
let's convert to pthread_once, but this code is slated for removal
anyway once signals are no longer used for SIGEV_THREAD timers.
> From b98f243e7921ddff6978ee9b0ce9f08efaa17951 Mon Sep 17 00:00:00 2001
> From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
> Date: Sun, 2 Jul 2023 20:29:41 -0400
> Subject: [PATCH 2/4] __year_to_secs: fix dangling pointer
>
> C11 6.5.2.5p5:
>
> > If the compound literal occurs outside the body of a function, the
> > object has static storage duration; otherwise, it has automatic
> > storage duration associated with the enclosing block.
>
> gcc also warns about this.
> ---
> src/time/__year_to_secs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/time/__year_to_secs.c b/src/time/__year_to_secs.c
> index 2824ec6d..d215880a 100644
> --- a/src/time/__year_to_secs.c
> +++ b/src/time/__year_to_secs.c
> @@ -10,9 +10,9 @@ long long __year_to_secs(long long year, int *is_leap)
> return 31536000*(y-70) + 86400*leaps;
> }
>
> - int cycles, centuries, leaps, rem;
> + int cycles, centuries, leaps, rem, tmp;
>
> - if (!is_leap) is_leap = &(int){0};
> + if (!is_leap) is_leap = &tmp;
> cycles = (year-100) / 400;
> rem = (year-100) % 400;
> if (rem < 0) {
> --
> 2.41.0
Seems like a bogus warning. The enclosing block is the whole function,
the same as the lifetime of the pointer. This might merit
investigation on whether GCC is doing something wrong though..
> From a30c4ab397af040d10d978d97dd4a6835d4b99a8 Mon Sep 17 00:00:00 2001
> From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
> Date: Sun, 2 Jul 2023 20:54:45 -0400
> Subject: [PATCH 3/4] fix mismatched VLA parameter types
>
> gcc warns about this, and it's probably technically UB
> ---
> src/internal/procfdname.c | 2 +-
> src/prng/seed48.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/internal/procfdname.c b/src/internal/procfdname.c
> index fd7306ab..bfa3e7e5 100644
> --- a/src/internal/procfdname.c
> +++ b/src/internal/procfdname.c
> @@ -1,6 +1,6 @@
> #include "syscall.h"
>
> -void __procfdname(char *buf, unsigned fd)
> +void __procfdname(char buf[static 15+3*sizeof(int)], unsigned fd)
> {
> unsigned i, j;
> for (i=0; (buf[i] = "/proc/self/fd/"[i]); i++);
This was raised/proposed before and is probably an okay change, but
I'd like to understand what the reason "it's probably technically UB"
is.
> diff --git a/src/prng/seed48.c b/src/prng/seed48.c
> index bce7b339..7b789086 100644
> --- a/src/prng/seed48.c
> +++ b/src/prng/seed48.c
> @@ -2,7 +2,7 @@
> #include <string.h>
> #include "rand48.h"
>
> -unsigned short *seed48(unsigned short *s)
> +unsigned short *seed48(unsigned short s[3])
> {
> static unsigned short p[3];
> memcpy(p, __seed48, sizeof p);
> --
This one is almost surely not UB because there's no static and the 3
is ignored. The question is just whether the static produces a
difference in the declaration type that makes them clash.
Rich
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [musl] fix various warnings/theoretical UB
2023-07-03 19:59 ` Rich Felker
@ 2023-07-03 21:23 ` Jens Gustedt
2023-07-03 22:57 ` Rich Felker
2023-07-03 22:30 ` Alex Xu (Hello71)
1 sibling, 1 reply; 6+ messages in thread
From: Jens Gustedt @ 2023-07-03 21:23 UTC (permalink / raw)
To: musl
Hello,
Am 3. Juli 2023 21:59:57 MESZ schrieb Rich Felker <dalias@libc.org>:
> On Mon, Jul 03, 2023 at 01:55:57PM -0400, Alex Xu (Hello71) wrote:
> > See attached patches.
>
> > From 978f2cded65ce73450277d3fde48f038b339d5f9 Mon Sep 17 00:00:00 2001
> > From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
> > Date: Sun, 2 Jul 2023 20:28:23 -0400
> > Subject: [PATCH 1/4] volatile static -> static volatile
> >
> > C11 6.11.5p1:
> >
> > > The placement of a storage-class specifier other than at the
> > > beginning of the declaration specifiers in a declaration is an
> > > obsolescent feature.
> >
> > gcc also warns about this.
> > ---
> > src/time/timer_create.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/time/timer_create.c b/src/time/timer_create.c
> > index cd32c945..9216b3ab 100644
> > --- a/src/time/timer_create.c
> > +++ b/src/time/timer_create.c
> > @@ -61,7 +61,7 @@ static void *start(void *arg)
> >
> > int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict res)
> > {
> > - volatile static int init = 0;
> > + static volatile int init = 0;
> > pthread_t td;
> > pthread_attr_t attr;
> > int r;
> > --
> > 2.41.0
>
> No objection to this change. It's contrary to usual style. I would say
> let's convert to pthread_once, but this code is slated for removal
> anyway once signals are no longer used for SIGEV_THREAD timers.
>
> > From b98f243e7921ddff6978ee9b0ce9f08efaa17951 Mon Sep 17 00:00:00 2001
> > From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
> > Date: Sun, 2 Jul 2023 20:29:41 -0400
> > Subject: [PATCH 2/4] __year_to_secs: fix dangling pointer
> >
> > C11 6.5.2.5p5:
> >
> > > If the compound literal occurs outside the body of a function, the
> > > object has static storage duration; otherwise, it has automatic
> > > storage duration associated with the enclosing block.
> >
> > gcc also warns about this.
> > ---
> > src/time/__year_to_secs.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/time/__year_to_secs.c b/src/time/__year_to_secs.c
> > index 2824ec6d..d215880a 100644
> > --- a/src/time/__year_to_secs.c
> > +++ b/src/time/__year_to_secs.c
> > @@ -10,9 +10,9 @@ long long __year_to_secs(long long year, int *is_leap)
> > return 31536000*(y-70) + 86400*leaps;
> > }
> >
> > - int cycles, centuries, leaps, rem;
> > + int cycles, centuries, leaps, rem, tmp;
> >
> > - if (!is_leap) is_leap = &(int){0};
> > + if (!is_leap) is_leap = &tmp;
> > cycles = (year-100) / 400;
> > rem = (year-100) % 400;
> > if (rem < 0) {
> > --
> > 2.41.0
>
> Seems like a bogus warning. The enclosing block is the whole function,
No, the `if` statement forms a block of itself, and then the dependent statement forms yet another block.
We rectify the terminology a bit in C23 hopefully make it easier to read without changing semantics
> the same as the lifetime of the pointer. This might merit
> investigation on whether GCC is doing something wrong though..
>
> > From a30c4ab397af040d10d978d97dd4a6835d4b99a8 Mon Sep 17 00:00:00 2001
> > From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
> > Date: Sun, 2 Jul 2023 20:54:45 -0400
> > Subject: [PATCH 3/4] fix mismatched VLA parameter types
> >
> > gcc warns about this, and it's probably technically UB
> > ---
> > src/internal/procfdname.c | 2 +-
> > src/prng/seed48.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/internal/procfdname.c b/src/internal/procfdname.c
> > index fd7306ab..bfa3e7e5 100644
> > --- a/src/internal/procfdname.c
> > +++ b/src/internal/procfdname.c
> > @@ -1,6 +1,6 @@
> > #include "syscall.h"
> >
> > -void __procfdname(char *buf, unsigned fd)
> > +void __procfdname(char buf[static 15+3*sizeof(int)], unsigned fd)
> > {
> > unsigned i, j;
> > for (i=0; (buf[i] = "/proc/self/fd/"[i]); i++);
>
> This was raised/proposed before and is probably an okay change, but
> I'd like to understand what the reason "it's probably technically UB"
> is.
>
> > diff --git a/src/prng/seed48.c b/src/prng/seed48.c
> > index bce7b339..7b789086 100644
> > --- a/src/prng/seed48.c
> > +++ b/src/prng/seed48.c
> > @@ -2,7 +2,7 @@
> > #include <string.h>
> > #include "rand48.h"
> >
> > -unsigned short *seed48(unsigned short *s)
> > +unsigned short *seed48(unsigned short s[3])
> > {
> > static unsigned short p[3];
> > memcpy(p, __seed48, sizeof p);
> > --
>
> This one is almost surely not UB because there's no static and the 3
> is ignored. The question is just whether the static produces a
> difference in the declaration type that makes them clash.
>
> Rich
Jens
--
Jens Gustedt - INRIA & ICube, Strasbourg, France
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [musl] fix various warnings/theoretical UB
2023-07-03 19:59 ` Rich Felker
2023-07-03 21:23 ` Jens Gustedt
@ 2023-07-03 22:30 ` Alex Xu (Hello71)
1 sibling, 0 replies; 6+ messages in thread
From: Alex Xu (Hello71) @ 2023-07-03 22:30 UTC (permalink / raw)
To: musl
Excerpts from Rich Felker's message of July 3, 2023 3:59 pm:
> On Mon, Jul 03, 2023 at 01:55:57PM -0400, Alex Xu (Hello71) wrote:
>> From b98f243e7921ddff6978ee9b0ce9f08efaa17951 Mon Sep 17 00:00:00 2001
>> From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
>> Date: Sun, 2 Jul 2023 20:29:41 -0400
>> Subject: [PATCH 2/4] __year_to_secs: fix dangling pointer
>>
>> C11 6.5.2.5p5:
>>
>> > If the compound literal occurs outside the body of a function, the
>> > object has static storage duration; otherwise, it has automatic
>> > storage duration associated with the enclosing block.
>>
>> gcc also warns about this.
>> ---
>> src/time/__year_to_secs.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/time/__year_to_secs.c b/src/time/__year_to_secs.c
>> index 2824ec6d..d215880a 100644
>> --- a/src/time/__year_to_secs.c
>> +++ b/src/time/__year_to_secs.c
>> @@ -10,9 +10,9 @@ long long __year_to_secs(long long year, int *is_leap)
>> return 31536000*(y-70) + 86400*leaps;
>> }
>>
>> - int cycles, centuries, leaps, rem;
>> + int cycles, centuries, leaps, rem, tmp;
>>
>> - if (!is_leap) is_leap = &(int){0};
>> + if (!is_leap) is_leap = &tmp;
>> cycles = (year-100) / 400;
>> rem = (year-100) % 400;
>> if (rem < 0) {
>> --
>> 2.41.0
>
> Seems like a bogus warning. The enclosing block is the whole function,
> the same as the lifetime of the pointer. This might merit
> investigation on whether GCC is doing something wrong though..
As Jens says, an if statement "is a block whose scope is a strict subset
of the scope of its enclosing block. Each associated substatement is
also a block whose scope is a strict subset of the scope of the
selection statement.".
>> From a30c4ab397af040d10d978d97dd4a6835d4b99a8 Mon Sep 17 00:00:00 2001
>> From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
>> Date: Sun, 2 Jul 2023 20:54:45 -0400
>> Subject: [PATCH 3/4] fix mismatched VLA parameter types
>>
>> gcc warns about this, and it's probably technically UB
>> ---
>> src/internal/procfdname.c | 2 +-
>> src/prng/seed48.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/internal/procfdname.c b/src/internal/procfdname.c
>> index fd7306ab..bfa3e7e5 100644
>> --- a/src/internal/procfdname.c
>> +++ b/src/internal/procfdname.c
>> @@ -1,6 +1,6 @@
>> #include "syscall.h"
>>
>> -void __procfdname(char *buf, unsigned fd)
>> +void __procfdname(char buf[static 15+3*sizeof(int)], unsigned fd)
>> {
>> unsigned i, j;
>> for (i=0; (buf[i] = "/proc/self/fd/"[i]); i++);
>
> This was raised/proposed before and is probably an okay change, but
> I'd like to understand what the reason "it's probably technically UB"
> is.
>
>> diff --git a/src/prng/seed48.c b/src/prng/seed48.c
>> index bce7b339..7b789086 100644
>> --- a/src/prng/seed48.c
>> +++ b/src/prng/seed48.c
>> @@ -2,7 +2,7 @@
>> #include <string.h>
>> #include "rand48.h"
>>
>> -unsigned short *seed48(unsigned short *s)
>> +unsigned short *seed48(unsigned short s[3])
>> {
>> static unsigned short p[3];
>> memcpy(p, __seed48, sizeof p);
>> --
>
> This one is almost surely not UB because there's no static and the 3
> is ignored. The question is just whether the static produces a
> difference in the declaration type that makes them clash.
After reading the function declarations section in the C2x draft, I
think you're right. These are both well-defined because they are
adjusted to the same pointer type, because neither the static nor
non-static sizes are actually propagated to the pointer type.
Thanks,
Alex.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [musl] fix various warnings/theoretical UB
2023-07-03 21:23 ` Jens Gustedt
@ 2023-07-03 22:57 ` Rich Felker
0 siblings, 0 replies; 6+ messages in thread
From: Rich Felker @ 2023-07-03 22:57 UTC (permalink / raw)
To: Jens Gustedt; +Cc: musl
On Mon, Jul 03, 2023 at 11:23:00PM +0200, Jens Gustedt wrote:
> Hello,
>
> Am 3. Juli 2023 21:59:57 MESZ schrieb Rich Felker <dalias@libc.org>:
> > On Mon, Jul 03, 2023 at 01:55:57PM -0400, Alex Xu (Hello71) wrote:
> > > See attached patches.
> >
> > > From 978f2cded65ce73450277d3fde48f038b339d5f9 Mon Sep 17 00:00:00 2001
> > > From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
> > > Date: Sun, 2 Jul 2023 20:28:23 -0400
> > > Subject: [PATCH 1/4] volatile static -> static volatile
> > >
> > > C11 6.11.5p1:
> > >
> > > > The placement of a storage-class specifier other than at the
> > > > beginning of the declaration specifiers in a declaration is an
> > > > obsolescent feature.
> > >
> > > gcc also warns about this.
> > > ---
> > > src/time/timer_create.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/time/timer_create.c b/src/time/timer_create.c
> > > index cd32c945..9216b3ab 100644
> > > --- a/src/time/timer_create.c
> > > +++ b/src/time/timer_create.c
> > > @@ -61,7 +61,7 @@ static void *start(void *arg)
> > >
> > > int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict res)
> > > {
> > > - volatile static int init = 0;
> > > + static volatile int init = 0;
> > > pthread_t td;
> > > pthread_attr_t attr;
> > > int r;
> > > --
> > > 2.41.0
> >
> > No objection to this change. It's contrary to usual style. I would say
> > let's convert to pthread_once, but this code is slated for removal
> > anyway once signals are no longer used for SIGEV_THREAD timers.
> >
> > > From b98f243e7921ddff6978ee9b0ce9f08efaa17951 Mon Sep 17 00:00:00 2001
> > > From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
> > > Date: Sun, 2 Jul 2023 20:29:41 -0400
> > > Subject: [PATCH 2/4] __year_to_secs: fix dangling pointer
> > >
> > > C11 6.5.2.5p5:
> > >
> > > > If the compound literal occurs outside the body of a function, the
> > > > object has static storage duration; otherwise, it has automatic
> > > > storage duration associated with the enclosing block.
> > >
> > > gcc also warns about this.
> > > ---
> > > src/time/__year_to_secs.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/time/__year_to_secs.c b/src/time/__year_to_secs.c
> > > index 2824ec6d..d215880a 100644
> > > --- a/src/time/__year_to_secs.c
> > > +++ b/src/time/__year_to_secs.c
> > > @@ -10,9 +10,9 @@ long long __year_to_secs(long long year, int *is_leap)
> > > return 31536000*(y-70) + 86400*leaps;
> > > }
> > >
> > > - int cycles, centuries, leaps, rem;
> > > + int cycles, centuries, leaps, rem, tmp;
> > >
> > > - if (!is_leap) is_leap = &(int){0};
> > > + if (!is_leap) is_leap = &tmp;
> > > cycles = (year-100) / 400;
> > > rem = (year-100) % 400;
> > > if (rem < 0) {
> > > --
> > > 2.41.0
> >
> > Seems like a bogus warning. The enclosing block is the whole function,
>
> No, the `if` statement forms a block of itself, and then the
> dependent statement forms yet another block.
>
> We rectify the terminology a bit in C23 hopefully make it easier to
> read without changing semantics
Oh, yes, somehow I always forget this. I think we actually remedy it
somewhere else using ?: instead of if, which is a rather hilarious
footgun for anyone who goes gratuitously changing ?: to if for style
reasons...
Anyway, in that case this seems like a reasonable change, though
"dummy" would be a more appropriate var name than "tmp" I think.
Rich
^ permalink raw reply [flat|nested] 6+ messages in thread
* [musl] [v2] fix various warnings/theoretical UB
2023-07-03 17:55 ` [musl] fix various warnings/theoretical UB Alex Xu (Hello71)
2023-07-03 19:59 ` Rich Felker
@ 2023-11-01 23:44 ` Alex Xu (Hello71)
1 sibling, 0 replies; 6+ messages in thread
From: Alex Xu (Hello71) @ 2023-11-01 23:44 UTC (permalink / raw)
To: musl
[-- Attachment #1: Type: text/plain, Size: 156 bytes --]
See attached patches. Changes from previous version:
1. drop patches 3 and 4 (not UB)
2. rename "tmp" to "dummy"
3. adjust commit messages
Thanks!
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-timer_create-volatile-static-static-volatile.patch --]
[-- Type: text/x-patch; name=0001-timer_create-volatile-static-static-volatile.patch, Size: 923 bytes --]
From 5b7b756c1fbedc823319ec474cf56a22d978b8f4 Mon Sep 17 00:00:00 2001
From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
Date: Wed, 1 Nov 2023 19:37:08 -0400
Subject: [PATCH 1/2] timer_create: volatile static -> static volatile
C11 6.11.5p1:
> The placement of a storage-class specifier other than at the
> beginning of the declaration specifiers in a declaration is an
> obsolescent feature.
gcc also warns about this.
---
src/time/timer_create.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/time/timer_create.c b/src/time/timer_create.c
index cd32c945..9216b3ab 100644
--- a/src/time/timer_create.c
+++ b/src/time/timer_create.c
@@ -61,7 +61,7 @@ static void *start(void *arg)
int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict res)
{
- volatile static int init = 0;
+ static volatile int init = 0;
pthread_t td;
pthread_attr_t attr;
int r;
--
2.42.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-__year_to_secs-fix-dangling-pointer.patch --]
[-- Type: text/x-patch; name=0002-__year_to_secs-fix-dangling-pointer.patch, Size: 937 bytes --]
From beee5f00e0f19fa46166f7afbd92f108c25002fd Mon Sep 17 00:00:00 2001
From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
Date: Wed, 1 Nov 2023 19:37:44 -0400
Subject: [PATCH 2/2] __year_to_secs: fix dangling pointer
The lifetime of the compound literal ends after the "if" statement's
implicit block. gcc also warns about this.
---
src/time/__year_to_secs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/time/__year_to_secs.c b/src/time/__year_to_secs.c
index 2824ec6d..b42f5a6d 100644
--- a/src/time/__year_to_secs.c
+++ b/src/time/__year_to_secs.c
@@ -10,9 +10,9 @@ long long __year_to_secs(long long year, int *is_leap)
return 31536000*(y-70) + 86400*leaps;
}
- int cycles, centuries, leaps, rem;
+ int cycles, centuries, leaps, rem, dummy;
- if (!is_leap) is_leap = &(int){0};
+ if (!is_leap) is_leap = &dummy;
cycles = (year-100) / 400;
rem = (year-100) % 400;
if (rem < 0) {
--
2.42.0
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-01 23:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1688401586.hkqjuyrd3s.none.ref@localhost>
2023-07-03 17:55 ` [musl] fix various warnings/theoretical UB Alex Xu (Hello71)
2023-07-03 19:59 ` Rich Felker
2023-07-03 21:23 ` Jens Gustedt
2023-07-03 22:57 ` Rich Felker
2023-07-03 22:30 ` Alex Xu (Hello71)
2023-11-01 23:44 ` [musl] [v2] " Alex Xu (Hello71)
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).