[compiler-rt][XRay] Make xray_interface.h C compliant
#140068
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
Thyre
changed the title
xray_interface.h C compliantxray_interface.h C compliant
|
@llvm/pr-subscribers-xray Author: Jan André Reuter (Thyre) ChangesThe XRay interface header uses no C++ specific features aside from using the To make sure that further changes to not break the interface accidentally, 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/140068.diff 2 Files Affected:
diff --git a/compiler-rt/include/xray/xray_interface.h b/compiler-rt/include/xray/xray_interface.h
index 675ea0cbc48c8..325c463e976b2 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,22 @@
#ifndef XRAY_XRAY_INTERFACE_H
#define XRAY_XRAY_INTERFACE_H
-#include <cstddef>
-#include <cstdint>
+#include <stddef.h>
+#include <stdint.h>
+#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 +70,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 +88,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 +175,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 71%
rename from compiler-rt/test/xray/TestCases/Posix/patching-unpatching.cpp
rename to compiler-rt/test/xray/TestCases/Posix/patching-unpatching.c
index a8a820bbe45f2..1417c5d74548d 100644
--- a/compiler-rt/test/xray/TestCases/Posix/patching-unpatching.cpp
+++ b/compiler-rt/test/xray/TestCases/Posix/patching-unpatching.c
@@ -1,21 +1,21 @@
// 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
+// 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
#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 +28,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
}
|
Thyre
force-pushed
the
139902-xray-interface-h-not-c-compliant
branch
from
5668722 to
593cfff
Compare
|
@MaskRay Would you mind taking a look at this? |
MaskRay
reviewed
May 24, 2025
Thyre
force-pushed
the
139902-xray-interface-h-not-c-compliant
branch
2 times, most recently
from
43ff26e to
c24f6c2
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, guard `extern "C"` and handle the XRayPatchingStatus enum in a C compliant way. To make sure that further changes to not break the interface accidentially, duplicate one test written in C++ and convert it 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 <[email protected]>
Thyre
force-pushed
the
139902-xray-interface-h-not-c-compliant
branch
from
c24f6c2 to
3a168e6
Compare
MaskRay
approved these changes
May 24, 2025
|
@Thyre Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/13758 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/11568 Here is the relevant piece of the build log for the reference |
The test failure seems to be related to a missing |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/9949 Here is the relevant piece of the build log for the reference |
jplehr
added a commit
that referenced
this pull request
May 27, 2025
)" This reverts commit 80da58d.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 27, 2025
…liant" (#141570) Reverts llvm/llvm-project#140068 Failures on PPC buildbots.
|
I was able to reproduce the issue on a Power10 system, with the same error message. So this is not limited to the build runners, but likely a more wide spread issue. |
This was referenced May 27, 2025
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
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 accidentally, 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 <[email protected]>
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
…m#141570) Reverts llvm#140068 Failures on PPC buildbots.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The XRay interface header uses no C++ specific features aside from using the
stdnamespace and including the C++ variant of C headers. Yet, these changes prevent usingxray_interface.hin external tools relying on C for different reasons. Make this header C compliant by using C headers, removing thestdnamespace fromstd::size_tand guardextern "C".To make sure that further changes to not break the interface accidentally, 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.his also not C compliant, but requires more work to port.Fixes #139902