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

If and return/exit statements causing uninitialized_use warnings #4385

Open
Trigary opened this issue Feb 1, 2024 · 2 comments
Open

If and return/exit statements causing uninitialized_use warnings #4385

Trigary opened this issue Feb 1, 2024 · 2 comments
Labels
bug This behavior is unintended and should be fixed. core Topics concerning the core segments of the compiler (frontend, midend, parser) regression Code that previously compiled correctly either no longer compiles or produces invalid results.

Comments

@Trigary
Copy link
Contributor

Trigary commented Feb 1, 2024

Compiling the following code:

apply {
    if (standard_metadata.ingress_global_timestamp == 123) return;
    bit<32> x = 42;
    if (standard_metadata.packet_length == 456) return;
    log_msg("test {}", {x});
}

Results in the following warning, which I believe is incorrect behaviour:

user@xyz:/workspace# p4c-bm2-ss switch.p4
switch.p4(24): [--Wwarn=uninitialized_use] warning: x may be uninitialized
        log_msg("test {}", {x});
                            ^
user@xyz:/workspace#

The warning persists if I replace exactly one return with exit, but the warning disappears if I replace both. The warning also disappears if I use a constant as the if statement condition (e.g. if (true) return;), this is why I used those standard_metadata related checks in the sample above.

p4c version: 1.2.4.2
These warnings were not present in the last p4c version I used, which I believe was 1.2.3.0 or 1.2.2.0.

I attached the full source file for your convenience: switch.zip

@fruffy fruffy added bug This behavior is unintended and should be fixed. core Topics concerning the core segments of the compiler (frontend, midend, parser) labels Feb 1, 2024
@kfcripps kfcripps added the regression Code that previously compiled correctly either no longer compiles or produces invalid results. label Jun 13, 2024
@kfcripps
Copy link
Contributor

This was also introduced by e60cb8a (along with #4500, #4507, #4548).

@kfcripps
Copy link
Contributor

e60cb8a added a second SimplifyDefUse pass, which sees the following transformed IR:

    apply {
        hasReturned = false;
        if (standard_metadata.ingress_global_timestamp == 48w123) {
            hasReturned = true;
        }
        if (hasReturned) {
            ;
        } else {
            x_0 = 32w42;
            if (standard_metadata.packet_length == 32w456) {
                hasReturned = true;
            }
        }
        if (hasReturned) {
            ;
        } else {
            log_msg<tuple<bit<32>>>("test {}", { x_0 });
        }
    }

whereas the first pass sees:

    apply {
        if (standard_metadata.ingress_global_timestamp == 48w123) {
            return;
        }
        x_0 = 32w42;
        if (standard_metadata.packet_length == 32w456) {
            return;
        }
        log_msg<tuple<bit<32>>>("test {}", { x_0 });
    }

The IR looks correct, but SimplifyDefUse, which emits the warning, is probably not able to determine that x_0 is guaranteed to be initialized when hasReturned is false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This behavior is unintended and should be fixed. core Topics concerning the core segments of the compiler (frontend, midend, parser) regression Code that previously compiled correctly either no longer compiles or produces invalid results.
Projects
None yet
Development

No branches or pull requests

3 participants