This is an archive of the discontinued LLVM Phabricator instance.

[MCParser] Set default alignment value when meeting invalid align
ClosedPublic

Authored by serge-sans-paille on May 16 2022, 7:07 AM.

Details

Summary

Upon invalid alignment value, still set a default valid alignment value to avoid
hitting later asserts.

Fix #55273

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2022, 7:07 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
serge-sans-paille requested review of this revision.May 16 2022, 7:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2022, 7:07 AM
nikic added a subscriber: nikic.May 16 2022, 7:11 AM

Is it possible to add a test for this?

Is it possible to add a test for this?

I wonder... is there a way to make the difference between an error and an abort? Because without this patch we get an error message followed by an abort, and with this patch we only get the error.

nikic added a comment.May 16 2022, 8:10 AM

Is it possible to add a test for this?

I wonder... is there a way to make the difference between an error and an abort? Because without this patch we get an error message followed by an abort, and with this patch we only get the error.

I think a test that aborts would always fail by default unless not --crash is used.

Update existing test case

Gentle ping :-) I'm a little unsure on the value change upon error, but that's also a bit strange to issue an error but still generate some output.

nikic added inline comments.Jun 1 2022, 6:32 AM
llvm/lib/MC/MCParser/AsmParser.cpp
3457

What if the alignment is a non-pow2 greater than 32-bit? I think that case will sneak through.

Address reviewer comment and add an extra test case to capture the issue.

nikic accepted this revision.Jun 2 2022, 5:37 AM

LGTM

This revision is now accepted and ready to land.Jun 2 2022, 5:37 AM