Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does the -O4 compiler directive give unexpected results?

I'm supposed to use the compiler directives gcc -pedantic -Wall -ansi -O4. But the -O4 flag is giving me some problems. It works without the O4 flag but with it I get unexpected results:

$ gcc -pedantic -Wall -ansi openshell.c 
$ ./a.out 
'PATH' is set to /home/dac/proj/google-cloud-sdk/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games.
miniShell>> ls | wc -l
82
6318: child 6319 status 0x0000
Execution time 1.197 ms
miniShell>> exit
$ gcc -pedantic -Wall -ansi -O4 openshell.c 
$ ./a.out 
'PATH' is set to /home/dac/proj/google-cloud-sdk/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games.
miniShell>> ls | wc -l
./a.out:6332: Syntax error: pipe with no command following (10: No child processes)

What can I do about it? The relevant code is:

static void exec_arguments(int argc, char **argv)
{
    /* Split the command line into sequences of arguments */
    /* Break at pipe symbols as arguments on their own */

    int i = 1;

    char ***cmdv = malloc(sizeof *cmdv * (argc/2));       
    char **args = malloc(sizeof *args * (argc+1));
    int cmdn = 0;
    int argn = 0;

    cmdv[cmdn++] = &args[argn];

    for (i=1; i < argc; i++)
    {
        char *arg = argv[i];
        if (strcmp(arg, "|") == 0)
        {
            if (i == 1)
                err_sysexit("Syntax error: pipe before any command");
            if (args[argn-1] == 0)
                err_sysexit("Syntax error: two pipes with no command between");
            arg = 0;
        }
        args[argn++] = arg;
        if (arg == 0)
            cmdv[cmdn++] = &args[argn];
    }
    if (args[argn-1] == 0)
        err_sysexit("Syntax error: pipe with no command following");
    args[argn] = 0;

    exec_pipeline(cmdn, cmdv);
    free(cmdv);
    free(args);
}

It happens if I compile with O3 and O4 but not with O2. The call to the above function is this code:

 argv2[1] = line;
 i = 1;
 p = strtok (line, " ");


 while (p != NULL)
 {
   array[i++] = p;
   p = strtok (NULL, " ");
 }
 exec_arguments(i, array);
 corpse_collector();

This is the profiling with valgrind:

$ valgrind --leak-check=full -v ./a.out 
==9145== Memcheck, a memory error detector
==9145== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==9145== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==9145== Command: ./a.out
==9145== 
--9145-- Valgrind options:
--9145--    --leak-check=full
--9145--    -v
--9145-- Contents of /proc/version:
--9145--   Linux version 3.16.0-48-generic (buildd@lcy01-10) (gcc version 4.8.2 (Ubuntu 4.8.2-19ubuntu1) ) #64~14.04.1-Ubuntu SMP Thu Aug 20 23:03:57 UTC 2015
--9145-- 
--9145-- Arch and hwcaps: AMD64, LittleEndian, amd64-cx16-lzcnt-rdtscp-sse3-avx-avx2-bmi
--9145-- Page sizes: currently 4096, max supported 4096
--9145-- Valgrind library directory: /usr/lib/valgrind
--9145-- Reading syms from /home/dac/ClionProjects/openshell/a.out
--9145-- Reading syms from /lib/x86_64-linux-gnu/ld-2.21.so
--9145--   Considering /lib/x86_64-linux-gnu/ld-2.21.so ..
--9145--   .. CRC mismatch (computed c1319e6e wanted c87fa39c)
--9145--   Considering /usr/lib/debug/lib/x86_64-linux-gnu/ld-2.21.so ..
--9145--   .. CRC is valid
--9145-- Reading syms from /usr/lib/valgrind/memcheck-amd64-linux
--9145--   Considering /usr/lib/valgrind/memcheck-amd64-linux ..
--9145--   .. CRC mismatch (computed cd34a87b wanted 936d30dc)
--9145--    object doesn't have a symbol table
--9145--    object doesn't have a dynamic symbol table
--9145-- Scheduler: using generic scheduler lock implementation.
--9145-- Reading suppressions file: /usr/lib/valgrind/default.supp
==9145== embedded gdbserver: reading from /tmp/vgdb-pipe-from-vgdb-to-9145-by-dac-on-???
==9145== embedded gdbserver: writing to   /tmp/vgdb-pipe-to-vgdb-from-9145-by-dac-on-???
==9145== embedded gdbserver: shared mem   /tmp/vgdb-pipe-shared-mem-vgdb-9145-by-dac-on-???
==9145== 
==9145== TO CONTROL THIS PROCESS USING vgdb (which you probably
==9145== don't want to do, unless you know exactly what you're doing,
==9145== or are doing some strange experiment):
==9145==   /usr/lib/valgrind/../../bin/vgdb --pid=9145 ...command...
==9145== 
==9145== TO DEBUG THIS PROCESS USING GDB: start GDB like this
==9145==   /path/to/gdb ./a.out
==9145== and then give GDB the following command
==9145==   target remote | /usr/lib/valgrind/../../bin/vgdb --pid=9145
==9145== --pid is optional if only one valgrind process is running
==9145== 
--9145-- REDIR: 0x401add0 (ld-linux-x86-64.so.2:strlen) redirected to 0x3809e1b1 (???)
--9145-- Reading syms from /usr/lib/valgrind/vgpreload_core-amd64-linux.so
--9145--   Considering /usr/lib/valgrind/vgpreload_core-amd64-linux.so ..
--9145--   .. CRC mismatch (computed 1c3ef3cc wanted d1ae2653)
--9145--    object doesn't have a symbol table
--9145-- Reading syms from /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so
--9145--   Considering /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so ..
--9145--   .. CRC mismatch (computed 6e6e6f70 wanted ea7b69f1)
--9145--    object doesn't have a symbol table
==9145== WARNING: new redirection conflicts with existing -- ignoring it
--9145--     old: 0x0401add0 (strlen              ) R-> (0000.0) 0x3809e1b1 ???
--9145--     new: 0x0401add0 (strlen              ) R-> (2007.0) 0x04c2f060 strlen
--9145-- REDIR: 0x401ab30 (ld-linux-x86-64.so.2:index) redirected to 0x4c2ec00 (index)
--9145-- REDIR: 0x401ad50 (ld-linux-x86-64.so.2:strcmp) redirected to 0x4c30110 (strcmp)
--9145-- REDIR: 0x401bac0 (ld-linux-x86-64.so.2:mempcpy) redirected to 0x4c33330 (mempcpy)
--9145-- Reading syms from /lib/x86_64-linux-gnu/libc-2.21.so
--9145--   Considering /lib/x86_64-linux-gnu/libc-2.21.so ..
--9145--   .. CRC mismatch (computed 871d6e12 wanted 63ccc9d2)
--9145--   Considering /usr/lib/debug/lib/x86_64-linux-gnu/libc-2.21.so ..
--9145--   .. CRC is valid
--9145-- REDIR: 0x4ec7950 (libc.so.6:strcasecmp) redirected to 0x4a26730 (_vgnU_ifunc_wrapper)
--9145-- REDIR: 0x4ec9c40 (libc.so.6:strncasecmp) redirected to 0x4a26730 (_vgnU_ifunc_wrapper)
--9145-- REDIR: 0x4ec70d0 (libc.so.6:memcpy@GLIBC_2.2.5) redirected to 0x4a26730 (_vgnU_ifunc_wrapper)
--9145-- REDIR: 0x4ec5370 (libc.so.6:rindex) redirected to 0x4c2e8e0 (rindex)
==9145== Syscall param rt_sigaction(act->sa_mask) points to uninitialised byte(s)
==9145==    at 0x4E6D3F1: __libc_sigaction (sigaction.c:62)
==9145==    by 0x401061: main (in /home/dac/ClionProjects/openshell/a.out)
==9145==  Address 0xffeffefc8 is on thread 1's stack
==9145== 
--9145-- REDIR: 0x4ec3670 (libc.so.6:strlen) redirected to 0x4c2efa0 (strlen)
--9145-- REDIR: 0x4ec3ae0 (libc.so.6:__GI_strncmp) redirected to 0x4c2f750 (__GI_strncmp)
--9145-- REDIR: 0x4ece6b0 (libc.so.6:strchrnul) redirected to 0x4c32e60 (strchrnul)
--9145-- REDIR: 0x4ec7300 (libc.so.6:__GI_mempcpy) redirected to 0x4c33060 (__GI_mempcpy)
'PATH' is set to /home/dac/proj/google-cloud-sdk/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games.
--9145-- REDIR: 0x4ebc440 (libc.so.6:malloc) redirected to 0x4c2bb60 (malloc)
--9145-- REDIR: 0x4ecc390 (libc.so.6:__GI_memcpy) redirected to 0x4c30b60 (__GI_memcpy)
--9145-- REDIR: 0x4ec30a0 (libc.so.6:strcpy) redirected to 0x4a26730 (_vgnU_ifunc_wrapper)
--9145-- REDIR: 0x4edcab0 (libc.so.6:__strcpy_sse2_unaligned) redirected to 0x4c2f080 (strcpy)
--9145-- REDIR: 0x4ebc7f0 (libc.so.6:free) redirected to 0x4c2cdc0 (free)
miniShell>> ls | wc -l
--9145-- REDIR: 0x4ec67c0 (libc.so.6:memchr) redirected to 0x4c301b0 (memchr)
--9145-- REDIR: 0x4ec5330 (libc.so.6:strncpy) redirected to 0x4a26730 (_vgnU_ifunc_wrapper)
--9145-- REDIR: 0x4edd0e0 (libc.so.6:__strncpy_sse2_unaligned) redirected to 0x4c2f5b0 (__strncpy_sse2_unaligned)
--9145-- REDIR: 0xffffffffff600000 (???:???) redirected to 0x3809e193 (???)
./a.out:9145: Syntax error: pipe with no command following--9145-- REDIR: 0x4ec77e0 (libc.so.6:__GI_stpcpy) redirected to 0x4c31fe0 (__GI_stpcpy)
 (10: No child processes)
