eclipse-score / eclipse-score/baselibs
Undefined behaviour: (1) Placement `new` done on an existing object without destroying and (2) the return value of placement `new` is discarded
- Dominant language
- C++
- Stars
- 26
- Forks
- 85
- Avg merge
- 2d 13h
- Merged PRs (30d)
- 47
Description
The following code on [this line](https://github.com/eclipse-score/baselibs/blob/529cd6ba80cc16220e1a3bf1f491736ab07317ea/score/concurrency/future/interruptible_state.h#L82):
```c++
new (&value_) score::Result{std::move(value)};
```
This has two problems:
1. The existing object is not destroyed. FYI, the `Error` type is currently trivially destructible. If this does not hold true in the future, this code would have the undefined behaviour of constructing another object at the same location without destroying the existing object. See [this](https://stackoverflow.com/questions/20541987/is-it-dangerous-to-use-placement-new-on-an-old-object-without-explicitly-calling).
2. The return of the placement new should not be discarded. It is okay to assign it back to `value_`. To read about the issue see [this reference](https://stackoverflow.com/questions/49568858/is-it-ok-to-discard-placement-new-return-value-when-initializing-objects).
It is suggested that either the existing object be destroyed before placement new OR a new object be assigned to the existing one:
Suggested:
```c++
value_ = score::Result{std::move(value)}; //Preferred
```
The following is a possible fix but I would discourage it.
```c++
value_->~score::Result();
value_ = new (&value_) score::Result{std::move(value)};
```
The same issue is observed on all these lines also:
1. https://github.com/eclipse-score/baselibs/blob/529cd6ba80cc16220e1a3bf1f491736ab07317ea/score/concurrency/future/interruptible_state.h#L102
2. https://github.com/eclipse-score/baselibs/blob/529cd6ba80cc16220e1a3bf1f491736ab07317ea/score/concurrency/future/interruptible_state.h#L118
It may be worth checking other instances of this error and address, preferably, by creating new issues.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in score/concurrency/future/interruptible_state.h at lines 82, 102, and 118, then inspect the surrounding lifetime and assignment logic. Check the other placement-new instances noted in the issue and run the existing concurrency/future tests. Done means the reported undefined-behaviour cases are addressed without introducing new object-lifetime problems.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100