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.

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

tinyusb: initial integration #9143

Merged
merged 1 commit into from Jan 4, 2023
Merged

Conversation

silvergasp
Copy link
Contributor

Signed-off-by: Nathaniel Brough nathaniel.brough@gmail.com

CC @hathach

@silvergasp silvergasp marked this pull request as draft December 6, 2022 05:15
@silvergasp
Copy link
Contributor Author

@hathach Now that hathach/tinyusb#1716 is merged I'm going to mark this as ready to review. I'm unsure if all emails in the project.yaml need to be in the git log or just some.

@silvergasp silvergasp marked this pull request as ready for review December 12, 2022 04:11
@hathach
Copy link
Contributor

hathach commented Dec 12, 2022

@silvergasp thank you for making this pr

Signed-off-by: Nathaniel Brough <nathaniel.brough@gmail.com>
@silvergasp
Copy link
Contributor Author

Attempting to address build failures in hathach/tinyusb#1823

@jonathanmetzman
Copy link
Contributor

We accept this project. Just fix the build failure please.

@silvergasp
Copy link
Contributor Author

Just waiting on a review from @hathach on hathach/tinyusb#1823 which fixes the build failure.

@silvergasp
Copy link
Contributor Author

That PR has been merged. So I think the build should work now. I did go through and check each fuzzing engine and each santizer. But I didn't check the entire matrix of each combination of the two locally.

@silvergasp
Copy link
Contributor Author

Apologies, it seems it's still broken. Is it possible somehow to run the complete CI locally?

@jonathanmetzman
Copy link
Contributor

jonathanmetzman commented Jan 4, 2023

Apologies, it seems it's still broken. Is it possible somehow to run the complete CI locally?

It doesn't look broken to me. Usually you just do what the CI job did locally (e.g. python infra/helper.py build_fuzzers --engine honggfuzz --sanitizer address tinyusb)

Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

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

lgtm

@jonathanmetzman jonathanmetzman merged commit d8cd246 into google:master Jan 4, 2023
@jonathanmetzman
Copy link
Contributor

lgtm

Oh man I must've been looking at something stale. Gonna have to revert this and you can submit another PR. Sorry for the trouble.

jonathanmetzman added a commit that referenced this pull request Jan 4, 2023
jonathanmetzman added a commit that referenced this pull request Jan 4, 2023
@jonathanmetzman
Copy link
Contributor

Yeah this must be some weird caching thing
image

@jonathanmetzman
Copy link
Contributor

Apologies, it seems it's still broken. Is it possible somehow to run the complete CI locally?

It doesn't look broken to me. Usually you just do what the CI job did locally (e.g. python infra/helper.py --engine hongfuzz --sanitizer address tinyusb)

The essential one is address (ASAN) so try to get that working first, and the others can be disabled (python infra/helper.py --engine libfuzzer --sanitizer address tinyusb

@jonathanmetzman
Copy link
Contributor

Apologies, it seems it's still broken. Is it possible somehow to run the complete CI locally?

It doesn't look broken to me. Usually you just do what the CI job did locally (e.g. python infra/helper.py --engine hongfuzz --sanitizer address tinyusb)

The essential one is address (ASAN) so try to get that working first, and the others can be disabled (python infra/helper.py build_fuzzers --engine libfuzzer --sanitizer address tinyusb

OK so your CI jobs are failing because they are failing the build check. To do this locally run the following command after running build_fuzzers: python infra/helper.py check_build --engine libfuzzer --sanitizer <sanitizer, e.g. "address"> tinyusb

@jonathanmetzman
Copy link
Contributor

the specific errors i see are:
clang-15: fatal error: invalid argument '-fsanitize=address' not allowed with '-fsanitize=memory'
BAD BUILD: ASan build of /tmp/not-out/tmpkpsh74_e/cdc seems to be compiled with UBSan.
BAD BUILD: UBSan build of /tmp/not-out/tmpwgxy6rrx/cdc seems to be compiled with ASan.

(the last two will repro with check_build)

I think it is doing something weird in builds. I think it must be adding sanitizer flags instead of using our CXXFLAGS and CFLAGS

@silvergasp
Copy link
Contributor Author

silvergasp commented Jan 5, 2023

Apologies, it seems it's still broken. Is it possible somehow to run the complete CI locally?

It doesn't look broken to me. Usually you just do what the CI job did locally (e.g. python infra/helper.py --engine hongfuzz --sanitizer address tinyusb)

The essential one is address (ASAN) so try to get that working first, and the others can be disabled (python infra/helper.py build_fuzzers --engine libfuzzer --sanitizer address tinyusb

OK so your CI jobs are failing because they are failing the build check. To do this locally run the following command after running build_fuzzers: python infra/helper.py check_build --engine libfuzzer --sanitizer <sanitizer, e.g. "address"> tinyusb

Yeah ok, that was about what I was doing already. I was more curious if there was an easy way to iterate over all the combinations locally.

Anyway, I must have missed something, so I'll investigate further this afternoon, and open a new PR when I've ironed out the kinks.

eamonnmcmanus pushed a commit to eamonnmcmanus/oss-fuzz that referenced this pull request Mar 15, 2023
Signed-off-by: Nathaniel Brough <nathaniel.brough@gmail.com>

CC @hathach

Signed-off-by: Nathaniel Brough <nathaniel.brough@gmail.com>
eamonnmcmanus pushed a commit to eamonnmcmanus/oss-fuzz that referenced this pull request Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants