3047. atomic compound assignment operators can cause undefined behavior when corresponding fetch_meow members don't

Section: 32.5.8.3 [atomics.types.int], 32.5.8.5 [atomics.types.pointer], 32.5.8.6 [atomics.types.memop] Status: New Submitter: Tim Song Opened: 2017-12-15 Last modified: 2020-09-06

Priority: 3

View all issues with New status.

Discussion:

Given atomic<int> meow{INT_MAX};, meow.fetch_add(1) has well-defined behavior because 32.5.8.3 [atomics.types.int] p7 says that

Remarks: For signed integer types, arithmetic is defined to use two's complement representation. There are no undefined results.

but meow += 1 and ++meow have undefined behavior, because these operator functions are defined (by, respectively, 32.5.8.3 [atomics.types.int] p8 and 32.5.8.6 [atomics.types.memop]) to be equivalent to return fetch_add(1) + 1;, and so the addition of 1 to the result of fetch_add — which causes an integer overflow in this case — occurs outside the protection of fetch_add magic. Additionally, the return value might differ from what fetch_add actually wrote since that addition isn't required to use two's complement. This seems like a trap for the unwary. Is it intended?

A similar issue affects the atomic<T*> partial specialization for pointers.

[2018-01; Priority set to 3 after mailing list discussion]

[2019-04-15; JF Bastien comments and provides wording]

As discussed by LWG during the San Diego 2018 meeting, Jens removed LWG 3047 from "P1236R1: Alternative Wording for P 0907R4 Signed Integers are Two's Complement".

Proposed resolution:

This wording is relative to N4810.

  1. Modify 32.5.7.3 [atomics.ref.int] as indicated:

    integral operator op=(integral operand) const noexcept;

    -7- Effects: Equivalent to: return static_cast<integral>(static_cast<make_unsigned_t<integral>>(fetch_key(operand)) op static_cast<make_unsigned_t<integral>>(operand));

  2. Modify 32.5.7.6 [atomics.ref.memop] as indicated:

    T* operator++() const noexcept;

    -3- Effects: Equivalent to: return static_cast<T>(static_cast<make_unsigned_t<T>>(fetch_add(1)) + static_cast<make_unsigned_t<T>>(1)); 

    T* operator--(int) const noexcept;

    -4- Effects: Equivalent to: return static_cast<T>(static_cast<make_unsigned_t<T>>(fetch_sub(1)) - static_cast<make_unsigned_t<T>>(1));

  3. Modify 32.5.8.3 [atomics.types.int] as indicated:

    T operator op=(T operand) volatile noexcept;
T operator op=(T operand) noexcept;

    -8- Effects: Equivalent to: return static_cast<T>(static_cast<make_unsigned_t<T>>(fetch_key(operand)) op static_cast<make_unsigned_t<T>>(operand));

    [Drafting note: atomic<integral>'s working for operator++/operator-- is shared with atomic<T*>. — end drafting note]

    [Drafting note: atomic<floating-point> seems to be correct, LWG should confirm that it is. — end drafting note]

  4. Modify 32.5.8.5 [atomics.types.pointer] as indicated:

    T* operator op=(ptrdiff_t operand) volatile noexcept;
T* operator op=(ptrdiff_t operand) noexcept;

    -8- Effects: Equivalent to: return reinterpret_cast<T*>(reinterpret_cast<ptrdiff_t>(fetch_key(operand)) op operand);

    Remarks: The result may be an undefined address, but the operations otherwise have no undefined behavior.

  5. Modify 32.5.8.6 [atomics.types.memop] as indicated:

    T operator++() volatile noexcept;
T operator++() noexcept;

    -3- Effects: Equivalent to: return static_cast<T>(static_cast<make_unsigned_t<T>>(fetch_add(1)) + static_cast<make_unsigned_t<T>>(1)); 

    T operator--() volatile noexcept;
T operator--() noexcept;

    -4- Effects: Equivalent to: return static_cast<T>(static_cast<make_unsigned_t<T>>(fetch_sub(1)) - static_cast<make_unsigned_t<T>>(1));

    [Drafting note: Alternatively, LWG may want to separate the integral overload of operator++/operator-- from that of atomic<T*>. end drafting note]