Help! Program crashed when call the inline assembly function continuously

Help! Program crashed when call the inline assembly function continuously

[spark.z]

Eclipse + gcc-4.4.1, compiled the program with optimization flag -O2
I wrote a inline function with inlined assembly
void pincrement(int* target)
{
    __asm__ __volatile__ ("pushl %eax");
    __asm__ __volatile__ ("lock ; incl (%%eax)" ::"a"(target));
    __asm__ __volatile__ ("popl %eax");
}

and call it in the main function like:
int main()
{
  static int my_an = 0;
  pincrement(&my_an);
  pincrement(&my_an);
  pincrement(&my_an);
  printf("my_an is %d\n",my_an);
}

It crashed when run the app. So I check the assembly code of the app. The
complier translate the three pincrement call become:

push    eax
mov     eax, offset dword_474048
lock inc dword ptr [eax]
pop     eax

push    eax
lock inc dword ptr [eax]
pop     eax

push    eax
lock inc dword ptr [eax]
pop     eax

It only has one "mov     eax, offset dword_474048" sentence in it. 
How can I tell the gcc complier to inline the whole assembly function three
times completely? 
:working:
-- 
View this message in context: http://old.nabble.com/Help%21-Program-crashed-when-call-the-inline-assembly-function-continuously-tp28094526p28094526.html
Sent from the gcc - Help mailing list archive at Nabble.com.

Re: Help! Program crashed when call the inline assembly function continuously

[Cedric Roux]

spark.z wrote:
> Eclipse + gcc-4.4.1, compiled the program with optimization flag -O2
> I wrote a inline function with inlined assembly
> void pincrement(int* target)
> {
>     __asm__ __volatile__ ("pushl %eax");
>     __asm__ __volatile__ ("lock ; incl (%%eax)" ::"a"(target));
>     __asm__ __volatile__ ("popl %eax");
> }

void pincrement(int* target)
{
    __asm__ __volatile__ ("pushl %%eax");
    __asm__ __volatile__ ("lock ; incl (%%eax)" ::"a"(target));
    __asm__ __volatile__ ("popl %%eax":::"eax");
}

seems to do the trick. (the third asm statement adds eax in the clobber list)
(and I also add to write %%eax instead of your %eax).

You should tell gcc what your asm statement modifies.
Here, the pop modifies eax.

But even the first asm statement modifies the stack
and we don't tell gcc, but does that matter? I don't know.

And the second also modifies *target, maybe we should tell
gcc too here.

Anyway, you've got the idea. Maybe someone else will have a
better answer. And you should read the gcc documentation
about "Extended Asm."

Re: Help! Program crashed when call the inline assembly function continuously

[spark.z]

Many thanks to you, the app works correctly according to your
sulotion:handshake:

But I am confused with another situation as below (static int my_an = 0 ===>
int my_an = 0)
int main() 
{ 
  int my_an = 0; 
  pincrement(&my_an); 
  pincrement(&my_an); 
  pincrement(&my_an); 
  printf("my_an is %d\n",my_an); 
} 

it show me a wrong result => "my_an is 0". Is there any differences between
the local variable and the static variable as the param? :confused:
-- 
View this message in context: http://old.nabble.com/Help%21-Program-crashed-when-call-the-inline-assembly-function-continuously-tp28094526p28097083.html
Sent from the gcc - Help mailing list archive at Nabble.com.

Re: Help! Program crashed when call the inline assembly function continuously

[Cedric Roux]

spark.z wrote:
> Many thanks to you, the app works correctly according to your
> sulotion:handshake:
> 
> But I am confused with another situation as below (static int my_an = 0 ===>
> int my_an = 0)
> int main() 
> { 
>   int my_an = 0; 
>   pincrement(&my_an); 
>   pincrement(&my_an); 
>   pincrement(&my_an); 
>   printf("my_an is %d\n",my_an); 
> } 
> 
> it show me a wrong result => "my_an is 0". Is there any differences between
> the local variable and the static variable as the param? :confused:

If i write:

void pincrement(long* target)
{
    __asm__ __volatile__ ("pushq %rax");
    __asm__ __volatile__ ("lock ; incb (%%rax)" :"=m" (*target) :"a"(target), "m" (*target));
    __asm__ __volatile__ ("popq %%rax":::"rax");
}

(64b, I'm on a 64b, adapt to your needs, it's more or less the same)

some code is produced.

But it is compiled to (just the beginning):
0x0000000000400540 <main+0>:    sub    $0x18,%rsp
0x0000000000400544 <main+4>:    movq   $0xa,0x8(%rsp)
0x000000000040054d <main+13>:   push   %rax
0x000000000040054e <main+14>:   lea    0x8(%rsp),%rdx
0x0000000000400553 <main+19>:   mov    %rdx,%rax
0x0000000000400556 <main+22>:   lock incb (%rax)
0x0000000000400559 <main+25>:   pop    %rax

