On Mon, 16 Dec 2024 at 19:02, Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > > Hi Naresh, > > Thanks for the report. > > + Bartosz as author of ce8fd0500b74 > > On Mon, Dec 16, 2024 at 10:04:05PM +0530, Naresh Kamboju wrote: > > The arm and arm64 builds failed on Linux next-20241216 due to following > > build warnings / errors with clang-19 and clang-nightly toolchain. > > Whereas the gcc-13 builds pass. > > > > arm, arm64: > > * build/clang-19-defconfig > > * build/clang-nightly-defconfig > > > > First seen on Linux next-20241216. > > Good: next-20241216 > > Bad: next-20241213 > > > > Build log: > > ----------- > <trimmed irrelevant warning> > > drivers/crypto/qce/sha.c:365:3: error: cannot jump from this goto > > statement to its label > > 365 | goto err_free_ahash; > > | ^ > > drivers/crypto/qce/sha.c:373:6: note: jump bypasses initialization of > > variable with __attribute__((cleanup)) > > 373 | u8 *buf __free(kfree) = kzalloc(keylen + QCE_MAX_ALIGN_SIZE, > > | ^ > > 1 error generated. > > It is a bug to jump over the initialization of a cleanup variable > because the cleanup function will be called on an uninitialized pointer > in those cases. GCC does not catch this at compile time like clang does > (it would be nice if we could document this somewhere and really > encourage people doing cleanup annotations to ensure their patches pass > a clang build except in architecture code where clang does not support > that target): > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951 > > It may be worth just reverting commit ce8fd0500b74 ("crypto: qce - use > __free() for a buffer that's always freed") since it seems like little > value in this case but if we want to forward fix it, I think we could > just mirror what the rest of the kernel does and keep the declaration at > the top of the function and initialize the pointer to NULL. The diff > below resolves the issue for me, which I don't mind sending as a formal > patch. > > Cheers, > Nathan I'm fine with dropping that commit from next. Bartosz > > diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c > index c4ddc3b265ee..e251f0f9a4fd 100644 > --- a/drivers/crypto/qce/sha.c > +++ b/drivers/crypto/qce/sha.c > @@ -337,6 +337,7 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key, > struct scatterlist sg; > unsigned int blocksize; > struct crypto_ahash *ahash_tfm; > + u8 *buf __free(kfree) = NULL; > int ret; > const char *alg_name; > > @@ -370,8 +371,7 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key, > crypto_req_done, &wait); > crypto_ahash_clear_flags(ahash_tfm, ~0); > > - u8 *buf __free(kfree) = kzalloc(keylen + QCE_MAX_ALIGN_SIZE, > - GFP_KERNEL); > + buf = kzalloc(keylen + QCE_MAX_ALIGN_SIZE, GFP_KERNEL); > if (!buf) { > ret = -ENOMEM; > goto err_free_req;