Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] UB in s21_add with negative numbers #39

Closed
posidoni opened this issue Jun 10, 2022 · 2 comments
Closed

[Bug] UB in s21_add with negative numbers #39

posidoni opened this issue Jun 10, 2022 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@posidoni
Copy link
Contributor

image

@posidoni posidoni added the bug Something isn't working label Jun 10, 2022
@posidoni
Copy link
Contributor Author

posidoni commented Jun 10, 2022

This causes stack overflow. Probably this is also due to incorrect comparison of negative numbers. Andrey was writing & testing his functions with old buggy Stepan's comparisons. Probably we need to write ADD, SUB, MUL, DIV from scratch 🍭 This is not that hard actually. We brainstormed this with Stepa this evening.

Moreover, we will write our own functions & rewrite completely unreadable Andrey's code.

START_TEST(int64_t_negatives) {
    int64_t long_a = get_random_ll();
    int64_t long_b = get_random_ll();

    if (rand() % 2)
        long_a *= -1;

    if (rand() % 2)
        long_b *= -1;

    int64_t a = long_a;
    int64_t b = long_b;

    int64_t sum = a + b;

    s21_decimal res128 = {0};

    for (long long int i = 0, k = 0; i < 2; i++)
        for (long long int j = 0; j < 32; j++, k++)
            if (IS_SET(sum, k))
                ADD_BIT(res128.bits[i], j);

    s21_decimal dec_a = ll_to_decimal(long_a);
    s21_decimal dec_b = ll_to_decimal(long_b);
    s21_decimal dec_sum = {0};

    s21_add(dec_a, dec_b, &dec_sum);
    /* print_bits_r(dec_sum); */

    ck_assert_int_eq(s21_is_equal(res128, dec_sum), TRUE);
}

Suite *suite_s21_add(void) {
    Suite *s = suite_create(PRETTY_PRINT("s21_add"));
    TCase *tc = tcase_create("s21_add_tc");

    /* tcase_add_loop_test(tc, add_test1, 0, 1); */
    tcase_add_loop_test(tc, gcc_128_bits, 0, 10000);
    tcase_add_loop_test(tc, int64_t_negatives, 0, 10000);

    suite_add_tcase(s, tc);
    return s;
}

image

@posidoni
Copy link
Contributor Author

It turns out there were two bugs with s21_sub. Now everything works fine. #57 closes this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants