This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel][TableGen] Fix seg fault for zero instruction
ClosedPublic

Authored by ehjogab on Nov 5 2020, 12:11 AM.

Details

Summary

Tablegen seg faulted when parsing a Pat where the destination part has
no output (zero instruction), due to a register class lookup using
nullptr.

Diff Detail

Event Timeline

ehjogab created this revision.Nov 5 2020, 12:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 5 2020, 12:11 AM
ehjogab requested review of this revision.Nov 5 2020, 12:11 AM
bjope added inline comments.Nov 11 2020, 1:39 PM
llvm/test/TableGen/GlobalISelEmitter-zero-instr.td
7

Nice that we no longer crash, but I don't know how relevant this check is. What about checking that we get the "warning: Skipped pattern: Dst pattern root isn't a known leaf" message instead?

ehjogab added inline comments.Nov 11 2020, 11:11 PM
llvm/test/TableGen/GlobalISelEmitter-zero-instr.td
7

That'd be nice, but can it be done? I'm not very familiar with FileCheck...

ehjogab added inline comments.Nov 11 2020, 11:13 PM
llvm/test/TableGen/GlobalISelEmitter-zero-instr.td
7

I missed to mention that this check at least checks that we no longer crashes, so it's not entirely useless.

Added Paul-C-Anagnostopoulos since he's now one of the TableGen code owners.

bjope added inline comments.Nov 12 2020, 5:16 AM
llvm/test/TableGen/GlobalISelEmitter-zero-instr.td
7

One solution could be to do -o /dev/null and ... 2>&1 | FileCheck %s, to simply ignore regular output, and FileCheck on both stdout and stderr (in case the warning is printed on stderr).

I'm not sure exactly how tblgen emit those warnings, which stream, or if it is conditional based on how the binary is built (e.g. using -debug in test cases usually also require that you add // REQUIRES: asserts to avoid running the test case when debug support isn't included.

But I think that simply checking for the warning would work here. FileCheck is just doing pattern matching on it's input.

ehjogab retitled this revision from [GlobalISel][TableGen] fix seg fault for zero instruction to [GlobalISel][TableGen] Fix seg fault for zero instruction.Nov 23 2020, 1:49 AM
ehjogab updated this revision to Diff 306997.Nov 23 2020, 1:59 AM

Fixed testcase according to bjope's feedback

ehjogab updated this revision to Diff 307000.Nov 23 2020, 2:00 AM
ehjogab marked 2 inline comments as done.

Missed a part of the CHECK

This revision is now accepted and ready to land.Nov 23 2020, 10:18 AM
This revision was landed with ongoing or failed builds.Nov 23 2020, 10:49 PM
This revision was automatically updated to reflect the committed changes.

I submitted this for @ehjogab