[meta-xilinx] [PATCH v2 2/2] arm-trusted-firmware: Fix a false positive out of bounds

Fri Aug 25 09:09:22 PDT 2017

On Fri, Aug 25, 2017 at 7:13 AM, Nathan Rossi <nathan at nathanrossi.com> wrote:
> On 23 August 2017 at 02:41, Alistair Francis
> <alistair.francis at xilinx.com> wrote:
>> On Tue, Aug 22, 2017 at 7:31 AM, Nathan Rossi <nathan at nathanrossi.com> wrote:
>>> On 22 August 2017 at 04:07, Alistair Francis
>>> <alistair.francis at xilinx.com> wrote:
>>>> This error is received while building
>>>> services/std_svc/psci/psci_common.c: In function 'psci_do_state_coordination':
>>>> services/std_svc/psci/psci_common.c:220:27: error: array subscript is above
>>>> array bounds [-Werror=array-bounds]
>>>>   psci_req_local_pwr_states[pwrlvl - 1][cpu_idx] = req_pwr_state;
>>>>
>>>> Patch 555ebb34db8f3424c1b394df2f10ecf9c1f70901 explains why the error
>>>> is seen and supposibly includes a fix. As the fix appears not to work,
>>>> let's disable the error checking.
>>>>
>>>> Signed-off-by: Alistair Francis <alistair.francis at xilinx.com>
>>>
>>> Applied.
>>>
>>>> ---
>>>>  .../arm-trusted-firmware_2017.1.bb                 |  1 +
>>>>  ...-Resolve-GCC-static-analysis-false-positi.patch | 40 ++++++++++++++++++++++
>>>>  2 files changed, 41 insertions(+)
>>>>  create mode 100644 recipes-bsp/arm-trusted-firmware/files/psci_common-Resolve-GCC-static-analysis-false-positi.patch
>>>>
>>>> diff --git a/recipes-bsp/arm-trusted-firmware/arm-trusted-firmware_2017.1.bb b/recipes-bsp/arm-trusted-firmware/arm-trusted-firmware_2017.1.bb
>>>> index 52901f2..e68e5cf 100644
>>>> --- a/recipes-bsp/arm-trusted-firmware/arm-trusted-firmware_2017.1.bb
>>>> +++ b/recipes-bsp/arm-trusted-firmware/arm-trusted-firmware_2017.1.bb
>>>> @@ -6,3 +6,4 @@ SRCREV ?= "7d1a6732c9ae113999aeabcb9912369760d05c13"
>>>>  PV = "1.3-xilinx-${XILINX_RELEASE_VERSION}+git${SRCPV}"
>>>>
>>>>  SRC_URI += "file://zynqmp-Remove-duplicate-const-declaration.patch"
>>>> +SRC_URI += "file://psci_common-Resolve-GCC-static-analysis-false-positi.patch"
>>>> diff --git a/recipes-bsp/arm-trusted-firmware/files/psci_common-Resolve-GCC-static-analysis-false-positi.patch b/recipes-bsp/arm-trusted-firmware/files/psci_common-Resolve-GCC-static-analysis-false-positi.patch
>>>> new file mode 100644
>>>> index 0000000..f706585
>>>> --- /dev/null
>>>> +++ b/recipes-bsp/arm-trusted-firmware/files/psci_common-Resolve-GCC-static-analysis-false-positi.patch
>>>> @@ -0,0 +1,40 @@
>>>> +From 0197ad57b44fb7f10ca604891e0974110748fbd5 Mon Sep 17 00:00:00 2001
>>>> +From: Alistair Francis <alistair.francis at xilinx.com>
>>>> +Date: Mon, 21 Aug 2017 10:19:40 -0700
>>>> +Subject: [PATCH] psci_common: Resolve GCC static analysis false positive
>>>> +
>>>> +Previously commit 555ebb34db8f3424c1b394df2f10ec attmpted to fix this
>>>> +GCC issue:
>>>> +
>>>> +services/std_svc/psci/psci_common.c: In function 'psci_do_state_coordination':
>>>> +services/std_svc/psci/psci_common.c:220:27: error: array subscript is above
>>>> +array bounds [-Werror=array-bounds]
>>>> +  psci_req_local_pwr_states[pwrlvl - 1][cpu_idx] = req_pwr_state;
>>>> +
>>>> +This fix doesn't work as asserts aren't built in non-debug build flows.
>>>> +Let's ensure this error is fixed for all build cases.
>>>> +
>>>> +Signed-off-by: Alistair Francis <alistair.francis at xilinx.com>
>>>> +Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
>>>> +---
>>>> +Upstream Status: Pending
>>>
>>> For reference I don't think upstream will accept this patch, so it
>>> might be better to resolve by enabling assertions on the zynqmp target
>>> like is done for tegra. That way instead of a silent failure it will
>>> result in a assertion failure which can be seen or debugged. But this
>>> change is ok for the current Xilinx version of ATF which ties debug
>>> with assertion support.
>>
>> At the moment it seems like they are willing to accept it. It has
>> changed a little, but hopefully it will make it in. From there we
>> should be able to port it to Xilinx's fork.
>>
>> https://github.com/ARM-software/arm-trusted-firmware/pull/1058
>
> Looks like your pull request got merged (with modifications to the
> change). Did you want to update the patch or get it applied to the
> Xilinx v2017.3 ATF and then just replace v2017.1 once released?

I think I'll do both, I'm going to send a patch to meta-xilinx that
updates the patch to match the commit in ATF. I'll also send the patch
internally so hopefully it'll be applied to 2017.3.

As we seem to keep the last two releases in Yocto I think it still
makes sense to keep these patches for 2017.3.

Thanks,
Alistair

>
> Regards,
> Nathan
>
>>
>> Thanks,
>> Alistair
>>
>>>
>>> Thanks,
>>> Nathan
>>>
>>>> +
>>>> + lib/psci/psci_common.c | 2 ++
>>>> + 1 file changed, 2 insertions(+)
>>>> +
>>>> +diff --git a/lib/psci/psci_common.c b/lib/psci/psci_common.c
>>>> +index 68cdd6eb..a8c94215 100644
>>>> +--- a/lib/psci/psci_common.c
>>>> ++++ b/lib/psci/psci_common.c
>>>> +@@ -394,6 +394,8 @@ void psci_do_state_coordination(unsigned int end_pwrlvl,
>>>> +       plat_local_state_t target_state, *req_states;
>>>> +
>>>> +       assert(end_pwrlvl <= PLAT_MAX_PWR_LVL);
>>>> ++      if (end_pwrlvl > PLAT_MAX_PWR_LVL)
>>>> ++              return;
>>>> +       parent_idx = psci_cpu_pd_nodes[cpu_idx].parent_node;
>>>> +
>>>> +       /* For level 0, the requested state will be equivalent
>>>> +--
>>>> +2.11.0
>>>> +
>>>> --
>>>> 2.11.0
>>>>
>>> --
>>> _______________________________________________
>>> meta-xilinx mailing list
>>> meta-xilinx at yoctoproject.org
>>> https://lists.yoctoproject.org/listinfo/meta-xilinx
> --
> _______________________________________________
> meta-xilinx mailing list
> meta-xilinx at yoctoproject.org
> https://lists.yoctoproject.org/listinfo/meta-xilinx