From: "Linux regression tracking (Thorsten Leemhuis)" <regressions@leemhuis.info>
To: Guenter Roeck <linux@roeck-us.net>,
Linux regressions mailing list <regressions@lists.linux.dev>,
dsterba@suse.cz
Cc: Josef Bacik <josef@toxicpanda.com>,
linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 8/8] btrfs: turn on -Wmaybe-uninitialized
Date: Sun, 12 Mar 2023 15:57:00 +0100 [thread overview]
Message-ID: <10432722-1aa6-17ca-6375-a53a472207a8@leemhuis.info> (raw)
In-Reply-To: <caed9824-c05d-19a9-d321-edefab17c4f0@roeck-us.net>
On 12.03.23 15:37, Guenter Roeck wrote:
> On 3/12/23 06:06, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 22.02.23 18:18, Guenter Roeck wrote:
>>> On 2/22/23 08:38, David Sterba wrote:
>>>> On Tue, Feb 21, 2023 at 06:59:18PM -0800, Guenter Roeck wrote:
>>>>> On Fri, Dec 16, 2022 at 03:15:58PM -0500, Josef Bacik wrote:
>>>>>> We had a recent bug that would have been caught by a newer compiler
>>>>>> with
>>>>>> -Wmaybe-uninitialized and would have saved us a month of failing
>>>>>> tests
>>>>>> that I didn't have time to investigate.
>>>>>>
>>>>>
>>>>> Thanks to this patch, sparc64:allmodconfig and parisc:allmodconfig now
>>>>> fail to commpile with the following error when trying to build images
>>>>> with gcc 11.3.
>>>>>
>>>>> Building sparc64:allmodconfig ... failed
>>>>> --------------
>>>>> Error log:
>>>>> <stdin>:1517:2: warning: #warning syscall clone3 not implemented
>>>>> [-Wcpp]
>>>>> fs/btrfs/inode.c: In function 'btrfs_lookup_dentry':
>>>>> fs/btrfs/inode.c:5730:21: error: 'location.type' may be used
>>>>> uninitialized [-Werror=maybe-uninitialized]
>>>>> 5730 | if (location.type == BTRFS_INODE_ITEM_KEY) {
>>>>> | ~~~~~~~~^~~~~
>>>>> fs/btrfs/inode.c:5719:26: note: 'location' declared here
>>>>> 5719 | struct btrfs_key location;
>>>>
>>>> Thanks for the report, Linus warned me that there might be some
>>>> fallouts
>>>> and that the warning flag might need reverted. But before I do that I'd
>>>> like to try to understand why the warnings happen in a code where is no
>>>> reason for it.
>>>>
>>>> I did a quick test on gcc 11.3 (on x86_64, not on sparc64 unlike you
>>>> report), and there is no warning
>>>>
>>>> gcc (SUSE Linux) 11.3.1 20220721 [revision
>>>> a55184ada8e2887ca94c0ab07027617885beafc9]
>>>> Copyright (C) 2021 Free Software Foundation, Inc.
>>>> This is free software; see the source for copying conditions. There
>>>> is NO
>>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>>>> PURPOSE.
>>>>
>>>> DESCEND objtool
>>>> CALL scripts/checksyscalls.sh
>>>> CC [M] fs/btrfs/inode.o
>>>>
>>>> I.e. it's the same version, different arch and likely not the same
>>>> config. In the function itself thre's a local variable passed by
>>>> address
>>>> to a static function in the same file.
>>>>
>>>> struct btrfs_key location;
>>>> ...
>>>> ret = btrfs_inode_by_name(BTRFS_I(dir), dentry, &location,
>>>> &di_type);
>>>>
>>>> and there it's
>>>>
>>>> btrfs_dir_item_key_to_cpu(path->nodes[0], di, location);
>>>>
>>>> which is a series of helpers to read some data and store that to the
>>>> strucutre. At some point there's a call to read_extent_buffer() that's
>>>> in a different file.
>>>>
>>>> A local variable passed by address to external function is quite common
>>>> so I'd expect more warnings and I don't see what's different in this
>>>> case.
>>>
>>> Me not either. I also don't see the problem with other architectures,
>>> only
>>> with sparc and parisc. It doesn't have to be gcc 11.3, though; it also
>>> happens
>>> with gcc 11.1, 11.2, 12.1, and 12.2 (tested on sparc).
>>>
>>> Too bad that gcc doesn't tell why exactly it believes that the object
>>> may be uninitialized. Anyway, the following change would fix the
>>> problem.
>>>
>>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>>> index 6c18dc9a1831..4bab8ab39948 100644
>>> --- a/fs/btrfs/inode.c
>>> +++ b/fs/btrfs/inode.c
>>> @@ -5421,7 +5421,7 @@ static int btrfs_inode_by_name(struct btrfs_inode
>>> *dir, struct dentry *dentry,
>>> return -ENOMEM;
>>>
>>> ret = fscrypt_setup_filename(&dir->vfs_inode, &dentry->d_name,
>>> 1, &fname);
>>> - if (ret)
>>> + if (ret < 0)
>>> goto out;
>>>
>>> Presumably gcc assumes that fscrypt_setup_filename() could return
>>> a positive value.
>>
>> This discussion seems to have stalled, but from a kernelci report it
>> looks like above warning still happens:
>> https://lore.kernel.org/all/640bceb7.a70a0220.af8cd.146b@mx.google.com/
>>
>> @btrfs developers, do you still have it on your radar?
>>
>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>> --
>> Everything you wanna know about Linux kernel regression tracking:
>> https://linux-regtracking.leemhuis.info/about/#tldr
>> If I did something stupid, please tell me, as explained on that page.
>>
>> #regzbot poke
>
> There was a patch:
>
> #regzbot monitor:
> https://patchwork.kernel.org/project/linux-btrfs/patch/dc091588d770923c3afe779e1eb512724662db71.1678290988.git.sweettea-kernel@dorminy.me/
> #regzbot fix: btrfs: fix compilation error on sparc/parisc
> #regzbot ignore-activity
Ahh, great, thx.
Ciao, Thorsten
next prev parent reply other threads:[~2023-03-12 14:57 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-16 20:15 [PATCH 0/8] Fixup uninitialized warnings and enable extra checks Josef Bacik
2022-12-16 20:15 ` [PATCH 1/8] btrfs: fix uninit warning in run_one_async_start Josef Bacik
2022-12-17 0:15 ` Qu Wenruo
2022-12-19 7:51 ` Johannes Thumshirn
2022-12-20 19:03 ` David Sterba
2022-12-21 18:26 ` David Sterba
2022-12-16 20:15 ` [PATCH 2/8] btrfs: fix uninit warning in btrfs_cleanup_ordered_extents Josef Bacik
2022-12-17 0:16 ` Qu Wenruo
2022-12-19 7:53 ` Johannes Thumshirn
2022-12-16 20:15 ` [PATCH 3/8] btrfs: fix uninit warning from get_inode_gen usage Josef Bacik
2022-12-17 0:16 ` Qu Wenruo
2022-12-19 7:55 ` Johannes Thumshirn
2022-12-20 19:16 ` David Sterba
2022-12-16 20:15 ` [PATCH 4/8] btrfs: fix uninit warning in btrfs_update_block_group Josef Bacik
2022-12-17 0:16 ` Qu Wenruo
2022-12-19 7:56 ` Johannes Thumshirn
2022-12-16 20:15 ` [PATCH 5/8] btrfs: fix uninit warning in __set_extent_bit and convert_extent_bit Josef Bacik
2022-12-17 0:17 ` Qu Wenruo
2022-12-16 20:15 ` [PATCH 6/8] btrfs: extract out zone cache usage into it's own helper Josef Bacik
2022-12-19 7:05 ` Naohiro Aota
2022-12-20 19:24 ` David Sterba
2022-12-21 16:47 ` Naohiro Aota
2022-12-21 18:08 ` David Sterba
2022-12-16 20:15 ` [PATCH 7/8] btrfs: fix uninit warning in btrfs_sb_log_location Josef Bacik
2022-12-19 6:23 ` Naohiro Aota
2022-12-19 7:59 ` Johannes Thumshirn
2022-12-16 20:15 ` [PATCH 8/8] btrfs: turn on -Wmaybe-uninitialized Josef Bacik
2022-12-17 0:18 ` Qu Wenruo
2022-12-26 4:17 ` Nathan Chancellor
2022-12-26 14:04 ` Naresh Kamboju
2023-01-02 12:42 ` David Sterba
2023-02-22 2:59 ` Guenter Roeck
2023-02-22 16:38 ` David Sterba
2023-02-22 17:18 ` Guenter Roeck
2023-03-12 13:06 ` Linux regression tracking (Thorsten Leemhuis)
2023-03-12 14:37 ` Guenter Roeck
2023-03-12 14:57 ` Linux regression tracking (Thorsten Leemhuis) [this message]
2023-03-26 18:03 ` Linux regression tracking #update (Thorsten Leemhuis)
2023-03-14 21:59 ` David Sterba
2023-02-24 17:22 ` Guenter Roeck
2022-12-16 23:55 ` [PATCH 0/8] Fixup uninitialized warnings and enable extra checks Qu Wenruo
2022-12-17 0:07 ` Qu Wenruo
2022-12-20 19:37 ` David Sterba
2022-12-21 18:36 ` David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=10432722-1aa6-17ca-6375-a53a472207a8@leemhuis.info \
--to=regressions@leemhuis.info \
--cc=dsterba@suse.cz \
--cc=josef@toxicpanda.com \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=regressions@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).