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

Open Beginner friendly
#301 3 comments 0 reactions 0 assignees View on GitHub
comp-concurrency
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.