sophgo / opensbi

RISC-V Open Source Supervisor Binary Interface
Other
4 stars 18 forks source link

compile error #26

Open ixgbe01 opened 6 months ago

ixgbe01 commented 6 months ago

compile command: make PLATFORM=generic PLATFORM_RISCV_XLEN=64 CROSS_COMPILE=riscv64-linux-gnu-

error log is shown above: /home/yangwang/official_opensbi/opensbi/lib/utils/reset/fdt_reset_sophgo_wdt.c: In function ‘mango_wdt_reset_init’: /home/yangwang/official_opensbi/opensbi/lib/utils/reset/fdt_reset_sophgo_wdt.c:80:16: error: ‘noff’ may be used uninitialized in this function [be-uninitialized] 80 | return fdt_get_node_addr_size(fdt, noff, 0, addr, NULL); | ^~~~~~~~~~~~ /home/yangwang/official_opensbi/opensbi/lib/utils/reset/fdt_reset_sophgo_wdt.c:71:18: note: ‘noff’ was declared here 71 | int len, noff; | ^~~~

orlitzky commented 2 months ago

This only "appears" when DEBUG=1 is not passed on the command-line, but if you look at the code, the variable noff can obviously be used uninitialized.

orlitzky commented 2 months ago

It looks like this code is based on similar code in lib/utils/fdt/fdt_helper.c. What I would really like to know is, why does that file repeatedly compare the len returned from fdt_getprop() against the size of a fdt32_t?

I am guessing that it's some kind of check for a correct value, specifically the "pointer" <&top_misc> that can be seen at https://github.com/sophgo/linux-riscv/blob/sg2042-dev/arch/riscv/boot/dts/sophgo/mango.dtsi#L494. If that guess is right, there are only a few things that can happen in this function:

  1. fdt_getprop() can fail. If val is NULL, we can return len, because len indicates the error.
  2. If val is non-NULL, then fdt_getprop() succeeded. But in that case we could still have (len < sizeof(fdt32_t)), which is undesirable for some reason (bad parameter?). In this case we can return SBI_EINVAL?
  3. In the "normal" case, where fdt_getprop() succeeds and len is large enough, we can set noff = fdt_node_offset_by_phandle(fdt, fdt32_to_cpu(*val)); and return fdt_get_node_addr_size(fdt, noff, 0, addr, NULL);

This "works" (it compiles and doesn't crash my machine), but keep in mind that I have no idea what I'm doing :)

static int sophgo_wdt_system_get_top_base(void *fdt,
                 int nodeoff, unsigned long *addr)
{
        const fdt32_t *val;
        int len, noff;

        val = fdt_getprop(fdt, nodeoff, "subctrl-syscon", &len);
        if (!val) {
                return len;
        }

        if (len < sizeof(fdt32_t)) {
                return SBI_EINVAL;
        }

        noff = fdt_node_offset_by_phandle(fdt, fdt32_to_cpu(*val));
        if (noff < 0)
                return noff;
        return fdt_get_node_addr_size(fdt, noff, 0, addr, NULL);
}