Skip to content

fix(libutil/posix-source-accessor.cc): get rid of use-after-move bug (backport #11837)#11838

Merged
Mic92 merged 1 commit into2.24-maintenancefrom
mergify/bp/2.24-maintenance/pr-11837
Nov 8, 2024
Merged

fix(libutil/posix-source-accessor.cc): get rid of use-after-move bug (backport #11837)#11838
Mic92 merged 1 commit into2.24-maintenancefrom
mergify/bp/2.24-maintenance/pr-11837

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify bot commented Nov 8, 2024

Naming class member variables the same as constructor arguments is a very slippery slope because of how member variable names get resolved. Compiler is not very helpful here and we need static analysis to forbid this kind of stuff.

The following example illustrates the cause quite well:

struct B {
    B(int) {}
};

struct A {
    A(int b): b([&](){
        return b;
        static_assert(std::is_same_v<decltype(b), int>);
    }()) {
       static_assert(std::is_same_v<decltype(b), int>);
    }
    void member() {
        static_assert(std::is_same_v<decltype(b), B>);
    }
    B b;
};

int main() {
    A(1).member();
}

From N4861 6.5.1 Unqualified name lookup:

In all the cases listed in [basic.lookup.unqual], the scopes are searched
for a declaration in the order listed in each of the respective categories;
name lookup ends as soon as a declaration is found for the name.
If no declaration is found, the program is ill-formed.

In the affected code there was a use-after-move for all accessses in the constructor body, but this UB wasn't triggered.

These types of errors are trivial to catch via clang-tidy's [clang-analyzer-cplusplus.Move].

Motivation

UB is bad.

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.


This is an automatic backport of pull request #11837 done by [Mergify](https://mergify.com).

Naming class member variables the same as constructor arguments is a very
slippery slope because of how member variable names get resolved. Compiler
is not very helpful here and we need static analysis to forbid this kind of
stuff.

The following example illustrates the cause quite well:

```cpp

struct B {
    B(int) {}
};

struct A {
    A(int b): b([&](){
        return b;
        static_assert(std::is_same_v<decltype(b), int>);
    }()) {
       static_assert(std::is_same_v<decltype(b), int>);
    }
    void member() {
        static_assert(std::is_same_v<decltype(b), B>);
    }
    B b;
};

int main() {
    A(1).member();
}
```

From N4861 6.5.1 Unqualified name lookup:

> In all the cases listed in [basic.lookup.unqual], the scopes are searched
> for a declaration in the order listed in each of the respective categories;
> name lookup ends as soon as a declaration is found for the name.
> If no declaration is found, the program is ill-formed.

In the affected code there was a use-after-move for all accesses in the constructor
body, but this UB wasn't triggered.

These types of errors are trivial to catch via clang-tidy's [clang-analyzer-cplusplus.Move].

(cherry picked from commit 3e0129c)
@mergify mergify bot requested a review from edolstra as a code owner November 8, 2024 13:58
@mergify mergify bot added the merge-queue label Nov 8, 2024
@Mic92 Mic92 merged commit ef21dfa into 2.24-maintenance Nov 8, 2024
@Mic92 Mic92 deleted the mergify/bp/2.24-maintenance/pr-11837 branch November 8, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants