[clang] ccd8b7b - [LSan] Enable for SystemZ
Author: Ilya Leoshkevich
Date: 2020-06-16T13:45:29+02:00
New Revision: ccd8b7b103470beb79140ecf9d6ccfaddf7fbc11
URL:
https://github.com/llvm/llvm-project/commit/ccd8b7b103470beb79140ecf9d6ccfaddf7fbc11
DIFF:
https://github.com/llvm/llvm-project/commit/ccd8b7b103470beb79140ecf9d6ccfaddf7fbc11.diff
LOG: [LSan] Enable for SystemZ
Summary: Add runtime support, adjust the tests and enable LSan.
Reviewers: vitalybuka, eugenis, uweigand, jonpa
Reviewed By: uweigand
Subscribers: mgorny, cfe-commits, #sanitizers
Tags: #clang, #sanitizers
Differential Revision: https://reviews.llvm.org/D78644
Added:
Modified:
clang/lib/Driver/ToolChains/Linux.cpp
compiler-rt/cmake/config-ix.cmake
compiler-rt/lib/lsan/lsan_allocator.h
compiler-rt/lib/lsan/lsan_common.h
compiler-rt/test/lsan/TestCases/stale_stack_leak.cpp
compiler-rt/test/lsan/TestCases/use_registers.cpp
compiler-rt/test/lsan/lit.common.cfg.py
compiler-rt/test/sanitizer_common/print_address.h
Removed:
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp
b/clang/lib/Driver/ToolChains/Linux.cpp
index 7df49c787c8e..222c351016a7 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -840,6 +840,7 @@ SanitizerMask Linux::getSupportedSanitizers() const {
getTriple().getArch() == llvm::Triple::thumb ||
getTriple().getArch() == llvm::Triple::armeb ||
getTriple().getArch() == llvm::Triple::thumbeb;
+ const bool IsSystemZ = getTriple().getArch() == llvm::Triple::systemz;
SanitizerMask Res = ToolChain::getSupportedSanitizers();
Res |= SanitizerKind::Address;
Res |= SanitizerKind::PointerCompare;
@@ -852,7 +853,8 @@ SanitizerMask Linux::getSupportedSanitizers() const {
Res |= SanitizerKind::SafeStack;
if (IsX86_64 || IsMIPS64 || IsAArch64)
Res |= SanitizerKind::DataFlow;
- if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsArmArch || IsPowerPC64)
+ if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsArmArch || IsPowerPC64 ||
+ IsSystemZ)
Res |= SanitizerKind::Leak;
if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64)
Res |= SanitizerKind::Thread;
diff --git a/compiler-rt/cmake/config-ix.cmake
b/compiler-rt/cmake/config-ix.cmake
index ef62d701dee2..1f3697ff6f65 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -303,7 +303,7 @@ set(ALL_GWP_ASAN_SUPPORTED_ARCH ${X86} ${X86_64})
if(APPLE)
set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64})
else()
- set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64} ${ARM32}
${PPC64})
+ set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64} ${ARM32}
${PPC64} ${S390X})
endif()
set(ALL_MSAN_SUPPORTED_ARCH ${X86_64} ${MIPS64} ${ARM64} ${PPC64} ${S390X})
set(ALL_HWASAN_SUPPORTED_ARCH ${X86_64} ${ARM64})
diff --git a/compiler-rt/lib/lsan/lsan_allocator.h
b/compiler-rt/lib/lsan/lsan_allocator.h
index bda9d8cdf746..17e13cd014ba 100644
--- a/compiler-rt/lib/lsan/lsan_allocator.h
+++ b/compiler-rt/lib/lsan/lsan_allocator.h
@@ -65,13 +65,16 @@ struct AP32 {
template
using PrimaryAllocatorASVT = SizeClassAllocator32>;
using PrimaryAllocator = PrimaryAllocatorASVT;
-#elif defined(__x86_64__) || defined(__powerpc64__)
+#elif defined(__x86_64__) || defined(__powerpc64__) || defined(__s390x__)
# if SANITIZER_FUCHSIA
const uptr kAllocatorSpace = ~(uptr)0;
const uptr kAllocatorSize = 0x400ULL; // 4T.
# elif defined(__powerpc64__)
const uptr kAllocatorSpace = 0xa00ULL;
const uptr kAllocatorSize = 0x200ULL; // 2T.
+#elif defined(__s390x__)
+const uptr kAllocatorSpace = 0x400ULL;
+const uptr kAllocatorSize = 0x400ULL; // 4T.
# else
const uptr kAllocatorSpace = 0x6000ULL;
const uptr kAllocatorSize = 0x400ULL; // 4T.
diff --git a/compiler-rt/lib/lsan/lsan_common.h
b/compiler-rt/lib/lsan/lsan_common.h
index 6252d52c1978..3434beede828 100644
--- a/compiler-rt/lib/lsan/lsan_common.h
+++ b/compiler-rt/lib/lsan/lsan_common.h
@@ -29,10 +29,10 @@
// To enable LeakSanitizer on a new architecture, one needs to implement the
// internal_clone function as well as (probably) adjust the TLS machinery for
// the new architecture inside the sanitizer library.
-#if (SANITIZER_LINUX && !SANITIZER_ANDROID || SANITIZER_MAC) && \
-(SANITIZER_WORDSIZE == 64) && \
+#if (SANITIZER_LINUX && !SANITIZER_ANDROID || SANITIZER_MAC) && \
+(SANITIZER_WORDSIZE == 64) &&\
(defined(__x86_64__) || defined(__mips64) || defined(__aarch64__) || \
- defined(__powerpc64__))
+ defined(__powerpc64__) || defined(__s390x__))
#define CAN_SANITIZE_LEAKS 1
#elif defined(__i386__) && \
(SANITIZER_LINUX && !SANITIZER_ANDROID || SANITIZER_MAC)
[llvm] [clang] [SystemZ] Add backchain target-feature (PR #71668)
https://github.com/iii-i created https://github.com/llvm/llvm-project/pull/71668
GCC supports building individual functions with backchain using the
__attribute__((target("backchain"))) syntax, and Clang should too.
Clang translates this into the "target-features"="+backchain" attribute, and
the -mbackchain command-line option into the "backchain" attribute. The backend
currently checks only the latter; furthermore, the backchain target feature is
not defined.
Handle backchain like soft-float. Define a target feature, convert function
attribute into it in getSubtargetImpl(), and check for target feature instead
of function attribute everywhere. Add an end-to-end test to the Clang testsuite.
>From 20c1609d4bf38b811ced90b43153912a834ea7b0 Mon Sep 17 00:00:00 2001
From: Ilya Leoshkevich
Date: Tue, 7 Nov 2023 17:18:03 +0100
Subject: [PATCH] [SystemZ] Add backchain target-feature
GCC supports building individual functions with backchain using the
__attribute__((target("backchain"))) syntax, and Clang should too.
Clang translates this into the "target-features"="+backchain"
attribute, and the -mbackchain command-line option into the "backchain"
attribute. The backend currently checks only the latter; furthermore,
the backchain target feature is not defined.
Handle backchain like soft-float. Define a target feature, convert
function attribute into it in getSubtargetImpl(), and check for target
feature instead of function attribute everywhere. Add an end-to-end
test to the Clang testsuite.
---
clang/test/CodeGen/SystemZ/mbackchain-4.c | 11 ++
llvm/lib/Target/SystemZ/SystemZFeatures.td| 5 +
.../Target/SystemZ/SystemZFrameLowering.cpp | 20 ++-
.../Target/SystemZ/SystemZISelLowering.cpp| 8
.../Target/SystemZ/SystemZTargetMachine.cpp | 8 +---
5 files changed, 36 insertions(+), 16 deletions(-)
create mode 100644 clang/test/CodeGen/SystemZ/mbackchain-4.c
diff --git a/clang/test/CodeGen/SystemZ/mbackchain-4.c
b/clang/test/CodeGen/SystemZ/mbackchain-4.c
new file mode 100644
index 000..6e5f4fc5da40084
--- /dev/null
+++ b/clang/test/CodeGen/SystemZ/mbackchain-4.c
@@ -0,0 +1,11 @@
+// RUN: %clang --target=s390x-linux -O1 -S -o - %s | FileCheck %s
+
+__attribute__((target("backchain")))
+void *foo(void) {
+ return __builtin_return_address(1);
+}
+
+// CHECK-LABEL: foo:
+// CHECK: lg %r1, 0(%r15)
+// CHECK: lg %r2, 112(%r1)
+// CHECK: br %r14
diff --git a/llvm/lib/Target/SystemZ/SystemZFeatures.td
b/llvm/lib/Target/SystemZ/SystemZFeatures.td
index 78b8394d6486522..fdd94206421a418 100644
--- a/llvm/lib/Target/SystemZ/SystemZFeatures.td
+++ b/llvm/lib/Target/SystemZ/SystemZFeatures.td
@@ -32,6 +32,11 @@ def FeatureSoftFloat : SystemZFeature<
"Use software emulation for floating point"
>;
+def FeatureBackChain : SystemZFeature<
+ "backchain", "BackChain", (all_of FeatureBackChain),
+ "Store the address of the caller's frame into the callee's stack frame"
+>;
+
//===--===//
//
// New features added in the Ninth Edition of the z/Architecture
diff --git a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
index bfd31709eb3e0bc..7522998fd06d8e5 100644
--- a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
@@ -443,7 +443,7 @@ void
SystemZELFFrameLowering::processFunctionBeforeFrameFinalized(
MachineFrameInfo &MFFrame = MF.getFrameInfo();
SystemZMachineFunctionInfo *ZFI = MF.getInfo();
MachineRegisterInfo *MRI = &MF.getRegInfo();
- bool BackChain = MF.getFunction().hasFnAttribute("backchain");
+ bool BackChain = MF.getSubtarget().hasBackChain();
if (!usePackedStack(MF) || BackChain)
// Create the incoming register save area.
@@ -628,7 +628,7 @@ void SystemZELFFrameLowering::emitPrologue(MachineFunction
&MF,
.addImm(StackSize);
}
else {
- bool StoreBackchain = MF.getFunction().hasFnAttribute("backchain");
+ bool StoreBackchain = MF.getSubtarget().hasBackChain();
// If we need backchain, save current stack pointer. R1 is free at
// this point.
if (StoreBackchain)
@@ -786,7 +786,7 @@ void SystemZELFFrameLowering::inlineStackProbe(
.addMemOperand(MMO);
};
- bool StoreBackchain = MF.getFunction().hasFnAttribute("backchain");
+ bool StoreBackchain = MF.getSubtarget().hasBackChain();
if (StoreBackchain)
BuildMI(*MBB, MBBI, DL, ZII->get(SystemZ::LGR))
.addReg(SystemZ::R1D, RegState::Define).addReg(SystemZ::R15D);
@@ -861,8 +861,9 @@ StackOffset SystemZELFFrameLowering::getFrameIndexReference(
unsigned SystemZELFFrameLowering::getRegSpillOffset(MachineFunction &MF,
Register Reg) const {
bool IsVarArg = MF.getFunction().isVarArg();
- bool BackChain = MF.getFunction().hasFnAttribute("backchain");
- bool SoftFl
[llvm] [clang] [SystemZ] Add backchain target-feature (PR #71668)
https://github.com/iii-i closed https://github.com/llvm/llvm-project/pull/71668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SystemZ] Add backchain target-feature (PR #71668)
iii-i wrote: This is now causing a CI failure: https://lab.llvm.org/buildbot/#/builders/139/builds/53143 I think I need something like `// REQUIRES: s390x-registered-target` in the new test. I will open a PR with the fix soon. https://github.com/llvm/llvm-project/pull/71668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SystemZ] Do not run mbackchain-4.c test without SystemZ target (PR #71678)
https://github.com/iii-i created https://github.com/llvm/llvm-project/pull/71678
mbackchain-4.c requires running the backend and causes CI failures when this
target is not configured.
>From 284f2d4f844a03ece42eb6dbe7c0d889cf1b01e4 Mon Sep 17 00:00:00 2001
From: Ilya Leoshkevich
Date: Wed, 8 Nov 2023 15:16:51 +0100
Subject: [PATCH] [SystemZ] Do not run mbackchain-4.c test without SystemZ
target
mbackchain-4.c requires running the backend and causes CI failures when
this target is not configured.
---
clang/test/CodeGen/SystemZ/mbackchain-4.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/test/CodeGen/SystemZ/mbackchain-4.c
b/clang/test/CodeGen/SystemZ/mbackchain-4.c
index 6e5f4fc5da40084..43f25137fffa51c 100644
--- a/clang/test/CodeGen/SystemZ/mbackchain-4.c
+++ b/clang/test/CodeGen/SystemZ/mbackchain-4.c
@@ -1,3 +1,4 @@
+// REQUIRES: systemz-registered-target
// RUN: %clang --target=s390x-linux -O1 -S -o - %s | FileCheck %s
__attribute__((target("backchain")))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SystemZ] Do not run mbackchain-4.c test without SystemZ target (PR #71678)
https://github.com/iii-i closed https://github.com/llvm/llvm-project/pull/71678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e34078f - [TSan] Enable SystemZ support
Author: Ilya Leoshkevich
Date: 2021-07-15T12:18:48+02:00
New Revision: e34078f121a58b503d225cf715d1494117e7948b
URL:
https://github.com/llvm/llvm-project/commit/e34078f121a58b503d225cf715d1494117e7948b
DIFF:
https://github.com/llvm/llvm-project/commit/e34078f121a58b503d225cf715d1494117e7948b.diff
LOG: [TSan] Enable SystemZ support
Enable building the runtime and enable -fsanitize=thread in clang.
Reviewed By: dvyukov
Differential Revision: https://reviews.llvm.org/D105629
Added:
Modified:
clang/lib/Driver/ToolChains/Linux.cpp
compiler-rt/cmake/config-ix.cmake
Removed:
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp
b/clang/lib/Driver/ToolChains/Linux.cpp
index 886e0b35ece8..c9360fc67165 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -709,7 +709,7 @@ SanitizerMask Linux::getSupportedSanitizers() const {
if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsArmArch || IsPowerPC64 ||
IsRISCV64 || IsSystemZ)
Res |= SanitizerKind::Leak;
- if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64)
+ if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64 || IsSystemZ)
Res |= SanitizerKind::Thread;
if (IsX86_64)
Res |= SanitizerKind::KernelMemory;
diff --git a/compiler-rt/cmake/config-ix.cmake
b/compiler-rt/cmake/config-ix.cmake
index 9b27631f4af9..6f13acfa2757 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -329,7 +329,7 @@ set(ALL_HWASAN_SUPPORTED_ARCH ${X86_64} ${ARM64})
set(ALL_MEMPROF_SUPPORTED_ARCH ${X86_64})
set(ALL_PROFILE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${PPC32}
${PPC64}
${MIPS32} ${MIPS64} ${S390X} ${SPARC} ${SPARCV9})
-set(ALL_TSAN_SUPPORTED_ARCH ${X86_64} ${MIPS64} ${ARM64} ${PPC64})
+set(ALL_TSAN_SUPPORTED_ARCH ${X86_64} ${MIPS64} ${ARM64} ${PPC64} ${S390X})
set(ALL_UBSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${RISCV64}
${MIPS32} ${MIPS64} ${PPC64} ${S390X} ${SPARC} ${SPARCV9})
set(ALL_SAFESTACK_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM64} ${MIPS32} ${MIPS64})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen] Use byval for SystemZ indirect arguments (PR #66404)
https://github.com/iii-i created
https://github.com/llvm/llvm-project/pull/66404:
SystemZ ABI mandates that in order to pass large objects by value, one should
be place them on stack and pass the callee a pointer. Currently on LLVM IR
level it's impossible to distinguish whether a pointer argument was there in
the C code, or whether it was introduced in order to satisfy the ABI
requirement.
MSan and DFSan need to know that in order to correctly fill the arguments'
shadow TLS. Fix by adding ByVal attributes to synthetic pointer arguments. This
attribute serves exactly this purpose.
This resolves an outstanding MSan issue, and enables the future DFSan support.
>From 02dce697b79d01321f9db37ee7b048b4e7879341 Mon Sep 17 00:00:00 2001
From: Ilya Leoshkevich
Date: Thu, 14 Sep 2023 14:51:58 +0200
Subject: [PATCH] [clang][CodeGen] Use byval for SystemZ indirect arguments
SystemZ ABI mandates that in order to pass large objects by value, one
should be place them on stack and pass the callee a pointer. Currently
on LLVM IR level it's impossible to distinguish whether a pointer
argument was there in the C code, or whether it was introduced in order
to satisfy the ABI requirement.
MSan and DFSan need to know that in order to correctly fill the
arguments' shadow TLS. Fix by adding ByVal attributes to synthetic
pointer arguments. This attribute serves exactly this purpose.
This resolves an outstanding MSan issue, and enables the future DFSan
support.
---
clang/lib/CodeGen/Targets/SystemZ.cpp | 6 +--
.../test/CodeGen/SystemZ/systemz-abi-vector.c | 52 +--
clang/test/CodeGen/SystemZ/systemz-abi.c | 34 ++--
.../test/CodeGen/SystemZ/systemz-inline-asm.c | 2 +-
clang/test/CodeGen/ext-int-cc.c | 4 +-
compiler-rt/test/msan/param_tls_limit.cpp | 6 ---
6 files changed, 49 insertions(+), 55 deletions(-)
diff --git a/clang/lib/CodeGen/Targets/SystemZ.cpp
b/clang/lib/CodeGen/Targets/SystemZ.cpp
index 6eb0c6ef2f7d637..3a0177cfc73446c 100644
--- a/clang/lib/CodeGen/Targets/SystemZ.cpp
+++ b/clang/lib/CodeGen/Targets/SystemZ.cpp
@@ -432,7 +432,7 @@ ABIArgInfo SystemZABIInfo::classifyArgumentType(QualType
Ty) const {
// Values that are not 1, 2, 4 or 8 bytes in size are passed indirectly.
if (Size != 8 && Size != 16 && Size != 32 && Size != 64)
-return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
+return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
// Handle small structures.
if (const RecordType *RT = Ty->getAs()) {
@@ -440,7 +440,7 @@ ABIArgInfo SystemZABIInfo::classifyArgumentType(QualType
Ty) const {
// fail the size test above.
const RecordDecl *RD = RT->getDecl();
if (RD->hasFlexibleArrayMember())
- return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
+ return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
// The structure is passed as an unextended integer, a float, or a double.
llvm::Type *PassTy;
@@ -457,7 +457,7 @@ ABIArgInfo SystemZABIInfo::classifyArgumentType(QualType
Ty) const {
// Non-structure compounds are passed indirectly.
if (isCompoundType(Ty))
-return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
+return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
return ABIArgInfo::getDirect(nullptr);
}
diff --git a/clang/test/CodeGen/SystemZ/systemz-abi-vector.c
b/clang/test/CodeGen/SystemZ/systemz-abi-vector.c
index f7606641a3747e5..764d3f04140b694 100644
--- a/clang/test/CodeGen/SystemZ/systemz-abi-vector.c
+++ b/clang/test/CodeGen/SystemZ/systemz-abi-vector.c
@@ -54,91 +54,91 @@ unsigned int align = __alignof__ (v16i8);
// CHECK-VECTOR: @align ={{.*}} global i32 8
v1i8 pass_v1i8(v1i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1i8(ptr noalias sret(<1 x i8>) align
1 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1i8(ptr noalias sret(<1 x i8>) align
1 %{{.*}}, ptr byval(<1 x i8>) align 1 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x i8> @pass_v1i8(<1 x i8> %{{.*}})
v2i8 pass_v2i8(v2i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v2i8(ptr noalias sret(<2 x i8>) align
2 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v2i8(ptr noalias sret(<2 x i8>) align
2 %{{.*}}, ptr byval(<2 x i8>) align 2 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <2 x i8> @pass_v2i8(<2 x i8> %{{.*}})
v4i8 pass_v4i8(v4i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v4i8(ptr noalias sret(<4 x i8>) align
4 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v4i8(ptr noalias sret(<4 x i8>) align
4 %{{.*}}, ptr byval(<4 x i8>) align 4 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <4 x i8> @pass_v4i8(<4 x i8> %{{.*}})
v8i8 pass_v8i8(v8i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v8i8(ptr noalias sret(<8 x i8>) align
8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v8i8(ptr noalias sret(<8 x i8>) align
8 %{{.*}}, ptr byval(<8 x i8>) align 8 %0)
// CHECK-VECTOR-LABEL: define{
[clang] [clang][CodeGen] Use byval for SystemZ indirect arguments (PR #66404)
https://github.com/iii-i review_requested https://github.com/llvm/llvm-project/pull/66404 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen] Use byval for SystemZ indirect arguments (PR #66404)
iii-i wrote:
I just checked with a few small examples, and while the ABI does not seem to
change, the code generation looks broken for tail recursion:
```
$ cat 1.c
void baz(long double);
void quux(void) { baz((long double)1); }
$ ./bin/clang -O3 -S 1.c
$ cat 1.s
[...]
aghi%r15, -176 # stack frame
larl%r1, .LCPI0_0 # address of (long double)1 in the literal pool
lxeb%f0, 0(%r1) # f0:f2=(long double)1
la %r2, 160(%r15) # r2=&var_160
std %f0, 160(%r15) # var_160=(long double)1
std %f2, 168(%r15)
aghi%r15, 176 # var_160 is above the stack pointer now!
jg baz@PLT # baz(&var_160)
[...]
```
Regarding MSan/DFSan, they need to know whether to copy value's or pointer's
shadow into arguments' TLS. MSan already has checks for that
(`CB.paramHasAttr(ArgNo, Attribute::ByVal)`), and for DFSan I've added them in
my local branch.
https://github.com/llvm/llvm-project/pull/66404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen] Use byval for SystemZ indirect arguments (PR #66404)
iii-i wrote:
There is special byval handling in MSan:
```
void visitCallBase(CallBase &CB, IRBuilder<> &IRB) override {
...
for (const auto &[ArgNo, A] : llvm::enumerate(CB.args())) {
...
bool IsByVal = CB.paramHasAttr(ArgNo, Attribute::ByVal);
```
If a target does not make use of byval, I would argue that this part of MSan is
broken there.
Unfortunately this applies to SystemZ at the moment: `param_tls_limit.cpp` is
marked as XFAIL because of this (the rationale that is written there, which
says that the test is not applicable, is not correct).
A quick experiment shows that x86_64 uses byval:
```
$ uname -m
x86_64
$ clang --version
clang version 16.0.6 (Fedora 16.0.6-2.fc38)
$ cat 1.c
struct foo { char x[800]; };
void bar(struct foo) {};
$ clang -mllvm -print-after-all -c 1.c
[...]
define dso_local void @bar(ptr noundef byval(%struct.foo) align 8 %0) #0 {
[...]
```
As for the reasons why this all is the case, I think this is because of how
arguments' shadow TLS is defined: it contains shadows of all argument values,
where arguments are understood in terms of C, and not in terms of the ABI. This
definition is then used in the runtime. See, for example, the handling of long
doubles in `dfsan_custom.cpp`:
```
static int format_buffer(char *str, size_t size, const char *fmt,
dfsan_label *va_labels, dfsan_label *ret_label,
dfsan_origin *va_origins, dfsan_origin *ret_origin,
va_list ap) {
[...]
case 'G':
if (*(formatter.fmt_cur - 1) == 'L') {
retval = formatter.format(va_arg(ap, long double));
} else {
retval = formatter.format(va_arg(ap, double));
}
if (va_origins == nullptr)
dfsan_set_label(*va_labels++, formatter.str_cur(),
formatter.num_written_bytes(retval));
else
dfsan_set_label_origin(*va_labels++, *va_origins++,
formatter.str_cur(),
formatter.num_written_bytes(retval));
end_fmt = true;
break;
```
Here is wants the shadow of the long double itself, even if the target ABI
requires passing it through a pointer.
https://github.com/llvm/llvm-project/pull/66404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen] Use byval for SystemZ indirect arguments (PR #66404)
iii-i wrote: I don't quite get why can't we use byval here. How is this different from x86_64? Both s390x and x86_64 ABIs require passing large structs via a pointer, why can x86_64 implement this using byval in LLVM and s390x can't? I agree that currently s390x backend does not handle it properly and the current version of this PR is not suitable for inclusion, but what is the conceptual problem here? https://github.com/llvm/llvm-project/pull/66404 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen] Use byval for SystemZ indirect arguments (PR #66404)
iii-i wrote:
Sorry, I my wording was not precise enough, it is indeed important that we
create a copy, and not pass a pointer to the original. Still, what you
described matches the s390x ABI:
```
1.2.2.3. Parameter Area
The parameter area shall be allocated by a calling function if some parameters
cannot
be passed in registers, but must be passed on the stack instead (see section
1.2.3).
[...]
1.2.3. Parameter Passing
[...]
A struct or union of any other size, a complex type, an __int128, a long
double, a _Decimal128, or a vector whose size exceeds 16 bytes. Replace
such an argument by a pointer to the object, or to a copy where necessary
to enforce call-by-value semantics. Only if the caller can ascertain that the
object is “constant” can it pass a pointer to the object itself.
```
---
Ah, that's the source of my confusion. I didn't realize the call instruction
had to make a copy, I thought it just had to be done somewhere. "The attribute
implies that a hidden copy of the pointee is made between the caller and the
callee" actually does mean the former, but one has to squint to see that. The
way you phrased it is much clearer.
So in the following example:
```
struct foo { char x[800]; };
void bar(struct foo);
void baz(void) { struct foo f = {}; bar(f); };
```
x84_64 generates:
```
define dso_local void @baz() #0 {
%1 = alloca %struct.foo, align 8
call void @llvm.memset.p0.i64(ptr align 1 %1, i8 0, i64 800, i1 false)
call void @bar(ptr noundef byval(%struct.foo) align 8 %1)
ret void
}
```
and relies on the backend to expand `call void @bar` into roughly
`REP_MOVSQ_64` and `CALL64pcrel32`. Whereas on s390x we get:
```
define dso_local void @baz() #0 {
%1 = alloca %struct.foo, align 1
%2 = alloca %struct.foo, align 1
call void @llvm.memset.p0.i64(ptr align 1 %1, i8 0, i64 800, i1 false)
call void @llvm.memcpy.p0.p0.i64(ptr align 1 %2, ptr align 1 %1, i64 800, i1
false)
call void @bar(ptr noundef %2)
ret void
}
```
so the creation of the copy is explicit in the LLVM IR. Even though the ABIs
are saying roughly the same thing, it's implemented differently.
I wonder if it would still be beneficial to switch s390x to byval? I think it
can be done in a way that correctly implements the ABI, even though it would of
course be more complex than this PR. An obvious benefit is that s390x would
become more similar to x86_64, but maybe there are some drawbacks that I'm not
seeing.
---
I revisited MSan's `param_tls_limit.cpp`, and the XFAIL is actually fine. The
instrumentation does indeed put the shadow of the synthetic pointer into the
parameters' TLS area on s390x, but this is not a problem, since the shadow of
the actual value is still preserved and checked. This prevents the overflow,
which the test expects, from happening, so the conclusion that the test is not
applicable is correct. Sorry for the noise.
I will check if there is a different solution for DFSan. It currently passes
the label of the pointer to the copy, which is always 0, instead of the label
of the actual value, to vararg functions on s390x. Even though this is similar
to what MSan does, the difference is that the DFSan runtime (e.g.,
`format_buffer`) expects the label of the actual value, regardless of the ABI.
https://github.com/llvm/llvm-project/pull/66404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a3e56a8 - [KMSAN] Enable on SystemZ
Author: Ilya Leoshkevich
Date: 2023-04-27T13:44:54+02:00
New Revision: a3e56a8792ffaf3a3d3538736e1042b8db45ab89
URL:
https://github.com/llvm/llvm-project/commit/a3e56a8792ffaf3a3d3538736e1042b8db45ab89
DIFF:
https://github.com/llvm/llvm-project/commit/a3e56a8792ffaf3a3d3538736e1042b8db45ab89.diff
LOG: [KMSAN] Enable on SystemZ
Enable -fsanitize=kernel-memory support in Clang.
The x86_64 ABI requires that shadow_origin_ptr_t must be returned via a
register pair, and the s390x ABI requires that it must be returned via
memory pointed to by a hidden parameter. Normally Clang takes care of
the ABI, but the sanitizers run long after it, so unfortunately they
have to duplicate the ABI logic.
Therefore add a special case for SystemZ and manually emit the
s390x-ABI-compliant calling sequences. Since it's only 2 architectures,
do not create a VarArgHelper-like abstraction layer.
The kernel functions are compiled with the "packed-stack" and
"use-soft-float" attributes. For the "packed-stack" functions, it's not
correct for copyRegSaveArea() to copy 160 bytes of shadow and origins,
since the save area is dynamically sized. Things are greatly simplified
by the fact that the vararg "use-soft-float" functions use precisely
56 bytes in order to save the argument registers to where va_arg() can
find them.
Make copyRegSaveArea() copy only 56 bytes in the "use-soft-float" case.
The "packed-stack" && !"use-soft-float" case has no practical uses at
the moment, so leave it for the future.
Add tests.
Reviewed By: eugenis
Differential Revision: https://reviews.llvm.org/D148596
Added:
llvm/test/Instrumentation/MemorySanitizer/SystemZ/basic-kernel.ll
Modified:
clang/lib/Driver/ToolChains/Linux.cpp
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
llvm/test/Instrumentation/MemorySanitizer/SystemZ/vararg-kernel.ll
Removed:
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp
b/clang/lib/Driver/ToolChains/Linux.cpp
index 1fbd26aed596d..5b1922970ce2b 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -795,7 +795,7 @@ SanitizerMask Linux::getSupportedSanitizers() const {
if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64 || IsSystemZ ||
IsLoongArch64)
Res |= SanitizerKind::Thread;
- if (IsX86_64)
+ if (IsX86_64 || IsSystemZ)
Res |= SanitizerKind::KernelMemory;
if (IsX86 || IsX86_64)
Res |= SanitizerKind::Function;
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 93667c9482921..e2269c2a8638d 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -122,6 +122,10 @@
///Arbitrary sized accesses are handled with:
/// __msan_metadata_ptr_for_load_n(ptr, size)
/// __msan_metadata_ptr_for_store_n(ptr, size);
+///Note that the sanitizer code has to deal with how shadow/origin pairs
+///returned by the these functions are represented in
diff erent ABIs. In
+///the X86_64 ABI they are returned in RDX:RAX, and in the SystemZ ABI they
+///are written to memory pointed to by a hidden parameter.
/// - TLS variables are stored in a single per-task struct. A call to a
///function __msan_get_context_state() returning a pointer to that struct
///is inserted into every instrumented function before the entry block;
@@ -135,7 +139,7 @@
/// Also, KMSAN currently ignores uninitialized memory passed into inline asm
/// calls, making sure we're on the safe side wrt. possible false positives.
///
-/// KernelMemorySanitizer only supports X86_64 at the moment.
+/// KernelMemorySanitizer only supports X86_64 and SystemZ at the moment.
///
//
// FIXME: This sanitizer does not yet handle scalable vectors
@@ -543,6 +547,10 @@ class MemorySanitizer {
void createKernelApi(Module &M, const TargetLibraryInfo &TLI);
void createUserspaceApi(Module &M, const TargetLibraryInfo &TLI);
+ template
+ FunctionCallee getOrInsertMsanMetadataFunction(Module &M, StringRef Name,
+ ArgsTy... Args);
+
/// True if we're compiling the Linux kernel.
bool CompileKernel;
/// Track origins (allocation points) of uninitialized values.
@@ -550,6 +558,7 @@ class MemorySanitizer {
bool Recover;
bool EagerChecks;
+ Triple TargetTriple;
LLVMContext *C;
Type *IntptrTy;
Type *OriginTy;
@@ -620,13 +629,18 @@ class MemorySanitizer {
/// Functions for poisoning/unpoisoning local variables
FunctionCallee MsanPoisonAllocaFn, MsanUnpoisonAllocaFn;
- /// Each of the MsanMetadataPtrXxx functions returns a pair of shadow/origin
- /// pointers.
+ /// Pair of shadow/origin pointers.
+ Type *MsanMetadata;
+
+ /// Each of the MsanMetadataPtrXxx functions returns a MsanMetadata.
FunctionCallee Msa
[clang] [llvm] Revert changes in AddDefaultGCCPrefixes() for SystemZTriples. (PR #94729)
iii-i wrote: Looking at the code, this is because `Generic_GCC::GCCInstallationDetector::init()` will not try to strip any vendor except `unknown`. This will in turn cause `s390x-ibm-linux-gnu` to not detect GCC installation with prefix `s390x-linux-gnu`, which Debian and Ubuntu have. On the other hand, precisely for the same reason, `s390x-unknown-linux-gnu` works. Therefore I propose in #95407 to use that. As to why the code written this way now, I don't have a good explanation, but, according to the comments in this PR, this looks intentional. https://github.com/llvm/llvm-project/pull/94729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Revert changes in AddDefaultGCCPrefixes() for SystemZTriples. (PR #94729)
iii-i wrote: This is still failing for me on Debian. The problem seems to be that `/etc/lsb-release` does not exist. There are ``` # cat /etc/os-release PRETTY_NAME="Debian GNU/Linux bookworm/sid" NAME="Debian GNU/Linux" VERSION_CODENAME=bookworm ID=debian HOME_URL="https://www.debian.org/"; SUPPORT_URL="https://www.debian.org/support"; BUG_REPORT_URL="https://bugs.debian.org/"; ``` and ``` # lsb_release -a No LSB modules are available. Distributor ID: Debian Description:Debian GNU/Linux bookworm/sid Release:n/a Codename: bookworm ``` though. Would it make sense to include both of them in the check? https://github.com/llvm/llvm-project/pull/94729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
