This is an archive of the discontinued LLVM Phabricator instance.

Differential D112820

Emit hidden hostcall argument for sanitized kernels.
ClosedPublic

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

Details

Reviewers
b-sumner
kpyzhov
JonChesterfield
yaxunl
sameerds
Commits
rG4b3881e9f319: Emit hidden hostcall argument for sanitized kernels
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
Herald added subscribers: foad, kerbowa, hiraditya and 3 others. · View Herald TranscriptOct 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
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B131427: Diff 383349.Oct 29 2021, 8:51 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.

pvellien updated this revision to Diff 384075.Nov 2 2021, 6:53 AM

rebased

Harbormaster completed remote builds in B131961: Diff 384075.Nov 2 2021, 7:30 AM
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.

Harbormaster completed remote builds in B132013: Diff 384163.Nov 2 2021, 11:34 AM
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

Harbormaster completed remote builds in B133431: Diff 386083.Nov 10 2021, 1:40 AM
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
pvellien added a comment.Nov 10 2021, 8:31 AM

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

Closed by commit rG4b3881e9f319: Emit hidden hostcall argument for sanitized kernels (authored by yaxunl). · Explain WhyNov 10 2021, 2:06 PM
This revision was automatically updated to reflect the committed changes.
yaxunl added a commit: rG4b3881e9f319: Emit hidden hostcall argument for sanitized kernels.
Herald added a project: Restricted Project. · View Herald TranscriptNov 10 2021, 2:06 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
pvellien mentioned this in D116216: Prevent adding module flag - amdgpu_hostcall multiple times..Dec 23 2021, 8:52 AM

Revision Contents

PathSize
llvm/
lib/
Target/
AMDGPU/
5 lines
test/
CodeGen/
AMDGPU/
52 lines

Diff 383349

llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp

Context not available.
if (M->getNamedMetadata("llvm.printf.fmts")) if (M->getNamedMetadata("llvm.printf.fmts"))
emitKernelArg(DL, Int8PtrTy, Align(8), "hidden_printf_buffer", Offset, emitKernelArg(DL, Int8PtrTy, Align(8), "hidden_printf_buffer", Offset,
Args); Args);
else if (M->getModuleFlag("amdgpu_hostcall")) { else if (M->getModuleFlag("amdgpu_hostcall") ||
//Emit hidden_hostcall_buffer argument for address sanitized functions
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-        //Emit hidden_hostcall_buffer argument for address sanitized functions
-        //since they use hostcall buffer to report invalid memory access.
-        Func.hasFnAttribute(llvm::Attribute::SanitizeAddress)) {
+             // Emit hidden_hostcall_buffer argument for address sanitized
+             // functions since they use hostcall buffer to report invalid memory
+             // access.
+             Func.hasFnAttribute(llvm::Attribute::SanitizeAddress)) {
Lint: Pre-merge checks: clang-format: please reformat the code ``` - //Emit hidden_hostcall_buffer argument for…
arsenmUnsubmitted
Done
ReplyInline Actions

Space after //

arsenm: Space after //
//since they use hostcall buffer to report invalid memory access.
Func.hasFnAttribute(llvm::Attribute::SanitizeAddress)) {
// The printf runtime binding pass should have ensured that hostcall and // The printf runtime binding pass should have ensured that hostcall and
// printf are not used in the same module. // printf are not used in the same module.
assert(!M->getNamedMetadata("llvm.printf.fmts")); assert(!M->getNamedMetadata("llvm.printf.fmts"));
Context not available.

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

  • This file was added.
; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 --amdhsa-code-object-version=3 -filetype=obj -o - < %s | llvm-readelf --notes - | FileCheck %s
; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 --amdhsa-code-object-version=3 -amdgpu-dump-hsa-metadata -amdgpu-verify-hsa-metadata -filetype=obj -o - < %s 2>&1 | FileCheck --check-prefix=PARSER %s
arsenmUnsubmitted
Done
ReplyInline Actions

You can do this with a single llc invocation.

Also typo "metadate" in the filename

arsenm: You can do this with a single llc invocation. Also typo "metadate" in the filename
; CHECK: ---
; CHECK: amdhsa.kernels:
; CHECK: - .args:
; CHECK-NEXT: - .name: a
; CHECK-NEXT: .offset: 0
; CHECK-NEXT: .size: 1
; CHECK-NEXT: .type_name: char
; CHECK-NEXT: .value_kind: by_value
; CHECK-NEXT: - .offset: 8
; CHECK-NEXT: .size: 8
; CHECK-NEXT: .value_kind: hidden_global_offset_x
; CHECK-NEXT: - .offset: 16
; CHECK-NEXT: .size: 8
; CHECK-NEXT: .value_kind: hidden_global_offset_y
; CHECK-NEXT: - .offset: 24
; CHECK-NEXT: .size: 8
; CHECK-NEXT: .value_kind: hidden_global_offset_z
; CHECK-NEXT: - .address_space: global
; CHECK-NEXT: .offset: 32
; CHECK-NEXT: .size: 8
; CHECK-NEXT: .value_kind: hidden_hostcall_buffer
; CHECK: .language: OpenCL C
; CHECK-NEXT: .language_version:
; CHECK-NEXT: - 2
; CHECK-NEXT: - 0
; CHECK: .name: test_kernel
; CHECK: .symbol: test_kernel.kd
define amdgpu_kernel void @test_kernel(i8 %a) #0
!kernel_arg_addr_space !1 !kernel_arg_access_qual !2 !kernel_arg_type !3
!kernel_arg_base_type !3 !kernel_arg_type_qual !4 {
ret void
}
; CHECK: amdhsa.version:
; CHECK-NEXT: - 1
; CHECK-NEXT: - 0
attributes #0 = { sanitize_address "amdgpu-implicitarg-num-bytes"="48" }
!1 = !{i32 0}
!2 = !{!"none"}
!3 = !{!"char"}
!4 = !{!""}
!opencl.ocl.version = !{!90}
!90 = !{i32 2, i32 0}
; PARSER: AMDGPU HSA Metadata Parser Test: PASS