tcg: pointer size warning on x32 arch

tcg: pointer size warning on x32 arch

[Michael Tokarev]

Hi.

The following warning is reported by the C compiler when compiling
tcg code on x32 architecture:

In file included from ../../tcg/tcg.c:429:
tcg/i386/tcg-target.c.inc: In function ‘tcg_out_movi_int’:
tcg/i386/tcg-target.c.inc:959:30: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   959 |     diff = tcg_pcrel_diff(s, (const void *)arg) - 7;

I dunno if we should be concerned or if it is worth to support x32
in the first place. But I thought I'd report it anyway.

Thanks!

/mjt

Re: tcg: pointer size warning on x32 arch

[Philippe Mathieu-Daudé]

On 9/11/21 7:50 PM, Michael Tokarev wrote:
> Hi.
> 
> The following warning is reported by the C compiler when compiling
> tcg code on x32 architecture:
> 
> In file included from ../../tcg/tcg.c:429:
> tcg/i386/tcg-target.c.inc: In function ‘tcg_out_movi_int’:
> tcg/i386/tcg-target.c.inc:959:30: warning: cast to pointer from integer
> of different size [-Wint-to-pointer-cast]
>   959 |     diff = tcg_pcrel_diff(s, (const void *)arg) - 7;

Likely fixed by:

---
diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
index 98d924b91a8..0895f5670a1 100644
--- a/tcg/i386/tcg-target.c.inc
+++ b/tcg/i386/tcg-target.c.inc
@@ -956,7 +956,7 @@ static void tcg_out_movi_int(TCGContext *s, TCGType
type,
     }

     /* Try a 7 byte pc-relative lea before the 10 byte movq.  */
