4

I am trying to write a Hamming code program in C. However, I keep getting a Segmentation Fault(Core Dumped) error when trying to run the ./a.out after compiling. It compiles with no errors, and I understand that this error can occur when trying to address freed space or modifying a string literal. I don't believe I am doing either of those things, I just have a simple matrix I am filling and cross checking. Any insight on the issue would be appreciated, I have left what code I have so far below:

This is for a homework problem involving creating a Hamming code program to handle data.dat input and output to a file a sorted list of 1's and 0's

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

FILE *fp;
FILE *fpOut;

int main(int argc, char *argv[])
{
        fp = fopen(argv[1], "r");
        fpOut = fopen("sortedCodeWords.dat", "w");

        int temp;
        int numI = 0;
        int count = 0;

        char matrix1[7];

        if(fp == NULL)
                printf("File can not be opened");
        else
        {
                char ch = getc(fp);

                while(ch != EOF)
                {

                        matrix1[2] = ch;
                        ch = getc(fp);

                        matrix1[4] = ch;
                        ch = getc(fp);

                        matrix1[5] = ch;
                        ch = getc(fp);

                        matrix1[6] = ch;
                        ch = getc(fp);

                        ch = getc(fp);

                        if(ch == '\n')
                        {
                                for(int i = 2; i < 7; i++)
                                {
                                        if(matrix1[i] == '1')
                                            numI++;

                                        i++;
                                }

                                if(numI % 2 == 0)
                                        matrix1[0] = 0;
                                else
                                        matrix1[0] = 1;

                                numI = 0;

                                for(int i = 1; i < 7; i++)
                                {
                                        if(matrix1[i] == '1')
                                                numI++;
                                        if(matrix1[i+1] == '1')
                                                numI++;

                                        i++;
                                        i++;
                                }

                                if(numI % 2 == 0)
                                        matrix1[1] = 0;
                                else
                                        matrix1[1] = 1;

                                numI = 0;

                                for(int i = 4; i < 7; i++)
                                {
                                        if(matrix1[i] == '1')
                                                numI++;
                                }

                                if(numI % 2 == 0)
                                        matrix1[3] = 0;
                                else
                                        matrix1[3] = 1;
                                numI = 0;

                                for (int i = 0; i < 7; i++)
                                {
                                        fprintf(fpOut, "%s", matrix1[i]);
                                }

                                fprintf(fpOut, "\n");

                                ch = getc(fp);
                        }

                        count++;
                }
        }
}

I expect an output to a file. I didn't always get this error, but when I changed from a 2D array to 1D array, I am now getting this error (I changed because I realized it wasn't necessary)

