Re: drivers/irqchip/irq-apple-aic.c:941 aic_of_ic_init() error: uninitialized symbol 'off'.

Marc Zyngier
2023-12-04T10:32:53Z

On Mon, 04 Dec 2023 06:41:46 +0000,
Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
> commit: dc97fd6fec009957e81026055fc99a03877ff3b8 irqchip/apple-aic: Dynamically compute register offsets
> config: arm64-randconfig-r081-20231127 (https://download.01.org/0day-ci/archive/20231203/202312032327.J915WcaL-lkp@intel.com/config)
> compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
> reproduce: (https://download.01.org/0day-ci/archive/20231203/202312032327.J915WcaL-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202312032327.J915WcaL-lkp@intel.com/
>
> New smatch warnings:
> drivers/irqchip/irq-apple-aic.c:941 aic_of_ic_init() error: uninitialized symbol 'off'.
>
> Old smatch warnings:
> drivers/irqchip/irq-apple-aic.c:920 aic_of_ic_init() warn: possible memory leak of 'irqc'
> drivers/irqchip/irq-apple-aic.c:998 aic_of_ic_init() warn: 'regs' from of_iomap() not released on lines: 914,920.
>
> vim +/off +941 drivers/irqchip/irq-apple-aic.c
>
> 76cde26394114f Hector Martin 2021-01-21 900 static int __init aic_of_ic_init(struct device_node *node, struct device_node *parent)
> 76cde26394114f Hector Martin 2021-01-21 901 {
> 76cde26394114f Hector Martin 2021-01-21 902 int i;
> dc97fd6fec0099 Hector Martin 2022-03-10 903 u32 off;
> 76cde26394114f Hector Martin 2021-01-21 904 void __iomem *regs;
> 76cde26394114f Hector Martin 2021-01-21 905 struct aic_irq_chip *irqc;
> 2cf68211664acd Hector Martin 2022-03-10 906 const struct of_device_id *match;
> 76cde26394114f Hector Martin 2021-01-21 907
> 76cde26394114f Hector Martin 2021-01-21 908 regs = of_iomap(node, 0);
> 76cde26394114f Hector Martin 2021-01-21 909 if (WARN_ON(!regs))
> 76cde26394114f Hector Martin 2021-01-21 910 return -EIO;
> 76cde26394114f Hector Martin 2021-01-21 911
> 76cde26394114f Hector Martin 2021-01-21 912 irqc = kzalloc(sizeof(*irqc), GFP_KERNEL);
> 76cde26394114f Hector Martin 2021-01-21 913 if (!irqc)
> 76cde26394114f Hector Martin 2021-01-21 914 return -ENOMEM;
> 76cde26394114f Hector Martin 2021-01-21 915
> 76cde26394114f Hector Martin 2021-01-21 916 irqc->base = regs;
> 76cde26394114f Hector Martin 2021-01-21 917
> 2cf68211664acd Hector Martin 2022-03-10 918 match = of_match_node(aic_info_match, node);
> 2cf68211664acd Hector Martin 2022-03-10 919 if (!match)
> 2cf68211664acd Hector Martin 2022-03-10 920 return -ENODEV;
> 2cf68211664acd Hector Martin 2022-03-10 921
> 2cf68211664acd Hector Martin 2022-03-10 922 irqc->info = *(struct aic_info *)match->data;
> 2cf68211664acd Hector Martin 2022-03-10 923
> 2cf68211664acd Hector Martin 2022-03-10 924 aic_irqc = irqc;
> 2cf68211664acd Hector Martin 2022-03-10 925
> dc97fd6fec0099 Hector Martin 2022-03-10 926 switch (irqc->info.version) {
> dc97fd6fec0099 Hector Martin 2022-03-10 927 case 1: {
> dc97fd6fec0099 Hector Martin 2022-03-10 928 u32 info;
> dc97fd6fec0099 Hector Martin 2022-03-10 929
> 76cde26394114f Hector Martin 2021-01-21 930 info = aic_ic_read(irqc, AIC_INFO);
> 7c841f5f6fa3f9 Hector Martin 2022-03-10 931 irqc->nr_irq = FIELD_GET(AIC_INFO_NR_IRQ, info);
> dc97fd6fec0099 Hector Martin 2022-03-10 932 irqc->max_irq = AIC_MAX_IRQ;
> dc97fd6fec0099 Hector Martin 2022-03-10 933
> dc97fd6fec0099 Hector Martin 2022-03-10 934 off = irqc->info.target_cpu;
> dc97fd6fec0099 Hector Martin 2022-03-10 935 off += sizeof(u32) * irqc->max_irq; /* TARGET_CPU */
> dc97fd6fec0099 Hector Martin 2022-03-10 936
> dc97fd6fec0099 Hector Martin 2022-03-10 937 break;
> dc97fd6fec0099 Hector Martin 2022-03-10 938 }
>
> not default statement.

This is yet another case of data-driven dependency that smatch cannot
infer easily. info.version is statically set in aic_info_match[]. 1
and 2 are the only values provided by this array, so off is never left
uninitialised.

Thanks,

M.

--
Without deviation from the norm, progress is not possible.