[llvm] r187106 - Debug Info: improve the verifier to check field types.

Manman Ren manman.ren at gmail.com
Thu Jul 25 14:25:31 PDT 2013


On 7/25/13, 2:01 PM, David Blaikie wrote:
> On Thu, Jul 25, 2013 at 1:57 PM, Manman Ren <manman.ren at gmail.com> wrote:
>> On 7/25/13, 1:46 PM, David Blaikie wrote:
>>> On Thu, Jul 25, 2013 at 1:40 PM, Eric Christopher <echristo at gmail.com>
>>> wrote:
>>>> Oh I see what you mean now. We should just disallow the usage of
>>>> strings in the MDNode fields.
>>> I think we use i32 0 for null in these fields too, if I recall
>>> correctly - if I'm right, we probably should disallow that too. If
>>> it's an MDNode field, it should be an MDNode, most likely (I'm open to
>>> being shown a reason for this not to be true - but we should talk
>>> about/justify that rather than just accepting it as "that's the way
>>> things are").
>> Yes, MDNodes should be MDNodes, but currently we use null, i32 0, maybe
>> empty string "".
>> We can improve the verifier incrementally.
>> And each time we improve the verifier, we will update the testing cases.
> Yep, please include a FIXME that the implementation of that function
> should be replaced by simply "getFieldAs<MDNode>(num)" or whatever
> simple implementation would work.
getFieldAs used a dyn_cast so it is not going to catch any error.
I added a FIXME in r187157.

Thanks,
Manman
>
> (& if you want, you could include the i32 int case, rather than
> allowing any i32 value)
>
>> Thanks,
>> Manman




More information about the llvm-commits mailing list