Elysian Shadows

I am trying to code a brainfuck compiler thingy, and am almost done. However, I have ran into a problem. For some reason, the following code segfaults when ran.

lotios611 wrote:I am trying to code a brainfuck compiler thingy, and am almost done. However, I have ran into a problem. For some reason, the following code segfaults when ran.

Code: Select all

char* fileName = malloc((int)strlen(strcat(stripFileExtension(argv[1]), ".c")) * sizeof(char));
strcpy(fileName, strcat(stripFileExtension(argv[1]), ".c"));
...
system(strcat("gcc -std=c99 ", fileName));

I don't really know if C and C++ differentiate in terms of their arguments. My assumption though, is that you want "argv[0]" not "argv[1]" as you're probably indexing a cell in the argv array that doesn't exist by doing the latter.

JesseGuarascia wrote:

lotios611 wrote:I am trying to code a brainfuck compiler thingy, and am almost done. However, I have ran into a problem. For some reason, the following code segfaults when ran.

Code: Select all

char* fileName = malloc((int)strlen(strcat(stripFileExtension(argv[1]), ".c")) * sizeof(char));
strcpy(fileName, strcat(stripFileExtension(argv[1]), ".c"));
...
system(strcat("gcc -std=c99 ", fileName));

I don't really know if C and C++ differentiate in terms of their arguments. My assumption though, is that you want "argv[0]" not "argv[1]" as you're probably indexing a cell in the argv array that doesn't exist by doing the latter.

The first part works AFAIK because I can use the filename before the call to system(). Also, my application requires you to at least pass in the name of the file to compile, which would be argv[1]

Edit: I check for argc !=2 before that code.

The problem lies in this line: I think you're missing some of the basic concepts of C strings, or maybe just C memory management. What's going on here is something like this: When you compile this file, the compiler will create a 14-byte space in your object file's "read-only" data section, and store the data "gcc -std=c99 \0" in it. The details of this likely change from executable format to executable format, but the idea is the same. Literal strings that are used on the fly like this, without being stored in an array of characters (whether allocated dynamically or statically, it doesn't matter), are only long enough to store the string that they are defined as when the file is compiled. You are attempting to append however many bytes to a 14-byte buffer. This is causing a buffer overflow. I'm surprised your compiler hasn't warned you about constness or something of the like--you might want to try passing "-Wall -Wextra" when you compile your code. It works wonders for pointing out such errors.

The solution, then, is to get a new buffer that *will* fit the command string. You can do this one of two ways--using a statically allocated string of some arbitrary length, or dynamically allocating it with a size that will exactly fit the string. I recommend the dynamic method, because it guarantees that all strings will fit, and that you won't have to use size-aware functions (like strncat) when dealing with the string to prevent another overflow.
Static: Dynamic: Of course, with the dynamic approach, you're hardcoding the same literal in multiple places, so you might want to move "gcc -std=c99 " into a constant string, like so: Hope this helps.

Krolgar wrote:

Code: Select all

char *cmd = (char *)malloc(sizeof(char) * (strlen("gcc -std=c99 ") + strlen(fileName) + 1)); // The sizeof(char) may look a little redundant, but I have a habit of writing my C-style allocations to be ready to upgrade to wide characters with just a few project-wide find/replaces.
sprintf(cmd, "gcc -std=c99 %s", fileName);
system(cmd);
free(cmd);

Is the +1 to account for the null character? This is my first C program by the way.

lotios611 wrote:

Krolgar wrote:

Code: Select all

char *cmd = (char *)malloc(sizeof(char) * (strlen("gcc -std=c99 ") + strlen(fileName) + 1)); // The sizeof(char) may look a little redundant, but I have a habit of writing my C-style allocations to be ready to upgrade to wide characters with just a few project-wide find/replaces.
sprintf(cmd, "gcc -std=c99 %s", fileName);
system(cmd);
free(cmd);

Is the +1 to account for the null character? This is my first C program by the way.

Yes. When allocating a string, you should always add one extra character for the null-terminator.

Krolgar wrote:

lotios611 wrote:

Krolgar wrote:

Code: Select all

char *cmd = (char *)malloc(sizeof(char) * (strlen("gcc -std=c99 ") + strlen(fileName) + 1)); // The sizeof(char) may look a little redundant, but I have a habit of writing my C-style allocations to be ready to upgrade to wide characters with just a few project-wide find/replaces.
sprintf(cmd, "gcc -std=c99 %s", fileName);
system(cmd);
free(cmd);

Is the +1 to account for the null character? This is my first C program by the way.

Yes. When allocating a string, you should always add one extra character for the null-terminator.

Alright, thanks for the help.