-    diff = tcg_pcrel_diff(s, (const void *)arg) - 7;
+    diff = tcg_pcrel_diff(s, (const void *)(uintptr_t)arg) - 7;
     if (diff == (int32_t)diff) {
         tcg_out_opc(s, OPC_LEA | P_REXW, ret, 0, 0);
         tcg_out8(s, (LOWREGMASK(ret) << 3) | 5);
---

Re: tcg: pointer size warning on x32 arch

[Philippe Mathieu-Daudé]

On 9/11/21 11:06 PM, Philippe Mathieu-Daudé wrote:
> On 9/11/21 7:50 PM, Michael Tokarev wrote:
>> Hi.
>>
>> The following warning is reported by the C compiler when compiling
>> tcg code on x32 architecture:
>>
>> In file included from ../../tcg/tcg.c:429:
>> tcg/i386/tcg-target.c.inc: In function ‘tcg_out_movi_int’:
>> tcg/i386/tcg-target.c.inc:959:30: warning: cast to pointer from integer
>> of different size [-Wint-to-pointer-cast]
>>   959 |     diff = tcg_pcrel_diff(s, (const void *)arg) - 7;
> 
> Likely fixed by:
> 
> ---
> diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
> index 98d924b91a8..0895f5670a1 100644
> --- a/tcg/i386/tcg-target.c.inc
> +++ b/tcg/i386/tcg-target.c.inc
> @@ -956,7 +956,7 @@ static void tcg_out_movi_int(TCGContext *s, TCGType
> type,
>      }
> 
>      /* Try a 7 byte pc-relative lea before the 10 byte movq.  */
> -    diff = tcg_pcrel_diff(s, (const void *)arg) - 7;
> +    diff = tcg_pcrel_diff(s, (const void *)(uintptr_t)arg) - 7;

Hmm not quite. At this point tcg_out_movi_int() already checked 'arg'
does not fit into a 32-bit value... And on x32 we have sizeof(void*) = 4
so we can't cast a >32-bit value that way.

But tcg_out_movi_int() is called by tcg_out_movi(), and all 'arg' values
are either 0, 1 or a host address (often casted as uintptr_t). So on x32
this value is already truncated (safe enough?). Thus this code seems
unlikely reached there (with a >32-bit value).

So maybe this is sufficient to silent the warning?

---
diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
index 98d924b91a8..2e6304f26b1 100644
--- a/tcg/i386/tcg-target.c.inc
+++ b/tcg/i386/tcg-target.c.inc
@@ -955,6 +955,8 @@ static void tcg_out_movi_int(TCGContext *s, TCGType
type,
         return;
     }

+    assert(sizeof(uintptr_t) > sizeof(uint32_t));
+
     /* Try a 7 byte pc-relative lea before the 10 byte movq.  */
     diff = tcg_pcrel_diff(s, (const void *)arg) - 7;
     if (diff == (int32_t)diff) {
---

Re: tcg: pointer size warning on x32 arch

[Richard Henderson]

On 9/11/21 2:46 PM, Philippe Mathieu-Daudé wrote:
> On 9/11/21 11:06 PM, Philippe Mathieu-Daudé wrote:
>> On 9/11/21 7:50 PM, Michael Tokarev wrote:
>>> Hi.
>>>
>>> The following warning is reported by the C compiler when compiling
>>> tcg code on x32 architecture:
>>>
>>> In file included from ../../tcg/tcg.c:429:
>>> tcg/i386/tcg-target.c.inc: In function ‘tcg_out_movi_int’:
>>> tcg/i386/tcg-target.c.inc:959:30: warning: cast to pointer from integer
>>> of different size [-Wint-to-pointer-cast]
>>>    959 |     diff = tcg_pcrel_diff(s, (const void *)arg) - 7;
>>
>> Likely fixed by:
>>
>> ---
>> diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
>> index 98d924b91a8..0895f5670a1 100644
>> --- a/tcg/i386/tcg-target.c.inc
>> +++ b/tcg/i386/tcg-target.c.inc
>> @@ -956,7 +956,7 @@ static void tcg_out_movi_int(TCGContext *s, TCGType
>> type,
>>       }
>>
>>       /* Try a 7 byte pc-relative lea before the 10 byte movq.  */
>> -    diff = tcg_pcrel_diff(s, (const void *)arg) - 7;
>> +    diff = tcg_pcrel_diff(s, (const void *)(uintptr_t)arg) - 7;
> 
> Hmm not quite. At this point tcg_out_movi_int() already checked 'arg'
> does not fit into a 32-bit value... And on x32 we have sizeof(void*) = 4
> so we can't cast a >32-bit value that way.
> 
> But tcg_out_movi_int() is called by tcg_out_movi(), and all 'arg' values
> are either 0, 1 or a host address (often casted as uintptr_t).

That's false -- 'arg' is an arbitrary 64-bit constant here, for x32. But you're right that 
no x32 pointers will arrive here, because TCG_TYPE_PTR == TCG_TYPE_I32 for that case and 
we'll use the 5-byte mov immediate insn.

> +    assert(sizeof(uintptr_t) > sizeof(uint32_t));
> +
>       /* Try a 7 byte pc-relative lea before the 10 byte movq.  */
>       diff = tcg_pcrel_diff(s, (const void *)arg) - 7;
>       if (diff == (int32_t)diff) {

We may need something like

     if (sizeof(void *) == 8) {
         diff = tcg_pcrel_diff(s, (const void *)(uintptr_t)arg) - 7;
         ...
     }

I wonder if I still have an x32 vmm hanging about.


r~

Re: tcg: pointer size warning on x32 arch

[Richard Henderson]

On 9/12/21 6:10 AM, Richard Henderson wrote:
> We may need something like
> 
>      if (sizeof(void *) == 8) {
>          diff = tcg_pcrel_diff(s, (const void *)(uintptr_t)arg) - 7;
>          ...
>      }
> 
> I wonder if I still have an x32 vmm hanging about.

I do not have an x32 vmm anymore, and I don't see a distro ships one either.  So: Michael, 
are you able to test and submit a patch?

Otherwise, because of the lack of distro thing, I'm tempted to drop x32 support entirely.


r~