Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Specify callback calling convention for callbacks on Windows x86 causes crashes on Mono. #89571

Merged
merged 5 commits into from
Aug 3, 2023
Merged

Specify callback calling convention for callbacks on Windows x86 causes crashes on Mono. #89571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a3cf4b9
Fix #88992. Explicit specify callback calling convention on Windows x86.
lateralusX Jul 27, 2023
c2e3e93
Re-enable tests previously disabled by #88992.
lateralusX Jul 27, 2023
1e73251
Fix native EventPipe test to build and run under Windows x86.
lateralusX Jul 27, 2023
62a643c
Temporary only run windows x64/x86 mono outerloop tests.
lateralusX Aug 1, 2023
38e8360
Restore mono outerloop configuration.
lateralusX Aug 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/coreclr/scripts/genEventPipe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,9 @@ def generateClrEventPipeWriteEventsImpl(
eventPipeCallbackCastExpr = ""
if target_cpp:
eventPipeCallbackCastExpr = "reinterpret_cast<EventPipeCallback>"
else:
eventPipeCallbackCastExpr = "(EventPipeCallback)"

if runtimeFlavor.mono:
WriteEventImpl.append("void " + callbackName + "(const uint8_t *, unsigned long, uint8_t, uint64_t, uint64_t, EventFilterDescriptor *, void *);\n\n")
WriteEventImpl.append("void EP_CALLBACK_CALLTYPE " + callbackName + "(const uint8_t *, unsigned long, uint8_t, uint64_t, uint64_t, EventFilterDescriptor *, void *);\n\n")

if extern: WriteEventImpl.append('extern "C" ')
WriteEventImpl.append(
Expand Down
4 changes: 1 addition & 3 deletions src/libraries/System.Diagnostics.DiagnosticSource/tests/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,4 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using Xunit;

[assembly: ActiveIssue("https://github.com/dotnet/runtime/issues/88992", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoRuntime), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsX86Process))]
using Xunit;
3 changes: 1 addition & 2 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,4 @@
using Xunit;

[assembly: SkipOnCoreClr("System.Net.Tests are flaky and/or long running: https://github.com/dotnet/runtime/issues/131", ~RuntimeConfiguration.Release)]
[assembly: SkipOnPlatform(TestPlatforms.Browser, "System.Net.Sockets is not supported on Browser")]
[assembly: ActiveIssue("https://github.com/dotnet/runtime/issues/88992", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoRuntime), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsX86Process))]
[assembly: SkipOnPlatform(TestPlatforms.Browser, "System.Net.Sockets is not supported on Browser")]
4 changes: 1 addition & 3 deletions src/libraries/System.Runtime.Caching/tests/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,4 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using Xunit;

[assembly: ActiveIssue("https://github.com/dotnet/runtime/issues/88992", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoRuntime), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsX86Process))]
using Xunit;
4 changes: 1 addition & 3 deletions src/libraries/System.Runtime.InteropServices.RuntimeInformation/tests/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,4 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using Xunit;

[assembly: ActiveIssue("https://github.com/dotnet/runtime/issues/88992", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoRuntime), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsX86Process))]
using Xunit;
4 changes: 1 addition & 3 deletions src/libraries/System.Transactions.Local/tests/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,4 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using Xunit;

