Re: [PATCH] Fix incorrect sign-extension of pointer in 32-bit acl __to_entry
On Jul 10 18:31, David Allsopp via Cygwin-patches wrote: > Corinna Vinschen wrote: > > On Jul 10 15:22, David Allsopp via Cygwin-patches wrote: > > > Corinna Vinschen wrote: > > > > On Jul 9 20:30, David Allsopp via Cygwin-patches wrote: > > > > > I have some code where the acl_t returned by get_file_acl is > > > > > allocated at 0x80038248. As a result the acl_entry_t generated by > > > > > acl_get_entry has an "index" of -1, since the pointer was sign- > > extended to 64-bits. > > > > > > > > > > My fix is trivial and simply casts the pointer to uintptr_t first. > > > > > > > > Pushed. I still don't quite understand what the compiler is > > > > thinking there, sign-extending a pointer when casted to an unsigend > > > > int type, but your patch works, so all is well, I guess. > > > > > > Thank you - it is indeed hard to imagine when you'd ever want that > > behaviour! > > > > I wonder if this is a bug in x86 gcc... Jon? > > I put it to our C gurus in the OCaml team - one (who has also written > a formally verified C compiler which on purpose 0-extends in this > case) observed that GCC does the same for ARM32 and another (who has > occasionally delighted in abusing language memory models to concoct > highly bizarre, but legal, abuses of undefined behaviour) found > https://gcc.gnu.org/onlinedocs/gcc/Arrays-and-pointers-implementation.html... "Bizarre" is the right description here :) Corinna -- Corinna Vinschen Cygwin Maintainer
RE: [PATCH] Fix incorrect sign-extension of pointer in 32-bit acl __to_entry
Corinna Vinschen wrote: > On Jul 10 15:22, David Allsopp via Cygwin-patches wrote: > > Corinna Vinschen wrote: > > > On Jul 9 20:30, David Allsopp via Cygwin-patches wrote: > > > > I have some code where the acl_t returned by get_file_acl is > > > > allocated at 0x80038248. As a result the acl_entry_t generated by > > > > acl_get_entry has an "index" of -1, since the pointer was sign- > extended to 64-bits. > > > > > > > > My fix is trivial and simply casts the pointer to uintptr_t first. > > > > > > Pushed. I still don't quite understand what the compiler is > > > thinking there, sign-extending a pointer when casted to an unsigend > > > int type, but your patch works, so all is well, I guess. > > > > Thank you - it is indeed hard to imagine when you'd ever want that > behaviour! > > I wonder if this is a bug in x86 gcc... Jon? I put it to our C gurus in the OCaml team - one (who has also written a formally verified C compiler which on purpose 0-extends in this case) observed that GCC does the same for ARM32 and another (who has occasionally delighted in abusing language memory models to concoct highly bizarre, but legal, abuses of undefined behaviour) found https://gcc.gnu.org/onlinedocs/gcc/Arrays-and-pointers-implementation.html... although the Git history for GCC makes it fairly clear that this behaviour is retrospectively documented[1][2]! David [1] https://github.com/gcc-mirror/gcc/commit/cbf4c36fa373 [2] https://github.com/gcc-mirror/gcc/commit/58f4de4f271c
Re: [PATCH] Fix incorrect sign-extension of pointer in 32-bit acl __to_entry
On Jul 10 15:22, David Allsopp via Cygwin-patches wrote: > Corinna Vinschen wrote: > > On Jul 9 20:30, David Allsopp via Cygwin-patches wrote: > > > I have some code where the acl_t returned by get_file_acl is allocated > > > at 0x80038248. As a result the acl_entry_t generated by acl_get_entry > > > has an "index" of -1, since the pointer was sign-extended to 64-bits. > > > > > > My fix is trivial and simply casts the pointer to uintptr_t first. > > > > Pushed. I still don't quite understand what the compiler is thinking > > there, sign-extending a pointer when casted to an unsigend int type, but > > your patch works, so all is well, I guess. > > Thank you - it is indeed hard to imagine when you'd ever want that behaviour! I wonder if this is a bug in x86 gcc... Jon? > Would it be possible to have a snapshot with it, just for continuous > integration servers which need the fix, please? Sure, done. Corinna -- Corinna Vinschen Cygwin Maintainer
RE: [PATCH] Fix incorrect sign-extension of pointer in 32-bit acl __to_entry
Corinna Vinschen wrote: > On Jul 9 20:30, David Allsopp via Cygwin-patches wrote: > > I have some code where the acl_t returned by get_file_acl is allocated > > at 0x80038248. As a result the acl_entry_t generated by acl_get_entry > > has an "index" of -1, since the pointer was sign-extended to 64-bits. > > > > My fix is trivial and simply casts the pointer to uintptr_t first. > > Pushed. I still don't quite understand what the compiler is thinking > there, sign-extending a pointer when casted to an unsigend int type, but > your patch works, so all is well, I guess. Thank you - it is indeed hard to imagine when you'd ever want that behaviour! Would it be possible to have a snapshot with it, just for continuous integration servers which need the fix, please? David
Re: [PATCH] Fix incorrect sign-extension of pointer in 32-bit acl __to_entry
On Jul 9 20:30, David Allsopp via Cygwin-patches wrote: > I have some code where the acl_t returned by get_file_acl is allocated at > 0x80038248. As a result the acl_entry_t generated by acl_get_entry has an > "index" of -1, since the pointer was sign-extended to 64-bits. > > My fix is trivial and simply casts the pointer to uintptr_t first. Pushed. I still don't quite understand what the compiler is thinking there, sign-extending a pointer when casted to an unsigend int type, but your patch works, so all is well, I guess. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer
[PATCH] Fix incorrect sign-extension of pointer in 32-bit acl __to_entry
I have some code where the acl_t returned by get_file_acl is allocated at 0x80038248. As a result the acl_entry_t generated by acl_get_entry has an "index" of -1, since the pointer was sign-extended to 64-bits. My fix is trivial and simply casts the pointer to uintptr_t first. All best, David 0001-Fix-invalid-acl_entry_t-on-32-bit-Cygwin.patch Description: Binary data
