The default bug view has changed. See this FAQ.

Fix errors in SIPCC found by Valgrind of unittests

RESOLVED FIXED in mozilla18

Status

()

Core
WebRTC: Signaling
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ehugg, Assigned: ehugg)

Tracking

({valgrind})

Trunk
mozilla18
valgrind
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

5 years ago
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)

Updated

5 years ago
Assignee: nobody → ethanhugg
(Assignee)

Comment 1

5 years ago
Created attachment 657701 [details] [diff] [review]
Fix errors in SIPCC found by Valgrind of unittests
(Assignee)

Updated

5 years ago
Attachment #657701 - Flags: feedback?(ekr)
(Assignee)

Comment 2

5 years ago
Created attachment 657707 [details] [diff] [review]
Fix errors in SIPCC found by Valgrind of unittests
(Assignee)

Updated

5 years ago
Attachment #657701 - Attachment is obsolete: true
Attachment #657701 - Flags: feedback?(ekr)
(Assignee)

Updated

5 years ago
Attachment #657707 - Flags: feedback?(ekr)

Comment 3

5 years ago
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.
(Assignee)

Comment 4

5 years ago
>::: 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.

Comment 5

5 years ago
OK. This all sounds reasonable.
(Assignee)

Comment 6

5 years ago
Created attachment 657770 [details] [diff] [review]
Fix errors in SIPCC found by Valgrind of unittests
(Assignee)

Updated

5 years ago
Attachment #657707 - Attachment is obsolete: true
Attachment #657707 - Flags: feedback?(ekr)
(Assignee)

Comment 7

5 years ago
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

Updated

5 years ago
Whiteboard: [WebRTC]

Updated

5 years ago
Whiteboard: [WebRTC] → [WebRTC], [blocking-webrtc+]
This appears resolved and is in alder
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

5 years ago
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 → ---
(Assignee)

Comment 10

5 years ago
Created attachment 661858 [details] [diff] [review]
Patch 2 Fix errors in SIPCC found by Valgrind of unittests
(Assignee)

Comment 11

5 years ago
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)
(Assignee)

Comment 12

5 years ago
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)
(Assignee)

Updated

5 years ago
Attachment #661858 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Created attachment 661891 [details] [diff] [review]
Patch 2 Fix errors in SIPCC found by Valgrind of unittests
(Assignee)

Comment 14

5 years ago
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+
(Assignee)

Comment 16

5 years ago
Yes.  I considered cleaning it up, but I think I'll be able to remove it entirely later.
(Assignee)

Comment 17

5 years ago
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
(Assignee)

Comment 18

5 years ago
Pushed from Alder to M-I in the review-rollup (Bug 792188).
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Keywords: valgrind
Target Milestone: --- → mozilla18

Updated

5 years ago
Whiteboard: [WebRTC], [blocking-webrtc+] → [WebRTC], [blocking-webrtc+] [qa-]
You need to log in before you can comment on or make changes to this bug.