[assembly: ActiveIssue("https://github.com/dotnet/runtime/issues/88992", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoRuntime), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsX86Process))]
using Xunit;
1 change: 1 addition & 0 deletions src/mono/mono/eventpipe/ep-rt-mono-profiler-provider.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2965,6 +2965,7 @@ eventpipe_provider_callback (
}

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntimeMonoProfiler (
const uint8_t *source_id,
unsigned long is_enabled,
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4336,6 +4336,7 @@ gc_heap_dump_trigger_callback (MonoProfiler *prof)
}

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntime (
const uint8_t *source_id,
unsigned long is_enabled,
Expand Down
3 changes: 3 additions & 0 deletions src/mono/mono/eventpipe/ep-rt-mono.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ ep_rt_mono_fini (void)
}

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntimeRundown (
const uint8_t *source_id,
unsigned long is_enabled,
Expand All @@ -904,6 +905,7 @@ EventPipeEtwCallbackDotNETRuntimeRundown (
}

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntimePrivate (
const uint8_t *source_id,
unsigned long is_enabled,
Expand All @@ -919,6 +921,7 @@ EventPipeEtwCallbackDotNETRuntimePrivate (
}

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntimeStress (
const uint8_t *source_id,
unsigned long is_enabled,
Expand Down
5 changes: 5 additions & 0 deletions src/mono/mono/eventpipe/ep-rt-mono.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1920,6 +1920,7 @@ ep_rt_write_event_contention_stop (
*/

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntime (
const uint8_t *source_id,
unsigned long is_enabled,
Expand All @@ -1930,6 +1931,7 @@ EventPipeEtwCallbackDotNETRuntime (
void *callback_data);

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntimeRundown (
const uint8_t *source_id,
unsigned long is_enabled,
Expand All @@ -1940,6 +1942,7 @@ EventPipeEtwCallbackDotNETRuntimeRundown (
void *callback_data);

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntimePrivate (
const uint8_t *source_id,
unsigned long is_enabled,
Expand All @@ -1950,6 +1953,7 @@ EventPipeEtwCallbackDotNETRuntimePrivate (
void *callback_data);

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntimeStress (
const uint8_t *source_id,
unsigned long is_enabled,
Expand All @@ -1960,6 +1964,7 @@ EventPipeEtwCallbackDotNETRuntimeStress (
void *callback_data);

void
EP_CALLBACK_CALLTYPE
EventPipeEtwCallbackDotNETRuntimeMonoProfiler (
const uint8_t *source_id,
unsigned long is_enabled,
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/eventpipe/test/ep-provider-callback-dataqueue-tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ test_provider_callback_data_queue_setup (void)

static
void
EP_CALLBACK_CALLTYPE
provider_callback (
const uint8_t *source_id,
unsigned long is_enabled,
Expand Down
35 changes: 18 additions & 17 deletions src/mono/mono/eventpipe/test/ep-tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ test_enable_disable_default_provider_config (void)

test_location = 2;

result = validate_default_provider_config ((EventPipeSession *)session_id);
result = validate_default_provider_config ((EventPipeSession *)(uintptr_t)session_id);
ep_raise_error_if_nok (result == NULL);

test_location = 3;
Expand Down Expand Up @@ -409,7 +409,7 @@ test_enable_disable_multiple_default_provider_config (void)

test_location = 2;

result = validate_default_provider_config ((EventPipeSession *)session_id_1);
result = validate_default_provider_config ((EventPipeSession *)(uintptr_t)session_id_1);
ep_raise_error_if_nok (result == NULL);

test_location = 3;
Expand Down Expand Up @@ -441,7 +441,7 @@ test_enable_disable_multiple_default_provider_config (void)

test_location = 5;

result = validate_default_provider_config ((EventPipeSession *)session_id_2);
result = validate_default_provider_config ((EventPipeSession *)(uintptr_t)session_id_2);
ep_raise_error_if_nok (result == NULL);

test_location = 6;
Expand Down Expand Up @@ -491,7 +491,7 @@ test_enable_disable_provider_config (void)

test_location = 2;

EventPipeSessionProviderList *provider_list = ep_session_get_providers ((EventPipeSession *)session_id);
EventPipeSessionProviderList *provider_list = ep_session_get_providers ((EventPipeSession *)(uintptr_t)session_id);
EventPipeSessionProvider *session_provider = ep_session_provider_list_find_by_name (ep_session_provider_list_get_providers (provider_list), TEST_PROVIDER_NAME);
ep_raise_error_if_nok (session_provider != NULL);

Expand Down Expand Up @@ -572,7 +572,7 @@ test_enable_disable_provider_parse_default_config (void)

test_location = 2;

result = validate_default_provider_config ((EventPipeSession *)session_id);
result = validate_default_provider_config ((EventPipeSession *)(uintptr_t)session_id);
ep_raise_error_if_nok (result == NULL);

test_location = 3;
Expand All @@ -598,6 +598,7 @@ static bool provider_callback_data;

static
void
EP_CALLBACK_CALLTYPE
provider_callback (
const uint8_t *source_id,
unsigned long is_enabled,
Expand Down Expand Up @@ -795,7 +796,7 @@ test_session_write_event (void)

EventPipeEventPayload payload;;
ep_event_payload_init (&payload, NULL, 0);
write_result = ep_session_write_event ((EventPipeSession *)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
write_result = ep_session_write_event ((EventPipeSession *)(uintptr_t)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
ep_event_payload_fini (&payload);

ep_raise_error_if_nok (write_result == true);
Expand Down Expand Up @@ -848,14 +849,14 @@ test_session_write_event_seq_point (void)

EventPipeEventPayload payload;;
ep_event_payload_init (&payload, NULL, 0);
write_result = ep_session_write_event ((EventPipeSession *)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
write_result = ep_session_write_event ((EventPipeSession *)(uintptr_t)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
ep_event_payload_fini (&payload);

ep_raise_error_if_nok (write_result == true);

test_location = 5;

ep_session_write_sequence_point_unbuffered ((EventPipeSession *)session_id);
ep_session_write_sequence_point_unbuffered ((EventPipeSession *)(uintptr_t)session_id);

ep_on_exit:
ep_disable (session_id);
Expand Down Expand Up @@ -905,20 +906,20 @@ test_session_write_wait_get_next_event (void)

EventPipeEventPayload payload;;
ep_event_payload_init (&payload, NULL, 0);
write_result = ep_session_write_event ((EventPipeSession *)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
write_result = ep_session_write_event ((EventPipeSession *)(uintptr_t)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
ep_event_payload_fini (&payload);

ep_raise_error_if_nok (write_result == true);

test_location = 5;

EventPipeEventInstance *event_instance = ep_session_get_next_event ((EventPipeSession *)session_id);
EventPipeEventInstance *event_instance = ep_session_get_next_event ((EventPipeSession *)(uintptr_t)session_id);

ep_raise_error_if_nok (event_instance != NULL);

test_location = 6;

event_instance = ep_session_get_next_event ((EventPipeSession *)session_id);
event_instance = ep_session_get_next_event ((EventPipeSession *)(uintptr_t)session_id);

ep_raise_error_if_nok (event_instance == NULL);

Expand Down Expand Up @@ -970,14 +971,14 @@ test_session_write_get_next_event (void)

// Starts as signaled.
// TODO: Is this expected behavior, just a way to notify observer that we are up and running?
uint32_t test = ep_rt_wait_event_wait ((ep_rt_wait_event_handle_t *)ep_session_get_wait_event ((EventPipeSession *)session_id), 0, false);
uint32_t test = ep_rt_wait_event_wait ((ep_rt_wait_event_handle_t *)ep_session_get_wait_event ((EventPipeSession *)(uintptr_t)session_id), 0, false);
ep_raise_error_if_nok (test == 0);

test_location = 5;

EventPipeEventPayload payload;;
ep_event_payload_init (&payload, NULL, 0);
write_result = ep_session_write_event ((EventPipeSession *)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
write_result = ep_session_write_event ((EventPipeSession *)(uintptr_t)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
ep_event_payload_fini (&payload);

ep_raise_error_if_nok (write_result == true);
Expand All @@ -986,17 +987,17 @@ test_session_write_get_next_event (void)

// TODO: Is this really the correct behavior, first write signals event, meaning that buffer will converted to read only
// with just one event in it.
test = ep_rt_wait_event_wait ((ep_rt_wait_event_handle_t *)ep_session_get_wait_event ((EventPipeSession *)session_id), 0, false);
test = ep_rt_wait_event_wait ((ep_rt_wait_event_handle_t *)ep_session_get_wait_event ((EventPipeSession *)(uintptr_t)session_id), 0, false);
ep_raise_error_if_nok (test == 0);

test_location = 7;

EventPipeEventInstance *event_instance = ep_session_get_next_event ((EventPipeSession *)session_id);
EventPipeEventInstance *event_instance = ep_session_get_next_event ((EventPipeSession *)(uintptr_t)session_id);
ep_raise_error_if_nok (event_instance != NULL);

test_location = 8;

event_instance = ep_session_get_next_event ((EventPipeSession *)session_id);
event_instance = ep_session_get_next_event ((EventPipeSession *)(uintptr_t)session_id);
ep_raise_error_if_nok (event_instance == NULL);

ep_on_exit:
Expand Down Expand Up @@ -1047,7 +1048,7 @@ test_session_write_suspend_event (void)

EventPipeEventPayload payload;;
ep_event_payload_init (&payload, NULL, 0);
write_result = ep_session_write_event ((EventPipeSession *)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
write_result = ep_session_write_event ((EventPipeSession *)(uintptr_t)session_id, ep_rt_thread_get_handle (), ep_event, &payload, NULL, NULL, NULL, NULL);
ep_event_payload_fini (&payload);

ep_raise_error_if_nok (write_result == true);
Expand Down
10 changes: 9 additions & 1 deletion src/native/eventpipe/ds-ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,18 @@ ipc_log_poll_handles (dn_vector_t *ipc_poll_handles)
} DN_VECTOR_FOREACH_END;
}

static
bool
EP_CALLBACK_CALLTYPE
ipc_stream_factory_callback (void)
{
return ds_ipc_stream_factory_any_suspended_ports ();
}

bool
ds_ipc_stream_factory_init (void)
{
ep_ipc_stream_factory_callback_set (ds_ipc_stream_factory_any_suspended_ports);
ep_ipc_stream_factory_callback_set (ipc_stream_factory_callback);
_ds_port_array = dn_vector_ptr_alloc ();
return _ds_port_array != NULL;
}
Expand Down
11 changes: 8 additions & 3 deletions src/native/eventpipe/ep-types-forward.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,14 @@ typedef int64_t ep_system_timestamp_t;
/*
* EventPipe Callbacks.
*/
#if defined(_WIN32) && defined(_M_IX86)
#define EP_CALLBACK_CALLTYPE __stdcall
#else
#define EP_CALLBACK_CALLTYPE
#endif

// Define the event pipe callback to match the ETW callback signature.
typedef void (*EventPipeCallback)(
typedef void (EP_CALLBACK_CALLTYPE *EventPipeCallback)(
const uint8_t *source_id,
unsigned long is_enabled,
uint8_t level,
Expand All @@ -186,7 +191,7 @@ typedef void (*EventPipeCallback)(
EventFilterDescriptor *filter_data,
void *callback_data);

typedef void (*EventPipeSessionSynchronousCallback)(
typedef void (EP_CALLBACK_CALLTYPE *EventPipeSessionSynchronousCallback)(
EventPipeProvider *provider,
uint32_t event_id,
uint32_t event_version,
Expand All @@ -201,7 +206,7 @@ typedef void (*EventPipeSessionSynchronousCallback)(
uintptr_t *stack_frames,
void *additional_data);

typedef bool (*EventPipeIpcStreamFactorySuspendedPortsCallback)(void);
typedef bool (EP_CALLBACK_CALLTYPE *EventPipeIpcStreamFactorySuspendedPortsCallback)(void);

#endif /* ENABLE_PERFTRACING */
#endif /* __EVENTPIPE_TYPES_FORWARD_H__ */
8 changes: 8 additions & 0 deletions src/native/libs/Common/pal_compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ do_abort_unless (bool condition, const char* fmt, ...)
#endif
#endif // PALEXPORT

#ifndef PAL_CALLBACK_CALLTYPE
#if defined(_WIN32) && defined(_M_IX86)
#define PAL_CALLBACK_CALLTYPE __stdcall
#else
#define PAL_CALLBACK_CALLTYPE
#endif
#endif // PAL_CALLBACK_CALLTYPE

#ifndef EXTERN_C
#ifdef __cplusplus
#define EXTERN_C extern "C"
Expand Down
2 changes: 1 addition & 1 deletion src/native/libs/System.Globalization.Native/pal_calendarData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ typedef enum
} CalendarDataType;

// the function pointer definition for the callback used in EnumCalendarInfo
typedef void (*EnumCalendarInfoCallback)(const UChar*, const void*);
typedef void (PAL_CALLBACK_CALLTYPE *EnumCalendarInfoCallback)(const UChar*, const void*);

PALEXPORT int32_t GlobalizationNative_GetCalendars(const UChar* localeName,
CalendarId* calendars,
Expand Down
Loading