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

[Nathan Rossi]

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.

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
>