7

I currently trying to learn assembly language. But I'm stuck. Suppose I have this C code:

for ( int i = 100; i > 0; i-- ) {
  // Some code
}

Now I want to do the same in assembly language. I tried it like this:

__asm__ ("movq $100, %rax;"
        ".loop:"
        //Some code
        "decq %rax;"
        "cmpq $0, (%rax);"
        "jnz .loop;"
);

Compile and run results in a seg fault. It does not seg fault if I remove the cmpq line. But then of course the program will not terminate.

So basically my question is what am I doing wrong here?

Alexis Wilke
  • 19,179
  • 10
  • 84
  • 156
Giganull
  • 89
  • 1
  • 4
  • 2
    Plain `asm` is unlikely what you want if you're writing inline assembly. Maybe you need to learn the `extended asm` https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html. – llllllllll Feb 03 '18 at 14:21
  • 2
    wrong: 1) memory access as described in answers 2) different loop kind (your asm is `i = 100; do { .. } while (--i);`, not the `for`. 3) that `cmp` is not needed at all, `dec` instruction will set also zero flag, so `decq %rax` `jnz .loop` is enough. "will not terminate" = some other problem 4) inline asm is harder than plain asm, maybe learn x86-64 assembler basics first before moving to inline asm in C 5) use some tutorial+books to at least understand your syntax ("gas" or "AT&T") and of utmost importance is instruction reference guide. ... this is cca. "low effort" question to me. – Ped7g Feb 03 '18 at 17:11
  • just one more note: if you have the code already in C, use the compiler to see how it does compile it. It may be tricky to force him to both use optimizations, but preserve your code, if it is not doing anything observable, so rather check with real app code, do some printf of results, etc, to avoid complete code removal from optimizer. – Ped7g Feb 03 '18 at 17:13
  • 3
    Beyond the actual answer. If you alter registers inside inline assembly you need to tell the compiler (especially if you turn on optimizations). If you modify RAX iside inline assembly the compiler may assume that RAX is the same before and after your code is run. If you modify it this can cause the program to run improperly. You need to look into [GCC's extended inline assembly](https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html). At a minimum you'll need clobbers for all the registers you modify. – Michael Petch Feb 03 '18 at 17:43
  • @Ped7g: Of course it's a `do{}while()`, [that's the correct and natural way to implement a for loop over a compile-time-constant range](https://stackoverflow.com/questions/47783926/why-are-loops-always-compiled-like-this/47790760#47790760), or any asm loop that always runs at least one iteration. – Peter Cordes Feb 03 '18 at 20:23
  • @PeterCordes sure... but the C is `for`. :) And the optimizer will change it into `do {} while`, because it is smart, not because the C source is written like that. I mean, we probably agree about everything, I just wrote from different perspective? – Ped7g Feb 03 '18 at 20:27
  • @Ped7g: You're arguing that there should be a `test`/`jcc` at the top of the loop, and a `jmp` at the bottom, as if you'd transliterated the C abstract machine equivalent into asm. But why would anybody ever want a loop like that in asm? Semantically I think what the OP wanted is a loop over a range, not *actually* a transliteration of how it's normally written in C (with a redundant compare before the first iteration). C doesn't have a range-for syntax that would express it even more directly. – Peter Cordes Feb 03 '18 at 22:37
  • @Ped7g: Anyway, yes I see your point, but encouraging or suggesting that transliterating C constructs to asm is a good idea instead of making nice efficient idiomatic loops in asm that do what you actually want is not helpful. The OP is already on the right track with loop structure. It's the last thing to mention, after the un-safety of doing this without clobbers in inline asm, etc. – Peter Cordes Feb 03 '18 at 22:43
  • @PeterCordes uh, that's misunderstanding, I was not suggesting to model the asm loop after the C original, I was just clearing up where the OP was wrong. Would he state it was intentional in the comments, he would be correct. But as it was not stated, I can't know, whether the OP realized what he did. I'm glad you posted about it being the better way, I can agree on that easily. (I was after the formal "wrong", not the result) – Ped7g Feb 03 '18 at 22:52
  • @Ped7g: I disagree with calling the loop structure "wrong" in any sense. It makes more sense to assume the OP just wants the loop body to run with rax=100..1, i.e. do the same thing as the C code. I don't think un-optimized compiler output (with a compare at the top) is the only kind of asm that would be "equivalent" to that C. – Peter Cordes Feb 03 '18 at 23:07

3 Answers3

9
cmpq $0, (%rax)

This instruction will try to read memory at the address in rax.

rax will be 99 the first time. Address 99 is not mapped in, so your program segfaults.

You are intending to compare the value in rax to 0, so remove the parentheses.

cmpq $0, %rax
Jonathon Reinhart
  • 132,704
  • 33
  • 254
  • 328
8

The following instruction: 

cmpq $0, (%rax)

is accessing the memory address specified by the rax register, whose value is 99.

The first memory page is not mapped. That memory address, 99, belongs to the first memory page. Therefore, the access above results in a segmentation fault.

You don't want the indirection, instead you want:

cmpq $0, %rax

That is, you want to compare against the contents of rax, not the contents at the memory address specified by rax.

Consider however optimizing the cmp instruction away:

decq %rax is immediately preceding the cmp $0, %rax instruction, which sets ZF if rax is zero. The conditional jump is then performed based on the state of the ZF flag:

decq %rax
cmpq $0, %rax
jnz .loop

The dec instruction affects the ZF flag (as cmp does), so if decrementing rax results in zero, ZF will be set. You can take advantage of that fact and place jnz directly after dec. You don't need cmp at all:

decq %rax
jnz .loop
JFMR
  • 23,265
  • 4
  • 52
  • 76
  • 1
    Instead you *really* want to `jnz` on the flags already set by `dec`. – Peter Cordes Feb 03 '18 at 20:25
  • Right, the whole instruction can be optimized away. – JFMR Feb 03 '18 at 20:39
  • 1
    Related: when you *do* need to re-test a register for zero/non-zero, [`test %rax,%rax` is (usually) the optimal way](https://stackoverflow.com/questions/33721204/test-whether-a-register-is-zero-with-cmp-reg-0-vs-or-reg-reg/33724806#33724806); it sets FLAGS the same as `cmp $0, %rax` would. Anyway, upvoted now that you addressed part of the XY problem, but only MichaelPetch's comment addressed the issue of putting this in a GNU C "Basic" asm (unsafe because there's no clobber list) – Peter Cordes Feb 03 '18 at 22:30
3

Remove parenthesis from %rax to get the value of rax. Adding the parenthesis basically tells the assembler, "hey! rax holds an address, please return the contents in that address".

Therefore,

cmpq $0, %rax

is what you need to do.

Muzahir Hussain
  • 1,009
  • 2
  • 16
  • 35