Skip to content

Fix simple clang-tidy warnings#10

Open
Mic92 wants to merge 1545 commits intomasterfrom
clang-tidy-simple-warnings
Open

Fix simple clang-tidy warnings#10
Mic92 wants to merge 1545 commits intomasterfrom
clang-tidy-simple-warnings

Conversation

@Mic92
Copy link
Owner

@Mic92 Mic92 commented Jul 17, 2025

Summary

This PR fixes several simple clang-tidy warnings:

  • Fix SIZE_MAX undefined warning in fchmodat2-compat.hh
  • Fix NIX_WITH_S3_SUPPORT undefined warning in s3.hh
  • Fix clang-tidy uninitialized value warning in derivation-options.cc
  • Fix uninitialized field in Attr constructor
  • Make widechar_wcwidth inline to fix unused function warning

gustavderdrache and others added 30 commits May 27, 2025 09:18
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
…e/trace-import-from-derivation

Emit warnings for IFDs with new `trace-import-from-derivation` option
nix flake archive: add --no-check-sigs option
…e/system-builderror

Make platform checks throw BuildError like other failures
Move platform-specific code out of `DerivationBuilder`
This is the utility changes from NixOS#9968, which were easier to rebase
first.

I (@Ericson2314) didn't write this code; I just rebased it.

Co-Authored-By: Artemis Tosini <me@artem.ist>
Co-Authored-By: Audrey Dutcher <audrey@rhelmot.io>
Prepare for FreeBSD sandboxing support
These tests have been collected from nixpkgs f870c6ccc8951fc48aeb293cf3e98ade6ac42668
usage of builtins.match for x86_64-linux eval system. At most 2 matching and
non-matching cases are included for each encountered regex. This should
hopefully add more confidence when possibly trying to switch the regex implementation
in the future.
Clang doesn't like the double indent that is needed for the `if...else`
that is CPP'd away. Adding braces is fine in the `if...else...` case,
and fine as a naked block in the CPP'd away case, and properly-indented
both ways.
The method tested for in the previous commit is now deprecated.

Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Fix warning when `HAVE_EMBEDDED_SANDBOX_SHELL` is not set
…hack

Deprecate hacky way of making structured attrs
tests/functional: Add more language tests for `builtins.match`
Since the migration to meson precompiled-headers.h isn't actually used
anymore and is just confusing. Meson can't handle shared pch across
subprojects [1] and without that there's no performances benefit of PCH
at all. Also rolling our own support for that isn't trivial. See [2] for
an example of how that would look like.

[1]: mesonbuild/meson#4350
[2]: https://github.com/WayfireWM/wayfire/blob/22bc8b647351c45829baac570b828c8f852111dc/plugins/meson.build
Judging by the comment for `makeEmptySourceAccessor` the prefix has
to be empty:

> Return a source accessor that contains only an empty root directory.

Fixes NixOS#13295.
This restores doing seccomp/personality initialization even when
sandboxing is disabled.

https://hydra.nixos.org/build/298482132
`pre-commit` builds fine with the flake's input nixpkgs
on i686-linux.
The underlying bug seems to have been fixed in diffoscope 293 [1] [2].
Our nixpkgs input has 295.

[1]: NixOS/nixpkgs#393381 (comment)
[2]: https://diffoscope.org/news/diffoscope-292-released/
…ding

flake: Restore `packaging-overriding` check
roberth and others added 27 commits July 10, 2025 17:50
c39cc00 has added assertions for
all Value accesses and the following case has started failing with
an `unreachable`:

(/tmp/fun.nix):

```nix
{a}: a
```

```
$ nix eval --impure --expr 'import /tmp/fun.nix {a="a";b="b";}'
```

This would crash:

```
terminating due to unexpected unrecoverable internal error: Unexpected condition in getStorage at ../include/nix/expr/value.hh:844
```

This is not a regression, but rather surfaces an existing problem, which previously
was left undiagnosed. In the case of an import `fun` is the `import` primOp, so that read is invalid
and previously this resulted in an access into an inactive union member, which is UB.
The correct thing to use is `vCur`. Identical problem also affected the case of a missing argument.

Add previously failing test cases to the functional/lang test suite.

Fixes NixOS#13448.
libexpr: Fix invalid handling of errors for imported functions
This works around the macOS issue that the prior commit addresses.
…e/fix-sandbox-ifdef

Address ifdef problem with macOS/BSD sandboxing
When AWS credentials expired, in some scenarios they led to the
nix process aborting with an error similar to ' Unable to parse ExceptionName: ExpiredToken'.

