Bug 189070 – lldb-webkit: KeyError thrown for OptionSet with invalid value

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

An OptionSet variable can be in scope and uninitialized as a variable is not initialized until execution has flowed over its assignment. We need to take care to handle this case and not try to compute the set of enumerators in an uninitialized OptionSet.

[Daniel Bates]

Created attachment 348363 [details]
Patch

[Daniel Bates]

Comment on attachment 348363 [details]
Patch

This is wrong.

[Daniel Bates]

Created attachment 348436 [details]
Patch

[Daniel Bates]

Committed r235482: <https://trac.webkit.org/changeset/235482>

[Radar WebKit Bug Importer]

<rdar://problem/43860433>

[Darin Adler]

Comment on attachment 348436 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=348436&action=review

> Tools/lldb/lldb_webkit.py:627
> +            return  # self.valobj is uninitialized memory

This comment and the bug title is too specific and potentially incorrect.

This code path can occur any time the OptionSet has an invalid value.

An uninitialized variable is one case when something might have an invalid value, but of course something uninitialized might also have a valid value. But there are many others such as a variable that has been overwritten by a buffer overrun, a variable been overwritten by something writing through a stale pointer, trying to display a value after a local variable memory region is reused for another purpose because we are past the point of use of the local variable in the code, a value from following a pointer to an object that has already been deleted, and many more.

On the other hand, I suspect the *behavior* of the code you wrote is probably good; it writes out something that lets you see the value of the memory and makes it clear that the OptionSet has an invalid value.

[Daniel Bates]

(In reply to Darin Adler from comment #6)
> Comment on attachment 348436 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=348436&action=review
> 
> > Tools/lldb/lldb_webkit.py:627
> > +            return  # self.valobj is uninitialized memory
> 
> This comment and the bug title is too specific and potentially incorrect.
> 
> This code path can occur any time the OptionSet has an invalid value.
> 
> An uninitialized variable is one case when something might have an invalid
> value, but of course something uninitialized might also have a valid value.
> But there are many others such as a variable that has been overwritten by a
> buffer overrun, a variable been overwritten by something writing through a
> stale pointer, trying to display a value after a local variable memory
> region is reused for another purpose because we are past the point of use of
> the local variable in the code, a value from following a pointer to an
> object that has already been deleted, and many more.

You're right!

We could either remove the comment or I could update the comment to explain that self.valobj has an invalid value,  maybe:

# self.valobj has an invalid value.

What do you suggest?

[Darin Adler]

Either removing the comment or updating it is fine with me.

[Darin Adler]

An even more useful comment would make it clear why just returning early is the right thing to do for an invalid value.

    # Since this is an invalid value, return so the raw hex form is written out.

Or a comment that is correct.

[Daniel Bates]

(In reply to Darin Adler from comment #9)
> An even more useful comment would make it clear why just returning early is
> the right thing to do for an invalid value.
> 
>     # Since this is an invalid value, return so the raw hex form is written
> out.
> 
> Or a comment that is correct.

Updated comment as well as the change log entry for r235482 to reflect the updated title for this bug and better describe the purpose of the change. Committed this in <https://trac.webkit.org/changeset/235555>.