User Details
- User Since
- Jun 9 2020, 10:09 AM (31 w, 4 d)
Dec 9 2020
This whole thing is a little unfortunate, but maybe a better substitution would be leaving %clang as referring to the pure clang binary, with default arguments. Then we can have a %clang_cc which may only be used for a standard C compiler invocation.
Oct 16 2020
Did we decide that we wanted this change then? I remember there being discussion around whether it's the right approach.
Sep 22 2020
Sorry about the mix-up. This seems like exactly what we should do to clean up here.
Aug 23 2020
@rjmccall We've discussed several different possibilities here. Does any of them strike you as a good step forward here?
LGTM, although I'm just curious about the omissions here. why not mark something like round noundef?
Aug 22 2020
Aug 14 2020
Emit a nop instruction to always mark the end of the MSan prologue, and insert prologue instructions before that.
Move removeUnreachableBlocks before inserting the prologue. This makes sure there's no issue with the ActualFnStart instruction being deleted before the visitor loop.
Committed as 97de0188dd5d845ff90c8ac779a2ea09688b17df
Remove BB splitting for KMSAN
Added a test for hash codegen.
Updated to depend on https://reviews.llvm.org/D85985 (ActualFnStart becomes a Instruction *)
Addressed comments
Aug 12 2020
Aug 11 2020
Made the compiler flag non-public
Allows passing different extra arguments for different clang expansions
Added willreturn. I think nocapture is probably best left to a future change since it's more invasive -- it would require keeping tabs of which arguments to each function is a pointer arg.
I think I'd like someone to take a look at the llvm-lit changes to see if this makes sense as an approach
To try to alleviate the tests issue, @eugenis and I discussed that it might be best to take it slow. So now this patch will mask off emitting the attribute on clang tests by default.
After discussing with @eugenis, for the meantime it might be best to do the following:
- Change the masking attribute to be -fdisable-noundef-analysis (name notwithstanding), and have it completely turn off all noundefs
- Change the llvm-lit configuration to use the new codegen flag for all the tests by default
- Have noundef emitted in the frontend by default (when the codegen flag isn't present)
LGTM, but I don't really maintain this code.
Aug 10 2020
I don't think we can necessarily guarantee argmemonly/readonly/writeonly, particularly since these library calls can take a lock somewhere inside. I'll definitely add nocapture and willreturn though.
Addressed comments
Addressed comments. For handling number of uses per stack trace, this uses a bit of a heuristic:
Aug 7 2020
Separated out the frontend change. Addressed other comments.
Rebased on master (again)
Rebased on master
Simplified by returning to the old implementation, but having libatomic calls made nounwind (so we never see them as invokes).
Aug 6 2020
Modified to just use __sanitizer::mem_is_zero
Most of the discussion has been in https://reviews.llvm.org/D81678 (implementing the attribute in clang) and https://reviews.llvm.org/D82316 (adding the attribute to langref).
Aug 5 2020
LGTM. Does this support the a format specifier as well, or just m?
Aug 4 2020
LGTM
Integrated with Alias Analyzer, uses simpler mechanism for walking through BB and determining stores to alloca
Aug 3 2020
Jul 31 2020
It seems like collectInitializers leans heavily on the isPointerOffset function, which returns an offset if two pointers have a constant difference, nullopt if they don't. The problem here is that we can't distinguish isPointerOffset == nullopt happening because the offset is determined at runtime, or because the two pointers are completely unrelated.
Updated.
Oh, wait, the patch didn't update. One sec.
Fixed to actually set the shadow and origin. Sorry for the mix-up!
Rebased; all tests passing again. Removed the change to the ppc-*mmintrin.c tests, instead I just use the -disable-noundef-args flag`. Cleaned up typos.
Oh! I need to test that it's not checked as well.
Flattened some control flow, updated to properly use StoreOffs, and updated tests to cover chained GEPs
Jul 30 2020
I've cut it down to only within a basic block as @eugenis and @vitalybuka suggested.
This actually has very significant effects on some, but not all, benchmarks.
@jdoerfert what would the procedure be for reviewing these test changes / getting this landed with the noundef patch?
Jul 29 2020
I agree, this probably isn't going to be worth it.
Updated comment on disable-noundef-args option
Addressed comments
Jul 28 2020
All tests up to date. Of particular note are the ppc-*mmintrin.c tests, which seemed to drastically change upon rerunning the test autogen script.
Fix typo in MayDropFunctionReturn
Fixes regression; allows emitting noundef for non-FunctionDecls as well.
Incorporate C++'s more conservative checks for omitted return values
LGTM. Thanks!
Jul 27 2020
LGTM
Jul 24 2020
Added an across-the-board test with several different interesting cases. @rsmith, I'm not sure how to test things like VTables/VTTs, since afaik the way they would be exposed to function attributes would be if a struct is being passed by value. But in that case, we currently never emit noundef.
Update tests to reflect more strict noundef rules
I think we can actually just drop the #include <stdatomic.h> line here. It was used for a previous version of the test but I forgot to remove it after some changes.