Skip to content

tests: verify sanitizer cc_feature injection via compile-time assertion - #31

Open
RSingh1511 wants to merge 1 commit into
eclipse-score:mainfrom
RSingh1511:rs/sanitizer-test-coverage
Open

tests: verify sanitizer cc_feature injection via compile-time assertion#31
RSingh1511 wants to merge 1 commit into
eclipse-score:mainfrom
RSingh1511:rs/sanitizer-test-coverage

Conversation

@RSingh1511

@RSingh1511 RSingh1511 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Notes for Reviewer

Pre-Review Checklist for the PR Author

  • PR title is short, expressive and meaningful
  • Commits are properly organized
  • Relevant issues are linked in the References section
  • Tests are conducted
  • Unit tests are added

Checklist for the PR Reviewer

  • Commits are properly organized and messages follow S-CORE commit guidelines
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

Post-review Checklist for the PR Author

  • All open points are addressed and tracked via issues

References

Closes #24

@RSingh1511
RSingh1511 requested a review from nradakovic July 20, 2026 13:17
@RSingh1511
RSingh1511 force-pushed the rs/sanitizer-test-coverage branch 4 times, most recently from 99c6076 to e0215f5 Compare July 21, 2026 14:05
Comment thread sanitizers/features/BUILD.bazel Outdated
visibility = ["//visibility:public"],
)

cc_feature(

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.

So I had something else in mind. Option number 1:

cc_feature(
    name = "ubsan_base",
    args = [
        "":ubsan_link_args_base",
    ],
    visibility = ["//visibility:public"],
    implies = ["debug_symbols"],
    feature_name = "score_ubsan_base",
)

cc_feature(
    name = "ubsan_gcc",
    feature_name = "score_ubsan_gcc",
    visibility = ["//visibility:public"],
    implies = ["ubsan_base"],
)

cc_feature(
    name = "ubsan_clang",
    args = [
        "":ubsan_link_args_clang_runtime",
    ],
    feature_name = "score_ubsan_clang",
    visibility = ["//visibility:public"],
    implies = ["ubsan_base"],
)

Option 2 (requires some changes on bazel_platforms side):

cc_feature(
    name = "ubsan",
    args = select({
        "@score_bazel_platforms//config:clang": [
             ":ubsan_link_args_clang_runtime",
         ],
        "//condition:default": [],
    }) + [
         ":ubsan_compile_args",
         ":ubsan_link_args_base",
    ]
    visibility = ["//visibility:public"],
    implies = ["debug_symbols"],
    feature_name = "score_ubsan",
)

Flag @score_bazel_platforms//config:clang is global config set via platform (--platforms=@score_bazel_platforms//:x86_64-linux-gcc_12.2.0-posix or --platforms=@score_bazel_platforms//:x86_64-linux-clang_16-posix.
With your change we still cannot use ubsan on gcc based toolchains. With my, we can.

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.

For start, I would maybe go with 1 since this one doesn't require that we introduce new platform definitions (which is OK, just we need approval from infra group).

@RSingh1511
RSingh1511 force-pushed the rs/sanitizer-test-coverage branch from e0215f5 to ccd5827 Compare July 27, 2026 07:38
@RSingh1511
RSingh1511 force-pushed the rs/sanitizer-test-coverage branch from ccd5827 to e6bbfce Compare July 27, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add validation and test coverage for migrated sanitizer behavior

2 participants