==9145== 
==9145== HEAP SUMMARY:
==9145==     in use at exit: 64 bytes in 2 blocks
==9145==   total heap usage: 8 allocs, 6 frees, 300 bytes allocated
==9145== 
==9145== Searching for pointers to 2 not-freed blocks
==9145== Checked 73,312 bytes
==9145== 
==9145== LEAK SUMMARY:
==9145==    definitely lost: 0 bytes in 0 blocks
==9145==    indirectly lost: 0 bytes in 0 blocks
==9145==      possibly lost: 0 bytes in 0 blocks
==9145==    still reachable: 64 bytes in 2 blocks
==9145==         suppressed: 0 bytes in 0 blocks
==9145== Reachable blocks (those to which a pointer was found) are not shown.
==9145== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==9145== 
==9145== Use --track-origins=yes to see where uninitialised values come from
==9145== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
==9145== 
==9145== 1 errors in context 1 of 1:
==9145== Syscall param rt_sigaction(act->sa_mask) points to uninitialised byte(s)
==9145==    at 0x4E6D3F1: __libc_sigaction (sigaction.c:62)
==9145==    by 0x401061: main (in /home/dac/ClionProjects/openshell/a.out)
==9145==  Address 0xffeffefc8 is on thread 1's stack
==9145== 
==9145== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
$ 

The thing does compile and run cleanly with the clang compiler. I suppose I'm left to making a minimal example and compare the generated assembly if I want to proceed solving this problem. The entire code is available here.

Oddly, if we insert a print statement in the loop, then the code works:

miniShell>> ls | wc -l
Processing 1 [ls]
Processing 2 [|]
Processing 3 [wc]
Processing 4 [-l]
84
27892: child 27900 status 0x0000
Execution time 1.358 ms



static void exec_arguments(int argc, char **argv)
{
    /* Split the command line into sequences of arguments */
    /* Break at pipe symbols as arguments on their own */

    int i = 1;
    char ***cmdv = malloc(sizeof *cmdv * (argc/2));
    char **args = malloc(sizeof *args * (argc+1));
    int cmdn = 0;
    int argn = 0;
    char *arg;
    cmdv[cmdn++] = &args[argn];

    for (i=1; i < argc; i++)
    {
        arg = argv[i];
        printf("Processing %d [%s]\n", i, arg);
        if (strcmp(arg, "|") == 0)
        {
            if (i == 1)
                err_sysexit("Syntax error: pipe before any command");
            if (args[argn-1] == 0)
                err_sysexit("Syntax error: two pipes with no command between");
            arg = 0;
        }
        args[argn] = arg;
        argn++;
        /*printf("argn %d", argn);*/
        if (arg == 0)
            cmdv[cmdn++] = &args[argn];
    }
    if (args[argn-1] == 0)
        err_sysexit("Syntax error: pipe with no command following");
    args[argn] = 0;

    exec_pipeline(cmdn, cmdv);
    free(cmdv);
    free(args);
}

Knowing the above, I could put in a debug statement that doesn't write any output and makes the loop work (but we don't know why) that will allow my code to compile with the highest optimization level. We still suspect some pointer handling that we might investigate by making stand-alone minimal example of the bug. For now, this will make me proceed:

