[PATCH] D91781: [VE] Add regression test for D91151

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D91781#2406488 <https://reviews.llvm.org/D91781#2406488>, @kaz7 wrote:

> In D91781#2406224 <https://reviews.llvm.org/D91781#2406224>, @dblaikie wrote:
>
>> In D91781#2405239 <https://reviews.llvm.org/D91781#2405239>, @kaz7 wrote:
>>
>>> Hi, @dblaikie.  I generated a test case which causes a segmentation fault if we didn't apply D91151 <https://reviews.llvm.org/D91151> (https://reviews.llvm.org/D91151) following your suggestion.  I appreciate if you have more suggestions.
>>
>> Were you testing with assertions enabled? I'd expect a test case to cause an assertion failure (when the "cast" was applied to an object that wasn't the intended type) rather than a segmentation fault.
>
> There is no assertions.

'cast' has an assertion built into it if the cast is not valid ( https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Support/Casting.h#L250 ). If this dyn_cast has any effect (ie: if any execution of the program has different behavior with the dyn_cast compared to the cast) it should be true that the cast version of the code would've triggered an assertion on that same program.

Are you building/running the code with assertions enabled? (I'm not asking you to add a new assertion to the code - I'm asking if you have assertions enabled in your build so you'd see an assertion before/rather than a segmentation fault)

with assertions enabled/if the old code asserting, you may be able to simplify the test case down much further, as the assertion will ensure a more reliable/fast failure rather than some unspecified failure due to undefined behavior in the execution later on/elsewhere.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91781/new/

https://reviews.llvm.org/D91781