diff mbox series

[22/40] sim/m32r: Fixes to Linux emulator

Message ID a3693e7a058878c5a64f39ce0e58c7a2605b3a07.1666258361.git.research_trasio@irq.a4lg.com
State Committed
Headers
Series sim+gdb: Suppress warnings if built with Clang (big batch 1) |

Commit Message

Tsukasa OI Oct. 20, 2022, 9:32 a.m. UTC 
  This commit fixes various M32R Linux emulator issues.

1.  Some header files were missing
    a.  <sys/ioctl.h> for ioctl
    b.  <sys/fsuid.h> for setfsuid/setfsgid (Linux 1.2 or later)
    c.  <sys/file.h> for flock (a syscall on Linux 2.0 or later)
    d.  <sys/sendfile.h> for sendfile (Linux 2.2 or later)
2.  syslog function must be called as a syscall rather than POSIX syslog
    because we are emulating Linux system calls on the Linux host.
3.  ftime function is deprecated but used intentionally.
    We have to disable deprecated function warning.
---
 sim/m32r/traps.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Mike Frysinger Oct. 23, 2022, 2:56 p.m. UTC | #1 
On 20 Oct 2022 09:32, Tsukasa OI wrote:
> 1.  Some header files were missing

this looks fine

> 2.  syslog function must be called as a syscall rather than POSIX syslog
>     because we are emulating Linux system calls on the Linux host.

i don't think that is accurate.  if that were true, every other call in here
would be using syscall() instead of calling the C library functions.

> 3.  ftime function is deprecated but used intentionally.
>     We have to disable deprecated function warning.

this looks fine
-mike
Tsukasa OI Oct. 27, 2022, 2:40 a.m. UTC | #2 
On 2022/10/23 23:56, Mike Frysinger wrote:
> On 20 Oct 2022 09:32, Tsukasa OI wrote:
>> 1.  Some header files were missing
> 
> this looks fine
> 
>> 2.  syslog function must be called as a syscall rather than POSIX syslog
>>     because we are emulating Linux system calls on the Linux host.
> 
> i don't think that is accurate.  if that were true, every other call in here
> would be using syscall() instead of calling the C library functions.

Note that this part is a Linux to Linux syscall translator (though I
consider this whole idea unsafe).  Yes, it uses many C library functions
but I consider this is because they are (almost/completely) compatible
with underlying syscalls.

However, this isn't the case for syslog.  POSIX syslog (C library
function on <syslog.h>) and Linux syslog (syscall) have very different
semantics and Linux syslog should be used here.

Probably using glibc wrapper klogctl (<sys/klog.h>) might be a solution
(see man page syslog.2).  I would like to hear your thoughts.

Best regards,
Tsukasa

> 
>> 3.  ftime function is deprecated but used intentionally.
>>     We have to disable deprecated function warning.
> 
> this looks fine
> -mike
Mike Frysinger Oct. 27, 2022, 4:05 p.m. UTC | #3 
On 27 Oct 2022 11:40, Tsukasa OI wrote:
> On 2022/10/23 23:56, Mike Frysinger wrote:
> > On 20 Oct 2022 09:32, Tsukasa OI wrote:
> >> 2.  syslog function must be called as a syscall rather than POSIX syslog
> >>     because we are emulating Linux system calls on the Linux host.
> > 
> > i don't think that is accurate.  if that were true, every other call in here
> > would be using syscall() instead of calling the C library functions.
> 
> Note that this part is a Linux to Linux syscall translator (though I
> consider this whole idea unsafe).  Yes, it uses many C library functions
> but I consider this is because they are (almost/completely) compatible
> with underlying syscalls.

this is incorrect.  let's take a step back.  the point of the sim runtime
environments is to emulate the specified userland regardless of the host
OS.  it is not a linux-to-linux translator.  the runtime decodes syscalls,
then attempts to emulate the requested behavior.  ideally that would only
ever involve going through the standard C library APIs.

the m32r linux runtime environment layer has a bunch of linuxisms in there
(because it can be hard to emulate linux behavior on non-linux systems).
but that doesn't mean it's a linux-to-linux layer, and we shouldn't be
making it worse.

> However, this isn't the case for syslog.  POSIX syslog (C library
> function on <syslog.h>) and Linux syslog (syscall) have very different
> semantics and Linux syslog should be used here.

you are correct here.

the arch backends for diff runtime environments are inconsistent whether
they pass through to the real host env, or they provide stub/fake data so
the program doesn't get real access to the host (basically constructing a
sandbox).  i see the value in both modes (especially since a sandbox is
easier to run cross-OS), but haven't fleshed out how to support these.

> Probably using glibc wrapper klogctl (<sys/klog.h>) might be a solution
> (see man page syslog.2).  I would like to hear your thoughts.

klogctl would be OK
-mike
diff mbox series

Patch

diff --git a/sim/m32r/traps.c b/sim/m32r/traps.c
index f0fb218a11d..de275b06a40 100644
--- a/sim/m32r/traps.c
+++ b/sim/m32r/traps.c
@@ -20,6 +20,7 @@ 
 /* This must come before any other includes.  */
 #include "defs.h"
 
+#include "diagnostics.h"
 #include "portability.h"
 #include "sim-main.h"
 #include "sim-signal.h"
@@ -38,9 +39,14 @@ 
    NB: The emulation is also missing argument conversion (endian & bitsize)
    even on Linux hosts.  */
 #ifdef __linux__
+#include <sys/file.h>
+#include <sys/fsuid.h>
+#include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <sys/poll.h>
 #include <sys/resource.h>
+#include <sys/sendfile.h>
+#include <sys/syscall.h>
 #include <sys/sysinfo.h>
 #include <sys/stat.h>
 #include <sys/time.h>
@@ -397,7 +403,10 @@  m32r_trap (SIM_CPU *current_cpu, PCADDR pc, int num)
 	    {
 	      struct timeb t;
 
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
 	      result = ftime (&t);
+DIAGNOSTIC_POP
 	      errcode = errno;
 
 	      if (result != 0)
@@ -851,7 +860,7 @@  m32r_trap (SIM_CPU *current_cpu, PCADDR pc, int num)
 	    break;
 
 	  case TARGET_LINUX_SYS_syslog:
-	    result = syslog (arg1, (char *) t2h_addr (cb, &s, arg2));
+	    result = syscall (SYS_syslog, arg1, (char *) t2h_addr (cb, &s, arg2), arg3);
 	    errcode = errno;
 	    break;