Bug hunting with Valgrind

This is a discussion on Bug hunting with Valgrind within the C Programming forums, part of the General Programming Boards category; I'm using Valgrind to find if there's bug in my program (i'm coding under linux) and it find someting, but ...

  1. #1
    Registered User
    Join Date
    Nov 2008
    Posts
    14

    Bug hunting with Valgrind

    I'm using Valgrind to find if there's bug in my program (i'm coding under linux) and it find someting, but i don't understand where's the problem.

    Valgrind sais:
    ==5397== Thread 2:
    ==5397== Conditional jump or move depends on uninitialised value(s)
    ==5397== at 0x401358: ListSearch (packethandler.c:97)
    ==5397== by 0x40177D: HandlePackets (packetreader.c:47)
    ==5397== by 0x401643: Listen (packethandler.c:146)
    ==5397== by 0x4E2BFC6: start_thread (in /lib/libpthread-2.7.so)
    ==5397== by 0x51105AC: clone (in /lib/libc-2.7.so)
    ==5397==
    ==5397== Jump to the invalid address stated on the next line
    ==5397== at 0x0: ???
    ==5397== by 0x401643: Listen (packethandler.c:146)
    ==5397== by 0x4E2BFC6: start_thread (in /lib/libpthread-2.7.so)
    ==5397== by 0x51105AC: clone (in /lib/libc-2.7.so)
    ==5397== Address 0x0 is not stack'd, malloc'd or (recently) free'd
    ==5397==
    ==5397== Process terminating with default action of signal 11 (SIGSEGV)
    ==5397== Bad permissions for mapped region at address 0x0
    ==5397== at 0x0: ???
    ==5397== by 0x401643: Listen (packethandler.c:146)
    ==5397== by 0x4E2BFC6: start_thread (in /lib/libpthread-2.7.so)
    ==5397== by 0x51105AC: clone (in /lib/libc-2.7.so)
    --5397-- REDIR: 0x50b69e0 (free) redirected to 0x4c20b00 (free)
    For the first error valgrind is referring to that conditional jump:
    Code:
    RecvPacketList *ListSearch(RecvPacketList *plist, unsigned char i)
    {
            while (plist != NULL)<- THIS ONE
            {
                    if (plist->ID == i)
                    {
                            return plist;
                    }
                    plist = plist->next;
            }
            return NULL;
    }
    ListSearch is called here:

    Code:
    void HandlePackets(RecvPacketList **plist, char *recvBuff)
    {	
    	unsigned int ID = -1;
    	RecvPacketList *list;
    	while(ID != 0)
    	{
    		ID = (unsigned int)*recvBuff;
    		recvBuff++;
    		list = ListSearch(*plist, ID);
    		if(list != NULL){
    			pFunction pfunc = list->func;
    			(*pfunc)(&recvBuff);
    		}
    	}
    }
    And going again "back", HandlePackets is called here:

    Code:
    void *Listen(void *args)
    {
    	char recvPacketBuff[1024] = {0};
    	RecvPacketList *plist;
    	fd_set fset;
    	FD_ZERO(&fset);
    	printf("Initializing Packet List: ");
    	Initialize(&plist);
    	printf("Done\n");
    	
    	while(1)
    	{
    		FD_SET(sock_fd, &fset);
    		select(sock_fd + 1, &fset, NULL, NULL, NULL);
    		if(FD_ISSET(sock_fd, &fset)){
    			recv(sock_fd, &recvPacketBuff, sizeof(recvPacketBuff), 0);
    			HandlePackets(&plist, recvPacketBuff); <- THE SECOND ERROR, ln 146
    			CleanBuff(recvPacketBuff);
    		}
    	}
    }
    And as you can see here.. plist IS initialized:

    Code:
    void Initialize(RecvPacketList **plist)
    {
    	pFunction temp = NULL;
    	*plist = malloc(sizeof(RecvPacketList));
    	if(*plist == NULL){
    		printf("Error to allocate space for plist");
    		exit(1);
    	}
    	(*plist)->ID = 0;
    	(*plist)->func = NULL;
    	Register(0x0B,plist, temp = DamagePacket);	
    	Register(0x24,plist, temp = DisplayBuyList);
    	Register(0x25,plist, temp = TradeEquipOp);
    	Register(0x3C,plist, temp = VendorBuyContentOp);
    	Register(0x6F,plist, temp = SecureTradeOp);
    	Register(0x74,plist, temp = VendorBuyList);
    	Register(0x82,plist, temp = AccountLoginRej);
    	Register(0xA8,plist, temp = AccountLoginAck);
    	Register(0xBA,plist, temp = ArrowOp);
    	Register(0xB7,plist, temp = ObjectHelpResponse);
    }
    If you want also to see register:

    Code:
    void Register(unsigned char value, RecvPacketList **plist,  pFunction func)
    {
    	RecvPacketList *nextlist = (RecvPacketList *)malloc(sizeof(RecvPacketList));
    
    	if(nextlist == NULL){
    		printf("Memory allocation failed for nextlist");
    		return;
    	}
     	nextlist->next = *plist;
    	*plist = nextlist;
    	nextlist->ID = value;
    	nextlist->func = func;
    
    	
    }
    Anyway plist is a linked list that contains "static" packet informations (packet to be read).
    I also highlighted where's the second error.

  2. #2
    Registered User C_ntua's Avatar
    Join Date
    Jun 2008
    Posts
    1,853
    edit: read it more carefully. Hmmm. Well, dunno yet, but putting plist as a ** and a * was confusing and I really see no reason on having a ** when you use it as a *
    Last edited by C_ntua; 11-04-2008 at 05:45 PM.

  3. #3
    Registered User
    Join Date
    Nov 2008
    Posts
    14
    But it's initialized!
    Maybe it's difficult to understand the order of the execution so:

    All starts from Listen function (called by a new thread created), as you can see there is the "real" RecvPacketList *plist.
    Now i need to initialize this pointer to a structure so i pass it byreference through function Initialize and the function accept a pointer to pointer to structure so i'm sure i'm modifying that pointer and not a copied one.
    In the function it allocates memory for that pointer, it set ID to 0 and func to NULL, then Register function appends other RecvPacketList structures (to create the linked list) so the last structure it link is the first i can encounter, and it's intialized as you can see:
    Code:
    RecvPacketList *nextlist = (RecvPacketList *)malloc(sizeof(RecvPacketList));
    
    	if(nextlist == NULL){
    		printf("Memory allocation failed for nextlist");
    		return;
    	}
     	nextlist->next = *plist;
    	*plist = nextlist;
    	nextlist->ID = value;
    	nextlist->func = func;
    After initialization we are back in Listen function and it pass plist (the "real" one, now it's no more the pointer pointing to the space allocated in Initialize function but the pointer to the space allocated in the last Register) by reference through HandlePackets, so i'm sure again is that pointer and not another one, and then it pass it to ListSearch.

    I also debugged with Kdevelop and gdb and i can see that plist in ListSearch is initialized because has values etc..
    Last edited by Smjert; 11-04-2008 at 05:53 PM.

  4. #4
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,767
    > In the function it allocates memory for that pointer, it set ID to 0 and func to NULL
    But it doesn't set ->next to NULL (which is what you eventually get to test, and valgrind objects to)

    > then Register function appends other RecvPacketList structures
    Being pedantic, it prepends to the front of the list, not appends to the end.

    > Register(0x0B,plist, temp = DamagePacket);
    Do all these temp = do anything useful?
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  5. #5
    Registered User
    Join Date
    Nov 2008
    Posts
    14
    Quote Originally Posted by Salem View Post
    > In the function it allocates memory for that pointer, it set ID to 0 and func to NULL
    But it doesn't set ->next to NULL (which is what you eventually get to test, and valgrind objects to)
    Oh lol it was so simple XD, but i thought valgrind was referring to the last object i added to the list :\

    Quote Originally Posted by Salem View Post
    > then Register function appends other RecvPacketList structures
    Being pedantic, it prepends to the front of the list, not appends to the end.
    Don't worry you're not pedantic :P and i'm not english, i chose the wrong word ^^

    Quote Originally Posted by Salem View Post
    > Register(0x0B,plist, temp = DamagePacket);
    Do all these temp = do anything useful?
    No, it was an old try, when i wasn't sure if had to pass the function by name or pass it as a pointer (so using a variable).
    I removed it.
    Aniway there's still the second problem...

  6. #6
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,767
    So what's the second problem?

    If you're referring to the "Jump to the invalid address stated on the next line", well that's the code falling off the end of the broken list.
    If the list is fixed, then the code won't jump off into hyperspace.

    Fresh code + fresh trace if you've still got problems.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  7. #7
    Registered User
    Join Date
    Nov 2008
    Posts
    14
    Yes i'm referring to the jump, and i still have that problem, i only changed a line to solve the first problem.

    This:
    Code:
    void Initialize(RecvPacketList **plist)
    {
    	pFunction temp = NULL;
    	*plist = malloc(sizeof(RecvPacketList));
    	if(*plist == NULL){
    		printf("Error to allocate space for plist");
    		exit(1);
    	}
    	(*plist)->ID = 0;
    	(*plist)->func = NULL;
    	Register(0x0B,plist, temp = DamagePacket);	
    	Register(0x24,plist, temp = DisplayBuyList);
    	Register(0x25,plist, temp = TradeEquipOp);
    	Register(0x3C,plist, temp = VendorBuyContentOp);
    	Register(0x6F,plist, temp = SecureTradeOp);
    	Register(0x74,plist, temp = VendorBuyList);
    	Register(0x82,plist, temp = AccountLoginRej);
    	Register(0xA8,plist, temp = AccountLoginAck);
    	Register(0xBA,plist, temp = ArrowOp);
    	Register(0xB7,plist, temp = ObjectHelpResponse);
            etc....................................
    }
    Now is (i also removed the useless temp variable ^^):
    Code:
    void Initialize(RecvPacketList **plist)
    {
    	*plist = malloc(sizeof(RecvPacketList));
    	if(*plist == NULL){
    		printf("Error to allocate space for plist");
    		exit(1);
    	}
    	(*plist)->ID = 0;
    	(*plist)->func = NULL;
    	(*plist)->next = NULL; <- THIS IS THE NEW LINE
    	Register(0x0B,plist, DamagePacket);	
    	Register(0x1A,plist, WorldItem);
    	Register(0x1D,plist, RemoveItem);
            etc............................
    }

  8. #8
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,767
    Maybe test pfunc for NULL before trying to call it
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  9. #9
    Registered User
    Join Date
    Nov 2008
    Posts
    14
    The fact is that pfunc cannot be null if ListSearch find a valid ID, it was my fault to initialize plist->ID (in Initialize function) to 0, because when HandlePacket encounter a 0 ID in the buffer and use ListSearch, this function returns the structure with all NULL, so to resolve i simply initialized ID to -1.
    Thnx to all, stupid bugs, the next time is better to think more eheh!

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Profiler Valgrind
    By afflictedd2 in forum C++ Programming
    Replies: 4
    Last Post: 07-18-2008, 10:38 AM
  2. ATL bug of CComPtr?
    By George2 in forum Windows Programming
    Replies: 6
    Last Post: 04-07-2008, 08:52 AM
  3. Replies: 1
    Last Post: 12-01-2007, 02:06 AM
  4. Is valgrind always right?
    By g4j31a5 in forum Linux Programming
    Replies: 3
    Last Post: 07-16-2007, 11:39 PM
  5. Valgrind output
    By m_larkin2001 in forum C++ Programming
    Replies: 1
    Last Post: 06-09-2006, 03:28 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21