[Mono-devel-list] (AMD64) Compiler Warnings (2)
Paolo Molaro
lupus at ximian.com
Mon Dec 6 10:41:22 EST 2004
On 12/03/04 Willibald Krenn wrote:
> Index: mono/metadata/class.c
> ===================================================================
> --- mono/metadata/class.c (revision 37033)
> +++ mono/metadata/class.c (working copy)
> @@ -852,9 +852,9 @@
> if (g_hash_table_lookup_extended (iid_hash, str, NULL, &value)) {
> mono_loader_unlock ();
> g_free (str);
> - return (guint)value;
> + return (guint) GPOINTER_TO_UINT(value);
The extra cast to guint is not needed.
> Index: mono/metadata/debug-mono-symfile.h
> ===================================================================
> --- mono/metadata/debug-mono-symfile.h (revision 37033)
> +++ mono/metadata/debug-mono-symfile.h (working copy)
> @@ -7,6 +7,7 @@
> #define __MONO_DEBUG_MONO_SYMFILE_H__
>
> #include <glib.h>
> +#include <intps.h>
> #include <mono/metadata/class.h>
> #include <mono/metadata/reflection.h>
> #include <mono/metadata/mono-debug.h>
> @@ -112,7 +113,7 @@
>
> struct MonoDebugLineNumberEntry {
> guint32 offset;
> - guint32 address;
> + uintptr_t address;
Unfortunately the names here are confusing: this is not a pointer-sized
address, it is actually the offset in the native code from the start of
the method (at least after a quick look at the code). So that change is not
needed.
A separate patch here could be done that changes the names of both fields to
il_offset and native_offset.
In other parts of the debug code it's used as a real address, though: this
really requires one of the debug guys to fix the mess. Just changing
the types to uintptr_t is not going to help.
> Index: mono/metadata/mono-debug.h
> ===================================================================
> --- mono/metadata/mono-debug.h (revision 37033)
> +++ mono/metadata/mono-debug.h (working copy)
> @@ -5,7 +5,7 @@
>
> #ifndef __MONO_DEBUG_H__
> #define __MONO_DEBUG_H__
> -
> +#include <intps.h>
mono-debug.h is an installed header and you can't add headers that require config.h
to it.
> --- mono/metadata/marshal.c (revision 37033)
> +++ mono/metadata/marshal.c (working copy)
> @@ -6879,7 +6882,7 @@
> {
> MONO_ARCH_SAVE_REGS;
>
> - return ((guint32)TlsGetValue (last_error_tls_id));
> + return ((guint32)GPOINTER_TO_UINT(TlsGetValue (last_error_tls_id)));
No need for the extra cast.
> gpointer
> ves_icall_System_Security_Cryptography_RNGCryptoServiceProvider_RngGetBytes (gpointer handle, MonoArray *arry)
> {
> - gint file = (gint) handle;
> + gint file = (gint) GPOINTER_TO_INT(handle);
idem.
> ves_icall_System_Security_Cryptography_RNGCryptoServiceProvider_RngClose (gpointer handle)
> {
> if (!egd)
> - close ((gint) handle);
> + close ((gint) GPOINTER_TO_INT(handle) );
idem.
> @@ -5951,7 +5951,7 @@
> unverified:
> if (cfg->method != method)
> g_hash_table_destroy (bbhash);
> - g_error ("Invalid IL code at IL%04x in %s: %s\n", ip - header->code,
> + g_error ("Invalid IL code at IL%04"x_INT_PTR_T" in %s: %s\n", ip - header->code,
Here and in other places like this, the correct thing to do is to change the code
to cast the pointer difference to an int:
g_error ("Invalid IL code at IL%04x in %s: %s\n", (int)(ip - header->code),
because we're not going to have IL methods bigger than 2 gigabytes.
> Index: mono/mini/mini.h
> ===================================================================
> --- mono/mini/mini.h (revision 37033)
> +++ mono/mini/mini.h (working copy)
> @@ -2,6 +2,7 @@
> #define __MONO_MINI_H__
>
> #include "config.h"
> +#include "intps.h"
> #include <glib.h>
> #include <signal.h>
> #include <mono/metadata/loader.h>
> @@ -138,8 +139,8 @@
> gint32 cil_length;
>
> /* The address of the generated code, used for fixups */
> - int native_offset;
> - int max_offset;
> + intptr_t native_offset;
> + intptr_t max_offset;
They are offsets, the ints are fine.
> Index: mono/mini/mini-exceptions.c
> ===================================================================
> --- mono/mini/mini-exceptions.c (revision 37033)
> +++ mono/mini/mini-exceptions.c (working copy)
> @@ -16,6 +16,7 @@
> #include <mono/metadata/tabledefs.h>
> #include <mono/metadata/threads.h>
> #include <mono/metadata/debug-helpers.h>
> +#include <mono/metadata/mono-debug-debugger.h>
> #include <mono/metadata/exception.h>
> #include <mono/metadata/gc-internal.h>
> #include <mono/metadata/mono-debug.h>
> @@ -499,8 +500,8 @@
> /* Switch back to normal stack */
> if (stack_overflow)
> /* Free up some stack space */
> - MONO_CONTEXT_SET_SP (&initial_ctx, (guint32)(MONO_CONTEXT_GET_SP (&initial_ctx)) + (64 * 1024));
> - MONO_CONTEXT_SET_IP (&initial_ctx, (unsigned int)jit_tls->abort_func);
> + MONO_CONTEXT_SET_SP (&initial_ctx, GPOINTER_TO_UINT(MONO_CONTEXT_GET_SP (&initial_ctx)) + (64 * 1024));
> + MONO_CONTEXT_SET_IP (&initial_ctx, GPOINTER_TO_UINT(jit_tls->abort_func));
The code here is broken and the change just masks the bug. abort_func
should not be cast to a uint: it is a pointer.
> Index: mono/mini/linear-scan.c
> ===================================================================
> --- mono/mini/linear-scan.c (revision 37033)
> +++ mono/mini/linear-scan.c (working copy)
> @@ -90,7 +90,7 @@
> max_regs = g_list_length (regs);
>
> for (l = regs; l; l = l->next) {
> - int regnum = (int)l->data;
> + int regnum = (int) GPOINTER_TO_INT(l->data);
Extra cast.
> @@ -154,7 +154,7 @@
>
> g_assert (regs);
>
> - vmv->reg = (int)regs->data;
> + vmv->reg = (int) GPOINTER_TO_INT(regs->data);
Idem.
> #if SIZEOF_VOID_P == 8
> /*only applies if sizeof(void*)==8*/
> #define d_INT_PTR_T "ld"
> #define d_INT_64 "ld"
> #define u_INT_PTR_T "lu"
> #define u_INT_64 "lu"
> #define x_INT_PTR_T "lx"
> #define x_INT_64 "lx"
> #else
> /*else switch back to normal*/
> #define d_INT_PTR_T "d"
> #define d_INT_64 "lld"
> #define u_INT_PTR_T "u"
> #define u_INT_64 "llu"
> #define x_INT_PTR_T "x"
> #define x_INT_64 "llx"
> #endif
I don't like the macro names and they don't follow the convention
in mono: they pullute the namespace. Maybe MONO_FMT_PTR/MONO_FMT_64
and leave the d/u/x format in the string.
lupus
--
-----------------------------------------------------------------
lupus at debian.org debian/rules
lupus at ximian.com Monkeys do it better
More information about the Mono-devel-list
mailing list