* [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 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
* 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
* [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).