0

I'm writing a code which calculates the inverse matrix given a matrix, the thing is, I need that to be included in other code that makes statistical fits, so I need something like a function that receives the size of the matrix (matrix is square matrix) and the matrix itself and returns his inverse, I found something about the syntax and then have this (Gauss-Jordan)

float* inv(int n, float *A)
{
    float* I = 0;//*

    float aux;
    float pivote;

    for(int i = 0; i<n; i++){
        for(int j = 0; j<n; j++){

            if(i == j)
            {
                *(I+i*n+j) = 1.0; //*
            }

            else {
                *(I+i*n+j) = 0.0;
            }
        }
    }

    for(int i = 0; i<n; i++)
    {
        pivote = *(A+i*n+i);

        for(int k = 0; k<n; k++)
        {
            *(A+i*n+k) = *(A+i*n+k)/pivote;//*
            *(I+i*n+k) = *(I+i*n+k)/pivote;//*
        }

        for(int j = 0; j<n; j++)
        {
            if(i!=j)
            {
                aux = *(A+j*n+i);//*

                for(int k = 0; k<n;k++)
                {
                    *(A+j*n+k)=*(A+j*n+k)-aux**(A+i*n+k);//*
                    *(I+j*n+k)=*(I+j*n+k)-aux**(I+i*n+k);//*
                }
            }
        }
    }

    return I;//*
}

There where I put the //* is where I have my doubts, is the syntax correct? the declarations, there in the return should be something else than just I?. When I compile I get a segmentation fault, Following Taekahn recommendations, compiling with sanitizers g++ -fsanitize=address -fsanitize=undefined -fsanitize=leak inverse.cpp I get

inverse.cpp:148:28: runtime error: store to null pointer of type 'float'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==11993==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000040338c bp 0x7ffdd6a14510 sp 0x7ffdd6a144b0 T0)
==11993==The signal is caused by a WRITE memory access.
==11993==Hint: address points to the zero page.
    #0 0x40338b in inv(int, float*) (/home/live/med_elect/a.out+0x40338b)
    #1 0x402f30 in main (/home/live/med_elect/a.out+0x402f30)
    #2 0x7f90ffed9e5a in __libc_start_main (/lib64/libc.so.6+0x23e5a)
    #3 0x402289 in _start (/home/live/med_elect/a.out+0x402289)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/home/live/med_elect/a.out+0x40338b) in inv(int, float*)
==11993==ABORTING

I really appreciate if you can help me, thank you very much in advance and thank you very much for the feedback in the comments, I'm new here with the questions.

UPDATE: Thanks to nasy for the answer, It is important to note that many people mentioned the vector approach, so, to anyone reading this, check the comments and better try the vector approach.

