Add support for Ubuntu 20.04 LTS (where OSS-Fuzz is at) #274

hartwork · 2025-02-19T01:06:54Z

OSS-Fuzz is using Ubuntu 20.04. This pull request will help unlock use of libprotobuf-mutator in OSS-Fuzz with system Protobuf (i.e. of Ubuntu >=20.04) and eventually allow re-enabling compilation of libexpat upstream fuzzer xml_lpm_fuzzer that pull request libexpat/libexpat#955 had to disable for a chance at a successful build in OSS-Fuzz. Thank you! 🙏

CC @vitalybuka

hartwork · 2025-02-19T22:13:28Z

@vitalybuka any thoughts? I understand that Ubuntu 20.04 is on its way out — both in GitHub Actions and its life with support by Canonical — and that therefore in other places support for Ubuntu 20.04 is rather removed than added. However OSS-Fuzz will take more time to perform a non-trivial migration off of Ubuntu 20.04 that affects all OSS-Fuzz projects at once, so I personally believe that it's a viable short-term strategy to first increase support for the past, and to then take it away again once OSS-Fuzz had time to move. I am happy to create a second (trivial) pull request dropping Ubuntu 20.04 from CI again once GitHub Actions pulled the plug and then a third (trivial) pull request reverting 4 of the commits supporting less recent Protobuf to clean the code again once OSS-Fuzz is off of Ubuntu "focal" 20.04 for good. What do you think?

PS: Review on commit level will be most enjoyable here.

vitalybuka · 2025-02-19T23:21:48Z

.github/workflows/cmake-multi-platform.yml

@@ -40,6 +40,16 @@ jobs:
            c_compiler: clang
            cpp_compiler: clang++
            bundled_protobuf: 'OFF'
+          - os: ubuntu-20.04
+            build_type: Release
+            c_compiler: clang-12


what is default compiler there?

@vitalybuka Clang 11 from focal-updates. Ubuntu 20.04 natively has clang-10 and clang-12 from "focal" and clang-11 from "focal-updates":

https://packages.ubuntu.com/focal/clang-10

https://packages.ubuntu.com/focal-updates/clang-11

https://packages.ubuntu.com/focal/clang-12

vitalybuka · 2025-02-19T23:27:29Z

README.md

