2

I mistakenly have dereferenced an uninitialized std::optional and only found out using valgrind (Syscall param exit_group(status) contains uninitialised byte(s)), like in this minimal working example:

#include<optional>
struct Foo {
    int i=1;
};

std::optional<Foo> bar(){
    return {};
}

int main() {
    return bar()->i;
}

What can protect me from these bugs?

Is there some macro/flag that I can define before #include<optional> in order to have at least a runtime error (make std::optional throw on every bad access or assert(this->has_value()) )? I don't care about performance here and prefer defensive programming.

phinz
  • 1,225
  • 10
  • 21
  • 3
    `optional::value` serves exactly that purpose. Why not just use this and ban/warn about asterisk operator using some linter, e.g. clang-tidy? – alagner Mar 24 '23 at 07:47
  • @alagner 1. Yes I will do this in future, actually I wrote the buggy code because I had assumed that optional does always throw and knowing the truth now, I thin I can live with this best practice (using `optional::value`). – phinz Mar 24 '23 at 07:49
  • @alagner 2. Do you have a link on this clang-tidy functionality (I don't know this capability yet). This would be great to check my existing code. – phinz Mar 24 '23 at 07:50
  • 1
    I'm afraid you might need to write it yourself: https://blog.audio-tk.com/2018/03/20/writing-custom-checks-for-clang-tidy/ – alagner Mar 24 '23 at 07:56
  • 1
    Turns out I was wrong, there is a ready-made check for it, please see the whole answer :) – alagner Mar 24 '23 at 08:10

1 Answers1

3

On a language level I am not aware on any flags/switches that would allow you to achieve it. However, such places can be detected using clang-tidy. Contrary to what I initially thought, turns out there is a ready-made check for it called bugprone-unchecked-optional-access

Interestingly enough, it's smart enough to detect if the optional has been check before accessing it:

int main() {
    int x = 0;
    x = bar()->i; //reports this
    if (auto z=bar(); z.has_value())  {
        x = z->i +1; //but not this
    }
    return x;
}

Demo

alagner
  • 3,448
  • 1
  • 13
  • 25
  • 1
    Hm... Might come with false positives if test and access are far apart in complex code. – Aconcagua Mar 24 '23 at 08:25
  • @Aconcagua I'd be more afraid of false negatives tbh (e.g. lambdas mentioned in the docs). In a worst case scenario, false-postitves can always be nolinted away, but false-negatives can cause more hassle. – alagner Mar 24 '23 at 09:05
  • Depending on how strict the checks are (*any* unchecked access annotated?) false negatives appear rather unlikely to me... – Aconcagua Mar 24 '23 at 10:32
  • Would suffice to annotate those between assignment and first test only – where passing by non-const reference or pointer would need to be considered as assignment (ignoring the fact that `const`-ness might be casted away in some scenarios...). – Aconcagua Mar 24 '23 at 10:36