- User Since
- Oct 3 2012, 3:00 AM (409 w, 3 d)
Fri, Aug 7
__libatomic_load might come at the end of the function, with no succeeding BB
Not exactly. It may come at the end of a BB.
Thu, Aug 6
Wed, Aug 5
I can push this tomorrow unless anyone objects before then.
The problem with "a" is that we do not always know what it means exactly (could be a float), and it's dangerous to dereference the pointer.
Tue, Aug 4
Mon, Aug 3
Right, that's what the isNoModRef check is for.
Sat, Aug 1
Fri, Jul 31
Testing this would be pretty hard - we'd need to touch a bunch of memory pages, and then somehow wait for the huge page daemon to merge them (or not).
You want to start scanning at the same point where poisoning is going to be inserted.
In general, this implementation looks pretty complex and easy to get wrong. I'd prefer something along the lines of AArch64StackTagging::collectInitializers - directly calculate the offset for each store/load instruction. It might do some extra work with unrelated memory instructions, but probably not too much.
Thu, Jul 30
Seems fine to me.
Wed, Jul 29
Tue, Jul 28
Mon, Jul 27
Fri, Jul 24
Does it help to replace %clangxx_msan with %clang_msan instead?
Thu, Jul 23
Do you want to use this in eager checks when passing an array as a function argument (does that even happen in C++?) ?
Wed, Jul 22
LGTM with 2 comments
The patch is missing context
Please add a test (instrumentation only, no need for a compiler-rt test).
Sure, no problem.
Thanks for the patch!
Tue, Jul 21
LGTM w/ nit
Mon, Jul 20
Please upload with context.
They are "errors reported by Control Flow Integrity" - that part is true. This is valid C++, but it triggers false positive errors in CFI.
This is not without precedent - see https://github.com/llvm/llvm-project/commit/3e58a6a7, https://github.com/llvm/llvm-project/commit/f11c00d7 and more.
The fact that CFI is stricter that the wording of the standard has been discussed many times before, but the conclusion is always that these reports are useful.
This type of false positive is not common at all outside of libc++.
Fri, Jul 17
Thu, Jul 16
There is _LIBCPP_NO_CFI, if you prefer it that way.
I'm not an expert, but I've heard that such casts are UB somewhere.
Looking at [basic.life] 6.8/6 in c++17
Mon, Jul 13
LGTM with a nit
Fri, Jul 10
Allocas are often used through bitcast or GEP, we should handle them as well.
I don't think 1.5% is good enough to introduce new runtime functions for the off-by-default code path.
Jul 8 2020
read also needs to be wrapped to do COMMON_INTERCEPTOR_UNPOISON_PARAM
This needs to include tests for bitcode writing and parsing.
Jul 7 2020
This code generates a libcall out of thin air. My intuition says the safest thing to do is to drop all call site attributes, because they generally specify something about how an attribute must be passed to the callee, and not a property of the value being passed, so there is no reason for the attribute lists on pow and on exp to have anything in common at all.
Jul 6 2020
Is unwinding actually broken on an all-clang, all-thumb system?
Jun 30 2020
Jun 29 2020
Jun 26 2020
Oh, it would be great to run libc++ tests with CFI on the buildbot, but it may be complicated, as CFI requires LTO.
This change look right to me, but I defer to libc++ maintainers.
Jun 24 2020
Jun 23 2020
The sanitizer_common test seems excessive, but I'm inclined to take it anyway simply because it costs us very little to have it there.
Also, what's the plan to detect these cases in ubsan?
I don't think this has any practical impact on our goals with sanitizers. We should detect undefined behavior before it gets to the point of actually passing or returning an undef or poison value.