[PATCH] D132950: Remove unnecessary `REQUIRES: x86-registered-target` from ps4/ps5 driver tests.
MaggieYi created this revision. MaggieYi added reviewers: probinson, wristow. Herald added a subscriber: pengfei. Herald added a project: All. MaggieYi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Some PS4/PS45 tests include the line `REQUIRES: x86-registered-target` meaning they depend on having the X86 backend included in the build. This should not be necessary since the driver understands all triples regardless of which backends are included. The change removes all unnecessary `REQUIRES: x86-registered-target` from ps4/ps5 driver tests. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D132950 Files: clang/test/Driver/ps4-pic.c clang/test/Driver/ps4-ps5-header-search.c clang/test/Driver/ps4-ps5-linker-non-win.c clang/test/Driver/ps4-ps5-linker-win.c clang/test/Driver/ps4-ps5-relax-relocations.c clang/test/Driver/ps4-ps5-runtime-flags.c clang/test/Driver/ps4-sdk-root.c clang/test/Driver/ps4ps5base.c clang/test/Driver/ps5-pic.c clang/test/Driver/ps5-sdk-root.c Index: clang/test/Driver/ps5-sdk-root.c === --- clang/test/Driver/ps5-sdk-root.c +++ clang/test/Driver/ps5-sdk-root.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - /// (Essentially identical to ps4-sdk-root.c except for the target.) /// Check that PS5 clang doesn't report a warning message when locating Index: clang/test/Driver/ps5-pic.c === --- clang/test/Driver/ps5-pic.c +++ clang/test/Driver/ps5-pic.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // Test the driver's control over the PIC behavior for PS5 compiler. // These consist of tests of the relocation model flags and the // pic level flags passed to CC1. Index: clang/test/Driver/ps4ps5base.c === --- clang/test/Driver/ps4ps5base.c +++ clang/test/Driver/ps4ps5base.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // Test that the driver always emits -fno-use-init-array on the PS4/PS5 targets // since their ABI does not support the .init_array section. Index: clang/test/Driver/ps4-sdk-root.c === --- clang/test/Driver/ps4-sdk-root.c +++ clang/test/Driver/ps4-sdk-root.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // Check that PS4 clang doesn't report a warning message when locating // system header files (either by looking at the value of SCE_ORBIS_SDK_DIR // or relative to the location of the compiler driver), if "-nostdinc", Index: clang/test/Driver/ps4-ps5-runtime-flags.c === --- clang/test/Driver/ps4-ps5-runtime-flags.c +++ clang/test/Driver/ps4-ps5-runtime-flags.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target -// /// Test the profile runtime library to be linked for PS4/PS5 compiler. /// Check runtime flag --dependent-lib which does not append the default library search path. // Index: clang/test/Driver/ps4-ps5-relax-relocations.c === --- clang/test/Driver/ps4-ps5-relax-relocations.c +++ clang/test/Driver/ps4-ps5-relax-relocations.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // RUN: %clang -### -target x86_64-scei-ps4 %s -o - 2>&1 | \ // RUN: FileCheck %s // RUN: %clang -### -target x86_64-scei-ps4 -Wa,-mrelax-relocations=yes %s -o - 2>&1 | \ Index: clang/test/Driver/ps4-ps5-linker-win.c === --- clang/test/Driver/ps4-ps5-linker-win.c +++ clang/test/Driver/ps4-ps5-linker-win.c @@ -1,7 +1,7 @@ // This test checks that orbis-ld is used for PS4 linker all the time, and // prospero-lld is used for PS5 linker. Specifying -fuse-ld causes an error. -// REQUIRES: system-windows, x86-registered-target +// REQUIRES: system-windows // RUN: mkdir -p %t // RUN: touch %t/orbis-ld.exe Index: clang/test/Driver/ps4-ps5-linker-non-win.c === --- clang/test/Driver/ps4-ps5-linker-non-win.c +++ clang/test/Driver/ps4-ps5-linker-non-win.c @@ -1,6 +1,5 @@ /// Checks proper linker prefixing for PS4 and PS5. // UNSUPPORTED: system-windows -// REQUIRES: x86-registered-target // RUN: mkdir -p %t // RUN: rm -f %t/orbis-ld Index: clang/test/Driver/ps4-ps5-header-search.c === --- clang/test/Driver/ps4-ps5-header-search.c +++ clang/test/Driver/ps4-ps5-header-search.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - /// PS4 and PS5 use the same SDK layout, so use the same tree for both. // RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target x86_64-scei-ps4 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4 // RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/sc
[PATCH] D132950: Remove unnecessary `REQUIRES: x86-registered-target` from ps4/ps5 driver tests.
MaggieYi added a comment. Thanks Paul. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132950/new/ https://reviews.llvm.org/D132950 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D132950: Remove unnecessary `REQUIRES: x86-registered-target` from ps4/ps5 driver tests.
This revision was automatically updated to reflect the committed changes. Closed by commit rG0e5fe1cdacdc: Remove `REQUIRES: x86-registered-target` from ps4/ps5 driver tests (authored by MaggieYi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132950/new/ https://reviews.llvm.org/D132950 Files: clang/test/Driver/ps4-pic.c clang/test/Driver/ps4-ps5-header-search.c clang/test/Driver/ps4-ps5-linker-non-win.c clang/test/Driver/ps4-ps5-linker-win.c clang/test/Driver/ps4-ps5-relax-relocations.c clang/test/Driver/ps4-ps5-runtime-flags.c clang/test/Driver/ps4-sdk-root.c clang/test/Driver/ps4ps5base.c clang/test/Driver/ps5-pic.c clang/test/Driver/ps5-sdk-root.c Index: clang/test/Driver/ps5-sdk-root.c === --- clang/test/Driver/ps5-sdk-root.c +++ clang/test/Driver/ps5-sdk-root.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - /// (Essentially identical to ps4-sdk-root.c except for the target.) /// Check that PS5 clang doesn't report a warning message when locating Index: clang/test/Driver/ps5-pic.c === --- clang/test/Driver/ps5-pic.c +++ clang/test/Driver/ps5-pic.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // Test the driver's control over the PIC behavior for PS5 compiler. // These consist of tests of the relocation model flags and the // pic level flags passed to CC1. Index: clang/test/Driver/ps4ps5base.c === --- clang/test/Driver/ps4ps5base.c +++ clang/test/Driver/ps4ps5base.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // Test that the driver always emits -fno-use-init-array on the PS4/PS5 targets // since their ABI does not support the .init_array section. Index: clang/test/Driver/ps4-sdk-root.c === --- clang/test/Driver/ps4-sdk-root.c +++ clang/test/Driver/ps4-sdk-root.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // Check that PS4 clang doesn't report a warning message when locating // system header files (either by looking at the value of SCE_ORBIS_SDK_DIR // or relative to the location of the compiler driver), if "-nostdinc", Index: clang/test/Driver/ps4-ps5-runtime-flags.c === --- clang/test/Driver/ps4-ps5-runtime-flags.c +++ clang/test/Driver/ps4-ps5-runtime-flags.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target -// /// Test the profile runtime library to be linked for PS4/PS5 compiler. /// Check runtime flag --dependent-lib which does not append the default library search path. // Index: clang/test/Driver/ps4-ps5-relax-relocations.c === --- clang/test/Driver/ps4-ps5-relax-relocations.c +++ clang/test/Driver/ps4-ps5-relax-relocations.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // RUN: %clang -### -target x86_64-scei-ps4 %s -o - 2>&1 | \ // RUN: FileCheck %s // RUN: %clang -### -target x86_64-scei-ps4 -Wa,-mrelax-relocations=yes %s -o - 2>&1 | \ Index: clang/test/Driver/ps4-ps5-linker-win.c === --- clang/test/Driver/ps4-ps5-linker-win.c +++ clang/test/Driver/ps4-ps5-linker-win.c @@ -1,7 +1,7 @@ // This test checks that orbis-ld is used for PS4 linker all the time, and // prospero-lld is used for PS5 linker. Specifying -fuse-ld causes an error. -// REQUIRES: system-windows, x86-registered-target +// REQUIRES: system-windows // RUN: mkdir -p %t // RUN: touch %t/orbis-ld.exe Index: clang/test/Driver/ps4-ps5-linker-non-win.c === --- clang/test/Driver/ps4-ps5-linker-non-win.c +++ clang/test/Driver/ps4-ps5-linker-non-win.c @@ -1,6 +1,5 @@ /// Checks proper linker prefixing for PS4 and PS5. // UNSUPPORTED: system-windows -// REQUIRES: x86-registered-target // RUN: mkdir -p %t // RUN: rm -f %t/orbis-ld Index: clang/test/Driver/ps4-ps5-header-search.c === --- clang/test/Driver/ps4-ps5-header-search.c +++ clang/test/Driver/ps4-ps5-header-search.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - /// PS4 and PS5 use the same SDK layout, so use the same tree for both. // RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target x86_64-scei-ps4 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4 // RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target x86_64-sie-ps5 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4 Index: clang/test/Driver/ps4-pic.c === --- clang/test/Driver/ps4-pic.c +++ clang/test/Driver/ps4-pic.c @@ -1,5 +1,3 @@ -// REQUIRES: x86-registered-target - // Test the driver's control over
[PATCH] D133191: Driver test: remove `REQUIRES: x86-registered-target` and set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`.
MaggieYi created this revision.
MaggieYi added reviewers: probinson, wristow, dyung.
Herald added a subscriber: pengfei.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When testing clang that has been compiled with -DDEFAULT_SYSROOT set to some
path, ps4-ps5-header-search.c would fail.
The test needs to be updated.
1. Remove unnecessary REQUIRES: x86-registered-target.
2. Override sysroot to be empty string for the test to succeed when clang is
configured with DEFAULT_SYSROOT.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D133191
Files:
clang/test/Driver/ps4-ps5-header-search.c
Index: clang/test/Driver/ps4-ps5-header-search.c
===
--- clang/test/Driver/ps4-ps5-header-search.c
+++ clang/test/Driver/ps4-ps5-header-search.c
@@ -1,8 +1,6 @@
-// REQUIRES: x86-registered-target
-
/// PS4 and PS5 use the same SDK layout, so use the same tree for both.
-// RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target
x86_64-scei-ps4 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
-// RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target
x86_64-sie-ps5 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
+// RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target
x86_64-scei-ps4 --sysroot="" -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
+// RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target
x86_64-sie-ps5 --sysroot="" -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
// ENVPS4: Inputs/scei-ps4_tree/target/include{{$}}
// ENVPS4: Inputs/scei-ps4_tree/target/include_common{{$}}
// ENVPS4-NOT: /usr/include
Index: clang/test/Driver/ps4-ps5-header-search.c
===
--- clang/test/Driver/ps4-ps5-header-search.c
+++ clang/test/Driver/ps4-ps5-header-search.c
@@ -1,8 +1,6 @@
-// REQUIRES: x86-registered-target
-
/// PS4 and PS5 use the same SDK layout, so use the same tree for both.
-// RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target x86_64-scei-ps4 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
-// RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target x86_64-sie-ps5 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
+// RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target x86_64-scei-ps4 --sysroot="" -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
+// RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target x86_64-sie-ps5 --sysroot="" -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
// ENVPS4: Inputs/scei-ps4_tree/target/include{{$}}
// ENVPS4: Inputs/scei-ps4_tree/target/include_common{{$}}
// ENVPS4-NOT: /usr/include
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D130493: Disable stack-sizes section by default for PS4.
MaggieYi updated this revision to Diff 447756.
MaggieYi added a comment.
Use `Args.addOptInFlag` instead of `Args.hasFlag`, thanks.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130493/new/
https://reviews.llvm.org/D130493
Files:
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/stack-size-section.c
Index: clang/test/Driver/stack-size-section.c
===
--- clang/test/Driver/stack-size-section.c
+++ clang/test/Driver/stack-size-section.c
@@ -4,7 +4,7 @@
// CHECK-ABSENT-NOT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -### 2>&1 |
FileCheck %s --check-prefix=CHECK-PRESENT
-// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s
--check-prefix=CHECK-PRESENT
+// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s
--check-prefix=CHECK-ABSENT
// CHECK-PRESENT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section
-fno-stack-size-section %s -### 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5879,9 +5879,8 @@
CmdArgs.push_back(A->getValue());
}
- if (Args.hasFlag(options::OPT_fstack_size_section,
- options::OPT_fno_stack_size_section, RawTriple.isPS4()))
-CmdArgs.push_back("-fstack-size-section");
+ Args.addOptInFlag(CmdArgs, options::OPT_fstack_size_section,
+options::OPT_fno_stack_size_section);
if (Args.hasArg(options::OPT_fstack_usage)) {
CmdArgs.push_back("-stack-usage-file");
Index: clang/test/Driver/stack-size-section.c
===
--- clang/test/Driver/stack-size-section.c
+++ clang/test/Driver/stack-size-section.c
@@ -4,7 +4,7 @@
// CHECK-ABSENT-NOT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT
-// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT
+// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT
// CHECK-PRESENT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -fno-stack-size-section %s -### 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5879,9 +5879,8 @@
CmdArgs.push_back(A->getValue());
}
- if (Args.hasFlag(options::OPT_fstack_size_section,
- options::OPT_fno_stack_size_section, RawTriple.isPS4()))
-CmdArgs.push_back("-fstack-size-section");
+ Args.addOptInFlag(CmdArgs, options::OPT_fstack_size_section,
+options::OPT_fno_stack_size_section);
if (Args.hasArg(options::OPT_fstack_usage)) {
CmdArgs.push_back("-stack-usage-file");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D130493: Disable stack-sizes section by default for PS4.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbfe191dfa79b: Disable stack-sizes section by default for
PS4. (authored by MaggieYi).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130493/new/
https://reviews.llvm.org/D130493
Files:
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/stack-size-section.c
Index: clang/test/Driver/stack-size-section.c
===
--- clang/test/Driver/stack-size-section.c
+++ clang/test/Driver/stack-size-section.c
@@ -4,7 +4,7 @@
// CHECK-ABSENT-NOT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -### 2>&1 |
FileCheck %s --check-prefix=CHECK-PRESENT
-// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s
--check-prefix=CHECK-PRESENT
+// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s
--check-prefix=CHECK-ABSENT
// CHECK-PRESENT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section
-fno-stack-size-section %s -### 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5886,9 +5886,8 @@
CmdArgs.push_back(A->getValue());
}
- if (Args.hasFlag(options::OPT_fstack_size_section,
- options::OPT_fno_stack_size_section, RawTriple.isPS4()))
-CmdArgs.push_back("-fstack-size-section");
+ Args.addOptInFlag(CmdArgs, options::OPT_fstack_size_section,
+options::OPT_fno_stack_size_section);
if (Args.hasArg(options::OPT_fstack_usage)) {
CmdArgs.push_back("-stack-usage-file");
Index: clang/test/Driver/stack-size-section.c
===
--- clang/test/Driver/stack-size-section.c
+++ clang/test/Driver/stack-size-section.c
@@ -4,7 +4,7 @@
// CHECK-ABSENT-NOT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT
-// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT
+// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT
// CHECK-PRESENT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -fno-stack-size-section %s -### 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5886,9 +5886,8 @@
CmdArgs.push_back(A->getValue());
}
- if (Args.hasFlag(options::OPT_fstack_size_section,
- options::OPT_fno_stack_size_section, RawTriple.isPS4()))
-CmdArgs.push_back("-fstack-size-section");
+ Args.addOptInFlag(CmdArgs, options::OPT_fstack_size_section,
+options::OPT_fno_stack_size_section);
if (Args.hasArg(options::OPT_fstack_usage)) {
CmdArgs.push_back("-stack-usage-file");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.
MaggieYi created this revision.
MaggieYi added reviewers: echristo, jmorse, wolfgangp, probinson, dblaikie,
aprantl.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
The detailed description of the issue could be found in
https://bugs.llvm.org/show_bug.cgi?id=51277.
In the original code, the 'getDeclAlignIfRequired' function is used.
The 'getDeclAlignIfRequired' function will return the max alignment
of all aligned attributes if the type has aligned attributes. The
function doesn’t consider the type at all.
The 'getTypeAlignIfRequired' function uses the type’s alignment value,
which also used by the 'alignof' function. I think we should use the
function of 'getTypeAlignIfRequired'.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124006
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGen/debug-info-struct-align.c
Index: clang/test/CodeGen/debug-info-struct-align.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-struct-align.c
@@ -0,0 +1,21 @@
+// Test for debug info related to DW_AT_alignment attribute in the struct
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType",
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1",
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2",
{{.*}}, align: 8
+
+struct MyType {
+ int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+struct MyType1 {
+ int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+struct MyType2 {
+ __attribute__((packed))int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
return getOrCreateRecordFwdDecl(Ty, RDContext);
uint64_t Size = CGM.getContext().getTypeSize(Ty);
- auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+ // __attribute__((aligned)) can increase or decrease alignment *except* on a
+ // struct or struct member, where it only increases alignment unless
'packed'
+ // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+ // to be used.
+ auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
Index: clang/test/CodeGen/debug-info-struct-align.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-struct-align.c
@@ -0,0 +1,21 @@
+// Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+ int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+struct MyType1 {
+ int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+struct MyType2 {
+ __attribute__((packed))int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
return getOrCreateRecordFwdDecl(Ty, RDContext);
uint64_t Size = CGM.getContext().getTypeSize(Ty);
- auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+ // __attribute__((aligned)) can increase or decrease alignment *except* on a
+ // struct or struct member, where it only increases alignment unless 'packed'
+ // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+ // to be used.
+ auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.
MaggieYi updated this revision to Diff 424257.
MaggieYi added a comment.
Thanks David, I have modified the test following your advice.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124006/new/
https://reviews.llvm.org/D124006
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-struct-align.cpp
Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+// Test for debug info related to DW_AT_alignment attribute in the struct
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType",
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1",
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2",
{{.*}}, align: 8
+
+struct MyType {
+ int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+static_assert(alignof(struct MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+ int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+static_assert(alignof(struct MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+ __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
+
+static_assert(alignof(struct MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
return getOrCreateRecordFwdDecl(Ty, RDContext);
uint64_t Size = CGM.getContext().getTypeSize(Ty);
- auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+ // __attribute__((aligned)) can increase or decrease alignment *except* on a
+ // struct or struct member, where it only increases alignment unless
'packed'
+ // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+ // to be used.
+ auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+// Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+ int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+static_assert(alignof(struct MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+ int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+static_assert(alignof(struct MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+ __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
+
+static_assert(alignof(struct MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
return getOrCreateRecordFwdDecl(Ty, RDContext);
uint64_t Size = CGM.getContext().getTypeSize(Ty);
- auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+ // __attribute__((aligned)) can increase or decrease alignment *except* on a
+ // struct or struct member, where it only increases alignment unless 'packed'
+ // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+ // to be used.
+ auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.
MaggieYi updated this revision to Diff 424420.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124006/new/
https://reviews.llvm.org/D124006
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-struct-align.cpp
Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+// Test for debug info related to DW_AT_alignment attribute in the struct
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType",
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1",
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2",
{{.*}}, align: 8
+
+struct MyType {
+ int m;
+} __attribute__((aligned(1)));
+MyType mt;
+
+static_assert(alignof(MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+ int m;
+} __attribute__((packed, aligned(1)));
+MyType1 mt1;
+
+static_assert(alignof(MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+ __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+MyType2 mt2;
+
+static_assert(alignof(MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,7 +3561,11 @@
return getOrCreateRecordFwdDecl(Ty, RDContext);
uint64_t Size = CGM.getContext().getTypeSize(Ty);
- auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+ // __attribute__((aligned)) can increase or decrease alignment *except* on a
+ // struct or struct member, where it only increases alignment unless
'packed'
+ // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+ // to be used.
+ auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+// Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+ int m;
+} __attribute__((aligned(1)));
+MyType mt;
+
+static_assert(alignof(MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+ int m;
+} __attribute__((packed, aligned(1)));
+MyType1 mt1;
+
+static_assert(alignof(MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+ __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+MyType2 mt2;
+
+static_assert(alignof(MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,7 +3561,11 @@
return getOrCreateRecordFwdDecl(Ty, RDContext);
uint64_t Size = CGM.getContext().getTypeSize(Ty);
- auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+ // __attribute__((aligned)) can increase or decrease alignment *except* on a
+ // struct or struct member, where it only increases alignment unless 'packed'
+ // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+ // to be used.
+ auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.
MaggieYi added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-struct-align.cpp:11 +} __attribute__((aligned(1))); +struct MyType mt; + dblaikie wrote: > You can drop the "struct" here and from other references to these types (in > mt1/mt2 and the alignof calls) Thanks David. All unnecessary `struct`s have been removed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124006/new/ https://reviews.llvm.org/D124006 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb09ba4262076: Bug 51277: [DWARF] DW_AT_alignment incorrect
when (authored by MaggieYi).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124006/new/
https://reviews.llvm.org/D124006
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-struct-align.cpp
Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+// Test for debug info related to DW_AT_alignment attribute in the struct
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType",
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1",
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2",
{{.*}}, align: 8
+
+struct MyType {
+ int m;
+} __attribute__((aligned(1)));
+MyType mt;
+
+static_assert(alignof(MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+ int m;
+} __attribute__((packed, aligned(1)));
+MyType1 mt1;
+
+static_assert(alignof(MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+ __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+MyType2 mt2;
+
+static_assert(alignof(MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,7 +3561,11 @@
return getOrCreateRecordFwdDecl(Ty, RDContext);
uint64_t Size = CGM.getContext().getTypeSize(Ty);
- auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+ // __attribute__((aligned)) can increase or decrease alignment *except* on a
+ // struct or struct member, where it only increases alignment unless
'packed'
+ // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+ // to be used.
+ auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+// Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+ int m;
+} __attribute__((aligned(1)));
+MyType mt;
+
+static_assert(alignof(MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+ int m;
+} __attribute__((packed, aligned(1)));
+MyType1 mt1;
+
+static_assert(alignof(MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+ __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+MyType2 mt2;
+
+static_assert(alignof(MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,7 +3561,11 @@
return getOrCreateRecordFwdDecl(Ty, RDContext);
uint64_t Size = CGM.getContext().getTypeSize(Ty);
- auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+ // __attribute__((aligned)) can increase or decrease alignment *except* on a
+ // struct or struct member, where it only increases alignment unless 'packed'
+ // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+ // to be used.
+ auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D130493: Disable stack-sizes section by default for PS4.
MaggieYi created this revision.
MaggieYi added reviewers: probinson, wristow.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.
Change Clang's default so that -fstack-size-section is no longer enabled by
default for PS4.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130493
Files:
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/stack-size-section.c
Index: clang/test/Driver/stack-size-section.c
===
--- clang/test/Driver/stack-size-section.c
+++ clang/test/Driver/stack-size-section.c
@@ -4,7 +4,7 @@
// CHECK-ABSENT-NOT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -### 2>&1 |
FileCheck %s --check-prefix=CHECK-PRESENT
-// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s
--check-prefix=CHECK-PRESENT
+// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s
--check-prefix=CHECK-ABSENT
// CHECK-PRESENT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section
-fno-stack-size-section %s -### 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5880,7 +5880,7 @@
}
if (Args.hasFlag(options::OPT_fstack_size_section,
- options::OPT_fno_stack_size_section, RawTriple.isPS4()))
+ options::OPT_fno_stack_size_section, false))
CmdArgs.push_back("-fstack-size-section");
if (Args.hasArg(options::OPT_fstack_usage)) {
Index: clang/test/Driver/stack-size-section.c
===
--- clang/test/Driver/stack-size-section.c
+++ clang/test/Driver/stack-size-section.c
@@ -4,7 +4,7 @@
// CHECK-ABSENT-NOT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT
-// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT
+// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT
// CHECK-PRESENT: -fstack-size-section
// RUN: %clang -target x86_64-unknown -fstack-size-section -fno-stack-size-section %s -### 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5880,7 +5880,7 @@
}
if (Args.hasFlag(options::OPT_fstack_size_section,
- options::OPT_fno_stack_size_section, RawTriple.isPS4()))
+ options::OPT_fno_stack_size_section, false))
CmdArgs.push_back("-fstack-size-section");
if (Args.hasArg(options::OPT_fstack_usage)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.
MaggieYi created this revision.
MaggieYi added reviewers: MaskRay, peter.smith, vitalybuka, probinson,
pgousseau, glandium, uabelho.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
PR for https://github.com/llvm/llvm-project/issues/64931.
An execute-only target disallows data access to code sections. When enabling
the function sanitizer (-fsanitize=function), UBSan function signatures and
type hashes are emitted within the function's prologue data to enable checking
of the function type. This results in a non-execute access to the code section
and a runtime error.
To solve the issue, -fsanitize=function should not be included in any check
group (e.g. undefined) on an execute-only target. If a user passes
-fsanitize=undefined, there is no error and no warning. However, if the user
explicitly passes -fsanitize=function on an execute-only target, an error will
be emitted.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158614
Files:
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/include/clang/Basic/Sanitizers.h
clang/lib/Basic/CMakeLists.txt
clang/lib/Basic/Sanitizers.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/ubsan-function.c
clang/test/CodeGenObjCXX/crash-function-type.mm
clang/test/Driver/fsanitize.c
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -971,3 +971,14 @@
// RUN: not %clang --target=x86_64-linux-gnu -fsanitize=undefined,function -mcmodel=large %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION-CODE-MODEL
// CHECK-UBSAN-FUNCTION-CODE-MODEL: error: invalid argument '-fsanitize=function' only allowed with '-mcmodel=small'
+
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=undefined,function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined,function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// CHECK-UBSAN-FUNCTION: error: unsupported option '-fsanitize=function' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED-NOT: error: unsupported option '-fsanitize=function' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
Index: clang/test/CodeGenObjCXX/crash-function-type.mm
===
--- clang/test/CodeGenObjCXX/crash-function-type.mm
+++ clang/test/CodeGenObjCXX/crash-function-type.mm
@@ -1,3 +1,6 @@
+// Mark test as unsupported on PS5 due to PS5 doesn't support function sanitizer.
+// UNSUPPORTED: target=x86_64-sie-ps5
+
// RUN: %clang_cc1 -fblocks -fsanitize=function -emit-llvm %s -o %t
void g(void (^)());
Index: clang/test/CodeGen/ubsan-function.c
===
--- clang/test/CodeGen/ubsan-function.c
+++ clang/test/CodeGen/ubsan-function.c
@@ -1,4 +1,6 @@
// RUN: %clang_cc1 -emit-llvm -triple x86_64 -std=c17 -fsanitize=function %s -o - | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm -triple x86_64-sie-ps5 -fsanitize=function %s -o 2>&1 | FileCheck %s --check-prefix=UBSAN-FUNCTION-ERR
+// RUN: not %clang_cc1 -emit-llvm -triple armv6t2-unknown-unknown-eabi -target-feature +execute-only -fsanitize=function %s -o 2>&1 | FileCheck %s --check-prefix=UBSAN-FUNCTION-ERR
// CHECK-LABEL: define{{.*}} @call_no_prototype(
// CHECK-NOT: __ubsan_handle_function_type_mismatch
@@ -7,3 +9,5 @@
// CHECK-LABEL: define{{.*}} @call_prototype(
// CHECK: __ubsan_handle_function_type_mismatch
void call_prototype(void (*f)(void)) { f(); }
+
+// UBSAN-FUNCTION-ERR: error: unsupported option '-fsanitize=function' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4398,6 +4398,16 @@
ParseLangArgs(LangOpts, Args, DashX, T, Res.getPreprocessorOpts().Includes,
Diags);
+
+ // An execute-only target doesn't support the function sanitizer. Since `clang
+
[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.
MaggieYi updated this revision to Diff 553409.
MaggieYi edited the summary of this revision.
MaggieYi added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
The changes include:
1. Added an ARM::supportedExecuteOnly function to avoid the duplicated code.
2. Modified the `isExecuteOnlyTarget` function to only deal with the
`-mexecute-only` option.
3. Added `SanitizerKind::KCFI` to the SanitizerMask of
NotAllowedWithExecuteOnly.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158614/new/
https://reviews.llvm.org/D158614
Files:
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/include/clang/Basic/Sanitizers.h
clang/lib/Basic/CMakeLists.txt
clang/lib/Basic/Sanitizers.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/test/CodeGenObjCXX/crash-function-type.mm
clang/test/Driver/fsanitize.c
llvm/include/llvm/TargetParser/ARMTargetParser.h
llvm/lib/TargetParser/ARMTargetParser.cpp
Index: llvm/lib/TargetParser/ARMTargetParser.cpp
===
--- llvm/lib/TargetParser/ARMTargetParser.cpp
+++ llvm/lib/TargetParser/ARMTargetParser.cpp
@@ -598,3 +598,11 @@
llvm_unreachable("invalid arch name");
}
+
+bool ARM::supportedExecuteOnly(const Triple &TT) {
+ if (parseArchVersion(TT.getArchName()) < 7 &&
+ parseArch(TT.getArchName()) != ArchKind::ARMV6T2 &&
+ parseArch(TT.getArchName()) != ArchKind::ARMV6M)
+return false;
+ return true;
+}
Index: llvm/include/llvm/TargetParser/ARMTargetParser.h
===
--- llvm/include/llvm/TargetParser/ARMTargetParser.h
+++ llvm/include/llvm/TargetParser/ARMTargetParser.h
@@ -259,6 +259,9 @@
/// string then the triple's arch name is used.
StringRef getARMCPUForArch(const llvm::Triple &Triple, StringRef MArch = {});
+/// Return true if the arch supports the execute-only output. Currently, the
+/// execute-only output is only supported on ARMv6T2 and ARMv7 and above.
+bool supportedExecuteOnly(const Triple &TT);
} // namespace ARM
} // namespace llvm
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -970,3 +970,19 @@
// RUN: not %clang --target=x86_64-linux-gnu -fsanitize=undefined,function -mcmodel=large %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION-CODE-MODEL
// CHECK-UBSAN-FUNCTION-CODE-MODEL: error: invalid argument '-fsanitize=function' only allowed with '-mcmodel=small'
+
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=undefined -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// CHECK-UBSAN-KCFI-DAG: error: unsupported option '-fsanitize=kcfi' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-FUNCTION-DAG: error: unsupported option '-fsanitize=function' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED-NOT: error: unsupported option '-fsanitize=function' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
Index: clang/test/CodeGenObjCXX/crash-function-type.mm
===
--- clang/test/CodeGenObjCXX/crash-function-type.mm
+++ clang/test/CodeGenObjCXX/crash-function-type.mm
@@ -1,3 +1,6 @@
+// Mark test as unsupported on PS5 due to PS5 doesn't support function sanitizer.
+// UNSUPPORTED: target=x86_64-sie-ps5
+
// RUN: %clang_cc1 -fblocks -fsanitize=function
[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.
MaggieYi added a comment.
Thanks @simon_tatham and @MaskRay for the quick code review.
When I work on this issue, I want to add an error for both clang and clang
-cc1.
`-mexecute-only` is an ARM-only compiler option. The clang Driver will convert
`-mexecute-only` to `-target-feature +execute-only` in the clang cc1 command.
To pass the `execute-only` option, the clang command is: `clang
-target=armv6t2-eabi -mexecute-only …`. The Arm clang cc1 command:
`clang -cc1 -triple armv6t2-unknown-unknown-eabi -target-feature
+execute-only…`.
If I move the change to the code in Driver/ToolChains/Arch/ARM.cpp, the error
will only deal with the `-mexecute-only` option, but not handle the
`-target-feature +execute-only` in the `clang -cc1` command.
As @MaskRay commented that we don't perform rigid error checking for cc1. In
this case, we could handle the error in the Driver/ToolChains/Arch/ARM.cpp.
However, the target-specific predicate function (named isExecuteOnlyTarget)
allows any other targets that support execute-only could get the effect by
modifying just the `isExecuteOnlyTarget` function. Therefore, I have modified
the `isExecuteOnlyTarget` function to only deal with the `-mexecute-only`
option. I have also added a function (named ARM::supportedExecuteOnly) to avoid
the duplicated code.
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:326
def err_unsupported_abi_for_opt : Error<"'%0' can only be used with the '%1'
ABI">;
+def err_unsupported_opt_for_execute_only_target
+: Error<"unsupported option '%0' for the execute only target '%1'">;
MaskRay wrote:
> We don't need this diagnostic as a common kind (we only use it in driver).
>
> I think we can reuse `err_drv_argument_not_allowed_with` . Though for PS5 you
> will get `... allowed with '-mexecute-only'` even if the user doesn't specify
> `-mexecute-only`, but I hope it is fine.
Since `err_drv_argument_not_allowed_with` is an ARM-only option, We cannot
reuse it.
Comment at: clang/lib/Driver/SanitizerArgs.cpp:406
+if (SanitizerMask KindsToDiagnose =
+Add & NotAllowedWithExecuteOnly & ~DiagnosedKinds) {
+ if (DiagnoseErrors)
MaskRay wrote:
> I think it's clear not not to add the variable `NotAllowedWithExecuteOnly`.
>
> Currently, I need to check the definition of `NotAllowedWithExecuteOnly` to
> understand that this comment does what it says. For now, just encoding
> `Function` here is clearer.
`NotAllowedWithExecuteOnly` is modified to include the `SanitizerKind::KCFI`,
therefore I keep it in the code.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158614/new/
https://reviews.llvm.org/D158614
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.
MaggieYi updated this revision to Diff 554474.
MaggieYi marked 7 inline comments as done.
MaggieYi added a comment.
Thanks @MaskRay, I have updated the patch following your suggestions.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158614/new/
https://reviews.llvm.org/D158614
Files:
clang/include/clang/Basic/Sanitizers.h
clang/lib/Basic/CMakeLists.txt
clang/lib/Basic/Sanitizers.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/test/CodeGenObjCXX/crash-function-type.mm
clang/test/Driver/fsanitize.c
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -970,3 +970,18 @@
// RUN: not %clang --target=x86_64-linux-gnu -fsanitize=undefined,function -mcmodel=large %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION-CODE-MODEL
// CHECK-UBSAN-FUNCTION-CODE-MODEL: error: invalid argument '-fsanitize=function' only allowed with '-mcmodel=small'
+
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=undefined -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// CHECK-UBSAN-KCFI-DAG: error: invalid argument '-fsanitize=kcfi' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-FUNCTION-DAG: error: invalid argument '-fsanitize=function' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED-NOT: error: invalid argument '-fsanitize=function' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED: "-fsanitize={{((alignment|array-bounds|bool|builtin|enum|float-cast-overflow|integer-divide-by-zero|nonnull-attribute|null|pointer-overflow|return|returns-nonnull-attribute|shift-base|shift-exponent|signed-integer-overflow|unreachable|vla-bound),?){17}"}}
Index: clang/test/CodeGenObjCXX/crash-function-type.mm
===
--- clang/test/CodeGenObjCXX/crash-function-type.mm
+++ clang/test/CodeGenObjCXX/crash-function-type.mm
@@ -1,3 +1,6 @@
+// Mark test as unsupported on PS5 due to PS5 doesn't support function sanitizer.
+// UNSUPPORTED: target=x86_64-sie-ps5
+
// RUN: %clang_cc1 -fblocks -fsanitize=function -emit-llvm %s -o %t
void g(void (^)());
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -37,6 +37,8 @@
SanitizerKind::Vptr | SanitizerKind::CFI;
static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
static const SanitizerMask NotAllowedWithMinimalRuntime = SanitizerKind::Vptr;
+static const SanitizerMask NotAllowedWithExecuteOnly =
+SanitizerKind::Function | SanitizerKind::KCFI;
static const SanitizerMask RequiresPIE =
SanitizerKind::DataFlow | SanitizerKind::Scudo;
static const SanitizerMask NeedsUnwindTables =
@@ -395,6 +397,22 @@
DiagnosedKinds |= SanitizerKind::Function;
}
}
+ // -fsanitize=function and -fsanitize=kcfi instrument indirect function
+ // calls to load a type hash before the function label. Therefore, an
+ // execute-only target doesn't support the function and kcfi sanitizers.
+ const llvm::Triple &Triple = TC.getTriple();
+ if (isExecuteOnlyTarget(Triple, Args)) {
+if (SanitizerMask KindsToDiagnose =
+Add & NotAllowedWithExecuteOnly & ~DiagnosedKinds) {
+ if (DiagnoseErrors) {
+std::string Desc = describeSanitizeArg(Arg, KindsToDiagnose);
+D.Diag(diag::err_drv_argument_not_allowed_with)
+<< Desc << Triple.str();
+ }
+ DiagnosedKinds |= KindsToDiagnose;
+}
+Add &= ~NotAllowedWithExecuteOnly;
+ }
// FIXME: Make CFI on member function calls compatible with cross-DSO CFI.
// There are currently two problems:
@@ -457,6 +475,10 @@
if (Minima
[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.
MaggieYi added inline comments. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:326 def err_unsupported_abi_for_opt : Error<"'%0' can only be used with the '%1' ABI">; +def err_unsupported_opt_for_execute_only_target +: Error<"unsupported option '%0' for the execute only target '%1'">; MaskRay wrote: > MaggieYi wrote: > > MaskRay wrote: > > > We don't need this diagnostic as a common kind (we only use it in driver). > > > > > > I think we can reuse `err_drv_argument_not_allowed_with` . Though for PS5 > > > you will get `... allowed with '-mexecute-only'` even if the user doesn't > > > specify `-mexecute-only`, but I hope it is fine. > > Since `err_drv_argument_not_allowed_with` is an ARM-only option, We cannot > > reuse it. > `err_drv_argument_not_allowed_with` is a generic diagnostic. My point is that > we don't need an extra err_unsupported_opt_for_execute_only_target. Sorry, I mean that `-mexecute-only` is the arm-only option [[https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L4202|Options.td#L4202]]. As you suggested that we could use `err_drv_argument_not_allowed_with` by passing `-fsanitize=function` (not `-mexecute-only`) and the target triple. For example, "error: invalid argument '-fsanitize=function' not allowed with 'x86_64-sie-ps5'". I have removed `err_unsupported_opt_for_execute_only_target.` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158614/new/ https://reviews.llvm.org/D158614 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.
MaggieYi updated this revision to Diff 554665.
MaggieYi marked 5 inline comments as done.
MaggieYi added a comment.
Thanks @MaskRay. The patch is updated.
Hi @simon_tatham, are you happy with the patch?
Thanks
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158614/new/
https://reviews.llvm.org/D158614
Files:
clang/include/clang/Basic/Sanitizers.h
clang/lib/Basic/CMakeLists.txt
clang/lib/Basic/Sanitizers.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/test/CodeGenObjCXX/crash-function-type.mm
clang/test/Driver/fsanitize.c
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -966,3 +966,17 @@
// RUN: not %clang --target=x86_64-linux-gnu -fsanitize=undefined,function -mcmodel=large %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION-CODE-MODEL
// CHECK-UBSAN-FUNCTION-CODE-MODEL: error: invalid argument '-fsanitize=function' only allowed with '-mcmodel=small'
+
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=undefined -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// CHECK-UBSAN-KCFI-DAG: error: invalid argument '-fsanitize=kcfi' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-FUNCTION-DAG: error: invalid argument '-fsanitize=function' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED: "-fsanitize={{((alignment|array-bounds|bool|builtin|enum|float-cast-overflow|integer-divide-by-zero|nonnull-attribute|null|pointer-overflow|return|returns-nonnull-attribute|shift-base|shift-exponent|signed-integer-overflow|unreachable|vla-bound),?){17}"}}
Index: clang/test/CodeGenObjCXX/crash-function-type.mm
===
--- clang/test/CodeGenObjCXX/crash-function-type.mm
+++ clang/test/CodeGenObjCXX/crash-function-type.mm
@@ -1,3 +1,6 @@
+// Mark test as unsupported on PS5 due to PS5 doesn't support function sanitizer.
+// UNSUPPORTED: target=x86_64-sie-ps5
+
// RUN: %clang_cc1 -fblocks -fsanitize=function -emit-llvm %s -o %t
void g(void (^)());
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -37,6 +37,8 @@
SanitizerKind::Vptr | SanitizerKind::CFI;
static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
static const SanitizerMask NotAllowedWithMinimalRuntime = SanitizerKind::Vptr;
+static const SanitizerMask NotAllowedWithExecuteOnly =
+SanitizerKind::Function | SanitizerKind::KCFI;
static const SanitizerMask RequiresPIE =
SanitizerKind::DataFlow | SanitizerKind::Scudo;
static const SanitizerMask NeedsUnwindTables =
@@ -395,6 +397,22 @@
DiagnosedKinds |= SanitizerKind::Function;
}
}
+ // -fsanitize=function and -fsanitize=kcfi instrument indirect function
+ // calls to load a type hash before the function label. Therefore, an
+ // execute-only target doesn't support the function and kcfi sanitizers.
+ const llvm::Triple &Triple = TC.getTriple();
+ if (isExecuteOnlyTarget(Triple, Args)) {
+if (SanitizerMask KindsToDiagnose =
+Add & NotAllowedWithExecuteOnly & ~DiagnosedKinds) {
+ if (DiagnoseErrors) {
+std::string Desc = describeSanitizeArg(Arg, KindsToDiagnose);
+D.Diag(diag::err_drv_argument_not_allowed_with)
+<< Desc << Triple.str();
+ }
+ DiagnosedKinds |= KindsToDiagnose;
+}
+Add &= ~NotAllowedWithExecuteOnly;
+ }
// FIXME: Make CFI on member function calls compatible with cross-DSO CFI.
// There are currently two problems:
@@ -457,6 +475,10 @@
if (MinimalRuntime) {
Add &= ~NotAllowedWithMinimalRuntime;
}
+ // NotAllowedWithExecuteOnly is silently discar
[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ef536a12ea6: [UBSan] Disable the function and kcfi
sanitizers on an execute-only target. (authored by MaggieYi).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158614/new/
https://reviews.llvm.org/D158614
Files:
clang/include/clang/Basic/Sanitizers.h
clang/lib/Basic/CMakeLists.txt
clang/lib/Basic/Sanitizers.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/test/CodeGenObjCXX/crash-function-type.mm
clang/test/Driver/fsanitize.c
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -966,3 +966,17 @@
// RUN: not %clang --target=x86_64-linux-gnu -fsanitize=undefined,function -mcmodel=large %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION-CODE-MODEL
// CHECK-UBSAN-FUNCTION-CODE-MODEL: error: invalid argument '-fsanitize=function' only allowed with '-mcmodel=small'
+
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=undefined -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// CHECK-UBSAN-KCFI-DAG: error: invalid argument '-fsanitize=kcfi' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-FUNCTION-DAG: error: invalid argument '-fsanitize=function' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED: "-fsanitize={{((alignment|array-bounds|bool|builtin|enum|float-cast-overflow|integer-divide-by-zero|nonnull-attribute|null|pointer-overflow|return|returns-nonnull-attribute|shift-base|shift-exponent|signed-integer-overflow|unreachable|vla-bound),?){17}"}}
Index: clang/test/CodeGenObjCXX/crash-function-type.mm
===
--- clang/test/CodeGenObjCXX/crash-function-type.mm
+++ clang/test/CodeGenObjCXX/crash-function-type.mm
@@ -1,3 +1,6 @@
+// Mark test as unsupported on PS5 due to PS5 doesn't support function sanitizer.
+// UNSUPPORTED: target=x86_64-sie-ps5
+
// RUN: %clang_cc1 -fblocks -fsanitize=function -emit-llvm %s -o %t
void g(void (^)());
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -37,6 +37,8 @@
SanitizerKind::Vptr | SanitizerKind::CFI;
static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
static const SanitizerMask NotAllowedWithMinimalRuntime = SanitizerKind::Vptr;
+static const SanitizerMask NotAllowedWithExecuteOnly =
+SanitizerKind::Function | SanitizerKind::KCFI;
static const SanitizerMask RequiresPIE =
SanitizerKind::DataFlow | SanitizerKind::Scudo;
static const SanitizerMask NeedsUnwindTables =
@@ -395,6 +397,22 @@
DiagnosedKinds |= SanitizerKind::Function;
}
}
+ // -fsanitize=function and -fsanitize=kcfi instrument indirect function
+ // calls to load a type hash before the function label. Therefore, an
+ // execute-only target doesn't support the function and kcfi sanitizers.
+ const llvm::Triple &Triple = TC.getTriple();
+ if (isExecuteOnlyTarget(Triple, Args)) {
+if (SanitizerMask KindsToDiagnose =
+Add & NotAllowedWithExecuteOnly & ~DiagnosedKinds) {
+ if (DiagnoseErrors) {
+std::string Desc = describeSanitizeArg(Arg, KindsToDiagnose);
+D.Diag(diag::err_drv_argument_not_allowed_with)
+<< Desc << Triple.str();
+ }
+ DiagnosedKinds |= KindsToDiagnose;
+}
+Add &= ~NotAllowedWithExecuteOnly;
+ }
// FIXME: Make CFI on member function calls compatible with cross-DSO CFI.
// There are currently two problems:
@@ -457,6 +475,10 @@
if (MinimalRuntime) {
Add &= ~NotAllowedWithMinimalRuntime;
}
+ // NotAllowedW
[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.
MaggieYi added a comment. Thanks @MaskRay. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158614/new/ https://reviews.llvm.org/D158614 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144371: [LLVM-COV] Fix an issue: a statement after calling 'assert()' function is wrongly marked as 'not executed'
MaggieYi added a comment. Gentle ping ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144371/new/ https://reviews.llvm.org/D144371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144371: [LLVM-COV] Fix an issue: a statement after calling 'assert()' function is wrongly marked as 'not executed'
MaggieYi updated this revision to Diff 501509.
MaggieYi added a comment.
Thanks Zequan. I have added two tests following your suggestion.
Kind regards,
Maggie
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144371/new/
https://reviews.llvm.org/D144371
Files:
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/terminate-statements.cpp
Index: clang/test/CoverageMapping/terminate-statements.cpp
===
--- clang/test/CoverageMapping/terminate-statements.cpp
+++ clang/test/CoverageMapping/terminate-statements.cpp
@@ -320,6 +320,32 @@
included_func();
}
+// CHECK-LABEL: _Z7ornoretv:
+void abort() __attribute__((noreturn));
+
+int ornoret(void) {
+ ( true || (abort(), 0) ); // CHECK: Gap,File 0, [[@LINE]]:28 -> [[@LINE+1]]:3 = #0
+ ( false || (abort(), 0) ); // CHECK: Gap,File 0, [[@LINE]]:29 -> [[@LINE+1]]:3 = 0
+ return 0;
+}
+
+// CHECK-LABEL: _Z17abstractcondnoretv:
+int abstractcondnoret(void) {
+ ( true ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = #1
+ ( false ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = #2
+ ( true ? abort() : void (0) ); // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = (#2 - #3)
+ ( false ? abort() : void (0) ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = ((#2 - #3) - #4)
+ return 0;
+}
+
+// CHECK-LABEL: _Z13elsecondnoretv:
+int elsecondnoret(void) {
+ if (true) {} else {
+true ? void (0) : abort();
+ } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = (#1 + #2)
+ return 0;
+}
+
int main() {
foo(0);
foo(1);
@@ -339,5 +365,8 @@
while_loop();
gotos();
include();
+ ornoret();
+ abstractcondnoret();
+ elsecondnoret();
return 0;
}
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1466,6 +1466,7 @@
Counter TrueCount = getRegionCounter(E);
propagateCounts(ParentCount, E->getCond());
+Counter OutCount;
if (!isa(E)) {
// The 'then' count applies to the area immediately after the condition.
@@ -1475,12 +1476,18 @@
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), TrueCount);
extendRegion(E->getTrueExpr());
- propagateCounts(TrueCount, E->getTrueExpr());
+ OutCount = propagateCounts(TrueCount, E->getTrueExpr());
}
extendRegion(E->getFalseExpr());
-propagateCounts(subtractCounters(ParentCount, TrueCount),
-E->getFalseExpr());
+OutCount = addCounters(
+OutCount, propagateCounts(subtractCounters(ParentCount, TrueCount),
+ E->getFalseExpr()));
+
+if (OutCount != ParentCount) {
+ pushRegion(OutCount);
+ GapRegionCounter = OutCount;
+}
// Create Branch Region around condition.
createBranchRegion(E->getCond(), TrueCount,
@@ -1514,9 +1521,19 @@
subtractCounters(RHSExecCnt, RHSTrueCnt));
}
+ // Determine whether the right side of OR operation need to be visited.
+ bool shouldVisitRHS(const Expr *LHS) {
+bool LHSIsTrue = false;
+bool LHSIsConst = false;
+if (!LHS->isValueDependent())
+ LHSIsConst = LHS->EvaluateAsBooleanCondition(
+ LHSIsTrue, CVM.getCodeGenModule().getContext());
+return !LHSIsConst || (LHSIsConst && !LHSIsTrue);
+ }
+
void VisitBinLOr(const BinaryOperator *E) {
extendRegion(E->getLHS());
-propagateCounts(getRegion().getCounter(), E->getLHS());
+Counter OutCount = propagateCounts(getRegion().getCounter(), E->getLHS());
handleFileExit(getEnd(E->getLHS()));
// Counter tracks the right hand side of a logical or operator.
@@ -1529,6 +1546,10 @@
// Extract the RHS's "False" Instance Counter.
Counter RHSFalseCnt = getRegionCounter(E->getRHS());
+if (!shouldVisitRHS(E->getLHS())) {
+ GapRegionCounter = OutCount;
+}
+
// Extract the Parent Region Counter.
Counter ParentCnt = getRegion().getCounter();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144371: [LLVM-COV] Fix an issue: a statement after calling 'assert()' function is wrongly marked as 'not executed'
MaggieYi marked an inline comment as done. MaggieYi added inline comments. Comment at: clang/test/CoverageMapping/terminate-statements.cpp:335 + ( true ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = #1 + ( false ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = #2 + return 0; zequanwu wrote: > For completeness, can you add following two tests: > ``` > ( true ? abort() : void (0) ); > ( false ? abort() : void (0) ); > ``` Thanks, two tests have been added. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144371/new/ https://reviews.llvm.org/D144371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144371: [LLVM-COV] Fix an issue: a statement after calling 'assert()' function is wrongly marked as 'not executed'
This revision was automatically updated to reflect the committed changes.
MaggieYi marked an inline comment as done.
Closed by commit rG94dd4766a61b: [Coverage] Fix an issue: a statement after
calling 'assert()' function is… (authored by MaggieYi).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144371/new/
https://reviews.llvm.org/D144371
Files:
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/terminate-statements.cpp
Index: clang/test/CoverageMapping/terminate-statements.cpp
===
--- clang/test/CoverageMapping/terminate-statements.cpp
+++ clang/test/CoverageMapping/terminate-statements.cpp
@@ -320,6 +320,32 @@
included_func();
}
+// CHECK-LABEL: _Z7ornoretv:
+void abort() __attribute__((noreturn));
+
+int ornoret(void) {
+ ( true || (abort(), 0) ); // CHECK: Gap,File 0, [[@LINE]]:28 -> [[@LINE+1]]:3 = #0
+ ( false || (abort(), 0) ); // CHECK: Gap,File 0, [[@LINE]]:29 -> [[@LINE+1]]:3 = 0
+ return 0;
+}
+
+// CHECK-LABEL: _Z17abstractcondnoretv:
+int abstractcondnoret(void) {
+ ( true ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = #1
+ ( false ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = #2
+ ( true ? abort() : void (0) ); // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = (#2 - #3)
+ ( false ? abort() : void (0) ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = ((#2 - #3) - #4)
+ return 0;
+}
+
+// CHECK-LABEL: _Z13elsecondnoretv:
+int elsecondnoret(void) {
+ if (true) {} else {
+true ? void (0) : abort();
+ } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = (#1 + #2)
+ return 0;
+}
+
int main() {
foo(0);
foo(1);
@@ -339,5 +365,8 @@
while_loop();
gotos();
include();
+ ornoret();
+ abstractcondnoret();
+ elsecondnoret();
return 0;
}
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1466,6 +1466,7 @@
Counter TrueCount = getRegionCounter(E);
propagateCounts(ParentCount, E->getCond());
+Counter OutCount;
if (!isa(E)) {
// The 'then' count applies to the area immediately after the condition.
@@ -1475,12 +1476,18 @@
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), TrueCount);
extendRegion(E->getTrueExpr());
- propagateCounts(TrueCount, E->getTrueExpr());
+ OutCount = propagateCounts(TrueCount, E->getTrueExpr());
}
extendRegion(E->getFalseExpr());
-propagateCounts(subtractCounters(ParentCount, TrueCount),
-E->getFalseExpr());
+OutCount = addCounters(
+OutCount, propagateCounts(subtractCounters(ParentCount, TrueCount),
+ E->getFalseExpr()));
+
+if (OutCount != ParentCount) {
+ pushRegion(OutCount);
+ GapRegionCounter = OutCount;
+}
// Create Branch Region around condition.
createBranchRegion(E->getCond(), TrueCount,
@@ -1514,9 +1521,19 @@
subtractCounters(RHSExecCnt, RHSTrueCnt));
}
+ // Determine whether the right side of OR operation need to be visited.
+ bool shouldVisitRHS(const Expr *LHS) {
+bool LHSIsTrue = false;
+bool LHSIsConst = false;
+if (!LHS->isValueDependent())
+ LHSIsConst = LHS->EvaluateAsBooleanCondition(
+ LHSIsTrue, CVM.getCodeGenModule().getContext());
+return !LHSIsConst || (LHSIsConst && !LHSIsTrue);
+ }
+
void VisitBinLOr(const BinaryOperator *E) {
extendRegion(E->getLHS());
-propagateCounts(getRegion().getCounter(), E->getLHS());
+Counter OutCount = propagateCounts(getRegion().getCounter(), E->getLHS());
handleFileExit(getEnd(E->getLHS()));
// Counter tracks the right hand side of a logical or operator.
@@ -1529,6 +1546,10 @@
// Extract the RHS's "False" Instance Counter.
Counter RHSFalseCnt = getRegionCounter(E->getRHS());
+if (!shouldVisitRHS(E->getLHS())) {
+ GapRegionCounter = OutCount;
+}
+
// Extract the Parent Region Counter.
Counter ParentCnt = getRegion().getCounter();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D147520: Fix a time-trace issue of incorrect header hierarchy when a header contains a template function for its last symbol.
MaggieYi created this revision.
MaggieYi added reviewers: Whitney, jamieschmeiser, MaskRay, rnk, aras-p,
anton-afanasyev.
Herald added a subscriber: hiraditya.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
`HandleEndOfFile` is invoked when the lexer hits the end of the current file.
This either returns the EOF token or pops a level off the include stack and
keeps going. If it keeps going, clang parses from one header to another header.
This results in incorrect header hierarchy in the time trace.
This patch corrects this, as reported by #56554
Fixes: https://github.com/llvm/llvm-project/issues/56554
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147520
Files:
clang/lib/Sema/Sema.cpp
clang/test/Driver/Inputs/header5.h
clang/test/Driver/Inputs/header6.h
clang/test/Driver/check-time-trace-header-hierarchy.cpp
clang/test/Driver/check-time-trace-header-hierarchy.py
llvm/include/llvm/Support/TimeProfiler.h
llvm/lib/Support/TimeProfiler.cpp
Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -107,7 +107,7 @@
Detail());
}
- void end() {
+ void element_end() {
assert(!Stack.empty() && "Must call begin() first");
TimeTraceProfilerEntry &E = Stack.back();
E.End = ClockType::now();
@@ -143,6 +143,33 @@
Stack.pop_back();
}
+ void end(bool IsSource = false) {
+bool TopElementIsSource = Stack.back().Name == "Source";
+// If time trace profiler is ended by exiting a file (IsSource=true), we
+// expect the top of Stack is time section entry with name "Source".
+// If IsSource is false, the last time section should not be "Source". In
+// these two cases, only pop the top element of the Stack.
+if ((IsSource && TopElementIsSource) ||
+(!IsSource && !TopElementIsSource)) {
+ element_end();
+ return;
+}
+
+// If the last time section entry is "Source" but time trace profiler is not
+// ended by exiting a file (IsSource=false), we don't need to do anything
+// since the time section entry has been popped out when exiting the
+// previous 'Source' time section entry.
+if (!IsSource && TopElementIsSource)
+ return;
+
+// If the last time section entry is not "Source" but time trace profiler is
+// ended by exiting a file (IsSource=true), we will pop time sections from
+// the top of Stack until the 'Source' time section.
+while (Stack.back().Name != "Source")
+ element_end();
+element_end();
+ }
+
// Write events from this TimeTraceProfilerInstance and
// ThreadTimeTraceProfilerInstances.
void write(raw_pwrite_stream &OS) {
@@ -353,7 +380,7 @@
TimeTraceProfilerInstance->begin(std::string(Name), Detail);
}
-void llvm::timeTraceProfilerEnd() {
+void llvm::timeTraceProfilerEnd(bool IsSource) {
if (TimeTraceProfilerInstance != nullptr)
-TimeTraceProfilerInstance->end();
+TimeTraceProfilerInstance->end(IsSource);
}
Index: llvm/include/llvm/Support/TimeProfiler.h
===
--- llvm/include/llvm/Support/TimeProfiler.h
+++ llvm/include/llvm/Support/TimeProfiler.h
@@ -124,8 +124,10 @@
void timeTraceProfilerBegin(StringRef Name,
llvm::function_ref Detail);
-/// Manually end the last time section.
-void timeTraceProfilerEnd();
+/// Manually end the last time section, with the given \p IsSource.
+/// Pass true to \p IsSource if the name of the last time section is
+/// "Source". By default, \p IsSource is false.
+void timeTraceProfilerEnd(bool IsSource = false);
/// The TimeTraceScope is a helper class to call the begin and end functions
/// of the time trace profiler. When the object is constructed, it begins
Index: clang/test/Driver/check-time-trace-header-hierarchy.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-header-hierarchy.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+
+import json, sys, time
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (c <= a <= d) and (c <= b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+log_contents = json.loads(sys.stdin.read())
+events = log_contents["traceEvents"]
+
+count = 0;
+header5 = header6 = parsetemplate = parseclass = None
+for event in events:
+if event["name"] == "Source" and "header5.h" in event["args"]["detail"]:
+header5 = event
+count += 1
+elif event["name"] == "Source" and "header6.h" in event["args"]["detail"]:
+header6 = event
+count += 1
+elif event["n
[PATCH] D147520: Fix a time-trace issue of incorrect header hierarchy when a header contains a template function for its last symbol.
MaggieYi added a comment.
A simple test to reproduce the issue:
% cat main.cpp
#include "1.h"
#include "2.h"
int foo();
% cat 1.h
template auto Zero() -> T { return T{}; }
%cat 2.h
struct Bla {};
Compile the code with `-ftime-trace-granularity=0 -ftime-trace` to show the
issue:
prospero-clang -ftime-trace-granularity=0 -ftime-trace main.cpp -c -o main.o
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147520/new/
https://reviews.llvm.org/D147520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D147520: Fix a time-trace issue of incorrect header hierarchy when a header contains a template function for its last symbol.
MaggieYi added a comment. Analysis the issue using the above simple example: When clang parses the first line (`#include "1.h"`) in the `main.cpp`, a time section entry is created and put on the top of the time-trace stack. Assuming this time section entry is named `Source-1.h`, which has its name `Source` and detail `1.h`. Then, clang parses the template function `Zero` defined in the `1.h` header file. Clang will create a `TimeTraceScope` variable in which its name is `ParseTemplate` and its detail is `Zero`. A new time section entry named `ParseTemplate-Zero` is created and put on the top of the time-trace stack. `ParseTemplate-Zero` has its name `ParseTemplate` and detail `Zero`. Now, the top element of the time-trace stack is `ParseTemplate-Zero` and `Source-1.h` is under `ParseTemplate-Zero`. Please note: since `ParseTemplate-Zero` is `TimeTraceScope` type variable. It should be popped out from the time-trace stack once the destructor of `TimeTraceScope` is called. Since the template `Zero` is the last symbol defined in the `1.h` header, the `LexEndOfFile` is called, then `LexedFileChanged` is called. `LexedFileChanged` is invoked when the lexer hits the end of the current file. This either returns the EOF token or pops a level off the include stack and keeps going. In our case, it keeps going and notifies the client that we are in a new header file: `2.h`. The function of `timeTraceProfilerEnd` is called to pop the top element out from the time-trace stack and calculate the corresponding compilation time. `Source-1.h` is expected to pop out from the time-trace stack. However, since `ParseTemplate-Zero` is still alive and is on the top of the time-trace stack. `ParseTemplate-Zero` is popped out from the time trace stack instead of `Source-1.h`, which is wrong. Until now, the top of the time-trace stack is `Source-1.h`. After that, a time section entry named `Source-2.h` is created and put on the top of the time-trace stack. Now, the top element of the time-trace stack is `Source-2.h`, and then `Source-1.h` is under `Source-2.h`. This results in incorrect header hierarchy in the time trace: the `2.h` shows up as an inclusion under `1.h`. Currently, two available time profiling APIs cannot deal with this special case. I have modified `timeTraceProfilerEnd()` to handle the header file specially. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147520/new/ https://reviews.llvm.org/D147520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D147520: Fix a time-trace issue of incorrect header hierarchy when a header contains a template function for its last symbol.
MaggieYi added a comment. Gentle ping ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147520/new/ https://reviews.llvm.org/D147520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148410: [Parse] Remove TimeTraceScope for "ParseTemplate"
MaggieYi added a comment. Thanks @MaskRay, the fix is fine for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148410/new/ https://reviews.llvm.org/D148410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144371: [LLVM-COV] Fix an issue: a statement after calling 'assert()' function is wrongly marked as 'not executed'
MaggieYi created this revision.
MaggieYi added reviewers: vsk, zequanwu.
MaggieYi added a project: clang.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added a subscriber: cfe-commits.
In the current coverage mapping implementation, we terminate the current region
and start a zero region when we hit a nonreturn function. However, for logical
OR, the second operand is not executed if the first operand evaluates to true.
If the nonreturn function is called in the right side of logical OR and the
left side of logical OR is TRUE, we should not start a zero `GapRegionCounter`.
This will also apply to `VisitAbstractConditionalOperator`.
The following issues are fixed by this patch:
1. https://github.com/llvm/llvm-project/issues/59030
2. https://github.com/llvm/llvm-project/issues/57388
3. https://github.com/llvm/llvm-project/issues/57481
4. https://github.com/llvm/llvm-project/issues/60701
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D144371
Files:
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/terminate-statements.cpp
Index: clang/test/CoverageMapping/terminate-statements.cpp
===
--- clang/test/CoverageMapping/terminate-statements.cpp
+++ clang/test/CoverageMapping/terminate-statements.cpp
@@ -320,6 +320,30 @@
included_func();
}
+// CHECK-LABEL: _Z7ornoretv:
+void abort() __attribute__((noreturn));
+
+int ornoret(void) {
+ ( true || (abort(), 0) ); // CHECK: Gap,File 0, [[@LINE]]:28 -> [[@LINE+1]]:3 = #0
+ ( false || (abort(), 0) ); // CHECK: Gap,File 0, [[@LINE]]:29 -> [[@LINE+1]]:3 = 0
+ return 0;
+}
+
+// CHECK-LABEL: _Z17abstractcondnoretv:
+int abstractcondnoret(void) {
+ ( true ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = #1
+ ( false ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = #2
+ return 0;
+}
+
+// CHECK-LABEL: _Z13elsecondnoretv:
+int elsecondnoret(void) {
+ if (true) {} else {
+true ? void (0) : abort();
+ } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = (#1 + #2)
+ return 0;
+}
+
int main() {
foo(0);
foo(1);
@@ -339,5 +363,8 @@
while_loop();
gotos();
include();
+ ornoret();
+ abstractcondnoret();
+ elsecondnoret();
return 0;
}
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1466,6 +1466,7 @@
Counter TrueCount = getRegionCounter(E);
propagateCounts(ParentCount, E->getCond());
+Counter OutCount;
if (!isa(E)) {
// The 'then' count applies to the area immediately after the condition.
@@ -1475,12 +1476,18 @@
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), TrueCount);
extendRegion(E->getTrueExpr());
- propagateCounts(TrueCount, E->getTrueExpr());
+ OutCount = propagateCounts(TrueCount, E->getTrueExpr());
}
extendRegion(E->getFalseExpr());
-propagateCounts(subtractCounters(ParentCount, TrueCount),
-E->getFalseExpr());
+OutCount = addCounters(
+OutCount, propagateCounts(subtractCounters(ParentCount, TrueCount),
+ E->getFalseExpr()));
+
+if (OutCount != ParentCount) {
+ pushRegion(OutCount);
+ GapRegionCounter = OutCount;
+}
// Create Branch Region around condition.
createBranchRegion(E->getCond(), TrueCount,
@@ -1514,9 +1521,19 @@
subtractCounters(RHSExecCnt, RHSTrueCnt));
}
+ // Determine whether the right side of OR operation need to be visited.
+ bool shouldVisitRHS(const Expr *LHS) {
+bool LHSIsTrue = false;
+bool LHSIsConst = false;
+if (!LHS->isValueDependent())
+ LHSIsConst = LHS->EvaluateAsBooleanCondition(
+ LHSIsTrue, CVM.getCodeGenModule().getContext());
+return !LHSIsConst || (LHSIsConst && !LHSIsTrue);
+ }
+
void VisitBinLOr(const BinaryOperator *E) {
extendRegion(E->getLHS());
-propagateCounts(getRegion().getCounter(), E->getLHS());
+Counter OutCount = propagateCounts(getRegion().getCounter(), E->getLHS());
handleFileExit(getEnd(E->getLHS()));
// Counter tracks the right hand side of a logical or operator.
@@ -1529,6 +1546,10 @@
// Extract the RHS's "False" Instance Counter.
Counter RHSFalseCnt = getRegionCounter(E->getRHS());
+if (!shouldVisitRHS(E->getLHS())) {
+ GapRegionCounter = OutCount;
+}
+
// Extract the Parent Region Counter.
Counter ParentCnt = getRegion().getCounter();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
