ci: bump nixpkgs to latest nixpkgs-unstable + use poetry 1.4.0

[prusnak]

• bumping nixpkgs alllows us to use poetry 1.4.0
• regenerate poetry.lock file (with --no-update)

Fixes #2805

[prusnak]

ISTM the env in GitLab needs to be regenerated, otherwise the tests will fail?

[mmilata]

Possibly a temporary GitLab problem. I re-ran the pipeline and got past this to a bunch of errors in legacy build.

https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/pipelines/810452875

[matejcik]

legacy fails because protobuf version in poetry does not match protoc version in the nix env.
upgrading protobuf package messes up nanopb in a way that is maybe fixable but maybe not ... my Christmas wish is to have a full-featured proto file parser in Python without the fragile code generator step 😿

could we downgrade protoc easily?

[matejcik]

got a workaround in b780936

but for that matter, with the above nix env I have poetry 1.3.2, i thought this was supposed to be 1.4.0 ?

[mmilata]

could we downgrade protoc easily?

We can downgrade to 3.20, 3.19, 3.17 and 3.8 very easily, and fairly easily to whatever is in nixpkgs used by the current master.

[matejcik]

3.19 is what we had before so let's go with that.
the workaround worked for me locally but it's still failing in CI so 🤷‍♀️

[mmilata]

Pinned to protobuf-3.19 in 41865a7, bumped to today's nixpkgs-unstable in 0e89c4f as it seems to contain poetry-1.4.0.

[matejcik]

why did this fail so horribly? do we need to regenerate the environment after all?

[mmilata]

Seems like poetry-side issue, perhaps clearing gitlab cache could help?

@vdovhanych can you please clear it and retrigger this branch? https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/pipelines

[matejcik]

we're left with:
https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/jobs/3974380500

In file included from /nix/store/1zsc48wwlplpkzms83m7zr94xnfalq2q-glibc-2.35-224-dev/include/string.h:535,
                 from cardano.c:25:
In function ‘memcpy’,
    inlined from ‘hdnode_from_secret_cardano’ at cardano.c:162:3:
/nix/store/1zsc48wwlplpkzms83m7zr94xnfalq2q-glibc-2.35-224-dev/include/bits/string_fortified.h:29:10: error: ‘__builtin_memcpy’ offset [0, 31] is out of the bounds [0, 0] [-Werror=array-bounds]
   29 |   return __builtin___memcpy_chk (__dest, __src, __len,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   30 |                                  __glibc_objsize0 (__dest));
      |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~

and some weird btconly failures:
https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/jobs/3974380625
https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/jobs/3974380691

$ $NIX_SHELL --run "poetry run ./tools/check-bitcoin-only legacy/firmware/trezor.bin"
zUYPYNYZYXYbY`YgYlYiYxY
Wsjshspsxsus{szs
ERROR: Altcoin strings found in Bitcoin-only firmware.

not worried about matching some altcoins that will be gone very soon(tm)
but wtf are those strings??

[matejcik]

rebased and took the chance to improve check-bitcoin-only.
i can't reproduce the crypto build failure locally 🤷‍♀️ so i want to see if it went away by itself after the rebase

[matejcik]

managed to reproduce it, it only manifests with ADDRESS_SANTIZER=1

wtf, this is the offending line: https://github.com/trezor/trezor-firmware/blob/master/crypto/cardano.c#L162
this works:

  memcpy(out->private_key_extension, secret, 32);

this works:

  memcpy(out->private_key, secret + 32, 32);

this doesn't:

  memcpy(out->private_key_extension, secret + 32, 32);

either there's miscompilation or the ASAN thing is getting confused, or I don't know
@onvej-sl or @hiviah any ideas? this might be in your purview

[prusnak]

Btw, I just noticed that pyright is already a part of nixpkgs, so we might want to drop our custom code.

[mmilata]

Hypothesis: ASan instrumentation confuses gcc's array-bounds analysis causing it to emit false positive. We can opt hdnode_from_secret_cardano out of ASan, not sure if it's possible the other way: https://github.com/google/sanitizers/wiki/AddressSanitizer#turning-off-instrumentation

Still might be worth investigating further in case it's a gcc bug.

[mmilata]

We used to require a very specific version of pyright while nixpkgs has whatever's the latest on NPM. Is it still the case or can we now use the latter? (cc @grdddj)

We currently have 1.1.204, nixpkgs has 1.1.298.

[matejcik]

We currently have 1.1.204, nixpkgs has 1.1.298.

we don't typecheck with that version. haven't looked into whether it's easily fixable or not

[grdddj]

We used to require a very specific version of pyright while nixpkgs has whatever's the latest on NPM. Is it still the case or can we now use the latter? (cc @grdddj)

We currently have 1.1.204, nixpkgs has 1.1.298.

It would be nice to increase the version, it might bring some improvements (enable us to do less type: ignores, etc.)

So I think we can use the 1.1.298 - and as is mentioned above - we will not need our custom code specifying the version

[mmilata]

Hypothesis: ASan instrumentation confuses gcc

__attribute__((no_sanitize_address)) doesn't seem to help so that's probably not it.

[onvej-sl]

either there's miscompilation or the ASAN thing is getting confused, or I don't know

You found a gcc bug!

This is a minimum working failing example:

gcc -fsanitize=undefined -Werror=array-bounds bug.c

#include <string.h>

#define LENGTH 17

typedef struct {
  char first[LENGTH];
  char second[LENGTH];
  char third[LENGTH];
} triple;

void bug(char in[3 * LENGTH], triple *out) {
  memcpy(out->first, in, LENGTH);
  memcpy(out->second, in + LENGTH, LENGTH);
  memcpy(out->third, in + 2 * LENGTH, LENGTH);
}

The gcc that is used to compile the tests is pulled by the rustNightly dependency in shell.nix. And this build of gcc seems to be buggy. So we can either

• update rustProfiles (if it doesn't break anything) or
• introduce gcc dependency and force the tests to use it.

[onvej-sl]

Updating rustProfiles doesn't seem to fix it. The same version of gcc (12.2.0) from gcc seems to be ok.

[mmilata]

Forced use of GCC 11 in d38d80c, seems to work now. @onvej-sl did you report this bug to GCC?

[matejcik]

ahhh, the new clippy with the unnecessary_cast lint :))

[onvej-sl]

@onvej-sl did you report this bug to GCC?

No, I didn't.