[Lldb-commits] [lldb] r295369 - Fix build

Tim Hammerquist via lldb-commits lldb-commits at lists.llvm.org
Sun Aug 6 20:00:30 PDT 2017


Thanks, Zachary.

Maybe that's where I'm getting confused. What is the case that it's testing?

It appears to simply be setting the locale such that the characters in
Invalid fail to be encoded correctly, then asserts that (a) the call to
VASPrintf() fails and (b) the resulting string is the expected
"<EncodingError>".

Is there something special about this locale ".932"? Or do we just need to
ensure that VASPrintf() responds correctly when the underlying calls to
vsnprintf() fail to encode/fit?

If it's the latter case, is it possible to find both an encoding that is
present on most systems, and a test string that will fail for that encoding?

Alternately, if the test relies on being able to set the locale to ".932",
would it be acceptable to test for the failure of setlocale() and bail out
of the test?



On Sun, Aug 6, 2017 at 10:27 AM, Zachary Turner <zturner at google.com> wrote:

> I believe locale codes are platform specific, so perhaps this ".932" is
> not present on these systems.  This was always a risk, but there's no other
> good way to test this.  Given that it's testing a very obscure error case,
> one option would be to remove the test.  Another would be to find out a
> correct locale code to use on each of the platforms.
>
> On Sun, Aug 6, 2017 at 2:28 AM Tim Hammerquist <penryu at gmail.com> wrote:
>
>> Hi Zachary, Pavel,
>>
>> I'm working on integrating the VASprintfTest.cpp test and other unittests
>> into the Xcode project and I wonder if I can get some information about
>> the EncodingError test in LLDB's VASprintfTest.cpp.
>>
>> It seems to try to store the current locale; set the locale to a new,
>> invalid locale; and then try to output a wide character (asserted to be
>> invalid) before restoring the original locale and closing.
>>
>> However, in practice, it seems to be failing to set the new locale
>> (setlocale(3) returns nullptr) and the locale retains the same value it had
>> on entering the test. In this case, any 7- or 8-bit locale will succeed
>> ("C", "*/US-ASCII", "*/ISO8859-1", et al. will fail to render the provided
>> Invalid characters) while wide-char friendly locales ("*/UTF-8") will
>> succeed in representing the "Invalid" wchar_t string, causing the test to
>> fail.
>>
>> I've witnessed this behavior ('setlocale(LC_ALL, ".932") => nullptr) in
>> Darwin, Linux, and FreeBSD and want to make sure I'm not misunderstanding
>> something about how this test works. Do I understand this test correctly?
>>
>> Thanks for your input!
>> -Tim
>>
>>
>> On Fri, Feb 17, 2017 at 7:42 AM, Zachary Turner via lldb-commits <
>> lldb-commits at lists.llvm.org> wrote:
>>
>>> Sorry about that, it's often frustrating to check every single buildbot
>>> that was failing because 99% of the time they fail for exactly the same
>>> reason, and when one is fixed all of them get fixed. In this case i used
>>> http://lab.llvm.org:8011/builders/lldb-amd64-ninja-freebsd11/builds/5417
>>> as evidence that the change was good, but in hindsight this one doesn't run
>>> tests, so that was my mistake.
>>>
>>> Regarding the tests, i ran all of UtilityTests but not the whole test
>>> suite.  Again my mistake, was trying to cut corners and it backfired :(
>>>
>>>
>>>
>>> On Fri, Feb 17, 2017 at 2:35 AM Pavel Labath <labath at google.com> wrote:
>>>
>>> Hey Zach,
>>>
>>> after you think you've fixed the build, could you check back on the
>>> buildbot to make sure that it actually fixes things? In this case you
>>> would've seen that after the build is fixed, the next thing it runs
>>> into is about a dozen test failures.
>>>
>>> In fact, this breakage was something that would have been already
>>> caught by running the unit tests on windows, which leads me to believe
>>> you didn't run them before submission.
>>>
>>> Thanks,
>>> pl
>>>
>>>
>>> 2017-02-16 20:15 GMT+00:00 Zachary Turner via lldb-commits
>>> <lldb-commits at lists.llvm.org>:
>>> > Author: zturner
>>> > Date: Thu Feb 16 14:15:26 2017
>>> > New Revision: 295369
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=295369&view=rev
>>> > Log:
>>> > Fix build
>>> >
>>> > Modified:
>>> >     lldb/trunk/source/Utility/VASprintf.cpp
>>> >     lldb/trunk/unittests/Utility/VASprintfTest.cpp
>>> >
>>> > Modified: lldb/trunk/source/Utility/VASprintf.cpp
>>> > URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/
>>> Utility/VASprintf.cpp?rev=295369&r1=295368&r2=295369&view=diff
>>> > ============================================================
>>> ==================
>>> > --- lldb/trunk/source/Utility/VASprintf.cpp (original)
>>> > +++ lldb/trunk/source/Utility/VASprintf.cpp Thu Feb 16 14:15:26 2017
>>> > @@ -7,7 +7,7 @@
>>> >  //
>>> >  //===-------------------------------------------------------
>>> ---------------===//
>>> >
>>> > -#include "lldb/Utility/VASprintf.h"
>>> > +#include "lldb/Utility/VASPrintf.h"
>>> >
>>> >  #include "llvm/ADT/SmallString.h"
>>> >
>>> >
>>> > Modified: lldb/trunk/unittests/Utility/VASprintfTest.cpp
>>> > URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/
>>> Utility/VASprintfTest.cpp?rev=295369&r1=295368&r2=295369&view=diff
>>> > ============================================================
>>> ==================
>>> > --- lldb/trunk/unittests/Utility/VASprintfTest.cpp (original)
>>> > +++ lldb/trunk/unittests/Utility/VASprintfTest.cpp Thu Feb 16
>>> 14:15:26 2017
>>> > @@ -7,7 +7,7 @@
>>> >  //
>>> >  //===-------------------------------------------------------
>>> ---------------===//
>>> >
>>> > -#include "lldb/Utility/VASprintf.h"
>>> > +#include "lldb/Utility/VASPrintf.h"
>>> >  #include "llvm/ADT/SmallString.h"
>>> >
>>> >  #include "gtest/gtest.h"
>>> >
>>> >
>>> > _______________________________________________
>>> > lldb-commits mailing list
>>> > lldb-commits at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>>
>>>
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>>
>>>
>>
>>
>> --
>> Tim <penryu at gmail.com>
>>
>


-- 
Tim <penryu at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170806/047a6a92/attachment.html>


More information about the lldb-commits mailing list