-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Reapply "[compiler-rt][XRay] Make xray_interface.h C compliant" #141728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-xray Author: Jan André Reuter (Thyre) ChangesChanges to initial PR (#140068):
Original description (from #140068) The XRay interface header uses no C++ specific features aside from using the std namespace and including the C++ variant of C headers. Yet, these changes prevent using To make sure that further changes to not break the interface accidentially, port one test from C++ to C. This requires the C23 standard to officially support the attribute syntax used in this test case. Note that this only resolves this issue for Fixes #139902 Full diff: https://github.com/llvm/llvm-project/pull/141728.diff 2 Files Affected:
diff --git a/compiler-rt/include/xray/xray_interface.h b/compiler-rt/include/xray/xray_interface.h
index 675ea0cbc48c8..a34c025657621 100644
--- a/compiler-rt/include/xray/xray_interface.h
+++ b/compiler-rt/include/xray/xray_interface.h
@@ -1,4 +1,4 @@
-//===- xray_interface.h -----------------------------------------*- C++ -*-===//
+//===- xray_interface.h ---------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -14,20 +14,27 @@
#ifndef XRAY_XRAY_INTERFACE_H
#define XRAY_XRAY_INTERFACE_H
+#ifdef __cplusplus
#include <cstddef>
#include <cstdint>
+#else
+#include <stddef.h>
+#include <stdint.h>
+#endif
+#ifdef __cplusplus
extern "C" {
+#endif
/// Synchronize this with AsmPrinter::SledKind in LLVM.
-enum XRayEntryType {
+typedef enum {
ENTRY = 0,
EXIT = 1,
TAIL = 2,
LOG_ARGS_ENTRY = 3,
CUSTOM_EVENT = 4,
TYPED_EVENT = 5,
-};
+} XRayEntryType;
/// Provide a function to invoke for when instrumentation points are hit. This
/// is a user-visible control surface that overrides the default implementation.
@@ -68,7 +75,7 @@ extern int __xray_set_handler_arg1(void (*entry)(int32_t, XRayEntryType,
extern int __xray_remove_handler_arg1();
/// Provide a function to invoke when XRay encounters a custom event.
-extern int __xray_set_customevent_handler(void (*entry)(void *, std::size_t));
+extern int __xray_set_customevent_handler(void (*entry)(void *, size_t));
/// This removes whatever the currently provided custom event handler is.
/// Returns 1 on success, 0 on error.
@@ -86,12 +93,12 @@ extern int __xray_remove_typedevent_handler();
extern uint16_t __xray_register_event_type(const char *event_type);
-enum XRayPatchingStatus {
+typedef enum {
NOT_INITIALIZED = 0,
SUCCESS = 1,
ONGOING = 2,
FAILED = 3,
-};
+} XRayPatchingStatus;
/// This tells XRay to patch the instrumentation points in all currently loaded
/// objects. See XRayPatchingStatus for possible result values.
@@ -173,6 +180,8 @@ extern int32_t __xray_pack_id(int32_t FuncId, int32_t ObjId);
/// Calling __xray_init() more than once is safe across multiple threads.
extern void __xray_init();
+#ifdef __cplusplus
} // end extern "C"
+#endif
#endif // XRAY_XRAY_INTERFACE_H
diff --git a/compiler-rt/test/xray/TestCases/Posix/patching-unpatching.cpp b/compiler-rt/test/xray/TestCases/Posix/patching-unpatching.c
similarity index 64%
rename from compiler-rt/test/xray/TestCases/Posix/patching-unpatching.cpp
rename to compiler-rt/test/xray/TestCases/Posix/patching-unpatching.c
index a8a820bbe45f2..488c55a0280be 100644
--- a/compiler-rt/test/xray/TestCases/Posix/patching-unpatching.cpp
+++ b/compiler-rt/test/xray/TestCases/Posix/patching-unpatching.c
@@ -1,21 +1,26 @@
// Check that we can patch and un-patch on demand, and that logging gets invoked
// appropriately.
//
-// RUN: %clangxx_xray -fxray-instrument -std=c++11 %s -o %t
+// Do not run on powerpc64le, as linking XRay with C compiler causes linker error
+// due to std::__throw_system_error(int) being present in XRay libraries.
+// See https://github.com/llvm/llvm-project/issues/141598
+//
+// RUN: %clang_xray -fxray-instrument -std=c23 %s -o %t
// RUN: env XRAY_OPTIONS="patch_premain=false" %run %t 2>&1 | FileCheck %s
-// RUN: %clangxx_xray -fxray-instrument -fno-xray-function-index -std=c++11 %s -o %t
+// RUN: %clang_xray -fxray-instrument -fno-xray-function-index -std=c23 %s -o %t
// RUN: env XRAY_OPTIONS="patch_premain=false" %run %t 2>&1 | FileCheck %s
// UNSUPPORTED: target-is-mips64,target-is-mips64el
+// UNSUPPORTED: target=powerpc64le-{{.*}}
#include "xray/xray_interface.h"
-#include <cstdio>
+#include <stdio.h>
bool called = false;
void test_handler(int32_t fid, XRayEntryType type) {
- printf("called: %d, type=%d\n", fid, static_cast<int32_t>(type));
+ printf("called: %d, type=%d\n", fid, (int32_t)(type));
called = true;
}
@@ -28,24 +33,24 @@ int main() {
always_instrument();
// CHECK: always instrumented called
auto status = __xray_patch();
- printf("patching status: %d\n", static_cast<int32_t>(status));
+ printf("patching status: %d\n", (int32_t)status);
// CHECK-NEXT: patching status: 1
always_instrument();
// CHECK-NEXT: called: {{.*}}, type=0
// CHECK-NEXT: always instrumented called
// CHECK-NEXT: called: {{.*}}, type=1
status = __xray_unpatch();
- printf("patching status: %d\n", static_cast<int32_t>(status));
+ printf("patching status: %d\n", (int32_t)status);
// CHECK-NEXT: patching status: 1
always_instrument();
// CHECK-NEXT: always instrumented called
status = __xray_patch();
- printf("patching status: %d\n", static_cast<int32_t>(status));
+ printf("patching status: %d\n", (int32_t)status);
// CHECK-NEXT: patching status: 1
__xray_remove_handler();
always_instrument();
// CHECK-NEXT: always instrumented called
status = __xray_unpatch();
- printf("patching status: %d\n", static_cast<int32_t>(status));
+ printf("patching status: %d\n", (int32_t)status);
// CHECK-NEXT: patching status: 1
}
|
a70b96d
to
8d6c1ab
Compare
The XRay interface header uses no C++ specific features aside from using the std namespace and including the C++ variant of C headers. Yet, these changes prevent using `xray_interface.h` in external tools relying on C for different reasons. Make this header C compliant by using C headers, removing the std namespace from std::size_t and guard `extern "C"`. To make sure that further changes to not break the interface accidentially, port one test from C++ to C. This requires the C23 standard to officially support the attribute syntax used in this test case. Note that this only resolves this issue for `xray_interface.h`. `xray_records.h` is also not C compliant, but requires more work to port. Fixes llvm#139902 Signed-off-by: Jan André Reuter <j.reuter@fz-juelich.de>
8d6c1ab
to
20a0363
Compare
Checked on the same system used to report #141598, and the test is ignored correctly.
@sebastiankreutzer, @MaskRay Can you take another look? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes to initial PR (#140068):
Original description (from #140068)
The XRay interface header uses no C++ specific features aside from using the std namespace and including the C++ variant of C headers. Yet, these changes prevent using
xray_interface.h
in external tools relying on C for different reasons. Make this header C compliant by using C headers, removing the std namespace from std::size_t and guardextern "C"
.To make sure that further changes to not break the interface accidentially, port one test from C++ to C. This requires the C23 standard to officially support the attribute syntax used in this test case.
Note that this only resolves this issue for
xray_interface.h
.xray_records.h
is also not C compliant, but requires more work to port.Fixes #139902