Error with simple program

[master94ga]

Hello, I did a simple program that have to count and print the numbers of uppercase letters but doesn't work, can you please help me.

Code: [Select]

section .data
stringa db "Hello", 0ah, 0 ; string where I have to count the uppercase letters

section .bss
cont resb 1 ;numbers of uppercase letters

section .text

global _start

_start:

call ini
call conta
call fine

ini:
        mov eax, 0
mov ecx, 0
mov esi, 0
ret

conta:
cmp eax, 0ah ; compare eax that at start is "0" but after contain the character of the string
je stampa ; when it find "0ah" that is after all characthers it have go to "stampa" and print the numbers of uppercase letters
movzx eax, byte[stringa+ecx] ; put in eax the first character, after the second and etc...
inc ecx
cmp eax, 5Bh ; compare eax with "5Bh", the last uppercase letters in ascii code is 5A
jl incrementa ; if is lower jump to "incrementa"
jg conta ; if is  greater jump to "conta"
ret

incrementa: ; increment "cont", that contain the numbers of uppercase characther
        inc esi
jmp conta

stampa: ;print the numbers of uppercase characther
add esi, 30h
        mov [cont], esi

mov eax,4
mov ebx,1
mov ecx, cont
mov edx, 1
int 80h
ret

fine: ;close the program
mov eax, 1
mov ebx, 0
int 80h

[gammac]

I've stopped reading at the first mistake.

Code: [Select]

conta:
cmp eax, '0ah'

at the first time you come to this point, eax is uninitilazid! eax holds a random value.

[master94ga]

I've stopped reading at the first mistake.

Code: [Select]

conta:
cmp eax, '0ah'

at the first time you come to this point, eax is uninitilazid! eax holds a random value.

Thanks, I updated the code, now I get "segmentation error"

[gammac]

you do not compare eax with the value 0ah you compares it with the string '0ah'. I think, this string is 3 bytes long and the assembler makes a 4 byte value out of this. But thats not what you want.

try to comment your code, it makes things clearer! For you and for others.

[master94ga]

you do not compare eax with the value 0ah you compares it with the string '0ah'. I think, this string is 3 bytes long and the assembler makes a 4 byte value out of this. But thats not what you want.

try to comment your code, it makes things clearer! For you and for others.

Yes, I think the problem is in the compare, I have to find a way to end and go to "stampa" when it finish to check the entire string, I added comments.

[gammac]

Code: [Select]

conta:
cmp eax, '0ah' ; compare eax that at start is "0" but after contain the character of the string
...
cmp eax, 5Bh ; compare eax with "5Bh", the last uppercase letters in ascii code is 5A

you still compare eax with a string and not with the newline character. see the difference between your both comparisons.

btw: I don't had a look at your jcc mnemonics up to now.

Code: [Select]

incrementa: ; increment "cont", that contain the numbers of uppercase characther
inc esi
add esi, 30h
        mov [cont], esi

and here you doesn't increment you add the decimal value 49 to [cont]. But only then, if esi allway contain the value of [cont] and esi has not been altered elsewhere.

[master94ga]

Code: [Select]

conta:
cmp eax, '0ah' ; compare eax that at start is "0" but after contain the character of the string
...
cmp eax, 5Bh ; compare eax with "5Bh", the last uppercase letters in ascii code is 5A

you still compare eax with a string and not with the newline character. see the difference between your both comparisons.

btw: I don't had a look at your jcc mnemonics up to now.

Code: [Select]

incrementa: ; increment "cont", that contain the numbers of uppercase characther
inc esi
add esi, 30h
        mov [cont], esi

and here you doesn't increment you add the decimal value 49 to [cont]. But only then, if esi allway contain the value of [cont] and esi has not been altered elsewhere.

If I do

Code: [Select]

cmp eax, 0ahAs output I get "b", withouth changing "incrementa"
If in "incrementa", I don't do the

Code: [Select]

add esi, 30has output I get a strange character

Edit: problem almost fixed

[master94ga]

Code updated on top, now almost work, If it have to print 2 it print 3, if have to print 1 it print 2...
Now I try to fix it
I can fix with a

Code: [Select]

dec esi, but I don't think is a good solution

[Frank Kotler]

Actually, I think the very first error is only reserving a single byte for "cont", and then putting a dword in it. That isn't doing any harm right now, but it will if you put another variable after it.

Code: [Select]

section .data
stringa db "Hello", 0ah, 0

section .bss
cont resb 1 ; <- needs to be more!

section .text

global _start

_start:

call ini
call conta
call fine

ini:
        mov eax, 0
mov ecx, 0
mov esi, 0
ret

conta:
cmp eax, '0ah' ;
je stampa
movzx eax, byte[stringa+ecx]
inc ecx
cmp eax, 5Bh ; <- '['
jl incrementa
jg conta
ret

incrementa:
inc esi
add esi, 30h
mov [cont], esi
jmp conta
; this works the first time, but what happens
; when you encounter a second (or more) uppercase
; character? esi is already 31h - we bump it up to 32h
; and then add another 30h! not going to be the
; number you want...

stampa:
mov eax,4
mov ebx,1
mov ecx, cont
mov edx, 1
int 80h
ret

fine:
mov eax, 1
mov ebx, 0
int 80h

I think Gammac has nailed the cause of your segfault. The comparison with '0ah' is never going to succeed, so you keep incrementing ecx until you run off into memory that you don't "own". Go boom! You've probably already got that fixed.

I've noted a problem with counting the second (or more) uppercase character. Could fix that by subtracting 30h from esi again after putting it in "[cont]". There's another problem awaiting when you get to 10. That's the whole "how do I display a number?" question. We've covered that, but we can go over it again. Get it working for single digits first...

I absolutely agree that commenting your code will help. There's a guy over on Stack Overflow (Brendan?) who claims there are only two possible errors in assembly code. Either the comments don't describe a correct algorithm, or the code doesn't do what the comments say. An interesting way to look at it...

Well, I see you guys have gotten way ahead of me while I've been typing. Go for it!

Best,
Frank

[master94ga]

Actually, I think the very first error is only reserving a single byte for "cont", and then putting a dword in it. That isn't doing any harm right now, but it will if you put another variable after it.

Code: [Select]

section .data
stringa db "Hello", 0ah, 0

section .bss
cont resb 1 ; <- needs to be more!

section .text

global _start

_start:

call ini
call conta
call fine

ini:
        mov eax, 0
mov ecx, 0
mov esi, 0
ret

conta:
cmp eax, '0ah' ;
je stampa
movzx eax, byte[stringa+ecx]
inc ecx
cmp eax, 5Bh ; <- '['
jl incrementa
jg conta
ret

incrementa:
inc esi
add esi, 30h
mov [cont], esi
jmp conta
; this works the first time, but what happens
; when you encounter a second (or more) uppercase
; character? esi is already 31h - we bump it up to 32h
; and then add another 30h! not going to be the
; number you want...

stampa:
mov eax,4
mov ebx,1
mov ecx, cont
mov edx, 1
int 80h
ret

fine:
mov eax, 1
mov ebx, 0
int 80h

I think Gammac has nailed the cause of your segfault. The comparison with '0ah' is never going to succeed, so you keep incrementing ecx until you run off into memory that you don't "own". Go boom! You've probably already got that fixed.

I've noted a problem with counting the second (or more) uppercase character. Could fix that by subtracting 30h from esi again after putting it in "[cont]". There's another problem awaiting when you get to 10. That's the whole "how do I display a number?" question. We've covered that, but we can go over it again. Get it working for single digits first...

I absolutely agree that commenting your code will help. There's a guy over on Stack Overflow (Brendan?) who claims there are only two possible errors in assembly code. Either the comments don't describe a correct algorithm, or the code doesn't do what the comments say. An interesting way to look at it...

Well, I see you guys have gotten way ahead of me while I've been typing. Go for it!

Best,
Frank

Thanks for the reply Frank, but the code get updated
About "cont", how much I have to reserve for it? To be correct I mean

[gammac]

Code: [Select]

cont resb 1 ; <- needs to be more!

As Frank Kotler said, you reserve 1 byte but you write a 4 byte value at this address.

I can fix with a

Code: [Select]

dec esi, but I don't think is a good solution

But, maybe it is a solution.
You could also try to initialize esi with -1. I am sure, you can find another solution.

Think about your initialization of eax with 0. You compares this initialization value with 0ah, but eax doesn't contain a byte of your string.

At this point, where you think your algorithm works, you should test it under multiple circumstances, with some different strings. e.g.:

tst2 db "ABCZ", 0ah, 0       ; test the uppercase letter 'A' and 'Z', your jcc mnemonics could be wrong

and maybe some more string, e.g. with unexpected character or missing newline character, etc.

[master94ga]

Code: [Select]

cont resb 1 ; <- needs to be more!

As Frank Kotler said, you reserve 1 byte but you write a 4 byte value at this address.

I can fix with a

Code: [Select]

dec esi, but I don't think is a good solution

But, maybe it is a solution.
You could also try to initialize esi with -1. I am sure, you can find another solution.

Think about your initialization of eax with 0. You compares this initialization value with 0ah, but eax doesn't contain a byte of your string.

At this point, where you think your algorithm works, you should test it under multiple circumstances, with some different strings. e.g.:

tst2 db "ABCZ", 0ah, 0       ; test the uppercase letter 'A' and 'Z', your jcc mnemonics could be wrong

and maybe some more string, e.g. with unexpected character or missing newline character, etc.

Ok, so for "cont", 4 byte will be good, about the test and other I will try more solution, thanks for the reply