What is wrong with this function (written in assembly) to print hexadecimal numbers?

Question

I am only a beginner in assembly and was reading the OS book by Nick Blundell, when I came across the question of writing a function which could print hexadecimal number. But, despite verifying the logic multiple times , I can't seem to find why this code doesn't work. Please help, I would be grateful.


HEX_OUT:    db  '0x0000', 0
MASK:       dw  0b1111000000000000
COUNTER:    db  3

print_string :

    pusha                               ;SAVES ALL REGISTER VALUES TO BE RESTORED WHEN RETURNING.
    mov ah, 0x0e

    jmp print_loop                      ;NOT COMPULSORY

    print_loop :
        mov al, [bx]
        add bx, 1                       ;ADD 1, NOT 8, NOT 16.
        int 0x10
        cmp al, 0                       ;SETS A FLAG ACCORDING TO RESULT OF COMPARISON.
        jne print_loop                  ;CAUSES LOOP.
        jmp final_block                 ;CAN BE REPLACED BY THE STATEMENTS IN final_block, NO NEED FOR MAKING NEW LABEL.

    final_block :
        popa
        ret                             ;RETURNS TO THE POINT WHERE CALL HAPPENED.

print_hex :
    pusha

    mov bx, HEX_OUT
    add bx, 2

    alter_loop :                        ;LOOP TO ALTER HEX_OUT
        mov ax, [MASK]
        cmp ax, 0                       ;CONDITION TO END LOOP
        je after_loop

        mov ax, dx                      ;GETTING(INTO AX) THE DATA FOR N-TH POSITION 
        and ax, [MASK]

        mov cx, [COUNTER]

        shift_loop :
            cmp cx, 0
            je end_shift_loop
            shr ax, 4
            sub cx, 1
        end_shift_loop:

        cmp ax, 0x0009                       ;DO HEX->ALPHABET IF NUMBER IS GREATER THAN 9
        jle skip_hex_to_alphabet

        add ax, 39                       ;EQUIVALENT TO (sub ax, 48--- sub ax, 9 ---add ax, 96)

        skip_hex_to_alphabet :

        add ax, 48                      ;ADDING 48(ASCII OF 0), IS ALREADY SUBTRACTED IF N-TH NUMBER>9

        mov [bx], al                    ;STORING DATA IN LOCATION POINTED TO BY BX
        add bx, 1                       ;INCREMENT FOR LOOP
        mov ax, [MASK]                  ;CHANGING MASK
        shr ax, 4
        mov [MASK], ax

        mov ax, [COUNTER]               ;UPDATING COUNTER
        sub ax, 1
        mov [COUNTER], ax

        jmp alter_loop

    after_loop :
    mov bx, HEX_OUT
    call print_string    
    popa
    ret

Upon calling the function like :-

mov dx, 0x1fd6
call print_hex

It prints, 0xWGd0 instead of 0x1fd6 .

