Skip to content

ci: use host-ASan for submodule bumps and add an ASan-debug label - #8716

Open
sa-faizal wants to merge 10 commits into
mainfrom
users/sa-faizal/enable-submodule-host-asan
Open

sa-faizal wants to merge 10 commits into
mainfrom
users/sa-faizal/enable-submodule-host-asan

Conversation

@sa-faizal

@sa-faizal sa-faizal commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Validate library and system submodule bumps with host-ASan builds, and let LLVM promotion PRs opt into full ASan builds with debug information.

ISSUE ID: #6624

Technical Details

  • Bump automation adds ci:host-asan to library and system bump PRs.
  • Add ci:asan-debug to select the existing linux-release-asan-debug preset for LLVM promotions or any other PR.
  • Sanitizer CI requires an explicit label. Updating a submodule pointer alone does not enable it or change the selected preset.
  • Label precedence: ci:asan-debug, then ci:asan, then ci:host-asan. Removing sanitizer labels opts out; ci:skip still skips CI.
  • Sanitizer PR builds remain build-only. GPU testing on the regular CI lane and existing scheduled/manual sanitizer test gating are unchanged.

Test Plan

Check label selection, precedence, opt-out, and disabled GPU testing using CI configuration tests. Run bump automation tests and repository lint checks.

Test Result

  • CI configuration and bump automation tests: 201 passed, 1 skipped, 55 subtests passed.
  • Repository lint checks passed.
  • GitHub unit and lint checks will rerun on the updated commit.

Submission Checklist

  • Looked over the contributing guidelines.

@therock-pr-bot

therock-pr-bot Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 3, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@sa-faizal sa-faizal changed the title ci: enable host-ASan tests for library and system submodule bumps ci: enable host-ASan testing for library, system, and LLVM bumps Oct 3, 2026
@sa-faizal
sa-faizal requested review from geomin12 and ronlieb October 7, 2026 04:09
@sa-faizal
sa-faizal marked this pull request as ready for review October 7, 2026 04:23
@sa-faizal

Copy link
Copy Markdown
Contributor Author

This will prevent introduction of any compiler promotion related build failures also like LCOMPILER-2794

@sa-faizal sa-faizal changed the title ci: enable host-ASan testing for library, system, and LLVM bumps ci: use host-ASan for library/system bumps and ASan debug for LLVM Oct 8, 2026
@sa-faizal
sa-faizal requested a review from nunnikri October 8, 2026 04:14
@sa-faizal sa-faizal changed the title ci: use host-ASan for library/system bumps and ASan debug for LLVM ci: use host-ASan for submodule bumps and add an ASan-debug label Oct 8, 2026
@sa-faizal
sa-faizal requested a review from ScottTodd October 8, 2026 18:15

@ScottTodd ScottTodd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking mostly better now

@sa-faizal

sa-faizal commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review feedback in commit 0eeb302. Thank you!

@sa-faizal
sa-faizal requested a review from ScottTodd October 14, 2026 03:58
@sa-faizal
sa-faizal requested a review from geomin12 October 14, 2026 03:58
Comment on lines +96 to +98
Sanitizer labels do not override GPU test gating. GPU sanity checks and
component tests remain subject to trigger and runner restrictions. CPU tests
may still run. `ci:skip` still skips CI.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unnecessary/confusing detail here (often seen with "low effort level" AI written docs). Please trim. This whole paragraph could just be deleted.

  • The section is talking about submodule update pull requests. Why is ci:skip mentioned at all here?
  • What is "CPU tests may still run" adding? That seems better suited for a section on GPU test labels, separate from automated submodule update PRs.

Comment on lines 1856 to +1860
# ASAN variant selection:
# 1. ci:asan label -> asan (explicit full ASAN, highest priority)
# 2. ci:host-asan label -> host-asan (explicit)
# 3. push/pull_request events -> host-asan (default for pre/postsubmit)
# 4. schedule/workflow_dispatch -> asan (nightly/manual get full ASAN)
# 1. ci:asan-debug label -> asan-debug (full ASAN with debug info)
# 2. ci:asan label -> asan (explicit full ASAN)
# 3. ci:host-asan-debug label -> host-asan-debug (host-only ASAN with debug info)
# 4. ci:host-asan label -> host-asan (explicit)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I just merged #9018 which validates that some known labels conflicts are not present. Let's make it so only one ci:*asan label at a time can be added and if multiple are seen the CI fails. Then we won't need this priority list.

Comment on lines -369 to +377
git = cm.GitContext(
changed_files=["CMakeLists.txt", "build_tools/script.py"],
)
self.assertTrue(cm.should_skip_ci(inputs, git))
for changed_files in (
["CMakeLists.txt", "build_tools/script.py"],
["compiler/amd-llvm"],
):
with self.subTest(changed_files=changed_files):
git = cm.GitContext(
changed_files=changed_files, submodule_paths=["compiler/amd-llvm"]
)
self.assertTrue(cm.should_skip_ci(inputs, git))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Subtest is not useful here. All that matters is that CI is skipped when there are code changes and no asan label. We don't need to separately check CMakeLists.txt and build_tools/script.py as different types of code changes for that testing.

@geomin12 geomin12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm. we can roll back if needed

This branch has not been deployed

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

Labels

None yet

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

5 participants