[Lldb-commits] AdvDataFormatterTestCase relies on zeroed memory to pass

[jingham at apple.com]

I'm not sure I agree with this.  It doesn't seem like a good idea to have tests with undefined behavior ("on some systems there might be junk there that trips us up" seems a unreliable way to test handling bad data.)  If we want to set up that kind of test we should either initialize values with random junk on purpose (though this seems a bit brute force) or test with patterns that we know have caused us problems or suspect might be tricky to handle.

Jim

On Jun 25, 2013, at 10:39 AM, Enrico Granata <egranata at apple.com> wrote:

> That is definitely the other option.
> I tend to agree with Ed’s argument that the main.cpp is not doing anything illegal or that real programmers who use LLDB won’t ever run into.
> It seems a reasonable thing to test: what happens when a data formatter encounters random memory.
> On OSX it happens to be 0-filled so no special behavior will occur, but on other platforms we might mask out potential issues by doing the explicit zeroing
> My vote goes with letting LLDB read and dump data and just checking the first byte - as long as we produce the “X and don’t crash, we are doing the right thing for the user
> 
> Enrico Granata
> 📩 egranata@.com
> ☎️ 27683
> 
> On Jun 25, 2013, at 10:28 AM, "Malea, Daniel" <daniel.malea at intel.com> wrote:
> 
>> I don't think the test is supposed to depend on null-allocated stack.
>> Unless we're explicitly testing what happens when LLDB reads random
>> memory, I vote we initialize the char* properly, like:
>> 
>> diff --git 
>> a/test/functionalities/data-formatter/data-formatter-advanced/main.cpp
>> b/test/functionalities/data-formatter/data-formatter-advanced/main.cpp
>> index 8e5ea0c..2462e28 100644
>> --- a/test/functionalities/data-formatter/data-formatter-advanced/main.cpp
>> +++ b/test/functionalities/data-formatter/data-formatter-advanced/main.cpp
>> @@ -61,8 +61,11 @@ struct SimpleWithPointers
>>        SimpleWithPointers(int X, float Y, char Z) :
>>        x(new int (X)),
>>        y(new float (Y)),
>> -       z(new char (Z))
>> -       {}
>> +       z(new char[2])
>> +       {
>> +          z[0] = Z;
>> +          z[1] = '\0';
>> +        }
>> };
>> 
>> 
>> Cheers,
>> Dan
>> 
>> 
>> 
>> On 2013-06-25 1:06 PM, "Ed Maste" <emaste at freebsd.org> wrote:
>> 
>>> To see if there's a common set of root causes for test failures on
>>> FreeBSD I took a quick look at a sample of failing test cases.  One
>>> that I happened to look at, which doesn't have a common failure cause,
>>> is AdvDataFormatterTestCase.
>>> 
>>> It relies on a zeroed stack to pass. My FreeBSD desktop runs with
>>> memory diagnostics enabled, so memory is filled with 0xa5 by default.
>>> Running the command from the test case gives me:
>>> 
>>> (lldb) frame variable couple
>>> (Couple) couple = {
>>> sp = {
>>>   x = 0x0000000801808098
>>>   y = 0x000000080180809c
>>>   z = 0x0000000801809104 "X\xa53\xa56\xa59\xa5"
>>> }
>>> s = 0x0000000801807050
>>> 
>>> and if I turn off the memory diagnostics, I get:
>>> 
>>> (lldb) frame variable couple
>>> (Couple) couple = {
>>> sp = {
>>>   x = 0x0000000801808098
>>>   y = 0x000000080180809c
>>>   z = 0x0000000801809104 "X"
>>> }
>>> s = 0x0000000801807050
>>> 
>>> We could change main.cpp to avoid this, but it is a case
>>> representative of real debugging experience, so I'm tempted to just
>>> drop the final " from the match string:
>>> 
>>> -            substrs = ['(Couple) couple =  {','sp = {','z =','"X"'])
>>> +            substrs = ['(Couple) couple =  {','sp = {','z =','"X'])
>>> 
>>> Seem reasonable?
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits