-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
std lib test suite: fix illegal behavior found by valgrind #24833
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
Conversation
That fix doesn't make sense. Maybe move |
I also just fixed the IOUring errors. This is my diff: diff --git a/lib/std/os/linux/IoUring.zig b/lib/std/os/linux/IoUring.zig
index 62a87ac103..5c7e5ff1c2 100644
--- a/lib/std/os/linux/IoUring.zig
+++ b/lib/std/os/linux/IoUring.zig
@@ -1692,7 +1692,9 @@ pub const BufferGroup = struct {
pub fn get(self: *BufferGroup, cqe: linux.io_uring_cqe) ![]u8 {
const buffer_id = try cqe.buffer_id();
const used_len = @as(usize, @intCast(cqe.res));
- return self.get_by_id(buffer_id)[0..used_len];
+ const buf = self.get_by_id(buffer_id)[0..used_len];
+ std.valgrind.memcheck.makeMemDefinedIfAddressable(buf);
+ return buf;
}
// Release buffer from CQE to the kernel.
@@ -4404,9 +4406,7 @@ fn expect_buf_grp_cqe(
try testing.expectEqual(posix.E.SUCCESS, cqe.err());
// get buffer from pool
- const buffer_id = try cqe.buffer_id();
- const len = @as(usize, @intCast(cqe.res));
- const buf = buf_grp.get_by_id(buffer_id)[0..len];
+ const buf = try buf_grp.get(cqe);
try testing.expectEqualSlices(u8, expected, buf);
return cqe; |
I committed it, feel free to change the author |
That's not important to me. |
I edited the title to better reflect the task: fixing things, not suppressing warnings |
30ee994
to
6894775
Compare
.graph = graph, | ||
.build_root = build_root, | ||
.cache_root = cache_root, | ||
.debug_stack_frames_count = std.Build.debug_stack_frames_count, // strange: errors out as undeclared without the prefixes |
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.
this also errors out when running the std lib test suite
how do I fix this?
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.
@jedisct1 any ideas on this? I could just hardcode 8, but that seems like a bug waiting to happen.
no valgrind errors on linux, tested with .debug_stack_frames_count = 8 (don't know how to fix that) |
3a833f4
to
873e9ff
Compare
we don't care about the contents, we just need a file
…UEUE fixes a valgrind warning
…re buffer fixes a valgrind warning
the buffers are set to 0 after reading provide_buffers: triggered when checking cqe.user_data remove_buffers: expectEqualSlices
it is used to allocate something later, triggering a valgrind warning
…y fix valgrind warning
disabling the false positive warning is a bit tricky
I have a feeling this is never going to get merged. Please do one at a time, and no drafts |
this addresses some of the warnings in #24753