@@ -8,6 +8,14 @@ libprotobuf-mutator is a library to randomly mutate
 [protobuffers](https://github.com/google/protobuf). <BR>
 It could be used together with guided fuzzing engines, such as [libFuzzer](http://libfuzzer.info).

+The core of libprotobuf-mutator has the following dependencies:
+
+- CMake >=3.10


can we avoid adding another place which can get out of sync
Maybe https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-a-permanent-link-to-a-code-snippet

can we avoid adding another place which can get out of sync?

@vitalybuka it would not be fun to dig out that info as a user when needed, and the damage of these going out of sync is minimal. The reason I'm adding it is because this info is not documented anywhere. I can drop the part on CMake if it takes to much attention, Clang and Protobuf are key here.

Maybe https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-a-permanent-link-to-a-code-snippet

These links would continue working but also go stale and not gain anything.

User will hit error if something is out of date. And then see Cmake condition.
I don't think anyone will start from documentation either before seeing error, or after seeing error.

@vitalybuka I'm reading that as a wish to drop CMake versions from this. Dropped and pushed.

vitalybuka · 2025-02-19T23:32:07Z

port/protobuf.h

 // clang-format off
-#include "google/protobuf/port_def.inc"  // MUST be last header included
+# include "google/protobuf/port_def.inc"  // MUST be last header included


I though we need this for PROTOBUF_VERSION
if we switch to GOOGLE_PROTOBUF_VERSION, can we drop this include?

@vitalybuka I don't know. I can drop it, throw it at the CI and see if it sticks. Protobuf examples want us to add it though, so we should probably keep it?

@vitalybuka PS: I confirm that offering PROTOBUF_VERSION is one of the effects of that include, yes.

what kind of examples?

@vitalybuka seems like I mis-remembered the headers own usage instructions at https://github.com/protocolbuffers/protobuf/blob/7229ef1b4f0f975036b140fe71c76d25feff4918/src/google/protobuf/port_def.inc#L19-L23 as a tutorial. I cannot find other places promoting it. Let me query CI for a version without it…

@vitalybuka CI started at https://github.com/hartwork/libprotobuf-mutator-fork/actions/runs/13424660914 …

I though we need this for PROTOBUF_VERSION if we switch to GOOGLE_PROTOBUF_VERSION, can we drop this include?

@vitalybuka dropped and pushed now

vitalybuka · 2025-02-19T23:34:38Z

src/mutator_test.cc

@@ -678,12 +678,23 @@ TYPED_TEST(MutatorTypedTest, Serialization) {
 }

 TYPED_TEST(MutatorTypedTest, UnknownFieldTextFormat) {
+  // NOTE: This test has little chance of passing when method
+  //       TextFormat::Parser::AllowUnknownField is not available
+#if GOOGLE_PROTOBUF_VERSION >= 3008000  // commit 176f7db11d8242b36a3ea6abb1cc436fca5bf75d of >=3.8.0


Do we really need before 3.8.0?

@vitalybuka yes, see https://packages.ubuntu.com/focal/libprotobuf-dev

There are two Protobuf version macros: GOOGLE_PROTOBUF_VERSION and PROTOBUF_VERSION without the "GOOGLE_" prefix. Both ancient and recent only expose macro GOOGLE_PROTOBUF_VERSION with the "GOOGLE_" prefix but not the other, so we depart from PROTOBUF_VERSION. Motivation for this change was that headers port_def.inc and port_undef.inc were only introduced to Protobuf in commit 6bbe197e9c1b6fc38cbdc45e3bf83fa7ced792a3 of release >=3.7.0 while we want to support Protobuf 3.6.1.3 of Ubuntu 20.04, too. Signed-off-by: Sebastian Pipping <[email protected]>

Signed-off-by: Sebastian Pipping <[email protected]>

…ield Signed-off-by: Sebastian Pipping <[email protected]>

Seed 9 was selected after quick experiments with values 1 to 20 for its speed, i.e. low number of runs needed to find the respective bug in both libfuzzer_example (text) and libfuzzer_bin_example (binary), combined (i.e. number of runs added). It was not a scientifc experiment by any means, and determinism (rather than speed) is my primary goal here. Signed-off-by: Sebastian Pipping <[email protected]>

.. to ease future debugging Signed-off-by: Sebastian Pipping <[email protected]>

.. now that the fixed seed makes that safe to do Signed-off-by: Sebastian Pipping <[email protected]>

Signed-off-by: Sebastian Pipping <[email protected]>

…F=OFF) .. with Protobuf 3.6.1.3 (see https://packages.ubuntu.com/focal/libprotobuf-dev) Signed-off-by: Sebastian Pipping <[email protected]>

.. while allowing to build the core project with CMake 3.10. Signed-off-by: Sebastian Pipping <[email protected]>

Signed-off-by: Sebastian Pipping <[email protected]>

hartwork · 2025-02-21T15:23:34Z

@vitalybuka update: I have all remaining pieces figured out now, OSS-Fuzz CI for google/oss-fuzz#13015 is also green just now including i386, it just needs one commit reverted from a custom branch back to mainline and we're good to go… once this pull request here is merged to master. Does it need any more adjustments from your point of view? Thanks! 🙏

hartwork requested a review from vitalybuka as a code owner February 19, 2025 01:06

hartwork force-pushed the ubuntu-20-04-support branch from 8a5ffb2 to 46cc89f Compare February 19, 2025 23:20

vitalybuka reviewed Feb 19, 2025

View reviewed changes

hartwork force-pushed the ubuntu-20-04-support branch from 46cc89f to 3156448 Compare February 19, 2025 23:43

hartwork added 11 commits February 20, 2025 01:03

Support Protobuf <3.12.0 lacking FieldDescriptor::has_presence()

ac1d4d2

Signed-off-by: Sebastian Pipping <[email protected]>

Support Protobuf <3.8.0 lacking Any::GetDescriptor()

6e38394

Signed-off-by: Sebastian Pipping <[email protected]>

Support Protobuf <3.8.0 lacking .SetRecursionLimit and .AllowUnknownF…

4ed4690

…ield Signed-off-by: Sebastian Pipping <[email protected]>

fuzzer_test.h: Add -print_final_stats=1 to libFuzzer call

6a66865

.. to ease future debugging Signed-off-by: Sebastian Pipping <[email protected]>

Make LibFuzzerExampleTest fail earlier

5b6909a

.. now that the fixed seed makes that safe to do Signed-off-by: Sebastian Pipping <[email protected]>

Reject Clang <12 because it breaks libfuzzer_example_test

77ebfaa

Signed-off-by: Sebastian Pipping <[email protected]>

Make CI cover Ubuntu 20.04 (with -DLIB_PROTO_MUTATOR_DOWNLOAD_PROTOBU…

dd6f94b

…F=OFF) .. with Protobuf 3.6.1.3 (see https://packages.ubuntu.com/focal/libprotobuf-dev) Signed-off-by: Sebastian Pipping <[email protected]>

Sync CMake requirement for -DLIB_PROTO_MUTATOR_EXAMPLES=ON with reality

97d4b3b

.. while allowing to build the core project with CMake 3.10. Signed-off-by: Sebastian Pipping <[email protected]>

README.md: Document dependencies of core libprotobuf-mutator

561cd60

Signed-off-by: Sebastian Pipping <[email protected]>

hartwork force-pushed the ubuntu-20-04-support branch from 3156448 to 561cd60 Compare February 20, 2025 00:10

hartwork requested a review from vitalybuka February 20, 2025 00:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Ubuntu 20.04 LTS (where OSS-Fuzz is at) #274

Add support for Ubuntu 20.04 LTS (where OSS-Fuzz is at) #274

hartwork commented Feb 19, 2025 •

edited

Loading

hartwork commented Feb 19, 2025 •

edited

Loading

vitalybuka Feb 19, 2025

hartwork Feb 19, 2025 •

edited

Loading

vitalybuka Feb 19, 2025

hartwork Feb 19, 2025 •

edited

Loading

vitalybuka Feb 19, 2025

hartwork Feb 19, 2025

vitalybuka Feb 19, 2025

hartwork Feb 19, 2025

hartwork Feb 19, 2025

vitalybuka Feb 19, 2025

hartwork Feb 19, 2025

hartwork Feb 19, 2025

hartwork Feb 20, 2025

vitalybuka Feb 19, 2025

hartwork Feb 19, 2025

hartwork commented Feb 21, 2025

Add support for Ubuntu 20.04 LTS (where OSS-Fuzz is at) #274

Are you sure you want to change the base?

Add support for Ubuntu 20.04 LTS (where OSS-Fuzz is at) #274

Conversation

hartwork commented Feb 19, 2025 • edited Loading

hartwork commented Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

hartwork Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hartwork Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hartwork commented Feb 21, 2025

hartwork commented Feb 19, 2025 •

edited

Loading

hartwork commented Feb 19, 2025 •

edited

Loading

hartwork Feb 19, 2025 •

edited

Loading

hartwork Feb 19, 2025 •

edited

Loading