bug-bash
[Top][All Lists]
Advanced

[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/
>
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]