diff mbox series

timezone: C23 [[reproducible]] and [[unsequenced]] fixups

Message ID 20240904173259.3073903-1-adhemerval.zanella@linaro.org
State New
Headers show
Series timezone: C23 [[reproducible]] and [[unsequenced]] fixups | expand

Commit Message

Adhemerval Zanella Netto Sept. 4, 2024, 5:32 p.m. UTC 
C23’s [[reproducible]] and [[unsequenced]] are weaker than GCC’s
__attribute__((pure)) and __attribute__((const) respectively,
so rework the code so that it doesn’t imply they’re equivalent.

This is a sync with the fix 'C23 [[reproducible]] and [[unsequenced]]
fixups' [1] (not yet in any release).  It fixes glibc build with gcc
master [2].

Checked on x86_64-linux-gnu.

[1] https://github.com/eggert/tz/commit/c0789e468753b3c67805dd5fd08b479e5b101c86
[2] https://sourceware.org/pipermail/libc-alpha/2024-September/159571.html
---
 timezone/private.h | 41 ++++++++++++++++++++---------------------
 timezone/zdump.c   |  6 +++---
 timezone/zic.c     | 18 +++++++++---------
 3 files changed, 32 insertions(+), 33 deletions(-)

Comments

Paul Eggert Sept. 4, 2024, 5:48 p.m. UTC | #1 
On 2024-09-04 10:32, Adhemerval Zanella wrote:
> This is a sync with the fix 'C23 [[reproducible]] and [[unsequenced]]
> fixups' [1] (not yet in any release).

It's not a sync, it's a cherry-pick, right? That is, it doesn't sync to 
that commit, it merely applies that commit's patch.

Instead, I suggest that we simply sync to the current TZDB master. This 
contains a few other fixes, e.g., a workaround for GCC bug 114833, and 
it omits the "# define ATTRIBUTE_CONST ATTRIBUTE_UNSEQUENCED" hack in 
the patch you proposed.

Would it help if I pushed out a new TZDB release now? That way, you 
could say that you're just syncing with the latest TZDB release. I had 
been thinking of pushing one out soon anyway.
Adhemerval Zanella Netto Sept. 4, 2024, 5:53 p.m. UTC | #2 
On 04/09/24 14:48, Paul Eggert wrote:
> On 2024-09-04 10:32, Adhemerval Zanella wrote:
>> This is a sync with the fix 'C23 [[reproducible]] and [[unsequenced]]
>> fixups' [1] (not yet in any release).
> 
> It's not a sync, it's a cherry-pick, right? That is, it doesn't sync to that commit, it merely applies that commit's patch.

Indeed.

> 
> Instead, I suggest that we simply sync to the current TZDB master. This contains a few other fixes, e.g., a workaround for GCC bug 114833, and it omits the "# define ATTRIBUTE_CONST ATTRIBUTE_UNSEQUENCED" hack in the patch you proposed.
> 
> Would it help if I pushed out a new TZDB release now? That way, you could say that you're just syncing with the latest TZDB release. I had been thinking of pushing one out soon anyway.

I don't have a strong preference, I can sync with current TZDB master.  It is
just that this issue is hitting our CI tests and cherry-picking the fix
seems the most straightforward fix.
Paul Eggert Sept. 4, 2024, 6:50 p.m. UTC | #3 
On 2024-09-04 10:53, Adhemerval Zanella Netto wrote:
> I don't have a strong preference, I can sync with current TZDB master.

Thanks, let's do that then. It's one less variant for me to worry about.
diff mbox series

Patch

diff --git a/timezone/private.h b/timezone/private.h
index 0dac6af4e3..fd13cc7264 100644
--- a/timezone/private.h
+++ b/timezone/private.h
@@ -460,14 +460,6 @@  typedef unsigned long uintmax_t;
 # define ckd_mul(r, a, b) __builtin_mul_overflow(a, b, r)
 #endif
 
-#if 3 <= __GNUC__
-# define ATTRIBUTE_MALLOC __attribute__((malloc))
-# define ATTRIBUTE_FORMAT(spec) __attribute__((format spec))
-#else
-# define ATTRIBUTE_MALLOC /* empty */
-# define ATTRIBUTE_FORMAT(spec) /* empty */
-#endif
-
 #if (defined __has_c_attribute \
      && (202311 <= __STDC_VERSION__ || !defined __STRICT_ANSI__))
 # define HAVE___HAS_C_ATTRIBUTE true
