Re: [tianocore/edk2] Stdlib: Add calling convention to completion callbacks (#123)

[Marco Guerri <marco.guerri.dev@fastmail.com>]

Hi Daryl,

First, sorry for not having RTFM and submitting the patch via github. I
was planning to read your workflow during the weekend and re-submit via
the mailing list.
Second, I am using gcc. I tried gcc 4.8 and 5.2 and with both, my
applications using Stdlib badly hang. I dug a bit and discovered that
the signatures of the completion callbacks are missing EFIAPI, so
necessarily under Unix systems they are using the wrong ABI. When the
functions arguments are pointers, the machine ends up in a triple fault
I guess (I can clearly see that the first two integer parameters are
fetched from %rdi and %rsi, which is the SysV ABI). I was a bit
surprised to see this bug, as the buildscripts are pretty comprehensive
in terms of compilers support. Does this make sense to you?
Cheers,
Marco

--
  Marco Guerri
  marco.guerri.dev@fastmail.com



On Thu, Jan 18, 2018, at 11:12 AM, Daryl McDaniel (EDK2 Lists) wrote:
> Marco,


>  


> Thank you for your submission.


>  


> Can you please tell me which compiler you are using?


>  


> Thank you,


> Daryl McDaniel


>  


>  


> **From:** Marco Guerri [mailto:notifications@github.com] **Sent:**
> Wednesday, January 17, 2018 7:22 AM **To:** tianocore/edk2
> <edk2@noreply.github.com> **Cc:** Subscribed
> <subscribed@noreply.github.com> **Subject:** [tianocore/edk2] Stdlib:
> Add calling convention to completion callbacks (#123)>  


> When compiling in Unix environments, the ABI defaults to SysV.
> Callbacks are invoked directly by the system firmware so they must
> necessarily use MS ABI.> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by:
> Marco Guerri marco.guerri.dev@fastmail.com> 
> *You can view, comment on, or merge this pull request online at:*


>   https://github.com/tianocore/edk2/pull/123


> *Commit Summary*


>  * Stdlib: Add calling convention to completion callbacks> *File Changes*


>  * **M** StdLib/EfiSocketLib/Ip4.c[1] (10)
>  * **M** StdLib/EfiSocketLib/Socket.h[2] (24)
>  * **M** StdLib/EfiSocketLib/Tcp4.c[3] (11)
>  * **M** StdLib/EfiSocketLib/Tcp6.c[4] (3)
>  * **M** StdLib/EfiSocketLib/Udp4.c[5] (30)
>  * **M** StdLib/EfiSocketLib/Udp6.c[6] (20)> *Patch Links:*


>  * https://github.com/tianocore/edk2/pull/123.patch
>  * https://github.com/tianocore/edk2/pull/123.diff> — You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub[7], or mute the
> thread[8].

Links:

  1. https://github.com/tianocore/edk2/pull/123/files#diff-0
  2. https://github.com/tianocore/edk2/pull/123/files#diff-1
  3. https://github.com/tianocore/edk2/pull/123/files#diff-2
  4. https://github.com/tianocore/edk2/pull/123/files#diff-3
  5. https://github.com/tianocore/edk2/pull/123/files#diff-4
  6. https://github.com/tianocore/edk2/pull/123/files#diff-5
  7. https://github.com/tianocore/edk2/pull/123
  8. https://github.com/notifications/unsubscribe-auth/AM0XkaIxBOrC6Kacm-Vqq6z4tgQFar04ks5tLhAHgaJpZM4RhgXz