Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 6 Jan 2023 at 10:21, Evgeny Iakovlev
> <eiakovlev@linux.microsoft.com> wrote:
>>
>> Windows open(2) implementation opens files in text mode by default and
>> needs a Windows-only O_BINARY flag to open files as binary. QEMU already
>> knows about that flag in osdep and it is defined to 0 on non-Windows,
>> so we can just add it to the host_flags for better compatibility.
>>
>> Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>  semihosting/syscalls.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
>> index 508a0ad88c..b621d78c2d 100644
>> --- a/semihosting/syscalls.c
>> +++ b/semihosting/syscalls.c
>> @@ -253,7 +253,7 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete,
>>  {
>>      CPUArchState *env G_GNUC_UNUSED = cs->env_ptr;
>>      char *p;
>> -    int ret, host_flags;
>> +    int ret, host_flags = O_BINARY;
>
> The semihosting API, at least for Arm, has a modeflags string so the
> guest can say whether it wants to open O_BINARY or not:
> https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst#sys-open-0x01
>
> So we need to plumb that down through the common semihosting code
> into this function and set O_BINARY accordingly. Otherwise guest
> code that asks for a text-mode file won't get one.

We used to, in fact we still have a remnant of the code where we do:

  #ifndef O_BINARY
  #define O_BINARY 0
  #endif

I presume because the only places it exists in libc is wrapped in stuff
like:

  #if defined (__CYGWIN__)
  #define O_BINARY	_FBINARY

So the mapping got removed in a1a2a3e609 (semihosting: Remove
GDB_O_BINARY) because GDB knows nothing of this and as far as I can tell
neither does Linux whatever ISO C might say about it.

Is this a host detail leakage to the guest? Should a semihosting app be
caring about what fopen() modes the underlying host supports? At least a
default O_BINARY for windows is most likely to DTRT.

> I don't know about other semihosting APIs, so those would need
> to be checked to see what they should do.
>
> thanks
> -- PMM


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro