RESOLVED FIXED 189070
lldb-webkit: KeyError thrown for OptionSet with invalid value
https://bugs.webkit.org/show_bug.cgi?id=189070
Summary lldb-webkit: KeyError thrown for OptionSet with invalid value
Daniel Bates
Reported Wednesday, August 29, 2018 1:03:45 AM UTC
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.
Attachments
Patch (2.67 KB, patch)
2018-08-28 17:17 PDT, Daniel Bates
no flags
Patch (2.79 KB, patch)
2018-08-29 15:12 PDT, Daniel Bates
simon.fraser: review+
Daniel Bates
Comment 1 Wednesday, August 29, 2018 1:17:32 AM UTC
Daniel Bates
Comment 2 Wednesday, August 29, 2018 1:22:24 AM UTC
Comment on attachment 348363 [details] Patch This is wrong.
Daniel Bates
Comment 3 Wednesday, August 29, 2018 11:12:02 PM UTC
Daniel Bates
Comment 4 Wednesday, August 29, 2018 11:20:28 PM UTC
Radar WebKit Bug Importer
Comment 5 Wednesday, August 29, 2018 11:21:17 PM UTC
Darin Adler
Comment 6 Thursday, August 30, 2018 2:13:33 AM UTC
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
Comment 7 Thursday, August 30, 2018 5:40:51 PM UTC
(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
Comment 8 Friday, August 31, 2018 2:25:44 AM UTC
Either removing the comment or updating it is fine with me.
Darin Adler
Comment 9 Friday, August 31, 2018 2:26:34 AM UTC
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
Comment 10 Friday, August 31, 2018 5:45:48 PM UTC
(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>.
Note You need to log in before you can comment on or make changes to this bug.