Re: [PATCH] ACPI/IORT: Fix GCC 12 warning

[Robin Murphy <robin.murphy@arm.com>]

On 2022-02-10 18:27, Victor Erminpour wrote:
> 
> On 2/10/22 10:06 AM, Robin Murphy wrote:
>> On 2022-02-10 00:32, Victor Erminpour wrote:
>>> When building with automatic stack variable initialization, GCC 12
>>> complains about variables defined outside of switch case statements.
>>> Move the variable into the case that uses it, which silences the 
>>> warning:
>>>
>>> ./drivers/acpi/arm64/iort.c:1670:59: error: statement will never be 
>>> executed [-Werror=switch-unreachable]
>>>    1670 |                         struct acpi_iort_named_component 
>>> *ncomp;
>>> | ^~~~~
>>
>> Notwithstanding the fact that that warning is nonsensical, this patch 
>> changes valid C code into invalid C code that doesn't even compile:
>>
>> drivers/acpi/arm64/iort.c: In function 
>> ‘acpi_iort_dma_get_max_cpu_address’:
>> drivers/acpi/arm64/iort.c:1669:4: error: a label can only be part of a 
>> statement and a declaration is not a statement
>>  1669 |    struct acpi_iort_named_component *ncomp;
>>       |    ^~~~~~
>> drivers/acpi/arm64/iort.c:1676:4: error: a label can only be part of a 
>> statement and a declaration is not a statement
>>  1676 |    struct acpi_iort_root_complex *rc;
>>       |    ^~~~~~
>>
>> Robin.
>>
> 
> Hi Robin,
> 
> Thank you for your constructive criticism.
> Could the solution be enclosing the case statement in curly braces?
> 
> I know this isn't a big issue for you, but this is a legitimate error 
> for people
> building the kernel with GCC 12 and CONFIG_INIT_STACK_ALL_ZERO enabled.

As Ard pointed out first, it is not a legitimate error, it is a spurious 
error. The fact that you seemingly didn't get the label errors above, 
nor even "ISO C90 forbids mixed declarations and code 
[-Wdeclaration-after-statement]", implies that GCC 12 currently has a 
completely broken notion of declarations vs. statements with that option 
enabled, so GCC 12 needs fixing.

Robin.

> 
> Regards,
> --Victor
> 
> 
> 
>>> Signed-off-by: Victor Erminpour <victor.erminpour@oracle.com>
>>> ---
>>>   drivers/acpi/arm64/iort.c | 8 +++-----
>>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>>> index 3b23fb775ac4..5c5d2e56d756 100644
>>> --- a/drivers/acpi/arm64/iort.c
>>> +++ b/drivers/acpi/arm64/iort.c
>>> @@ -1645,7 +1645,7 @@ void __init acpi_iort_init(void)
>>>    */
>>>   phys_addr_t __init acpi_iort_dma_get_max_cpu_address(void)
>>>   {
>>> -    phys_addr_t limit = PHYS_ADDR_MAX;
>>> +    phys_addr_t local_limit, limit = PHYS_ADDR_MAX;
>>>       struct acpi_iort_node *node, *end;
>>>       struct acpi_table_iort *iort;
>>>       acpi_status status;
>>> @@ -1667,17 +1667,15 @@ phys_addr_t __init 
>>> acpi_iort_dma_get_max_cpu_address(void)
>>>               break;
>>>             switch (node->type) {
>>> -            struct acpi_iort_named_component *ncomp;
>>> -            struct acpi_iort_root_complex *rc;
>>> -            phys_addr_t local_limit;
>>> -
>>>           case ACPI_IORT_NODE_NAMED_COMPONENT:
>>> +            struct acpi_iort_named_component *ncomp;
>>>               ncomp = (struct acpi_iort_named_component 
>>> *)node->node_data;
>>>               local_limit = DMA_BIT_MASK(ncomp->memory_address_limit);
>>>               limit = min_not_zero(limit, local_limit);
>>>               break;
>>>             case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
>>> +            struct acpi_iort_root_complex *rc;
>>>               if (node->revision < 1)
>>>                   break;
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-arm-kernel__;!!ACWV5N9M2RV99hQ!ZcNRWTaQIb1uVqcZAusSzXd2kBnWuqAuICpxkOm6bOWBouQoFygbcIMJVUNp34LqhNrc$ 
>>
>>