[PATCH] Add a 'no-asserts' requirement option to LIT.

[David Blaikie]

On Tue, Feb 3, 2015 at 1:41 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> On Feb 3, 2015, at 1:32 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Feb 3, 2015 at 1:17 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>
>>
>> On Feb 3, 2015, at 12:32 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Tue, Feb 3, 2015 at 12:19 PM, Mehdi Amini <mehdi.amini at apple.com>
>> wrote:
>>
>>> Hi David,
>>>
>>> On Feb 3, 2015, at 11:15 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>> On Tue, Feb 3, 2015 at 11:03 AM, Owen Anderson <resistor at mac.com> wrote:
>>>
>>>> Hi chandlerc, joker.eph,
>>>>
>>>> This is useful when checking diagnostics and annotations that are only
>>>> enabled in asserts mode.
>>>
>>>
>>> Not sure I quite follow - "REQUIRES: Asserts" would be used for any test
>>> case that's verifying a failure that's only enabled in asserts mode.
>>> "REQUIRES: no-asserts" would be for testing the absence of that same
>>> failure in a no-asserts build?
>>>
>>> That seems like a thing I wouldn't want to test for. The idea is that
>>> the program has undefined behavior if it would assert but you're in a
>>> non-asserts build. That's not a thing to test for - there's no
>>> specific/guaranteed behavior in that case.
>>>
>>>
>>> To provide you with more context: I am expecting some optional metadata
>>> from the front-end, but in case they are malformed for any reason I want to
>>> be bullet proof and handle this case gracefully (i.e. undefined behavior /
>>> crashing is not acceptable). So I’d like my assert build to verify the
>>> validity of the metadata but my release build to ignore it and set a
>>> suitable default value that I know allows recovery most of the time.
>>>
>>> I can’t add a test to check if we recover correctly from an invalid
>>> metadata without this.
>>>
>>> This is valid in general for the category of assertions we put to
>>> enforce some invariant/properties that are important for optimization
>>> purposes but does not prevent us from generating a correct code if they are
>>> broken.
>>>
>>> Does it make sense?
>>>
>>
>> Not sure how everyone else feels, but I'd rather classify something like
>> that that has well-defined recovery semantics, as something other than
>> 'validity’.
>>
>>
>> To be sure I understand correctly: do you mean an assertions is not
>> welcome on the normal path if we have a well-defined recovery path?
>>
>
> That's my opinion: Failure to satisfy assertions implies UB and cannot be
> tested for or verified in any way. Any assertion failure represents an
> invariant in the program. If that's not the case then it's defined behavior
> we should be always testing for. Otherwise it seems too easy to introduce
> failures in the expected graceful recovery path.
>
>
> Thanks for your input. That’s fine with me.
> But then what is it the alternative? I’d still like a mechanism that would
> be “report an error in debug mode and handle gracefully in release” for the
> thing that don’t go in the category you describe. If assert() is not
> appropriated, do you have a suggestion?
>

Sounds like Reid had an idea that might be compatible - possibly an
error-by-default that's either disabled in your use case, or you already
have a diagnostic handler that suppresses all warnings in your embedded
use-case anyway, so it might be no work to integrate that. (essentially not
a release-v-asserts difference (which I think isn't the right
differentiatior) and a llvm-commandline-tools-V-embedded use case - the
tools always error, the embedded caller suppresses the warnings and carries
on regardless. And you can add test cases that disable the warning/error to
test the recovery). This also has the advantage that you get full coverage
regardless of an asserts or non-asserts build. (either can still run the
test cases that have the warning/error disabled and check for good fallback
behavior)

- David


>
> Thanks,
>
> Mehdi
>
>
>
> Not sure about other people, maybe the prevailing attitude is not
> consistent with mine.
>
>
>>
>> Thanks,
>>
>> —
>> Mehdi
>>
>>
>>
>> When/where/how do you expect invalid metadata to come into your system?
>>
>>
>>>
>>> Thanks.
>>>
>>> Mehdi
>>>
>>>
>>>
>>>
>>>
>>>> Today we can only test the positive case (when asserts are enabled) but
>>>> not the negative case.
>>>>
>>>> REPOSITORY
>>>>   rL LLVM
>>>>
>>>> http://reviews.llvm.org/D7383
>>>>
>>>> Files:
>>>>   test/lit.cfg
>>>>
>>>> Index: test/lit.cfg
>>>> ===================================================================
>>>> --- test/lit.cfg
>>>> +++ test/lit.cfg
>>>> @@ -370,6 +370,8 @@
>>>>
>>>>  if re.search(r'ON', llvm_config_cmd.stdout.read().decode('ascii')):
>>>>      config.available_features.add('asserts')
>>>> +else:
>>>> +    config.available_features.add('no-asserts')
>>>>  llvm_config_cmd.wait()
>>>>
>>>>  if 'darwin' == sys.platform:
>>>>
>>>> EMAIL PREFERENCES
>>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150203/b5797439/attachment.html>