-2

The following function fails in the case of a call like:

CreateMatrix(A, 2,
             1, 2,
             3, 4);

but, passes in the case of a call like:

CreateMatrix(A, 2,
             1.0, 2.0,
             3.0, 4.0);

How can I make this function work in the case of integer arguments?

Source code:

void CreateMatrix(float* &A, int count, ...) {
    A = (float*)malloc(count * count * sizeof(float));
    va_list args;
    va_start(args, count);
    for (int i = 0; i < count; i++) {
        for (int j = 0; j < count; j++) {
            A[i * count + j] = (float)va_arg(args, double); // Use "double" type parameter
        }
    }
    va_end(args);
}

For example:

TEST(matrix_hpp_test, create_matrix_test)
{
    float *A = nullptr;

    CreateMatrix(A, 2,
                 1, 2,
                 3, 4);

    ASSERT_EQ(A[0], 1);
    ASSERT_EQ(A[1], 2);
    ASSERT_EQ(A[2], 3);
    ASSERT_EQ(A[3], 4);
}

Output:

C:\git\test_run.exe --gtest_color=no
Testing started at 5:00 PM ...
Running 1 calendar_tests from 1 calendar_tests suite.[----------] Global calendar_tests environment set-up.
[----------] 1 calendar_tests from matrix_hpp_test
C:/git/matrix_test.cpp:13: Failure
Expected equality of these values:
  A[0]
    Which is: 0
  1.0
    Which is: 1


[----------] 1 calendar_tests from matrix_hpp_test (0 ms total)

[----------] Global calendar_tests environment tear-down
1 calendar_tests from 1 calendar_tests suite ran. (0 ms total)[  PASSED  ] 0 tests.
[  FAILED  ] 1 calendar_tests, listed below:
[  FAILED  ] matrix_hpp_test.create_matrix_test

 1 FAILED TEST
Process finished with exit code 1

On the other hand,

TEST(matrix_hpp_test, create_matrix_test)
{
    float *A = nullptr;

    CreateMatrix(A, 2,
                 1.0, 2.0,
                 3.0, 4.0);

    ASSERT_EQ(A[0], 1);
    ASSERT_EQ(A[1], 2);
    ASSERT_EQ(A[2], 3);
    ASSERT_EQ(A[3], 4);
}

Output

C:\git\test_run.exe --gtest_color=no
Testing started at 5:04 PM ...
Running 1 calendar_tests from 1 calendar_tests suite.[----------] Global calendar_tests environment set-up.
[----------] 1 calendar_tests from matrix_hpp_test
[----------] 1 calendar_tests from matrix_hpp_test (0 ms total)

[----------] Global calendar_tests environment tear-down
1 calendar_tests from 1 calendar_tests suite ran. (0 ms total)[  PASSED  ] 1 calendar_tests.
Process finished with exit code 0

Here is a template version:

template<typename T>
void CreateMatrix(T* &A, int count, ...) {
    A = (T*)malloc(count * count * sizeof(T));

    va_list args;
    va_start(args, count);

    for (int i = 0; i < count; i++) {
        for (int j = 0; j < count; j++) {
            A[i * count + j] = va_arg(args, T);
        }
    }

    va_end(args);
}

It also fails:

TEST(matrix_hpp_test, create_matrix_test)
{
    float *A = nullptr;
    CreateMatrix<float>(A, 2,
                      1, 2,
                      3, 4);
    ASSERT_EQ(A[0], 1);
    ASSERT_EQ(A[1], 2);
    ASSERT_EQ(A[2], 3);
    ASSERT_EQ(A[3], 4);
}

Output