This change updates the S3 handling code such that those errors are treated like 403s or 404s.

Closes NixOS#13459
Fix documentation for GC w.r.t. symlinks
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
We’re already allowing `/tmp` anyway, so this should be harmless,
and it fixes a regression in the default configuration caused by
moving the build directories out of `temp-dir`. (For instance, that
broke the Lix `guessOrInventPath.sockets` test.)

Note that removing `/tmp` breaks quite a few builds, so although it may
be a good idea in general it would require work on the Nixpkgs side.

Fixes: 749afbb
Change-Id: I6a6a69645f429bc50d4cb24283feda3d3091f534

(This is a cherry-pick of commit d1db3e5)

Lix patch: https://gerrit.lix.systems/c/lix/+/3500
libstore: fix Unix sockets in the build directory on sandboxed macOS
This should provide more coverage for the build from master that
is being dogfooded.
docker: set default parameters values
Include <cstdint> to ensure SIZE_MAX is defined
The s3.hh public header was incorrectly including store-config-private.hh
instead of the public config.hh. Since NIX_WITH_S3_SUPPORT is defined in
the public config, this caused clang-tidy to report it as undefined.
Make lambda capture explicit to avoid clang-analyzer-core.CallAndMessage warning
The default constructor for Attr was not initializing the value pointer,
which could lead to undefined behavior when the uninitialized pointer is
accessed. This was caught by clang-tidy's UninitializedObject check.

This fixes the warning:
  1 uninitialized field at the end of the constructor call
  [clang-analyzer-optin.cplusplus.UninitializedObject]
@Mic92 Mic92 force-pushed the clang-tidy-simple-warnings branch 2 times, most recently from 42a7d8d to 3c0cd73 Compare July 17, 2025 17:18
Mic92 pushed a commit that referenced this pull request Aug 11, 2025
This was removed in NixOS#11152. However,
we need it for the multi-threaded evaluator, because otherwise Boehm
GC will crash while scanning the thread stack:

  #0  GC_push_all_eager (bottom=<optimized out>, top=<optimized out>) at extra/../mark.c:1488
  #1  0x00007ffff74691d5 in GC_push_all_stack_sections (lo=<optimized out>, hi=<optimized out>, traced_stack_sect=0x0) at extra/../mark_rts.c:704
  #2  GC_push_all_stacks () at extra/../pthread_stop_world.c:876
  #3  GC_default_push_other_roots () at extra/../os_dep.c:2893
  #4  0x00007ffff746235c in GC_mark_some (cold_gc_frame=0x7ffee8ecaa50 "`\304G\367\377\177") at extra/../mark.c:374
  #5  0x00007ffff7465a8d in GC_stopped_mark (stop_func=stop_func@entry=0x7ffff7453c80 <GC_never_stop_func>) at extra/../alloc.c:875
  #6  0x00007ffff7466724 in GC_try_to_collect_inner (stop_func=0x7ffff7453c80 <GC_never_stop_func>) at extra/../alloc.c:624
  #7  0x00007ffff7466a22 in GC_collect_or_expand (needed_blocks=needed_blocks@entry=1, ignore_off_page=ignore_off_page@entry=0, retry=retry@entry=0) at extra/../alloc.c:1688
  #8  0x00007ffff746878f in GC_allocobj (gran=<optimized out>, kind=<optimized out>) at extra/../alloc.c:1798
  #9  GC_generic_malloc_inner (lb=<optimized out>, k=k@entry=1) at extra/../malloc.c:193
  #10 0x00007ffff746cd40 in GC_generic_malloc_many (lb=<optimized out>, k=<optimized out>, result=<optimized out>) at extra/../mallocx.c:477
  #11 0x00007ffff746cf35 in GC_malloc_kind (bytes=120, kind=1) at extra/../thread_local_alloc.c:187
  #12 0x00007ffff796ede5 in nix::allocBytes (n=<optimized out>, n=<optimized out>) at ../src/libexpr/include/nix/expr/eval-inline.hh:19

This is because it will use the stack pointer of the coroutine, so it
will scan a region of memory that doesn't exist, e.g.

  Stack for thread 0x7ffea4ff96c0 is [0x7ffe80197af0w,0x7ffea4ffa000)

(where 0x7ffe80197af0w is the sp of the coroutine and 0x7ffea4ffa000
is the base of the thread stack).

We don't scan coroutine stacks, because currently they don't have GC
roots (there is no evaluation happening in coroutines). So there is
currently no need to restore the other parts of the original patch,
such as BoehmGCStackAllocator.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.