[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Bogus (intptr_t) casts
From: |
Martin D Kealey |
Subject: |
Re: Bogus (intptr_t) casts |
Date: |
Tue, 6 Aug 2024 23:36:46 +1000 |
Why just those ones? Mainly:
(a) I'm looking at patching that area of the code for other reasons, so
they're the ones that I happened to encounter; and
(b) I didn't want to over-cook it, so I only included the ones where I
could see that it was actually a pointer (casting a number to an intptr_t
doesn't result in UB).
Other cases that involve casting a pointer to an intptr_t or uintptr_t, and
then comparing against a *numeric* zero should be similarly updated.
To my knowledge all current compilers use a numeric zero to represent
NULL, but this is not guaranteed, and might change in the future.
-Martin
On Tue, 6 Aug 2024, 01:17 Chet Ramey, <chet.ramey@case.edu> wrote:
> On 8/1/24 4:12 AM, Martin D Kealey wrote:
>
>
> > It follows that the following assertions are allowed to fail:
> >
> > intptr_t i = 0;
> > assert(*(void*)i == (void*)0*);
> > void *p = 0;
> > assert(*(intptr_t)p == 0*);
> >
> > Accordingly I provide the following patch:
>
> I'm wondering why you chose these two cases, since there are other very
> similar uses of intptr_t casts.
>
> >
> > diff --git a/*subst.c* b/subst.c
> > index 37e0bfa7..140a3a92 100644
> > --- a/subst.c
> > +++ b/subst.c
> > @@ -6875,7 +6875,7 @@ uw_restore_pipeline (void *discard)
> > static void
> > uw_restore_errexit (void *eflag)
> > {
> >
> > *- change_flag ('e', (intptr_t) eflag ? FLAG_ON : FLAG_OFF);+
> change_flag
> > ('e', eflag ? FLAG_ON : FLAG_OFF);*
> > set_shellopts ();
> > }
> >
> > diff --git a/*variables.c* b/variables.c
> > index cd336c85..d44453fe 100644
> > --- a/variables.c
> > +++ b/variables.c
> > @@ -5444,7 +5444,7 @@ pop_scope (void *is_special)
> > FREE (vcxt->name);
> > if (vcxt->table)
> > {
> >
> > *- if ((intptr_t) is_special)+ if (is_special)*
> > hash_flush (vcxt->table, push_builtin_var);
> > else
> > hash_flush (vcxt->table, push_exported_var);
>
> You might want to look at the unwind-protect implementation, which doesn't
> use assignments. It uses byte copies, so instead of using an assignment of,
> say, 0, where the compiler can assign whatever it wants to denote a NULL
> pointer, it copies 4-8 bytes, depending on the size of an integer. The cast
> of that memory back to an intptr_t should be transparent on all reasonably
> common systems.
>
> Of course, if you can provide an example where it fails, I'll look at it
> and fix it.
>
> --
> ``The lyf so short, the craft so long to lerne.'' - Chaucer
> ``Ars longa, vita brevis'' - Hippocrates
> Chet Ramey, UTech, CWRU chet@case.edu http://tiswww.cwru.edu/~chet/
>
>