Closed Bug 787814 Opened 9 years ago Closed 9 years ago

Fix errors in SIPCC found by Valgrind of unittests

Categories

(Core :: WebRTC: Signaling, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: ehugg, Assigned: ehugg)

Details

(Keywords: valgrind, Whiteboard: [WebRTC], [blocking-webrtc+] [qa-])

Attachments

(2 files, 3 obsolete files)

Valgrind results for signaling_unittests on OSX



==49326== Thread 9:
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0xDFE7CA2: strcasecmp_l (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0x1000B2A71: CCAPI_CallInfo_getIsConference
(ccapi_call_info.c:522)
==49326==    by 0x1000BDCF8: ccappUpdateSessionData (ccprovider.c:1572)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Use of uninitialised value of size 8
==49326==    at 0xDFE7CA4: strcasecmp_l (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0x1000B2A71: CCAPI_CallInfo_getIsConference
(ccapi_call_info.c:522)
==49326==    by 0x1000BDCF8: ccappUpdateSessionData (ccprovider.c:1572)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC59C: partyInfoPassedTheNameFilter (call_logger.c:171)
==49326==    by 0x1000AC8B7: handlePlacedCall (call_logger.c:224)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDF05: ccappUpdateSessionData (ccprovider.c:1597)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC5B0: partyInfoPassedTheNameFilter (call_logger.c:171)
==49326==    by 0x1000AC8B7: handlePlacedCall (call_logger.c:224)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDF05: ccappUpdateSessionData (ccprovider.c:1597)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC48C: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDF05: ccappUpdateSessionData (ccprovider.c:1597)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4A0: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDF05: ccappUpdateSessionData (ccprovider.c:1597)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4B4: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDF05: ccappUpdateSessionData (ccprovider.c:1597)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4C8: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDF05: ccappUpdateSessionData (ccprovider.c:1597)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4DC: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDF05: ccappUpdateSessionData (ccprovider.c:1597)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4F0: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDF05: ccappUpdateSessionData (ccprovider.c:1597)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0xDFACBA8: __vfprintf (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFABEDA: vsnprintf_l (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0x1001DDBDC: notice_msg (cpr_darwin_stdio.c:181)
==49326==    by 0x1000BD9A1: ccappUpdateSessionData (ccprovider.c:1487)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0xDFADD7D: __vfprintf (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFABEDA: vsnprintf_l (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0x1001DDBDC: notice_msg (cpr_darwin_stdio.c:181)
==49326==    by 0x1000BD9A1: ccappUpdateSessionData (ccprovider.c:1487)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0xDFAC20F: __ultoa (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFADDB1: __vfprintf (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFABEDA: vsnprintf_l (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0x1001DDBDC: notice_msg (cpr_darwin_stdio.c:181)
==49326==    by 0x1000BD9A1: ccappUpdateSessionData (ccprovider.c:1487)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0xE579: strlen (in
/usr/local/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==49326==    by 0xDFAD8C2: __vfprintf (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFABEDA: vsnprintf_l (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0x100044952: CSFLogV (CSFLog.cpp:61)
==49326==    by 0x100044C1B: CSFLog (CSFLog.cpp:93)
==49326==    by 0x1001DDC32: notice_msg (cpr_darwin_stdio.c:187)
==49326==    by 0x1000BD9A1: ccappUpdateSessionData (ccprovider.c:1487)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC59C: partyInfoPassedTheNameFilter (call_logger.c:171)
==49326==    by 0x1000AC8B7: handlePlacedCall (call_logger.c:224)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDA66: ccappUpdateSessionData (ccprovider.c:1509)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC5B0: partyInfoPassedTheNameFilter (call_logger.c:171)
==49326==    by 0x1000AC8B7: handlePlacedCall (call_logger.c:224)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDA66: ccappUpdateSessionData (ccprovider.c:1509)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC48C: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDA66: ccappUpdateSessionData (ccprovider.c:1509)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4A0: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDA66: ccappUpdateSessionData (ccprovider.c:1509)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4B4: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDA66: ccappUpdateSessionData (ccprovider.c:1509)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4C8: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDA66: ccappUpdateSessionData (ccprovider.c:1509)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4DC: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDA66: ccappUpdateSessionData (ccprovider.c:1509)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
==49326== Conditional jump or move depends on uninitialised value(s)
==49326==    at 0x1000AC4F0: partyInfoPassedTheNumberFilter (call_logger.c:146)
==49326==    by 0x1000AC8D3: handlePlacedCall (call_logger.c:225)
==49326==    by 0x1000ACF80: calllogger_update (call_logger.c:334)
==49326==    by 0x1000BDA66: ccappUpdateSessionData (ccprovider.c:1509)
==49326==    by 0x1000BB0BA: ccp_handler (ccprovider.c:1886)
==49326==    by 0x1000BA511: CCApp_task (ccapp_task.c:199)
==49326==    by 0xDFB88BE: _pthread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==    by 0xDFBBB74: thread_start (in /usr/lib/system/libsystem_c.dylib)
==49326==
Assignee: nobody → ethanhugg
Attachment #657701 - Flags: feedback?(ekr)
Attachment #657701 - Attachment is obsolete: true
Attachment #657701 - Flags: feedback?(ekr)
Attachment #657707 - Flags: feedback?(ekr)
Comment on attachment 657707 [details] [diff] [review]
Fix errors in SIPCC found by Valgrind of unittests

Review of attachment 657707 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_call_info.c
@@ +515,5 @@
>    CCAPP_DEBUG(DEB_F_PREFIX"Entering\n", DEB_F_PREFIX_ARGS(SIP_CC_PROV, fname));
>  
> +  memset(isConf_buf, 0, sizeof(isConf_buf));
> +
> +  if(platGetPhraseText(CONFERENCE_LOCALE_CODE, isConf, sizeof(isConf_buf)) == CC_FAILURE){

I'm trying to figure out how the isConf variable is used... it looks like we just write to it. What have I missed?

::: media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c
@@ +987,5 @@
>  {
>     session_data_t *newData = (session_data_t *) cpr_malloc(sizeof(session_data_t));
>  
>     if ( newData != NULL ) {
> +       memset(newData, 0, sizeof(session_data_t));

Do you know which variable is the UMR here? I wonder if we need to give it a more specific initializer.
>::: media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_call_info.c
>@@ +515,5 @@
>>    CCAPP_DEBUG(DEB_F_PREFIX"Entering\n", DEB_F_PREFIX_ARGS(SIP_CC_PROV, fname));
>>  
>> +  memset(isConf_buf, 0, sizeof(isConf_buf));
>> +
>> +  if(platGetPhraseText(CONFERENCE_LOCALE_CODE, isConf, sizeof(isConf_buf)) == >CC_FAILURE){
>
>I'm trying to figure out how the isConf variable is used... it looks like we just >write to it. What have I missed?

It's compared right below with a strcasecmp.  What is silly though is that isConf_bug and isConf are essentially the same, I should probably get rid of the pointer version.

Also down the rabbit-hole is the fact that platGetPhraseText is being pulled from the stub version which doesn't set isConf which is why it was uninitialized.  Adding to the silliness, is the version that Linux uses in ccapi_plat_api_impl.cpp doesn't do anything either except set it to a bunch of question marks.  It's called 20 times or so, I'm tempted to go in and tear it out, but thought that might be out-of-scope for this bug.



>::: media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c
>@@ +987,5 @@
>>  {
>>     session_data_t *newData = (session_data_t *) >cpr_malloc(sizeof(session_data_t));
>>  
>>     if ( newData != NULL ) {
>> +       memset(newData, 0, sizeof(session_data_t));
>
>Do you know which variable is the UMR here? I wonder if we need to give it a more >specific initializer.

attr and inst are the two integer values that are not being set here that throw the valgrind error in a debug message.  There are other int values as well in this data structure that were not being set on copy.  Despite the name of the function saying 'getDeepCopy' it doesn't copy everything, and the init version already had the memset.  

This session_data_t has 44 data members, I could copy them all, but I'm not sure yet if there was some reason for not copying them all, with the obvious exception of ref_count.  I suppose I could try it and see what happens.
OK. This all sounds reasonable.
Attachment #657707 - Attachment is obsolete: true
Attachment #657707 - Flags: feedback?(ekr)
Comment on attachment 657770 [details] [diff] [review]
Fix errors in SIPCC found by Valgrind of unittests


Pushed to Alder - http://hg.mozilla.org/projects/alder/rev/aaaff7c6a473
Whiteboard: [WebRTC]
Whiteboard: [WebRTC] → [WebRTC], [blocking-webrtc+]
This appears resolved and is in alder
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
More Valgrind errors still in:

(Patch fixed partyInfoPassedTheNameFilter only, missed this one)
==13812== Conditional jump or move depends on uninitialised value(s)
==13812==    at 0x4C21D4: partyInfoPassedTheNumberFilter (media/webrtc/signaling/src/sipcc/core/ccapp/call_logger.c:146)
==13812==    by 0x4C252F: handlePlacedCall (media/webrtc/signaling/src/sipcc/core/ccapp/call_logger.c:226)
==13812==    by 0x4C2924: calllogger_update (media/webrtc/signaling/src/sipcc/core/ccapp/call_logger.c:335)
==13812==    by 0x4CCF26: ccappUpdateSessionData (media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1594)
==13812==    by 0x4CF13D: ccp_handler (media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1883)
==13812==    by 0x4CB5B5: CCApp_task (media/webrtc/signaling/src/sipcc/core/ccapp/ccapp_task.c:199)
==13812==    by 0x4E3AE99: start_thread (/build/buildd/eglibc-2.15/nptl/pthread_create.c:308)
==13812==    by 0x77984BC: clone (/build/buildd/eglibc-2.15/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:112)
==13812==  Uninitialised value was created by a heap allocation
==13812==    at 0x4C2C62F: malloc (/home/sewardj/Vg38BRANCH/branch38/coregrind/m_replacemalloc/vg_replace_malloc.c:270)
==13812==    by 0x585ECE: cpr_malloc (media/webrtc/signaling/src/sipcc/cpr/linux/cpr_linux_stdlib.c:485)
==13812==    by 0x5817BE: strlib_malloc (media/webrtc/signaling/src/sipcc/core/src-common/string_lib.c:88)
==13812==    by 0x581A36: strlib_empty (media/webrtc/signaling/src/sipcc/core/src-common/string_lib.c:409)
==13812==    by 0x4C98E4: ccsnap_set_line_label (media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_snapshot.c:74)
==13812==    by 0x4D1EE0: config_setup_elements (media/webrtc/signaling/src/sipcc/core/common/config_parser.c:492)
==13812==    by 0x4D2417: config_setup_main (media/webrtc/signaling/src/sipcc/core/common/config_parser.c:662)
==13812==    by 0x4C6DB9: parse_setup_properties (media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_config.c:90)
==13812==    by 0x4C6EB7: CCAPI_Start_response (media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_config.c:76)
==13812==    by 0x4ABD34: configCtlFetchReq (media/webrtc/signaling/src/softphonewrapper/CC_SIPCCService.cpp:138)
==13812==    by 0x4AC5DD: configFetchReq (media/webrtc/signaling/src/softphonewrapper/CC_SIPCCService.cpp:162)
==13812==    by 0x4C4BD3: registration_processEvent (media/webrtc/signaling/src/sipcc/core/ccapp/cc_device_manager.c:298)
==13812==

(This one appears to be new)

==13812== Syscall param msgsnd(msgp->mtype) points to uninitialised byte(s)
==13812==    at 0x779A4A3: ??? (/build/buildd/eglibc-2.15/sysvipc/../sysdeps/unix/syscall-template.S:82)
==13812==    by 0x583B5B: cprPostMessage (media/webrtc/signaling/src/sipcc/cpr/linux/cpr_linux_ipc.c:808)
==13812==    by 0x5844AD: cprSendMessage (media/webrtc/signaling/src/sipcc/cpr/linux/cpr_linux_ipc.c:669)
==13812==    by 0x4CB468: ccappTaskSendMsg (media/webrtc/signaling/src/sipcc/core/ccapp/ccapp_task.c:162)
==13812==    by 0x4CB4F5: ccappTaskPostMsg (media/webrtc/signaling/src/sipcc/core/ccapp/ccapp_task.c:129)
==13812==    by 0x4C452C: cc_invokeDeviceFeature (media/webrtc/signaling/src/sipcc/core/ccapp/cc_device_feature.c:52)
==13812==    by 0x4C45D2: CC_DeviceFeature_enableVideo (media/webrtc/signaling/src/sipcc/core/ccapp/cc_device_feature.c:84)
==13812==    by 0x4C706A: CCAPI_Device_enableVideo (media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_device.c:161)
==13812==    by 0x49F0B1: CSF::CC_SIPCCDevice::enableVideo(bool) (media/webrtc/signaling/src/softphonewrapper/CC_SIPCCDevice.cpp:114)
==13812==    by 0x49F1EA: CSF::CC_SIPCCDevice::CC_SIPCCDevice(unsigned int) (media/webrtc/signaling/src/softphonewrapper/CC_SIPCCDevice.cpp:75)
==13812==    by 0x49F304: CSF::CC_SIPCCDevice::wrap(unsigned int) (media/webrtc/signaling//./src/common/Wrapper.h:98)
==13812==    by 0x4AACD5: CSF::CC_SIPCCService::onDeviceEvent(ccapi_device_event_e, unsigned int, cc_device_info_t_*) (media/webrtc/signaling/src/softphonewrapper/CC_SIPCCService.cpp:645)
==13812==  Address 0x7feffef24 is on thread 1's stack
==13812==  Uninitialised value was created by a stack allocation
==13812==    at 0x583B26: cprPostMessage (media/webrtc/signaling/src/sipcc/cpr/linux/cpr_linux_ipc.c:788)
==13812==

(this one must not have been fixed completely)

==13812== Thread 7:
==13812== Conditional jump or move depends on uninitialised value(s)
==13812==    at 0x76ED98E: vfprintf (/build/buildd/eglibc-2.15/stdio-common/vfprintf.c:1623)
==13812==    by 0x77AD57F: __vsnprintf_chk (/build/buildd/eglibc-2.15/debug/vsnprintf_chk.c:65)
==13812==    by 0x58583D: notice_msg (/usr/include/x86_64-linux-gnu/bits/stdio2.h:78)
==13812==    by 0x4CCFF3: ccappUpdateSessionData (media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1484)
==13812==    by 0x4CF13D: ccp_handler (media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1883)
==13812==    by 0x4CB5B5: CCApp_task (media/webrtc/signaling/src/sipcc/core/ccapp/ccapp_task.c:199)
==13812==    by 0x4E3AE99: start_thread (/build/buildd/eglibc-2.15/nptl/pthread_create.c:308)
==13812==    by 0x77984BC: clone (/build/buildd/eglibc-2.15/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:112)
==13812==  Uninitialised value was created by a stack allocation
==13812==    at 0x4D8573: ui_create_offer (media/webrtc/signaling/src/sipcc/core/common/ui.c:1610)
==13812==
==13812== Use of uninitialised value of size 8
==13812==    at 0x76ED4AB: _itoa_word (/build/buildd/eglibc-2.15/stdio-common/_itoa.c:195)
==13812==    by 0x76F1FDA: vfprintf (/build/buildd/eglibc-2.15/stdio-common/vfprintf.c:1623)
==13812==    by 0x77AD57F: __vsnprintf_chk (/build/buildd/eglibc-2.15/debug/vsnprintf_chk.c:65)
==13812==    by 0x58583D: notice_msg (/usr/include/x86_64-linux-gnu/bits/stdio2.h:78)
==13812==    by 0x4CCFF3: ccappUpdateSessionData (media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1484)
==13812==    by 0x4CF13D: ccp_handler (media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1883)
==13812==    by 0x4CB5B5: CCApp_task (media/webrtc/signaling/src/sipcc/core/ccapp/ccapp_task.c:199)
==13812==    by 0x4E3AE99: start_thread (/build/buildd/eglibc-2.15/nptl/pthread_create.c:308)
==13812==    by 0x77984BC: clone (/build/buildd/eglibc-2.15/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:112)
==13812==  Uninitialised value was created by a stack allocation
==13812==    at 0x4D8573: ui_create_offer (media/webrtc/signaling/src/sipcc/core/common/ui.c:1610)
==13812==
==13812== Conditional jump or move depends on uninitialised value(s)
==13812==    at 0x76ED4B5: _itoa_word (/build/buildd/eglibc-2.15/stdio-common/_itoa.c:195)
==13812==    by 0x76F1FDA: vfprintf (/build/buildd/eglibc-2.15/stdio-common/vfprintf.c:1623)
==13812==    by 0x77AD57F: __vsnprintf_chk (/build/buildd/eglibc-2.15/debug/vsnprintf_chk.c:65)
==13812==    by 0x58583D: notice_msg (/usr/include/x86_64-linux-gnu/bits/stdio2.h:78)
==13812==    by 0x4CCFF3: ccappUpdateSessionData (media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1484)
==13812==    by 0x4CF13D: ccp_handler (media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1883)
==13812==    by 0x4CB5B5: CCApp_task (media/webrtc/signaling/src/sipcc/core/ccapp/ccapp_task.c:199)
==13812==    by 0x4E3AE99: start_thread (/build/buildd/eglibc-2.15/nptl/pthread_create.c:308)
==13812==    by 0x77984BC: clone (/build/buildd/eglibc-2.15/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:112)
==13812==  Uninitialised value was created by a stack allocation
==13812==    at 0x4D8573: ui_create_offer (media/webrtc/signaling/src/sipcc/core/common/ui.c:1610)
==13812==

(Possibly an artifact of the test framework)
==13812== Mismatched free() / delete / delete []
==13812==    at 0x4C2B599: free (/home/sewardj/Vg38BRANCH/branch38/coregrind/m_replacemalloc/vg_replace_malloc.c:446)
==13812==    by 0x4052012: moz_free (memory/mozalloc/mozalloc.cpp:51)
==13812==    by 0x457985: test::SignalingTest::OfferAnswer(char const*, char const*) (ff-opt/media/webrtc/signaling/test/../../../../dist/include/mozilla/mozalloc.h:236)
==13812==    by 0x457E7E: test::SignalingTest_OfferAnswer_Test::TestBody() (media/webrtc/signaling/test/signaling_unittests.cpp:698)
==13812==    by 0xB1F0C1: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2090)
==13812==    by 0xB1FD3A: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2142)
==13812==    by 0xB28A74: testing::Test::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2162)
==13812==    by 0xB28B4F: testing::TestInfo::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2338)
==13812==    by 0xB28C89: testing::TestCase::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2445)
==13812==    by 0xB28FA7: testing::internal::UnitTestImpl::RunAllTests() (media/webrtc/trunk/testing/gtest/src/gtest.cc:4237)
==13812==    by 0xB1F0C1: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2090)
==13812==    by 0xB1FD3A: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2142)
==13812==  Address 0x154c4160 is 0 bytes inside a block of size 45 alloc'd
==13812==    at 0x4C2BA97: operator new[](unsigned long) (/home/sewardj/Vg38BRANCH/branch38/coregrind/m_replacemalloc/vg_replace_malloc.c:363)
==13812==    by 0xB111BA: testing::internal::String::String(char const*) (media/webrtc/trunk/testing//gtest/include/gtest/internal/gtest-string.h:303)
==13812==    by 0xB2199F: testing::internal::StringStreamToString(std::basic_stringstream<char, std::char_traits<char>, std::allocator<char> >*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:1751)
==13812==    by 0xB24FB3: testing::internal::GetBoolAssertionFailureMessage(testing::AssertionResult const&, char const*, char const*, char const*) (media/webrtc/trunk/testing//gtest/include/gtest/gtest-message.h:191)
==13812==    by 0x457931: test::SignalingTest::OfferAnswer(char const*, char const*) (media/webrtc/signaling/test/signaling_unittests.cpp:618)
==13812==    by 0x457E7E: test::SignalingTest_OfferAnswer_Test::TestBody() (media/webrtc/signaling/test/signaling_unittests.cpp:698)
==13812==    by 0xB1F0C1: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2090)
==13812==    by 0xB1FD3A: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2142)
==13812==    by 0xB28A74: testing::Test::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2162)
==13812==    by 0xB28B4F: testing::TestInfo::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2338)
==13812==    by 0xB28C89: testing::TestCase::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2445)
==13812==    by 0xB28FA7: testing::internal::UnitTestImpl::RunAllTests() (media/webrtc/trunk/testing/gtest/src/gtest.cc:4237)

==13812== Conditional jump or move depends on uninitialised value(s)
==13812==    at 0x9A2022: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (/usr/include/c++/4.6/bits/basic_string.tcc:130)
==13812==    by 0x6F365E2: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.16)
==13812==    by 0x457752: test::SignalingTest::OfferAnswer(char const*, char const*) (media/webrtc/signaling/test/signaling_unittests.cpp:613)
==13812==    by 0x457B37: test::SignalingTest_FullCall_Test::TestBody() (media/webrtc/signaling/test/signaling_unittests.cpp:704)
==13812==    by 0xB1F0C1: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2090)
==13812==    by 0xB1FD3A: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2142)
==13812==    by 0xB28A74: testing::Test::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2162)
==13812==    by 0xB28B4F: testing::TestInfo::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2338)
==13812==    by 0xB28C89: testing::TestCase::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2445)
==13812==    by 0xB28FA7: testing::internal::UnitTestImpl::RunAllTests() (media/webrtc/trunk/testing/gtest/src/gtest.cc:4237)
==13812==    by 0xB1F0C1: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2090)
==13812==    by 0xB1FD3A: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2142)
==13812==  Uninitialised value was created by a heap allocation
==13812==    at 0x4C2C62F: malloc (/home/sewardj/Vg38BRANCH/branch38/coregrind/m_replacemalloc/vg_replace_malloc.c:270)
==13812==    by 0x4052028: moz_xmalloc (memory/mozalloc/mozalloc.cpp:57)
==13812==    by 0x455CE4: testing::internal::TestFactoryImpl<test::SignalingTest_FullCall_Test>::CreateTest() (ff-opt/media/webrtc/signaling/test/../../../../dist/include/mozilla/mozalloc.h:200)
==13812==    by 0xB1F0C1: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2090)
==13812==    by 0xB1FD3A: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2142)
==13812==    by 0xB28B36: testing::TestInfo::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2331)
==13812==    by 0xB28C89: testing::TestCase::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:2445)
==13812==    by 0xB28FA7: testing::internal::UnitTestImpl::RunAllTests() (media/webrtc/trunk/testing/gtest/src/gtest.cc:4237)
==13812==    by 0xB1F0C1: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2090)
==13812==    by 0xB1FD3A: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (media/webrtc/trunk/testing/gtest/src/gtest.cc:2142)
==13812==    by 0xB1FE15: testing::UnitTest::Run() (media/webrtc/trunk/testing/gtest/src/gtest.cc:3874)
==13812==    by 0x4511E5: main (media/webrtc/signaling/test/signaling_unittests.cpp:761)
==13812==
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 661858 [details] [diff] [review]
Patch 2 Fix errors in SIPCC found by Valgrind of unittests