you are executing `add ax, 48` even if `add ax, 39` is executed. — Margaret Bloom, Mar 31 '20 at 09:40
Yes, @MargaretBloom . So that, 48 is added when a number is to be printed and 87 is added when alphabet is to be printed. So, to print ```0xa``` the program first identifies 10 using masks, then adds 87, producing 97, which is ascii for 'a' — Jeevesh Juneja, Mar 31 '20 at 09:46
Hint: simplify your code by masking *after* shifting or rotating a nibble to the bottom of a register, so you can use a simple `and reg, 0xf`. IDK why you're using static storage at all for anything instead of just registers. You already use `pusha` to save/restore all of them. See [How to convert a binary integer number to a hex string?](https://stackoverflow.com/q/53823756) which has a nice simple implementation, and links to a couple other 16-bit implementations. — Peter Cordes, Mar 31 '20 at 09:48
@PeterCordes, I am quite new to assembly. Can you please point me to some resource illustrating the syntax and types of different variables and their declarations ? It would be very helpful, I guess. Thanks :) — Jeevesh Juneja, Mar 31 '20 at 09:53
https://nasm.us/doc/nasmdoc3.html#section-3.2.1. See also https://stackoverflow.com/tags/x86/info for more links to docs and guides. — Peter Cordes, Mar 31 '20 at 09:54
@MargaretBloom: yeah, I think that part of the code looks right. It always does `c += '0'`, and conditionally does `c += 'a'-10 - '0'`, i.e. 39. That works, and is what I did in an x86-64 version of int->hex string on https://codegolf.stackexchange.com/posts/193842/revisions. It's easier to grok when written symbolically. — Peter Cordes, Mar 31 '20 at 09:56
@PeterCordes Please let know if you find the mistake. I have been looking for a long time now. — Jeevesh Juneja, Mar 31 '20 at 10:01
@JeeveshJuneja: use a debugger to single-step through your code and look at register values. Probably you're not masking correctly and the number you're adding to is larger than 15. Like I said, it can be as simple as rotating the high nibble to the bottom with `rol reg, 4` and isolating it with MOV + AND. — Peter Cordes, Mar 31 '20 at 10:03
My mistake, sorry. Your code is so convoluted that is hard to follow. BTW your code doesn't work because you are shifting right the masked value by 4 all the times. As Peter pointed out, it's easier to use a variable shift decremented by 4 at each iteration (and getting rid of the globals). Or you are missing a jump back to `shift_loop`. — Margaret Bloom, Mar 31 '20 at 10:16

Margaret Bloom · Accepted Answer · 2020-03-31T11:17:12.420

You are missing a jump back to shift_loop and have wrongly declared the size of COUNTER.

Since you use mov cx, [COUNTER], COUNTER must be a word, fix it:

COUNTER:    dw  3

Finally, you are not correctly shifting the masked values. At the first iteration the and ax, [MASK] produces 0x1000 and in the shift_loop this is reduced to 0x0100 because it only iterates once.
Close the loop with a jump:

    shift_loop :
        cmp cx, 0
        je end_shift_loop
        shr ax, 4
        sub cx, 1
    jmp shift_loop

    end_shift_loop:

My two cents: I've been writing and reading assembly for more than two decades and your code managed to confuse me. I did not expect a hex printing routine to loop over a static mask and store the result in a static string. It's way too convoluted for the task given.
You can simply extract the nibbles with a variable shift counter decreased by four and a (constant) mask. You can then even use a 16 bytes lookup table to convert the nibble into a char, avoiding a branch.

Also, since you are programming for DOS, it is very worthwhile finding a copy of TASM online and use its debugger (td - Turbo Debugger). It's easy to use the wrong size for a variable and work with garbage, a debugger will immediately show you this.

If you like a concrete example, here's a simple implementation.

;AX = number to print in hex
hex:
  mov cx, 12              ;Shift counter, we start isolating the higher nibble (which starts at 12)
  mov bx, hexDigits       ;Lookup tables for the digitals
  mov dx, ax              ;We need a copy of the number and AX is used by the int10 service

.extract:
  mov si, dx              ;Make a copy of the original number so we don't lose it. Also we need it in SI for addressing purpose
  shr si, cl              ;Isolate a nibble by bringing it at the lower position
  and si, 0fh             ;Isolate the nibble by masking off any higher nibble

  mov al, [bx + si]       ;Transform the nibble into a digit (that's why we needed it in SI)
  mov ah, 0eh             ;You can also lift this out of the loop. It put it here for readability.
  int 10h                 ;Print it

  sub cx, 4               ;Next nibble is 4 bits apart
jnc .extract              ;Keep looping until we go from 0000h to 0fffch. This will set the CF

  ret

hexDigits db "0123456789abcdef"

@JeeveshJuneja You are welcome. I've added a concrete example in case you were wondering what the heck were we talking about. — Margaret Bloom, Mar 31 '20 at 11:17

What is wrong with this function (written in assembly) to print hexadecimal numbers?

1 Answers1