userver

userver copied to clipboard

В файле userver/utils/fast_pimpl.hpp в функции AsHeld после применения reinterpret_cast пропущен вызов std::launder, что приводит к UB.

T* AsHeld() noexcept { return reinterpret_cast<T*>(&storage_); }

const T* AsHeld() const noexcept {
  return reinterpret_cast<const T*>(&storage_);
}

После добавления, AsHeld нельзя (?) будет использовать в конструкторе

Нужно либо воспользоваться std::launder, либо сохранять результат placement new НО только если мы уничтожаем старый объект с константными полями и поверх него размещаем новый объект:

[basic.life]

If, after the lifetime of an object has ended and before the storage which the object occupied is reused...

FastPimpl однако не является nullable объектом, он всегда хранит один и тот же объект. А значит мы ничего не уничтожаем/завершаем и launder должен вызывать тот кто хранит FastPimpl и переиспользует память под разные FastPimpl

@apolukhin, позвольте я распишу подробнее.

По [expr.reinterpret.cast#7] reinterpret_cast суть два последовательных применения static_cast (T cv *->void cv *->U cv *), и данный пункт говорит о смене типа указателя (не более). Пункты [conv.ptr#2] и [expr.static.cast#14] говорят уже о смене значения указателя. Первый говорит, что при T cv *-> void cv * значение указателя не изменяется. Второй же устанавливает следующие условия:

1. значение исходного указателя "указывает на объект a"
2. (по данному адресу) существует объект b типа similar to T
3. объекты a и b pointer-interconvertible

при несоблюдении которых значение указателя не изменяется на "указывает на объект b", а остаётся прежним (даже если изменение типа корректно).

Что же происходит при вызове AsHeld:

1. [expr.unary.op#3]: после применения & к storage_ мы получаем указатель на массив storage_

...and points to the designated object...

2. происходит неявный static_cast к void * не меняющий значение указателя (указывает на массив storage_)
3. происходит неявный static_cast к T *, и поскольку объекты "массив std::byte" и "созданный по placement new" (в дальнейшем подразумеваемый объект) в общем случае не являются pointer-interconvertible, значение указателя не меняется (указывает на массив storage_)

То есть, получили указатель другого типа но с тем же значением, и разыменование этого указателя формирует lvalue, определяющее объект "массив std::byte",

...to which the operand points.

а не тот объект, который подразумевался. Очевидно, что использование такого указателя некорректно, причём как с целью обращения к подразумеваемому объекту ([expr.ref#8]), так и с целью обращения к массиву std::byte ([expr#basic.lval-11]), поскольку статический тип выражения и динамический тип вычисленного объекта не соответствуют в обоих случаях.

То, что вы написали про [basic.life#8], вероятно подразумевая примечание, несомненно верно, однако примечание начинается со слов "If", а не "If and only if", что ничего не говорит о том, что других случаев применения std::launder быть не может.

В данном случае std::launder как раз необходим, чтобы получить указатель, который "указывает на подразумеваемый объект".

...that points to X.

Данные выводы я сделал также благодаря дискуссии, которую я начал на std-disscussion https://lists.isocpp.org/std-discussion/2023/07/2304.php

Чёрт побери, мне казалось мы вбили последний гвоздь в крышку гроба std::launder + placement new в C++20.

Я полностью согласен с Ville Voutilainen https://lists.isocpp.org/std-discussion/2023/08/2311.php :

If the status quo wording somewhere says that's UB, then we should have an almost trivial issue submission saying "surely not".

@ddvamp заведите плиз core issue, или это могу сделать я. Убрав отсюда launder по идее починится и https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2182

P.S.: добавлять std::launder тут не хочется, так как:

• ни один компилятор не делает и (насколько мне известно) не планирует делать подобных оптимизаций - код не сломается. А если компиляторы начнут делать - то код развалится в невероятном количестве мест.
• std::launderпугает всех. Добавлять его в подобный тривиальный код - это пугать всех без видимой причины

@apolukhin к сожалению, я не знаю как это сделать. Более того, я не понимаю, что вы подразумеваете. Я смотрю на эту ситуацию с точки зрения стандарта, а не реализаций компиляторов. Не могли бы вы коротко пояснить?

@ddvamp попробуем вот так исправить https://isocpp.org/files/papers/D3006R0.html

Обновлённая версия исправления https://isocpp.org/files/papers/P3006R0.html