Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

This snippet is about detecting whether the carry flag is set or cleared. I work on Mac OSX, so my snippet supports Mac only.

First, we need a routine that does the job:

func.s:

global _read_carry_flag

_read_carry_flag:
    mov al, 0
    jnc end 
    mov al, 1
end:
    ret

(Try nasm2 -f macho64 func.s for compiling into an object file.)

main.c:

#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>

#define A (2 * 1000 * 1000 * 1000)
#define B (1 * 1000 * 1000 * 1000)

extern bool read_carry_flag();

int main(int argc, char* argv[])
{
    int32_t a = A;
    int32_t b = B;
    int32_t ret = a + a + b;

    printf("%d\n", read_carry_flag());

    a = A;
    b = 1;
    ret = a + b;

    printf("%d\n", read_carry_flag());
    return 0;
}

(Try gcc -o prog main.c func.o for obtaining a process image.)

I would like to hear about possible improvements/extensions to the idea.

share|improve this question
    
Hint: You can run nasm with --prefix_ and remove the '_' in front of the symbols. This way the code is prefix-agnostic and doesn't care about macho/elf/etc. – larkey Jan 7 at 8:18
up vote 12 down vote accepted

Wrong flag

I believe you should be looking at the overflow flag instead of the carry flag, since all of your operands are signed values. On x86, the overflow flag is set if signed addition overflows. The carry flag is set if unsigned addition overflows.

Not reliable

As @Edward pointed out, it doesn't seem reliable to use this kind of function because you never know how the compiler is going to rearrange your code. Even without the compiler rearranging your code, the results could be confusing. From your own example:

ret = a + a + b;
overflow = read_overflow_flag();

Here, you are only detecting overflow over the second of the two additions. If the first addition overflowed but the second didn't, you wouldn't catch it. In other words, the assembly might look like this:

add %ecx, %edx, %edx  // ret = a + a  <- overflow not detected here
add %ecx, %ecx, %ebx  // ret = ret + b
seto %al              // overflow = overflow flag

To do it correctly, I would suggest you use a function that adds two numbers together and updates a cumulative overflow:

// If the add overflows, 1 will be added to *pOverflow.
int32_t add32_with_overflow(int32_t x, int32_t y, int *pOverflow);

int32_t ret      = 0;
int     overflow = 0;

ret = add32_with_overflow(a,   a, &overflow);
ret = add32_with_overflow(ret, b, &overflow);
printf("Overflow = %d\n", overflow);
share|improve this answer

It can be made 2 bytes shorter by using the ADC instruction and avoiding the jmp (which should make it faster as well, but there's not much point to that in this case).

global _read_carry_flag

_read_carry_flag:
    mov al, 0
    adc al, 0
    ret

As @JS1 correctly points out in a comment, there's an even shorter method:

_read_carry_flag:
    setc al
    ret

A bigger issue is whether such a mechanism can be reliably used to detect overflow. It implicitly relies on the compiler to not generate any code that might alter the flag register after the addition is performed. That is likely to be true in this simple code, but it's not at all certain to me that one can generally rely on this.

share|improve this answer
2  
I believe there is an even shorter way: setc al. I agree with your concern about the reliability of such a function as a separate entity. It would be best to actually perform the add inside the function. – JS1 Jan 6 at 20:23

A possible improvement in read_carry_flag():

It might be worthwhile to either xor the eax register with itself before moving the return value into it, or just mov the return value into eax instead of al which will zero out the upper 32 bits of rax. The reason being the compiler may have used the *ax register before calling your function, and there could still be a value in there that will interfere with the return value of your function.

You pass the return value of your function directly to printf(). In x86-64 ABI, the first several integral arguments are passed in 8-byte registers, so the return value from read_carry_flag() will be passed in the rsi register. I don't know how exactly the compiler will move the return value into the register, but according to this SO answer the size of the bool type can be larger than one byte.

So if for example sizeof bool == 2, then the compiler can do:

call _read_carry_flags
mov rdi, formatString
mov si, ax
mov eax, 0
call printf

(The mov eax, 0 instruction is needed to signal printf() that no further arguments are passed on the stack)

If ax had the value 0x100 before your function was called and your function executed

mov al, 0

After your function was called, ax would still contain the value 0x100 and that would be passed to printf() instead of 0.

share|improve this answer

When adding 2 32-bits numbers and attempting to detect overflow, code should be profiled against the most direct alternative. An optimizing compiler will often make performance acceptable (and far easier to maintain) code.

int32_t a = A;
int32_t b = B;

// int32_t ret = a + a + b;
// printf("%d\n", read_carry_flag());

int64_t ret = (int64_t) a + a + b;
printf("%d\n", ret < INT32_MIN || ret > INT32_MAX);

Else, without going to wider types, code could use the highly portable:

// Does a+b overflow?
if (a >= 0) {
  if (b > INT32_MAX - a) Over();
} else {
  if (b < INT32_MIN - a) Under();
}
sum = a + b;
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.