All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Rix <trix@redhat.com>
To: Zheng Zengkai <zhengzengkai@huawei.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Howells <dhowells@redhat.com>,
	Hulk Robot <hulkci@huawei.com>,
	linux-afs@lists.infradead.org,
	Marc Dionne <marc.dionne@auristor.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] afs: fix no return statement in function returning non-void
Date: Wed, 16 Jun 2021 05:56:39 -0700	[thread overview]
Message-ID: <200ea6f7-0182-9da1-734c-c49102663ccc@redhat.com> (raw)
In-Reply-To: <051421e0-afe8-c6ca-95cd-4dc8cd20a43e@huawei.com>


On 6/15/21 8:15 PM, Zheng Zengkai wrote:
> Oops, Sorry for the late reply and missing the compilation details.
>
>> On 6/15/21 5:32 PM, Linus Torvalds wrote:
>>> On Tue, Jun 15, 2021 at 4:58 PM Randy Dunlap <rdunlap@infradead.org> 
>>> wrote:
>>>> Some implementations of BUG() are macros, not functions,
>>> Not "some", I think. Most.
>>>
>>>> so "unreachable" is not applicable AFAIK.
>>> Sure it is. One common pattern is the x86 one:
>>>
>>>    #define BUG()                                                   \
>>>    do {                                                            \
>>> instrumentation_begin();                                \
>>>            _BUG_FLAGS(ASM_UD2, 0);                                 \
>>> unreachable();                                          \
>>>    } while (0)
>> duh.
>>
>>> and that "unreachable()" is exactly what I'm talking about.
>>>
>>> So I repeat: what completely broken compiler / config / architecture
>>> is it that needs that "return 0" after a BUG() statement?
>> I have seen it on ia64 -- most likely GCC 9.3.0, but I'll have to
>> double check that.
>
> Actually we build the kernel with the following compiler, config and 
> architecture :
>
> powerpc64-linux-gnu-gcc --version
> powerpc64-linux-gnu-gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
> Copyright (C) 2019 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.
>
> CONFIG_AFS_FS=y
> # CONFIG_AFS_DEBUG is not set
> CONFIG_AFS_DEBUG_CURSOR=y
>
> make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu- -j64
>
> ...
>
> fs/afs/dir.c: In function ‘afs_dir_set_page_dirty’:
> fs/afs/dir.c:51:1: error: no return statement in function returning 
> non-void [-Werror=return-type]
>    51 | }
>       | ^
> cc1: some warnings being treated as errors
>
powerpc64 gcc 10.3.1 is what I used to find this problem.

A fix is to use the __noreturn attribute on this function and not add a 
return like this

-static int afs_dir_set_page_dirty(struct page *page)
+static int __noreturn afs_dir_set_page_dirty(struct page *page)

and to the set of ~300 similar functions in these files.

$ grep -r -P "^\tBUG\(\)" .

If folks are ok with this, I'll get that set started.

Tom

>>> Because that environment is broken, and the warning is bogus and wrong.
>>>
>>> It might not be the compiler. It might be some architecture that does
>>> this wrong. It might be some very particular configuration that does
>>> something bad and makes the "unreachable()" not work (or not exist).
>>>
>>> But *that* is the bug that should be fixed. Not adding a pointless and
>>> incorrect line that makes no sense, just to hide the real bug.
>> .
>>
>


  reply	other threads:[~2021-06-16 12:56 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15 11:55 [PATCH] afs: fix no return statement in function returning non-void David Howells
2021-06-15 12:03 ` David Howells
2021-06-15 14:49 ` Linus Torvalds
2021-06-15 23:58   ` Randy Dunlap
2021-06-16  0:32     ` Linus Torvalds
2021-06-16  1:38       ` Randy Dunlap
2021-06-16  2:19         ` Randy Dunlap
2021-06-16  3:15         ` Zheng Zengkai
2021-06-16 12:56           ` Tom Rix [this message]
2021-06-16 14:34             ` Linus Torvalds
2021-06-16 16:22               ` Tom Rix
2021-06-16 16:29                 ` Linus Torvalds
2021-06-18 15:23                   ` Arnd Bergmann
2021-06-16 13:41           ` David Howells
  -- strict thread matches above, loose matches on Subject: below --
2021-03-27 12:16 Zheng Zengkai
2021-03-31  2:32 ` Zheng Zengkai
2021-04-08 14:06 ` David Howells
2021-04-19 22:31   ` Randy Dunlap
2021-05-27 19:48     ` Randy Dunlap

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=200ea6f7-0182-9da1-734c-c49102663ccc@redhat.com \
    --to=trix@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=hulkci@huawei.com \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.dionne@auristor.com \
    --cc=rdunlap@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=zhengzengkai@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.