UBSan has an object size check (-fsanitize=object-size) which can determine when an object is not large enough to represent a value of its type. The check uses the @llvm.objectsize intrinsic to determine the size of objects.
AFAICT, and please let me know if I’ve missed something here, @llvm.objectsize always conservatively returns “I don’t know” (i.e -1) at -O0, which means that it can’t catch any issues at -O0. This is a problem because there is a substantial compile-time cost to enabling the object size check in debug builds. It seems unlikely that we can make @llvm.objectsize more precise at -O0 without regressing compile time and the debugging experience in other ways.
So, I’m proposing that we disable ubsan’s object size check at -O0. This will speed up debug builds without compromising on diagnostic quality. E.g I measured a 26% decrease in the compile time for X86FastISel.cpp with this change, and a 32% decrease in the *.o size:
No ubsan [1]
Average compile time: 5.27 s
X86FastISel.cpp.o size: 3.06 MB
Ubsan [2]
Average compile time: 9.49 s
X86FastISel.cpp.o size: 8.93 MB
Ubsan without the object size check [3]
Average compile time: 6.99 s
X86FastISel.cpp.o size: 6.06 MB
There’s reason to expect similar compile-time / binary size savings with other *.cpp files. The object size check is in the same category of checks as the null check and the alignment check. This group of checks accounts for the vast majority of checks inserted by ubsan (over 90% in some macOS apps), so any savings here would be helpful.
AFAICT, and please let me know if I've missed something here, @llvm.objectsize always conservatively returns "I don't know" (i.e -1) at -O0
Yup. The object-size sanitizer lowers directly to @llvm.objectsize
(bypassing clang's __builtin_object_size evaluator, which is the
right thing to do here), and the only places we try to lower @llvm.objectsize to a non-default value, to my knowledge, are
in InstCombine and CodeGenPrepare. Looks like both of those
are disabled on -O0.
As you mentioned, if we did decide that we should try to
lower @llvm.objectsize calls more aggressively in -O0, I'd
imagine we'd still end up handing back -1 in ~99% of cases.
(With the remaining 1% looking something like:
struct Foo {int i;};
char c;
int bar() { return reinterpret_cast<Foo *>(&c)->i; } // error: c is
too small, ...
)
So, unless I'm missing something, I think this is a pure win for
ubsan in -O0. I'm in favor of it.
George Burgess IV via cfe-dev <cfe-dev@lists.llvm.org> writes:
Hi,
AFAICT, and please let me know if I've missed something here, @llvm.objectsize always conservatively returns "I don't know" (i.e
-1) at -O0
Yup. The object-size sanitizer lowers directly to @llvm.objectsize
(bypassing clang's __builtin_object_size evaluator, which is the
right thing to do here), and the only places we try to lower @llvm.objectsize to a non-default value, to my knowledge, are
in InstCombine and CodeGenPrepare. Looks like both of those
are disabled on -O0.
As you mentioned, if we did decide that we should try to
lower @llvm.objectsize calls more aggressively in -O0, I'd
imagine we'd still end up handing back -1 in ~99% of cases.
(With the remaining 1% looking something like:
struct Foo {int i;};
char c;
int bar() { return reinterpret_cast<Foo *>(&c)->i; } // error: c is
too small, ...
)
How reasonable would it be to do something simpler that catches this in
particular? Though, ISTM this is probably obvious enough that we could
catch it at compile time.
AFAICT, and please let me know if I’ve missed something here, @llvm.objectsize always conservatively returns “I don’t know” (i.e
-1) at -O0
Yup. The object-size sanitizer lowers directly to @llvm.objectsize
(bypassing clang’s __builtin_object_size evaluator, which is the
right thing to do here), and the only places we try to lower @llvm.objectsize to a non-default value, to my knowledge, are
in InstCombine and CodeGenPrepare. Looks like both of those
are disabled on -O0.
As you mentioned, if we did decide that we should try to
lower @llvm.objectsize calls more aggressively in -O0, I’d
imagine we’d still end up handing back -1 in ~99% of cases.
(With the remaining 1% looking something like:
struct Foo {int i;};
char c;
int bar() { return reinterpret_cast<Foo *>(&c)->i; } // error: c is
too small, …
)
How reasonable would it be to do something simpler that catches this in
particular? Though, ISTM this is probably obvious enough that we could
catch it at compile time.
It would be feasible to warn at compile-time for simple cases like this one, or the one in [1]. We currently don’t: at least, not in the static analyzer or with -Wall -Wextra.
That said, I’m not sure that this “simple” type of violation is commonplace. All of the -fsanitize=object-size violations I’ve seen involve either heap allocation or cross-TU calls. I imagine that these bugs would be hard to diagnose at compile-time.