static void exec_arguments(int argc, char **argv)
{
    /* Split the command line into sequences of arguments */
    /* Break at pipe symbols as arguments on their own */

    int i = 1;
    char ***cmdv = malloc(sizeof *cmdv * (argc/2));
    char **args = malloc(sizeof *args * (argc+1));
    int cmdn = 0;
    int argn = 0;
    char *arg;
    FILE* debug;

    cmdv[cmdn++] = &args[argn];

    for (i=1; i < argc; i++)
    {
        arg = argv[i];
        debug = fopen("/dev/null", "w");
        fprintf(debug, "Debug Information");

        if (strcmp(arg, "|") == 0)
        {
            if (i == 1)
                err_sysexit("Syntax error: pipe before any command");
            if (args[argn-1] == 0)
                err_sysexit("Syntax error: two pipes with no command between");
            arg = 0;
        }
        args[argn] = arg;
        argn++;
        /*printf("argn %d", argn);*/
        if (arg == 0)
            cmdv[cmdn++] = &args[argn];
    }
    if (args[argn-1] == 0)
        err_sysexit("Syntax error: pipe with no command following");
    args[argn] = 0;

    exec_pipeline(cmdn, cmdv);
    free(cmdv);
    free(args);
}
like image 534
Niklas Rosencrantz Avatar asked Mar 12 '23 15:03

Niklas Rosencrantz


2 Answers

After taking a pruning hook to the code, I've discovered the cause of trouble.

As I first suggested, the trouble is "most likely, there's some undefined behaviour in your code, and using optimization is revealing it".

The problem is the array in main() defined as char *array[4];. The code overflows that array, leading to undefined behaviour. Changing the size to 40 means the program works reliably.

Although I hacked out large quantities of the code, there are still well over 400 lines of code in the file. I'm sure a more thorough reduction job could be done, but I'm confident that this is the primary cause of the trouble.

Here's the simplified main() function with the too-small setting of char *array[4]l, and two new auxilliary functions, dump_argv() and AllWhiteSpace(). A number of other functions have been removed. The foreground/background stuff has gone; the built-in commands (like cd and exit and unixinfo was removed. The corpse_collector() already collected corpses; the Janitor() function was also trying to collect them, and seldom (okay, I mean never!) found anything because the corpse collector had already done the dirty work. I removed the signal handling as not immediately relevant.

#include <ctype.h>
#include <stdbool.h>

static void dump_argv(const char *tag, int argc, char **argv)
{
    assert(argv != 0 && tag != 0);
    printf("%s: (%d)", tag, argc);
    for (int i = 0; i < argc; i++)
        printf(" {%s}", (argv[i] == NULL) ? "(null)" : argv[i]);
    putchar('\n');
    fflush(stdout);
    assert(argv[argc] == 0);
}

static bool AllWhiteSpace(char *source)
{
    unsigned char c;
    while (isspace(c = *source))
        source++;
    return(c == '\0');
}

int main(int argc, char *argv[])
{
    err_setarg0(argv[argc-argc]);

    while (1)
    {
        char *p;
        char *array[40];
        char line[BUFFER_LEN];
        size_t length;
        long time;
        struct timeval time_start;
        struct timeval time_end;

        printf("miniShell>> ");
        fflush(stdout);
        if (!fgets(line, BUFFER_LEN, stdin))
        {
            putchar('\n');
            break;
        }

        if (AllWhiteSpace(line))
            continue;

        length = strlen(line);
        if (line[length - 1] == '\n')
            line[length - 1] = '\0';

        gettimeofday(&time_start, NULL);

        printf("Command line: %s\n", line);
        int i = 1;
        p = strtok(line, " ");

        array[0] = NULL;
        while (p != NULL)
        {
            array[i++] = p;
            p = strtok(NULL, " ");
        }
        array[i] = NULL;
        dump_argv("Before exec_arguments", i, array);
        exec_arguments(i, array);
        corpse_collector();

        gettimeofday(&time_end, NULL);
        time = (time_end.tv_sec  - time_start.tv_sec) * 1000000 +
                time_end.tv_usec - time_start.tv_usec;
        printf("Execution time %ld.%03ld ms\n", time / 1000, time % 1000);
    }

    return(0);
}

The symptoms of trouble now are somewhat different — the strings to be executed are not meaningful, and therefore things fail to execute, but increase the space large enough (40 is overkill for the command line in use; 6 is sufficient) and it works fine.

like image 169
Jonathan Leffler Avatar answered Apr 28 '23 02:04

Jonathan Leffler


There is a bug in your program. The -O4 option doesn't do anything different from -O3. A quick look at man gcc would've enumerated the list of options that do anything, and a sanity check:

diff <(gcc -Q --help=optimizers -O4) <(gcc -Q --help=optimizers -O3)

shows no difference.

Typically changing optimization levels is a good way to find bugs in your program as it may behave perfectly fine in one optimization level or break in another. This is indicative of undefined behavior, etc. I'm not going to waste my time debugging your program for you but at least you know that the problem is not directly caused by -O4.

Sidenote: I'm assuming that -ansi is a requirement that you can't change, however that compiles in C90 mode, aka ancient C mode. I would recommend C99 or C11 to avoid surprises.

like image 30
user6193956 Avatar answered Apr 28 '23 01:04

user6193956