Re: byteswap.h behavior
On 5/12/24 1:30 PM, Paul Eggert wrote: > I sense a bit of confusion here. Although POSIX allows symbols > like be16toh to be macros, I don't see where it allows be16toh(X) to evaluate > X more than once, so an expression like be16toh(i++) has well-defined > behavior even though it has a side effect. Ah, okay that makes sense thanks. I didn't realize that it would be written explicitly I guess. I started working on the module a bit using the POSIX specification. When writing the test module I noticed that glibc doesn't define uint16_t, uint32_t, and uint64_t in endian.h. Does glibc take bug reports for that or is it waiting for the specification to be official (i.e. not a draft)? I should probably request a bugzilla account anyways so I can report bugs... Collin
Re: byteswap.h behavior
On 2024-05-12 12:47, Collin Funk wrote: Yeah, I read the POSIX draft and it says that they may be macros. I doubt the byteswap.h and endian.h functions are used with non-constant expression arguments that often. I sense a bit of confusion here. Although POSIX allows symbols like be16toh to be macros, I don't see where it allows be16toh(X) to evaluate X more than once, so an expression like be16toh(i++) has well-defined behavior even though it has a side effect. A few function-like macros, like getc, are explicitly allowed to evaluate their arguments more than once, but ordinarily function-like macros are supposed to act like their corresponding functions.
Re: byteswap.h behavior
On 5/12/24 1:02 PM, Bruno Haible wrote:
> The simple fix to your worry is: Let the user use '-Wall' routinely.
> gcc has a warning option '-Wsequence-point', included in '-Wall'.
> clang has a warning option '-Wunsequenced', included in '-Wall'.
> The do the job, and I have not seen them produce false warnings ever.
>
> $ gcc -Wall foo.c
> foo.c: In function ‘main’:
> foo.c:15:59: warning: operation on ‘value_macro’ may be undefined
> [-Wsequence-point]
>15 | printf ("2. %#" PRIX32 "\n", bswap_32_macro (value_macro++));
> | ^
Oh, I completely forgot about this warning. It is very rare that I
write something that angers it. That alleviates my worry since those
messages are hard to miss. Thanks.
Collin
Re: byteswap.h behavior
Collin Funk wrote:
> I worry though that in some cases programs will be accustomed to
> glibc's behavior. Since all of the functions are just macros to static
> inline functions in arguments will only be evaluated
> once. It seems like it could be the cause of some unexpected bugs...
The simple fix to your worry is: Let the user use '-Wall' routinely.
gcc has a warning option '-Wsequence-point', included in '-Wall'.
clang has a warning option '-Wunsequenced', included in '-Wall'.
The do the job, and I have not seen them produce false warnings ever.
$ gcc -Wall foo.c
foo.c: In function ‘main’:
foo.c:15:59: warning: operation on ‘value_macro’ may be undefined
[-Wsequence-point]
15 | printf ("2. %#" PRIX32 "\n", bswap_32_macro (value_macro++));
| ^
foo.c:7:31: note: in definition of macro ‘bswap_32_macro’
7 |(((x) & 0x00FF) >> 8) | \
| ^
foo.c:15:59: warning: operation on ‘value_macro’ may be undefined
[-Wsequence-point]
15 | printf ("2. %#" PRIX32 "\n", bswap_32_macro (value_macro++));
| ^
foo.c:7:31: note: in definition of macro ‘bswap_32_macro’
7 |(((x) & 0x00FF) >> 8) | \
| ^
foo.c:15:59: warning: operation on ‘value_macro’ may be undefined
[-Wsequence-point]
15 | printf ("2. %#" PRIX32 "\n", bswap_32_macro (value_macro++));
| ^
foo.c:7:31: note: in definition of macro ‘bswap_32_macro’
7 |(((x) & 0x00FF) >> 8) | \
| ^
$ clang -Wall foo.c
foo.c:15:59: warning: multiple unsequenced modifications to 'value_macro'
[-Wunsequenced]
15 | printf ("2. %#" PRIX32 "\n", bswap_32_macro (value_macro++));
| ^~
foo.c:5:31: note: expanded from macro 'bswap_32_macro'
5 | #define bswap_32_macro(x) x) & 0x00FF) << 24) | \
| ^
6 |(((x) & 0xFF00) << 8) | \
| ~
1 warning generated.
Assume this warning option is used. Then you can leave byteswap.h as it is.
Bruno
Re: byteswap.h behavior
On Sun, May 12, 2024 at 3:23 PM Collin Funk wrote:
> A few months ago there was a suggestion on emacs-devel to use
> __builtin_bswap functions when available [1]. While I agree that GCC
> can deal with the optimizations properly, I noted an important
> difference between the macros in byteswap.h.in and inline functions
> provided by glibc.
>
> Using this test program to compare the real glibc header to a macro
> copy-pasted from the replacement header:
>
> ==
> #define _GNU_SOURCE 1
> #include
> #include
> #include
> #define bswap_32_macro(x) x) & 0x00FF) << 24) | \
>(((x) & 0xFF00) << 8) | \
>(((x) & 0x00FF) >> 8) | \
>(((x) & 0xFF00) >> 24))
> int
> main (void)
> {
> uint32_t value = 0x12345678;
> uint32_t value_macro = 0x12345678;
> printf ("1. %#" PRIX32 "\n", bswap_32 (value++));
> printf ("2. %#" PRIX32 "\n", bswap_32_macro (value_macro++));
> printf ("3. %#" PRIX32 "\n", value);
> printf ("4. %#" PRIX32 "\n", value_macro);
> return 0;
> }
> ==
>
> We get the output:
>
> $ ./a.out
> 1. 0X78563412
> 2. 0X78563412
> 3. 0X12345679
> 4. 0X1234567C
>
> I would like to deal with this concern before I implement the
> replacement for . I think the best decision is to use
> 'extern inline' to match the behavior of glibc. Any other opinions on
> this?
>
> Also if we can agree upon making sure these are defined as functions,
> what is the proper way to test it in a configure script? My instinct
> tells me that assigning a function pointer to bswap_16, etc. would
> fail if they are macros but I am not sure the "standard" way of
> performing that check.
>
> Collin
>
> [1] https://lists.gnu.org/archive/html/emacs-devel/2024-03/msg00736.html
>
I think extern is the least of your worries. Pre- and post- increment are
not safe in a macro. Maybe you should make a local copy of the value in a
do {} while, and then operate on the local value. Or make it a function.
Jeff
Re: byteswap.h behavior
On 5/12/24 12:38 PM, Paul Eggert wrote: >> Also if we can agree upon making sure these are defined as functions, >> what is the proper way to test it in a configure script? My instinct >> tells me that assigning a function pointer to bswap_16, etc. would >> fail if they are macros > > I don't see the need to check that they are functions. POSIX does not require > endian.h symbols like be16toh to be defined as functions, and the glibc > manual doesn't mention endian.h, so it sounds like Gnulib-using code > shouldn't assume that these symbols are functions. Yeah, I read the POSIX draft and it says that they may be macros. I doubt the byteswap.h and endian.h functions are used with non-constant expression arguments that often. I worry though that in some cases programs will be accustomed to glibc's behavior. Since all of the functions are just macros to static inline functions in arguments will only be evaluated once. It seems like it could be the cause of some unexpected bugs... Collin
Re: byteswap.h behavior
On 2024-05-12 12:23, Collin Funk wrote: I think the best decision is to use 'extern inline' to match the behavior of glibc. Yes, with the usual _GL_EXTERN business. Also if we can agree upon making sure these are defined as functions, what is the proper way to test it in a configure script? My instinct tells me that assigning a function pointer to bswap_16, etc. would fail if they are macros I don't see the need to check that they are functions. POSIX does not require endian.h symbols like be16toh to be defined as functions, and the glibc manual doesn't mention endian.h, so it sounds like Gnulib-using code shouldn't assume that these symbols are functions.
