Skip to content

Fix use-after-free warning in repl.cc#13497

Merged
Mic92 merged 1 commit intoNixOS:masterfrom
Mic92:repl-use-after-free-fix
Aug 13, 2025
Merged

Fix use-after-free warning in repl.cc#13497
Mic92 merged 1 commit intoNixOS:masterfrom
Mic92:repl-use-after-free-fix

Conversation

@Mic92
Copy link
Member

@Mic92 Mic92 commented Jul 17, 2025

NixRepl inherits from gc which uses Boehm GC for memory management. Using std::unique_ptr with GC-managed objects causes use-after-free because unique_ptr calls delete while the GC expects to manage the memory itself.

Replace std::make_unique with raw pointer allocation to let the GC handle memory management properly.

Fixes clang-tidy warning: Use of memory after it is freed [clang-analyzer-cplusplus.NewDelete]

Isolated from: #13494 because of #13494 (comment)

Motivation

Context


Add 👍 to pull requests you find important.

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

@Mic92 Mic92 requested a review from edolstra as a code owner July 17, 2025 15:41
@github-actions github-actions bot added the repl The Read Eval Print Loop, "nix repl" command and debugger label Jul 17, 2025
@NaN-git
Copy link
Contributor

NaN-git commented Jul 18, 2025

NAK. The PR does not fix any use-after-free or double-free. Instead it prevents the destructor of NixRepl from being called.

First, the life-time of the allocated objects ends when the function returns. During its life-time the unique_ptr is stored on the stack, i.e. the NixRepl instance won't be GCed. The destructor of unique_ptr calls the destructor of NixRepl.

The class gc inherited by NixRepl defines custom new and delete operators that are just wrapping GC_MALLOC and GC_FREE. Thus no destructor will be called, when the object should be GCed. Calling GC_FREE explicitly is optional, but OK, too.

Maybe the warning is caused by this line of code because the constructor of StaticEnv can throw an exception.

@Mic92 Mic92 marked this pull request as draft July 23, 2025 19:56
@Mic92
Copy link
Member Author

Mic92 commented Jul 23, 2025

Ok. I'll find a way to make turn the warning off for this line than.

The clang-analyzer incorrectly flags a use-after-free for GC-managed objects
when used with std::unique_ptr. Since NixRepl inherits from gc, its memory
is properly managed by Boehm GC and this is a false positive.

Added NOLINTNEXTLINE directive to suppress the warning.
@Mic92 Mic92 force-pushed the repl-use-after-free-fix branch from 96d7294 to 0675094 Compare August 11, 2025 07:27
@Mic92 Mic92 marked this pull request as ready for review August 11, 2025 07:28
@Mic92 Mic92 requested a review from xokdvium August 11, 2025 07:29
@Mic92 Mic92 merged commit 5d3197b into NixOS:master Aug 13, 2025
14 checks passed
@Mic92 Mic92 deleted the repl-use-after-free-fix branch August 13, 2025 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

repl The Read Eval Print Loop, "nix repl" command and debugger

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants