1

I have been using the following function for quite some time:

void AddRow(int iNumOfColumns,...)
{
    int* pValuePerColumn = (int*)&iNumOfColumns+1;

    for (int i=0; i<iNumOfColumns; i++)
    {
        // Do something with pValuePerColumn[i]
    }
}

Now it turns out that it crashes on Win64 for one of our customers.

I do not have a 64-bit platform at hand, but I am assuming that the reason is:

When the function is invoked, the arguments are pushed into the stack as 64-bit values.

Under this assumption, I believe that replacing int* with size_t* should resolve the problem.


My questions are:

  • Is my analysis correct?
  • Is my solution correct?
  • Is there a more "conventional" way for solving this?
barak manos
  • 29,648
  • 10
  • 62
  • 114
  • 3
    You're familiar with stdarg.h? – 2501 Jul 21 '16 at 08:28
  • 1
    @2501: Yes, I assume that you're implying for the use of `va_list` and `va_args`? – barak manos Jul 21 '16 at 08:33
  • You should use va_args for portability. See [Microsoft x86 calling convention](https://en.wikipedia.org/wiki/X86_calling_conventions#Microsoft_x64_calling_convention) for more info. Anyways using stdarg.h makes your compiler take care of that. – Felix Bytow Jul 21 '16 at 09:30

2 Answers2

5

Derefencing a pointer to one past the last element of the array, or a non array object, is undefined behavior:

int* pValuePerColumn = (int*)&iNumOfColumns+1;
...
pValuePerColumn[i]

Changing the type to size_t is irrelevant for this problem.

The only correct way of using variable arguments are macros provided in stdarg.h.

2501
  • 25,460
  • 4
  • 47
  • 87
  • Thank you. An example of how to iterate the arguments would make the answer perfect (though I'm fine without it). – barak manos Jul 21 '16 at 08:38
  • BTW, "Derefencing a pointer to one past the last element... is UB" - Isn't that what `stdarg.h` does anyway? Or is it a platform-dependent issue since the language standard does not define how function arguments are pushed into the stack (in other words, is a unique version of `stdarg.h` provided along with every compiler)? – barak manos Jul 21 '16 at 08:42
  • @barakmanos It's implemented differently for every architecture. – 2501 Jul 21 '16 at 08:48
  • @barakmanos You should read this: https://stackoverflow.com/questions/23104628/technically-how-do-variadic-functions-work-how-does-printf-work?rq=1 – 2501 Jul 21 '16 at 08:56
  • Thanks, very detailed description indeed! – barak manos Jul 21 '16 at 09:07
  • I would like to accept your answer, I think it might be good if you add a coding example (for the sake of this community, and due to the fact that the other answer does, which makes it kinda difficult for me to choose one over the other). – barak manos Jul 21 '16 at 16:10
  • The other answer already provided an example, I don't see the need to add another one. – 2501 Jul 21 '16 at 16:12
  • So which one should I accept then? Yours refers pretty well to my first two bullets, the answer below refers better to my last bullet. – barak manos Jul 21 '16 at 16:14
  • The one that answered your question. – 2501 Jul 21 '16 at 16:16
3

You should use varargs to access extra parameters in portable way. Look for va_list docs. Probably your code should look next

void AddRow(int iNumOfColumns,...)
{
    va_list ap;

    va_start(ap, iNumOfColumns);
    for (int i=0; i<iNumOfColumns; i++)
    {
        int col = va_arg(ap, int);
        // Do something with col
    }

    va_end(ap);
}

And as I remember on Win64 first four integer args are passed via registers, not via the stack, so tricks with pointers won't work.

Sergio
  • 8,099
  • 2
  • 26
  • 52
  • Where in my question did you see any use of `varargs`? – barak manos Jul 21 '16 at 08:59
  • OK, thank you for the coding example. You should probably remove the opening statement, since I have not mentioned `varargs` anywhere in my question. – barak manos Jul 21 '16 at 09:03
  • @barakmanos There is "Variadic function" in the title, isn't it? And it seems like your code is trying to acess arguments passed after `iNumOfColumns`. – Sergio Jul 21 '16 at 09:03
  • Yes, well, I wouldn't call it `varargs`... Anyway, it's just a matter of terminology I suppose... Thanks again. – barak manos Jul 21 '16 at 09:04
  • @barakmanos Oh, yes. Opening phrase was really confusing. Thanks. – Sergio Jul 21 '16 at 09:12