Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #162587

Re: Working code

Show all headers | View raw

On 9/2/21 8:37 AM, Ben Bacarisse wrote:
> Richard Damon <Richard@Damon-Family.org> writes:
> 
>> On 9/2/21 6:54 AM, Ben Bacarisse wrote:
>>> Ike Naar <ike@rie.sdf.org> writes:
>>>
>>>> On 2021-09-02, Andrey Tarasevich <andreytarasevich@hotmail.com> wrote:
>>>>> On 9/1/2021 4:58 PM, Tim Rentsch wrote:
>>>>>>> The following is simplified version of "working code":  it
>>>>>>> is more than 20 years old (probably closer to 30 years old)
>>>>>>> and apparently did what its author intended:
>>>>>>>
>>>>>>> int (*handlers[2])();
>>>>>>>
>>>>>>> #define DO_IT(n)   ((p = handlers[n]) ? (*p)() : 0)
>>>>>>>
>>>>>>> static void foo() {
>>>>>>>      register int (*p)();
>>>>>>>      register int bar = DO_IT(0) | DO_IT(1);
>>>>>>>      /* ... */
>>>>>>>      /* Use of bar */
>>>>>>> }
>>>>>>>
>>>>>>> /* Code that fills 'handlers' and calls foo() */
>>>>>>>
>>>>>>> I wonder what you think about this code?
>>>>>>
>>>>>> I think it has undefined behavior, and needlessly so:
>>>>>>
>>>>>>      #define DO_IT(n)   (handlers[n] ? handlers[n]() : 0)
>>>>>>
>>>>>>      ...
>>>>>
>>>>> That intermediate variable `p` is indeed completely unnecessary. But 
>>>>> where did you find undefined behavior in the original version?
>>>>
>>>> In the expression 'DO_IT(0) | DO_IT(1);' p is being modified more than once.
>>>> The behaviour of the expression depends on how its subexpressions are
>>>> sequenced.
>>>
>>> There is a sequence point after each assignment.  Am I missing
>>> something?
>>
>> No, there is a dependency that that assignment can't happen until after
>> the right hand side is evalutated, but no full sequence point.
> 
> What's a full sequence point?
> 
>> There is no sequencing between the assignments of DO_IT(0) and DO_IT(1)
>> and very importantly between the (*p)()s, both assignements can happen
>> then both (*p)() happen, using whichever value of p was assigned last.
> 
> There is one between each assignment and the corresponding use, but you
> are saying that that is not enough.  Have I got that about right?  Is
> this derived from the new C11 "sequenced before" wording that replaced
> the old wording C99 or was it UB in C99 as well?
> 

Expanding the macros, the relevant expression becomes

    ((p = handlers[0]) ? (*p)() : 0)|((p = handlers[1]) ? (*p)() : 0)

That contains the following sub-expressions involving p:

A: p = handlers[0]
B: (*p)()
C: p = handlers[1]
D: (*p)()

There's a sequence point associated with the first ?: expression
separating A and B(6.5.15p4), guaranteeing that A is sequenced before B
(5.1.2.3p3) . There's a sequence point associated with the second ?:
expression separating C and D, guaranteeing that C is sequenced before
D. There is no sequence point associated with the |, so the two
evaluations of DO_IT() are unsequenced relative to each other.
Importantly, this means that it is permissible for the evaluation of the
two ?: expressions to be interleaved (see footnote 13).
In itself, this would be bad enough, because the possibility of
interleaving those evaluations means that, in each of the two places
where (*p)() is evaluated, there's no guarantee whether p has the value
handlers[0] or handlers[1].
However, because A and B both assign values to the same variable, the
fact that they are not sequenced relative to each other has worse
implications: the behavior is undefined (6.5p2).

While I've worded this argument in terms of the new "sequencing"
terminology introduced in C2011, the same conclusion applied (though
less clearly) in older versions of the standard. The relevant clause was
one I have occasionally referred to as one of the most confusing in the
standard: "Between the previous and next sequence point an object shall
have its stored value modified at most once by the evaluation of an
expression. 72) Furthermore, the prior value shall be read only to
determine the value to be stored." (C99, 6.5p2).
That version of the standard didn't explicitly permit interleaved
evaluation, it simply failed to prohibit it. A defect report resolution
confirmed that failing to prohibit interleaving was deliberate.

Back to comp.lang.c | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Working code antispam@math.uni.wroc.pl - 2021-09-01 22:23 +0000
  Re: Working code Tim Rentsch <tr.17687@z991.linuxsc.com> - 2021-09-01 16:58 -0700
    Re: Working code Andrey Tarasevich <andreytarasevich@hotmail.com> - 2021-09-01 20:57 -0700
      Re: Working code Ike Naar <ike@rie.sdf.org> - 2021-09-02 05:22 +0000
        Re: Working code Andrey Tarasevich <andreytarasevich@hotmail.com> - 2021-09-01 22:33 -0700
        Re: Working code Ben Bacarisse <ben.usenet@bsb.me.uk> - 2021-09-02 11:54 +0100
          Re: Working code Richard Damon <Richard@Damon-Family.org> - 2021-09-02 07:08 -0400
            Re: Working code Ben Bacarisse <ben.usenet@bsb.me.uk> - 2021-09-02 13:37 +0100
              Re: Working code James Kuyper <jameskuyper@alumni.caltech.edu> - 2021-09-02 12:03 -0400
              Re: Working code Tim Rentsch <tr.17687@z991.linuxsc.com> - 2021-09-02 09:57 -0700
  Re: Working code Andrey Tarasevich <andreytarasevich@hotmail.com> - 2021-09-01 21:02 -0700
    Re: Working code John McCue <jmccue@obsd2.mhome.org> - 2021-09-02 13:31 +0000

csiph-web