built error

[meixsh]

Hello, I was using your code for test. I just execute make all but get an error, like following and I do not know how to solve it, could you please help me, thanks.

Environment:
Ubuntu 14.04 LTS
gcc version 4.8.4

dist/count_min_sketch.o: In function `cms_add_inc_alt':
count_min_sketch.c:(.text+0x3dc): undefined reference to `__builtin_add_overflow'
count_min_sketch.c:(.text+0x424): undefined reference to `__builtin_add_overflow'
dist/count_min_sketch.o: In function `cms_remove_inc_alt':
count_min_sketch.c:(.text+0x539): undefined reference to `__builtin_sub_overflow'
count_min_sketch.c:(.text+0x581): undefined reference to `__builtin_sub_overflow'
dist/count_min_sketch.o: In function `__safe_add':
count_min_sketch.c:(.text+0xbc1): undefined reference to `__builtin_add_overflow'
dist/count_min_sketch.o: In function `__safe_sub':
count_min_sketch.c:(.text+0xbf1): undefined reference to `__builtin_sub_overflow'
collect2: error: ld returned 1 exit status
make: *** [all] Error 1

[meixsh]

This is a gcc version problem. Using gcc 5 and above will solve this problem. But I need to include this version cms implementation to my existed project which was only tested under gcc 4.6. If I use gcc 5 to built whole project, there will be some unexpected problem. And I know that the root of all problem is the unsupported built-in function of __built_sub_overflow and __built_add_overflow in gcc 4.6, so what functions could I use to replace these two ?

[barrust]

@meixsh It seems that the built in functions to handle overflows was added in GCC 5. I do not personally have a system with which to validate that the below fix would work, but you could do something like this to allow the code to use the non-gcc builtins to be used for GCC < 5. The lines to modify (untested, sorry) are 313 and 332. In essence, what you will want to do is change the ifdef to check if we are using GCC >=5 to use the _builtin*_overflow functions. I added the section here. If this works, please submit a patch and I will be happy to merge it in!

    int32_t __safe_add(int a, int b) {
    /* use the gcc macro if compiling with GCC, otherwise, simple overflow check */
    int32_t c = 0;
    // CHANGE THIS LINE!
    #if defined(__GNUC__) && __GNUC__ >=5 
        int bl = __builtin_add_overflow(a, b, &c);
        if (bl != 0) {
            c = INT_MAX;
        }
    #else
        if (b < INT_MIN + a) {
            c = INT_MIN;
        } else {
            c = a - b;
        }
    #endif

    return c;
}

int32_t __safe_sub(int32_t a, int32_t b) {
    /* use the gcc macro if compiling with GCC, otherwise, simple overflow check */
    int32_t c = 0;
    // CHANGE THIS LINE!
    #if defined(__GNUC__) && __GNUC__ >=5
        int32_t bl = __builtin_sub_overflow(a, b, &c);
        if (bl != 0) {
            c = INT_MAX;
        }
    #else
        if (b > INT_MAX - a) {
            c = INT_MAX;
        } else {
            c = a + b;
        }
    #endif

    return c;
}

[meixsh]

@barrust Thanks for your reply. I found that your code logic maybe have a little problem. Please check the code logic included in else branch. I can not understand corresponding code meaning and it won't pass the code test which uses your usage applet in README file after I added your newly changes. So I think your else branch have logic error.Your implementations for overflow detection on adding two numbers and subtracting two numbers actually aren't right.

[meixsh]

The version I modified:

int32_t __safe_add(int a, int b) {
    /* use the gcc macro if compiling with GCC, otherwise, simple overflow check */
    int32_t c = 0;
    #if defined(__GNUC__) && __GNUC__ >=5
        int bl = __builtin_add_overflow(a, b, &c);
        if (bl != 0) {
            c = INT_MAX;
        }
    #else */
        if (b > INT_MAX - a) {
            c = INT_MAX;
        } else {
            c = a + b;
        }
    #endif

    return c;
}

int32_t __safe_sub(int32_t a, int32_t b) {
    /* use the gcc macro if compiling with GCC, otherwise, simple overflow check */
    int32_t c = 0;

   #if defined(__GNUC__) && __GNUC__ >=5
        int32_t bl = __builtin_sub_overflow(a, b, &c);
        if (bl != 0) {
            c = INT_MAX;
        }
   #else
        if (b < a - INT_MAX) {
            c = INT_MAX;
        } else {
            c = a - b;
        }
   #endif

   return c;
}

[barrust]

Yes, those changes look correct. I am not sure when or how I messed those up. Likely, I never tested on a non-gcc compiler.

[barrust]

If you put in a PR, I will be happy to review, test, and merge!

[meixsh]

Thanks. I have tested it on my personal computer(Ubuntu14.04 LTS, gcc 4.8.4). And it have passed your test successfully. I will try to pull request for this project but please wait a while. Because I am not very familiar with git now. ^_^

[barrust]

I merged your PR with a minor change. If you would like to test again, we can close this ticket. Thanks!