0

The codes:

#include <stdio.h>
#include <stdarg.h>
#include <stdlib.h>

typedef unsigned int uint32_t;

float average(int n_values, ... )
{
    va_list var_arg; 
    int count;
    float sum = 0;

    va_start(var_arg, n_values);

    for (count = 0; count < n_values; count += 1) {
        sum += va_arg(var_arg, signed long long int);
    }   

    va_end(var_arg);

    return sum / n_values;
}

int main(int argc, char *argv[])
{
    (void)argc;
    (void)argv;

    printf("hello world!\n");

    uint32_t t1 = 1;  
    uint32_t t2 = 4;  
    uint32_t t3 = 4;  
    printf("result:%f\n", average(3, t1, t2, t3));

    return 0;
}

When I run in ubuntu (x86_64), It's Ok.

lix@lix-VirtualBox:~/test/c$ ./a.out 
hello world!
result:3.000000
lix@lix-VirtualBox:~/test/c$ uname -a
Linux lix-VirtualBox 4.4.0-116-generic #140-Ubuntu SMP Mon Feb 12 21:23:04 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
lix@lix-VirtualBox:~/test/c$ 

But when I cross-compiler and run it in openwrt(ARM 32bit), It's wrong.

[root@OneCloud_0723:/root/lx]#./helloworld 
hello world!
result:13952062464.000000
[root@OneCloud_0723:/root/lx]#uname -a
Linux OneCloud_0723 3.10.33 #1 SMP PREEMPT Thu Nov 2 19:55:17 CST 2017 armv7l GNU/Linux

I know do not call va_arg with an argument of the incorrect type. But Why we can get right result in x86_64 not in arm?

Thank you.

lxgeek
  • 1,732
  • 2
  • 22
  • 33
  • 2
    It's probably because for arm a `long long int` has a different size as for x86_64. Your `var_arg` should be on both systems `va_arg(var_arg, uint32_t)`, only that would yield correct results. – Pablo Mar 01 '18 at 02:58
  • 3
    Undefined behaviour can lead to things appearing to work, and it can lead to things breaking. You’re lucky you’ve got one of each so now you can fix it. – Jonathan Leffler Mar 01 '18 at 03:08
  • @Pablo long long int also has 8 bytes on arm, I test it. Thank you. – lxgeek Mar 01 '18 at 05:45

2 Answers2

7

On x86-64 Linux, each 32-bit arg is passed in a separate 64-bit register (because that's what the x86-64 System V calling convention requires).

The caller happens to have zero-extended the 32-bit arg into the 64-bit register. (This isn't required; the undefined behaviour in your program could bite you with a different caller that left high garbage in the arg-passing registers.)

The callee (average()) is looking for three 64-bit args, and looks in the same registers where the caller put them, so it happens to work.


On 32-bit ARM, long long is doesn't fit in a single register, so the callee looking for long long args is definitely looking in different places than where the caller placed uint32_t args.

The first 64-bit arg the callee sees is probably ((long long)t1<<32) | t2, or the other way around. But since the callee is looking for 6x 32 bits of args, it will be looking at registers / memory that the caller didn't intend as args at all.

(Note that this could cause corruption of the caller's locals on the stack, because the callee is allowed to clobber stack args.)


For the full details, look at the asm output of your code with your compiler + compile options to see what exactly what behaviour resulted from the C Undefined Behaviour in your source. objdump -d ./helloworld should do the trick, or look at compiler output directly: How to remove "noise" from GCC/clang assembly output?.

Peter Cordes
  • 328,167
  • 45
  • 605
  • 847
  • This is a nice answer, I haven't dealt with calling conventions so I didn't even think about these things. – Pablo Mar 01 '18 at 03:26
1

On my system (x86_64)

#include <stdio.h>


int main(void)
{
    printf("%zu\n", sizeof(long long int));

    return 0;
}

this prints 8, which tells me that long long int is 64bits wide, I don't know the size of a long long int on arm.

Regardless your va_arg call is wrong, you have to use the correct type, in this case uint32, so your function has undefined behaviour and happens to get the correct values. average should look like this:

float average(int n_values, ... )
{
    va_list var_arg; 
    int count;
    float sum = 0;

    va_start(var_arg, n_values);

    for (count = 0; count < n_values; count += 1) {
        sum += va_arg(var_arg, uint32_t);
    }   

    va_end(var_arg);

    return sum / n_values;
}

Also don't declare your uint32_t as

typedef unsigned int uint32_t;

this is not portable, because int is not guaranteed to be 4 bytes long across all architectures. The Standard C Library actually declares this type in stdint.h, you should use the thos types instead.

So you program should look like this:

#include <stdio.h>
#include <stdarg.h>
#include <stdlib.h>
#include <stdint.h>

float average(int n_values, ... )
{
    va_list var_arg; 
    int count;
    float sum = 0;

    va_start(var_arg, n_values);

    for (count = 0; count < n_values; count += 1) {
        sum += va_arg(var_arg, uint32_t);
    }   

    va_end(var_arg);

    return sum / n_values;
}

int main(void)
{
    printf("hello world!\n");

    uint32_t t1 = 1;  
    uint32_t t2 = 4;  
    uint32_t t3 = 4;  
    printf("result:%f\n", average(3, t1, t2, t3));

    return 0;
}

this is portable and should yield the same results across different architectures.

Pablo
  • 13,271
  • 4
  • 39
  • 59
  • To be standard compliant, the size on ARM must be at least 64 bits. I suppose that could use 128 bits, but it’s a bit unlikely. – Jonathan Leffler Mar 01 '18 at 03:10
  • @JonathanLeffler I don't have an armv7 processor right now at my desk, I cannot test it. But based on http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/Babfcgfc.html a `long long` is 64bit wide. The OPs code is using the wrong types anyway. – Pablo Mar 01 '18 at 03:14
  • IDK why you think `long long` might not be 8 on 32-bit ARM. You can easily test it by looking at ARM compiler output for a function that returns the `sizeof`: https://godbolt.org/g/pZZW74. I'm not aware of any implementation where `long long` is wider than it needs to be to represent the range of value required by ISO C, so it's pretty universally a 64-bit type. – Peter Cordes Mar 01 '18 at 03:20
  • This doesn't answer the question. The question is asking what exactly happened, not how to fix it. The OP already knows that a type mismatch is UB. (Although you're right to point out using your own `typedef` for `uint32_t` is an error that the OP didn't seem to realize.) – Peter Cordes Mar 01 '18 at 03:22
  • @PeterCordes - You can find some rare 36-bit computers where a `long long` is using 72 bits. https://stackoverflow.com/a/6972551/597607 So *pretty* universally, but not totally. – Bo Persson Mar 01 '18 at 04:52
  • @BoPersson: Neat, I'd forgotten there were still 36-bit word-size machines being made/sold. (But this is still technically compatible with what I said: `long long` needs to take 2 full words (`sizeof`=8) on that hardware. Even one 9-bit `char` less than 2 full words / 72 bits would make it only 63 bits. If e.g. word + half-word was sufficient for the required range, then there'd be a tradeoff between alignment vs. wasted space, but making a few of those 72 bits into padding instead of value bits would pointlessly require AND instructions for unsigned, and not be smaller) – Peter Cordes Mar 01 '18 at 05:08