CosmeticMichu
  • 73
  • 7
  • Have you tried running it under gdb and seeing where the issue is coming from? What about asan and ubsan? Warnings turned on and not ignored? – Taekahn Apr 25 '22 at 03:35
  • `int n = std::atoi(argv[1]); float A[n][n];` is not valid, `n` is not a constant expression. – Jason Apr 25 '22 at 03:41
  • 2
    *Passing 2D arrays as argument to a function and get another 2D array in c++* -- That's the title of this thread, however your post contains 98% of irrelevant code not related to the issue. If the issue is passing and returning a 2D array, then a simple 3 or 4 line `main` program, showing a declaration, and maybe a 3 or 4 line function that takes the array, changes a value, and returns it back, is all you need to show where you are stuck. That is what is meant by a [mcve]. Inverting matrices, etc. is not relevant to the issue concerning how C++ deals with 2D arrays. – PaulMcKenzie Apr 25 '22 at 03:49
  • Hi @AnoopRana , so you mean setting `const int n = std::atoi(argv[1])` instead of `int n`? – CosmeticMichu Apr 25 '22 at 03:50
  • *"As you know, arrays in c++ are not beginners friendly,"* -- nope, I did not know that. (I know people trip over details when they try to import knowledge from other languages, but that's a different issue than being a beginner.) Then again, I don't really see what relevance this claim has to your question since calculating the inverse of a matrix is not exactly a beginner's task... – JaMiT Apr 25 '22 at 03:50
  • @PaulMcKenzie ok, I will try to edit the question. Thanks for your comment – CosmeticMichu Apr 25 '22 at 03:53
  • [Please do not upload images of code/errors when asking a question.](//meta.stackoverflow.com/q/285551) – Ken White Apr 25 '22 at 03:54
  • 2
    *"I need something like a function that receives the size of the matrix (matrix is square matrix)"* -- this is more of a C-style approach than an object-oriented C++ approach. In fact, aside from the streaming operations to/from `std::cout`/`std::cin`, your code looks more like C than C++. Are you sure C++, not C, is the language you want to use? (Note that the objection someone made to `float A[n][n];` disappears if you switch to C, where variable-length arrays are standard.) – JaMiT Apr 25 '22 at 03:55
  • @CosmeticMichu No, even `const int n` won't work if the right hand side(i.e., the initializer) isn't a constant expression. Also, you can try out your example [here live](https://onlinegdb.com/HXcMgKjkw) and share a link to that so that we can also verify if you're getting the said output/error. Try to create a very minimal example that reproduces your problem. And better would be to use `std::vector`. – Jason Apr 25 '22 at 03:57
  • @CosmeticMichu -- [Start with this](http://coliru.stacked-crooked.com/a/6af771c9a332189e). Now add to that code what you are trying to accomplish in terms of passing and returning a 2D array. Once you do that, and when you are stuck, then post *that* code. – PaulMcKenzie Apr 25 '22 at 03:57
  • @CosmeticMichu *As you know, arrays in c++ are not beginners friendly,* -- `std::vector>` should have been chosen at first, since you want a dynamic array. All of that other stuff with variable length arrays is not C++. If you had chosen `std::vector>`, then you would have less issues (the code wouldn't be efficient, but that's another story). – PaulMcKenzie Apr 25 '22 at 04:01
  • @CosmeticMichu Also, even if you didn't want to use vector, you have no other choice except to use `new[]/delete[]`. And even then, if you really and truly want to have a contiguous memory layout of a 2D array while keeping the `[][]` syntax for element access, then [this answer may help you](https://stackoverflow.com/questions/21943621/how-to-create-a-contiguous-2d-array-in-c/21944048#21944048). – PaulMcKenzie Apr 25 '22 at 04:08
  • Thanks to you all for the feedback, I rewrote my question, I hope it fits better to the guidelines. I will check the vector implementation some of you mentioned and @JaMiT I will have in mind C, actually I just learned something about c++ in college but I'm not a software engineer student, I'm from physics so my knowledge is not very deep – CosmeticMichu Apr 25 '22 at 04:41

2 Answers2

2

In your second function, you have float *I = 0. Later on, you try to write to this array but you have not allocated it. The way you're indexing your matrices is the flattening approach so you must write float *I = new float[n*n]. There are different approaches, of course, like using dynamic 2D arrays, 2D vectors, etc. as mentioned in the comments.

nasy
  • 78
  • 3
  • 7
  • Yep it worked, thank you very much. But there is a memory leak, do you know how do I fix that? I changed exactly what you said. I will try the vector approach anyways since it was mentioned multiple times – CosmeticMichu Apr 25 '22 at 04:56
  • 1
    @CosmeticMichu When you use dynamic arrays, memory is allocated on the heap for your program and will not be freed until the program ends or you delete it yourself during the program. You should deallocate it using `delete[] arrName` (since you used `new[]`) after you are done with the array, for example after printing out the result. – nasy Apr 25 '22 at 05:06
0

The problem is that the pointer I doesn't point to any object and you've the following statement:

*(I+i*n+j) = 1.0; //undefined behavior

The above statement leads to undefined behavior. Imagine what happens when i and j are both 0 in the first iteration. Then you're dereferencing I which doesn't point to any float object and thus this is undefined behavior.

Undefined behavior means anything1 can happen including but not limited to the program giving your expected output. But never rely(or make conclusions based) on the output of a program that has undefined behavior. The program may just crash.

So the output that you're seeing(maybe seeing) is a result of undefined behavior. And as i said don't rely on the output of a program that has UB. The program may just crash which happens in your case.

So the first step to make the program correct would be to remove UB. Then and only then you can start reasoning about the output of the program.

Also, it would be better to use std::vector than doing manual memory management using new and delete.

1For a more technically accurate definition of undefined behavior see this where it is mentioned that: there are no restrictions on the behavior of the program.

Jason
  • 36,170
  • 5
  • 26
  • 60