[Mono-dev] Bug 10784

Andi McClure andi.mcclure at xamarin.com
Wed Jan 6 16:49:18 UTC 2016


Jamie:

A good place to start would be with running the automated tests to see if
there are any new failures. You can quickly run the most relevant tests for
this change with:
make -C mono/mini check
make -C mono/tests check
make -C mcs/class/corlib check
If you run `make check` at the toplevel, this will run all tests. This will
take a pretty long time however (and if you submit a pull request our
jenkins bot will do it for you).

We normally accept patches through pull requests to mono/mono on Github.
When your patch is ready (which it seems to me to basically be, if the
tests pass) you should sign the CLA ( https://cla.xamarin.com/ ) and then
open a pull request so it can be merged.

Two comments I would leave if I saw this as a pull request are:
    g_assert (iid <= 1000000000);
This number is pretty arbitrary. Please make a constant like
LOADED_INTERFACES_MAX to hold it instead of just putting it inline the
code. Also, I think you might as well bump the value up to 2^31-2, since
that's the highest number there's a technical reason not to go above…
Also, "however haven't bitset code still uses" seems to be a typo.
    g_assert(icount < 65535);
This can and I think should be <=

I wonder if there's a non-ridiculous way to add a new automated test for a
program with >65535 interfaces.

Thanks

On Tue, Jan 5, 2016 at 9:04 PM, Jamie Venning <
jamie_venning at technologyonecorp.com> wrote:

> I've gone through and changed interface_id to a guint32, along with
> max_interface_id, leaving interface_count (I didn't see the need for it to
> be increased).
>
> I've forked the repo and have it over at
> https://github.com/tastywheattasteslikechicken/mono/commit/6e7f1d94eb07822fb7bf5687b67df5076f969818
> Are there any other considerations or things that may be missed?
>
> On Wed, Jan 6, 2016 at 12:51 AM, Jamie Venning <
> jamie_venning at technologyonecorp.com> wrote:
>
>> Yeah, what I'm chasing are the best ways to cause failures without this
>> assertion -- that way as I go through and fix things I can be more
>> confident that things haven't been missed.
>>
>> On Tue, Jan 5, 2016 at 11:45 PM, Alexander Köplinger <
>> alexander.koeplinger at xamarin.com> wrote:
>>
>>> I don't think it's as easy as removing the assert.
>>>
>>> For one, the interface ID returned by mono_get_unique_iid() is stored as
>>> a guint16 (
>>> https://github.com/mono/mono/blob/36c7332104eb5250a93079ae77c2e0dbf12c6c9a/mono/metadata/class-internals.h#L344)
>>> which means you only have 0-65,535 values there. I'm not familiar enough
>>> with the runtime codebase to say which other problems you'll run into if
>>> you try to fix this.
>>>
>>> - Alex
>>>
>>> 2016-01-05 8:38 GMT+01:00 Jamie Venning <
>>> jamie_venning at technologyonecorp.com>:
>>>
>>>> Good Morning.
>>>>
>>>> Bug 10784 (
>>>> https://bugzilla.xamarin.com/show_bug.cgi?format=multiple&id=10784) is
>>>> a major blocker for me, and a big obstacle for larger applications moving
>>>> from .NET to Mono. Because I need this fixed, I am prepared to fix it
>>>> myself.
>>>>
>>>> I'm wondering if anyone knowledgeable in this area could answer these
>>>> few, short questions just to confirm a few things.
>>>>
>>>> Removing the offending assertion from class.c, and running generated
>>>> test cases that previously triggered it, I did not notice any incorrect
>>>> behaviour in terms of reflection, the 'is' operator failing, or the wrong
>>>> implementation of functions being used. Is there anywhere that I should be
>>>> looking in particular or any particular type of tests I can use to expose
>>>> an underlying fault that removing this assertion causes?
>>>>
>>>> Is it possible that this assertion is vestigial or unnecessary?
>>>>
>>>> --
>>>>
>>>> James Venning
>>>> Research & Development - Ci2 - Product
>>>> Phone: +61 7 3167 7300 | Fax: +61 7 3167 7301 Address: Level 11,
>>>> TechnologyOne HQ, 540 Wickham Street, Fortitude Valley QLD 4006 (PO Box 96
>>>> Fortitude Valley QLD 4006) Email: jamie_venning at technologyonecorp.com
>>>> Web: TechnologyOneCorp.com <http://www.technologyonecorp.com/>
>>>>
>>>> <http://www.technologyonecorp.com/>
>>>>
>>>> Financials | Human Resource & Payroll | Supply Chain | Corporate
>>>> Performance Management | Property & Rating | Student Management | Asset
>>>> Management | Enterprise Content Management | Customer Relationship
>>>> Management | Consulting Services
>>>>
>>>> *TechnologyOne (ASX:TNE) is Australia's largest publicly listed
>>>> software company, with offices across six countries including each state
>>>> and territory of Australia, as well as New Zealand, the South Pacific, Asia
>>>> and the United Kingdom. For 25 years, we have been providing powerful and
>>>> deeply integrated enterprise software solutions that are used every day by
>>>> more than 1000 leading corporations, government departments and statutory
>>>> authorities.*
>>>>
>>>> TechnologyOne accepts no liability for any damage caused by this email
>>>> or its attachments. The information in this email is only for the intended
>>>> recipient and may contain confidential and/or privileged material. If you
>>>> received this in error, please kindly notify the sender and delete this
>>>> email and any attachments from your system. Opinions, conclusions and other
>>>> information in this message that do not relate to the official business of
>>>> the company shall be understood as neither given nor endorsed by it. If you
>>>> believe that you have been spammed please email
>>>> Stop_Spam at TechnologyOneCorp.com <stop_spam at technologyonecorp.com>.
>>>>
>>>> _______________________________________________
>>>> Mono-devel-list mailing list
>>>> Mono-devel-list at lists.ximian.com
>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>
>>>>
>>>
>>
>>
>> --
>>
>> Jamie Venning
>> Research & Development - Ci2 - Product
>> Phone: +61 7 3167 7300 | Fax: +61 7 3167 7301 Address: Level 11,
>> TechnologyOne HQ, 540 Wickham Street, Fortitude Valley QLD 4006 (PO Box 96
>> Fortitude Valley QLD 4006) Email: jamie_venning at technologyonecorp.com
>> Web: TechnologyOneCorp.com <http://www.technologyonecorp.com/>
>>
>> <http://www.technologyonecorp.com/>
>>
>> Financials | Human Resource & Payroll | Supply Chain | Corporate
>> Performance Management | Property & Rating | Student Management | Asset
>> Management | Enterprise Content Management | Customer Relationship
>> Management | Consulting Services
>>
>> *TechnologyOne (ASX:TNE) is Australia's largest publicly listed software
>> company, with offices across six countries including each state and
>> territory of Australia, as well as New Zealand, the South Pacific, Asia and
>> the United Kingdom. For 25 years, we have been providing powerful and
>> deeply integrated enterprise software solutions that are used every day by
>> more than 1000 leading corporations, government departments and statutory
>> authorities.*
>>
>> TechnologyOne accepts no liability for any damage caused by this email or
>> its attachments. The information in this email is only for the intended
>> recipient and may contain confidential and/or privileged material. If you
>> received this in error, please kindly notify the sender and delete this
>> email and any attachments from your system. Opinions, conclusions and other
>> information in this message that do not relate to the official business of
>> the company shall be understood as neither given nor endorsed by it. If you
>> believe that you have been spammed please email
>> Stop_Spam at TechnologyOneCorp.com <stop_spam at technologyonecorp.com>.
>>
>
>
>
> --
>
> Jamie Venning
> Intern Developer | Research & Development - Ci2 - Product
> Phone: +61 7 3167 7300 | Fax: +61 7 3167 7301 Address: Level 11,
> TechnologyOne HQ, 540 Wickham Street, Fortitude Valley QLD 4006 (PO Box 96
> Fortitude Valley QLD 4006) Email: jamie_venning at technologyonecorp.com
> Web: TechnologyOneCorp.com <http://www.technologyonecorp.com/>
>
> <http://www.technologyonecorp.com/>
>
> Financials | Human Resource & Payroll | Supply Chain | Corporate
> Performance Management | Property & Rating | Student Management | Asset
> Management | Enterprise Content Management | Customer Relationship
> Management | Consulting Services
>
> *TechnologyOne (ASX:TNE) is Australia's largest publicly listed software
> company, with offices across six countries including each state and
> territory of Australia, as well as New Zealand, the South Pacific, Asia and
> the United Kingdom. For 25 years, we have been providing powerful and
> deeply integrated enterprise software solutions that are used every day by
> more than 1000 leading corporations, government departments and statutory
> authorities.*
>
> TechnologyOne accepts no liability for any damage caused by this email or
> its attachments. The information in this email is only for the intended
> recipient and may contain confidential and/or privileged material. If you
> received this in error, please kindly notify the sender and delete this
> email and any attachments from your system. Opinions, conclusions and other
> information in this message that do not relate to the official business of
> the company shall be understood as neither given nor endorsed by it. If you
> believe that you have been spammed please email
> Stop_Spam at TechnologyOneCorp.com <stop_spam at technologyonecorp.com>.
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/mono-devel-list/attachments/20160106/26f92779/attachment-0001.html>


More information about the Mono-devel-list mailing list