[libc-commits] [PATCH] D148837: [LIBC] Strengthen `pthread_join` tests; NFC

Mon Jun 5 18:07:42 PDT 2023

goldstein.w.n added a comment.

In D148837#4397630 <https://reviews.llvm.org/D148837#4397630>, @sivachandra wrote:

> Thanks @michaelrj for responding on this review. @goldstein.w.n - the tests you have added are great but couple of reasons for why I was putting this review off and eventually forgot about it: 1. The logic was not straightforward enough that without proper comments, I have to read through line by line to understand what exactly is going on. 2. Use of random values and I wanted to spend time to see what would be good alternates. Thanks to @michaelrj, I have actually read this patch in detail and have couple of comments:

Fair enough regarding the comments, can properly document.

> 1. The non-null join test is best suited as a fuzz test. See examples here: https://github.com/llvm/llvm-project/tree/main/libc/fuzzing/stdlib

Why is that? Its really just checking that the runtime 1) returns the correct value and 2) doesn't return early.
I can drop the detached aspect if thats getting in the way.

I don't really see the fuzz aspect of it. There are no invalid joins (or at least I hope not) and its not really meant
to stress the system in some way to see if breaks down.

> 2. For this patch, make an equivalent of the non-null join test which is deterministic (as in it does not use random numbers).

The random values are seeded with with `srand(123)` which should make the test entirely deterministic.

> We do not run any of libc's fuzz tests on public infrastructure but we do run them downstream. If you can convert the test into a fuzz test, it might actually become complicated enough that we can make an attempt at running it on OSS-fuzz: https://github.com/google/oss-fuzz

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148837/new/

https://reviews.llvm.org/D148837