Here's a patch to the two most common ones.

partyInfoPassedTheNumberFilter was missed earlier when partyInfoPassedTheNameFilter was fixed.

The problem in ccappTaskPostMsg I believe was caused by a recent patch that made signaling use malloc/free directly.  This allocation was effectively changed from calloc to malloc so was not being initialized.
Attachment #661858 - Flags: feedback?(rjesup)
Comment on attachment 661858 [details] [diff] [review]
Patch 2 Fix errors in SIPCC found by Valgrind of unittests


Second patch is wrong, going to re-try.
Attachment #661858 - Flags: feedback?(rjesup)
Attachment #661858 - Attachment is obsolete: true
Comment on attachment 661891 [details] [diff] [review]
Patch 2 Fix errors in SIPCC found by Valgrind of unittests


Here's a patch for the most common valgrind error.  partyInfoPassedTheNumberFilter was missed earlier when partyInfoPassedTheNameFilter was fixed.

I'm not sure about the others yet.  I'm tempted to take the log message out of ccappUpdateSessionData and see if the problem reappears somewhere else, I don't see out the session_data_t could created without being initialized.
Attachment #661891 - Flags: feedback?(rjesup)
Comment on attachment 661891 [details] [diff] [review]
Patch 2 Fix errors in SIPCC found by Valgrind of unittests

f+, though that's some darn freaky code.... haven't people heard of '+' or '\t' or even using 0xXX with a comment?  But in any case f+, and I see no reason to improve the code there at this point beyond this.
Attachment #661891 - Flags: feedback?(rjesup) → feedback+
Yes.  I considered cleaning it up, but I think I'll be able to remove it entirely later.
Comment on attachment 661891 [details] [diff] [review]
Patch 2 Fix errors in SIPCC found by Valgrind of unittests


Pushed patch 2 to Alder branch - http://hg.mozilla.org/projects/alder/rev/5064358f445a
Pushed from Alder to M-I in the review-rollup (Bug 792188).
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Keywords: valgrind
Target Milestone: --- → mozilla18
Whiteboard: [WebRTC], [blocking-webrtc+] → [WebRTC], [blocking-webrtc+] [qa-]
You need to log in before you can comment on or make changes to this bug.