This is a compiler-rt part.
The clang part is D50250.
See PR21530, https://github.com/google/sanitizers/issues/940.
Paths
| Differential D50251
[compiler-rt][ubsan] Implicit Conversion Sanitizer - integer sign change - compiler-rt part ClosedPublic Authored by lebedev.ri on Aug 3 2018, 6:22 AM.
Details
Summary This is a compiler-rt part. See PR21530, https://github.com/google/sanitizers/issues/940.
Diff Detail
Event TimelineComment Actions Ping. Comment Actions Rebased. lebedev.ri changed the repository for this revision from rCTE Clang Tools Extra to rCRT Compiler Runtime.Oct 24 2018, 12:32 PM Comment Actions I have minor questions in inline comments, but sorry, I'll request a big change :-/ Please match the types of casts and parameters. You're often matching uint32_t to unsigned int (also a signed char with int8_t (not as problematic here, though), etc). We don't have a guarantee, and the "assuming int is 32-bit" part might cause problems eventually. I'm ok with either version:
Otherwise, this looks good. Thank you, P.S: Yay, I managed not to ask about the ubsan-standalone feature! (sorry about that in the earlier reviews :-) )
Comment Actions
Yay, thank you for the feedback!
:)
lebedev.ri marked an inline comment as done. Comment ActionsRebased, changed types in ImplicitSignedIntegerTruncationOrSignChangeTest fuzzer test. This revision is now accepted and ready to land.Oct 30 2018, 11:59 AM Closed by commit rCRT345659: [compiler-rt][ubsan] Implicit Conversion Sanitizer - integer sign change… (authored by lebedevri). · Explain WhyOct 30 2018, 3:02 PM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 171813 lib/ubsan/ubsan_checks.inc
lib/ubsan/ubsan_handlers.h
lib/ubsan/ubsan_handlers.cc
test/fuzzer/ImplicitIntegerSignChangeTest.cpp
test/fuzzer/ImplicitSignedIntegerTruncationOrSignChangeTest.cpptest/fuzzer/fuzzer-implicit-integer-sign-change.test
test/fuzzer/fuzzer-implicit-signed-integer-truncation-or-sign-change.test
test/ubsan/TestCases/ImplicitConversion/integer-arithmetic-value-change.c
test/ubsan/TestCases/ImplicitConversion/integer-conversion.c
test/ubsan/TestCases/ImplicitConversion/integer-sign-change-blacklist.c
test/ubsan/TestCases/ImplicitConversion/integer-sign-change-summary.cpp
test/ubsan/TestCases/ImplicitConversion/integer-sign-change.c
test/ubsan/TestCases/ImplicitConversion/signed-integer-truncation-or-sign-change-blacklist.c
test/ubsan/TestCases/ImplicitConversion/signed-integer-truncation-or-sign-change-summary.cpp
test/ubsan_minimal/TestCases/implicit-integer-sign-change.c
test/ubsan_minimal/TestCases/implicit-signed-integer-truncation-or-sign-change.c
|
This means *both* errors got triggered, right? Or can we have this one with just one being triggered?
If it's always both, I'd rather have an And instead of Or in its name.