Skip to content

Conversation

@JakeIsMeh
Copy link
Contributor

What does this PR do?
Add TSan and UBSan to the list of sanitize options

How does this PR change Premake's behavior?
It is possible for users to enable TSan and UBSan now.

Anything else we should know?
I'm unable to run the tests so I'm unsure if they actually pass.

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits
  • Align documentation to your changes

You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!

Comment on lines 15 to 16
| Thread | Enables compiler support for ThreadSanitizer. |
| Undefined | Enables compiler support for UndefinedBehaviorSanitizer (UBSan). |
Copy link
Contributor

Choose a reason for hiding this comment

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

supported only by gcc/clang

Check also if msvc with clang toolset can support it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

clang-cl supports passing -fsanitize=undefined to it, but I am unable to find a corresponding vcxproj flag for it.

Copy link
Member

Choose a reason for hiding this comment

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

Just keeping it with gcc/clang without MSVC/MSBuild support is perfectly sufficient for now.

@Jarod42
Copy link
Contributor

Jarod42 commented Nov 10, 2024

Tests fail.
allowed in api.register seems missing.

@JakeIsMeh
Copy link
Contributor Author

Failing lines:

test_clang.lua:

135: test.contains({ "-fsanitize=fuzzer" }, clang.getcflags(cfg))
144: test.contains({ "-fsanitize=thread" }, clang.getcflags(cfg))
153: test.contains({ "-fsanitize=undefined" }, clang.getcflags(cfg))

test_gcc.lua:

401: test.contains({ "-fsanitize=thread" }, gcc.getldflags(cfg))
410: test.contains({ "-fsanitize=undefined" }, gcc.getldflags(cfg))

@JakeIsMeh
Copy link
Contributor Author

JakeIsMeh commented Nov 10, 2024

Regarding Clang libFuzzer, I can make the test pass, but I believe that the existing implementation might be wrong?
-fsanitize=fuzzer should be passed during both the compilation and linking step, but it is never passed as a linker flag.
It is also implemented in cxxflags only when it should be shared with cflags, because it can be used with C libraries.

@JakeIsMeh
Copy link
Contributor Author

I can revert the last commit (69bf538) and also the test changes for clang, and put them into another PR if desired.

@nickclark2016 nickclark2016 merged commit 7463784 into premake:master Nov 11, 2024
17 checks passed
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.

4 participants