* Re: [PATCH 8/8] btrfs: turn on -Wmaybe-uninitialized
[not found] ` <20230222025918.GA1651385@roeck-us.net>
@ 2023-02-24 17:22 ` Guenter Roeck
[not found] ` <20230222163855.GU10580@twin.jikos.cz>
1 sibling, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2023-02-24 17:22 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs, kernel-team, regressions
Copying regzbot.
#regzbot ^introduced 1ec49744ba83
#regzbot title Build failures for sparc64:allmodconfig and parisc:allmodconfig with gcc 11.x+
On Tue, Feb 21, 2023 at 06:59:21PM -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;
>
> Bisect log attached.
>
> Guenter
>
> ---
> # bad: [8bf1a529cd664c8e5268381f1e24fe67aa611dd3] Merge tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux
> # good: [c9c3395d5e3dcc6daee66c6908354d47bf98cb0c] Linux 6.2
> git bisect start 'HEAD' 'v6.2'
> # good: [e43efb6d713bca3855909a2f9caec280a2b0a503] dt-bindings: riscv: correct starfive visionfive 2 compatibles
> git bisect good e43efb6d713bca3855909a2f9caec280a2b0a503
> # bad: [1f2d9ffc7a5f916935749ffc6e93fb33bfe94d2f] Merge tag 'sched-core-2023-02-20' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> git bisect bad 1f2d9ffc7a5f916935749ffc6e93fb33bfe94d2f
> # bad: [553637f73c314c742243b8dc5ef072e9dadbe581] Merge tag 'for-6.3/dio-2023-02-16' of git://git.kernel.dk/linux
> git bisect bad 553637f73c314c742243b8dc5ef072e9dadbe581
> # good: [274978f173276c5720a3cd8d0b6047d2c0d3a684] Merge tag 'fixes_for_v6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs
> git bisect good 274978f173276c5720a3cd8d0b6047d2c0d3a684
> # bad: [801fcfc5d790f4a9be2897713bd6dd08bed253f1] btrfs: raid56: add a bio_list_put helper
> git bisect bad 801fcfc5d790f4a9be2897713bd6dd08bed253f1
> # bad: [e2fd83064a9bae368ce1c88a0cb9aee64ad4e124] btrfs: skip backref walking during fiemap if we know the leaf is shared
> git bisect bad e2fd83064a9bae368ce1c88a0cb9aee64ad4e124
> # bad: [cb0922f264643f03b04352f7a04abb913c609369] btrfs: don't use size classes for zoned file systems
> git bisect bad cb0922f264643f03b04352f7a04abb913c609369
> # good: [cd30d3bc78d9acbd505d0d6a4cff3b87e40a8187] btrfs: zoned: fix uninitialized variable warning in btrfs_get_dev_zones
> git bisect good cd30d3bc78d9acbd505d0d6a4cff3b87e40a8187
> # bad: [235e1c7b872f9cf16e8a3e6050a05774b8763c58] btrfs: use a single variable to track return value for log_dir_items()
> git bisect bad 235e1c7b872f9cf16e8a3e6050a05774b8763c58
> # bad: [d31de3785047a24959eda835b0bafb1f8629f8a9] btrfs: go to matching label when cleaning em in btrfs_submit_direct
> git bisect bad d31de3785047a24959eda835b0bafb1f8629f8a9
> # bad: [1ec49744ba83f0429c5c706708610f7821a7b6f4] btrfs: turn on -Wmaybe-uninitialized
> git bisect bad 1ec49744ba83f0429c5c706708610f7821a7b6f4
> # good: [a6ca692ec22bdac5ae700e82ff575a1b5141fa40] btrfs: fix uninitialized variable warning in run_one_async_start
> git bisect good a6ca692ec22bdac5ae700e82ff575a1b5141fa40
> # first bad commit: [1ec49744ba83f0429c5c706708610f7821a7b6f4] btrfs: turn on -Wmaybe-uninitialized
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 8/8] btrfs: turn on -Wmaybe-uninitialized
[not found] ` <feb05eef-cc80-2fbe-f28a-b778de73b776@leemhuis.info>
@ 2023-03-12 14:37 ` Guenter Roeck
2023-03-12 14:57 ` Linux regression tracking (Thorsten Leemhuis)
2023-03-26 18:03 ` Linux regression tracking #update (Thorsten Leemhuis)
2023-03-14 21:59 ` David Sterba
1 sibling, 2 replies; 5+ messages in thread
From: Guenter Roeck @ 2023-03-12 14:37 UTC (permalink / raw)
To: Linux regressions mailing list, dsterba
Cc: Josef Bacik, linux-btrfs, kernel-team
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
Guenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 8/8] btrfs: turn on -Wmaybe-uninitialized
2023-03-12 14:37 ` Guenter Roeck
@ 2023-03-12 14:57 ` Linux regression tracking (Thorsten Leemhuis)
2023-03-26 18:03 ` Linux regression tracking #update (Thorsten Leemhuis)
1 sibling, 0 replies; 5+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-03-12 14:57 UTC (permalink / raw)
To: Guenter Roeck, Linux regressions mailing list, dsterba
Cc: Josef Bacik, linux-btrfs, kernel-team
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 8/8] btrfs: turn on -Wmaybe-uninitialized
[not found] ` <feb05eef-cc80-2fbe-f28a-b778de73b776@leemhuis.info>
2023-03-12 14:37 ` Guenter Roeck
@ 2023-03-14 21:59 ` David Sterba
1 sibling, 0 replies; 5+ messages in thread
From: David Sterba @ 2023-03-14 21:59 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Josef Bacik, linux-btrfs, kernel-team, Guenter Roeck
On Sun, Mar 12, 2023 at 02:06:40PM +0100, 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:
> 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?
I'm aware of the warnings and that it's caused by enabling the
-Wmaybe-uninitialized warning. One has a patch, IIRC there are 2-3 more,
so either there's a fix or the commit enabling the warning will be
reverted before 6.3 final.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 8/8] btrfs: turn on -Wmaybe-uninitialized
2023-03-12 14:37 ` Guenter Roeck
2023-03-12 14:57 ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-03-26 18:03 ` Linux regression tracking #update (Thorsten Leemhuis)
1 sibling, 0 replies; 5+ messages in thread
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-03-26 18:03 UTC (permalink / raw)
To: Guenter Roeck, Linux regressions mailing list, dsterba
Cc: Josef Bacik, linux-btrfs, kernel-team
[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]
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
> 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
Thx for this, Guenter, unfortunately the fix got a different subject
when it was applied. Happens, no worries, but regzbot thus needs to get
told manually:
#regzbot fix: 10a8857a1beaa015efba7d56e06243d484549fb6
#regzbot ignore-activity
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
That page also explains what to do if mails like this annoy you.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-26 18:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1671221596.git.josef@toxicpanda.com>
[not found] ` <1d9deaa274c13665eca60dee0ccbc4b56b506d06.1671221596.git.josef@toxicpanda.com>
[not found] ` <20230222025918.GA1651385@roeck-us.net>
2023-02-24 17:22 ` [PATCH 8/8] btrfs: turn on -Wmaybe-uninitialized Guenter Roeck
[not found] ` <20230222163855.GU10580@twin.jikos.cz>
[not found] ` <6c308ddc-60f8-1b4d-28da-898286ddb48d@roeck-us.net>
[not found] ` <feb05eef-cc80-2fbe-f28a-b778de73b776@leemhuis.info>
2023-03-12 14:37 ` Guenter Roeck
2023-03-12 14:57 ` Linux regression tracking (Thorsten Leemhuis)
2023-03-26 18:03 ` Linux regression tracking #update (Thorsten Leemhuis)
2023-03-14 21:59 ` David Sterba
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).