Skip to main content
Code Review

Return to Answer

added 349 characters in body
Source Link
ChrisW
ChrisW
  • 12.9k
  • 1
  • 34
  • 76

I'm not sure whether it's correct:

  • If show_hash uses cdecl calling conventions you should preserve registers likeIf show_hash uses cdecl calling conventions you should preserve registers like %bl, and read the input parameter values from the stack into esi and edi.
    (see section "3.2.3 Parameter Passing" of %bl, and read the input parameter values fromthe Application Binary Interface: parameters are conveniently passed in the stack into esirdi and edi.rsi registers)
  • I don't understand the syntax ofI don't understand the syntax of mov 1b(%rax),%al: is it reading from the lookup array defined at label 1:? On re-reading, I think it is; however that will only work if the most-significant bytes of rax and rdx are all zero; perhaps you should initialize them as you did using xor %eax,%eax
    (apparently movxor 1b(%rax)%eax,%al: is it reading from the lookup array defined at label 1:%eax? On re-reading, I think it is; however that will only work ifclear the most-significant byteswhole of rax and rdx are all zero; perhaps you should initialize them as you did using xor %eax,%eax%rax)
  • You're using stosw which writes two bytes (two ASCII characters) a time; but %ah contains a value from %bl, and %bl wasn't previously initialized? I think that statement should have been mov %dl,%ah not mov %bl,%ah; or perhaps you could have done mov 1b(%rdx),%ah directly.
  • The function ends with mov %cl,(%rdi) to null-terminate the string; that is clever but could use a comment (it took me a bit to figure out). Most of the questions either have good comments, or answer which say that they should have good comments.

I'm not sure whether it's correct:

  • If show_hash uses cdecl calling conventions you should preserve registers like %bl, and read the input parameter values from the stack into esi and edi.
  • I don't understand the syntax of mov 1b(%rax),%al: is it reading from the lookup array defined at label 1:? On re-reading, I think it is; however that will only work if the most-significant bytes of rax and rdx are all zero; perhaps you should initialize them as you did using xor %eax,%eax
  • You're using stosw which writes two bytes (two ASCII characters) a time; but %ah contains a value from %bl, and %bl wasn't previously initialized? I think that statement should have been mov %dl,%ah not mov %bl,%ah
  • The function ends with mov %cl,(%rdi) to null-terminate the string; that is clever but could use a comment (it took me a bit to figure out). Most of the questions either have good comments, or answer which say that they should have good comments.

I'm not sure whether it's correct:

  • If show_hash uses cdecl calling conventions you should preserve registers like %bl, and read the input parameter values from the stack into esi and edi.
    (see section "3.2.3 Parameter Passing" of the Application Binary Interface: parameters are conveniently passed in the rdi and rsi registers)
  • I don't understand the syntax of mov 1b(%rax),%al: is it reading from the lookup array defined at label 1:? On re-reading, I think it is; however that will only work if the most-significant bytes of rax and rdx are all zero; perhaps you should initialize them as you did using xor %eax,%eax
    (apparently xor %eax,%eax will clear the whole of %rax)
  • You're using stosw which writes two bytes (two ASCII characters) a time; but %ah contains a value from %bl, and %bl wasn't previously initialized? I think that statement should have been mov %dl,%ah not mov %bl,%ah; or perhaps you could have done mov 1b(%rdx),%ah directly.
  • The function ends with mov %cl,(%rdi) to null-terminate the string; that is clever but could use a comment (it took me a bit to figure out). Most of the questions either have good comments, or answer which say that they should have good comments.
Source Link
ChrisW
ChrisW
  • 12.9k
  • 1
  • 34
  • 76

I'm not sure whether it's correct:

  • If show_hash uses cdecl calling conventions you should preserve registers like %bl, and read the input parameter values from the stack into esi and edi.
  • I don't understand the syntax of mov 1b(%rax),%al: is it reading from the lookup array defined at label 1:? On re-reading, I think it is; however that will only work if the most-significant bytes of rax and rdx are all zero; perhaps you should initialize them as you did using xor %eax,%eax
  • You're using stosw which writes two bytes (two ASCII characters) a time; but %ah contains a value from %bl, and %bl wasn't previously initialized? I think that statement should have been mov %dl,%ah not mov %bl,%ah
  • The function ends with mov %cl,(%rdi) to null-terminate the string; that is clever but could use a comment (it took me a bit to figure out). Most of the questions either have good comments, or answer which say that they should have good comments.