Skip to content

[sycl-rel] Cherry-pick sanitizer patches #18885

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
Merged
merged 4 commits into from
Jun 18, 2025
Merged

[sycl-rel] Cherry-pick sanitizer patches #18885

merged 4 commits into from
Jun 18, 2025

Conversation

KornevNikita
Copy link
Contributor

Cherry-pick commits that reached the internal branch between intel/llvm cutoff and release branch pulldown.

@yingcong-wu @KornevNikita
[compiler-rt][builtins] Add missing flag for builtins standalone buil…
fdf345b 
…d (#133046)

When builtins are built with runtimes, it is built before compiler-rt,
and this makes some of the HAS_XXX_FLAGs missing. In this case, the
COMPILER_RT_HAS_FCF_PROTECTION_FLAG is missing which makes it impossible
to enable CET in this case. This patch addresses this issue by also
check for such flag in standalone build instead of relying on the
compiler-rt's detection.
@yingcong-wu @KornevNikita
[DeviceSanitizer][Coverity] die when fail to create context (intel#17600
e9fd4e7 
)

There is uncaught exception through logger creation in sanitizer layer
`context_t` 's creation. We catch exceptions here and die if the
creation of sanitizer layer context fails.
@yingcong-wu @KornevNikita
[DeviceSanitizer] Skip adding device sanitizers internal variable to …
aa53e1f 
…bundler symbol table (intel#17399)

`__MsanDeviceGlobalMetadata` etc are sanitizer internal variables, and
should not be bundled to the symbol table.
@KornevNikita KornevNikita requested review from a team and bader as code owners June 10, 2025 11:55
@mdtoguchi mdtoguchi requested a review from asudarsa June 10, 2025 15:51
@asudarsa
Copy link
Contributor

Can we get some inputs about the test failures seen here?

Thanks

@kbenzie
Copy link
Contributor

kbenzie commented Jun 11, 2025

I retried the UR jobs and they are green now.

@asudarsa
Copy link
Contributor

I retried the UR jobs and they are green now.

Thanks @kbenzie. Can anyone comment on the other failures? Thanks

@KornevNikita
Copy link
Contributor Author

I retried the UR jobs and they are green now.

Thanks @kbenzie. Can anyone comment on the other failures? Thanks

Failures are unrelated, the pre-commit on this branch is broken. Will be tested separately, I will provide a link a bit later.

asudarsa
asudarsa approved these changes Jun 11, 2025
View reviewed changes
Copy link
Contributor

@asudarsa asudarsa left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Cherry picks look good from 'offload bundler' perspective. It is interesting that offload bundler and the tests are inside the 'Driver' directory. But that concern is out-of-scope for this PR.

Thanks

@KornevNikita
Copy link
Contributor Author

Ignore the pre-commit, it's broken. The actual test run - https://github.com/intel/llvm/actions/runs/15631542807
@bader take a look please.

bader
bader reviewed Jun 13, 2025
View reviewed changes
@bader
Copy link
Contributor

bader commented Jun 13, 2025

@AlexeySachkov, does it make sense to request code owners review for cherry-picks to the release branch? All these patches were reviewed when being committed to the sycl branch.

@KornevNikita
Copy link
Contributor Author

@bader

Since we're (not the authors of original commits) cherry-picking these patches ourselves, we may cherry-pick something excessive (like this compiler-rt patch), so it's good to have someone double-checking it (though it slows down the process).

I believe in the future we'll update the process so that authors cherry-pick their patches themselves, maybe then we can update the protection rules to not require an approval.

bader
bader previously requested changes Jun 16, 2025
View reviewed changes
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

Please, remove llvm/llvm-project@e54f31a changes.

@KornevNikita
Copy link
Contributor Author

@bader done

@bader bader dismissed their stale review June 17, 2025 16:39

My concerns are addressed. I leave it @AlexeySachkov to follow-up.

@AlexeySachkov AlexeySachkov merged commit 38818a4 into intel:sycl-rel-6_2 Jun 18, 2025
21 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yingcong-wu yingcong-wu yingcong-wu left review comments

@pbalcer pbalcer pbalcer approved these changes

@maarquitos14 maarquitos14 maarquitos14 approved these changes

@mdtoguchi mdtoguchi mdtoguchi approved these changes

@asudarsa asudarsa asudarsa approved these changes

@bader bader bader left review comments

@AlexeySachkov AlexeySachkov Awaiting requested review from AlexeySachkov

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

Successfully merging this pull request may close these issues.
9 participants
@KornevNikita @asudarsa @kbenzie @bader @pbalcer @maarquitos14 @mdtoguchi @yingcong-wu @AlexeySachkov