- User Since
- Jun 9 2020, 10:09 AM (9 w, 2 d)
Tue, Aug 11
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.
Mon, Aug 10
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. For handling number of uses per stack trace, this uses a bit of a heuristic:
Fri, Aug 7
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).
Thu, Aug 6
Modified to just use __sanitizer::mem_is_zero
Wed, Aug 5
LGTM. Does this support the a format specifier as well, or just m?
Tue, Aug 4
Integrated with Alias Analyzer, uses simpler mechanism for walking through BB and determining stores to alloca
Mon, Aug 3
Fri, Jul 31
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.
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
Thu, Jul 30
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?
Wed, Jul 29
I agree, this probably isn't going to be worth it.
Updated comment on disable-noundef-args option
Tue, Jul 28
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
Mon, Jul 27
Fri, Jul 24
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.
Thu, Jul 23
I don't believe this happens in C++ but wanted to add it for completeness's sake (or if some other LLVM based language might use it).
@amyk Sorry about that! The issue should be fixed now.
Wed, Jul 22
Added a test case
Just saw your comment about tests as well. The idea was to have all tests ported over as part of a separate commit (I linked it in the main patch description) and then only to push either commit once both are ready to land.
Adds additional constraints on noundef: Not a nullptr_t, not a member pointer, and not coerced to a type of larger size. Disabled emitting in return position for non-C++ languages (or inside extern "C").
Reuploaded with context
Sorry about that. Reuploaded with full context.
Kept only core of patch: adding new LIT configration for eager-checks
This patch actually has no dependency on clang emitting noundef, so it should be good to go.