Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compile error #26

Open
ixgbe01 opened this issue May 16, 2024 · 2 comments
Open

compile error #26

ixgbe01 opened this issue May 16, 2024 · 2 comments

Comments

@ixgbe01
Copy link

ixgbe01 commented May 16, 2024

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
Copy link

orlitzky commented Sep 6, 2024

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
Copy link

orlitzky commented Sep 6, 2024

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);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants