This is an archive of the discontinued LLVM Phabricator instance.

[ubsan] Minimize size of data for type_mismatch
AbandonedPublic

Authored by filcab on Apr 28 2016, 9:38 AM.

Details

Summary

This patch makes the type_mismatch static data 7 bytes smaller (and it ends up
being 16 bytes smaller due to alignment restrictions, at least on some x86-64
environments).

Currently, I have no binary compatibility for using an older object file
(compiled before this change) with a newer clang. Depending on the compiler-rt
code review, this patch might be updated to better signal to the library that
this object has this new binary format.

Diff Detail

Event Timeline

filcab updated this revision to Diff 55428.Apr 28 2016, 9:38 AM
filcab retitled this revision from to [ubsan] Minimize size of data for type_mismatch.
filcab updated this object.
filcab added reviewers: kcc, samsonov, rsmith.
filcab added a subscriber: cfe-commits.
bcraig added a subscriber: bcraig.Apr 28 2016, 10:11 AM

Is there an associated patch on the consuming side for this data?

http://reviews.llvm.org/D19668 is the compiler-rt side of the patch.

rsmith accepted this revision.Apr 28 2016, 4:12 PM
rsmith edited edge metadata.
This revision is now accepted and ready to land.Apr 28 2016, 4:12 PM
filcab updated this revision to Diff 56261.May 5 2016, 4:07 AM
filcab edited edge metadata.

Update catch-undef-behavior.c

Hi Richard. Just want to double-check something. If we have no alignment value, for this check, can we assume 1?
It seems to me that that shouldn't be a problem, but wanted to double-check.

lib/CodeGen/CGExpr.cpp
587

Would it be acceptable to set AlignVal to 1 if it's 0?

rsmith added inline comments.May 13 2016, 4:50 PM
lib/CodeGen/CGExpr.cpp
587

Yes, that seems fine. The alignment check will never fire if AlignVal is zero (that only happens if the type in question is incomplete, and we don't emit a check for it in that case). Setting it to 1 should prevent the runtime library from thinking that we detected an alignment problem.

It looks like the current patch will emit 255 as the alignment in that case; that also seems fine as a way of indicating "no alignment check". I doubt we'll ever need to deal with a real alignment value of 2^255 :)

filcab updated this revision to Diff 57342.May 16 2016, 6:07 AM

Minor update which never sets an AlignVal to 0.

filcab updated this revision to Diff 57350.May 16 2016, 7:40 AM

Added a version field.

filcab abandoned this revision.Dec 15 2016, 7:28 AM

I will post a new patch, using the features from rL289444.