-
Notifications
You must be signed in to change notification settings - Fork 4
fix(small_buf_map): use *const Self for read methods to avoid use-after-stack-free #2
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,19 +21,19 @@ pub fn SmallBufMap(comptime buffer_size: u8) type { | |
| const Self = @This(); | ||
|
|
||
| /// Assumes the existence of a key at i | ||
| inline fn getKey(self: Self, i: u8) []const u8 { | ||
| inline fn getKey(self: *const Self, i: u8) []const u8 { | ||
| const start = if (i == 0) 0 else self.buffer[buffer_size - (2 * i)]; | ||
| const end = self.buffer[buffer_size - (2 * i) - 1]; | ||
| return self.buffer[start..end]; | ||
| } | ||
| /// Assumes the existence of a value at i | ||
| inline fn getValue(self: Self, i: u8) []const u8 { | ||
| inline fn getValue(self: *const Self, i: u8) []const u8 { | ||
| const start = self.buffer[buffer_size - (2 * i) - 1]; | ||
| const end = self.buffer[buffer_size - (2 * i) - 2]; | ||
| return self.buffer[start..end]; | ||
| } | ||
| /// Assumes the existence of key+value at i | ||
| pub fn getKeyValue(self: Self, i: u8) [2][]const u8 { | ||
| pub fn getKeyValue(self: *const Self, i: u8) [2][]const u8 { | ||
| return [2][]const u8{ self.getKey(i), self.getValue(i) }; | ||
| } | ||
|
|
||
|
|
@@ -50,12 +50,12 @@ pub fn SmallBufMap(comptime buffer_size: u8) type { | |
| @memcpy(self.buffer[key_end..value_end], value); | ||
| } | ||
|
|
||
| fn hasEntry(self: Self, i: u8) bool { | ||
| fn hasEntry(self: *const Self, i: u8) bool { | ||
| const offset = self.buffer[buffer_size - (2 * i) - 1]; | ||
| return offset != 0; | ||
| } | ||
|
|
||
| pub fn count(self: Self) u8 { | ||
| pub fn count(self: *const Self) u8 { | ||
| var i: u8 = 0; | ||
| while (i < 256) : (i += 1) { | ||
| if (!self.hasEntry(i)) { | ||
|
|
@@ -65,12 +65,12 @@ pub fn SmallBufMap(comptime buffer_size: u8) type { | |
| return 255; | ||
| } | ||
|
|
||
| pub fn dataCount(self: Self) u8 { | ||
| pub fn dataCount(self: *const Self) u8 { | ||
| const c = self.count(); | ||
| return if (c == 0) 0 else self.buffer[buffer_size - (2 * c)]; | ||
| } | ||
|
|
||
| inline fn getKeyIndex(self: Self, key: []const u8) ?u8 { | ||
| inline fn getKeyIndex(self: *const Self, key: []const u8) ?u8 { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The implementation of You could simplify this method by calling inline fn getKeyIndex(self: *const Self, key: []const u8) ?u8 {
const res = self.getPutOp(key);
return if (res.op == .replace) res.i else null;
}This would improve maintainability and slightly improve performance for missing keys. |
||
| var i: u8 = 0; | ||
| while (i < 256) : (i += 1) { | ||
| if (!self.hasEntry(i)) { | ||
|
|
@@ -84,14 +84,14 @@ pub fn SmallBufMap(comptime buffer_size: u8) type { | |
| return null; | ||
| } | ||
|
|
||
| pub fn get(self: Self, key: []const u8) ?[]const u8 { | ||
| pub fn get(self: *const Self, key: []const u8) ?[]const u8 { | ||
| const i = self.getKeyIndex(key) orelse return null; | ||
| return self.getValue(i); | ||
| } | ||
|
|
||
| const PutOpType = enum { replace, insert, append }; | ||
| const PutOp = struct { op: PutOpType, i: u8 }; | ||
| fn getPutOp(self: Self, key: []const u8) PutOp { | ||
| fn getPutOp(self: *const Self, key: []const u8) PutOp { | ||
| var i: u8 = 0; | ||
| while (i < 256) : (i += 1) { | ||
| if (!self.hasEntry(i)) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loop condition
i < 256incount(and similarly ingetKeyIndexandgetPutOp) can lead to an out-of-bounds access and runtime panic if the map is full.Since each entry requires at least 2 bytes of metadata at the end of the buffer, the maximum number of entries is bounded by
buffer_size / 2. When the map is full,hasEntry(i)will eventually attempt to access an index below zero (e.g.,buffer_size - (2 * i) - 1).Consider bounding the loop by
buffer_size / 2to ensure safety when the map reaches capacity.