Re: [Qemu-devel] [PATCH] exec: Don't request an address for code_gen_buffer if -fpie

[Richard Henderson <rth@twiddle.net>]

[-- Attachment #1: Type: text/plain, Size: 945 bytes --]

On 10/07/2012 09:34 AM, Blue Swirl wrote:
>> > +#ifdef USE_MMAP
>> > +    code_gen_buffer = mmap((void *)start, code_gen_buffer_size,
>> > +                           PROT_WRITE | PROT_READ | PROT_EXEC,
>> > +                           flags, -1, 0);
>> > +    if (code_gen_buffer == MAP_FAILED) {
>> > +        fprintf(stderr, "Could not allocate dynamic translator buffer\n");
>> > +        exit(1);
>> >      }
>> >  #else
>> >      code_gen_buffer = g_malloc(code_gen_buffer_size);
>> >      map_exec(code_gen_buffer, code_gen_buffer_size);
> In this branch (e.g. mingw32), 'start' is unused:
> /src/qemu/exec.c: In function 'code_gen_alloc':
> /src/qemu/exec.c:531: warning: unused variable 'start'

Well, I've rearranged the code to handle this, and it does avoid the warning.
But I'm not sure I like the two separate blocks.  Especially for the x86_64
MAP32 case.  Perhaps we're better off with an __attribute__((unused)) there?


r~





[-- Attachment #2: z --]
[-- Type: text/plain, Size: 2993 bytes --]

diff --git a/exec.c b/exec.c
index 8f3bc74..704426c 100644
--- a/exec.c
+++ b/exec.c
@@ -527,15 +527,14 @@ static void code_gen_alloc(unsigned long tb_size)
 #else
 #ifdef USE_MMAP
     int flags = MAP_PRIVATE | MAP_ANONYMOUS;
+    uintptr_t start = 0;
 #endif
-    uintptr_t max_buf = -1, start = 0;
+    size_t max_buf = ~(size_t)0;
 
-    /* Constrain the size and position of the buffer based on the host cpu.  */
+    /* Constrain the size of the buffer based on the host cpu.  */
 #if defined(__x86_64__)
-# if !defined(__PIE__) && !defined(__PIC__) && defined(MAP_32BIT)
-    /* Force the memory down into low memory with the executable.
-       Leave the choice of exact location with the kernel.  */
-    flags |= MAP_32BIT;
+# if !defined(__PIE__) && !defined(__PIC__) \
+     && defined(USE_MMAP) && defined(MAP_32BIT)
     /* Cannot expect to map more than 800MB in low memory.  */
     max_buf = 800 * 1024 * 1024;
 # else
@@ -545,22 +544,12 @@ static void code_gen_alloc(unsigned long tb_size)
 #elif defined(__sparc__) && HOST_LONG_BITS == 64
     /* Maximum range of direct branches between TB (via "call").  */
     max_buf = 2ul * 1024 * 1024 * 1024;
-    start = 0x40000000ul;
 #elif defined(__arm__)
     /* Keep the buffer no bigger than 16MB to branch between blocks */
     max_buf = 16 * 1024 * 1024;
 #elif defined(__s390x__)
-    /* Map the buffer so that we can use direct calls and branches.  */
     /* We have a +- 4GB range on the branches; leave some slop.  */
     max_buf = 3ul * 1024 * 1024 * 1024;
-    start = 0x90000000ul;
-#endif
-#if defined(__PIE__) || defined(__PIC__)
-    /* Don't bother setting a preferred location if we're building
-       a position-independent executable.  We're more likely to get
-       an address near the main executable if we let the kernel
-       choose the address.  */
-    start = 0;
 #endif
 
     /* Size the buffer.  */
@@ -581,6 +570,23 @@ static void code_gen_alloc(unsigned long tb_size)
     }
 
 #ifdef USE_MMAP
+    /* Constrain the position of the buffer based on the host cpu.
+       Note that these addresses are chosen in concert with the
+       addresses assigned in the relevant linker script file.  */
+# if defined(__PIE__) || defined(__PIC__)
+    /* Don't bother setting a preferred location if we're building
+       a position-independent executable.  We're more likely to get
+       an address near the main executable if we let the kernel
+       choose the address.  */
+# elif defined(__x86_64__) && defined(MAP_32BIT)
+    /* Force the memory down into low memory with the executable.
+       Leave the choice of exact location with the kernel.  */
+    flags |= MAP_32BIT;
+# elif defined(__sparc__) && HOST_LONG_BITS == 64
+    start = 0x40000000ul;
+# elif defined(__s390x__)
+    start = 0x90000000ul;
+# endif
     code_gen_buffer = mmap((void *)start, code_gen_buffer_size,
                            PROT_WRITE | PROT_READ | PROT_EXEC,
                            flags, -1, 0);