Skip to content

Conversation

@am11
Copy link
Member

@am11 am11 commented Sep 9, 2021

An example of before/after diff:

--- artifacts/obj/coreclr/Linux.x64.Debug/vm/eventing/eventpipe/eventpipe/dotnetruntimeprivate.cpp
+++ artifacts/obj/coreclr/Linux.x64.Debug/vm/eventing/eventpipe/eventpipe/dotnetruntimeprivate.cpp

extern "C" ULONG EventPipeWriteEventExecExeEnd(
    LPCGUID ActivityId,
    LPCGUID RelatedActivityId)
{
    if (!EventPipeEventEnabledExecExeEnd())
        return ERROR_SUCCESS;

+     size_t size = 32;
+     BYTE stackBuffer[32];
-    BYTE stackBuffer[32];
    BYTE *buffer = stackBuffer;
    size_t offset = 0;
-    size_t size = 32;
-    bool fixedBuffer = true;
-    bool success = true;


-
-    if (!success)
-    {
-        if (!fixedBuffer)
-            delete[] buffer;
-        return ERROR_WRITE_FAULT;
-    }

    EventPipeAdapter::WriteEvent(EventPipeEventExecExeEnd, (BYTE *)buffer, (unsigned int)offset, ActivityId, RelatedActivityId);
-
-    if (!fixedBuffer)
-        delete[] buffer;

    return ERROR_SUCCESS;
}

This is to cleanup the bulk of warnings from gcc 11 by commenting out

add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-mismatched-new-delete>)
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-free-nonheap-object>)
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-placement-new>)
so we can see the real -Werror=mismatched-new-delete in coreclr/vm that is causing #56888 when DOTNET_EnableEventLog is enabled with debug corerun, e.g.

[ 29%] Building CXX object vm/wks/CMakeFiles/cee_wks_core.dir/__/clsload.cpp.o
/runtime/src/coreclr/vm/ceeload.cpp: In member function 'void Module::InitializeDynamicILCrst()':
/runtime/src/coreclr/vm/ceeload.cpp:1442:16: error: 'void operator delete(void*)' called on pointer returned from a mismatched allocation function [-Werror=mismatched-new-delete]
 1442 |         delete pCrst;
      |                ^~~~~
/runtime/src/coreclr/vm/ceeload.cpp:1438:97: note: returned from 'static void* Crst::operator new(size_t)'
 1438 |     Crst * pCrst = new Crst(CrstDynamicIL, CrstFlags(CRST_UNSAFE_ANYMODE | CRST_DEBUGGER_THREAD));
      |                                                                                                 ^

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-Tracing-coreclr labels Sep 9, 2021
@am11
Copy link
Member Author

am11 commented Sep 9, 2021

cc @josalem, @janvorli

@am11 am11 force-pushed the feature/address-native-warnings branch from 0e76d10 to b22bae7 Compare September 9, 2021 13:38
@am11 am11 changed the title Skip dead conditions in eventpipe codegen Skip dead conditions in eventpipe srcgen Sep 9, 2021
@am11 am11 force-pushed the feature/address-native-warnings branch 2 times, most recently from 8533bd4 to e23820e Compare September 9, 2021 14:55
@josalem
Copy link
Contributor

josalem commented Sep 9, 2021

Thanks for this PR @am11!

@josalem josalem merged commit 4d4aa2c into dotnet:main Sep 9, 2021
@am11 am11 deleted the feature/address-native-warnings branch September 9, 2021 20:47
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants