Skip to content

[SYCL][CMAKE] Drop nodlopen from hardening flags #19357

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

Conversation

AlexeySachkov
Copy link
Contributor

We can't have this flag being applied globally, because it then affects SYCL RT which has plenty of libraries that are supposed to be dlopened by design.

It probably makes sense to apply nodlopen for some of our executables, but that should be done on a case-by-case basis.

For now, let's just fix the toolchain with hardening flags applied, any improvements can be done as a follow-up later.

We can't have this flag being applied globally, because it then affects
SYCL RT which has plenty of libraries that are supposed to be `dlopen`ed
by design.

It probably makes sense to apply `nodlopen` for some of our executables,
but that should be done on a case-by-case basis.

For now, let's just fix the toolchain with hardening flags applied, any
improvements can be done as a follow-up later.
Copy link
Contributor

@maarquitos14 maarquitos14 left a comment

Choose a reason for hiding this comment

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

Makes sense. LGTM.

@AlexeySachkov AlexeySachkov merged commit 38553eb into intel:sycl Jul 9, 2025
24 checks passed
@AlexeySachkov AlexeySachkov deleted the private/asachkov/drop-nodlopen-from-hardening-flags branch July 9, 2025 13:59
AlexeySachkov added a commit that referenced this pull request Jul 11, 2025
We can't have this flag being applied globally, because it then affects
SYCL RT which has plenty of libraries that are supposed to be `dlopen`ed
by design.

It probably makes sense to apply `nodlopen` for some of our executables,
but that should be done on a case-by-case basis.

For now, let's just fix the toolchain with hardening flags applied, any
improvements can be done as a follow-up later.
AlexeySachkov added a commit that referenced this pull request Jul 16, 2025
This PR contains several cherry-picks and some unique changes which have
not been applied to the `sycl` branch yet.

The intent of this PR is to enable as much (quickly) possible hardening
flags to be in better compliance with our SDL requirements.
The main thing this PR is after are things like immediate bindings,
fortify source, stack protection and `relro`.
The thing that this PR is **not** after are extra warning flags - some
of them we can't apply globally because LLVM itself isn't warning free,
some of them we can't apply even locally to SYCL RT because we haven't
fixed corresponding warnings yet.

Patches which were cherry-picked from the `sycl` branch:

- [SYCL] Fix AddSecurityFlags having no side effects
(#17690)
  - Patch-By: Alexey Sachkov <[email protected]>
- [SYCL] Refresh hardening flags applied to the project
(#18398)
  - Patch-By: Nikita Kornev <[email protected]>
- [SYCL][CMAKE] Refactor -fPIE handling
(#19235)
  - Patch-By: Alexey Sachkov <[email protected]>
- [SYCL][CMAKE] Drop nodlopen from hardening flags
(#19357)
  - Patch-By: Alexey Sachkov <[email protected]>
- [SYCL][CMAKE] Fix _FORTIFY_SOURCE=3
(#19268)
  - Patch-By: Alexey Sachkov <[email protected]>
- [SYCL][CMake] Properly enable -pie hardening flag
(#19447)
  - Patch-By: Alexey Sachkov <[email protected]>

Additional changes which have **not** been applied to the `sycl` branch:
- Adjusted `configure.py` to the new way of `-fPIE` handling
- Dropped `/sdl` flag because LLVM isn't warning-free
  - it will be applied locally to SYCL RT in a separate PR against the `sycl` branch for future releases
- Dropped `/analyze` flag because SYCL RT isn't warning-free 
  - it will be applied locally to SYCL RT in a separate PR against the `sycl` branch for future releases
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.

2 participants