C:\git\test_run.exe --gtest_color=no
Testing started at 5:34 PM ...
Running 1 calendar_tests from 1 calendar_tests suite.[----------] Global calendar_tests environment set-up.
[----------] 1 calendar_tests from matrix_hpp_test
Process finished with exit code -1073741795 (0xC000001D)
user366312
  • 16,949
  • 65
  • 235
  • 452
  • 12
    Do you know how to use C++ templates, instead of old-style, type-unsafe C varargs which is always prone to this bug? This cannot be fixed using C-style varargs, which were in style back when dinosaurs roamed the Earth. A proper, modern C++ variadic template will precisely solve this issue. – Sam Varshavchik Apr 05 '23 at 15:09
  • 8
    Almost all your code could have been written as C, but you are a C++ programmer. Time to start using the features of C++, like templates and the standard library. – john Apr 05 '23 at 15:12
  • 2
    @Jabberwocky `CreateMatrix(A, 2, 1, 2, 3, 4);` has undefined behaviour. I wouldn't call that valid. In fact so does `CreateMatrix(A, 2, 1.0, 2.0, 3.0, 4.0);`, but unluckily that doesn't have obviously breaky symptoms. – Caleth Apr 05 '23 at 15:24
  • @Caleth you're right, but that's actually what the question is about. BTW: IMO the call to `CreateMatrix(A, 2, 1, 2, 3, 4)` itself is not yet UB, UB happens only in `va_arg(args, double)` when it tries to pull `double`s while the user has provided `int`s. – Jabberwocky Apr 05 '23 at 15:29
  • @Caleth why is `CreateMatrix(A, 2, 1.0, 2.0, 3.0, 4.0)` UB? Wouldn't `printf("%lf", 10.0)` be UB as well? – Jabberwocky Apr 05 '23 at 15:30
  • @Jabberwocky `(float*)malloc(count * count * sizeof(float));` doesn't create an array of float – Caleth Apr 05 '23 at 15:30
  • C++ style would be more like `float* A = CreateMatrix({ 1, 2, 3, 4 });` where `CreateMatrix` takes an `initializer_list` (which knows its own size) – Ben Voigt Apr 05 '23 at 15:31
  • @BenVoigt or `class Matrix { /*some members*/ public: Matrix(initializer_list); /*other members*/ };` – Caleth Apr 05 '23 at 15:32
  • @Caleth then what does `(float*)malloc(count * count * sizeof(float))` do? – Jabberwocky Apr 05 '23 at 15:32
  • 1
    @Caleth: Yes it does, `float[]` is a type with trivial initialization so it comes into existence once storage with suitable size and alignment is obtained, and `malloc` result is guaranteed to be suitably aligned for any fundamental type. – Ben Voigt Apr 05 '23 at 15:32
  • @Caleth: I was trying to clean up just the one broken function, without also breaking all the consumers of the pointer. – Ben Voigt Apr 05 '23 at 15:33
  • @BenVoigt ok, prior to C++20 it's always UB – Caleth Apr 05 '23 at 15:33
  • 1
    @Caleth: The rule I'm paraphrasing from `[basic.life]` goes back to C++03 or maybe even C++98 – Ben Voigt Apr 05 '23 at 15:34
  • @Caleth interesting. Why was it UB before C++20? – Jabberwocky Apr 05 '23 at 15:35
  • 1
    @user366312 the template version also fails because `CreateMatrix(...` still instanciates the version for `float` but you provide `int`s. If you provide `int`s you'd need `CreateMatrix(...` – Jabberwocky Apr 05 '23 at 15:39
  • @BenVoigt `malloc` doesn't create objects, it provides storage that objects can be created in. There is no `float[]` until C++20's vacuous initialisation – Caleth Apr 05 '23 at 15:42
  • Keeping the raw pointers, C++ style would be `float* A = new float[]{ 1, 2, 3, 4 };`, although you'd need to `delete[] A` instead of `free(A)` – Caleth Apr 05 '23 at 15:49
  • @Jabberwocky `(float*)malloc(count * count * sizeof(float))` gives you a pointer that you basically can't use, except pass to a placement-new expression. – Caleth Apr 05 '23 at 15:54
  • @Caleth: Completely incorrect. C++03, [basic.life]/2 says *Note: **the lifetime of an array object** or of an object of POD type (3.9) **starts as soon as storage with proper size and alignment is obtained**, and its lifetime ends when the storage which the array or object occupies is reused or released.* – Ben Voigt Apr 05 '23 at 15:58
  • @BenVoigt does that `malloc` create a `float[count * count]`, an `int[count * count]` or what? how is it to know what type? The cast is a `reinterpret_cast` – Caleth Apr 05 '23 at 16:01
  • @Caleth: It is all of them. But none of the elements can be read until they are assigned. It is the line `A[i * count + j] = (float)va_arg(args, double);` that detemines the effective run-time type of the element. Before that it cannot be read. After that it can be read as a `float`. – Ben Voigt Apr 05 '23 at 16:04
  • @Caleth This is going off topic, I'll write a question tagged [C++] about this topic. Look in 5-10 minutes.. – Jabberwocky Apr 05 '23 at 16:08
  • @BenVoigt This is going off topic, I'll write a question tagged [C++] about this topic. Look in 5-10 minutes. – Jabberwocky Apr 05 '23 at 16:08
  • @Caleth new question posted: https://stackoverflow.com/questions/75941788/is-using-malloc-in-c-undefined-behaviour – Jabberwocky Apr 05 '23 at 16:17

2 Answers2

2

The first case fails because you pass int values to the function, but in the function you assume these values are doubles and of course it fails. va_arg(args, double) blindly reads the variable arguments as if they were doubles.

It's similar to passing wrong variable types to printf-like functions, like e.g: printf("%lf", 10); which would most likely not print 10 but something else. It's actually undefined behaviour..

But as already mentioned, using modern C++ style, like variadic templates would be the best solution here.

Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • `CreateMatrix(A, 2, 1, 2, 3, 4);` this also fails the test. – user366312 Apr 05 '23 at 15:27
  • 2
    @user366312: You've not shown any code that would work with that syntax. – Ben Voigt Apr 05 '23 at 15:29
  • @BenVoigt, template void CreateMatrix(T* &A, int count, ...) { A = (T*)malloc(count * count * sizeof(T)); va_list args; va_start(args, count); for (int i = 0; i < count; i++) { for (int j = 0; j < count; j++) { A[i * count + j] = va_arg(args, T); } } va_end(args); } – user366312 Apr 05 '23 at 15:33
  • @user366312: You didn't do the one most important thing you were told, which was "get rid of varargs and use C++ variadics" – Ben Voigt Apr 05 '23 at 15:35
0

Your va_args implementation nails down the params to be fetched as "double" from the stack, while the function call (not knowing, what the usage of the params will be) pushes them to the stack as integers. There can't happen an automatic conversion as va_args doesn't know what the target type should be.

Compare this with e.g. printf() function, where the format string tells the function, which param types to expect on the stack. If the format string e.g. specifies a float to be printed, but you deliver a float, the result will be wild ants on the screen.

-- corrected "float" to "double" ...typo, sorry

Synopsis
  • 190
  • 10