This is an archive of the discontinued LLVM Phabricator instance.

AttributorAttributes: avoid a crashing on bad alignments
ClosedPublic

Authored by durin42 on Feb 11 2022, 4:15 PM.

Details

Summary

Prior to this change, LLVM would attempt to optimize an
aligned_alloc(33, ...) call to the stack. This flunked an assertion when
trying to emit the alloca, which crashed LLVM. Avoid that with extra
checks.

Diff Detail

Event Timeline

durin42 created this revision.Feb 11 2022, 4:15 PM
durin42 requested review of this revision.Feb 11 2022, 4:15 PM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
durin42 updated this revision to Diff 408121.Feb 11 2022, 5:08 PM
durin42 edited the summary of this revision. (Show Details)
jdoerfert accepted this revision.Feb 11 2022, 5:35 PM

LG, assuming we change the lang ref to make these alignments not UB but "ok"

This revision is now accepted and ready to land.Feb 11 2022, 5:35 PM
nikic added a subscriber: nikic.Feb 12 2022, 1:02 AM
nikic added inline comments.
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
6371–6372

It's best to avoid conversion to integer, to avoid a crash when the alignment is larger than 64-bit.

durin42 updated this revision to Diff 410063.Feb 18 2022, 7:34 PM
durin42 marked an inline comment as done.Feb 22 2022, 9:01 AM
durin42 updated this revision to Diff 410564.Feb 22 2022, 9:02 AM

LG, assuming we change the lang ref to make these alignments not UB but "ok"

I'm a little confused: isn't it fine for them to still be UB? The only point of this change was to avoid a crash in the compiler when encountering such an alignment.

jyknight accepted this revision.Feb 23 2022, 5:46 AM

It's not UB to provide invalid alignments to aligned_alloc, but that's not something the langref covers.

This change avoids using that invalid alignment on the LLVM-side, avoiding turning defined C-side invalid alignment into a crash/UB on the LLVM side.

This can be submitted first, separately from the rest of the stack.

This revision was landed with ongoing or failed builds.Feb 23 2022, 8:47 AM
This revision was automatically updated to reflect the committed changes.