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

std: use math overflow helpers instead of builtins #18165

Closed
wants to merge 1 commit into from
Closed

std: use math overflow helpers instead of builtins #18165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions lib/std/leb128.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const std = @import("std");
const math = std.math;
const testing = std.testing;

/// Read a single unsigned LEB128 value from the given reader as type T,
Expand All @@ -15,10 +16,8 @@ pub fn readULEB128(comptime T: type, reader: anytype) !T {
while (group < max_group) : (group += 1) {
const byte = try reader.readByte();

const ov = @shlWithOverflow(@as(U, byte & 0x7f), group * 7);
if (ov[1] != 0) return error.Overflow;

value |= ov[0];
const mask = try math.shlExact(U, byte & 0x7f, group * 7);
value |= mask;
Comment on lines +19 to +20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that Andrew prefers to use builtins when available. These should instead be changed to use the destructuring syntax #498:

Suggested change
const mask = try math.shlExact(U, byte & 0x7f, group * 7);
value |= mask;
const mask, const overflow = @shlWithOverflow(@as(U, byte & 0x7f), group * 7);
if (overflow != 0) return error.Overflow;
value |= mask;

Copy link
Contributor Author

@sno2 sno2 Dec 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I am confused. I feel like that would make sense if you were using overflow for bit stuff. However, we seem to be just checking if there wasn't an overflow. Therefore, does it not make much more sense to use the "zig" approach with try?

As a side note, this also removes the need for prerequisite knowledge for how overflow bits behave (even if they seem simple to some people).

if (byte & 0x80 == 0) break;
} else {
return error.Overflow;
Expand Down
12 changes: 3 additions & 9 deletions lib/std/math/powi.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,16 @@ pub fn powi(comptime T: type, x: T, y: T) (error{

while (exp > 1) {
if (exp & 1 == 1) {
const ov = @mulWithOverflow(acc, base);
if (ov[1] != 0) return error.Overflow;
acc = ov[0];
acc = try math.mul(T, acc, base);
}

exp >>= 1;

const ov = @mulWithOverflow(base, base);
if (ov[1] != 0) return error.Overflow;
base = ov[0];
base = try math.mul(T, base, base);
}

if (exp == 1) {
const ov = @mulWithOverflow(acc, base);
if (ov[1] != 0) return error.Overflow;
acc = ov[0];
acc = try math.mul(T, acc, base);
}

return acc;
Expand Down
7 changes: 3 additions & 4 deletions lib/std/mem.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3786,13 +3786,12 @@ pub fn alignPointerOffset(ptr: anytype, align_to: usize) ?usize {

// Calculate the aligned base address with an eye out for overflow.
const addr = @intFromPtr(ptr);
var ov = @addWithOverflow(addr, align_to - 1);
if (ov[1] != 0) return null;
ov[0] &= ~@as(usize, align_to - 1);
var aligned_addr = math.add(usize, addr, align_to - 1) catch return null;
aligned_addr &= ~@as(usize, align_to - 1);

// The delta is expressed in terms of bytes, turn it into a number of child
// type elements.
const delta = ov[0] - addr;
const delta = aligned_addr - addr;
const pointee_size = @sizeOf(info.Pointer.child);
if (delta % pointee_size != 0) return null;
return delta / pointee_size;
Expand Down
36 changes: 6 additions & 30 deletions lib/std/net.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,16 +402,8 @@ pub const Ip6Address = extern struct {
if (scope_id) {
if (c >= '0' and c <= '9') {
const digit = c - '0';
{
const ov = @mulWithOverflow(result.sa.scope_id, 10);
if (ov[1] != 0) return error.Overflow;
result.sa.scope_id = ov[0];
}
{
const ov = @addWithOverflow(result.sa.scope_id, digit);
if (ov[1] != 0) return error.Overflow;
result.sa.scope_id = ov[0];
}
result.sa.scope_id = try std.math.mul(u32, result.sa.scope_id, 10);
result.sa.scope_id = try std.math.add(u32, result.sa.scope_id, digit);
} else {
return error.InvalidCharacter;
}
Expand Down Expand Up @@ -462,16 +454,8 @@ pub const Ip6Address = extern struct {
return result;
} else {
const digit = try std.fmt.charToDigit(c, 16);
{
const ov = @mulWithOverflow(x, 16);
if (ov[1] != 0) return error.Overflow;
x = ov[0];
}
{
const ov = @addWithOverflow(x, digit);
if (ov[1] != 0) return error.Overflow;
x = ov[0];
}
x = try std.math.mul(u16, x, 16);
x = try std.math.add(u16, x, digit);
saw_any_digits = true;
}
}
Expand Down Expand Up @@ -584,16 +568,8 @@ pub const Ip6Address = extern struct {
return result;
} else {
const digit = try std.fmt.charToDigit(c, 16);
{
const ov = @mulWithOverflow(x, 16);
if (ov[1] != 0) return error.Overflow;
x = ov[0];
}
{
const ov = @addWithOverflow(x, digit);
if (ov[1] != 0) return error.Overflow;
x = ov[0];
}
x = try std.math.mul(u16, x, 16);
x = try std.math.add(u16, x, digit);
saw_any_digits = true;
}
}
Expand Down
24 changes: 4 additions & 20 deletions lib/std/process.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1531,16 +1531,8 @@ pub fn posixGetUserInfo(name: []const u8) !UserInfo {
'0'...'9' => byte - '0',
else => return error.CorruptPasswordFile,
};
{
const ov = @mulWithOverflow(uid, 10);
if (ov[1] != 0) return error.CorruptPasswordFile;
uid = ov[0];
}
{
const ov = @addWithOverflow(uid, digit);
if (ov[1] != 0) return error.CorruptPasswordFile;
uid = ov[0];
}
uid = std.math.mul(posix.uid_t, uid, 10) catch return error.CorruptPasswordFile;
uid = std.math.add(posix.uid_t, uid, digit) catch return error.CorruptPasswordFile;
},
},
.ReadGroupId => switch (byte) {
Expand All @@ -1555,16 +1547,8 @@ pub fn posixGetUserInfo(name: []const u8) !UserInfo {
'0'...'9' => byte - '0',
else => return error.CorruptPasswordFile,
};
{
const ov = @mulWithOverflow(gid, 10);
if (ov[1] != 0) return error.CorruptPasswordFile;
gid = ov[0];
}
{
const ov = @addWithOverflow(gid, digit);
if (ov[1] != 0) return error.CorruptPasswordFile;
gid = ov[0];
}
gid = std.math.mul(posix.gid_t, gid, 10) catch return error.CorruptPasswordFile;
gid = std.math.add(posix.gid_t, gid, digit) catch return error.CorruptPasswordFile;
},
},
}
Expand Down
Loading