@@ -535,11 +527,7 @@  typedef unsigned long uintmax_t;
 # endif
 #endif
 #ifndef ATTRIBUTE_REPRODUCIBLE
-# if 3 <= __GNUC__
-#  define ATTRIBUTE_REPRODUCIBLE __attribute__((pure))
-# else
-#  define ATTRIBUTE_REPRODUCIBLE /* empty */
-# endif
+# define ATTRIBUTE_REPRODUCIBLE /* empty */
 #endif
 
 #if HAVE___HAS_C_ATTRIBUTE
@@ -548,11 +536,22 @@  typedef unsigned long uintmax_t;
 # endif
 #endif
 #ifndef ATTRIBUTE_UNSEQUENCED
-# if 3 <= __GNUC__
-#  define ATTRIBUTE_UNSEQUENCED __attribute__((const))
-# else
-#  define ATTRIBUTE_UNSEQUENCED /* empty */
-# endif
+# define ATTRIBUTE_UNSEQUENCED /* empty */
+#endif
+
+/* __attribute__((const)) is stricter than [[unsequenced]] and
+   __attribute__((pure)) is stricter than [[reproducible]],
+   so the latter are adequate substitutes in non-GCC C23 platforms.  */
+#if __GNUC__ < 3
+# define ATTRIBUTE_CONST ATTRIBUTE_UNSEQUENCED
+# define ATTRIBUTE_FORMAT(spec) /* empty */
+# define ATTRIBUTE_MALLOC /* empty */
+# define ATTRIBUTE_PURE ATTRIBUTE_REPRODUCIBLE
+#else
+# define ATTRIBUTE_CONST __attribute__((const))
+# define ATTRIBUTE_FORMAT(spec) __attribute__((format spec))
+# define ATTRIBUTE_MALLOC __attribute__((malloc))
+# define ATTRIBUTE_PURE __attribute__((pure))
 #endif
 
 #if (__STDC_VERSION__ < 199901 && !defined restrict \
@@ -682,7 +681,7 @@  DEPRECATED_IN_C23 char *asctime(struct tm const *);
 char *asctime_r(struct tm const *restrict, char *restrict);
 DEPRECATED_IN_C23 char *ctime(time_t const *);
 char *ctime_r(time_t const *, char *);
-ATTRIBUTE_UNSEQUENCED double difftime(time_t, time_t);
+ATTRIBUTE_CONST double difftime(time_t, time_t);
 size_t strftime(char *restrict, size_t, char const *restrict,
 		struct tm const *restrict);
 # if HAVE_STRFTIME_L
@@ -798,10 +797,10 @@  timezone_t tzalloc(char const *);
 void tzfree(timezone_t);
 # if STD_INSPIRED
 #  if TZ_TIME_T || !defined posix2time_z
-ATTRIBUTE_REPRODUCIBLE time_t posix2time_z(timezone_t, time_t);
+ATTRIBUTE_PURE time_t posix2time_z(timezone_t, time_t);
 #  endif
 #  if TZ_TIME_T || !defined time2posix_z
-ATTRIBUTE_REPRODUCIBLE time_t time2posix_z(timezone_t, time_t);
+ATTRIBUTE_PURE time_t time2posix_z(timezone_t, time_t);
 #  endif
 # endif
 #endif
diff --git a/timezone/zdump.c b/timezone/zdump.c
index 7d99cc74bd..edc4d66627 100644
--- a/timezone/zdump.c
+++ b/timezone/zdump.c
@@ -89,7 +89,7 @@  static bool	warned;
 static bool	errout;
 
 static char const *abbr(struct tm const *);
-ATTRIBUTE_REPRODUCIBLE static intmax_t delta(struct tm *, struct tm *);
+ATTRIBUTE_PURE static intmax_t delta(struct tm *, struct tm *);
 static void dumptime(struct tm const *);
 static time_t hunt(timezone_t, time_t, time_t, bool);
 static void show(timezone_t, char *, time_t, bool);
@@ -97,7 +97,7 @@  static void showextrema(timezone_t, char *, time_t, struct tm *, time_t);
 static void showtrans(char const *, struct tm const *, time_t, char const *,
 		      char const *);
 static const char *tformat(void);
