[Mono-bugs] [Bug 526029] New: metadata-verify.c broken on big-endian hosts

[bugzilla_noreply at novell.com]

http://bugzilla.novell.com/show_bug.cgi?id=526029


           Summary: metadata-verify.c broken on big-endian hosts
    Classification: Mono
           Product: Mono: Runtime
           Version: SVN
          Platform: BigEndian
        OS/Version: All
            Status: NEW
          Severity: Critical
          Priority: P5 - None
         Component: misc
        AssignedTo: mono-bugs at lists.ximian.com
        ReportedBy: uweigand at de.ibm.com
         QAContact: mono-bugs at lists.ximian.com
          Found By: ---


Created an attachment (id=308604)
 --> (http://bugzilla.novell.com/attachment.cgi?id=308604)
Fix metadata-verify.c safe_read* argument type mismatches

Starting with SVN revision 136729, just about any of the more complex test
cases fails with the error "The assembly mscorlib.dll was not found or could
not be loaded." on PowerPC (and presumaby other big-endian host systems).

This SVN revision added a call to mono_verifier_verify_table_data in
do_mono_image_load:

Index: mono/metadata/ChangeLog
===================================================================
*** mono/metadata/ChangeLog     (revision 136728)
--- mono/metadata/ChangeLog     (revision 136729)
***************
*** 1,5 ****
--- 1,9 ----
  2009-06-23 Rodrigo Kumpera  <rkumpera at novell.com>

+       * image.c (do_mono_image_load): Enable table data verification.
+
+ 2009-06-23 Rodrigo Kumpera  <rkumpera at novell.com>
+
        * metadata-verify.c (is_valid_constant): Fix nullref check.

  2009-06-23 Rodrigo Kumpera  <rkumpera at novell.com>
Index: mono/metadata/image.c
===================================================================
*** mono/metadata/image.c       (revision 136728)
--- mono/metadata/image.c       (revision 136729)
*************** do_mono_image_load (MonoImage *image, Mo
*** 904,909 ****
--- 904,912 ----
        if (!mono_image_load_cli_data (image))
                goto invalid_image;

+       if (!mono_verifier_verify_table_data (image, NULL))
+               goto invalid_image;
+
        mono_image_load_names (image);

        load_modules (image);


Due to a pre-existing bug in metadata-verify.c, this call just about always
fails on big-endian host systems, causing the load of mscorlib.dll to fail.

The bug is ultimately caused by non-typesafe macros like safe_read8 in
metadata-verify.c:

#define safe_read8(VAR, PTR, LIMIT) safe_read (&PTR, LIMIT, &VAR, 1)

static gboolean
safe_read (const char **_ptr, const char *limit, void *dest, int size)
{
        const char *ptr = *_ptr;
        if (ptr + size > limit)
                return FALSE;
        switch (size) {
        case 1:
                *((guint8*)dest) = *((guint8*)ptr);
                ++ptr;
                break;
        case 2:
                *((guint16*)dest) = read16 (ptr);
                ptr += 2;
                break;
        case 4:
                *((guint32*)dest) = read32 (ptr);
                ptr += 4;
                break;
        }
        *_ptr = ptr;
        return TRUE;
}

In effect, the variable VAR is accessed via a pointer to guint8, no matter what
actual type VAR has.  Compiler warnings you'd otherwise get are suppressed by
the intermediate cast to a void pointer as argument to safe_read.

All this would still work out OK if every user of safe_read8 would actually
pass a variable of type uint8 as first argument.  But this is not always the
case, for example:

static gboolean
parse_return_type (VerifyContext *ctx, const char **_ptr, const char *end)
{
        const char *ptr;
        int type = 0;

        if (!parse_custom_mods (ctx, _ptr, end))
                return FALSE;

        ptr = *_ptr;
        if (!safe_read8 (type, ptr, end))
                FAIL (ctx, g_strdup ("ReturnType: Not enough room for the
type"));

Now, on a little-endian host, because type is pre-initialized to 0, accessing
*(guint8 *)&type happens to have the expected effect.  This is not the case on
big-endian host systems, however.

I'm appending a patch that fixes all uses of safe_read8, safe_read16,
safe_read32 and safe_read_cint to always pass an argument of the appropriate
expected type (guint8, guint16, guint32, and unsigned, respectively).  This
fixes the problem for me on PowerPC; test cases run fine again.

(Longer term, it might be safer to replace those macros by type-safe functions
to prevent new instances of the problem from being re-introduced ...)

-- 
Configure bugmail: http://bugzilla.novell.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.