Skip to content

Conversation

RoundofThree
Copy link
Member

This PR adds support for compiling purecap binaries with -fsanitize-coverage.

I have tested this in CHERI RISCV qemu.

This commit fixes type mismatches between uptr and usize.
This avoids the following errors:

<inline asm>:10:1: error: instruction requires the following: Not Capability Mode
   10 | tail __interceptor_signal@plt
      | ^
<inline asm>:19:1: error: instruction requires the following: Not Capability Mode
   19 | tail __interceptor_sigaction@plt
      |
@RoundofThree RoundofThree changed the title [SanCov] Support SanitizerCoverage for CHERI purecap binaries [SanCov][WIP] Support SanitizerCoverage for CHERI purecap binaries Apr 23, 2025
…n CHERI

When multiple private sancov global variables across object files share the same name,
even though they won't collide when they are linked together, their corresponding
__cap_merged_table will. This commit appends a unique module identifier to the sancov
global variable names to prevent collision. In the event that the unique module identifier
obtained through getUniqueModuleId() is empty, we use the clang PID and time.
@RoundofThree RoundofThree changed the title [SanCov][WIP] Support SanitizerCoverage for CHERI purecap binaries [SanCov] Support SanitizerCoverage for CHERI purecap binaries Apr 24, 2025
@RoundofThree RoundofThree requested a review from arichardson May 21, 2025 17:45
Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

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

First two commits look good the other two I am not sure are correct.

uptr Len = sizeof(InfoProc);
CHECK_EQ(internal_sysctl(Mib, ARRAY_SIZE(Mib), nullptr, (uptr *)&InfoProc, &Len, 0), 0);
usize Len = sizeof(InfoProc);
CHECK_EQ(internal_sysctl(Mib, ARRAY_SIZE(Mib), &InfoProc, &Len, nullptr, 0),
Copy link
Member

Choose a reason for hiding this comment

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

This change looks good, would you mind send it upstream?

#define SANITIZER_INTERCEPT_WCSDUP SI_POSIX
#define SANITIZER_INTERCEPT_SIGNAL_AND_SIGACTION (!SI_WINDOWS && SI_NOT_FUCHSIA)
#define SANITIZER_INTERCEPT_SIGNAL_AND_SIGACTION \
(!SI_WINDOWS && SI_NOT_FUCHSIA && !SANITIZER_RISCV64)
Copy link
Member

Choose a reason for hiding this comment

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

This seems wrong, do you know what is causing those problems?

auto Array = new GlobalVariable(
*CurModule, ArrayTy, false, GlobalVariable::PrivateLinkage,
Constant::getNullValue(ArrayTy), "__sancov_gen_");
Constant::getNullValue(ArrayTy),
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this change is needed? Also please avoid hardcoded AS200 checks and use DL.isFatPointer instead. But I am not convinced this change is actually required.

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