-ATTRIBUTE_REPRODUCIBLE static time_t yeartot(intmax_t);
+ATTRIBUTE_PURE static time_t yeartot(intmax_t);
 
 /* Is C an ASCII digit?  */
 static bool
@@ -134,7 +134,7 @@  size_overflow(void)
 
 /* Return A + B, exiting if the result would overflow either ptrdiff_t
    or size_t.  A and B are both nonnegative.  */
-ATTRIBUTE_REPRODUCIBLE static ptrdiff_t
+ATTRIBUTE_PURE static ptrdiff_t
 sumsize(ptrdiff_t a, ptrdiff_t b)
 {
 #ifdef ckd_add
diff --git a/timezone/zic.c b/timezone/zic.c
index 00f00e307a..3e7d3552a5 100644
--- a/timezone/zic.c
+++ b/timezone/zic.c
@@ -470,7 +470,7 @@  size_overflow(void)
   memory_exhausted(_("size overflow"));
 }
 
-ATTRIBUTE_REPRODUCIBLE static ptrdiff_t
+ATTRIBUTE_PURE static ptrdiff_t
 size_sum(size_t a, size_t b)
 {
 #ifdef ckd_add
@@ -484,7 +484,7 @@  size_sum(size_t a, size_t b)
   size_overflow();
 }
 
-ATTRIBUTE_REPRODUCIBLE static ptrdiff_t
+ATTRIBUTE_PURE static ptrdiff_t
 size_product(ptrdiff_t nitems, ptrdiff_t itemsize)
 {
 #ifdef ckd_mul
@@ -499,7 +499,7 @@  size_product(ptrdiff_t nitems, ptrdiff_t itemsize)
   size_overflow();
 }
 
-ATTRIBUTE_REPRODUCIBLE static ptrdiff_t
+ATTRIBUTE_PURE static ptrdiff_t
 align_to(ptrdiff_t size, ptrdiff_t alignment)
 {
   ptrdiff_t lo_bits = alignment - 1, sum = size_sum(size, lo_bits);
@@ -1435,7 +1435,7 @@  relname(char const *target, char const *linkname)
 /* Return true if A and B must have the same parent dir if A and B exist.
    Return false if this is not necessarily true (though it might be true).
    Keep it simple, and do not inspect the file system.  */
-static bool
+ATTRIBUTE_PURE static bool
 same_parent_dirs(char const *a, char const *b)
 {
   for (; *a == *b; a++, b++)
@@ -3627,7 +3627,7 @@  lowerit(char a)
 }
 
 /* case-insensitive equality */
-ATTRIBUTE_REPRODUCIBLE static bool
+ATTRIBUTE_PURE static bool
 ciequal(register const char *ap, register const char *bp)
 {
 	while (lowerit(*ap) == lowerit(*bp++))
@@ -3636,7 +3636,7 @@  ciequal(register const char *ap, register const char *bp)
 	return false;
 }
 
-ATTRIBUTE_REPRODUCIBLE static bool
+ATTRIBUTE_PURE static bool
 itsabbr(register const char *abbr, register const char *word)
 {
 	if (lowerit(*abbr) != lowerit(*word))
@@ -3652,7 +3652,7 @@  itsabbr(register const char *abbr, register const char *word)
 
 /* Return true if ABBR is an initial prefix of WORD, ignoring ASCII case.  */
 
-ATTRIBUTE_REPRODUCIBLE static bool
+ATTRIBUTE_PURE static bool
 ciprefix(char const *abbr, char const *word)
 {
   do
@@ -3762,7 +3762,7 @@  time_overflow(void)
   exit(EXIT_FAILURE);
 }
 
-ATTRIBUTE_REPRODUCIBLE static zic_t
+ATTRIBUTE_PURE static zic_t
 oadd(zic_t t1, zic_t t2)
 {
 #ifdef ckd_add
@@ -3776,7 +3776,7 @@  oadd(zic_t t1, zic_t t2)
   time_overflow();
 }
 
-ATTRIBUTE_REPRODUCIBLE static zic_t
+ATTRIBUTE_PURE static zic_t
 tadd(zic_t t1, zic_t t2)
 {
 #ifdef ckd_add