See what's wrong?
the first asm statement of pincrement modifies the stack pointer.
gcc doesn't know that. The second asm statement loads target by doing
"lea 0x8(%rsp), %rdx"
problem is: rsp has changed, so 0x8(%rsp) does not point anymore to target.

So one answer to your question is: add "=m" and "m" to your list of input/output
(I don't know which one is absolutely necessary, maybe both).
(And read the gcc documentation on "extended asm," it's all explained.)

But!

Your code still won't work because of the first push which modifies the stack
pointer. gcc doesn't know and on the second asm statement uses the old value.
Depending on how gcc will behave, it will work or not (it may access target
through ebp instead of esp for example, which will work).

I don't know if there is an answer to your problem. Maybe there is something to
add in the clobber list to tell the stack pointer moved, and moved of xxx bytes
in this or that direction.

Anyway, why the push/pop? gcc will know eax is modified since it's used as input
parameter in the asm statement. It will adapt itself automatically.
Or do you plan to use pincrement in another context where you absolutely
need eax not to change inside of pincrement? (in which case, maybe it's
better to write it directly in assembly).

Re: Help! Program crashed when call the inline assembly function continuously

[spark.z]

The app works fine if I add the volatile prefix before the variable
declaration.

inline void pincrement(volatile int* target)
{
    __asm__ __volatile__ ("pushl %%eax":::);
    __asm__ __volatile__ ("lock ; incl (%%eax)" :"=m"(target):"a"(target));
    __asm__ __volatile__ ("popl %%eax":::"eax");
}

inline void pdecrement(volatile int* target)
{
    __asm__ __volatile__ ("pushl %%eax":::);
    __asm__ __volatile__ ("lock ; decl (%%eax)" :"=m"(target):"a"(target));
    __asm__ __volatile__ ("popl %%eax":::"eax");
}


int main() {
	volatile int my_an = 0;
	pincrement(&my_an);
	pincrement(&my_an);
	pincrement(&my_an);
	pdecrement(&my_an);
	printf("my_an is %d\n",my_an);
	return 0;
}

-- 
View this message in context: http://old.nabble.com/Help%21-Program-crashed-when-call-the-inline-assembly-function-continuously-tp28094526p28107792.html
Sent from the gcc - Help mailing list archive at Nabble.com.

Re: Help! Program crashed when call the inline assembly function continuously

[Cedric Roux]

spark.z wrote:
> The app works fine if I add the volatile prefix before the variable
> declaration.
> 
> inline void pincrement(volatile int* target)
> {
>     __asm__ __volatile__ ("pushl %%eax":::);
>     __asm__ __volatile__ ("lock ; incl (%%eax)" :"=m"(target):"a"(target));
>     __asm__ __volatile__ ("popl %%eax":::"eax");
> }
> 
> inline void pdecrement(volatile int* target)
> {
>     __asm__ __volatile__ ("pushl %%eax":::);
>     __asm__ __volatile__ ("lock ; decl (%%eax)" :"=m"(target):"a"(target));
>     __asm__ __volatile__ ("popl %%eax":::"eax");
> }
> 
> 
> int main() {
> 	volatile int my_an = 0;
> 	pincrement(&my_an);
> 	pincrement(&my_an);
> 	pincrement(&my_an);
> 	pdecrement(&my_an);
> 	printf("my_an is %d\n",my_an);
> 	return 0;
> }
> 

try to compile like: gcc myprog.c -fomit-frame-pointer
does it still work? (not for me with gcc 3.3.5, I have
a segmentation fault)

your "push eax" statements are not correct because you
don't tell gcc they modify the stack pointer.

It works because you are lucky, but your code is
broken. One day or another it will crash.

Re: Help! Program crashed when call the inline assembly function continuously

[Ian Lance Taylor]

"spark.z" <spark.yj.zhao@gmail.com> writes:

> void pincrement(int* target)
> {
>     __asm__ __volatile__ ("pushl %eax");
>     __asm__ __volatile__ ("lock ; incl (%%eax)" ::"a"(target));
>     __asm__ __volatile__ ("popl %eax");
> }

Besides all the other answers, don't write your asm code like this.
Put all the assembler code in a single asm statement rather than using
separate asm statements.

In any case the pushl and popl appear to be unnecessary here.

Also you should look into the __sync_fetch_and_add and
__sync_add_and_fetch builtin functions.

Ian

Re: Help! Program crashed when call the inline assembly function continuously

[spark.z]

Thank you for reminding me of this potential risk. :handshake:
-- 
View this message in context: http://old.nabble.com/Help%21-Program-crashed-when-call-the-inline-assembly-function-continuously-tp28094526p28178214.html
Sent from the gcc - Help mailing list archive at Nabble.com.

Re: Help! Program crashed when call the inline assembly function continuously

[spark.z]

Many thanks to you for this new solution.
-- 
View this message in context: http://old.nabble.com/Help%21-Program-crashed-when-call-the-inline-assembly-function-continuously-tp28094526p28178271.html
Sent from the gcc - Help mailing list archive at Nabble.com.