Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add a script to produce combined fuzzing coverage profiles. #24

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add a script to produce combined fuzzing coverage profiles. #24

wants to merge 1 commit into from

Conversation

Crypt-iQ
Copy link
Collaborator

@Crypt-iQ Crypt-iQ commented Oct 8, 2024 •
edited
Loading

Pretty much a copy of corpus_merge.sh. After running the script, go to your lnd directory then run go tool cover -html ../lnd-fuzz/coverage/profile to see coverage in HTML.

Addresses part of #10

@Roasbeef Roasbeef requested a review from gijswijs October 17, 2024 17:48
morehouse
morehouse reviewed Oct 22, 2024
View reviewed changes
Copy link
Contributor

@morehouse morehouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

cov_profiles.sh Outdated

# Collect coverage profiles.
for p in ${packages[@]}; do
cd $PWD/$p/testdata/fuzz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems BASE_DIR and PWD are being used interchangeably, but it is possible the user runs cov_profiles.sh from a directory other than lnd-fuzz.

We should only use BASE_DIR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is true in the above case but not in for f in $(ls $PWD); do

cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
@Crypt-iQ Crypt-iQ force-pushed the cov_profiles_10_07_2024 branch from 82d98b2 to 6b373e3 Compare October 25, 2024 02:44
@Crypt-iQ Crypt-iQ requested a review from morehouse October 29, 2024 16:05
morehouse
morehouse reviewed Oct 30, 2024
View reviewed changes
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
morehouse
morehouse reviewed Oct 30, 2024
View reviewed changes
cov_profiles.sh Outdated Show resolved Hide resolved
gijswijs
gijswijs reviewed Nov 5, 2024
View reviewed changes
Copy link
Collaborator

@gijswijs gijswijs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very nice improvement. Concept ACK

Made some remarks throughout.

cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
cov_profiles.sh
for p in ${packages[@]}; do
cd "${BASE_DIR}/${p}/testdata/fuzz"

