search

p4lang / p4c

P4_16 reference compiler
https://p4.org/
Apache License 2.0
667 stars 441 forks source link

Bogus "uninitialized-use" warning when action is executed inside a switch statement clause #3967

Open vgurevich opened 1 year ago

vgurevich commented 1 year ago

Attached is a trivial test case bogus_uninitialized_warning.p4.txt which produces the following warning when compiled using p4_test:

$ ~/tools/p4_test.sh bogus_uninitialized_warning.p4 
bogus_uninitialized_warning.p4(95): [--Wwarn=uninitialized_use] warning: a may be uninitialized
        diff = a - b;
               ^
bogus_uninitialized_warning.p4(95): [--Wwarn=uninitialized_use] warning: b may be uninitialized
        diff = a - b;
                   ^

The problem is that the compiler does not understand that the action ab_diff() (that contains the line in question) can be executed only after the action ab_assign() has been executed.

mihaibudiu commented 1 year ago

The default_action on table t is NoAction. If table t can execute NoAction the values will be uninitialized. So on a table miss this can happen. I think that the compiler actually performs this dataflow analysis and finds this path.

apinski-cavium commented 1 year ago

The default_action on table t is NoAction. If table t can execute NoAction the values will be uninitialized. So on a table miss this can happen. I think that the compiler actually performs this dataflow analysis and finds this path.

There is no path as there is a check on what the action was run: switch (t.apply().action_run) { ab_assign: (use a/b) ...

mihaibudiu commented 1 year ago

Ok, I take it back. Our dataflow analysis isn't that smart and can't figure out (the lack of) this particular path.

vgurevich commented 1 year ago

Again, not a high-priority issue, but if it can highlight the weakness on the data flow analysis that can then be fixed, then maybe something good will come out of it.

I was also wondering whether @apinski-cavium will say about how his frontend handles the same situation :)

apinski-cavium commented 1 year ago

I was also wondering whether @apinski-cavium will say about how his frontend handles the same situation :)

I have not implemented uninitialized warnings or data flow yet. It is on my list of things to implement though. Most likely it will not warn as I treat all table.apply() as taking all local variables that are possibly modified by all of the actions of a table as arguments (implementing them as in, inout or out arguments to the IR).