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

ci: get sanitizer results #2926

Closed
wants to merge 5 commits into from
Closed

ci: get sanitizer results #2926

wants to merge 5 commits into from

Conversation

Saviq
Copy link
Collaborator

@Saviq Saviq commented May 12, 2023

No description provided.

@Saviq Saviq force-pushed the get-sanitizer-results branch from 59de261 to b34a4fa Compare May 12, 2023 13:26
@codecov
Copy link

codecov bot commented May 12, 2023 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (ci-enable-asan@3b9fe2d). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 5d644fc differs from pull request most recent head a4a7f3c. Consider uploading reports for the commit a4a7f3c to get more accurate results

@@                Coverage Diff                @@
##             ci-enable-asan    #2926   +/-   ##
=================================================
  Coverage                  ?   77.70%           
=================================================
  Files                     ?     1056           
  Lines                     ?    73394           
  Branches                  ?        0           
=================================================
  Hits                      ?    57034           
  Misses                    ?    16360           
  Partials                  ?        0

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@Saviq Saviq force-pushed the get-sanitizer-results branch from f2d4a80 to 710c6aa Compare May 12, 2023 15:00
@Saviq Saviq self-assigned this May 12, 2023
@Saviq Saviq force-pushed the get-sanitizer-results branch 3 times, most recently from c643d00 to f395fb2 Compare May 12, 2023 16:28
@Saviq Saviq force-pushed the get-sanitizer-results branch from f395fb2 to 4968b34 Compare May 19, 2023 15:46
@RAOF
Copy link
Contributor

RAOF commented Jun 5, 2023

Ok. So, UBSan looks like it should be good to add to the block-CI set; we don't invoke any behaviour it can detect as UB.

We should be able to turn on ASan and disable leak checking, but

  • The gcc asan build is baroquen.
  • The clang asan build picks up some legitimate bugs - ODR violations, something maybe broken in ShmMapping::CurrentMapping, and maybe a UAF in ObserverMultiplexer - and then something in the UDevWrapperDeathTest, which may well just be a false positive.

So, current status: We should add UBsan to the list off builds required to pass in CI, and fix the bugs that asan detects.

@Saviq Saviq force-pushed the get-sanitizer-results branch from 72a7aad to b263aaa Compare August 28, 2023 13:32
@Saviq Saviq mentioned this pull request Aug 28, 2023
Merged
@Saviq Saviq force-pushed the get-sanitizer-results branch 10 times, most recently from 1832636 to 31238ed Compare August 30, 2023 15:33
@Saviq
Copy link
Collaborator Author

Saviq commented Aug 30, 2023 •
edited
Loading

Parts of this are now in:

  1. ci: add ubsan to standard CI #3022
  2. ci: run tests in ubsan #3025
  3. sanitizers: clean things up for sanitizers, spread: simpler dependency install #3028
  4. ci: enable AddressSanitizer #3026

@Saviq Saviq changed the base branch from main to ci-enable-asan August 30, 2023 15:33
@Saviq Saviq force-pushed the get-sanitizer-results branch from 31238ed to 796e413 Compare August 30, 2023 15:40
@Saviq Saviq force-pushed the get-sanitizer-results branch from 796e413 to 5d644fc Compare September 11, 2023 08:21
Saviq and others added 3 commits November 8, 2023 11:34
@Saviq
spread: run tests in asan
231f1b0
@RAOF @Saviq
ASan: Don't do leak checking
e761d39 
Sadly, we *know* we have benign leaks in hard-to-fix places. It would still
be valuable to fail CI if ASan detects anything *else* - out of bounds writes,
use-after-free, etc.

Disable the leak checker so we can mark ASan failures as CI-fatal.
Hopefully we can get get the leaks fixed or correctly-suppressed *some*time!
@Saviq
asan: skip link order verification
174adcb 
We're LD_PRELOAD'ing a bunch when running tests.
@Saviq Saviq force-pushed the get-sanitizer-results branch from 5d644fc to a4a7f3c Compare November 8, 2023 09:37
@mattkae mattkae force-pushed the ci-enable-asan branch 2 times, most recently from bbdd1a4 to 4a5defa Compare July 17, 2024 20:07
@mattkae mattkae force-pushed the ci-enable-asan branch 2 times, most recently from 8458ca6 to a580cc8 Compare August 1, 2024 14:17
Base automatically changed from ci-enable-asan to main August 1, 2024 18:11
@Saviq Saviq closed this Sep 23, 2024
@Saviq Saviq deleted the get-sanitizer-results branch September 23, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Saviq Saviq

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Saviq @RAOF