for f in $(ls $PWD); do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use for with a glob pattern. ./* is safer than * but both work in this case.

Suggested change
for f in $(ls $PWD); do
for f in ./*; do

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason this did not work for me

cov_profiles.sh Show resolved Hide resolved
cov_profiles.sh Outdated Show resolved Hide resolved
@Crypt-iQ Crypt-iQ force-pushed the cov_profiles_10_07_2024 branch from 6b373e3 to 9054ca1 Compare December 10, 2024 16:51
@lightninglabs-deploy
Copy link

@gijswijs: review reminder
@morehouse: review reminder

morehouse
morehouse suggested changes Jan 6, 2025
View reviewed changes
Copy link
Contributor

@morehouse morehouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried to test the script, but it's broken for me. See inline comments.

cov_profiles.sh
for p in ${PACKAGES[@]}; do
pushd "${BASE_DIR}/${p}/testdata/fuzz"

for f in $(ls $PWD); do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The outer pushd actually causes issues if LND_DIR is a relative path and the script is run from a directory that is not BASE_DIR. For example:

$ ./lnd-fuzz/cov_profiles.sh ./lnd
./lnd-fuzz/cov_profiles.sh: line 38: pushd: ./lnd//lnwire/: No such file or directory

We can remove the outer loop pushd/popd entirely by doing this:

Suggested change
for f in $(ls $PWD); do
for f in $(ls "${BASE_DIR}/${p}/testdata/fuzz"); do

cov_profiles.sh
Comment on lines +58 to +59
profile_str+="./${coverage_dirs[$i]},"
done
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation is off:

Suggested change
profile_str+="./${coverage_dirs[$i]},"
done
profile_str+="./${coverage_dirs[$i]},"
done

cov_profiles.sh
done
profile_str+="./${coverage_dirs[coverage_dirs_len-1]}"

go tool covdata textfmt -i=$profile_str -o coverage/profile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
go tool covdata textfmt -i=$profile_str -o coverage/profile
go tool covdata textfmt -i="${profile_str}" -o="${BASE_DIR}/coverage/profile"

cov_profiles.sh
for f in $(ls $PWD); do
# Copy corpus to CACHE_DIR.
mkdir -p "${CACHE_DIR}/${f}"
cp -a "${BASE_DIR}/${p}/testdata/fuzz/${f}"/ "${CACHE_DIR}/${f}/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is broken. It creates a directory structure like this:

/tmp/tmp.dEGuCuET9c/
└── FuzzAcceptChannel
    └── FuzzAcceptChannel
        ├── 01974d805b10b06eade02cf7d9caa0691f5fae41e7364a61fe2ff471980b10b6
        ├── 036a0c9a80cd7d7dc5f072b195ee7b39ee31c54d69750fd4da7881c450ff3284
         ...

Then the find command below finds 0 files.

Please fix and test locally before requesting the next review.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is num_inputs zero for you? On my machine the directory structure isn't broken like that. I printed some out mid-execution:

/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/1bf7a2fb3006e4feff01f8e8cdb48550529359bd453f1e4e242f5135f723c8b5
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/e48b4dfb6c410edecfea78d8ce3f07129df3317f19fb634e696a8fc76f8da10b
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/d2e17402aa067bf2c54d7b4243609ad4c7ee30a44e0b72d9abc60014addeff21
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/48118d8a7e6ecb81d70698f66bffea50fc3bdbb0f22f103efb0b6144957e26d0
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/a2a5c30f20a35d72f110987a438f65e8b22d9c81e3c71f53a71dd5ab0e63a7db
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/2b1f18c6c3a15a09bd3ee69000b5a53b5a697898d70aeb715cd0e9e9e22a79f0
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/6eff1b81ae03b28e9c031d182dd0865f51acc7022a621d413db27cb742964541
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/c51f1953c050a6f4539277ab345cbf4ce2835806cca7757ac65e9c83b92f2ab0
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/4932a31eb46b1bbdbe6d1126f8c28e74902b1e1f2ad3ff1e41675c3c9081270c
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/7e549e95fb753bde329abc2b297cfc9c4a1f9048665bccf94ce764b101fdf058
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/4a9ad617da4337334a12d22c885fe378e22133e92011afaa20445b16251047f0
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/0d63a1630149416e7efd3e6b73dcf09ad8345629ef2d0eda3b18ddf5927b9e2e
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/d7401a75b0e2b3ac0fa278da389a94cacbda5a1f8452ebc217637dcf308029b1
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/422b15220ed8be5c341d6d143c8c6db4464f98f084f3c9017d1a3c6054014bed
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/8dd01d48e96d5943b24a9f9c280e5ca75c789991d90cef0422b7ab6599eb7e4d
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/251acda1764072fd515e8e4932e51a7b373f65df2200ceb7ff4e787ffe8e3dc4
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/b25be2e72815828424e79c2ef814c41b323987f840656f36a7ab116a2f23fa05
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/587f94cf7b1cf3bff954017f48cff3637883c529793f04bc7966d326f367e00e
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/887bfbd579888dcc30912465074cd0d67bce2f523207f1bfd505943c49e9bba0
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/e6d75e739802ed09f4ed97b1467baffaa2b0fcda1144ad64d337107ce09aa99b
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/256b090be8e9aecbd27aac8420633c7aab1a05040fc0f809143b98068ff72da9
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/430ccbd956522814d47c8dea647ca213d0e9326f0c06a3dfaeff7b4cdcbe8923
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/582528ddfad69eb57775199a43e0f9fd5c94bba343ce7bb6724d4ebafe311ed4
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/4a9938266e334ca22ce0b5c27083b61271ce2ba85d0a5b3c58b7f59bd7121a36
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/0b85443d6c2ae08a9f37c19fc6341ca8521b2538b68a2da93ed7c153da0ab621
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/fe2aeefa6273e28ad6b487464d9c15cec0c170d22aa76e2e4df50b44306b5b30
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/b26e76853c686e1ed6959e464530bac08c77baf4a4448af91f0325a0f65f3883
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/65b624cbccc4015447333cce1e85e801128ea135ae088d894e27d7a76eedf760
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/934353b38ce0d3d1dffad8741ea4c2b3be73feeb7054c9ddc5f439bcef2db6e5
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/e59545082786252c7f1dfc3017293ba11c16189cb542773df7be0938bc8b4d79
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/5aebca94512ff09b7d212b38f3e8f119e5c7ce9ab823619d0c9e37a6c106b502
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/5df343afd59baab158ae2c5319956ef887e41f2c3ab6d12614d4391dcf5ea172
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/af9eb444ef300ab57b97a08f7d675cfb3747e4e0ee12276641195fae1e8b14af
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/c51152df05a2c51153474f7cfc05031fc64d7a0d914234ddc0ee45b78073ad17
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/437edc4da2a09d32b1938fc2855523c50d6066e889fb3b8381c3396048d1c148
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/bb95a9841986605ee47242d3219bf80f23a554c2db7446ada81f6f9716e31998
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/8453ecd6906650e311f41a751955408849aa0794b8921ed90dd273f0551fe170
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/e78eca15ff87ed9c7251da63235f3848442357122b67ef229c20f16a6f5f54e8
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/c8c9c4055828e24dbd37a1ed2f73702dd4cae2441ff6f371d9ff8935c7d0f004
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/b4a7806f6ec36fc97415326a08e75d3b6d458879394106dd361a722878f7d82a
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/11c1cf5b566b8db1e74854669399b4995a6c7363aac81bb9ad6d75d8f3f4e5ac
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/caf81e9797b19c76c1fc4dbf537d4d81f389524539f402d13aa01f93a65ac7e9
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/c353d5d4424fa3f1ebdd05f4a725b0bb7c6c8a562f9700d55f451df54de018e3
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/aab9a578f92e33e40fffce8e7050d9fc2dc5e2be4626c40b9e211c691dc2b61f
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/a254ef20622c03557f2952070f3bfede07c689b4d329e3490b2a261536f8c4f4
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/73741b7495a574d21a294f4891ce6541f60f26908f2cd5c30131a75ea890a4e1
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/ec72f669d648d8d9b9f75a3b303897c59b11e4bfb7622f25ff251a92f182bc2a
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/24f53a36f8832fec65cac0aa0f3b43ec1c904414fa6d38f6fc288b0bbd69588a
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/5279c19cb4b4c5bc94dda5c9f9c1de5cabe2c906525ce661a2bbce2397f62aa9
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/ed1cf09c58aef0e5ce824a34c398f5a4c9ab10e20e99a91d55f0dcfed9324ee6
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/731951fe84fa6f3a7f6ee8adaa585d4f6a01f359a04737e51ffc70f16f480b9b
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/356e28f5914a0f16f3cef81330f1d92060be4d694a93dedd654bf48743a7d2bd
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/3be26ca8db01f4ca64e57f3665335ab84600b949c7249cbb81939d25a858bd83
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/bd113b4a2cd9911a52a153964221a976a5fb8966f1f3411427160f7277fbb8b5
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/022ab5ee587c4f1e98b0a5bfb21c60f0da62d3da71de1da782de960e85ef4b07
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/77d09f41ba2430c5d2c1031c789f78a091d7bc5c5eb53ae476d618113f27101c
/var/folders/19/1cm482qj5nx47r7035tllzrw0000gn/T/tmp.cTe4Bskq/FuzzChannelReestablish/6ebb29ad084df16e9aa8eaf66dc72230da023d04b83081637c6da02d8546785d

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. mkdir -p "${CACHE_DIR}/${f}" creates the first FuzzAcceptChannel directory. Then cp -a "${BASE_DIR}/${p}/testdata/fuzz/${f}"/ "${CACHE_DIR}/${f}/" copies the second FuzzAcceptChannel directory into the first one.

This is standard behavior of cp on every Linux machine I've used. I guess Mac is different?

There are a couple ways this could be fixed. One way is to copy the contents of the first directory into the second:

Suggested change
cp -a "${BASE_DIR}/${p}/testdata/fuzz/${f}"/ "${CACHE_DIR}/${f}/"
cp -a "${BASE_DIR}/${p}/testdata/fuzz/${f}"/* "${CACHE_DIR}/${f}/"

The other way is to remove the mkdir entirely and then copy the directory over like this:

Suggested change
cp -a "${BASE_DIR}/${p}/testdata/fuzz/${f}"/ "${CACHE_DIR}/${f}/"
cp -a "${BASE_DIR}/${p}/testdata/fuzz/${f}" "${CACHE_DIR}/"

By the way, it looks like there will be input cross-contamination if fuzz targets from different packages have the same name. We should probably clean out the cache dir between coverage runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morehouse morehouse morehouse requested changes

@gijswijs gijswijs Awaiting requested review from gijswijs

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Crypt-iQ @lightninglabs-deploy @gijswijs @morehouse