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.

Sign up for GitHub

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

Jump to bottom

Audit isPowerOfTwo in Sema and LLVM #21400

Closed
wants to merge 1 commit into from
Closed

Audit isPowerOfTwo in Sema and LLVM #21400

wants to merge 1 commit into from

Conversation

wooster0
Copy link
Contributor

@wooster0 wooster0 commented Sep 13, 2024 •
edited
Loading

Part of #16021

All that's left now is this:

$ git grep isPowerOfTwo src/arch
src/arch/arm/CodeGen.zig:                            if (std.math.isPowerOfTwo(imm)) {
src/arch/arm/CodeGen.zig:                            if (std.math.isPowerOfTwo(imm)) {
src/arch/arm/CodeGen.zig:                            if (std.math.isPowerOfTwo(imm)) {
src/arch/riscv64/CodeGen.zig:                if (!math.isPowerOfTwo(size))
src/arch/riscv64/CodeGen.zig:                    if (!math.isPowerOfTwo(bit_size)) {
src/arch/riscv64/CodeGen.zig:                    if (!math.isPowerOfTwo(bit_size))
src/arch/riscv64/CodeGen.zig:                if (int_info.bits >= 8 and math.isPowerOfTwo(int_info.bits)) {
src/arch/riscv64/CodeGen.zig:        if (!math.isPowerOfTwo(int_info.bits) or int_info.bits < 8) {
src/arch/riscv64/CodeGen.zig:        if (!math.isPowerOfTwo(bit_size)) try func.truncateRegister(ty, src_reg);
src/arch/riscv64/CodeGen.zig:        if (!math.isPowerOfTwo(operand_bit_size))
src/arch/riscv64/CodeGen.zig:        if (!math.isPowerOfTwo(val_size))
src/arch/wasm/CodeGen.zig:    if ((!std.math.isPowerOfTwo(elem_size) or elem_size % 8 != 0) and vector_len > 1) {
src/arch/x86_64/CodeGen.zig:                if (int_info.bits >= 8 and math.isPowerOfTwo(int_info.bits)) {
src/arch/x86_64/CodeGen.zig:                if (int_info.bits >= 8 and math.isPowerOfTwo(int_info.bits)) {
src/arch/x86_64/CodeGen.zig:        if (math.isPowerOfTwo(src_bits)) {
src/arch/x86_64/CodeGen.zig:        if (src_bits <= 8 or !math.isPowerOfTwo(src_bits)) {
src/arch/x86_64/encoder.zig:            if (args.scale_index) |si| assert(std.math.isPowerOfTwo(si.scale));

As for the one issue I found in the LLVM parts of the compiler I filed #21401

@wooster0
audit isPowerOfTwo in Sema
ab2be75 
The only suspicious part was the isPowerOfTwo in zirShl and zirShr.
This code should trigger a crash in zirShl because the values are not
comptime-known so it's going to reach the bottom where it checks whether
safety is needed and then does isPowerOfTwo() but it doesn't crash:
```
extern const a: u8;

export fn x() void {
    var idk: u0 = @truncate(a);
    _ = &idk;

    const y: u0 = @truncate(a);
    _ = y << idk;
}
```
I think this is because of OVP. So it's not going to reach it anyway.

The only thing I found is some error messages being wrong. This fixes
that.
@wooster0 wooster0 mentioned this pull request Sep 13, 2024
Open
@wooster0 wooster0 force-pushed the auditpowerofzero branch 2 times, most recently from bee4c79 to ab2be75 Compare September 13, 2024 18:16
@mlugg
Copy link
Member

mlugg commented Sep 18, 2024

I'm confused by this PR; all it seems to be doing is changing some error messages to be less accurate.

@wooster0
Copy link
Contributor Author

The "not a power of two or zero" wording confused me and I read it as "not a power of two, or zero".
Now realize it's supposed to mean "not a power of two or not zero"

@wooster0 wooster0 closed this Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wooster0 @mlugg