diff mbox series

[v1,2/5] linux-user: convert target_mmap debug to tracepoint

Message ID 20191128194603.24818-3-alex.bennee@linaro.org
State New
Headers show
Series linux-user mmap debug cleanup | expand

Commit Message

Alex Bennée Nov. 28, 2019, 7:46 p.m. UTC 
It is a pain to re-compile when you need to debug and tracepoints are
a fairly low impact way to instrument QEMU.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 linux-user/mmap.c       | 51 +++++++++++++++++++++++------------------
 linux-user/trace-events |  1 +
 2 files changed, 30 insertions(+), 22 deletions(-)

-- 
2.20.1

Comments

Richard Henderson Dec. 2, 2019, 1:11 a.m. UTC | #1 
On 11/28/19 7:46 PM, Alex Bennée wrote:
> +    if (TRACE_TARGET_MMAP_ENABLED) {

> +        char prot_str[4];

> +        g_autoptr(GString) flag_str = g_string_new(NULL);

> +

> +        pp_prot(&prot_str, prot);

> +

> +        if (flags & MAP_FIXED) {

> +            g_string_append(flag_str, "MAP_FIXED ");

> +        }

> +        if (flags & MAP_ANONYMOUS) {

> +            g_string_append(flag_str, "MAP_ANON ");

> +        }

> +

> +        switch (flags & MAP_TYPE) {

>          case MAP_PRIVATE:

> -            printf("MAP_PRIVATE ");

> +            g_string_append(flag_str, "MAP_PRIVATE ");

>              break;

>          case MAP_SHARED:

> -            printf("MAP_SHARED ");

> +            g_string_append(flag_str, "MAP_SHARED ");

>              break;

>          default:

> -            printf("[MAP_TYPE=0x%x] ", flags & MAP_TYPE);

> +            g_string_append_printf(flag_str, "[MAP_TYPE=0x%x] ",

> +                                   flags & MAP_TYPE);

>              break;

>          }

> -        printf("fd=%d offset=" TARGET_ABI_FMT_lx "\n", fd, offset);

> +        trace_target_mmap(start, len, prot_str, flag_str->str, fd, offset);

>      }


I don't think that you need to re-create -strace output.
There are also quite a lot of MAP_* flags that are not
being printed, without any indication that they are left out.

Again, I think we should just print the hex value.


r~
diff mbox series

Patch

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 66868762519..c81fd85fbd2 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -60,6 +60,15 @@  void mmap_fork_end(int child)
         pthread_mutex_unlock(&mmap_mutex);
 }
 
+/* mmap prot pretty printer */
+static void pp_prot(char (*str)[4], int prot)
+{
+    (*str)[0] = prot & PROT_READ ? 'r' : '-';
+    (*str)[1] = prot & PROT_WRITE ? 'w' : '-';
+    (*str)[2] = prot & PROT_EXEC ? 'x' : '-';
+    (*str)[3] = 0;
+}
+
 /* NOTE: all the constants are the HOST ones, but addresses are target. */
 int target_mprotect(abi_ulong start, abi_ulong len, int prot)
 {
@@ -68,10 +77,7 @@  int target_mprotect(abi_ulong start, abi_ulong len, int prot)
 
     if (TRACE_TARGET_MPROTECT_ENABLED) {
         char prot_str[4];
-        prot_str[0] = prot & PROT_READ ? 'r' : '-';
-        prot_str[1] = prot & PROT_WRITE ? 'w' : '-';
-        prot_str[2] = prot & PROT_EXEC ? 'x' : '-';
-        prot_str[3] = 0;
+        pp_prot(&prot_str, prot);
         trace_target_mprotect(start, len, prot_str);
     }
 
@@ -370,32 +376,33 @@  abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
     abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
 
     mmap_lock();
-#ifdef DEBUG_MMAP
-    {
-        printf("mmap: start=0x" TARGET_ABI_FMT_lx
-               " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
-               start, len,
-               prot & PROT_READ ? 'r' : '-',
-               prot & PROT_WRITE ? 'w' : '-',
-               prot & PROT_EXEC ? 'x' : '-');
-        if (flags & MAP_FIXED)
-            printf("MAP_FIXED ");
-        if (flags & MAP_ANONYMOUS)
-            printf("MAP_ANON ");
-        switch(flags & MAP_TYPE) {
+    if (TRACE_TARGET_MMAP_ENABLED) {
+        char prot_str[4];
+        g_autoptr(GString) flag_str = g_string_new(NULL);
+
+        pp_prot(&prot_str, prot);
+
+        if (flags & MAP_FIXED) {
+            g_string_append(flag_str, "MAP_FIXED ");
+        }
+        if (flags & MAP_ANONYMOUS) {
+            g_string_append(flag_str, "MAP_ANON ");
+        }
+
+        switch (flags & MAP_TYPE) {
         case MAP_PRIVATE:
-            printf("MAP_PRIVATE ");
+            g_string_append(flag_str, "MAP_PRIVATE ");
             break;
         case MAP_SHARED:
-            printf("MAP_SHARED ");
+            g_string_append(flag_str, "MAP_SHARED ");
             break;
         default:
-            printf("[MAP_TYPE=0x%x] ", flags & MAP_TYPE);
+            g_string_append_printf(flag_str, "[MAP_TYPE=0x%x] ",
+                                   flags & MAP_TYPE);
             break;
         }
-        printf("fd=%d offset=" TARGET_ABI_FMT_lx "\n", fd, offset);
+        trace_target_mmap(start, len, prot_str, flag_str->str, fd, offset);
     }
-#endif
 
     if (!len) {
         errno = EINVAL;
diff --git a/linux-user/trace-events b/linux-user/trace-events
index 41d72e61abb..9411ab357c9 100644
--- a/linux-user/trace-events
+++ b/linux-user/trace-events
@@ -14,3 +14,4 @@  user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_add
 
 # mmap.c
 target_mprotect(uint64_t start, uint64_t len, char *flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=%s"
+target_mmap(uint64_t start, uint64_t len, char *pflags, char *mflags, int fd, uint64_t offset) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=%s flags=%s fd=%d offset=0x%"PRIx64