This patch is suggested by linus[1], there may be some corner cases which
should be considered before merge, and is just for prove PATCH 2.
[1]: https://lore.kernel.org/all/CAHk-=wh97QY9fEQUK6zMVQwaQ_JWDvR=R+TxQ_0OYrMHQ+egvQ@mail.gmail.com/
Signed-off-by: Xiaomeng Tong <[email protected]>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 289ce2be8..84a96ae3c 100644
--- a/Makefile
+++ b/Makefile
@@ -515,7 +515,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE \
-Werror=implicit-function-declaration -Werror=implicit-int \
-Werror=return-type -Wno-format-security \
- -std=gnu89
+ -std=gnu11 -Wno-shift-negative-value
KBUILD_CPPFLAGS := -D__KERNEL__
KBUILD_AFLAGS_KERNEL :=
KBUILD_CFLAGS_KERNEL :=
--
2.17.1
Hi Xiaomeng,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linux/master]
[also build test WARNING on vkoul-dmaengine/next soc/for-next linus/master v5.17-rc6 next-20220301]
[cannot apply to hnaz-mm/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Xiaomeng-Tong/list_for_each_entry-make-iterator-invisiable-outside-the-loop/20220301-160113
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39
config: mips-rb532_defconfig (https://download.01.org/0day-ci/archive/20220302/[email protected]/config)
compiler: mipsel-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/84ec4077430a7e8c23ea1ebc7b69e254fda25cb1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Xiaomeng-Tong/list_for_each_entry-make-iterator-invisiable-outside-the-loop/20220301-160113
git checkout 84ec4077430a7e8c23ea1ebc7b69e254fda25cb1
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=mips SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> cc1: warning: result of '-117440512 << 16' requires 44 bits to represent, but 'int' only has 32 bits [-Wshift-overflow=]
arch/mips/pci/pci-rc32434.c: In function 'rc32434_pcibridge_init':
arch/mips/pci/pci-rc32434.c:111:22: warning: variable 'dummyread' set but not used [-Wunused-but-set-variable]
111 | unsigned int dummyread, pcicntlval;
| ^~~~~~~~~
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Sincerely thank you for your reply. I also look forward to your comments
on other patches, especially PATCH 2/6 which provides new *_inside macros
to make iterator invisiable outside the list_for_each_entry* loop.
Best regards,
--
Xiaomeng Tong
On Tue, Mar 1, 2022 at 9:16 PM Linus Torvalds
<[email protected]> wrote:
>
> On Tue, Mar 1, 2022 at 10:00 AM kernel test robot <[email protected]> wrote:
> >
> > All warnings (new ones prefixed by >>):
> >
> > >> cc1: warning: result of '-117440512 << 16' requires 44 bits to represent, but 'int' only has 32 bits [-Wshift-overflow=]
>
> So that's potentially an interesting warning, but this email doesn't
> actually tell *where* that warning happens.
>
> I'm not entirely sure why this warning is new to this '-std=gnu11'
> change, but it's intriguing.
...
>
> Is there some place to actually see the full log (or some way to get a
> better pointer to just the new warning) to see that actual shift
> overflow thing?
gcc-11 only shows the one line warning here. The source is
/* PCI CFG04 status fields */
#define PCI_CFG04_STAT_BIT 16
#define PCI_CFG04_STAT 0xffff0000
#define PCI_CFG04_STAT_66_MHZ (1 << 21)
#define PCI_CFG04_STAT_FBB (1 << 23)
#define PCI_CFG04_STAT_MDPE (1 << 24)
#define PCI_CFG04_STAT_DST (1 << 25)
#define PCI_CFG04_STAT_STA (1 << 27)
#define PCI_CFG04_STAT_RTA (1 << 28)
#define PCI_CFG04_STAT_RMA (1 << 29)
#define PCI_CFG04_STAT_SSE (1 << 30)
#define PCI_CFG04_STAT_PE (1 << 31)
#define KORINA_STAT (PCI_CFG04_STAT_MDPE | \
PCI_CFG04_STAT_STA | \
PCI_CFG04_STAT_RTA | \
PCI_CFG04_STAT_RMA | \
PCI_CFG04_STAT_SSE | \
PCI_CFG04_STAT_PE)
#define KORINA_CNFG1 ((KORINA_STAT<<16)|KORINA_CMD)
unsigned int korina_cnfg_regs[25] = {
KORINA_CNFG1, /* ... */
};
This looks like an actual bug to me, the bits are shifted 16 bits twice
by accident, and it's been like this since rb532 was introduced in
2008.
Arnd
On Tue, Mar 1, 2022 at 10:00 AM kernel test robot <[email protected]> wrote:
>
> All warnings (new ones prefixed by >>):
>
> >> cc1: warning: result of '-117440512 << 16' requires 44 bits to represent, but 'int' only has 32 bits [-Wshift-overflow=]
So that's potentially an interesting warning, but this email doesn't
actually tell *where* that warning happens.
I'm not entirely sure why this warning is new to this '-std=gnu11'
change, but it's intriguing.
Instead it then gives the location for another warning:
> arch/mips/pci/pci-rc32434.c: In function 'rc32434_pcibridge_init':
> arch/mips/pci/pci-rc32434.c:111:22: warning: variable 'dummyread' set but not used [-Wunused-but-set-variable]
> 111 | unsigned int dummyread, pcicntlval;
> | ^~~~~~~~~
but that wasn't the new one (and that 'dummyread' is obviously
_intentionally_ set but not used, as implied by the name).
Is there some place to actually see the full log (or some way to get a
better pointer to just the new warning) to see that actual shift
overflow thing?
Linus
On Tue, Mar 1, 2022 at 12:54 PM Arnd Bergmann <[email protected]> wrote:
>
> gcc-11 only shows the one line warning here.
What an odd warning. Not even a filename, much less a line number?
> The source is
>
> /* PCI CFG04 status fields */
> #define PCI_CFG04_STAT_BIT 16
> #define PCI_CFG04_STAT 0xffff0000
> #define PCI_CFG04_STAT_66_MHZ (1 << 21)
> #define PCI_CFG04_STAT_FBB (1 << 23)
> #define PCI_CFG04_STAT_MDPE (1 << 24)
> #define PCI_CFG04_STAT_DST (1 << 25)
> #define PCI_CFG04_STAT_STA (1 << 27)
> #define PCI_CFG04_STAT_RTA (1 << 28)
> #define PCI_CFG04_STAT_RMA (1 << 29)
> #define PCI_CFG04_STAT_SSE (1 << 30)
> #define PCI_CFG04_STAT_PE (1 << 31)
> #define KORINA_STAT (PCI_CFG04_STAT_MDPE | \
> PCI_CFG04_STAT_STA | \
> PCI_CFG04_STAT_RTA | \
> PCI_CFG04_STAT_RMA | \
> PCI_CFG04_STAT_SSE | \
> PCI_CFG04_STAT_PE)
> #define KORINA_CNFG1 ((KORINA_STAT<<16)|KORINA_CMD)
Yeah, looks like that "<< 16" is likely just wrong.
I'm guessing that that KORINA_CNFG1 thing either ends up not
mattering, or - probably more likely - nobody really used that
platform at all. It has had pretty much zero updates sinced 2008.
Linus
On Tue, Mar 1, 2022 at 1:04 PM Linus Torvalds
<[email protected]> wrote:
>
> Yeah, looks like that "<< 16" is likely just wrong.
.. and perhaps more importantly, I guess that means that -Wshift-overflow is
(a) somehow new to -std=gnu11
(b) possibly a lot more relevant and good than that
-Wshift-negative-value thing was
doing some grepping, it seems like we have never had that
'-Wshift-overflow' even in any extra warnings.
And trying it myself (keeping -std=gnu89), enabling it doesn't report
anything on a x86-64 allmodconfig build.
So I think this is likely a good new warning that -std=gnu11 brought
in by accident. No false positives that I can see, and one report for
a MIPS bug that looks real (but admittedly not a "sky-is-falling" one
;)
There's apparently a '-Wshift-overflow=2' mode too, but that warns
about things that change the sign bit, ie expressions like
1<<31
warns.
And I would not be in the least surprised if we had a ton of those
kinds of things all over (but I didn't check).
So the plain -Wshift-overflow seems to work just fine, and while it's
surprising that it got enabled by gnu11, I think it's all good.
Famous last words.
Linus