V.Bean
  • 65
  • 1
  • 1
  • 5
  • There is another potential bug: while(ch != EOF) may not become true if your "char" is unsigned. – MiCo Apr 09 '19 at 22:07
  • @V.Bean what platform and compiler? – QuestionC Apr 09 '19 at 22:12
  • In one of your for-loops your are modifying the loop-variable addionally in the loop-body. Is this really your intention!? If so, please use i+=3 in the loop command. This is much clearer. – MiCo Apr 09 '19 at 22:15
  • @QuestionC I am using gcc – V.Bean Apr 09 '19 at 22:17
  • @MiCo I am trying to accomplish the hamming skip and check functionality – V.Bean Apr 09 '19 at 22:18
  • `fprintf(fpOut, "%s", matrix1[i]);` write out on paper the string your think you are passing to `fprintf` (don't forget to include the *nul-terminating* character in your analysis). If you pass something that is not *nul-terminated* to a function expecting a *nul-terminated* string -- how does it know when to stop iterating? What happens when the read continues beyond the memory you can validly access? Further, what address does `matrix1[i]` hold? Where do you think address `0-126` is on a system? (Does *System Reserved* ring any bells?) When you access memory there, what happens? (SegFault) – David C. Rankin Apr 09 '19 at 23:07

4 Answers4

4

I’ve not compiled it, but the one thing I notice is that it looks as if you’re potentially going off the end of your array where you’re looping to i<7 but using an index of i+1 in one instance.

Maybe build with AddressSanitizer enabled and see what runtime warnings you then get once you’ve checked the potential issue above. (It should just be a matter of adding the following flags to you gcc command... -g -fsanitize=address -fno-omit-frame-pointer

Gwyn Evans
  • 1,361
  • 7
  • 17
  • It shouldn't, because the increments goes 1-2-3, 4-5-6, then the last goes to 7 and it should quit. It shouldn't loop more than twice and shouldn't check past 7 – V.Bean Apr 09 '19 at 22:08
  • @V.Bean But in `for(int i = 1; i < 7; i++) ... matrix1[i+1] ...` you have `i+1` reaches 7 right? – Déjà vu Apr 09 '19 at 22:09
  • But your valid array index values are 0..6, not 1..7 – Gwyn Evans Apr 09 '19 at 22:10
  • It still shouldnt try to access past 7? Unless I can't seem to count... – V.Bean Apr 09 '19 at 22:13
  • Why are you incrementing `i` twice in that same loop, in addition to the increment from the for loop? – Mark Benningfield Apr 09 '19 at 22:15
  • Also, there is no longer a segmentation fault due to changing my printf form %s to %c. Now I am just getting funky output such as "^@^A0^A 1 ^@^@0^@ 0" instead of 1's and 0's – V.Bean Apr 09 '19 at 22:15
  • @MarkBenningfield because for hamming code I need to skip 2 then check 2, so i am incrementing i two more times to skip 2 and also skip the ones i already checked – V.Bean Apr 09 '19 at 22:17
  • So you intend to not test `matrix[3]` ? – Mark Benningfield Apr 09 '19 at 22:21
  • Ah, yes I see - there’s got to be a better way to express that, but I think your real problem is what the other answer refers to, i.e. storing chars and ints in the array but printing as if it’s a string. – Gwyn Evans Apr 09 '19 at 22:21
  • @MarkBenningfield Yes, as to fill it in later. – V.Bean Apr 09 '19 at 22:24
  • @GwynEvans for the first loop, it will be 1, 2, 3, then 4. Second loop will check 4, 5. Then increment from 4 to 5, then 6, then the loop itself will increment it to 7 quiting the loop. – V.Bean Apr 09 '19 at 22:25
2

Two things I see. First, you're mixing chars with ints in the matrix array. Second, you're printing the elements of matrix out to a file using the "%s" format. "%s" expects a null-terminated string where as you are passing chars and ints. This will cause the printf to try and access memory that is out-of-bounds, thus the fault.

sizzzzlerz
  • 4,277
  • 3
  • 27
  • 35
2

You gotta learn how to use a debugger.

I'm gonna risk assuming you're working on linux here and step through the debug process for your code.

First we compile with debug info enabled.

james@debian:~/code$ gcc foo.c -g

Now let's go at it with two tools, valgrind and gdb

valgrind in particular is a pretty great tool, it catches whenever you use screw up with memory. It's virtually mandatory for tracking down memory leaks but can be used for a lot of Seg faults. Best of all, it is easy to use and requires no interaction.

james@debian:~/code$ valgrind ./a.out foo.dat
==1857== Memcheck, a memory error detector
==1857== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1857== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==1857== Command: ./a.out foo.dat
==1857==
==1857== Conditional jump or move depends on uninitialised value(s)
==1857==    at 0x109381: main (foo.c:61)
==1857==
==1857== Invalid read of size 1
==1857==    at 0x4837C38: __GI_strlen (in /usr/lib/i386-linux-gnu/valgrind/vgpreload_memcheck-x86-linux.so)
==1857==    by 0x48A4786: vfprintf (vfprintf.c:1638)
==1857==    by 0x48AAC57: fprintf (fprintf.c:32)
==1857==    by 0x109437: main (foo.c:91)
==1857==  Address 0x1 is not stack'd, malloc'd or (recently) free'd
==1857==
==1857==
==1857== Process terminating with default action of signal 11 (SIGSEGV)
==1857==  Access not within mapped region at address 0x1
==1857==    at 0x4837C38: __GI_strlen (in /usr/lib/i386-linux-gnu/valgrind/vgpreload_memcheck-x86-linux.so)
==1857==    by 0x48A4786: vfprintf (vfprintf.c:1638)
==1857==    by 0x48AAC57: fprintf (fprintf.c:32)
==1857==    by 0x109437: main (foo.c:91)
==1857==  If you believe this happened as a result of a stack
==1857==  overflow in your program's main thread (unlikely but
==1857==  possible), you can try to increase the size of the
==1857==  main thread stack using the --main-stacksize= flag.
==1857==  The main thread stack size used in this run was 8388608.
==1857==
==1857== HEAP SUMMARY:
==1857==     in use at exit: 688 bytes in 2 blocks
==1857==   total heap usage: 3 allocs, 1 frees, 4,784 bytes allocated
==1857==
==1857== LEAK SUMMARY:
==1857==    definitely lost: 0 bytes in 0 blocks
==1857==    indirectly lost: 0 bytes in 0 blocks
==1857==      possibly lost: 0 bytes in 0 blocks
==1857==    still reachable: 688 bytes in 2 blocks
==1857==         suppressed: 0 bytes in 0 blocks
==1857== Rerun with --leak-check=full to see details of leaked memory
==1857==
==1857== For counts of detected and suppressed errors, rerun with: -v
==1857== Use --track-origins=yes to see where uninitialised values come from
==1857== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
Segmentation fault

Just reading the first error, it's flagging a problem at line 61, saying if(matrix1[i] == '1') is using uninitialized memory.

Reading your code with this in mind, it jumps out that matrix1[1] is never initialized so there's a bug. This isn't the Seg Fault though.

Another error is flagged at line 91, which looks like the bug, but it's hard to understand. So let's bust out gdb.

james@debian:~/code$ gdb a.out
GNU gdb (Debian 8.2.1-2) 8.2.1
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "i686-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from a.out...done.
(gdb) run foo.dat
Starting program: /home/james/code/a.out foo.dat

Program received signal SIGSEGV, Segmentation fault.
__strlen_ia32 () at ../sysdeps/i386/i586/strlen.S:51
51      ../sysdeps/i386/i586/strlen.S: No such file or directory.

run foo.dat runs the program. We quickly get your segmentation fault with some info.

(gdb) info stack
#0  __strlen_ia32 () at ../sysdeps/i386/i586/strlen.S:51
#1  0xb7e28787 in _IO_vfprintf_internal (s=0x4052c0, format=0x402037 "%s",
    ap=0xbffff52c "\360\021@") at vfprintf.c:1638
