This is an archive of the discontinued LLVM Phabricator instance.

Emit hidden hostcall argument for sanitized kernels.
ClosedPublic

Authored by pvellien on Oct 29 2021, 7:54 AM.

Details

Summary

this patch - https://reviews.llvm.org/D110337 changes the way how hostcall hidden argument is emitted for printf, but the sanitized kernels also use hostcall buffer to report a error for invalid memory access, which is not handled by the above patch and it leads to vdi runtime error.

:0:rocdevice.cpp :2581: 2190944203723 us: 520847: [tid:0x7fee1d7d1700] Device::callbackQueue aborting with error : HSA_STATUS_ERROR_MEMORY_FAULT: Agent attempted to access an inaccessible address. code: 0x2b
Aborted (core dumped)

Diff Detail

Event Timeline

pvellien created this revision.Oct 29 2021, 7:54 AM
pvellien requested review of this revision.Oct 29 2021, 7:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 29 2021, 7:54 AM
arsenm added inline comments.Oct 30 2021, 7:35 AM
llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
825

Space after //

llvm/test/CodeGen/AMDGPU/hsa-metadate-hostcall-present-v3-asan.ll
1–2

You can do this with a single llc invocation.

Also typo "metadate" in the filename

pvellien updated this revision to Diff 384072.Nov 2 2021, 6:50 AM
pvellien marked 2 inline comments as done.

Addressed review comments.

yaxunl added a comment.Nov 2 2021, 8:26 AM

You can add

getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);

at https://github.com/llvm/llvm-project/blob/b2a2c38349a18b89b04d342632d5ea02f86dfdd6/clang/lib/CodeGen/CodeGenModule.cpp#L542

It will be a more consistent and efficient way to represent the need for hostcall.

pvellien updated this revision to Diff 384163.Nov 2 2021, 10:51 AM

Addressed comments.

yaxunl added inline comments.Nov 8 2021, 7:30 AM
clang/lib/CodeGen/CodeGenModule.cpp
547 ↗(On Diff #384163)

need to check this in clang/test/CodeGenCUDA/amdgpu-asan.cu

pvellien updated this revision to Diff 386083.Nov 10 2021, 12:53 AM

Added a check in amdgpu-asan.cu file

yaxunl accepted this revision.Nov 10 2021, 5:58 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Nov 10 2021, 5:58 AM

Thanks for accepting. I don't have commit access to llvm trunk yet. Could you please commit this patch? thanks.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptNov 10 2021, 2:06 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript