test: re-introduce fuzzers

[DavidKorczynski]

Signed-off-by: David Korczynski david@adalogics.com

This reintroduces the fuzzers as they were removed in #28085 and broke the OSS-Fuzz build. I added them with proper license headers now.

CC @AdamKorcz

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:

How it Works:

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

None

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

[CLAassistant]

All committers have signed the CLA.

[ti-chi-bot]

Welcome @DavidKorczynski!

It looks like this is your first PR to pingcap/tidb 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to pingcap/tidb. 😃

[tisonkun]

@DavidKorczynski thanks for your contribution!

Can you elaborate a bit how this files work? Also please check the CI failure.

I can see that these functions won't be referred from within the project.

[DavidKorczynski]

The goal is to re-enable what was committed here: #17250

The files are used by oss-fuzz (https://github.com/google/oss-fuzz) to continuously fuzz tidb and report any issues. Naturally, it would be great to have more fuzzers developed such that there is more code coverage.

[tisonkun]

@DavidKorczynski thanks for your explanation. May you add a comment for those methods so that we don't remove them in accident (as they're unused within the project) again?

Also I'd like to know how google/oss-fuzz works and how TiDB gains from this change explicitly.

[kennytm]

i know this is basically reverting #28085, but it looks very strange that

1. the fuzz programs are placed directly inside the types package
2. they are unconditionally public
3. it only fuzz three functions ParseBinaryFromString, NewBitLiteral and NewHexLiteral

imo

1. they should be moved into a separate fuzz package, ideally its own module, provided we are not going to fuzz private functions directly
2. if extraction is impossible (i don't know how oss-fuzz works), these files should be hidden by default with a //go:build
3. it should fuzz the SQL parser and other complex functions. those functions interacting with KVs and using hack.String need more scrutiny than the three types.* functions.

[tisonkun]

it should fuzz the SQL parser and other complex functions

This can be nice to have but not prevent this PR to merge. My concern is still what we gain from this change? Where does the fuzz run? I remove it because it seems like unused/dead code.

[DavidKorczynski]

The fuzzers are being run by the OSS-Fuzz service which is a service run by Google. This means the fuzzers run on a continuous basis (every day) in order to see if any bugs have been introduced or similar.

The OSS-Fuzz set up for TiDB is here: https://github.com/google/oss-fuzz/tree/master/projects/tidb and you can see the list of maintainers receiving bug reports here: https://github.com/google/oss-fuzz/blob/master/projects/tidb/project.yaml under the
primary_contact and auto_ccs list. As such, zhouqiang should have received some reports about the build failing. @tisonkun I can add your email to the list and then you will receive bug reports per email as well as access to detailed information on https://oss-fuzz.com

The benefit from TiDB's perspective is that bugs will be found and reported. The goal is really to then develop more fuzzers such that more code of TiDB can be analysed.

[kennytm]

/cc @zhouqiang-cl

[zhouqiang-cl]

@shuke987 PTAL

[zhouqiang-cl]

@jebter @mahjonp @oraluben PTAL

[oraluben]

I would love to see oss-fuzz support in TiDB.
I guess it's a good idea to add below directives to prevent these files being compiled in release.

//go:build gofuzz
// +build gofuzz

PS: copied from crdb, haven't tested.

[DavidKorczynski]

@AdamKorcz knows a lot more about how to structure Go fuzzers as he has done this for a ton of projects. Will just CC him as he is also the one who integrated TiDB into OSS-Fuzz in the first place

[DavidKorczynski]

I would love to see oss-fuzz support in TiDB.

Just to clarify: TiDB is in fact integrated already as per #17250 However, the fuzzers were removed and thus the OSS-Fuzz set up is currently not working (since it is not able to compile the fuzzers that no longer exist). As such, if we re-enable the three fuzzers then OSS-Fuzz will start running them and we can continue with building up a more mature fuzzing set up of TiDB

[kennytm]

yes the compile_go_fuzzer script uses -tags gofuzz.

[DavidKorczynski]

I would love to see oss-fuzz support in TiDB. I guess it's a good idea to add below directives to prevent these files being compiled in release.

//go:build gofuzz
// +build gofuzz

PS: copied from crdb, haven't tested.

Fixed!

[AdamKorcz]

Can this be merged soon so OSS-fuzz can continue to run TIDB's fuzzers?