#2  0xb7e2ec58 in __fprintf (stream=0x4052c0, format=0x402037 "%s")
    at fprintf.c:32
#3  0x00401438 in main (argc=2, argv=0xbffff614) at foo.c:91
(gdb) frame 3
#3  0x00401438 in main (argc=2, argv=0xbffff614) at foo.c:91
91                          fprintf(fpOut, "%s", matrix1[i]);
(gdb)

info stack prints the execution stack. We don't care about the error at a system file level, we care about the error in main. frame 3 switches to frame #3, where the main function is located. At this point you may have already seen the bug, but we can dig deeper if it's not obvious.

(gdb) info locals
i = 0
ch = 10 '\n'
temp = <optimized out>
numI = 0
count = 2
matrix1 = "\001\000\061\001\060\061\060"
(gdb)

info locals displays all the local variables. At this point we have a fairly complete snapshot of what is going on, but just to spell it out...

(gdb) print matrix1[0]
$1 = 1 '\001'
(gdb)

At this point you gotta know some C. matrix[0] simply isn't an appropriate argument for printf("%s"). It's expecting a character pointer to some meaningful value, but you are giving it the number 1, which results in a Seg Fault.

Looking back at the error we got from valgrind, it is easier to understand now, saying the same thing we deduced using gdb: that we were trying to read the number 1 as a memory address...

==1857==    by 0x109437: main (foo.c:91)
==1857==  Address 0x1 is not stack'd, malloc'd or (recently) free'd

Your program will continue to have errors after you fix this I am sure, and future programs will have similar errors, but using these two tools you should be able to track down most problems.

QuestionC
  • 10,006
  • 4
  • 26
  • 44
  • I appreciate the insight on tools, however I am running this code on a school server with linux. I do not have permissions to add such tools but I will keep in mind to check if the school server has them or to at least install them on my local machine for future reference. Thank you – V.Bean Apr 09 '19 at 23:07
1

If you want to print 1's and 0's, you should be assigning

matrix[i] = '1';

instead of 

matrix[i] = 1;
Mark Benningfield
  • 2,800
  • 9
  • 31
  • 31
  • Ah I appreciate that, I was checking them properly but not storing them properly. That might explain the funny output! I'll double check that when i can. – V.Bean Apr 09 '19 at 22:41